Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 32 additions & 7 deletions crates/vm/levm/src/opcode_handlers/frame_tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,14 @@ pub(crate) fn compute_tx_max_cost(ctx: &crate::vm::FrameTxContext) -> Result<U25

/// Apply APPROVE side effects for the given scope.
/// This is shared between OpApproveHandler and (future) default code.
/// Returns the EIP-8250 first-use surcharge the approval incurred; the caller charges it to the
/// approving frame, whose gas budget it comes out of.
pub fn apply_approve(
vm: &mut VM<'_>,
scope: u64,
frame_target: ethrex_common::Address,
) -> Result<(), VMError> {
) -> Result<u64, VMError> {
let mut surcharge = 0;
match scope {
0x1 => {
// APPROVE_PAYMENT: increment nonce, deduct max cost, record payer.
Expand Down Expand Up @@ -107,7 +110,7 @@ pub fn apply_approve(
let tx_cost = compute_tx_max_cost(ctx)?;
let sender = ctx.tx.sender;

vm.consume_keyed_nonces(sender)?;
surcharge = vm.consume_keyed_nonces(sender)?;
// Payer balance underflow is a frame-level revert, not a consensus
// fault: the outer restore_cache_state() path rolls back the nonce
// increment above when RevertOpcode propagates.
Expand Down Expand Up @@ -162,7 +165,7 @@ pub fn apply_approve(
let tx_cost = compute_tx_max_cost(ctx)?;
let sender = ctx.tx.sender;

vm.consume_keyed_nonces(sender)?;
surcharge = vm.consume_keyed_nonces(sender)?;
// See scope 0x1 above for the Underflow → RevertOpcode rationale.
match vm.decrease_account_balance(frame_target, tx_cost) {
Ok(()) => {}
Expand All @@ -182,7 +185,7 @@ pub fn apply_approve(
return Err(ExceptionalHalt::InvalidOpcode.into());
}
}
Ok(())
Ok(surcharge)
}

/// APPROVE (0xAA) -- Frame transaction approval opcode.
Expand Down Expand Up @@ -251,7 +254,22 @@ impl OpcodeHandler for OpApproveHandler {
vm.current_call_frame.memory.len(),
)?)?;

apply_approve(vm, scope_val, frame_target)?;
let approval_snapshot = vm
.frame_tx_context
.as_ref()
.map(|ctx| (ctx.payer_address, ctx.sender_approved));
let surcharge = apply_approve(vm, scope_val, frame_target)?;
// The approval context is not database-backed, so the frame rollback that undoes the debit
// and the nonce consumption would leave the payer recorded and refunded at end of tx.
if let Err(err) = vm.current_call_frame.increase_consumed_gas(surcharge) {
if let (Some((payer, sender_approved)), Some(ctx)) =
(approval_snapshot, vm.frame_tx_context.as_mut())
{
ctx.payer_address = payer;
ctx.sender_approved = sender_approved;
}
return Err(err.into());
}

let ctx = vm
.frame_tx_context
Expand Down Expand Up @@ -862,15 +880,22 @@ fn execute_default_verify(
return Ok((false, 0, Vec::new()));
}

apply_approve(vm, allowed_scope, target)?;
// The surcharge comes out of this frame's budget like any other gas, so a frame that cannot
// afford it approves nothing. The affordability gate has to run before any state is touched,
// hence the estimate here; what is charged below is what consumption actually took.
if vm.keyed_nonce_first_use_surcharge()? > frame.gas_limit {
return Ok((false, frame.gas_limit, Vec::new()));
}

let charged = apply_approve(vm, allowed_scope, target)?;

let ctx = vm
.frame_tx_context
.as_mut()
.ok_or(ExceptionalHalt::InvalidOpcode)?;
ctx.approve_called_in_current_frame = true;

Ok((true, 0, Vec::new()))
Ok((true, charged, Vec::new()))
}

#[cfg(test)]
Expand Down
47 changes: 38 additions & 9 deletions crates/vm/levm/src/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -908,6 +908,18 @@ pub fn find_batch_end(frames: &[Frame], failed_idx: usize) -> usize {
.unwrap_or(failed_idx)
}

/// The EIP-8250 nonce manager storage slot holding the sequence number of
/// `(sender, key)`: `keccak256(zeros(12) || sender || key)`.
///
/// The pre-check that prices a first use and the consumption that performs it
/// must address the same slot, so both derive it here.
fn keyed_nonce_slot(sender: Address, key: &U256) -> H256 {
let mut preimage = [0u8; 64];
preimage[12..32].copy_from_slice(sender.as_bytes());
preimage[32..64].copy_from_slice(&key.to_big_endian());
H256(ethrex_crypto::keccak::keccak_hash(preimage))
}

/// EIP-7906: install the transaction prestate map on `db` for `tx`, or clear it.
///
/// The map is needed only by transactions that can execute TXTRACE /
Expand Down Expand Up @@ -1662,31 +1674,28 @@ impl<'a> VM<'a> {
/// EIP-8250's strict "consumption MUST NOT be reverted by an atomic-batch
/// snapshot" durability is tracked for devnet/interop validation — see
/// `docs/eip-8250.md`. Key-0 consumption matches existing EIP-8141 behaviour.
pub(crate) fn consume_keyed_nonces(&mut self, sender: Address) -> Result<(), VMError> {
pub(crate) fn consume_keyed_nonces(&mut self, sender: Address) -> Result<u64, VMError> {
let (nonce_keys, next_seq) = match &self.tx {
Transaction::FrameTransaction(ft) => (
ft.nonce_keys.clone(),
ft.nonce_seq
.checked_add(1)
.ok_or(VMError::Internal(InternalError::Overflow))?,
),
_ => return Ok(()),
_ => return Ok(0),
};
let nonce_manager = ethrex_common::types::frame_tx_nonce_manager();
let mut surcharge: u64 = 0;
for key in &nonce_keys {
if key.is_zero() {
self.increment_account_nonce(sender)?;
continue;
}
let mut preimage = [0u8; 64];
preimage[12..32].copy_from_slice(sender.as_bytes());
preimage[32..64].copy_from_slice(&key.to_big_endian());
let slot = H256(ethrex_crypto::keccak::keccak_hash(preimage));
let slot = keyed_nonce_slot(sender, key);
let _ = self.db.get_account(nonce_manager)?;
let current = self.get_storage_value(nonce_manager, slot)?;
if current.is_zero() {
self.current_call_frame
.increase_consumed_gas(crate::gas_cost::KEYED_NONCE_FIRST_USE_GAS)?;
surcharge = surcharge.saturating_add(crate::gas_cost::KEYED_NONCE_FIRST_USE_GAS);
}
let slot_u256 = U256::from_big_endian(&slot.0);
self.update_account_storage(
Expand All @@ -1697,7 +1706,27 @@ impl<'a> VM<'a> {
current,
)?;
}
Ok(())
Ok(surcharge)
}

/// The surcharge `consume_keyed_nonces` will charge, without consuming anything.
pub(crate) fn keyed_nonce_first_use_surcharge(&mut self) -> Result<u64, VMError> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This reimplements the slot derivation from consume_keyed_nonces character-for-character - same [0u8; 64] preimage, same [12..32]/[32..64] layout, same keccak_hash. Two copies of a consensus-critical derivation that must agree: if either drifts, the pre-check prices a different slot than the one actually consumed, and the default-code path charges a surcharge unrelated to what it did.

Worth collapsing to one private helper, something like fn keyed_nonce_slot(sender: Address, key: &U256) -> H256, called from both. Cheap now, and it makes the invariant structural instead of something a future edit has to remember.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Collapsed into keyed_nonce_slot(sender, key), called from both consume_keyed_nonces and keyed_nonce_first_use_surcharge.

let (sender, nonce_keys) = match &self.tx {
Transaction::FrameTransaction(ft) => (ft.sender, ft.nonce_keys.clone()),
_ => return Ok(0),
};
let nonce_manager = ethrex_common::types::frame_tx_nonce_manager();
let mut surcharge: u64 = 0;
for key in &nonce_keys {
if key.is_zero() {
continue;
}
let slot = keyed_nonce_slot(sender, key);
if self.get_storage_value(nonce_manager, slot)?.is_zero() {
surcharge = surcharge.saturating_add(crate::gas_cost::KEYED_NONCE_FIRST_USE_GAS);
}
}
Ok(surcharge)
}

fn execute_frame_tx(&mut self) -> Result<ExecutionReport, VMError> {
Expand Down
Loading