From 2610f0c8eb9798734291716aa373d7e413fa70c3 Mon Sep 17 00:00:00 2001 From: Luca Steeb Date: Fri, 16 Aug 2024 17:57:37 +0100 Subject: [PATCH] skip nonce check on fee estimation (#2) Co-authored-by: Ammar Arif --- Cargo.lock | 4 +- Cargo.toml | 2 +- crates/executor/src/estimate.rs | 9 +- crates/executor/src/simulate.rs | 3 +- crates/rpc/src/v05/method/estimate_fee.rs | 8 +- crates/rpc/src/v06/method/estimate_fee.rs | 8 +- .../src/v06/method/estimate_message_fee.rs | 8 +- .../src/v06/method/simulate_transactions.rs | 33 ++++ crates/rpc/src/v07/method/estimate_fee.rs | 149 +++++++++++++++--- 9 files changed, 198 insertions(+), 26 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2c0d881f53..278f0772a2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -930,8 +930,7 @@ dependencies = [ [[package]] name = "blockifier" version = "0.8.0-rc.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7fb87d3c5d71fe593c120cd73036d3982986596942e3c1a5dc81e2661f397cf4" +source = "git+https://github.com/cartridge-gg/blockifier?branch=nonce-check-flag#442ac12942deff3a4ea9a7d71bf4fe1f71292447" dependencies = [ "anyhow", "ark-ec", @@ -954,6 +953,7 @@ dependencies = [ "num-rational", "num-traits 0.2.19", "once_cell", + "paste", "phf", "serde", "serde_json", diff --git a/Cargo.toml b/Cargo.toml index 213b6266b9..29e1badf26 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -53,7 +53,7 @@ axum = "0.7.5" base64 = "0.13.1" bincode = "2.0.0-rc.3" bitvec = "1.0.1" -blockifier = "=0.8.0-rc.1" +blockifier = { git = "https://github.com/cartridge-gg/blockifier", branch = "nonce-check-flag" } bloomfilter = "1.0.12" bytes = "1.4.0" cached = "0.44.0" diff --git a/crates/executor/src/estimate.rs b/crates/executor/src/estimate.rs index fc0731dd4b..3414b45c89 100644 --- a/crates/executor/src/estimate.rs +++ b/crates/executor/src/estimate.rs @@ -9,6 +9,7 @@ pub fn estimate( execution_state: ExecutionState<'_>, transactions: Vec, skip_validate: bool, + skip_nonce_check: bool, ) -> Result, TransactionExecutionError> { let block_number = execution_state.header.number; @@ -32,7 +33,13 @@ pub fn estimate( let tx_info: Result< blockifier::transaction::objects::TransactionExecutionInfo, blockifier::transaction::errors::TransactionExecutionError, - > = transaction.execute(&mut state, &block_context, false, !skip_validate); + > = transaction.execute( + &mut state, + &block_context, + false, + !skip_validate, + !skip_nonce_check, + ); match tx_info { Ok(tx_info) => { diff --git a/crates/executor/src/simulate.rs b/crates/executor/src/simulate.rs index c87e8aac08..d8abfc0386 100644 --- a/crates/executor/src/simulate.rs +++ b/crates/executor/src/simulate.rs @@ -108,6 +108,7 @@ pub fn simulate( &block_context, !skip_fee_charge, !skip_validate, + true, ); let state_diff = to_state_diff(&mut tx_state, transaction_declared_deprecated_class_hash)?; tx_state.commit(); @@ -185,7 +186,7 @@ pub fn trace( let mut tx_state = CachedState::<_>::create_transactional(&mut state); let tx_info = tx - .execute(&mut tx_state, &block_context, true, true) + .execute(&mut tx_state, &block_context, true, true, true) .map_err(|e| { // Update the cache with the error. Lock the cache before sending to avoid // race conditions between senders and receivers. diff --git a/crates/rpc/src/v05/method/estimate_fee.rs b/crates/rpc/src/v05/method/estimate_fee.rs index 6d0c670778..1e9ed30c25 100644 --- a/crates/rpc/src/v05/method/estimate_fee.rs +++ b/crates/rpc/src/v05/method/estimate_fee.rs @@ -142,7 +142,13 @@ pub async fn estimate_fee( .map(|tx| crate::executor::map_broadcasted_transaction(&tx, context.chain_id)) .collect::, _>>()?; - let result = pathfinder_executor::estimate(state, transactions, false)?; + let result = pathfinder_executor::estimate( + state, + transactions, + false, + // skip nonce check because it is not necessary for fee estimation + true, + )?; Ok::<_, EstimateFeeError>(result) }) diff --git a/crates/rpc/src/v06/method/estimate_fee.rs b/crates/rpc/src/v06/method/estimate_fee.rs index d0b5f33da0..8e4ab8ebb8 100644 --- a/crates/rpc/src/v06/method/estimate_fee.rs +++ b/crates/rpc/src/v06/method/estimate_fee.rs @@ -192,7 +192,13 @@ pub async fn estimate_fee_impl( .map(|tx| crate::executor::map_broadcasted_transaction(&tx, context.chain_id)) .collect::, _>>()?; - let result = pathfinder_executor::estimate(state, transactions, skip_validate)?; + let result = pathfinder_executor::estimate( + state, + transactions, + skip_validate, + // skip nonce check because it is not necessary for fee estimation + true, + )?; Ok::<_, EstimateFeeError>(result) }) diff --git a/crates/rpc/src/v06/method/estimate_message_fee.rs b/crates/rpc/src/v06/method/estimate_message_fee.rs index 81b8e65623..1954ba113d 100644 --- a/crates/rpc/src/v06/method/estimate_message_fee.rs +++ b/crates/rpc/src/v06/method/estimate_message_fee.rs @@ -152,7 +152,13 @@ pub(crate) async fn estimate_message_fee_impl( let transaction = create_executor_transaction(input, context.chain_id)?; - let result = pathfinder_executor::estimate(state, vec![transaction], false)?; + let result = pathfinder_executor::estimate( + state, + vec![transaction], + false, + // skip nonce check because it is not necessary for fee estimation + true, + )?; Ok::<_, EstimateMessageFeeError>(result) }) diff --git a/crates/rpc/src/v06/method/simulate_transactions.rs b/crates/rpc/src/v06/method/simulate_transactions.rs index 5b8e3a6d40..7ee6f550c7 100644 --- a/crates/rpc/src/v06/method/simulate_transactions.rs +++ b/crates/rpc/src/v06/method/simulate_transactions.rs @@ -902,6 +902,39 @@ pub(crate) mod tests { pretty_assertions_sorted::assert_eq!(result.0, expected); } + #[tokio::test] + async fn test_simulate_transaction_with_invalid_nonce_will_fail() { + let (context, _, _, _) = crate::test_setup::test_context().await; + + // create transaction with invalid nonce + let input_json = serde_json::json!({ + "block_id": {"block_number": 1}, + "transactions": [ + { + "contract_address_salt": "0x46c0d4abf0192a788aca261e58d7031576f7d8ea5229f452b0f23e691dd5971", + "max_fee": "0x0", + "signature": [], + "class_hash": DUMMY_ACCOUNT_CLASS_HASH, + "nonce": "0x1337", + "version": TransactionVersion::ONE_WITH_QUERY_VERSION, + "constructor_calldata": [], + "type": "DEPLOY_ACCOUNT" + } + ], + "simulation_flags": ["SKIP_FEE_CHARGE"] + }); + + // assert that the simulation returns an error due to invalid nonce + let input = SimulateTransactionInput::deserialize(&input_json).unwrap(); + let err = simulate_transactions(context, input).await.unwrap_err(); + + if let SimulateTransactionError::TransactionExecutionError { error, .. } = err { + assert!(error.contains("Invalid transaction nonce")) + } else { + panic!("Unexpected error") + } + } + #[tokio::test] async fn declare_cairo_v0_class() { pub const CAIRO0_DEFINITION: &[u8] = diff --git a/crates/rpc/src/v07/method/estimate_fee.rs b/crates/rpc/src/v07/method/estimate_fee.rs index 3a9fa64b1d..e43fb176c7 100644 --- a/crates/rpc/src/v07/method/estimate_fee.rs +++ b/crates/rpc/src/v07/method/estimate_fee.rs @@ -38,7 +38,10 @@ mod tests { use crate::v06::method::estimate_fee::*; use crate::v06::types::PriceUnit; - fn declare_transaction(account_contract_address: ContractAddress) -> BroadcastedTransaction { + fn declare_transaction( + account_contract_address: ContractAddress, + nonce: TransactionNonce, + ) -> BroadcastedTransaction { let sierra_definition = include_bytes!("../../../fixtures/contracts/storage_access.json"); let sierra_hash = class_hash!("0544b92d358447cb9e50b65092b7169f931d29e05c1404a2cd08c6fd7e32ba90"); @@ -60,7 +63,7 @@ mod tests { version: TransactionVersion::TWO, max_fee, signature: vec![], - nonce: TransactionNonce(Default::default()), + nonce, contract_class, sender_address: account_contract_address, compiled_class_hash: casm_hash, @@ -71,6 +74,7 @@ mod tests { fn deploy_transaction( account_contract_address: ContractAddress, universal_deployer_address: ContractAddress, + nonce: TransactionNonce, ) -> BroadcastedTransaction { let max_fee = Fee::default(); let sierra_hash = @@ -78,7 +82,7 @@ mod tests { BroadcastedTransaction::Invoke(BroadcastedInvokeTransaction::V1( BroadcastedInvokeTransactionV1 { - nonce: transaction_nonce!("0x1"), + nonce, version: TransactionVersion::ONE, max_fee, signature: vec![], @@ -104,12 +108,15 @@ mod tests { )) } - fn invoke_transaction(account_contract_address: ContractAddress) -> BroadcastedTransaction { + fn invoke_transaction( + account_contract_address: ContractAddress, + nonce: TransactionNonce, + ) -> BroadcastedTransaction { let max_fee = Fee::default(); BroadcastedTransaction::Invoke(BroadcastedInvokeTransaction::V1( BroadcastedInvokeTransactionV1 { - nonce: transaction_nonce!("0x2"), + nonce, version: TransactionVersion::ONE, max_fee, signature: vec![], @@ -147,7 +154,10 @@ mod tests { )) } - fn invoke_v3_transaction(account_contract_address: ContractAddress) -> BroadcastedTransaction { + fn invoke_v3_transaction( + account_contract_address: ContractAddress, + nonce: TransactionNonce, + ) -> BroadcastedTransaction { BroadcastedTransaction::Invoke(BroadcastedInvokeTransaction::V3( BroadcastedInvokeTransactionV3 { version: TransactionVersion::THREE, @@ -165,7 +175,7 @@ mod tests { // AccountCallArray::data_len call_param!("0"), ], - nonce: transaction_nonce!("0x3"), + nonce, resource_bounds: ResourceBounds::default(), tip: Tip(0), paymaster_data: vec![], @@ -185,16 +195,22 @@ mod tests { .await; // declare test class - let declare_transaction = declare_transaction(account_contract_address); + let declare_transaction = + declare_transaction(account_contract_address, TransactionNonce::default()); // deploy with unversal deployer contract - let deploy_transaction = - deploy_transaction(account_contract_address, universal_deployer_address); + let deploy_transaction = deploy_transaction( + account_contract_address, + universal_deployer_address, + transaction_nonce!("0x1"), + ); // invoke deployed contract - let invoke_transaction = invoke_transaction(account_contract_address); + let invoke_transaction = + invoke_transaction(account_contract_address, transaction_nonce!("0x2")); // do the same invoke with a v0 transaction let invoke_v0_transaction = invoke_v0_transaction(); // do the same invoke with a v3 transaction - let invoke_v3_transaction = invoke_v3_transaction(account_contract_address); + let invoke_v3_transaction = + invoke_v3_transaction(account_contract_address, transaction_nonce!("0x3")); let input = EstimateFeeInput { request: vec![ @@ -262,7 +278,98 @@ mod tests { } #[tokio::test] - async fn declare_deploy_and_invoke_sierra_class_starknet_0_13_1_1() { + async fn declare_deploy_and_invoke_sierra_class_starknet_0_13_1_with_invalid_nonce_will_pass() { + let (context, last_block_header, account_contract_address, universal_deployer_address) = + crate::test_setup::test_context_with_starknet_version(StarknetVersion::new( + 0, 13, 1, 1, + )) + .await; + + // declare test class + let declare_transaction = + declare_transaction(account_contract_address, transaction_nonce!("0x1")); + // deploy with unversal deployer contract + let deploy_transaction = deploy_transaction( + account_contract_address, + universal_deployer_address, + transaction_nonce!("0x2"), + ); + // invoke deployed contract + let invoke_transaction = + invoke_transaction(account_contract_address, transaction_nonce!("0x3")); + // do the same invoke with a v0 transaction + let invoke_v0_transaction = invoke_v0_transaction(); + // do the same invoke with a v3 transaction + let invoke_v3_transaction = + invoke_v3_transaction(account_contract_address, transaction_nonce!("0x4")); + + let input = EstimateFeeInput { + request: vec![ + declare_transaction, + deploy_transaction, + invoke_transaction, + invoke_v0_transaction, + invoke_v3_transaction, + ], + simulation_flags: SimulationFlags(vec![]), + block_id: BlockId::Number(last_block_header.number), + }; + let result = super::estimate_fee(context, input).await.unwrap(); + let declare_expected = FeeEstimate { + gas_consumed: 878.into(), + gas_price: 1.into(), + overall_fee: 1262.into(), + unit: PriceUnit::Wei, + data_gas_consumed: Some(192.into()), + data_gas_price: Some(2.into()), + }; + let deploy_expected = FeeEstimate { + gas_consumed: 16.into(), + gas_price: 1.into(), + overall_fee: 464.into(), + unit: PriceUnit::Wei, + data_gas_consumed: Some(224.into()), + data_gas_price: Some(2.into()), + }; + let invoke_expected = FeeEstimate { + gas_consumed: 12.into(), + gas_price: 1.into(), + overall_fee: 268.into(), + unit: PriceUnit::Wei, + data_gas_consumed: Some(128.into()), + data_gas_price: Some(2.into()), + }; + let invoke_v0_expected = FeeEstimate { + gas_consumed: 10.into(), + gas_price: 1.into(), + overall_fee: 266.into(), + unit: PriceUnit::Wei, + data_gas_consumed: Some(128.into()), + data_gas_price: Some(2.into()), + }; + let invoke_v3_expected = FeeEstimate { + gas_consumed: 12.into(), + // STRK gas price is 2 + gas_price: 2.into(), + overall_fee: 280.into(), + unit: PriceUnit::Fri, + data_gas_consumed: Some(128.into()), + data_gas_price: Some(2.into()), + }; + assert_eq!( + result, + vec![ + declare_expected, + deploy_expected, + invoke_expected, + invoke_v0_expected, + invoke_v3_expected, + ] + ); + } + + #[tokio::test] + async fn declare_deploy_and_invoke_sierra_class_starknet_0_13_1_1_with_arbitrary_nonce() { let (context, last_block_header, account_contract_address, universal_deployer_address) = crate::test_setup::test_context_with_starknet_version(StarknetVersion::new( 0, 13, 1, 1, @@ -270,16 +377,22 @@ mod tests { .await; // declare test class - let declare_transaction = declare_transaction(account_contract_address); + let declare_transaction = + declare_transaction(account_contract_address, transaction_nonce!("0x1000")); // deploy with unversal deployer contract - let deploy_transaction = - deploy_transaction(account_contract_address, universal_deployer_address); + let deploy_transaction = deploy_transaction( + account_contract_address, + universal_deployer_address, + transaction_nonce!("0x123"), + ); // invoke deployed contract - let invoke_transaction = invoke_transaction(account_contract_address); + let invoke_transaction = + invoke_transaction(account_contract_address, transaction_nonce!("0x31")); // do the same invoke with a v0 transaction let invoke_v0_transaction = invoke_v0_transaction(); // do the same invoke with a v3 transaction - let invoke_v3_transaction = invoke_v3_transaction(account_contract_address); + let invoke_v3_transaction = + invoke_v3_transaction(account_contract_address, transaction_nonce!("0x4")); let input = EstimateFeeInput { request: vec![