diff --git a/.github/PULL_REQUEST_TEMPLATE/pull_request_template.md b/.github/PULL_REQUEST_TEMPLATE/pull_request_template.md new file mode 100644 index 0000000000..c27926c04d --- /dev/null +++ b/.github/PULL_REQUEST_TEMPLATE/pull_request_template.md @@ -0,0 +1,42 @@ +_PR description_ + +### Contents + +- _item_ + +### Rationale + +_design decisions and extended information_ + + +
+ +## How to fill a PR description (remove this) + +Please give a concise description of your PR. + +The target readers could be future developers, reviewers, and auditors. By reading your description, they should easily understand the changes proposed in this pull request. + +MUST: Reference the issue to resolve + +### Single responsability + +Is RECOMMENDED to create single responsibility commits, but not mandatory. + +Anyway, you MUST enumerate the changes in a unitary way, e.g. + +``` +This PR contains: + +- Cleanup of xxxx, yyyy +- Changed xxxx to yyyy in order to bla bla +- Added xxxx function to ... +- Refactored .... +``` + +### Design choices + +RECOMMENDED to: +- What types of design choices did you face? +- What decisions you have made? +- Any valuable information that could help reviewers to think critically diff --git a/bus-mapping/src/circuit_input_builder.rs b/bus-mapping/src/circuit_input_builder.rs index 09c4b7e7a6..0504a91713 100644 --- a/bus-mapping/src/circuit_input_builder.rs +++ b/bus-mapping/src/circuit_input_builder.rs @@ -45,6 +45,8 @@ pub struct CircuitsParams { pub max_txs: usize, /// Maximum number of bytes from all txs calldata in the Tx Circuit pub max_calldata: usize, + /// Max ammount of rows that the CopyCircuit can have. + pub max_copy_rows: usize, /// Maximum number of bytes supported in the Bytecode Circuit pub max_bytecode: usize, // TODO: Rename for consistency @@ -60,6 +62,9 @@ impl Default for CircuitsParams { max_rws: 1000, max_txs: 1, max_calldata: 256, + // TODO: Check whether this value is correct or we should increase/decrease based on + // this lib tests + max_copy_rows: 1000, max_bytecode: 512, keccak_padding: None, } diff --git a/bus-mapping/src/circuit_input_builder/execution.rs b/bus-mapping/src/circuit_input_builder/execution.rs index 875e5b936f..9700bd72f6 100644 --- a/bus-mapping/src/circuit_input_builder/execution.rs +++ b/bus-mapping/src/circuit_input_builder/execution.rs @@ -155,8 +155,11 @@ impl ExecState { /// Defines the various source/destination types for a copy event. #[derive(Clone, Copy, Debug, PartialEq, Eq, EnumIter)] pub enum CopyDataType { + /// When we need to pad the Copy rows of the circuit up to a certain maximum + /// with rows that are not "useful". + Padding = 0, /// When the source for the copy event is the bytecode table. - Bytecode = 1, + Bytecode, /// When the source/destination for the copy event is memory. Memory, /// When the source for the copy event is tx's calldata. @@ -252,12 +255,12 @@ impl CopyEvent { .checked_sub(self.src_addr) .unwrap_or_default(), ), - CopyDataType::RlcAcc | CopyDataType::TxLog => unreachable!(), + CopyDataType::RlcAcc | CopyDataType::TxLog | CopyDataType::Padding => unreachable!(), }; let destination_rw_increase = match self.dst_type { CopyDataType::RlcAcc | CopyDataType::Bytecode => 0, CopyDataType::TxLog | CopyDataType::Memory => u64::try_from(step_index).unwrap() / 2, - CopyDataType::TxCalldata => unreachable!(), + CopyDataType::TxCalldata | CopyDataType::Padding => unreachable!(), }; source_rw_increase + destination_rw_increase } diff --git a/bus-mapping/src/circuit_input_builder/input_state_ref.rs b/bus-mapping/src/circuit_input_builder/input_state_ref.rs index f25e1aa0e1..6c1c31f99f 100644 --- a/bus-mapping/src/circuit_input_builder/input_state_ref.rs +++ b/bus-mapping/src/circuit_input_builder/input_state_ref.rs @@ -1190,6 +1190,18 @@ impl<'a> CircuitInputStateRef<'a> { CallContextField::IsSuccess, 0u64.into(), ); + + //Even call.rw_counter_end_of_reversion is zero for now, it will set in + //set_value_ops_call_context_rwc_eor later + // if call fails, no matter root or internal, read RwCounterEndOfReversion for + // circuit constraint. + self.call_context_read( + exec_step, + call.call_id, + CallContextField::RwCounterEndOfReversion, + call.rw_counter_end_of_reversion.into(), + ); + if call.is_root { return Ok(()); } diff --git a/bus-mapping/src/evm/opcodes.rs b/bus-mapping/src/evm/opcodes.rs index 2977af2aba..10a5c5f235 100644 --- a/bus-mapping/src/evm/opcodes.rs +++ b/bus-mapping/src/evm/opcodes.rs @@ -54,6 +54,7 @@ mod stop; mod swap; mod error_invalid_jump; +mod error_invalid_opcode; mod error_oog_call; #[cfg(test)] @@ -72,7 +73,8 @@ use codecopy::Codecopy; use codesize::Codesize; use create::DummyCreate; use dup::Dup; -use error_invalid_jump::ErrorInvalidJump; +use error_invalid_jump::InvalidJump; +use error_invalid_opcode::InvalidOpcode; use error_oog_call::OOGCall; use exp::Exponentiation; use extcodecopy::Extcodecopy; @@ -256,7 +258,8 @@ fn fn_gen_associated_ops(opcode_id: &OpcodeId) -> FnGenAssociatedOps { fn fn_gen_error_state_associated_ops(error: &ExecError) -> Option { match error { - ExecError::InvalidJump => Some(ErrorInvalidJump::gen_associated_ops), + ExecError::InvalidJump => Some(InvalidJump::gen_associated_ops), + ExecError::InvalidOpcode => Some(InvalidOpcode::gen_associated_ops), ExecError::OutOfGas(OogError::Call) => Some(OOGCall::gen_associated_ops), // call & callcode can encounter InsufficientBalance error, Use pop-7 generic CallOpcode ExecError::InsufficientBalance => Some(CallOpcode::<7>::gen_associated_ops), diff --git a/bus-mapping/src/evm/opcodes/error_invalid_jump.rs b/bus-mapping/src/evm/opcodes/error_invalid_jump.rs index 3678fb47df..991725397f 100644 --- a/bus-mapping/src/evm/opcodes/error_invalid_jump.rs +++ b/bus-mapping/src/evm/opcodes/error_invalid_jump.rs @@ -4,9 +4,9 @@ use crate::Error; use eth_types::{GethExecStep, ToAddress, ToWord, Word}; #[derive(Debug, Copy, Clone)] -pub(crate) struct ErrorInvalidJump; +pub(crate) struct InvalidJump; -impl Opcode for ErrorInvalidJump { +impl Opcode for InvalidJump { fn gen_associated_ops( state: &mut CircuitInputStateRef, geth_steps: &[GethExecStep], diff --git a/bus-mapping/src/evm/opcodes/error_invalid_opcode.rs b/bus-mapping/src/evm/opcodes/error_invalid_opcode.rs new file mode 100644 index 0000000000..d73f6bfae4 --- /dev/null +++ b/bus-mapping/src/evm/opcodes/error_invalid_opcode.rs @@ -0,0 +1,29 @@ +use crate::circuit_input_builder::{CircuitInputStateRef, ExecStep}; +use crate::evm::Opcode; +use crate::Error; +use eth_types::GethExecStep; + +#[derive(Debug, Copy, Clone)] +pub(crate) struct InvalidOpcode; + +impl Opcode for InvalidOpcode { + fn gen_associated_ops( + state: &mut CircuitInputStateRef, + geth_steps: &[GethExecStep], + ) -> Result, Error> { + let geth_step = &geth_steps[0]; + let mut exec_step = state.new_step(geth_step)?; + + let next_step = if geth_steps.len() > 1 { + Some(&geth_steps[1]) + } else { + None + }; + exec_step.error = state.get_step_err(geth_step, next_step).unwrap(); + + state.gen_restore_context_ops(&mut exec_step, geth_steps)?; + state.handle_return(geth_step)?; + + Ok(vec![exec_step]) + } +} diff --git a/circuit-benchmarks/src/super_circuit.rs b/circuit-benchmarks/src/super_circuit.rs index aa68bc1950..8d24b04403 100644 --- a/circuit-benchmarks/src/super_circuit.rs +++ b/circuit-benchmarks/src/super_circuit.rs @@ -71,7 +71,7 @@ mod tests { block.sign(&wallets); - let (_, circuit, instance, _) = SuperCircuit::<_, 1, 32, 512>::build(block).unwrap(); + let (_, circuit, instance, _) = SuperCircuit::<_, 1, 32, 512, 512>::build(block).unwrap(); let instance_refs: Vec<&[Fr]> = instance.iter().map(|v| &v[..]).collect(); // Bench setup generation @@ -96,7 +96,7 @@ mod tests { Challenge255, ChaChaRng, Blake2bWrite, G1Affine, Challenge255>, - SuperCircuit, + SuperCircuit, >( &general_params, &pk, diff --git a/integration-tests/src/integration_test_circuits.rs b/integration-tests/src/integration_test_circuits.rs index 58fe829064..1f17cc7e6a 100644 --- a/integration-tests/src/integration_test_circuits.rs +++ b/integration-tests/src/integration_test_circuits.rs @@ -38,6 +38,7 @@ const CIRCUITS_PARAMS: CircuitsParams = CircuitsParams { max_txs: 4, max_calldata: 4000, max_bytecode: 4000, + max_copy_rows: 16384, keccak_padding: None, }; @@ -344,6 +345,7 @@ pub async fn test_super_circuit_block(block_num: u64) { const MAX_CALLDATA: usize = 512; const MAX_RWS: usize = 5888; const MAX_BYTECODE: usize = 5000; + const MAX_COPY_ROWS: usize = 5888; log::info!("test super circuit, block number: {}", block_num); let cli = get_client(); @@ -354,6 +356,7 @@ pub async fn test_super_circuit_block(block_num: u64) { max_txs: MAX_TXS, max_calldata: MAX_CALLDATA, max_bytecode: MAX_BYTECODE, + max_copy_rows: MAX_COPY_ROWS, keccak_padding: None, }, ) @@ -361,7 +364,7 @@ pub async fn test_super_circuit_block(block_num: u64) { .unwrap(); let (builder, _) = cli.gen_inputs(block_num).await.unwrap(); let (k, circuit, instance) = - SuperCircuit::::build_from_circuit_input_builder( + SuperCircuit::::build_from_circuit_input_builder( &builder, ) .unwrap(); diff --git a/integration-tests/tests/circuit_input_builder.rs b/integration-tests/tests/circuit_input_builder.rs index 5c69ac31a6..e34c0a617f 100644 --- a/integration-tests/tests/circuit_input_builder.rs +++ b/integration-tests/tests/circuit_input_builder.rs @@ -18,6 +18,7 @@ async fn test_circuit_input_builder_block(block_num: u64) { max_txs: 1, max_calldata: 4000, max_bytecode: 4000, + max_copy_rows: 16384, keccak_padding: None, }, ) diff --git a/testool/Config.toml b/testool/Config.toml index 2b3c3ebee9..6dd53ff8cf 100644 --- a/testool/Config.toml +++ b/testool/Config.toml @@ -3,7 +3,6 @@ id="default" path="tests/src/GeneralStateTestsFiller/**/*" max_gas = 500000 max_steps = 1000 -max_rws = 51000 unimplemented_opcodes = [] ignore_tests = [] @@ -12,22 +11,18 @@ id="nightly" path="tests/src/GeneralStateTestsFiller/**/*" max_gas = 500000 max_steps = 1000 -max_rws = 51000 unimplemented_opcodes = [ - "SAR", - "EXTCODECOPY", "CREATE", "CREATE2", "SELFDESTRUCT" ] -ignore_tests=["&tofix"] +ignore_tests=["&tofix", "&sigkill"] [[suite]] id = "light" path="tests/src/GeneralStateTestsFiller/**/*" max_gas = 500000 max_steps = 1000 -max_rws = 51000 unimplemented_opcodes = [] allow_tests=[ "add_d0(add_neg1_neg1)_g0_v0", @@ -174,6 +169,13 @@ allow_tests=[ # must fix ------------------------------------------------------------------------------ +[[set]] +id = "sigkill" +desc = "tests that sigkill" +tests = [ + "CallToNameRegistratorMemOOGAndInsufficientBalance_d0_g0_v0" +] + [[set]] id = "tofix" desc = "***panicked at 'circuit should pass', contraint error" diff --git a/testool/debug.sh b/testool/debug.sh deleted file mode 100755 index 55dbf1c4fe..0000000000 --- a/testool/debug.sh +++ /dev/null @@ -1 +0,0 @@ -RUST_BACKTRACE=1 cargo run -- --path "tests/src/GeneralStateTestsFiller/**/*" --skip-state-circuit --test $* diff --git a/testool/loop.sh b/testool/loop.sh deleted file mode 100755 index 132af8b3e2..0000000000 --- a/testool/loop.sh +++ /dev/null @@ -1,4 +0,0 @@ -while : -do - cargo run --release -- --path "tests/src/GeneralStateTestsFiller/**/*" --skip-state-circuit -done diff --git a/testool/pre_commit.sh b/testool/pre_commit.sh deleted file mode 100755 index e14be593e1..0000000000 --- a/testool/pre_commit.sh +++ /dev/null @@ -1,5 +0,0 @@ -cargo fmt -cargo clippy -- -Dwarnings -W clippy::pedantic -cargo update -cargo test -cargo outdated --root-deps-only diff --git a/testool/raw.sh b/testool/raw.sh deleted file mode 100755 index 280500d43d..0000000000 --- a/testool/raw.sh +++ /dev/null @@ -1,2 +0,0 @@ -cargo run --release -- --raw $* - diff --git a/testool/src/config.rs b/testool/src/config.rs index 935ec0ec64..115ed3094d 100644 --- a/testool/src/config.rs +++ b/testool/src/config.rs @@ -18,7 +18,6 @@ pub struct TestSuite { pub path: String, pub max_gas: u64, pub max_steps: u64, - pub max_rws: u64, /// see [Implemented opcodes status](https://github.com/appliedzkp/zkevm-circuits/issues/477) pub unimplemented_opcodes: Vec, @@ -31,9 +30,8 @@ impl Default for TestSuite { Self { id: "default".to_string(), path: String::default(), - max_gas: 100000, - max_steps: 1000, - max_rws: 50104, + max_gas: u64::MAX, + max_steps: u64::MAX, unimplemented_opcodes: Vec::new(), ignore_tests: Some(Vec::new()), allow_tests: None, diff --git a/testool/src/main.rs b/testool/src/main.rs index d454b24ec7..6369888702 100644 --- a/testool/src/main.rs +++ b/testool/src/main.rs @@ -10,7 +10,7 @@ use anyhow::{bail, Result}; use clap::Parser; use compiler::Compiler; use config::Config; -use log::{debug, error, info}; +use log::{error, info}; use statetest::{ geth_trace, load_statetests_suite, run_statetests_suite, run_test, CircuitsConfig, Results, StateTest, @@ -72,15 +72,13 @@ struct Args { const RESULT_CACHE: &str = "result.cache"; fn run_single_test(test: StateTest, circuits_config: CircuitsConfig) -> Result<()> { - debug!("{}", &test); - + println!("{}", &test); let trace = geth_trace(test.clone())?; crate::utils::print_trace(trace)?; - debug!( + println!( "result={:?}", run_test(test, TestSuite::default(), circuits_config) ); - Ok(()) } diff --git a/testool/src/statetest/executor.rs b/testool/src/statetest/executor.rs index 45ef00e75b..dc74eacbc0 100644 --- a/testool/src/statetest/executor.rs +++ b/testool/src/statetest/executor.rs @@ -287,6 +287,7 @@ pub fn run_test( max_rws: 55000, max_calldata: 5000, max_bytecode: 5000, + max_copy_rows: 55000, keccak_padding: None, }; let block_data = BlockData::new_from_geth_data_with_params(geth_data, circuits_params); @@ -312,7 +313,7 @@ pub fn run_test( geth_data.sign(&wallets); let (k, circuit, instance, _builder) = - SuperCircuit::::build(geth_data).unwrap(); + SuperCircuit::::build(geth_data).unwrap(); builder = _builder; let prover = MockProver::run(k, &circuit, instance).unwrap(); diff --git a/testool/src/statetest/results.rs b/testool/src/statetest/results.rs index 0dc17b4535..99effe1df0 100644 --- a/testool/src/statetest/results.rs +++ b/testool/src/statetest/results.rs @@ -320,7 +320,12 @@ impl Results { result.details ); } - let entry = format!("{:?};{};{}\n", result.level, test_id, result.details); + let details = result + .details + .replace('\n', "") + .replace(' ', "") + .replace('\t', ""); + let entry = format!("{:?};{};{}\n", result.level, test_id, details); if let Some(path) = &self.cache { std::fs::OpenOptions::new() .read(true) diff --git a/testool/src/statetest/suite.rs b/testool/src/statetest/suite.rs index 09420d3ee1..6d3f6147cf 100644 --- a/testool/src/statetest/suite.rs +++ b/testool/src/statetest/suite.rs @@ -58,11 +58,18 @@ pub fn run_statetests_suite( results: &mut Results, ) -> Result<()> { // Filter already cached entries + let all_test_count = tcs.len(); let tcs: Vec = tcs .into_iter() - .filter(|t| !results.contains(&t.id)) + .filter(|t| !results.contains(&format!("{}#{}", t.id, t.path))) .collect(); + log::info!( + "{} test results cached, {} remaining", + all_test_count - tcs.len(), + tcs.len() + ); + let results = Arc::new(RwLock::from(results)); // for each test diff --git a/testool/test.sh b/testool/test.sh deleted file mode 100755 index 03349d2cb9..0000000000 --- a/testool/test.sh +++ /dev/null @@ -1,2 +0,0 @@ -cargo run --release -- --path "tests/src/GeneralStateTestsFiller/**/*" --skip-state-circuit --test $* - diff --git a/zkevm-circuits/src/copy_circuit.rs b/zkevm-circuits/src/copy_circuit.rs index 12bf051466..f3ad8111ab 100644 --- a/zkevm-circuits/src/copy_circuit.rs +++ b/zkevm-circuits/src/copy_circuit.rs @@ -169,28 +169,34 @@ impl SubCircuitConfig for CopyCircuitConfig { let not_last_two_rows = 1.expr() - meta.query_advice(is_last, Rotation::cur()) - meta.query_advice(is_last, Rotation::next()); - cb.condition(not_last_two_rows, |cb| { - cb.require_equal( - "rows[0].id == rows[2].id", - meta.query_advice(id, Rotation::cur()), - meta.query_advice(id, Rotation(2)), - ); - cb.require_equal( - "rows[0].tag == rows[2].tag", - tag.value(Rotation::cur())(meta), - tag.value(Rotation(2))(meta), - ); - cb.require_equal( - "rows[0].addr + 1 == rows[2].addr", - meta.query_advice(addr, Rotation::cur()) + 1.expr(), - meta.query_advice(addr, Rotation(2)), - ); - cb.require_equal( - "rows[0].src_addr_end == rows[2].src_addr_end for non-last step", - meta.query_advice(src_addr_end, Rotation::cur()), - meta.query_advice(src_addr_end, Rotation(2)), - ); - }); + cb.condition( + not_last_two_rows + * (not::expr(tag.value_equals(CopyDataType::Padding, Rotation::cur())( + meta, + ))), + |cb| { + cb.require_equal( + "rows[0].id == rows[2].id", + meta.query_advice(id, Rotation::cur()), + meta.query_advice(id, Rotation(2)), + ); + cb.require_equal( + "rows[0].tag == rows[2].tag", + tag.value(Rotation::cur())(meta), + tag.value(Rotation(2))(meta), + ); + cb.require_equal( + "rows[0].addr + 1 == rows[2].addr", + meta.query_advice(addr, Rotation::cur()) + 1.expr(), + meta.query_advice(addr, Rotation(2)), + ); + cb.require_equal( + "rows[0].src_addr_end == rows[2].src_addr_end for non-last step", + meta.query_advice(src_addr_end, Rotation::cur()), + meta.query_advice(src_addr_end, Rotation(2)), + ); + }, + ); let rw_diff = and::expr([ or::expr([ @@ -254,7 +260,10 @@ impl SubCircuitConfig for CopyCircuitConfig { ]), ); cb.condition( - not::expr(meta.query_advice(is_last, Rotation::next())), + not::expr(meta.query_advice(is_last, Rotation::next())) + * (not::expr(tag.value_equals(CopyDataType::Padding, Rotation::cur())( + meta, + ))), |cb| { cb.require_equal( "bytes_left == bytes_left_next + 1 for non-last step", @@ -423,6 +432,18 @@ impl CopyCircuitConfig { block: &Block, challenges: Challenges>, ) -> Result<(), Error> { + let copy_rows_needed = block + .copy_events + .iter() + .map(|c| c.bytes.len() * 2) + .sum::(); + + // The `+ 2` is used to take into account the two extra empty copy rows needed + // to satisfy the query at `Rotation(2)` performed inside of the + // `rows[2].value == rows[0].value * r + rows[1].value` requirement in the RLC + // Accumulation gate. + assert!(copy_rows_needed + 2 <= block.circuits_params.max_copy_rows); + let tag_chip = BinaryNumberChip::construct(self.copy_table.tag); let lt_chip = LtChip::construct(self.addr_lt_addr_end); @@ -497,11 +518,16 @@ impl CopyCircuitConfig { offset += 1; } } - // pad two rows in the end to satisfy Halo2 cell assignment check - for _ in 0..2 { - self.assign_padding_row(&mut region, offset, &tag_chip)?; + + for _ in 0..block.circuits_params.max_copy_rows - copy_rows_needed - 2 { + self.assign_padding_row(&mut region, offset, false, &tag_chip, <_chip)?; offset += 1; } + + self.assign_padding_row(&mut region, offset, true, &tag_chip, <_chip)?; + offset += 1; + self.assign_padding_row(&mut region, offset, true, &tag_chip, <_chip)?; + Ok(()) }, ) @@ -511,15 +537,24 @@ impl CopyCircuitConfig { &self, region: &mut Region, offset: usize, + is_last_two: bool, tag_chip: &BinaryNumberChip, + lt_chip: &LtChip, ) -> Result<(), Error> { - // q_enable - region.assign_fixed( - || "q_enable", - self.q_enable, - offset, - || Value::known(F::zero()), - )?; + if !is_last_two { + // q_enable + region.assign_fixed( + || "q_enable", + self.q_enable, + offset, + || Value::known(F::one()), + )?; + // q_step + if offset % 2 == 0 { + self.q_step.enable(region, offset)?; + } + } + // is_first region.assign_advice( || format!("assign is_first {}", offset), @@ -553,7 +588,7 @@ impl CopyCircuitConfig { || format!("assign src_addr_end {}", offset), self.copy_table.src_addr_end, offset, - || Value::known(F::zero()), + || Value::known(F::one()), )?; // bytes_left region.assign_advice( @@ -605,7 +640,10 @@ impl CopyCircuitConfig { || Value::known(F::zero()), )?; // tag - tag_chip.assign(region, offset, &CopyDataType::default())?; + tag_chip.assign(region, offset, &CopyDataType::Padding)?; + // Assing LT gadget + + lt_chip.assign(region, offset, F::zero(), F::one())?; Ok(()) } } @@ -648,8 +686,13 @@ impl SubCircuit for CopyCircuit { /// Return the minimum number of rows required to prove the block fn min_num_rows_block(block: &witness::Block) -> (usize, usize) { ( - block.copy_events.iter().map(|c| c.bytes.len() * 2).sum(), - block.copy_circuit_pad_to, + block + .copy_events + .iter() + .map(|c| c.bytes.len() * 2) + .sum::() + + 2, + block.circuits_params.max_copy_rows, ) } @@ -756,6 +799,8 @@ pub mod dev { #[cfg(test)] mod tests { use super::dev::test_copy_circuit; + use crate::evm_circuit::test::rand_bytes; + use crate::evm_circuit::witness::block_convert; use bus_mapping::evm::{gen_sha3_code, MemoryKind}; use bus_mapping::{ circuit_input_builder::{CircuitInputBuilder, CircuitsParams}, @@ -765,9 +810,7 @@ mod tests { use halo2_proofs::halo2curves::bn256::Fr; use mock::test_ctx::helpers::account_0_code_account_1_no_code; use mock::{TestContext, MOCK_ACCOUNTS}; - - use crate::evm_circuit::test::rand_bytes; - use crate::evm_circuit::witness::block_convert; + use pretty_assertions::assert_eq; fn gen_calldatacopy_data() -> CircuitInputBuilder { let length = 0x0fffusize; @@ -796,6 +839,7 @@ mod tests { block.clone(), CircuitsParams { max_rws: 8192, + max_copy_rows: 8192 + 2, ..Default::default() }, ) @@ -867,6 +911,7 @@ mod tests { block.clone(), CircuitsParams { max_rws: 2000, + max_copy_rows: 0x200 * 2 + 2, ..Default::default() }, ) diff --git a/zkevm-circuits/src/evm_circuit/execution.rs b/zkevm-circuits/src/evm_circuit/execution.rs index 26501d5dd0..a0ab16c16e 100644 --- a/zkevm-circuits/src/evm_circuit/execution.rs +++ b/zkevm-circuits/src/evm_circuit/execution.rs @@ -54,6 +54,7 @@ mod dup; mod end_block; mod end_tx; mod error_invalid_jump; +mod error_invalid_opcode; mod error_oog_call; mod error_oog_constant; mod error_oog_static_memory; @@ -119,6 +120,7 @@ use dup::DupGadget; use end_block::EndBlockGadget; use end_tx::EndTxGadget; use error_invalid_jump::ErrorInvalidJumpGadget; +use error_invalid_opcode::ErrorInvalidOpcodeGadget; use error_oog_call::ErrorOOGCallGadget; use error_oog_constant::ErrorOOGConstantGadget; use error_stack::ErrorStackGadget; @@ -282,6 +284,7 @@ pub(crate) struct ExecutionConfig { error_oog_code_store: DummyGadget, error_insufficient_balance: DummyGadget, error_invalid_jump: ErrorInvalidJumpGadget, + error_invalid_opcode: ErrorInvalidOpcodeGadget, error_depth: DummyGadget, error_write_protection: DummyGadget, error_contract_address_collision: @@ -289,7 +292,6 @@ pub(crate) struct ExecutionConfig { error_invalid_creation_code: DummyGadget, error_return_data_out_of_bound: DummyGadget, - invalid_opcode_gadget: DummyGadget, } impl ExecutionConfig { @@ -540,12 +542,12 @@ impl ExecutionConfig { error_oog_code_store: configure_gadget!(), error_insufficient_balance: configure_gadget!(), error_invalid_jump: configure_gadget!(), + error_invalid_opcode: configure_gadget!(), error_write_protection: configure_gadget!(), error_depth: configure_gadget!(), error_contract_address_collision: configure_gadget!(), error_invalid_creation_code: configure_gadget!(), error_return_data_out_of_bound: configure_gadget!(), - invalid_opcode_gadget: configure_gadget!(), // step and presets step: step_curr, height_map, @@ -1202,6 +1204,9 @@ impl ExecutionConfig { ExecutionState::ErrorInvalidJump => { assign_exec_step!(self.error_invalid_jump) } + ExecutionState::ErrorInvalidOpcode => { + assign_exec_step!(self.error_invalid_opcode) + } ExecutionState::ErrorWriteProtection => { assign_exec_step!(self.error_write_protection) } @@ -1218,10 +1223,6 @@ impl ExecutionConfig { assign_exec_step!(self.error_return_data_out_of_bound) } - ExecutionState::ErrorInvalidOpcode => { - assign_exec_step!(self.invalid_opcode_gadget) - } - _ => unimplemented!("unimplemented ExecutionState: {:?}", step.execution_state), } diff --git a/zkevm-circuits/src/evm_circuit/execution/error_invalid_jump.rs b/zkevm-circuits/src/evm_circuit/execution/error_invalid_jump.rs index b22fe74f00..fdb878dda3 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_invalid_jump.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_invalid_jump.rs @@ -34,6 +34,7 @@ pub(crate) struct ErrorInvalidJumpGadget { is_jumpi: IsEqualGadget, phase2_condition: Cell, is_condition_zero: IsZeroGadget, + rw_counter_end_of_reversion: Cell, restore_context: RestoreContextGadget, } @@ -47,6 +48,7 @@ impl ExecutionGadget for ErrorInvalidJumpGadget { let opcode = cb.query_cell(); let value = cb.query_cell(); let is_code = cb.query_cell(); + let rw_counter_end_of_reversion = cb.query_cell(); let phase2_condition = cb.query_cell_with_type(CellType::StoragePhase2); cb.require_in_set( @@ -96,6 +98,13 @@ impl ExecutionGadget for ErrorInvalidJumpGadget { cb.call_context_lookup(false.expr(), None, CallContextFieldTag::IsSuccess, 0.expr()); + cb.call_context_lookup( + false.expr(), + None, + CallContextFieldTag::RwCounterEndOfReversion, + rw_counter_end_of_reversion.expr(), + ); + // Go to EndTx only when is_root let is_to_end_tx = cb.next.execution_state_selector([ExecutionState::EndTx]); cb.require_equal( @@ -110,7 +119,7 @@ impl ExecutionGadget for ErrorInvalidJumpGadget { cb.require_step_state_transition(StepStateTransition { call_id: Same, rw_counter: Delta( - 2.expr() + is_jumpi.expr() + cb.curr.state.reversible_write_counter.expr(), + 3.expr() + is_jumpi.expr() + cb.curr.state.reversible_write_counter.expr(), ), ..StepStateTransition::any() @@ -131,6 +140,15 @@ impl ExecutionGadget for ErrorInvalidJumpGadget { ) }); + // constrain RwCounterEndOfReversion + let rw_counter_end_of_step = + cb.curr.state.rw_counter.expr() + cb.rw_counter_offset() - 1.expr(); + cb.require_equal( + "rw_counter_end_of_reversion = rw_counter_end_of_step + reversible_counter", + rw_counter_end_of_reversion.expr(), + rw_counter_end_of_step + cb.curr.state.reversible_write_counter.expr(), + ); + Self { opcode, destination, @@ -142,6 +160,7 @@ impl ExecutionGadget for ErrorInvalidJumpGadget { is_jumpi, phase2_condition, is_condition_zero, + rw_counter_end_of_reversion, restore_context, } } @@ -222,8 +241,13 @@ impl ExecutionGadget for ErrorInvalidJumpGadget { self.is_condition_zero .assign_value(region, offset, condition_rlc)?; + self.rw_counter_end_of_reversion.assign( + region, + offset, + Value::known(F::from(call.rw_counter_end_of_reversion as u64)), + )?; self.restore_context - .assign(region, offset, block, call, step, 2 + is_jumpi as usize)?; + .assign(region, offset, block, call, step, 3 + is_jumpi as usize)?; Ok(()) } } diff --git a/zkevm-circuits/src/evm_circuit/execution/error_invalid_opcode.rs b/zkevm-circuits/src/evm_circuit/execution/error_invalid_opcode.rs new file mode 100644 index 0000000000..c06ff4511a --- /dev/null +++ b/zkevm-circuits/src/evm_circuit/execution/error_invalid_opcode.rs @@ -0,0 +1,263 @@ +use crate::evm_circuit::execution::ExecutionGadget; +use crate::evm_circuit::step::ExecutionState; +use crate::evm_circuit::util::common_gadget::RestoreContextGadget; +use crate::evm_circuit::util::constraint_builder::Transition::{Delta, Same}; +use crate::evm_circuit::util::constraint_builder::{ConstraintBuilder, StepStateTransition}; +use crate::evm_circuit::util::math_gadget::LtGadget; +use crate::evm_circuit::util::{CachedRegion, Cell}; +use crate::evm_circuit::witness::{Block, Call, ExecStep, Transaction}; +use crate::table::CallContextFieldTag; +use eth_types::Field; +use gadgets::util::{and, or, Expr}; +use halo2_proofs::circuit::Value; +use halo2_proofs::plonk::Error; + +const SEPARATE_INVALID_OPCODES: [u8; 17] = [ + 0x0c, 0x0d, 0x0e, 0x0f, 0x1e, 0x1f, 0x5c, 0x5d, 0x5e, 0x5f, 0xf6, 0xf7, 0xf8, 0xf9, 0xfb, 0xfc, + 0xfe, +]; + +/// Gadget for invalid opcodes. It verifies invalid bytes in any condition of: +/// - `opcode > 0x20 && opcode < 0x30` +/// - `opcode > 0x48 && opcode < 0x50` +/// - `opcode > 0xa4 && opcode < 0xf0` +/// - one of [`SEPARATE_INVALID_OPCODES`] +#[derive(Clone, Debug)] +pub(crate) struct ErrorInvalidOpcodeGadget { + opcode: Cell, + op_gt_20: LtGadget, + op_lt_30: LtGadget, + op_gt_48: LtGadget, + op_lt_50: LtGadget, + op_gt_a4: LtGadget, + op_lt_f0: LtGadget, + restore_context: RestoreContextGadget, +} + +impl ExecutionGadget for ErrorInvalidOpcodeGadget { + const NAME: &'static str = "ErrorInvalidOpcode"; + + const EXECUTION_STATE: ExecutionState = ExecutionState::ErrorInvalidOpcode; + + fn configure(cb: &mut ConstraintBuilder) -> Self { + let opcode = cb.query_cell(); + cb.opcode_lookup(opcode.expr(), 1.expr()); + + let op_gt_20 = LtGadget::construct(cb, 0x20.expr(), opcode.expr()); + let op_lt_30 = LtGadget::construct(cb, opcode.expr(), 0x30.expr()); + let op_gt_48 = LtGadget::construct(cb, 0x48.expr(), opcode.expr()); + let op_lt_50 = LtGadget::construct(cb, opcode.expr(), 0x50.expr()); + let op_gt_a4 = LtGadget::construct(cb, 0xa4.expr(), opcode.expr()); + let op_lt_f0 = LtGadget::construct(cb, opcode.expr(), 0xf0.expr()); + + let op_range_20_30 = and::expr([op_gt_20.expr(), op_lt_30.expr()]); + let op_range_48_50 = and::expr([op_gt_48.expr(), op_lt_50.expr()]); + let op_range_a4_f0 = and::expr([op_gt_a4.expr(), op_lt_f0.expr()]); + + // Check separate byte set if no above condition is met. + cb.condition( + 1.expr() - or::expr([op_range_20_30, op_range_48_50, op_range_a4_f0]), + |cb| { + cb.require_in_set( + "Constrain separate invalid opcodes", + opcode.expr(), + SEPARATE_INVALID_OPCODES + .iter() + .map(|op| op.expr()) + .collect(), + ); + }, + ); + + // Current call must be failed. + cb.call_context_lookup(false.expr(), None, CallContextFieldTag::IsSuccess, 0.expr()); + + // Go to EndTx only when is_root. + let is_to_end_tx = cb.next.execution_state_selector([ExecutionState::EndTx]); + cb.require_equal( + "Go to EndTx only when is_root", + cb.curr.state.is_root.expr(), + is_to_end_tx, + ); + + // When it's a root call. + cb.condition(cb.curr.state.is_root.expr(), |cb| { + // Do step state transition. + cb.require_step_state_transition(StepStateTransition { + call_id: Same, + rw_counter: Delta(1.expr() + cb.curr.state.reversible_write_counter.expr()), + ..StepStateTransition::any() + }); + }); + + // When it is an internal call, need to restore caller's state as finishing this + // call. Restore caller state to next StepState. + let restore_context = cb.condition(1.expr() - cb.curr.state.is_root.expr(), |cb| { + RestoreContextGadget::construct( + cb, + 0.expr(), + // rw_offset is handled in construct internally + 0.expr(), + 0.expr(), + 0.expr(), + 0.expr(), + 0.expr(), + ) + }); + + Self { + opcode, + op_gt_20, + op_lt_30, + op_gt_48, + op_lt_50, + op_gt_a4, + op_lt_f0, + restore_context, + } + } + + fn assign_exec_step( + &self, + region: &mut CachedRegion<'_, '_, F>, + offset: usize, + block: &Block, + _: &Transaction, + call: &Call, + step: &ExecStep, + ) -> Result<(), Error> { + let opcode = F::from(step.opcode.unwrap().as_u64()); + + self.opcode.assign(region, offset, Value::known(opcode))?; + + self.op_gt_20 + .assign(region, offset, F::from(0x20), opcode)?; + self.op_lt_30 + .assign(region, offset, opcode, F::from(0x30))?; + self.op_gt_48 + .assign(region, offset, F::from(0x48), opcode)?; + self.op_lt_50 + .assign(region, offset, opcode, F::from(0x50))?; + self.op_gt_a4 + .assign(region, offset, F::from(0xa4), opcode)?; + self.op_lt_f0 + .assign(region, offset, opcode, F::from(0xf0))?; + + self.restore_context + .assign(region, offset, block, call, step, 1) + } +} + +#[cfg(test)] +mod test { + use crate::evm_circuit::test::rand_bytes; + use crate::test_util::run_test_circuits; + use eth_types::bytecode::Bytecode; + use eth_types::{bytecode, ToWord, Word}; + use itertools::Itertools; + use lazy_static::lazy_static; + use mock::TestContext; + + const TESTING_CALL_DATA_PAIRS: [(usize, usize); 2] = [(0x20, 0x00), (0x1010, 0xff)]; + + lazy_static! { + static ref TESTING_INVALID_CODES: [Vec; 14] = [ + // Single invalid opcode + vec![0x0e], + vec![0x1f], + vec![0x21], + vec![0x4f], + vec![0xa5], + vec![0xb0], + vec![0xc0], + vec![0xd0], + vec![0xe0], + vec![0xf6], + vec![0xfb], + vec![0xfe], + // Multiple invalid opcodes + vec![0x5c, 0x5d, 0x5e, 0x5f], + // Many duplicate invalid opcodes + vec![0x22; 256], + ]; + } + + #[test] + fn invalid_opcode_root() { + for invalid_code in TESTING_INVALID_CODES.iter() { + test_root_ok(invalid_code); + } + } + + #[test] + fn invalid_opcode_internal() { + for ((call_data_offset, call_data_length), invalid_opcode) in TESTING_CALL_DATA_PAIRS + .iter() + .cartesian_product(TESTING_INVALID_CODES.iter()) + { + test_internal_ok(*call_data_offset, *call_data_length, invalid_opcode); + } + } + + fn test_root_ok(invalid_code: &[u8]) { + let mut code = Bytecode::default(); + invalid_code.iter().for_each(|b| { + code.write(*b, true); + }); + + assert_eq!( + run_test_circuits( + TestContext::<2, 1>::simple_ctx_with_bytecode(code).unwrap(), + None + ), + Ok(()) + ); + } + + fn test_internal_ok(call_data_offset: usize, call_data_length: usize, invalid_code: &[u8]) { + let (addr_a, addr_b) = (mock::MOCK_ACCOUNTS[0], mock::MOCK_ACCOUNTS[1]); + + // Code B gets called by code A, so the call is an internal call. + let mut code_b = Bytecode::default(); + invalid_code.iter().for_each(|b| { + code_b.write(*b, true); + }); + + // code A calls code B. + let pushdata = rand_bytes(8); + let code_a = bytecode! { + // populate memory in A's context. + PUSH8(Word::from_big_endian(&pushdata)) + PUSH1(0x00) // offset + MSTORE + // call ADDR_B. + PUSH1(0x00) // retLength + PUSH1(0x00) // retOffset + PUSH32(call_data_length) // argsLength + PUSH32(call_data_offset) // argsOffset + PUSH1(0x00) // value + PUSH32(addr_b.to_word()) // addr + PUSH32(0x1_0000) // gas + CALL + STOP + }; + + let ctx = TestContext::<3, 1>::new( + None, + |accs| { + accs[0].address(addr_b).code(code_b); + accs[1].address(addr_a).code(code_a); + accs[2] + .address(mock::MOCK_ACCOUNTS[3]) + .balance(Word::from(1_u64 << 20)); + }, + |mut txs, accs| { + txs[0].to(accs[1].address).from(accs[2].address); + }, + |block, _tx| block, + ) + .unwrap(); + + assert_eq!(run_test_circuits(ctx, None), Ok(())); + } +} diff --git a/zkevm-circuits/src/evm_circuit/execution/error_oog_call.rs b/zkevm-circuits/src/evm_circuit/execution/error_oog_call.rs index c1713709df..5664d1528b 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_oog_call.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_oog_call.rs @@ -32,6 +32,7 @@ pub(crate) struct ErrorOOGCallGadget { is_warm: Cell, balance: Word, insufficient_gas: LtGadget, + rw_counter_end_of_reversion: Cell, restore_context: RestoreContextGadget, } @@ -51,6 +52,7 @@ impl ExecutionGadget for ErrorOOGCallGadget { OpcodeId::CALL.expr(), ); + let rw_counter_end_of_reversion = cb.query_cell(); let tx_id = cb.call_context(None, CallContextFieldTag::TxId); let is_static = cb.call_context(None, CallContextFieldTag::IsStatic); let call_gadget = CommonCallGadget::construct(cb, 1.expr(), 0.expr(), 0.expr()); @@ -84,6 +86,13 @@ impl ExecutionGadget for ErrorOOGCallGadget { // current call must be failed. cb.call_context_lookup(false.expr(), None, CallContextFieldTag::IsSuccess, 0.expr()); + cb.call_context_lookup( + false.expr(), + None, + CallContextFieldTag::RwCounterEndOfReversion, + rw_counter_end_of_reversion.expr(), + ); + // Go to EndTx only when is_root let is_to_end_tx = cb.next.execution_state_selector([ExecutionState::EndTx]); cb.require_equal( @@ -97,7 +106,7 @@ impl ExecutionGadget for ErrorOOGCallGadget { // Do step state transition cb.require_step_state_transition(StepStateTransition { call_id: Same, - rw_counter: Delta(14.expr() + cb.curr.state.reversible_write_counter.expr()), + rw_counter: Delta(15.expr() + cb.curr.state.reversible_write_counter.expr()), ..StepStateTransition::any() }); @@ -117,6 +126,15 @@ impl ExecutionGadget for ErrorOOGCallGadget { ) }); + // constrain RwCounterEndOfReversion + let rw_counter_end_of_step = + cb.curr.state.rw_counter.expr() + cb.rw_counter_offset() - 1.expr(); + cb.require_equal( + "rw_counter_end_of_reversion = rw_counter_end_of_step + reversible_counter", + rw_counter_end_of_reversion.expr(), + rw_counter_end_of_step + cb.curr.state.reversible_write_counter.expr(), + ); + Self { opcode, tx_id, @@ -125,6 +143,7 @@ impl ExecutionGadget for ErrorOOGCallGadget { is_warm, balance, insufficient_gas, + rw_counter_end_of_reversion, restore_context, } } @@ -214,8 +233,14 @@ impl ExecutionGadget for ErrorOOGCallGadget { Value::known(F::from(gas_cost)), )?; + self.rw_counter_end_of_reversion.assign( + region, + offset, + Value::known(F::from(call.rw_counter_end_of_reversion as u64)), + )?; + self.restore_context - .assign(region, offset, block, call, step, 14)?; + .assign(region, offset, block, call, step, 15)?; Ok(()) } } diff --git a/zkevm-circuits/src/evm_circuit/execution/error_oog_constant.rs b/zkevm-circuits/src/evm_circuit/execution/error_oog_constant.rs index f51b799ad1..31294d574a 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_oog_constant.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_oog_constant.rs @@ -24,6 +24,7 @@ pub(crate) struct ErrorOOGConstantGadget { // constrain gas left is less than required gas_required: Cell, insufficient_gas: LtGadget, + rw_counter_end_of_reversion: Cell, restore_context: RestoreContextGadget, } @@ -37,6 +38,7 @@ impl ExecutionGadget for ErrorOOGConstantGadget { cb.opcode_lookup(opcode.expr(), 1.expr()); let gas_required = cb.query_cell(); + let rw_counter_end_of_reversion = cb.query_cell(); cb.constant_gas_lookup(opcode.expr(), gas_required.expr()); // Check if the amount of gas available is less than the amount of gas @@ -52,6 +54,13 @@ impl ExecutionGadget for ErrorOOGConstantGadget { // current call must be failed. cb.call_context_lookup(false.expr(), None, CallContextFieldTag::IsSuccess, 0.expr()); + cb.call_context_lookup( + false.expr(), + None, + CallContextFieldTag::RwCounterEndOfReversion, + rw_counter_end_of_reversion.expr(), + ); + // Go to EndTx only when is_root let is_to_end_tx = cb.next.execution_state_selector([ExecutionState::EndTx]); cb.require_equal( @@ -65,7 +74,7 @@ impl ExecutionGadget for ErrorOOGConstantGadget { // Do step state transition cb.require_step_state_transition(StepStateTransition { call_id: Same, - rw_counter: Delta(1.expr() + cb.curr.state.reversible_write_counter.expr()), + rw_counter: Delta(2.expr() + cb.curr.state.reversible_write_counter.expr()), ..StepStateTransition::any() }); }); @@ -85,10 +94,20 @@ impl ExecutionGadget for ErrorOOGConstantGadget { ) }); + // constrain RwCounterEndOfReversion + let rw_counter_end_of_step = + cb.curr.state.rw_counter.expr() + cb.rw_counter_offset() - 1.expr(); + cb.require_equal( + "rw_counter_end_of_reversion = rw_counter_end_of_step + reversible_counter", + rw_counter_end_of_reversion.expr(), + rw_counter_end_of_step + cb.curr.state.reversible_write_counter.expr(), + ); + Self { opcode, gas_required, insufficient_gas, + rw_counter_end_of_reversion, restore_context, } } @@ -118,8 +137,14 @@ impl ExecutionGadget for ErrorOOGConstantGadget { F::from(step.gas_cost), )?; + self.rw_counter_end_of_reversion.assign( + region, + offset, + Value::known(F::from(call.rw_counter_end_of_reversion as u64)), + )?; + self.restore_context - .assign(region, offset, block, call, step, 1)?; + .assign(region, offset, block, call, step, 2)?; Ok(()) } diff --git a/zkevm-circuits/src/evm_circuit/execution/error_stack.rs b/zkevm-circuits/src/evm_circuit/execution/error_stack.rs index e534b669c8..f91c0f0d02 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_stack.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_stack.rs @@ -26,6 +26,7 @@ pub(crate) struct ErrorStackGadget { max_stack_pointer: Cell, is_overflow: LtGadget, is_underflow: LtGadget, + rw_counter_end_of_reversion: Cell, restore_context: RestoreContextGadget, } @@ -40,6 +41,7 @@ impl ExecutionGadget for ErrorStackGadget { let min_stack_pointer = cb.query_cell(); let max_stack_pointer = cb.query_cell(); + let rw_counter_end_of_reversion = cb.query_cell(); cb.opcode_stack_lookup( opcode.expr(), @@ -71,6 +73,13 @@ impl ExecutionGadget for ErrorStackGadget { // current call must be failed. cb.call_context_lookup(false.expr(), None, CallContextFieldTag::IsSuccess, 0.expr()); + cb.call_context_lookup( + false.expr(), + None, + CallContextFieldTag::RwCounterEndOfReversion, + rw_counter_end_of_reversion.expr(), + ); + // Go to EndTx only when is_root let is_to_end_tx = cb.next.execution_state_selector([ExecutionState::EndTx]); cb.require_equal( @@ -84,7 +93,7 @@ impl ExecutionGadget for ErrorStackGadget { // Do step state transition cb.require_step_state_transition(StepStateTransition { call_id: Same, - rw_counter: Delta(1.expr() + cb.curr.state.reversible_write_counter.expr()), + rw_counter: Delta(2.expr() + cb.curr.state.reversible_write_counter.expr()), ..StepStateTransition::any() }); }); @@ -104,12 +113,22 @@ impl ExecutionGadget for ErrorStackGadget { ) }); + // constrain RwCounterEndOfReversion + let rw_counter_end_of_step = + cb.curr.state.rw_counter.expr() + cb.rw_counter_offset() - 1.expr(); + cb.require_equal( + "rw_counter_end_of_reversion = rw_counter_end_of_step + reversible_counter", + rw_counter_end_of_reversion.expr(), + rw_counter_end_of_step + cb.curr.state.reversible_write_counter.expr(), + ); + Self { opcode, min_stack_pointer, max_stack_pointer, is_overflow, is_underflow, + rw_counter_end_of_reversion, restore_context, } } @@ -148,8 +167,13 @@ impl ExecutionGadget for ErrorStackGadget { F::from(step.stack_pointer as u64), )?; + self.rw_counter_end_of_reversion.assign( + region, + offset, + Value::known(F::from(call.rw_counter_end_of_reversion as u64)), + )?; self.restore_context - .assign(region, offset, block, call, step, 1)?; + .assign(region, offset, block, call, step, 2)?; Ok(()) } diff --git a/zkevm-circuits/src/super_circuit.rs b/zkevm-circuits/src/super_circuit.rs index 29421c3b30..c47b33419c 100644 --- a/zkevm-circuits/src/super_circuit.rs +++ b/zkevm-circuits/src/super_circuit.rs @@ -111,6 +111,7 @@ pub struct SuperCircuit< const MAX_TXS: usize, const MAX_CALLDATA: usize, const MAX_RWS: usize, + const MAX_COPY_ROWS: usize, > { /// EVM Circuit pub evm_circuit: EvmCircuit, @@ -130,8 +131,13 @@ pub struct SuperCircuit< pub keccak_circuit: KeccakCircuit, } -impl - SuperCircuit +impl< + F: Field, + const MAX_TXS: usize, + const MAX_CALLDATA: usize, + const MAX_RWS: usize, + const MAX_COPY_ROWS: usize, + > SuperCircuit { /// Return the number of rows required to verify a given block pub fn get_num_rows_required(block: &Block) -> usize { @@ -145,8 +151,13 @@ impl Circuit - for SuperCircuit +impl< + F: Field, + const MAX_TXS: usize, + const MAX_CALLDATA: usize, + const MAX_RWS: usize, + const MAX_COPY_ROWS: usize, + > Circuit for SuperCircuit { type Config = SuperCircuitConfig; type FloorPlanner = SimpleFloorPlanner; @@ -305,8 +316,13 @@ impl - SuperCircuit +impl< + F: Field, + const MAX_TXS: usize, + const MAX_CALLDATA: usize, + const MAX_RWS: usize, + const MAX_COPY_ROWS: usize, + > SuperCircuit { /// From the witness data, generate a SuperCircuit instance with all of the /// sub-circuits filled with their corresponding witnesses. @@ -323,6 +339,7 @@ impl { + let circuit = SuperCircuit::<_, MAX_TXS, MAX_CALLDATA, MAX_RWS, MAX_COPY_ROWS> { evm_circuit, state_circuit, tx_circuit, @@ -423,17 +440,23 @@ mod super_circuit_tests { #[test] fn super_circuit_degree() { let mut cs = ConstraintSystem::::default(); - SuperCircuit::<_, 1, 32, 256>::configure(&mut cs); + SuperCircuit::<_, 1, 32, 256, 32>::configure(&mut cs); log::info!("super circuit degree: {}", cs.degree()); log::info!("super circuit minimum_rows: {}", cs.minimum_rows()); assert!(cs.degree() <= 9); } - fn test_super_circuit( + fn test_super_circuit< + const MAX_TXS: usize, + const MAX_CALLDATA: usize, + const MAX_RWS: usize, + const MAX_COPY_ROWS: usize, + >( block: GethData, ) { let (k, circuit, instance, _) = - SuperCircuit::::build(block).unwrap(); + SuperCircuit::::build(block) + .unwrap(); let prover = MockProver::run(k, &circuit, instance).unwrap(); let res = prover.verify_par(); if let Err(err) = res { @@ -537,7 +560,8 @@ mod super_circuit_tests { const MAX_TXS: usize = 1; const MAX_CALLDATA: usize = 32; const MAX_RWS: usize = 256; - test_super_circuit::(block); + const MAX_COPY_ROWS: usize = 256; + test_super_circuit::(block); } #[ignore] #[test] @@ -546,7 +570,8 @@ mod super_circuit_tests { const MAX_TXS: usize = 2; const MAX_CALLDATA: usize = 32; const MAX_RWS: usize = 256; - test_super_circuit::(block); + const MAX_COPY_ROWS: usize = 256; + test_super_circuit::(block); } #[ignore] #[test] @@ -555,6 +580,7 @@ mod super_circuit_tests { const MAX_TXS: usize = 2; const MAX_CALLDATA: usize = 32; const MAX_RWS: usize = 256; - test_super_circuit::(block); + const MAX_COPY_ROWS: usize = 256; + test_super_circuit::(block); } } diff --git a/zkevm-circuits/src/witness/block.rs b/zkevm-circuits/src/witness/block.rs index 8a633ee6c5..a839a1cfe5 100644 --- a/zkevm-circuits/src/witness/block.rs +++ b/zkevm-circuits/src/witness/block.rs @@ -41,8 +41,6 @@ pub struct Block { pub evm_circuit_pad_to: usize, /// Pad exponentiation circuit to make selectors fixed. pub exp_circuit_pad_to: usize, - /// Pad copy circuit to make selectors fixed. - pub copy_circuit_pad_to: usize, /// Circuit Setup Parameters pub circuits_params: CircuitsParams, /// Inputs to the SHA3 opcode @@ -203,7 +201,6 @@ pub fn block_convert( circuits_params: block.circuits_params.clone(), evm_circuit_pad_to: ::default(), exp_circuit_pad_to: ::default(), - copy_circuit_pad_to: ::default(), prev_state_root: block.prev_state_root, keccak_inputs: circuit_input_builder::keccak_inputs(block, code_db)?, eth_block: block.eth_block.clone(),