Skip to content

feat(op-revm): add Soul Gas Token (SGT) support for OP Stack#1

Open
blockchaindevsh wants to merge 9 commits intoop-esfrom
op-revm-sgt-support
Open

feat(op-revm): add Soul Gas Token (SGT) support for OP Stack#1
blockchaindevsh wants to merge 9 commits intoop-esfrom
op-revm-sgt-support

Conversation

@blockchaindevsh
Copy link
Copy Markdown
Collaborator

Implements SGT-aware gas payment for OP Stack chains:

  • SGT balance reading from predeploy contract (0x4200...0800)
  • Deduction priority: SGT first, then native balance
  • Refund priority: native first, then SGT (reverse order)
  • Full test coverage with op-acceptance-tests
  • Preserves L1BlockInfo fetch behavior with SGT config

Implements SGT-aware gas payment for OP Stack chains:
- SGT balance reading from predeploy contract (0x4200...0800)
- Deduction priority: SGT first, then native balance
- Refund priority: native first, then SGT (reverse order)
- Full test coverage with op-acceptance-tests
- Preserves L1BlockInfo fetch behavior with SGT config

Co-authored-by: Claude Code <noreply@anthropic.com>
Move sgt_enabled and sgt_is_native_backed from L1BlockInfo to CfgEnv,
accessed via Cfg trait methods (is_sgt_enabled, is_sgt_native_backed).

This eliminates the need to:
- Mutate the EVM after creation (no configure_sgt on Evm trait)
- Preserve/restore SGT flags around L1BlockInfo::try_fetch
- Fork alloy-evm

SGT config now flows through CfgEnv at EVM creation time, matching
how OpSpecId and other hardfork config is passed.

Runtime deduction tracking (sgt_native_deducted, sgt_amount_deducted)
remains on L1BlockInfo as it is per-transaction mutable state.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
if let Ok(state_load) = journal.sload(SGT_CONTRACT, sgt_slot.into()) {
let sgt_balance = state_load.data;
let new_sgt = sgt_balance.saturating_sub(amount);
let _ = journal.sstore(SGT_CONTRACT, sgt_slot.into(), new_sgt);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this OK to ignore errors?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good point, fixed, and also fixed several other places.

…f silently ignoring

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
let native_to_deduct = total_cost.saturating_sub(sgt_to_deduct);

// Store deduction amounts for refund calculation
chain.sgt_amount_deducted = sgt_to_deduct;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should reset the sgt state when the transaction finishes.

Copy link
Copy Markdown
Collaborator Author

@blockchaindevsh blockchaindevsh Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e78df8a .


// Deduct native portion
let Some(new_balance) = balance.checked_sub(native_to_deduct) else {
return Err(InvalidTransaction::LackOfFundForMaxFee {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line cannot be reached because the previous check 'total balance (native + SGT) >= total_cost' ensures that 'balance >= native_to_deduct'.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e78df8a .

blockchaindevsh and others added 2 commits March 31, 2026 22:22
…ve unreachable check

Reset sgt_native_deducted and sgt_amount_deducted in clear_tx_l1_cost()
to prevent stale values persisting across transactions within a block.
Remove unreachable checked_sub since the prior balance validation
guarantees native_to_deduct <= balance.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When is_native_backed is true, deducting/adding SGT balance must also
update the SGT contract's native account balance, matching op-geth's
subSoulBalance/addSoulBalance behavior.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When SGT is enabled and not native-backed, fee recipients should only
receive the native portion of fees — the SGT portion is burned. This
matches op-geth's collectNativeBalance behavior.

- Add collect_native_balance to sgt.rs that splits fees between SGT
  (burned) and native (paid) pools
- Change reward_beneficiary return type to Result<U256> so the OP
  handler can capture the coinbase fee amount
- Apply collect_native_balance to coinbase fee and all OP-specific
  fees (L1, base fee, operator fee)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@blockchaindevsh
Copy link
Copy Markdown
Collaborator Author

The commit b1a98a9 implements the fee-burning logic for non-native-backed SGT, matching op-geth's
collectNativeBalance behavior.

What it does: When SGT is enabled and NOT native-backed, fee recipients (coinbase, L1 fee vault, base fee vault, operator fee vault) should only receive the native portion of fees.
The SGT portion is burned (not credited to anyone).

Breaking change: The post_execution::reward_beneficiary free function now returns Result<U256, ...> instead of Result<(), ...>. Any external code calling this function directly will need to handle or discard the returned value. The Handler::reward_beneficiary trait method signature is unchanged (still returns Result<(), Self::Error>), so
implementations of the Handler trait are not affected.

Update sgt_native_deducted and sgt_amount_deducted in place during
reimburse_caller_sgt, matching op-geth's deductGasFrom which mutates
usedNativeBalance/usedSGTBalance in place. This ensures
reward_beneficiary sees post-refund pool amounts when calling
collect_native_balance.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@syntrust
Copy link
Copy Markdown

syntrust commented Apr 2, 2026

Any unit tests for the reward_beneficiary part?

@blockchaindevsh
Copy link
Copy Markdown
Collaborator Author

Any unit tests for the reward_beneficiary part?

The original sgt e2e tests are ported over, we can add more cases there if necessary.

/// Last calculated l1 fee cost. Uses as a cache between validation and pre execution stages.
pub tx_l1_cost: Option<U256>,
/// Amount of native balance deducted for SGT gas payment (for refund calculation)
pub sgt_native_deducted: U256,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cargo test -p op-revm --lib failed

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 14c22d0.

/// Amount of native balance deducted for SGT gas payment (for refund calculation)
pub sgt_native_deducted: U256,
/// Amount of SGT balance deducted for gas payment (for refund calculation)
pub sgt_amount_deducted: U256,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It’s a bit weird to add SGT-related variables to the block info struct—it seems unrelated.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

// Read SGT balance (requires dropping caller_account to release journal borrow)
drop(caller_account);

let sgt_balance = read_sgt_balance(journal, tx.caller())?;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment from Codex:

One high-priority concern is that the SGT balance read/write path is using journal in the pre-execution deduction hook, before opcode execution begins.

To be precise, Handler::pre_execution() does not call SGT helpers directly, but it first calls self.validate_against_state_and_deduct_caller(...), and op-revm overrides that hook. In the SGT branch of that override, we call read_sgt_balance() / deduct_sgt_balance(), which currently go through journal.load_account / sload / sstore.

The problem is that these are metering-aware journal APIs: they can mark accounts and storage slots as warm. That is fine for protocol-required warming (e.g. coinbase, access list, precompiles) or for the sender account, but the SGT predeploy account and the caller’s SGT balance slot are not part of the initial warm set. As a result, this code may warm them before execution. If the transaction later touches the same contract/slot again, the first access can be charged as warm instead of cold, which diverges from op-geth.

So the issue is not just an implementation detail. It can change gas metering, and in edge cases can also change execution outcomes (for example, whether a transaction runs out of gas).

Copy link
Copy Markdown
Collaborator Author

@blockchaindevsh blockchaindevsh Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, fixed in bb81552, basically we use no-warm variant for sgt feature.

SGT balance operations (read, deduct, add) use journal APIs that mark
accounts and storage slots as warm. This causes the SGT predeploy and
its balance slots to appear warm during EVM execution, diverging from
op-geth where GetState/SetState don't affect the EIP-2929 access list.

Add `no_warm` parameter to `load_account_mut_optional`,
`sload_concrete_error`, and `sstore_concrete_error` that skips warming
side-effects (mark_warm_with_transaction_id, account_warmed/
storage_warmed journal entries) while still loading state and computing
is_cold accurately.

Expose `_no_warm` convenience methods on JournalTr (load_account_no_warm,
sload_no_warm, sstore_no_warm, load_account_mut_no_warm) and update all
SGT functions to use them.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
}
}
slot.mark_warm_with_transaction_id(self.transaction_id);
if !no_warm {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the no_warm fix is still incomplete at both the account level and the storage-slot level.

There are still two places where warming state leaks through:

  1. Account path (load_account_mut_optional)
    In the Vacant branch, a newly loaded account is inserted into journal state with the current transaction_id, but it is not kept cold when no_warm = true. As a result, a later normal access in the same transaction can observe the account as already warm, even though this protocol-level read was supposed to have no EIP-2929 side effects.

  2. Storage path (sload_concrete_error)
    In the Vacant branch, a newly loaded slot is inserted with EvmStorageSlot::new(value, self.transaction_id). That constructor sets transaction_id to the current tx and is_cold = false, so even when no_warm = true, the slot is effectively cached as warm for subsequent accesses in the same transaction.

Also, ENTRY::storage_warmed(self.address, key) is still pushed whenever is_cold is true, even under no_warm = true, so the function still records a warming side effect in the journal.

So the current change only avoids the explicit mark_warm_with_transaction_id(...) calls, but it does not fully preserve “no warming side effects” semantics.

I think no_warm should also:

  • keep newly inserted cold accounts cold in the Vacant account path,
  • keep newly inserted cold slots cold in the Vacant storage path, and
  • skip pushing warming journal entries when no_warm = true.

/// Used for protocol-level operations (e.g., SGT gas payment) that should not
/// influence EIP-2929 gas metering during execution. Still journals the storage
/// change so reverts work correctly.
fn sstore_no_warm(
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change adds new required methods to the JournalTr trait (sload_no_warm, sstore_no_warm, load_account_mut_no_warm, load_account_no_warm), but the repo still has another impl JournalTr that was not updated: examples/cheatcode_inspector/src/main.rs.

As a result, this is currently source-breaking for the workspace and cargo test --workspace fails with missing trait items for Backend.

I think this needs either:

  • updating all in-repo JournalTr implementations together, or
  • providing default trait implementations if the intent is to extend the interface in a non-breaking way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants