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

Commit

Permalink
Assign correct nonce in TestContext (#1464)
Browse files Browse the repository at this point in the history 
### Description

This PR changes the way TestContext sets tx nonces. Before nonce is set
serially. Applying this PR causes the tx to use nonce from the accounts
config.

### Issue Link


#1461

### 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

- Assigns correct nonce just after `func_tx` is called.
- Adds a test case to ensure that the nonces are assigned as expected.

### Rationale

The automatic assignment only works if all txs use same account and its
account nonce is zero. Otherwise dev has to manually override the nonce.
Inferring the nonce from account state adds convenience such that it
automatically works for same accounts as well as different accounts.

### How Has This Been Tested?

A test case has been added.
  • Loading branch information
@zemse
zemse authored Jun 20, 2023
1 parent 1e1d104 commit aa5cbb1
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 42 deletions.
34 changes: 16 additions & 18 deletions bus-mapping/src/circuit_input_builder/tracer_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ fn tracer_err_insufficient_balance() {
},
|mut txs, accs| {
txs[0].to(accs[0].address).from(accs[2].address);
txs[1].to(accs[1].address).from(accs[2].address).nonce(1);
txs[1].to(accs[1].address).from(accs[2].address);
},
|block, _tx| block.number(0xcafeu64),
LoggerConfig::enable_memory(),
Expand Down Expand Up @@ -405,7 +405,7 @@ fn tracer_err_address_collision() {
},
|mut txs, accs| {
txs[0].to(accs[0].address).from(accs[2].address);
txs[1].to(accs[1].address).from(accs[2].address).nonce(1);
txs[1].to(accs[1].address).from(accs[2].address);
},
|block, _tx| block.number(0xcafeu64),
LoggerConfig::enable_memory(),
Expand Down Expand Up @@ -525,7 +525,7 @@ fn tracer_create_collision_free() {
},
|mut txs, accs| {
txs[0].to(accs[0].address).from(accs[2].address);
txs[1].to(accs[1].address).from(accs[2].address).nonce(1);
txs[1].to(accs[1].address).from(accs[2].address);
},
|block, _tx| block.number(0xcafeu64),
LoggerConfig::enable_memory(),
Expand Down Expand Up @@ -655,7 +655,7 @@ fn tracer_err_code_store_out_of_gas() {
},
|mut txs, accs| {
txs[0].to(accs[0].address).from(accs[2].address);
txs[1].to(accs[1].address).from(accs[2].address).nonce(1);
txs[1].to(accs[1].address).from(accs[2].address);
},
|block, _tx| block.number(0xcafeu64),
LoggerConfig::enable_memory(),
Expand Down Expand Up @@ -705,7 +705,6 @@ fn tracer_err_code_store_out_of_gas_tx_deploy() {
txs[0]
.from(accs[1].address)
.gas(55000u64.into())
.nonce(0)
.input(code_creator.into());
},
|block, _tx| block.number(0x0264),
Expand Down Expand Up @@ -804,7 +803,7 @@ fn tracer_err_invalid_code() {
},
|mut txs, accs| {
txs[0].to(accs[0].address).from(accs[2].address);
txs[1].to(accs[1].address).from(accs[2].address).nonce(1);
txs[1].to(accs[1].address).from(accs[2].address);
},
|block, _tx| block.number(0xcafeu64),
LoggerConfig::enable_memory(),
Expand Down Expand Up @@ -901,7 +900,7 @@ fn tracer_err_max_code_size_exceeded() {
},
|mut txs, accs| {
txs[0].to(accs[0].address).from(accs[2].address);
txs[1].to(accs[1].address).from(accs[2].address).nonce(1);
txs[1].to(accs[1].address).from(accs[2].address);
},
|block, _tx| block.number(0xcafeu64),
LoggerConfig::enable_memory(),
Expand Down Expand Up @@ -951,7 +950,6 @@ fn tracer_err_max_code_size_exceeded_tx_deploy() {
txs[0]
.from(accs[1].address)
.gas(60000u64.into())
.nonce(0)
.input(code_creator.into());
},
|block, _tx| block.number(0x0264),
Expand Down Expand Up @@ -1042,7 +1040,7 @@ fn tracer_create_stop() {
},
|mut txs, accs| {
txs[0].to(accs[0].address).from(accs[2].address);
txs[1].to(accs[1].address).from(accs[2].address).nonce(1);
txs[1].to(accs[1].address).from(accs[2].address);
},
|block, _tx| block.number(0xcafeu64),
LoggerConfig::enable_memory(),
Expand Down Expand Up @@ -1160,7 +1158,7 @@ fn tracer_err_invalid_jump() {
},
|mut txs, accs| {
txs[0].to(accs[0].address).from(accs[2].address);
txs[1].to(accs[1].address).from(accs[2].address).nonce(1);
txs[1].to(accs[1].address).from(accs[2].address);
},
|block, _tx| block.number(0xcafeu64),
LoggerConfig::enable_memory(),
Expand Down Expand Up @@ -1260,7 +1258,7 @@ fn tracer_err_execution_reverted() {
},
|mut txs, accs| {
txs[0].to(accs[0].address).from(accs[2].address);
txs[1].to(accs[1].address).from(accs[2].address).nonce(1);
txs[1].to(accs[1].address).from(accs[2].address);
},
|block, _tx| block.number(0xcafeu64),
LoggerConfig::enable_memory(),
Expand Down Expand Up @@ -1319,7 +1317,7 @@ fn tracer_stop() {
},
|mut txs, accs| {
txs[0].to(accs[0].address).from(accs[2].address);
txs[1].to(accs[1].address).from(accs[2].address).nonce(1);
txs[1].to(accs[1].address).from(accs[2].address);
},
|block, _tx| block.number(0xcafeu64),
LoggerConfig::enable_memory(),
Expand Down Expand Up @@ -1387,7 +1385,7 @@ fn tracer_err_return_data_out_of_bounds() {
},
|mut txs, accs| {
txs[0].to(accs[0].address).from(accs[2].address);
txs[1].to(accs[1].address).from(accs[2].address).nonce(1);
txs[1].to(accs[1].address).from(accs[2].address);
},
|block, _tx| block.number(0xcafeu64),
LoggerConfig::enable_memory(),
Expand Down Expand Up @@ -1540,7 +1538,7 @@ fn tracer_err_write_protection(is_call: bool) {
},
|mut txs, accs| {
txs[0].to(accs[0].address).from(accs[2].address);
txs[1].to(accs[1].address).from(accs[2].address).nonce(1);
txs[1].to(accs[1].address).from(accs[2].address);
},
|block, _tx| block.number(0xcafeu64),
LoggerConfig::enable_memory(),
Expand Down Expand Up @@ -1742,7 +1740,7 @@ fn create2_address() {
},
|mut txs, accs| {
txs[0].to(accs[0].address).from(accs[2].address);
txs[1].to(accs[1].address).from(accs[2].address).nonce(1);
txs[1].to(accs[1].address).from(accs[2].address);
},
|block, _tx| block.number(0xcafeu64),
LoggerConfig::enable_memory(),
Expand Down Expand Up @@ -1840,7 +1838,7 @@ fn create_address() {
},
|mut txs, accs| {
txs[0].to(accs[0].address).from(accs[2].address);
txs[1].to(accs[1].address).from(accs[2].address).nonce(1);
txs[1].to(accs[1].address).from(accs[2].address);
},
|block, _tx| block.number(0xcafeu64),
LoggerConfig::enable_memory(),
Expand Down Expand Up @@ -1924,7 +1922,7 @@ fn test_gen_access_trace() {
},
|mut txs, accs| {
txs[0].to(accs[0].address).from(accs[2].address);
txs[1].to(accs[1].address).from(accs[2].address).nonce(1);
txs[1].to(accs[1].address).from(accs[2].address);
},
|block, _tx| block.number(0xcafeu64),
LoggerConfig::enable_memory(),
Expand Down Expand Up @@ -2146,7 +2144,7 @@ fn test_gen_access_trace_create_push_call_stack() {
},
|mut txs, accs| {
txs[0].to(accs[0].address).from(accs[2].address);
txs[1].to(accs[1].address).from(accs[2].address).nonce(1);
txs[1].to(accs[1].address).from(accs[2].address);
},
|block, _tx| block.number(0xcafeu64),
LoggerConfig::enable_memory(),
Expand Down
76 changes: 62 additions & 14 deletions mock/src/test_ctx.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,7 @@ pub use external_tracer::LoggerConfig;
/// txs[0].to(accs[0].address).from(accs[2].address);
/// txs[1]
/// .to(accs[1].address)
/// .from(accs[2].address)
/// .nonce(1);
/// .from(accs[2].address);
/// },
/// |block, _tx| block.number(0xcafeu64),
/// )
Expand Down Expand Up @@ -132,22 +131,26 @@ impl<const NACC: usize, const NTX: usize> TestContext<NACC, NTX> {
.expect("Mismatched acc len");

let mut transactions = vec![MockTransaction::default(); NTX];
// By default, set the TxIndex and the Nonce values of the multiple transactions
// of the context correlative so that any Ok test passes by default.
// If the user decides to override these values, they'll then be set to whatever
// inputs were provided by the user.
transactions
.iter_mut()
.enumerate()
.skip(1)
.for_each(|(idx, tx)| {
let idx = u64::try_from(idx).expect("Unexpected idx conversion error");
tx.transaction_idx(idx).nonce(idx);
});
let tx_refs = transactions.iter_mut().collect();

// Build Tx modifiers.
func_tx(tx_refs, accounts.clone());

// Sets the transaction_idx and nonce after building the tx modifiers. Hence, if user has
// overridden these values above using the tx modifiers, that will be ignored.
let mut acc_tx_count = vec![0u64; NACC];
transactions.iter_mut().enumerate().for_each(|(idx, tx)| {
let idx = u64::try_from(idx).expect("Unexpected idx conversion error");
tx.transaction_idx(idx);
if let Some((pos, from_acc)) = accounts
.iter()
.find_position(|acc| acc.address == tx.from.address())
{
tx.nonce(from_acc.nonce + acc_tx_count[pos]);
acc_tx_count[pos] += 1;
}
});

let transactions: Vec<MockTransaction> =
transactions.iter_mut().map(|tx| tx.build()).collect();

Expand Down Expand Up @@ -279,3 +282,48 @@ pub mod helpers {
txs[0].from(accs[1].address).to(accs[0].address);
}
}

#[cfg(test)]
mod tests {
use eth_types::{address, U256, U64};

use super::{eth, TestContext};

#[test]
fn test_nonce() {
let block = TestContext::<2, 5>::new(
None,
|accs| {
accs[0]
.address(address!("0x0000000000000000000000000000000000000000"))
.balance(eth(10));
accs[1]
.address(address!("0x000000000000000000000000000000000cafe001"))
.balance(eth(10))
.nonce(100);
},
|mut txs, accs| {
txs[0].from(accs[0].address);
txs[1].from(accs[0].address);
txs[2].from(accs[1].address);
txs[3].from(accs[1].address);
txs[4].from(accs[1].address).nonce(12345); // set nonce here is ignored
},
|block, _tx| block.number(0xcafeu64),
)
.unwrap();

// account 0 starts with nonce default 0
assert_eq!(block.eth_block.transactions[0].nonce, U256::from(0));
assert_eq!(block.eth_block.transactions[1].nonce, U256::from(1));

// account 1 starts with nonce specified 100
assert_eq!(block.eth_block.transactions[2].nonce, U256::from(100));
assert_eq!(block.eth_block.transactions[3].nonce, U256::from(101));
assert_eq!(block.eth_block.transactions[4].nonce, U256::from(102)); // 12345 is ignored

// nonce in accounts is the nonce before the block processing
assert_eq!(block.accounts[0].nonce, U64::from(0));
assert_eq!(block.accounts[1].nonce, U64::from(100));
}
}
8 changes: 4 additions & 4 deletions mock/src/transaction.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,8 +206,8 @@ impl MockTransaction {
self
}

/// Set nonce field for the MockTransaction.
pub fn nonce(&mut self, nonce: u64) -> &mut Self {
/// Set nonce field for the MockTransaction. Overridden in TestContext.
pub(crate) fn nonce(&mut self, nonce: u64) -> &mut Self {
self.nonce = nonce;
self
}
Expand All @@ -224,8 +224,8 @@ impl MockTransaction {
self
}

/// Set transaction_idx field for the MockTransaction.
pub fn transaction_idx(&mut self, transaction_idx: u64) -> &mut Self {
/// Set transaction_idx field for the MockTransaction. Overridden in TestContext.
pub(crate) fn transaction_idx(&mut self, transaction_idx: u64) -> &mut Self {
self.transaction_index = U64::from(transaction_idx);
self
}
Expand Down
3 changes: 1 addition & 2 deletions zkevm-circuits/src/evm_circuit/execution/begin_tx.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -740,7 +740,7 @@ mod test {
accs[1].address(from).balance(eth(1)).nonce(multibyte_nonce);
},
|mut txs, _| {
txs[0].to(to).from(from).nonce(multibyte_nonce);
txs[0].to(to).from(from);
},
|block, _| block,
)
Expand Down Expand Up @@ -841,7 +841,6 @@ mod test {
|mut txs, _accs| {
txs[0]
.from(MOCK_ACCOUNTS[0])
.nonce(nonce)
.gas_price(gwei(2))
.gas(Word::from(0x10000))
.value(eth(2))
Expand Down
6 changes: 2 additions & 4 deletions zkevm-circuits/src/evm_circuit/execution/end_tx.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -385,8 +385,7 @@ mod test {
.to(accs[0].address)
.from(accs[1].address)
.gas(Word::from(30_000))
.gas_price(gwei(2))
.nonce(0);
.gas_price(gwei(2));
// txs[1]
// .to(accs[2].address)
// .from(accs[3].address)
Expand Down Expand Up @@ -457,8 +456,7 @@ mod test {
.to(accs[0].address)
.from(accs[1].address)
.gas(Word::from(50_000))
.gas_price(gwei(2))
.nonce(0);
.gas_price(gwei(2));
},
|block, _tx| block,
)
Expand Down

0 comments on commit aa5cbb1

Please sign in to comment.