Skip to content

Commit

Permalink
refactor(fee): remove resource mapping usage in rpc and adjust to use…
Browse files Browse the repository at this point in the history
… all resources
  • Loading branch information
nimrod-starkware committed Aug 25, 2024
1 parent 618cb78 commit abe3c27
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 47 deletions.
11 changes: 3 additions & 8 deletions crates/gateway/src/stateless_transaction_validator.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
use starknet_api::rpc_transaction::{
ResourceBoundsMapping,
RpcDeclareTransaction,
RpcDeployAccountTransaction,
RpcInvokeTransaction,
RpcTransaction,
};
use starknet_api::state::EntryPoint;
use starknet_api::transaction::Resource;
use starknet_api::transaction::{AllResourceBounds, Resource};
use starknet_types_core::felt::Felt;
use tracing::{instrument, Level};

Expand Down Expand Up @@ -176,14 +175,10 @@ impl StatelessTransactionValidator {
}

fn validate_resource_is_non_zero(
resource_bounds_mapping: &ResourceBoundsMapping,
all_resource_bounds: &AllResourceBounds,
resource: Resource,
) -> StatelessTransactionValidatorResult<()> {
let resource_bounds = match resource {
Resource::L1Gas => resource_bounds_mapping.l1_gas,
Resource::L2Gas => resource_bounds_mapping.l2_gas,
Resource::L1DataGas => todo!(),
};
let resource_bounds = all_resource_bounds.get_bound(resource);
if resource_bounds.max_amount == 0 || resource_bounds.max_price_per_unit == 0 {
return Err(StatelessTransactionValidatorError::ZeroResourceBounds {
resource,
Expand Down
38 changes: 30 additions & 8 deletions crates/gateway/src/stateless_transaction_validator_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,15 @@ use mempool_test_utils::starknet_api_test_utils::{
};
use rstest::rstest;
use starknet_api::core::EntryPointSelector;
use starknet_api::rpc_transaction::{ContractClass, EntryPointByType, ResourceBoundsMapping};
use starknet_api::rpc_transaction::{ContractClass, EntryPointByType};
use starknet_api::state::EntryPoint;
use starknet_api::transaction::{Calldata, Resource, ResourceBounds, TransactionSignature};
use starknet_api::transaction::{
AllResourceBounds,
Calldata,
Resource,
ResourceBounds,
TransactionSignature,
};
use starknet_api::{calldata, felt};
use starknet_types_core::felt::Felt;

Expand Down Expand Up @@ -67,7 +73,11 @@ fn default_validator_config_for_testing() -> &'static StatelessTransactionValida
validate_non_zero_l2_gas_fee: false,
..default_validator_config_for_testing().clone()
},
create_resource_bounds_mapping(NON_EMPTY_RESOURCE_BOUNDS, ResourceBounds::default()),
create_resource_bounds_mapping(
NON_EMPTY_RESOURCE_BOUNDS,
ResourceBounds::default(),
ResourceBounds::default(),
),
calldata![],
TransactionSignature::default()
)]
Expand All @@ -77,7 +87,11 @@ fn default_validator_config_for_testing() -> &'static StatelessTransactionValida
validate_non_zero_l2_gas_fee: true,
..default_validator_config_for_testing().clone()
},
create_resource_bounds_mapping(ResourceBounds::default(), NON_EMPTY_RESOURCE_BOUNDS),
create_resource_bounds_mapping(
ResourceBounds::default(),
NON_EMPTY_RESOURCE_BOUNDS,
ResourceBounds::default(),
),
calldata![],
TransactionSignature::default()
)]
Expand All @@ -87,7 +101,11 @@ fn default_validator_config_for_testing() -> &'static StatelessTransactionValida
validate_non_zero_l2_gas_fee: true,
..default_validator_config_for_testing().clone()
},
create_resource_bounds_mapping(NON_EMPTY_RESOURCE_BOUNDS, NON_EMPTY_RESOURCE_BOUNDS),
create_resource_bounds_mapping(
NON_EMPTY_RESOURCE_BOUNDS,
NON_EMPTY_RESOURCE_BOUNDS,
ResourceBounds::default(),
),
calldata![],
TransactionSignature::default()
)]
Expand All @@ -111,7 +129,7 @@ fn default_validator_config_for_testing() -> &'static StatelessTransactionValida
)]
fn test_positive_flow(
#[case] config: StatelessTransactionValidatorConfig,
#[case] resource_bounds: ResourceBoundsMapping,
#[case] resource_bounds: AllResourceBounds,
#[case] tx_calldata: Calldata,
#[case] signature: TransactionSignature,
#[values(TransactionType::Declare, TransactionType::DeployAccount, TransactionType::Invoke)]
Expand Down Expand Up @@ -141,14 +159,18 @@ fn test_positive_flow(
validate_non_zero_l2_gas_fee: true,
..default_validator_config_for_testing().clone()
},
create_resource_bounds_mapping(NON_EMPTY_RESOURCE_BOUNDS, ResourceBounds::default()),
create_resource_bounds_mapping(
NON_EMPTY_RESOURCE_BOUNDS,
ResourceBounds::default(),
ResourceBounds::default(),
),
StatelessTransactionValidatorError::ZeroResourceBounds{
resource: Resource::L2Gas, resource_bounds: ResourceBounds::default()
}
)]
fn test_invalid_resource_bounds(
#[case] config: StatelessTransactionValidatorConfig,
#[case] resource_bounds: ResourceBoundsMapping,
#[case] resource_bounds: AllResourceBounds,
#[case] expected_error: StatelessTransactionValidatorError,
#[values(TransactionType::Declare, TransactionType::DeployAccount, TransactionType::Invoke)]
tx_type: TransactionType,
Expand Down
32 changes: 22 additions & 10 deletions crates/mempool_test_utils/src/starknet_api_test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ use starknet_api::data_availability::DataAvailabilityMode;
use starknet_api::executable_transaction::{InvokeTransaction, Transaction};
use starknet_api::rpc_transaction::{
ContractClass,
ResourceBoundsMapping,
RpcDeclareTransactionV3,
RpcDeployAccountTransactionV3,
RpcInvokeTransactionV3,
RpcTransaction,
};
use starknet_api::transaction::{
AccountDeploymentData,
AllResourceBounds,
Calldata,
ContractAddressSalt,
DeprecatedResourceBoundsMapping as ExecutableResourceBoundsMapping,
Expand Down Expand Up @@ -48,6 +48,8 @@ pub const VALID_L1_GAS_MAX_AMOUNT: u64 = 203484;
pub const VALID_L1_GAS_MAX_PRICE_PER_UNIT: u128 = 100000000000;
pub const VALID_L2_GAS_MAX_AMOUNT: u64 = 203484;
pub const VALID_L2_GAS_MAX_PRICE_PER_UNIT: u128 = 100000000000;
pub const VALID_L1_DATA_GAS_MAX_AMOUNT: u64 = 203484;
pub const VALID_L1_DATA_GAS_MAX_PRICE_PER_UNIT: u128 = 100000000000;
pub const TEST_SENDER_ADDRESS: u128 = 0x1000;

// Utils.
Expand All @@ -59,7 +61,7 @@ pub enum TransactionType {

pub fn external_tx_for_testing(
tx_type: TransactionType,
resource_bounds: ResourceBoundsMapping,
resource_bounds: AllResourceBounds,
calldata: Calldata,
signature: TransactionSignature,
) -> RpcTransaction {
Expand Down Expand Up @@ -95,18 +97,24 @@ pub fn external_tx_for_testing(
pub const NON_EMPTY_RESOURCE_BOUNDS: ResourceBounds =
ResourceBounds { max_amount: 1, max_price_per_unit: 1 };

// TODO(Nimrod): Delete this function.
pub fn create_resource_bounds_mapping(
l1_resource_bounds: ResourceBounds,
l2_resource_bounds: ResourceBounds,
) -> ResourceBoundsMapping {
ResourceBoundsMapping { l1_gas: l1_resource_bounds, l2_gas: l2_resource_bounds }
l1_data_resource_bounds: ResourceBounds,
) -> AllResourceBounds {
AllResourceBounds {
l1_gas: l1_resource_bounds,
l2_gas: l2_resource_bounds,
l1_data_gas: l1_data_resource_bounds,
}
}

pub fn zero_resource_bounds_mapping() -> ResourceBoundsMapping {
create_resource_bounds_mapping(ResourceBounds::default(), ResourceBounds::default())
pub fn zero_resource_bounds_mapping() -> AllResourceBounds {
AllResourceBounds::default()
}

pub fn test_resource_bounds_mapping() -> ResourceBoundsMapping {
pub fn test_resource_bounds_mapping() -> AllResourceBounds {
create_resource_bounds_mapping(
ResourceBounds {
max_amount: VALID_L1_GAS_MAX_AMOUNT,
Expand All @@ -116,6 +124,10 @@ pub fn test_resource_bounds_mapping() -> ResourceBoundsMapping {
max_amount: VALID_L2_GAS_MAX_AMOUNT,
max_price_per_unit: VALID_L2_GAS_MAX_PRICE_PER_UNIT,
},
ResourceBounds {
max_amount: VALID_L1_DATA_GAS_MAX_AMOUNT,
max_price_per_unit: VALID_L1_DATA_GAS_MAX_PRICE_PER_UNIT,
},
)
}

Expand Down Expand Up @@ -366,7 +378,7 @@ pub struct InvokeTxArgs {
pub sender_address: ContractAddress,
pub calldata: Calldata,
pub version: TransactionVersion,
pub resource_bounds: ResourceBoundsMapping,
pub resource_bounds: AllResourceBounds,
pub tip: Tip,
pub nonce_data_availability_mode: DataAvailabilityMode,
pub fee_data_availability_mode: DataAvailabilityMode,
Expand Down Expand Up @@ -397,7 +409,7 @@ impl Default for InvokeTxArgs {
pub struct DeployAccountTxArgs {
pub signature: TransactionSignature,
pub version: TransactionVersion,
pub resource_bounds: ResourceBoundsMapping,
pub resource_bounds: AllResourceBounds,
pub tip: Tip,
pub nonce_data_availability_mode: DataAvailabilityMode,
pub fee_data_availability_mode: DataAvailabilityMode,
Expand Down Expand Up @@ -431,7 +443,7 @@ pub struct DeclareTxArgs {
pub signature: TransactionSignature,
pub sender_address: ContractAddress,
pub version: TransactionVersion,
pub resource_bounds: ResourceBoundsMapping,
pub resource_bounds: AllResourceBounds,
pub tip: Tip,
pub nonce_data_availability_mode: DataAvailabilityMode,
pub fee_data_availability_mode: DataAvailabilityMode,
Expand Down
31 changes: 15 additions & 16 deletions crates/starknet_api/src/rpc_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ use crate::data_availability::DataAvailabilityMode;
use crate::state::EntryPoint;
use crate::transaction::{
AccountDeploymentData,
AllResourceBounds,
Calldata,
ContractAddressSalt,
PaymasterData,
Resource,
ResourceBounds,
Tip,
TransactionSignature,
};
Expand Down Expand Up @@ -63,7 +63,7 @@ macro_rules! implement_ref_getters {
impl RpcTransaction {
implement_ref_getters!(
(nonce, Nonce),
(resource_bounds, ResourceBoundsMapping),
(resource_bounds, AllResourceBounds),
(signature, TransactionSignature),
(tip, Tip)
);
Expand Down Expand Up @@ -135,7 +135,7 @@ pub struct RpcDeclareTransactionV3 {
pub signature: TransactionSignature,
pub nonce: Nonce,
pub contract_class: ContractClass,
pub resource_bounds: ResourceBoundsMapping,
pub resource_bounds: AllResourceBounds,
pub tip: Tip,
pub paymaster_data: PaymasterData,
pub account_deployment_data: AccountDeploymentData,
Expand All @@ -151,7 +151,7 @@ pub struct RpcDeployAccountTransactionV3 {
pub class_hash: ClassHash,
pub contract_address_salt: ContractAddressSalt,
pub constructor_calldata: Calldata,
pub resource_bounds: ResourceBoundsMapping,
pub resource_bounds: AllResourceBounds,
pub tip: Tip,
pub paymaster_data: PaymasterData,
pub nonce_data_availability_mode: DataAvailabilityMode,
Expand All @@ -165,7 +165,7 @@ pub struct RpcInvokeTransactionV3 {
pub calldata: Calldata,
pub signature: TransactionSignature,
pub nonce: Nonce,
pub resource_bounds: ResourceBoundsMapping,
pub resource_bounds: AllResourceBounds,
pub tip: Tip,
pub paymaster_data: PaymasterData,
pub account_deployment_data: AccountDeploymentData,
Expand All @@ -192,17 +192,16 @@ pub struct EntryPointByType {
pub l1handler: Vec<EntryPoint>,
}

// The serialization of the struct in transaction is in capital letters, not following the spec.
#[derive(Clone, Debug, Default, Deserialize, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)]
pub struct ResourceBoundsMapping {
pub l1_gas: ResourceBounds,
pub l2_gas: ResourceBounds,
}

impl From<ResourceBoundsMapping> for crate::transaction::DeprecatedResourceBoundsMapping {
fn from(mapping: ResourceBoundsMapping) -> crate::transaction::DeprecatedResourceBoundsMapping {
let map =
BTreeMap::from([(Resource::L1Gas, mapping.l1_gas), (Resource::L2Gas, mapping.l2_gas)]);
// TODO(Nimrod): Remove this conversion.
impl From<AllResourceBounds> for crate::transaction::DeprecatedResourceBoundsMapping {
fn from(
all_resource_bounds: AllResourceBounds,
) -> crate::transaction::DeprecatedResourceBoundsMapping {
let map = BTreeMap::from([
(Resource::L1Gas, all_resource_bounds.l1_gas),
(Resource::L2Gas, all_resource_bounds.l2_gas),
(Resource::L1DataGas, all_resource_bounds.l1_data_gas),
]);
crate::transaction::DeprecatedResourceBoundsMapping(map)
}
}
7 changes: 4 additions & 3 deletions crates/starknet_api/src/rpc_transaction_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use crate::core::{ClassHash, CompiledClassHash, ContractAddress, Nonce, Patricia
use crate::rpc_transaction::{
ContractClass,
DataAvailabilityMode,
ResourceBoundsMapping,
RpcDeclareTransaction,
RpcDeclareTransactionV3,
RpcDeployAccountTransaction,
Expand All @@ -18,6 +17,7 @@ use crate::rpc_transaction::{
};
use crate::transaction::{
AccountDeploymentData,
AllResourceBounds,
Calldata,
ContractAddressSalt,
PaymasterData,
Expand All @@ -27,10 +27,11 @@ use crate::transaction::{
};
use crate::{contract_address, felt, patricia_key};

fn create_resource_bounds_for_testing() -> ResourceBoundsMapping {
ResourceBoundsMapping {
fn create_resource_bounds_for_testing() -> AllResourceBounds {
AllResourceBounds {
l1_gas: ResourceBounds { max_amount: 100, max_price_per_unit: 12 },
l2_gas: ResourceBounds { max_amount: 58, max_price_per_unit: 31 },
l1_data_gas: ResourceBounds { max_amount: 66, max_price_per_unit: 25 },
}
}

Expand Down
4 changes: 2 additions & 2 deletions crates/starknet_api/src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -963,15 +963,15 @@ pub enum ValidResourceBounds {
AllResources(AllResourceBounds),
}

#[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq, Hash, Ord, PartialOrd, Serialize)]
pub struct AllResourceBounds {
pub l1_gas: ResourceBounds,
pub l2_gas: ResourceBounds,
pub l1_data_gas: ResourceBounds,
}

impl AllResourceBounds {
#[allow(dead_code)]
fn get_bound(&self, resource: Resource) -> ResourceBounds {
pub fn get_bound(&self, resource: Resource) -> ResourceBounds {
match resource {
Resource::L1Gas => self.l1_gas,
Resource::L2Gas => self.l2_gas,
Expand Down

0 comments on commit abe3c27

Please sign in to comment.