diff --git a/CHANGELOG.md b/CHANGELOG.md index 91a6edcafa..2b22fba22e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ madara node - feat(cache-option): add an option to enable aggressive caching in command-line parameters +- fix: Ensure transaction checks are compatible with starknet-rs ## v0.4.0 diff --git a/crates/client/rpc/src/lib.rs b/crates/client/rpc/src/lib.rs index 8a32c2e9dc..3411b471c4 100644 --- a/crates/client/rpc/src/lib.rs +++ b/crates/client/rpc/src/lib.rs @@ -600,14 +600,16 @@ where request: Vec, block_id: BlockId, ) -> RpcResult> { - let is_invalid_query_transaction = request.iter().any(|tx| match tx { - BroadcastedTransaction::Invoke(invoke_tx) => !invoke_tx.is_query, - BroadcastedTransaction::Declare(BroadcastedDeclareTransaction::V1(tx_v1)) => !tx_v1.is_query, - BroadcastedTransaction::Declare(BroadcastedDeclareTransaction::V2(tx_v2)) => !tx_v2.is_query, - BroadcastedTransaction::DeployAccount(deploy_tx) => !deploy_tx.is_query, + let is_query = request.iter().any(|tx| match tx { + BroadcastedTransaction::Invoke(invoke_tx) => invoke_tx.is_query, + BroadcastedTransaction::Declare(BroadcastedDeclareTransaction::V1(tx_v1)) => tx_v1.is_query, + BroadcastedTransaction::Declare(BroadcastedDeclareTransaction::V2(tx_v2)) => tx_v2.is_query, + BroadcastedTransaction::DeployAccount(deploy_tx) => deploy_tx.is_query, }); - if is_invalid_query_transaction { - return Err(StarknetRpcApiError::UnsupportedTxVersion.into()); + if !is_query { + log::error!( + "Got `is_query`: false. In a future version, this will fail fee estimation with UnsupportedTxVersion" + ); } let substrate_block_hash = self.substrate_block_hash_from_starknet_block(block_id).map_err(|e| { @@ -626,7 +628,7 @@ where let (actual_fee, gas_usage) = self .client .runtime_api() - .estimate_fee(substrate_block_hash, tx) + .estimate_fee(substrate_block_hash, tx, is_query) .map_err(|e| { error!("Request parameters error: {e}"); StarknetRpcApiError::InternalServerError diff --git a/crates/pallets/starknet/src/lib.rs b/crates/pallets/starknet/src/lib.rs index 1c4fe9d3d7..4203db1686 100644 --- a/crates/pallets/starknet/src/lib.rs +++ b/crates/pallets/starknet/src/lib.rs @@ -1092,7 +1092,7 @@ impl Pallet { } /// Estimate the fee associated with transaction - pub fn estimate_fee(transaction: UserTransaction) -> Result<(u64, u64), DispatchError> { + pub fn estimate_fee(transaction: UserTransaction, is_query: bool) -> Result<(u64, u64), DispatchError> { let chain_id = Self::chain_id(); fn execute_tx_and_rollback( @@ -1116,20 +1116,20 @@ impl Pallet { let execution_result = match transaction { UserTransaction::Declare(tx, contract_class) => execute_tx_and_rollback( - tx.try_into_executable::(chain_id, contract_class, true) + tx.try_into_executable::(chain_id, contract_class, is_query) .map_err(|_| Error::::InvalidContractClass)?, &mut blockifier_state_adapter, &block_context, disable_nonce_validation, ), UserTransaction::DeployAccount(tx) => execute_tx_and_rollback( - tx.into_executable::(chain_id, true), + tx.into_executable::(chain_id, is_query), &mut blockifier_state_adapter, &block_context, disable_nonce_validation, ), UserTransaction::Invoke(tx) => execute_tx_and_rollback( - tx.into_executable::(chain_id, true), + tx.into_executable::(chain_id, is_query), &mut blockifier_state_adapter, &block_context, disable_nonce_validation, diff --git a/crates/pallets/starknet/src/runtime_api.rs b/crates/pallets/starknet/src/runtime_api.rs index 5e2a523a41..466fd5bc6e 100644 --- a/crates/pallets/starknet/src/runtime_api.rs +++ b/crates/pallets/starknet/src/runtime_api.rs @@ -46,7 +46,7 @@ sp_api::decl_runtime_apis! { /// Returns the chain id. fn chain_id() -> Felt252Wrapper; /// Returns fee estimate - fn estimate_fee(transaction: UserTransaction) -> Result<(u64, u64), DispatchError>; + fn estimate_fee(transaction: UserTransaction, is_query: bool) -> Result<(u64, u64), DispatchError>; /// Filters extrinsic transactions to return only Starknet transactions /// /// To support runtime upgrades, the client must be unaware of the specific extrinsic diff --git a/crates/pallets/starknet/src/tests/query_tx.rs b/crates/pallets/starknet/src/tests/query_tx.rs index 15a602dedb..215fb7aae1 100644 --- a/crates/pallets/starknet/src/tests/query_tx.rs +++ b/crates/pallets/starknet/src/tests/query_tx.rs @@ -17,14 +17,14 @@ fn estimates_tx_fee_successfully_no_validate() { let tx = get_invoke_dummy(Felt252Wrapper::ZERO); let tx = UserTransaction::Invoke(tx.into()); - let (actual, l1_gas_usage) = Starknet::estimate_fee(tx).unwrap(); + let (actual, l1_gas_usage) = Starknet::estimate_fee(tx, true).unwrap(); assert!(actual > 0, "actual fee is missing"); assert!(l1_gas_usage == 0, "this should not be charged any l1_gas as it does not store nor send messages"); let tx = get_storage_read_write_dummy(); let tx = UserTransaction::Invoke(tx.into()); - let (actual, l1_gas_usage) = Starknet::estimate_fee(tx).unwrap(); + let (actual, l1_gas_usage) = Starknet::estimate_fee(tx, true).unwrap(); assert!(actual > 0, "actual fee is missing"); assert!(l1_gas_usage > 0, "this should be charged l1_gas as it store a value to storage"); }); @@ -39,7 +39,7 @@ fn estimates_tx_fee_with_query_version() { let pre_storage = Starknet::pending().len(); let tx = UserTransaction::Invoke(tx.into()); - assert_ok!(Starknet::estimate_fee(tx)); + assert_ok!(Starknet::estimate_fee(tx, true)); assert!(pre_storage == Starknet::pending().len(), "estimate should not add a tx to pending"); }); @@ -57,7 +57,7 @@ fn executable_tx_should_not_be_estimable() { // it should not be valid for estimate calls assert_err!( - Starknet::estimate_fee(UserTransaction::Invoke(tx.clone().into())), + Starknet::estimate_fee(UserTransaction::Invoke(tx.clone().into()), true), Error::::TransactionExecutionFailed ); @@ -77,7 +77,7 @@ fn query_tx_should_not_be_executable() { tx.signature = sign_message_hash(tx_hash); // it should be valid for estimate calls - assert_ok!(Starknet::estimate_fee(UserTransaction::Invoke(tx.clone().into())),); + assert_ok!(Starknet::estimate_fee(UserTransaction::Invoke(tx.clone().into()), true),); // it should not be executable assert_err!( diff --git a/crates/runtime/src/lib.rs b/crates/runtime/src/lib.rs index a3bef814fa..5df40246da 100644 --- a/crates/runtime/src/lib.rs +++ b/crates/runtime/src/lib.rs @@ -263,8 +263,8 @@ impl_runtime_apis! { Starknet::chain_id() } - fn estimate_fee(transaction: UserTransaction) -> Result<(u64, u64), DispatchError> { - Starknet::estimate_fee(transaction) + fn estimate_fee(transaction: UserTransaction, is_query: bool) -> Result<(u64, u64), DispatchError> { + Starknet::estimate_fee(transaction, is_query) } fn get_starknet_events_and_their_associated_tx_hash(block_extrinsics: Vec<::Extrinsic>, chain_id: Felt252Wrapper) -> Vec<(Felt252Wrapper, StarknetEvent)> {