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

feat: bus mapping for sload and sstore #418

Closed
lispc wants to merge 9 commits into from
Closed

feat: bus mapping for sload and sstore #418

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e47613b
pick
lispc Apr 1, 2022
1079392
change geth to nightly in integration-tests
lispc Apr 1, 2022
a891164
Merge remote-tracking branch 'origin/main' into feat/sstore_busmapping
lispc Apr 1, 2022
3419920
Merge remote-tracking branch 'origin/main' into feat/sstore_busmapping
lispc Apr 4, 2022
2077c0e
fix conflict
lispc Apr 4, 2022
2ba341d
minor
lispc Apr 4, 2022
c03ac9c
Merge branch 'main' into feat/sstore_busmapping
lispc Apr 5, 2022
11534d4
fix conflict
lispc Apr 5, 2022
6778b3a
Merge branch 'main' into feat/sstore_busmapping
lispc Apr 5, 2022
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
4 changes: 4 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions bus-mapping/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,6 @@ serde_json = "1.0.66"
[dev-dependencies]
mock = { path = "../mock" }
pretty_assertions = "1.0.0"
rand = "0.8"
tokio = { version = "1.13", features = ["macros"] }
url = "2.2.2"
66 changes: 31 additions & 35 deletions bus-mapping/src/circuit_input_builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,8 @@ pub struct ExecStep {
/// overflow", this value will **not** be the actual Gas cost of the
/// step.
pub gas_cost: GasCost,
/// Accumulated gas refund
pub gas_refund: Gas,
/// Call index within the [`Transaction`]
pub call_index: usize,
/// The global counter when this step was executed.
Expand Down Expand Up @@ -204,6 +206,7 @@ impl ExecStep {
memory_size: step.memory.0.len(),
gas_left: step.gas,
gas_cost: step.gas_cost,
gas_refund: Gas(0),
call_index,
rwc,
swc,
Expand All @@ -223,6 +226,7 @@ impl Default for ExecStep {
memory_size: 0,
gas_left: Gas(0),
gas_cost: GasCost(0),
gas_refund: Gas(0),
call_index: 0,
rwc: RWCounter(0),
swc: 0,
Expand Down Expand Up @@ -799,11 +803,7 @@ impl<'a> CircuitInputStateRef<'a> {
rw: RW,
op: T,
) -> Result<(), Error> {
let op_ref = self.block.container.insert(Operation::new_reversible(
self.block_ctx.rwc.inc_pre(),
rw,
op,
));
let op_ref = self.apply_op(false, rw, op.into_enum());
step.bus_mapping_instance.push(op_ref);

// Increase state_write_counter
Expand Down Expand Up @@ -1070,38 +1070,29 @@ impl<'a> CircuitInputStateRef<'a> {
}
}

/// Apply reverted op to state and push to container.
fn apply_reverted_op(&mut self, op: OpEnum) -> OperationRef {
match op {
/// Apply op to state and push to container.
fn apply_op(&mut self, is_revert: bool, rw: RW, op: OpEnum) -> OperationRef {
match &op {
OpEnum::Storage(op) => {
let (_, account) = self.sdb.get_storage_mut(&op.address, &op.key);
*account = op.value;
self.block.container.insert(Operation::new(
self.block_ctx.rwc.inc_pre(),
RW::WRITE,
op,
))
self.sdb.set_storage(&op.address, &op.key, &op.value);
}
OpEnum::TxAccessListAccount(op) => {
if !op.value {
if !op.value_prev && op.value {
self.sdb.add_account_to_access_list(op.address);
}
if op.value_prev && !op.value {
self.sdb.remove_account_from_access_list(&op.address);
}
self.block.container.insert(Operation::new(
self.block_ctx.rwc.inc_pre(),
RW::WRITE,
op,
))
}
OpEnum::TxAccessListAccountStorage(op) => {
if !op.value {
if !op.value_prev && op.value {
self.sdb
.add_account_storage_to_access_list((op.address, op.key));
}
if op.value_prev && !op.value {
self.sdb
.remove_account_storage_from_access_list(&(op.address, op.key));
}
self.block.container.insert(Operation::new(
self.block_ctx.rwc.inc_pre(),
RW::WRITE,
op,
))
}
OpEnum::Account(op) => {
let (_, account) = self.sdb.get_account_mut(&op.address);
Expand All @@ -1112,16 +1103,19 @@ impl<'a> CircuitInputStateRef<'a> {
account.code_hash = op.value.to_be_bytes().into();
}
}
self.block.container.insert(Operation::new(
self.block_ctx.rwc.inc_pre(),
RW::WRITE,
op,
))
}
OpEnum::TxRefund(_) => unimplemented!(),
OpEnum::TxRefund(op) => {
self.sdb.set_refund(op.value);
}
OpEnum::AccountDestructed(_) => unimplemented!(),
_ => unreachable!(),
};
if is_revert {
debug_assert!(rw == RW::WRITE, "revert read operation");
}
self.block
.container
.insert_op_enum(self.block_ctx.rwc.inc_pre(), rw, !is_revert, op)
}

/// Handle a reversion group
Expand All @@ -1135,7 +1129,7 @@ impl<'a> CircuitInputStateRef<'a> {
// Apply reversions
for (step_index, op_ref) in reversion_group.op_refs.into_iter().rev() {
if let Some(op) = self.get_rev_op_by_ref(&op_ref) {
let rev_op_ref = self.apply_reverted_op(op);
let rev_op_ref = self.apply_op(true, RW::WRITE, op);
self.tx.steps[step_index]
.bus_mapping_instance
.push(rev_op_ref);
Expand Down Expand Up @@ -1466,6 +1460,7 @@ impl<'a> CircuitInputBuilder {

for (index, geth_step) in geth_trace.struct_logs.iter().enumerate() {
let mut state_ref = self.state_ref(&mut tx, &mut tx_ctx);
log::trace!("handle {}th opcode {:?} ", index, geth_step.op);
Copy link
Copy Markdown
Contributor

@CPerezz CPerezz Apr 5, 2022

Choose a reason for hiding this comment

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

I prefer tokio/tracing for these things. But maybe it's not the best option.

Maybe @ed255 has some suggestions

Copy link
Copy Markdown
Contributor

@ed255 ed255 Apr 6, 2022

Choose a reason for hiding this comment

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

I'm happy with using the log crate here, it's very lightweight and if logs aren't activated they don't add any performance penalty as far as I know. I don't know about tokio/tracing so I can't comment on that.

let exec_steps = gen_associated_ops(
&geth_step.op,
&mut state_ref,
Expand All @@ -1481,8 +1476,8 @@ impl<'a> CircuitInputBuilder {
let end_tx_step = gen_end_tx_ops(&mut self.state_ref(&mut tx, &mut tx_ctx))?;
tx.steps.push(end_tx_step);

self.sdb.commit_tx();
self.block.txs.push(tx);
self.sdb.clear_access_list_and_refund();

Ok(())
}
Expand Down Expand Up @@ -1997,6 +1992,7 @@ mod tracer_tests {
&GethExecTrace {
gas: Gas(0),
failed: false,
return_value: "".to_owned(),
struct_logs: vec![geth_step.clone()],
},
false,
Expand Down
7 changes: 6 additions & 1 deletion bus-mapping/src/evm/opcodes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ use eth_types::{
use keccak256::EMPTY_HASH;
use log::warn;

#[cfg(test)]
mod test_util;

mod call;
mod calldatacopy;
mod calldatasize;
Expand All @@ -30,6 +33,7 @@ mod number;
mod origin;
mod selfbalance;
mod sload;
mod sstore;
mod stackonlyop;
mod stop;
mod swap;
Expand All @@ -47,6 +51,7 @@ use mstore::Mstore;
use origin::Origin;
use selfbalance::Selfbalance;
use sload::Sload;
use sstore::Sstore;
use stackonlyop::StackOnlyOpcode;
use stop::Stop;
use swap::Swap;
Expand Down Expand Up @@ -137,7 +142,7 @@ fn fn_gen_associated_ops(opcode_id: &OpcodeId) -> FnGenAssociatedOps {
OpcodeId::MSTORE => Mstore::<false>::gen_associated_ops,
OpcodeId::MSTORE8 => Mstore::<true>::gen_associated_ops,
OpcodeId::SLOAD => Sload::gen_associated_ops,
// OpcodeId::SSTORE => {},
OpcodeId::SSTORE => Sstore::gen_associated_ops,
OpcodeId::JUMP => StackOnlyOpcode::<1, 0>::gen_associated_ops,
OpcodeId::JUMPI => StackOnlyOpcode::<2, 0>::gen_associated_ops,
OpcodeId::PC => StackOnlyOpcode::<0, 1>::gen_associated_ops,
Expand Down
35 changes: 7 additions & 28 deletions bus-mapping/src/evm/opcodes/calldatacopy.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,18 +158,15 @@ fn gen_memory_copy_steps(
#[cfg(test)]
mod calldatacopy_tests {
use crate::{
circuit_input_builder::ExecState,
mock::BlockData,
evm::opcodes::test_util::TestCase,
operation::{CallContextField, CallContextOp, StackOp, RW},
};
use eth_types::{
bytecode,
evm_types::{OpcodeId, StackAddress},
geth_types::GethData,
Word,
};

use mock::test_ctx::{helpers::*, TestContext};
use pretty_assertions::assert_eq;

#[test]
Expand All @@ -182,30 +179,13 @@ mod calldatacopy_tests {
STOP
};

// Get the execution steps from the external tracer
let block: GethData = TestContext::<2, 1>::new(
None,
account_0_code_account_1_no_code(code),
tx_from_1_to_0,
|block, _tx| block.number(0xcafeu64),
)
.unwrap()
.into();

let mut builder = BlockData::new_from_geth_data(block.clone()).new_circuit_input_builder();
builder
.handle_block(&block.eth_block, &block.geth_traces)
.unwrap();

let step = builder.block.txs()[0]
.steps()
.iter()
.find(|step| step.exec_state == ExecState::Op(OpcodeId::CALLDATACOPY))
.unwrap();
let test = TestCase::new_from_bytecode(code);
let step = test.step_witness(OpcodeId::CALLDATACOPY, 0);
let call_id = test.tx_witness().calls()[0].call_id;

assert_eq!(
[0, 1, 2]
.map(|idx| &builder.block.container.stack[step.bus_mapping_instance[idx].as_usize()])
.map(|idx| &step.rws.stack[idx])
.map(|operation| (operation.rw(), operation.op())),
[
(
Expand All @@ -225,14 +205,13 @@ mod calldatacopy_tests {

assert_eq!(
{
let operation =
&builder.block.container.call_context[step.bus_mapping_instance[3].as_usize()];
let operation = &step.rws.call_context[0];
(operation.rw(), operation.op())
},
(
RW::READ,
&CallContextOp {
call_id: builder.block.txs()[0].calls()[0].call_id,
call_id,
field: CallContextField::TxId,
value: Word::from(1),
}
Expand Down
38 changes: 7 additions & 31 deletions bus-mapping/src/evm/opcodes/calldatasize.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,14 @@ impl Opcode for Calldatasize {
#[cfg(test)]
mod calldatasize_tests {
use crate::{
circuit_input_builder::ExecState,
mock::BlockData,
evm::opcodes::test_util::TestCase,
operation::{CallContextField, CallContextOp, StackOp, RW},
};
use eth_types::{
bytecode,
evm_types::{OpcodeId, StackAddress},
geth_types::GethData,
};

use mock::test_ctx::{helpers::*, TestContext};
use pretty_assertions::assert_eq;

#[test]
Expand All @@ -61,33 +58,13 @@ mod calldatasize_tests {
STOP
};

// Get the execution steps from the external tracer
let block: GethData = TestContext::<2, 1>::new(
None,
account_0_code_account_1_no_code(code),
tx_from_1_to_0,
|block, _tx| block.number(0xcafeu64),
)
.unwrap()
.into();

let mut builder = BlockData::new_from_geth_data(block.clone()).new_circuit_input_builder();
builder
.handle_block(&block.eth_block, &block.geth_traces)
.unwrap();

let step = builder.block.txs()[0]
.steps()
.iter()
.find(|step| step.exec_state == ExecState::Op(OpcodeId::CALLDATASIZE))
.unwrap();

let call_id = builder.block.txs()[0].calls()[0].call_id;
let call_data_size = block.eth_block.transactions[0].input.as_ref().len().into();
let test = TestCase::new_from_bytecode(code);
let step = test.step_witness(OpcodeId::CALLDATASIZE, 0);
let call_id = test.tx_witness().calls()[0].call_id;
let call_data_size = test.tx_input().input.as_ref().len().into();
assert_eq!(
{
let operation =
&builder.block.container.call_context[step.bus_mapping_instance[0].as_usize()];
let operation = &step.rws.call_context[0];
(operation.rw(), operation.op())
},
(
Expand All @@ -101,8 +78,7 @@ mod calldatasize_tests {
);
assert_eq!(
{
let operation =
&builder.block.container.stack[step.bus_mapping_instance[1].as_usize()];
let operation = &step.rws.stack[0];
(operation.rw(), operation.op())
},
(
Expand Down
Loading