Skip to content
This repository has been archived by the owner on Jul 5, 2024. It is now read-only.

Commit

Permalink
bus-mapping: improve self-destruct (#1613)
Browse files Browse the repository at this point in the history
### Description

resolves
#1612

### Issue Link

[_link issue here_]

### Type of change

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] This change requires a documentation update

### Contents

- [_item_]

### Rationale

[_design decisions and extended information_]

### How Has This Been Tested?

tested with thousands of mainnet blocks (in scroll fork mainnet.rs tool)
  • Loading branch information
lispc authored Sep 19, 2023
1 parent 9867bf3 commit 86c35ba
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 12 deletions.
20 changes: 16 additions & 4 deletions bus-mapping/src/circuit_input_builder/input_state_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -370,9 +370,14 @@ impl<'a> CircuitInputStateRef<'a> {
field: AccountField,
value: Word,
value_prev: Word,
reversible: bool,
) -> Result<(), Error> {
let op = AccountOp::new(address, field, value, value_prev);
self.push_op(step, RW::WRITE, op);
if reversible {
self.push_op_reversible(step, op)?;
} else {
self.push_op(step, RW::WRITE, op);
}
Ok(())
}

Expand Down Expand Up @@ -611,14 +616,15 @@ impl<'a> CircuitInputStateRef<'a> {
)
}

/// Transfer to an address irreversibly.
pub fn transfer_to_irreversible(
/// Transfer to an address. Create an account if it is not existed before.
pub fn transfer_to(
&mut self,
step: &mut ExecStep,
receiver: Address,
receiver_exists: bool,
must_create: bool,
value: Word,
reversible: bool,
) -> Result<(), Error> {
// If receiver doesn't exist, create it
if (!receiver_exists && !value.is_zero()) || must_create {
Expand All @@ -628,6 +634,7 @@ impl<'a> CircuitInputStateRef<'a> {
AccountField::CodeHash,
CodeDB::empty_code_hash().to_word(),
Word::zero(),
reversible,
)?;
}
if value.is_zero() {
Expand All @@ -643,6 +650,7 @@ impl<'a> CircuitInputStateRef<'a> {
AccountField::Balance,
receiver_balance,
receiver_balance_prev,
reversible,
)?;

Ok(())
Expand Down Expand Up @@ -1186,7 +1194,11 @@ impl<'a> CircuitInputStateRef<'a> {
} else {
0
};
geth_step.gas - memory_expansion_gas_cost - code_deposit_cost
let constant_step_gas = match geth_step.op {
OpcodeId::SELFDESTRUCT => geth_step.gas_cost,
_ => 0, // RETURN/STOP/REVERT have no "constant_step_gas"
};
geth_step.gas - memory_expansion_gas_cost - code_deposit_cost - constant_step_gas
};

let caller_gas_left = if is_revert_or_return_call_success || call.is_success {
Expand Down
51 changes: 44 additions & 7 deletions bus-mapping/src/evm/opcodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ use crate::{
circuit_input_builder::{CircuitInputStateRef, ExecState, ExecStep},
error::{DepthError, ExecError, InsufficientBalanceError, NonceUintOverflowError, OogError},
evm::OpcodeId,
operation::TxAccessListAccountOp,
operation::{AccountField, AccountOp, TxAccessListAccountOp},
Error,
};
use core::fmt::Debug;
use eth_types::{evm_unimplemented, GethExecStep, ToAddress};
use eth_types::{evm_unimplemented, GethExecStep, ToAddress, ToWord, Word};

mod address;
mod balance;
Expand Down Expand Up @@ -447,23 +447,60 @@ fn dummy_gen_selfdestruct_ops(
},
)?;

let (found, _) = state.sdb.get_account(&receiver);
let (found, receiver_account) = state.sdb.get_account(&receiver);
if !found {
return Err(Error::AccountNotFound(receiver));
}
let receiver_account = &receiver_account.clone();
let (found, sender_account) = state.sdb.get_account(&sender);
if !found {
return Err(Error::AccountNotFound(sender));
}
let sender_account = &sender_account.clone();
let value = sender_account.balance;
// NOTE: In this dummy implementation we assume that the receiver already
// exists.
state.transfer(&mut exec_step, sender, receiver, true, false, value)?;

state.push_op_reversible(
&mut exec_step,
AccountOp {
address: sender,
field: AccountField::Balance,
value: Word::zero(),
value_prev: value,
},
)?;
state.push_op_reversible(
&mut exec_step,
AccountOp {
address: sender,
field: AccountField::Nonce,
value: Word::zero(),
value_prev: sender_account.nonce.into(),
},
)?;
state.push_op_reversible(
&mut exec_step,
AccountOp {
address: sender,
field: AccountField::CodeHash,
value: Word::zero(),
value_prev: sender_account.code_hash.to_word(),
},
)?;
if receiver != sender {
state.transfer_to(
&mut exec_step,
receiver,
!receiver_account.is_empty(),
false,
value,
true,
)?;
}

if state.call()?.is_persistent {
state.sdb.destruct_account(sender);
}

state.handle_return(&mut exec_step, geth_steps, false)?;
state.handle_return(&mut exec_step, geth_steps, !state.call()?.is_root)?;
Ok(vec![exec_step])
}
5 changes: 4 additions & 1 deletion bus-mapping/src/evm/opcodes/begin_end_tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ fn gen_begin_tx_steps(state: &mut CircuitInputStateRef) -> Result<ExecStep, Erro
AccountField::Nonce,
(nonce_prev + 1).into(),
nonce_prev.into(),
false,
)?;

// Add caller, callee and coinbase (for EIP-3651) to access list.
Expand Down Expand Up @@ -270,6 +271,7 @@ fn gen_end_tx_steps(state: &mut CircuitInputStateRef) -> Result<ExecStep, Error>
AccountField::Balance,
caller_balance,
caller_balance_prev,
false,
)?;

let effective_tip = state.tx.gas_price - state.block.base_fee;
Expand All @@ -289,12 +291,13 @@ fn gen_end_tx_steps(state: &mut CircuitInputStateRef) -> Result<ExecStep, Error>
coinbase_account.code_hash.to_word()
},
);
state.transfer_to_irreversible(
state.transfer_to(
&mut exec_step,
state.block.coinbase,
coinbase_exist,
false,
coinbase_transfer_value,
false,
)?;

// handle tx receipt tag
Expand Down

0 comments on commit 86c35ba

Please sign in to comment.