Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: erroneous nonce updates #839

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
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
40 changes: 40 additions & 0 deletions crates/cheatcodes/assets/cheatcodes.json

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

8 changes: 8 additions & 0 deletions crates/cheatcodes/spec/src/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -908,6 +908,14 @@ interface Vm {
#[cheatcode(group = Testing, safety = Safe)]
function zkRegisterContract(string calldata name, bytes32 evmBytecodeHash, bytes calldata evmDeployedBytecode, bytes calldata evmBytecode, bytes32 zkBytecodeHash, bytes calldata zkDeployedBytecode) external pure;

/// Gets the transaction nonce of a zksync account.
#[cheatcode(group = Evm, safety = Safe)]
function zkGetTransactionNonce(address account) external view returns (uint64 nonce);

/// Gets the deployment nonce of a zksync account.
#[cheatcode(group = Evm, safety = Safe)]
function zkGetDeploymentNonce(address account) external view returns (uint64 nonce);

/// If the condition is false, discard this run's fuzz inputs and generate new ones.
#[cheatcode(group = Testing, safety = Safe)]
function assume(bool condition) external pure;
Expand Down
14 changes: 14 additions & 0 deletions crates/cheatcodes/src/evm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,20 @@ impl Cheatcode for getNonce_1Call {
}
}

impl Cheatcode for zkGetTransactionNonceCall {
fn apply_stateful(&self, ccx: &mut CheatsCtxt) -> Result {
let Self { account } = self;
get_nonce(ccx, account)
}
}

impl Cheatcode for zkGetDeploymentNonceCall {
fn apply_stateful(&self, ccx: &mut CheatsCtxt) -> Result {
let Self { account } = self;
get_nonce(ccx, account)
}
}

impl Cheatcode for loadCall {
fn apply_stateful(&self, ccx: &mut CheatsCtxt) -> Result {
let Self { target, slot } = *self;
Expand Down
82 changes: 66 additions & 16 deletions crates/forge/tests/fixtures/zk/ScriptSetup.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,42 @@ pragma solidity ^0.8.13;
import {Script} from "forge-std/Script.sol";
import {Greeter} from "../src/Greeter.sol";

interface VmExt {
function zkGetTransactionNonce(
address account
) external view returns (uint64 nonce);
function zkGetDeploymentNonce(
address account
) external view returns (uint64 nonce);
}

contract ScriptSetupNonce is Script {
VmExt internal constant vmExt = VmExt(VM_ADDRESS);

function setUp() public {
uint256 initial_nonce = checkNonce(address(tx.origin));
uint256 initialNonceTx = checkTxNonce(address(tx.origin));
uint256 initialNonceDeploy = checkDeployNonce(address(tx.origin));
// Perform transactions and deploy contracts in setup to increment nonce and verify broadcast nonce matches onchain
new Greeter();
new Greeter();
new Greeter();
new Greeter();
assert(checkNonce(address(tx.origin)) == initial_nonce);
assert(checkTxNonce(address(tx.origin)) == initialNonceTx);
assert(checkDeployNonce(address(tx.origin)) == initialNonceDeploy);
}

function run() public {
// Get initial nonce
uint256 initial_nonce = checkNonce(address(tx.origin));
assert(initial_nonce == vm.getNonce(address(tx.origin)));
uint256 initialNonceTx = checkTxNonce(tx.origin);
uint256 initialNonceDeploy = checkDeployNonce(tx.origin);
assert(initialNonceTx == vmExt.zkGetTransactionNonce(tx.origin));
assert(initialNonceDeploy == vmExt.zkGetDeploymentNonce(tx.origin));

// Create and interact with non-broadcasted contract to verify nonce is not incremented
Greeter notBroadcastGreeter = new Greeter();
notBroadcastGreeter.greeting("john");
assert(checkNonce(address(tx.origin)) == initial_nonce);
assert(checkTxNonce(tx.origin) == initialNonceTx);
assert(checkDeployNonce(tx.origin) == initialNonceDeploy);

// Start broadcasting transactions
vm.startBroadcast();
Expand All @@ -33,37 +49,71 @@ contract ScriptSetupNonce is Script {

// Deploy checker and verify nonce
NonceChecker checker = new NonceChecker();

vm.stopBroadcast();

// We expect the nonce to be incremented by 1 because the check is done in an external
// call
checker.assertNonce(vm.getNonce(address(tx.origin)) + 1);
vm.stopBroadcast();
checker.assertTxNonce(
vmExt.zkGetTransactionNonce(address(tx.origin)) + 1
);
checker.assertDeployNonce(
vmExt.zkGetDeploymentNonce(address(tx.origin))
);
}

function checkNonce(address addr) public returns (uint256) {
function checkTxNonce(address addr) public returns (uint256) {
// We prank here to avoid accidentally "polluting" the nonce of `addr` during the call
// for example when `addr` is `tx.origin`
vm.prank(address(this), address(this));
return NonceLib.getNonce(addr);
return NonceLib.getTxNonce(addr);
}

function checkDeployNonce(address addr) public returns (uint256) {
// We prank here to avoid accidentally "polluting" the nonce of `addr` during the call
// for example when `addr` is `tx.origin`
vm.prank(address(this), address(this));
return NonceLib.getDeployNonce(addr);
}
}

contract NonceChecker {
function checkNonce() public returns (uint256) {
return NonceLib.getNonce(address(tx.origin));
function checkTxNonce() public returns (uint256) {
return NonceLib.getTxNonce(address(tx.origin));
}

function checkDeployNonce() public returns (uint256) {
return NonceLib.getDeployNonce(address(tx.origin));
}

function assertTxNonce(uint256 expected) public {
uint256 real_nonce = checkTxNonce();
require(real_nonce == expected, "tx nonce mismatch");
}

function assertNonce(uint256 expected) public {
uint256 real_nonce = checkNonce();
require(real_nonce == expected, "Nonce mismatch");
function assertDeployNonce(uint256 expected) public {
uint256 real_nonce = checkDeployNonce();
require(real_nonce == expected, "deploy nonce mismatch");
}
}

library NonceLib {
address constant NONCE_HOLDER = address(0x8003);

/// Retrieve tx nonce for `addr` from the NONCE_HOLDER system contract
function getNonce(address addr) internal returns (uint256) {
(bool success, bytes memory data) = NONCE_HOLDER.call(abi.encodeWithSignature("getMinNonce(address)", addr));
function getTxNonce(address addr) internal returns (uint256) {
(bool success, bytes memory data) = NONCE_HOLDER.call(
abi.encodeWithSignature("getMinNonce(address)", addr)
);
require(success, "Failed to get nonce");
return abi.decode(data, (uint256));
}

/// Retrieve tx nonce for `addr` from the NONCE_HOLDER system contract
function getDeployNonce(address addr) internal returns (uint256) {
(bool success, bytes memory data) = NONCE_HOLDER.call(
abi.encodeWithSignature("getDeploymentNonce(address)", addr)
);
require(success, "Failed to get nonce");
return abi.decode(data, (uint256));
}
Expand Down
1 change: 1 addition & 0 deletions crates/forge/tests/it/zk/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,5 @@ mod paymaster;
mod proxy;
mod repros;
mod script;
mod sender;
mod traces;
2 changes: 1 addition & 1 deletion crates/forge/tests/it/zk/nonce.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ forgetest_async!(setup_block_on_script_test, |prj, cmd| {
"./script/ScriptSetup.s.sol",
"ScriptSetupNonce",
None,
4,
3,
Some(&["-vvvvv"]),
)
.await;
Expand Down
25 changes: 25 additions & 0 deletions crates/forge/tests/it/zk/sender.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
//! Forge tests for basic zkysnc functionality.

use crate::{config::*, test_helpers::TEST_DATA_DEFAULT};
use forge::revm::primitives::SpecId;
use foundry_test_utils::Filter;

#[tokio::test(flavor = "multi_thread")]
async fn test_zk_correctly_updates_sender_nonce() {
let runner = TEST_DATA_DEFAULT.runner_zksync();
let filter = Filter::new(
"testZkTxSenderNoncesAreConsistent|testZkTxSenderNoncesAreConsistentInBroadcast",
"ZkTxSenderTest",
".*",
);

TestConfig::with_filter(runner, filter).spec_id(SpecId::SHANGHAI).run().await;
}

#[tokio::test(flavor = "multi_thread")]
async fn test_zk_correctly_updates_sender_balance() {
let runner = TEST_DATA_DEFAULT.runner_zksync();
let filter = Filter::new("testZkTxSenderBalancesAreConsistent", "ZkTxSenderTest", ".*");

TestConfig::with_filter(runner, filter).spec_id(SpecId::SHANGHAI).run().await;
}
6 changes: 1 addition & 5 deletions crates/strategy/zksync/src/cheatcode/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use foundry_zksync_compilers::dual_compiled_contracts::{
use foundry_zksync_core::{vm::ZkEnv, ZkPaymasterData, H256};
use revm::primitives::Bytecode;

use super::types::{ZkPersistNonceUpdate, ZkStartupMigration};
use super::types::ZkStartupMigration;

/// Context for [ZksyncCheatcodeInspectorStrategyRunner].
#[derive(Debug, Default, Clone)]
Expand Down Expand Up @@ -48,9 +48,6 @@ pub struct ZksyncCheatcodeInspectorStrategyContext {
/// providing the necessary level of isolation.
pub persisted_factory_deps: HashMap<H256, Vec<u8>>,

/// Nonce update persistence behavior in zkEVM for the tx caller.
pub zk_persist_nonce_update: ZkPersistNonceUpdate,

/// Stores the factory deps that were detected as part of CREATE2 deployer call.
/// Must be cleared every call.
pub set_deployer_call_input_factory_deps: Vec<Vec<u8>>,
Expand Down Expand Up @@ -108,7 +105,6 @@ impl ZksyncCheatcodeInspectorStrategyContext {
zk_startup_migration: ZkStartupMigration::Defer,
zk_use_factory_deps: Default::default(),
persisted_factory_deps: Default::default(),
zk_persist_nonce_update: Default::default(),
set_deployer_call_input_factory_deps: Default::default(),
zk_env,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ use foundry_cheatcodes::{
createFork_0Call, createFork_1Call, createFork_2Call, createSelectFork_0Call,
createSelectFork_1Call, createSelectFork_2Call, dealCall, etchCall, getCodeCall,
getNonce_0Call, mockCallRevert_0Call, mockCall_0Call, resetNonceCall, rollCall,
selectForkCall, setNonceCall, setNonceUnsafeCall, warpCall, zkRegisterContractCall,
zkUseFactoryDepCall, zkUsePaymasterCall, zkVmCall, zkVmSkipCall,
selectForkCall, setNonceCall, setNonceUnsafeCall, warpCall, zkGetDeploymentNonceCall,
zkGetTransactionNonceCall, zkRegisterContractCall, zkUseFactoryDepCall, zkUsePaymasterCall,
zkVmCall, zkVmSkipCall,
},
};
use foundry_evm::backend::LocalForkId;
Expand Down Expand Up @@ -100,6 +101,26 @@ impl ZksyncCheatcodeInspectorStrategyRunner {
let nonce = foundry_zksync_core::cheatcodes::get_nonce(account, ccx.ecx);
Ok(nonce.abi_encode())
}
t if using_zk_vm && is::<zkGetTransactionNonceCall>(t) => {
let &zkGetTransactionNonceCall { account } =
cheatcode.as_any().downcast_ref().unwrap();

info!(?account, "cheatcode zkGetTransactionNonce");

let (tx_nonce, _) =
foundry_zksync_core::cheatcodes::get_full_nonce(account, ccx.ecx);
Ok(tx_nonce.abi_encode())
}
t if using_zk_vm && is::<zkGetDeploymentNonceCall>(t) => {
let &zkGetDeploymentNonceCall { account } =
cheatcode.as_any().downcast_ref().unwrap();

info!(?account, "cheatcode zkGetDeploymentNonce");

let (_, deploy_nonce) =
foundry_zksync_core::cheatcodes::get_full_nonce(account, ccx.ecx);
Ok(deploy_nonce.abi_encode())
}
t if using_zk_vm && is::<mockCall_0Call>(t) => {
let mockCall_0Call { callee, data, returnData } =
cheatcode.as_any().downcast_ref().unwrap();
Expand Down
Loading
Loading