From fdb5979ced6d8fbf60395dbd8b4189a3754770d9 Mon Sep 17 00:00:00 2001 From: Nimrod Weiss Date: Mon, 19 Aug 2024 11:38:05 +0300 Subject: [PATCH] build(fee): tx_args holds valid resource bounds --- crates/blockifier/src/test_utils.rs | 2 +- crates/blockifier/src/test_utils/declare.rs | 4 +- .../src/test_utils/deploy_account.rs | 4 +- crates/blockifier/src/test_utils/invoke.rs | 4 +- .../src/transaction/transactions.rs | 8 ++-- crates/papyrus_common/src/transaction_hash.rs | 15 ++++--- .../src/serialization/serializers.rs | 9 ++-- crates/papyrus_test_utils/src/lib.rs | 19 ++++++--- crates/starknet_api/src/transaction.rs | 42 +++++++++++++++++-- crates/starknet_api/src/transaction_hash.rs | 15 ++++--- 10 files changed, 82 insertions(+), 40 deletions(-) diff --git a/crates/blockifier/src/test_utils.rs b/crates/blockifier/src/test_utils.rs index beb25ae7890..b2afd89626a 100644 --- a/crates/blockifier/src/test_utils.rs +++ b/crates/blockifier/src/test_utils.rs @@ -18,9 +18,9 @@ use starknet_api::state::StorageKey; use starknet_api::transaction::{ Calldata, ContractAddressSalt, + DeprecatedResourceBoundsMapping, Resource, ResourceBounds, - DeprecatedResourceBoundsMapping, TransactionVersion, }; use starknet_api::{contract_address, felt, patricia_key}; diff --git a/crates/blockifier/src/test_utils/declare.rs b/crates/blockifier/src/test_utils/declare.rs index 5a70d808844..89592c631e0 100644 --- a/crates/blockifier/src/test_utils/declare.rs +++ b/crates/blockifier/src/test_utils/declare.rs @@ -5,9 +5,9 @@ use starknet_api::transaction::{ DeclareTransactionV0V1, DeclareTransactionV2, DeclareTransactionV3, + DeprecatedResourceBoundsMapping, Fee, PaymasterData, - DeprecatedResourceBoundsMapping, Tip, TransactionHash, TransactionSignature, @@ -108,7 +108,7 @@ pub fn declare_tx(declare_tx_args: DeclareTxArgs, class_info: ClassInfo) -> Acco starknet_api::transaction::DeclareTransaction::V3(DeclareTransactionV3 { signature: declare_tx_args.signature, sender_address: declare_tx_args.sender_address, - resource_bounds: declare_tx_args.resource_bounds, + resource_bounds: declare_tx_args.resource_bounds.0.try_into().expect("todo"), tip: declare_tx_args.tip, nonce_data_availability_mode: declare_tx_args.nonce_data_availability_mode, fee_data_availability_mode: declare_tx_args.fee_data_availability_mode, diff --git a/crates/blockifier/src/test_utils/deploy_account.rs b/crates/blockifier/src/test_utils/deploy_account.rs index b24e0c9d3d5..69302436a80 100644 --- a/crates/blockifier/src/test_utils/deploy_account.rs +++ b/crates/blockifier/src/test_utils/deploy_account.rs @@ -5,9 +5,9 @@ use starknet_api::transaction::{ ContractAddressSalt, DeployAccountTransactionV1, DeployAccountTransactionV3, + DeprecatedResourceBoundsMapping, Fee, PaymasterData, - DeprecatedResourceBoundsMapping, Tip, TransactionHash, TransactionSignature, @@ -96,7 +96,7 @@ pub fn deploy_account_tx( } else if deploy_tx_args.version == TransactionVersion::THREE { starknet_api::transaction::DeployAccountTransaction::V3(DeployAccountTransactionV3 { signature: deploy_tx_args.signature, - resource_bounds: deploy_tx_args.resource_bounds, + resource_bounds: deploy_tx_args.resource_bounds.0.try_into().expect("todo"), tip: deploy_tx_args.tip, nonce_data_availability_mode: deploy_tx_args.nonce_data_availability_mode, fee_data_availability_mode: deploy_tx_args.fee_data_availability_mode, diff --git a/crates/blockifier/src/test_utils/invoke.rs b/crates/blockifier/src/test_utils/invoke.rs index a62ef0597bd..a44535b57f9 100644 --- a/crates/blockifier/src/test_utils/invoke.rs +++ b/crates/blockifier/src/test_utils/invoke.rs @@ -4,12 +4,12 @@ use starknet_api::data_availability::DataAvailabilityMode; use starknet_api::transaction::{ AccountDeploymentData, Calldata, + DeprecatedResourceBoundsMapping, Fee, InvokeTransactionV0, InvokeTransactionV1, InvokeTransactionV3, PaymasterData, - DeprecatedResourceBoundsMapping, Tip, TransactionHash, TransactionSignature, @@ -96,7 +96,7 @@ pub fn invoke_tx(invoke_args: InvokeTxArgs) -> InvokeTransaction { }) } else if invoke_args.version == TransactionVersion::THREE { starknet_api::transaction::InvokeTransaction::V3(InvokeTransactionV3 { - resource_bounds: invoke_args.resource_bounds, + resource_bounds: invoke_args.resource_bounds.0.try_into().expect("todo"), calldata: invoke_args.calldata, sender_address: invoke_args.sender_address, nonce: invoke_args.nonce, diff --git a/crates/blockifier/src/transaction/transactions.rs b/crates/blockifier/src/transaction/transactions.rs index f32bb99a8fa..eeaaf4bab78 100644 --- a/crates/blockifier/src/transaction/transactions.rs +++ b/crates/blockifier/src/transaction/transactions.rs @@ -30,7 +30,7 @@ use crate::state::cached_state::TransactionalState; use crate::state::errors::StateError; use crate::state::state_api::{State, UpdatableState}; use crate::transaction::constants; -use crate::transaction::errors::{TransactionInfoCreationError, TransactionExecutionError}; +use crate::transaction::errors::{TransactionExecutionError, TransactionInfoCreationError}; use crate::transaction::objects::{ CommonAccountFields, CurrentTransactionInfo, @@ -296,7 +296,7 @@ impl TransactionInfoCreator for DeclareTransaction { starknet_api::transaction::DeclareTransaction::V3(tx) => { Ok(TransactionInfo::Current(CurrentTransactionInfo { common_fields, - resource_bounds: tx.resource_bounds.0.clone().try_into()?, + resource_bounds: tx.resource_bounds.clone(), tip: tx.tip, nonce_data_availability_mode: tx.nonce_data_availability_mode, fee_data_availability_mode: tx.fee_data_availability_mode, @@ -401,7 +401,7 @@ impl TransactionInfoCreator for DeployAccountTransaction { starknet_api::transaction::DeployAccountTransaction::V3(tx) => { Ok(TransactionInfo::Current(CurrentTransactionInfo { common_fields, - resource_bounds: tx.resource_bounds.0.clone().try_into()?, + resource_bounds: tx.resource_bounds.clone(), tip: tx.tip, nonce_data_availability_mode: tx.nonce_data_availability_mode, fee_data_availability_mode: tx.fee_data_availability_mode, @@ -519,7 +519,7 @@ impl TransactionInfoCreator for InvokeTransaction { starknet_api::transaction::InvokeTransaction::V3(tx) => { Ok(TransactionInfo::Current(CurrentTransactionInfo { common_fields, - resource_bounds: tx.resource_bounds.0.clone().try_into()?, + resource_bounds: tx.resource_bounds.clone(), tip: tx.tip, nonce_data_availability_mode: tx.nonce_data_availability_mode, fee_data_availability_mode: tx.fee_data_availability_mode, diff --git a/crates/papyrus_common/src/transaction_hash.rs b/crates/papyrus_common/src/transaction_hash.rs index c788b00b1aa..5c7c14dc67d 100644 --- a/crates/papyrus_common/src/transaction_hash.rs +++ b/crates/papyrus_common/src/transaction_hash.rs @@ -21,13 +21,12 @@ use starknet_api::transaction::{ InvokeTransactionV1, InvokeTransactionV3, L1HandlerTransaction, - Resource, ResourceBounds, - DeprecatedResourceBoundsMapping, Tip, Transaction, TransactionHash, TransactionVersion, + ValidResourceBounds, }; use starknet_api::StarknetApiError; use starknet_types_core::felt::Felt; @@ -220,15 +219,15 @@ pub(crate) fn ascii_as_felt(ascii_str: &str) -> Result { // An implementation of the SNIP: https://github.com/EvyatarO/SNIPs/blob/snip-8/SNIPS/snip-8.md fn get_tip_resource_bounds_hash( - resource_bounds_mapping: &DeprecatedResourceBoundsMapping, + resource_bounds: &ValidResourceBounds, tip: &Tip, ) -> Result { - let l1_resource_bounds = - resource_bounds_mapping.0.get(&Resource::L1Gas).expect("Missing l1 resource"); - let l1_resource = get_concat_resource(l1_resource_bounds, L1_GAS)?; + let (l1_resource_bounds, l2_resource_bounds) = match resource_bounds { + ValidResourceBounds::L1Gas(l1_gas_bounds) => (l1_gas_bounds, &ResourceBounds::default()), + ValidResourceBounds::AllResources { l1_gas, l2_gas, .. } => (l1_gas, l2_gas), + }; - let l2_resource_bounds = - resource_bounds_mapping.0.get(&Resource::L2Gas).expect("Missing l2 resource"); + let l1_resource = get_concat_resource(l1_resource_bounds, L1_GAS)?; let l2_resource = get_concat_resource(l2_resource_bounds, L2_GAS)?; Ok(HashChain::new() diff --git a/crates/papyrus_storage/src/serialization/serializers.rs b/crates/papyrus_storage/src/serialization/serializers.rs index 508976c97e9..6e5751ddd62 100644 --- a/crates/papyrus_storage/src/serialization/serializers.rs +++ b/crates/papyrus_storage/src/serialization/serializers.rs @@ -89,6 +89,7 @@ use starknet_api::transaction::{ DeployAccountTransactionV3, DeployTransaction, DeployTransactionOutput, + DeprecatedResourceBoundsMapping, Event, EventContent, EventData, @@ -109,7 +110,6 @@ use starknet_api::transaction::{ PaymasterData, Resource, ResourceBounds, - DeprecatedResourceBoundsMapping, RevertedTransactionExecutionStatus, Tip, Transaction, @@ -119,6 +119,7 @@ use starknet_api::transaction::{ TransactionOutput, TransactionSignature, TransactionVersion, + ValidResourceBounds, }; use starknet_types_core::felt::Felt; use tracing::warn; @@ -211,7 +212,7 @@ auto_storage_serde! { pub sender_address: ContractAddress, } pub struct DeclareTransactionV3 { - pub resource_bounds: DeprecatedResourceBoundsMapping, + pub resource_bounds: ValidResourceBounds, pub tip: Tip, pub signature: TransactionSignature, pub nonce: Nonce, @@ -1162,7 +1163,7 @@ auto_storage_serde_conditionally_compressed! { } pub struct DeployAccountTransactionV3 { - pub resource_bounds: DeprecatedResourceBoundsMapping, + pub resource_bounds: ValidResourceBounds, pub tip: Tip, pub signature: TransactionSignature, pub nonce: Nonce, @@ -1198,7 +1199,7 @@ auto_storage_serde_conditionally_compressed! { } pub struct InvokeTransactionV3 { - pub resource_bounds: DeprecatedResourceBoundsMapping, + pub resource_bounds: ValidResourceBounds, pub tip: Tip, pub signature: TransactionSignature, pub nonce: Nonce, diff --git a/crates/papyrus_test_utils/src/lib.rs b/crates/papyrus_test_utils/src/lib.rs index c0864aa6a77..db4df4a1797 100644 --- a/crates/papyrus_test_utils/src/lib.rs +++ b/crates/papyrus_test_utils/src/lib.rs @@ -112,6 +112,7 @@ use starknet_api::transaction::{ DeployAccountTransactionV3, DeployTransaction, DeployTransactionOutput, + DeprecatedResourceBoundsMapping, Event, EventContent, EventData, @@ -132,7 +133,6 @@ use starknet_api::transaction::{ PaymasterData, Resource, ResourceBounds, - DeprecatedResourceBoundsMapping, RevertedTransactionExecutionStatus, Tip, Transaction, @@ -142,6 +142,7 @@ use starknet_api::transaction::{ TransactionOutput, TransactionSignature, TransactionVersion, + ValidResourceBounds, }; use starknet_types_core::felt::Felt; @@ -517,7 +518,7 @@ auto_impl_get_test_instance! { pub sender_address: ContractAddress, } pub struct DeclareTransactionV3 { - pub resource_bounds: DeprecatedResourceBoundsMapping, + pub resource_bounds: ValidResourceBounds, pub tip: Tip, pub signature: TransactionSignature, pub nonce: Nonce, @@ -550,7 +551,7 @@ auto_impl_get_test_instance! { pub constructor_calldata: Calldata, } pub struct DeployAccountTransactionV3 { - pub resource_bounds: DeprecatedResourceBoundsMapping, + pub resource_bounds: ValidResourceBounds, pub tip: Tip, pub signature: TransactionSignature, pub nonce: Nonce, @@ -657,7 +658,7 @@ auto_impl_get_test_instance! { pub calldata: Calldata, } pub struct InvokeTransactionV3 { - pub resource_bounds: DeprecatedResourceBoundsMapping, + pub resource_bounds: ValidResourceBounds, pub tip: Tip, pub signature: TransactionSignature, pub nonce: Nonce, @@ -783,6 +784,10 @@ auto_impl_get_test_instance! { pub name: String, pub r#type: String, } + pub enum ValidResourceBounds { + L1Gas(ResourceBounds) = 0, + AllResources { l1_gas: ResourceBounds, l2_gas: ResourceBounds, l1_data_gas: ResourceBounds } = 1, + } pub struct CasmContractClass { pub prime: BigUint, @@ -894,7 +899,7 @@ macro_rules! auto_impl_get_test_instance { auto_impl_get_test_instance!($($rest)*); }; // enums. - ($(pub)? enum $name:ident { $($variant:ident $( ($ty:ty) )? = $num:expr ,)* } $($rest:tt)*) => { + ($(pub)? enum $name:ident { $($variant:ident $( ($ty:ty) )? $( { $($field:ident : $field_ty:ty),* } )? = $num:expr ,)* } $($rest:tt)*) => { impl GetTestInstance for $name { fn get_test_instance(rng: &mut rand_chacha::ChaCha8Rng) -> Self { use rand::Rng; @@ -903,6 +908,10 @@ macro_rules! auto_impl_get_test_instance { $( $num => { Self::$variant$((<$ty>::get_test_instance(rng)))? + + $(Self::$variant { + $($field: <$field_ty>::get_test_instance(rng)),* + })? } )* _ => { diff --git a/crates/starknet_api/src/transaction.rs b/crates/starknet_api/src/transaction.rs index f5865a567af..1ace5430f1e 100644 --- a/crates/starknet_api/src/transaction.rs +++ b/crates/starknet_api/src/transaction.rs @@ -209,7 +209,8 @@ impl TransactionHasher for DeclareTransactionV2 { /// A declare V3 transaction. #[derive(Clone, Debug, Deserialize, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)] pub struct DeclareTransactionV3 { - pub resource_bounds: DeprecatedResourceBoundsMapping, + #[serde(deserialize_with = "deserialize_resource_bounds")] + pub resource_bounds: ValidResourceBounds, pub tip: Tip, pub signature: TransactionSignature, pub nonce: Nonce, @@ -318,7 +319,8 @@ impl TransactionHasher for DeployAccountTransactionV1 { /// A deploy account V3 transaction. #[derive(Debug, Clone, Eq, PartialEq, Hash, Deserialize, Serialize, PartialOrd, Ord)] pub struct DeployAccountTransactionV3 { - pub resource_bounds: DeprecatedResourceBoundsMapping, + #[serde(deserialize_with = "deserialize_resource_bounds")] + pub resource_bounds: ValidResourceBounds, pub tip: Tip, pub signature: TransactionSignature, pub nonce: Nonce, @@ -455,7 +457,8 @@ impl TransactionHasher for InvokeTransactionV1 { /// An invoke V3 transaction. #[derive(Debug, Clone, Eq, PartialEq, Hash, Deserialize, Serialize, PartialOrd, Ord)] pub struct InvokeTransactionV3 { - pub resource_bounds: DeprecatedResourceBoundsMapping, + #[serde(deserialize_with = "deserialize_resource_bounds")] + pub resource_bounds: ValidResourceBounds, pub tip: Tip, pub signature: TransactionSignature, pub nonce: Nonce, @@ -949,12 +952,43 @@ impl TryFrom> for DeprecatedResourceBoundsMappin } } -#[derive(Clone, Debug, Eq, PartialEq)] +#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub enum ValidResourceBounds { L1Gas(ResourceBounds), // Pre 0.13.3. L2 bounds are signed but never used. AllResources { l1_gas: ResourceBounds, l2_gas: ResourceBounds, l1_data_gas: ResourceBounds }, } +/// Deserializes raw resource bounds, given as map, into valid resource bounds. +pub fn deserialize_resource_bounds<'de, D>(de: D) -> Result +where + D: Deserializer<'de>, +{ + let raw_resource_bounds: BTreeMap = Deserialize::deserialize(de)?; + ValidResourceBounds::try_from(raw_resource_bounds).map_err(serde::de::Error::custom) +} + +/// Serializes ValidResourceBounds as map for BC. +// TODO(Nimrod): Can this be removed? +impl Serialize for ValidResourceBounds { + fn serialize(&self, s: S) -> Result + where + S: Serializer, + { + let map = match self { + ValidResourceBounds::L1Gas(l1_gas) => BTreeMap::from([ + (Resource::L1Gas, *l1_gas), + (Resource::L2Gas, ResourceBounds::default()), + ]), + ValidResourceBounds::AllResources { l1_gas, l2_gas, l1_data_gas } => BTreeMap::from([ + (Resource::L1Gas, *l1_gas), + (Resource::L2Gas, *l2_gas), + (Resource::L1DataGas, *l1_data_gas), + ]), + }; + DeprecatedResourceBoundsMapping(map).serialize(s) + } +} + impl TryFrom> for ValidResourceBounds { type Error = StarknetApiError; fn try_from( diff --git a/crates/starknet_api/src/transaction_hash.rs b/crates/starknet_api/src/transaction_hash.rs index 55cdc33caac..dc8fc2da0c5 100644 --- a/crates/starknet_api/src/transaction_hash.rs +++ b/crates/starknet_api/src/transaction_hash.rs @@ -14,18 +14,17 @@ use crate::transaction::{ DeployAccountTransactionV1, DeployAccountTransactionV3, DeployTransaction, - DeprecatedResourceBoundsMapping, InvokeTransaction, InvokeTransactionV0, InvokeTransactionV1, InvokeTransactionV3, L1HandlerTransaction, - Resource, ResourceBounds, Tip, Transaction, TransactionHash, TransactionVersion, + ValidResourceBounds, }; use crate::StarknetApiError; @@ -175,15 +174,15 @@ pub(crate) fn ascii_as_felt(ascii_str: &str) -> Result { // An implementation of the SNIP: https://github.com/EvyatarO/SNIPs/blob/snip-8/SNIPS/snip-8.md fn get_tip_resource_bounds_hash( - resource_bounds_mapping: &DeprecatedResourceBoundsMapping, + resource_bounds: &ValidResourceBounds, tip: &Tip, ) -> Result { - let l1_resource_bounds = - resource_bounds_mapping.0.get(&Resource::L1Gas).expect("Missing l1 resource"); - let l1_resource = get_concat_resource(l1_resource_bounds, L1_GAS)?; + let (l1_resource_bounds, l2_resource_bounds) = match resource_bounds { + ValidResourceBounds::L1Gas(l1_gas_bounds) => (l1_gas_bounds, &ResourceBounds::default()), + ValidResourceBounds::AllResources { l1_gas, l2_gas, .. } => (l1_gas, l2_gas), + }; - let l2_resource_bounds = - resource_bounds_mapping.0.get(&Resource::L2Gas).expect("Missing l2 resource"); + let l1_resource = get_concat_resource(l1_resource_bounds, L1_GAS)?; let l2_resource = get_concat_resource(l2_resource_bounds, L2_GAS)?; Ok(HashChain::new()