diff --git a/node/src/components/transaction_acceptor/tests.rs b/node/src/components/transaction_acceptor/tests.rs index 68fc057a4b..1a419ee3c2 100644 --- a/node/src/components/transaction_acceptor/tests.rs +++ b/node/src/components/transaction_acceptor/tests.rs @@ -1140,7 +1140,6 @@ async fn run_transaction_acceptor_without_timeout( } else { chainspec }; - chainspec.core_config.administrators = iter::once(PublicKey::from(&admin)).collect(); let chainspec = Arc::new(chainspec); diff --git a/node/src/components/transaction_buffer/tests.rs b/node/src/components/transaction_buffer/tests.rs index 797dd43eda..1c030fc198 100644 --- a/node/src/components/transaction_buffer/tests.rs +++ b/node/src/components/transaction_buffer/tests.rs @@ -1123,8 +1123,9 @@ fn make_test_chainspec(max_standard_count: u64, max_mint_count: u64) -> Arc Result { match transaction { Transaction::Deploy(deploy) => Ok(MetaTransaction::Deploy(deploy.clone())), - Transaction::V1(v1) => MetaTransactionV1::from_transaction_v1(v1, transaction_config) - .map(MetaTransaction::V1), + Transaction::V1(v1) => MetaTransactionV1::from_transaction_v1( + v1, + &transaction_config.transaction_v1_config, + ) + .map(MetaTransaction::V1), } } diff --git a/node/src/types/transaction/meta_transaction/meta_transaction_v1.rs b/node/src/types/transaction/meta_transaction/meta_transaction_v1.rs index 179000aa1c..78831e4517 100644 --- a/node/src/types/transaction/meta_transaction/meta_transaction_v1.rs +++ b/node/src/types/transaction/meta_transaction/meta_transaction_v1.rs @@ -5,7 +5,7 @@ use casper_types::{ HashAddr, InitiatorAddr, InvalidTransaction, InvalidTransactionV1, PricingHandling, PricingMode, TimeDiff, Timestamp, TransactionArgs, TransactionConfig, TransactionEntryPoint, TransactionRuntimeParams, TransactionScheduling, TransactionTarget, TransactionV1, - TransactionV1ExcessiveSizeError, TransactionV1Hash, U512, + TransactionV1Config, TransactionV1ExcessiveSizeError, TransactionV1Hash, U512, }; use core::fmt::{self, Debug, Display, Formatter}; use datasize::DataSize; @@ -45,7 +45,7 @@ pub struct MetaTransactionV1 { impl MetaTransactionV1 { pub fn from_transaction_v1( v1: &TransactionV1, - transaction_config: &TransactionConfig, + transaction_v1_config: &TransactionV1Config, ) -> Result { let args: TransactionArgs = v1.deserialize_field(ARGS_MAP_KEY).map_err(|error| { InvalidTransaction::V1(InvalidTransactionV1::CouldNotDeserializeField { error }) @@ -70,12 +70,13 @@ impl MetaTransactionV1 { let payload_hash = v1.payload_hash()?; let serialized_length = v1.serialized_length(); + let pricing_mode = v1.payload().pricing_mode(); let lane_id = calculate_transaction_lane( &entry_point, &target, - v1.pricing_mode().additional_computation_factor(), - transaction_config, + pricing_mode, + transaction_v1_config, serialized_length as u64, )?; let transaction_lane = @@ -701,3 +702,127 @@ impl Display for MetaTransactionV1 { ) } } + +#[cfg(test)] +mod tests { + use super::MetaTransactionV1; + use crate::types::transaction::transaction_v1_builder::TransactionV1Builder; + use casper_types::{ + testing::TestRng, InvalidTransaction, InvalidTransactionV1, PricingMode, SecretKey, + TransactionInvocationTarget, TransactionLimitsDefinition, TransactionRuntimeParams, + TransactionV1Config, + }; + + #[test] + fn limited_amount_should_determine_transaction_lane_for_session() { + let rng = &mut TestRng::new(); + let secret_key = SecretKey::random(rng); + let pricing_mode = PricingMode::PaymentLimited { + payment_amount: 1001, + gas_price_tolerance: 1, + standard_payment: true, + }; + + let transaction_v1 = TransactionV1Builder::new_session( + false, + vec![1; 30].into(), + TransactionRuntimeParams::VmCasperV1, + ) + .with_chain_name("x".to_string()) + .with_pricing_mode(pricing_mode) + .with_secret_key(&secret_key) + .build() + .unwrap(); + let config = build_v1_config(); + + let meta_transaction = MetaTransactionV1::from_transaction_v1(&transaction_v1, &config) + .expect("meta transaction should be valid"); + assert_eq!(meta_transaction.transaction_lane(), 4); + } + + #[test] + fn limited_amount_should_fail_if_does_not_fit_in_any_lane() { + let rng = &mut TestRng::new(); + let secret_key = SecretKey::random(rng); + let pricing_mode = PricingMode::PaymentLimited { + payment_amount: 1000000, + gas_price_tolerance: 1, + standard_payment: true, + }; + + let transaction_v1 = TransactionV1Builder::new_session( + false, + vec![1; 30].into(), + TransactionRuntimeParams::VmCasperV1, + ) + .with_chain_name("x".to_string()) + .with_pricing_mode(pricing_mode) + .with_secret_key(&secret_key) + .build() + .unwrap(); + let config = build_v1_config(); + + let res = MetaTransactionV1::from_transaction_v1(&transaction_v1, &config); + assert!(matches!( + res, + Err(InvalidTransaction::V1( + InvalidTransactionV1::NoWasmLaneMatchesTransaction() + )) + )) + } + + #[test] + fn limited_amount_should_determine_transaction_lane_for_stored() { + let rng = &mut TestRng::new(); + let secret_key = SecretKey::random(rng); + let pricing_mode = PricingMode::PaymentLimited { + payment_amount: 1001, + gas_price_tolerance: 1, + standard_payment: true, + }; + + let transaction_v1 = TransactionV1Builder::new_targeting_stored( + TransactionInvocationTarget::ByName("xyz".to_string()), + "abc", + TransactionRuntimeParams::VmCasperV1, + ) + .with_chain_name("x".to_string()) + .with_secret_key(&secret_key) + .with_pricing_mode(pricing_mode) + .build() + .unwrap(); + let config = build_v1_config(); + + let meta_transaction = MetaTransactionV1::from_transaction_v1(&transaction_v1, &config) + .expect("meta transaction should be valid"); + assert_eq!(meta_transaction.transaction_lane(), 4); + } + + fn build_v1_config() -> TransactionV1Config { + let mut config = TransactionV1Config::default(); + config.set_wasm_lanes(vec![ + TransactionLimitsDefinition { + id: 3, + max_transaction_length: 100, + max_transaction_args_length: 100, + max_transaction_gas_limit: 100, + max_transaction_count: 10, + }, + TransactionLimitsDefinition { + id: 4, + max_transaction_length: 10000, + max_transaction_args_length: 100, + max_transaction_gas_limit: 100, + max_transaction_count: 10, + }, + TransactionLimitsDefinition { + id: 5, + max_transaction_length: 1000, + max_transaction_args_length: 100, + max_transaction_gas_limit: 100, + max_transaction_count: 10, + }, + ]); + config + } +} diff --git a/node/src/types/transaction/meta_transaction/transaction_lane.rs b/node/src/types/transaction/meta_transaction/transaction_lane.rs index 66cbc54db1..ee1246feb7 100644 --- a/node/src/types/transaction/meta_transaction/transaction_lane.rs +++ b/node/src/types/transaction/meta_transaction/transaction_lane.rs @@ -4,7 +4,7 @@ use core::{ }; use casper_types::{ - InvalidTransaction, InvalidTransactionV1, TransactionConfig, TransactionEntryPoint, + InvalidTransaction, InvalidTransactionV1, PricingMode, TransactionEntryPoint, TransactionRuntimeParams, TransactionTarget, TransactionV1Config, AUCTION_LANE_ID, INSTALL_UPGRADE_LANE_ID, MINT_LANE_ID, }; @@ -71,9 +71,9 @@ impl fmt::Display for TransactionLane { pub(crate) fn calculate_transaction_lane( entry_point: &TransactionEntryPoint, target: &TransactionTarget, - additional_computation_factor: u8, - transaction_config: &TransactionConfig, - transaction_size: u64, + pricing_mode: &PricingMode, + config: &TransactionV1Config, + size_estimation: u64, ) -> Result { match target { TransactionTarget::Native => match entry_point { @@ -95,11 +95,9 @@ pub(crate) fn calculate_transaction_lane( } }, TransactionTarget::Stored { .. } => match entry_point { - TransactionEntryPoint::Custom(_) => get_lane_for_non_install_wasm( - &transaction_config.transaction_v1_config, - transaction_size, - additional_computation_factor, - ), + TransactionEntryPoint::Custom(_) => { + get_lane_for_non_install_wasm(config, size_estimation, pricing_mode) + } TransactionEntryPoint::Call | TransactionEntryPoint::Transfer | TransactionEntryPoint::AddBid @@ -125,11 +123,7 @@ pub(crate) fn calculate_transaction_lane( if *is_install_upgrade { Ok(INSTALL_UPGRADE_LANE_ID) } else { - get_lane_for_non_install_wasm( - &transaction_config.transaction_v1_config, - transaction_size, - additional_computation_factor, - ) + get_lane_for_non_install_wasm(config, size_estimation, pricing_mode) } } TransactionEntryPoint::Custom(_) @@ -157,11 +151,7 @@ pub(crate) fn calculate_transaction_lane( if *is_install_upgrade { Ok(INSTALL_UPGRADE_LANE_ID) } else { - get_lane_for_non_install_wasm( - &transaction_config.transaction_v1_config, - transaction_size, - additional_computation_factor, - ) + get_lane_for_non_install_wasm(config, size_estimation, pricing_mode) } } TransactionEntryPoint::Transfer @@ -185,9 +175,18 @@ pub(crate) fn calculate_transaction_lane( fn get_lane_for_non_install_wasm( config: &TransactionV1Config, transaction_size: u64, - additional_computation_factor: u8, + pricing_mode: &PricingMode, ) -> Result { - config - .get_wasm_lane_id(transaction_size, additional_computation_factor) - .ok_or(InvalidTransactionV1::NoWasmLaneMatchesTransaction()) + match pricing_mode { + PricingMode::PaymentLimited { payment_amount, .. } => config + .get_wasm_lane_id_by_gas_limit(*payment_amount) + .ok_or(InvalidTransactionV1::NoWasmLaneMatchesTransaction()), + PricingMode::Fixed { + additional_computation_factor, + .. + } => config + .get_wasm_lane_id_by_size(transaction_size, *additional_computation_factor) + .ok_or(InvalidTransactionV1::NoWasmLaneMatchesTransaction()), + PricingMode::Prepaid { .. } => Err(InvalidTransactionV1::PricingModeNotSupported), + } } diff --git a/node/src/types/transaction/transaction_v1_builder.rs b/node/src/types/transaction/transaction_v1_builder.rs index b065fd7433..656258e411 100644 --- a/node/src/types/transaction/transaction_v1_builder.rs +++ b/node/src/types/transaction/transaction_v1_builder.rs @@ -176,7 +176,7 @@ impl<'a> TransactionV1Builder<'a> { /// Returns a new `TransactionV1Builder` suitable for building a native transfer transaction. #[cfg(test)] - pub fn new_transfer, T: Into>( + pub(crate) fn new_transfer, T: Into>( amount: A, maybe_source: Option, target: T, @@ -193,7 +193,7 @@ impl<'a> TransactionV1Builder<'a> { /// Returns a new `TransactionV1Builder` suitable for building a native add_bid transaction. #[cfg(test)] - pub fn new_add_bid>( + pub(crate) fn new_add_bid>( public_key: PublicKey, delegation_rate: u8, amount: A, @@ -220,7 +220,7 @@ impl<'a> TransactionV1Builder<'a> { /// Returns a new `TransactionV1Builder` suitable for building a native withdraw_bid /// transaction. #[cfg(test)] - pub fn new_withdraw_bid>( + pub(crate) fn new_withdraw_bid>( public_key: PublicKey, amount: A, ) -> Result { @@ -235,7 +235,7 @@ impl<'a> TransactionV1Builder<'a> { /// Returns a new `TransactionV1Builder` suitable for building a native delegate transaction. #[cfg(test)] - pub fn new_delegate>( + pub(crate) fn new_delegate>( delegator: PublicKey, validator: PublicKey, amount: A, @@ -251,7 +251,7 @@ impl<'a> TransactionV1Builder<'a> { /// Returns a new `TransactionV1Builder` suitable for building a native undelegate transaction. #[cfg(test)] - pub fn new_undelegate>( + pub(crate) fn new_undelegate>( delegator: PublicKey, validator: PublicKey, amount: A, @@ -266,7 +266,7 @@ impl<'a> TransactionV1Builder<'a> { } #[cfg(test)] - fn new_targeting_stored>( + pub(crate) fn new_targeting_stored>( id: TransactionInvocationTarget, entry_point: E, runtime: TransactionRuntimeParams, @@ -283,7 +283,7 @@ impl<'a> TransactionV1Builder<'a> { /// Returns a new `TransactionV1Builder` suitable for building a transaction targeting a stored /// entity. #[cfg(test)] - pub fn new_targeting_invocable_entity>( + pub(crate) fn new_targeting_invocable_entity>( hash: AddressableEntityHash, entry_point: E, runtime: TransactionRuntimeParams, @@ -295,7 +295,7 @@ impl<'a> TransactionV1Builder<'a> { /// Returns a new `TransactionV1Builder` suitable for building a transaction targeting a stored /// entity via its alias. #[cfg(test)] - pub fn new_targeting_invocable_entity_via_alias, E: Into>( + pub(crate) fn new_targeting_invocable_entity_via_alias, E: Into>( alias: A, entry_point: E, runtime: TransactionRuntimeParams, @@ -307,7 +307,7 @@ impl<'a> TransactionV1Builder<'a> { /// Returns a new `TransactionV1Builder` suitable for building a transaction targeting a /// package. #[cfg(test)] - pub fn new_targeting_package>( + pub(crate) fn new_targeting_package>( hash: PackageHash, version: Option, entry_point: E, @@ -320,7 +320,7 @@ impl<'a> TransactionV1Builder<'a> { /// Returns a new `TransactionV1Builder` suitable for building a transaction targeting a /// package via its alias. #[cfg(test)] - pub fn new_targeting_package_via_alias, E: Into>( + pub(crate) fn new_targeting_package_via_alias, E: Into>( alias: A, version: Option, entry_point: E, @@ -332,7 +332,7 @@ impl<'a> TransactionV1Builder<'a> { /// Returns a new `TransactionV1Builder` suitable for building a transaction for running session /// logic, i.e. compiled Wasm. - pub fn new_session( + pub(crate) fn new_session( is_install_upgrade: bool, module_bytes: Bytes, runtime: TransactionRuntimeParams, @@ -357,7 +357,7 @@ impl<'a> TransactionV1Builder<'a> { /// * unsigned by calling `with_no_secret_key` /// * given an invalid approval by calling `with_invalid_approval` #[cfg(test)] - pub fn new_random(rng: &mut TestRng) -> Self { + pub(crate) fn new_random(rng: &mut TestRng) -> Self { let secret_key = SecretKey::random(rng); let ttl_millis = rng.gen_range(60_000..TransactionConfig::default().max_ttl.millis()); let fields = FieldsContainer::random(rng); @@ -384,7 +384,7 @@ impl<'a> TransactionV1Builder<'a> { } #[cfg(test)] - pub fn new_random_with_category_and_timestamp_and_ttl( + pub(crate) fn new_random_with_category_and_timestamp_and_ttl( rng: &mut TestRng, lane: u8, timestamp: Option, @@ -426,7 +426,7 @@ impl<'a> TransactionV1Builder<'a> { /// Sets the `chain_name` in the transaction. /// /// Must be provided or building will fail. - pub fn with_chain_name>(mut self, chain_name: C) -> Self { + pub(crate) fn with_chain_name>(mut self, chain_name: C) -> Self { self.chain_name = Some(chain_name.into()); self } @@ -434,7 +434,7 @@ impl<'a> TransactionV1Builder<'a> { /// Sets the `timestamp` in the transaction. /// /// If not provided, the timestamp will be set to the time when the builder was constructed. - pub fn with_timestamp(mut self, timestamp: Timestamp) -> Self { + pub(crate) fn with_timestamp(mut self, timestamp: Timestamp) -> Self { self.timestamp = timestamp; self } @@ -442,7 +442,7 @@ impl<'a> TransactionV1Builder<'a> { /// Sets the `ttl` (time-to-live) in the transaction. /// /// If not provided, the ttl will be set to [`Self::DEFAULT_TTL`]. - pub fn with_ttl(mut self, ttl: TimeDiff) -> Self { + pub(crate) fn with_ttl(mut self, ttl: TimeDiff) -> Self { self.ttl = ttl; self } @@ -451,7 +451,7 @@ impl<'a> TransactionV1Builder<'a> { /// /// If not provided, the pricing mode will be set to [`Self::DEFAULT_PRICING_MODE`]. #[cfg(test)] - pub fn with_pricing_mode(mut self, pricing_mode: PricingMode) -> Self { + pub(crate) fn with_pricing_mode(mut self, pricing_mode: PricingMode) -> Self { self.pricing_mode = pricing_mode; self } @@ -461,7 +461,7 @@ impl<'a> TransactionV1Builder<'a> { /// If not provided, the public key derived from the secret key used in the builder will be /// used as the `InitiatorAddr::PublicKey` in the transaction. #[cfg(test)] - pub fn with_initiator_addr>(mut self, initiator_addr: I) -> Self { + pub(crate) fn with_initiator_addr>(mut self, initiator_addr: I) -> Self { self.initiator_addr = Some(initiator_addr.into()); self } @@ -470,7 +470,7 @@ impl<'a> TransactionV1Builder<'a> { /// /// If not provided, the transaction can still be built, but will be unsigned and will be /// invalid until subsequently signed. - pub fn with_secret_key(mut self, secret_key: &'a SecretKey) -> Self { + pub(crate) fn with_secret_key(mut self, secret_key: &'a SecretKey) -> Self { #[cfg(not(test))] { self.secret_key = Some(secret_key); @@ -487,7 +487,10 @@ impl<'a> TransactionV1Builder<'a> { /// Manually sets additional fields #[cfg(test)] - pub fn with_additional_fields(mut self, additional_fields: BTreeMap) -> Self { + pub(crate) fn with_additional_fields( + mut self, + additional_fields: BTreeMap, + ) -> Self { self.additional_fields = additional_fields; self } @@ -497,7 +500,7 @@ impl<'a> TransactionV1Builder<'a> { /// NOTE: this overwrites any existing runtime args. To append to existing args, use /// [`TransactionV1Builder::with_runtime_arg`]. #[cfg(test)] - pub fn with_runtime_args(mut self, args: RuntimeArgs) -> Self { + pub(crate) fn with_runtime_args(mut self, args: RuntimeArgs) -> Self { self.args = TransactionArgs::Named(args); self } @@ -505,7 +508,7 @@ impl<'a> TransactionV1Builder<'a> { /// Returns the new transaction, or an error if non-defaulted fields were not set. /// /// For more info, see [the `TransactionBuilder` documentation](TransactionV1Builder). - pub fn build(self) -> Result { + pub(crate) fn build(self) -> Result { self.do_build() } diff --git a/node/src/utils/chain_specification.rs b/node/src/utils/chain_specification.rs index fc5521cfb6..3f71c23531 100644 --- a/node/src/utils/chain_specification.rs +++ b/node/src/utils/chain_specification.rs @@ -1,16 +1,23 @@ pub(crate) mod error; pub(crate) mod parse_toml; +use std::collections::HashSet; + use num_rational::Ratio; +use once_cell::sync::Lazy; use tracing::{error, info, warn}; use casper_types::{ system::auction::VESTING_SCHEDULE_LENGTH_MILLIS, Chainspec, ConsensusProtocolName, CoreConfig, - ProtocolConfig, TimeDiff, TransactionConfig, + ProtocolConfig, TimeDiff, TransactionConfig, AUCTION_LANE_ID, INSTALL_UPGRADE_LANE_ID, + MINT_LANE_ID, }; use crate::components::network; +static RESERVED_LANE_IDS: Lazy> = + Lazy::new(|| vec![MINT_LANE_ID, AUCTION_LANE_ID, INSTALL_UPGRADE_LANE_ID]); + /// Returns `false` and logs errors if the values set in the config don't make sense. #[tracing::instrument(ret, level = "info", skip(chainspec), fields(hash = % chainspec.hash()))] pub fn validate_chainspec(chainspec: &Chainspec) -> bool { @@ -140,7 +147,34 @@ pub(crate) fn validate_transaction_config(transaction_config: &TransactionConfig let total_txn_slots = transaction_config .transaction_v1_config .get_max_block_count(); - transaction_config.block_max_approval_count >= total_txn_slots as u32 + if transaction_config.block_max_approval_count < total_txn_slots as u32 { + return false; + } + let mut seen_max_transaction_size = HashSet::new(); + for wasm_lane_config in transaction_config.transaction_v1_config.wasm_lanes().iter() { + if RESERVED_LANE_IDS.contains(&wasm_lane_config.id) { + error!("One of the defined wasm lanes has declared an id that is reserved for system lanes. Offending lane id: {}", wasm_lane_config.id); + return false; + } + let max_transaction_length = wasm_lane_config.max_transaction_length; + if seen_max_transaction_size.contains(&max_transaction_length) { + error!("Found wasm lane configuration that has non-unique max_transaction_length. Duplicate value: {}", max_transaction_length); + return false; + } + seen_max_transaction_size.insert(max_transaction_length); + } + + let mut seen_max_gas_prices = HashSet::new(); + for wasm_lane_config in transaction_config.transaction_v1_config.wasm_lanes().iter() { + //No need to check reserved lanes, we just did that + let max_transaction_gas_limit = wasm_lane_config.max_transaction_gas_limit; + if seen_max_gas_prices.contains(&max_transaction_gas_limit) { + error!("Found wasm lane configuration that has non-unique max_transaction_gas_limit. Duplicate value: {}", max_transaction_gas_limit); + return false; + } + seen_max_gas_prices.insert(max_transaction_gas_limit); + } + true } #[cfg(test)] @@ -154,8 +188,8 @@ mod tests { bytesrepr::FromBytes, ActivationPoint, BrTableCost, ChainspecRawBytes, ControlFlowCosts, CoreConfig, EraId, GlobalStateUpdate, HighwayConfig, HostFunction, HostFunctionCosts, MessageLimits, Motes, OpcodeCosts, ProtocolConfig, ProtocolVersion, StoredValue, - TestBlockBuilder, TimeDiff, Timestamp, TransactionConfig, TransactionV1Config, WasmConfig, - WasmV1Config, MINT_LANE_ID, + TestBlockBuilder, TimeDiff, Timestamp, TransactionConfig, TransactionLimitsDefinition, + TransactionV1Config, WasmConfig, WasmV1Config, MINT_LANE_ID, }; use super::*; @@ -654,4 +688,124 @@ mod tests { let (chainspec, _) = <(Chainspec, ChainspecRawBytes)>::from_resources("test/valid/1_0_0"); check_spec(chainspec, false); } + + #[test] + fn should_fail_when_wasm_lanes_have_duplicate_max_transaction_length() { + let mut v1_config = TransactionV1Config::default(); + let definition_1 = TransactionLimitsDefinition { + id: 3, + max_transaction_length: 100, + max_transaction_args_length: 100, + max_transaction_gas_limit: 100, + max_transaction_count: 10, + }; + let definition_2 = TransactionLimitsDefinition { + id: 4, + max_transaction_length: 10000, + max_transaction_args_length: 100, + max_transaction_gas_limit: 100, + max_transaction_count: 10, + }; + let definition_3 = TransactionLimitsDefinition { + id: 5, + max_transaction_length: 1000, + max_transaction_args_length: 100, + max_transaction_gas_limit: 100, + max_transaction_count: 10, + }; + v1_config.set_wasm_lanes(vec![ + definition_1.clone(), + definition_2.clone(), + definition_3.clone(), + ]); + let transaction_config = TransactionConfig { + transaction_v1_config: v1_config.clone(), + ..Default::default() + }; + assert!(validate_transaction_config(&transaction_config)); + let mut definition_2 = definition_2.clone(); + definition_2.max_transaction_length = definition_1.max_transaction_length; + v1_config.set_wasm_lanes(vec![ + definition_1.clone(), + definition_2.clone(), + definition_3.clone(), + ]); + let transaction_config = TransactionConfig { + transaction_v1_config: v1_config, + ..Default::default() + }; + assert!(!validate_transaction_config(&transaction_config)); + } + + #[test] + fn should_fail_when_wasm_lanes_have_duplicate_max_gas_price() { + let mut v1_config = TransactionV1Config::default(); + let definition_1 = TransactionLimitsDefinition { + id: 3, + max_transaction_length: 100, + max_transaction_args_length: 100, + max_transaction_gas_limit: 100, + max_transaction_count: 10, + }; + let definition_2 = TransactionLimitsDefinition { + id: 4, + max_transaction_length: 10000, + max_transaction_args_length: 100, + max_transaction_gas_limit: 101, + max_transaction_count: 10, + }; + let definition_3 = TransactionLimitsDefinition { + id: 5, + max_transaction_length: 1000, + max_transaction_args_length: 100, + max_transaction_gas_limit: 102, + max_transaction_count: 10, + }; + v1_config.set_wasm_lanes(vec![ + definition_1.clone(), + definition_2.clone(), + definition_3.clone(), + ]); + let transaction_config = TransactionConfig { + transaction_v1_config: v1_config.clone(), + ..Default::default() + }; + assert!(validate_transaction_config(&transaction_config)); + let mut definition_2 = definition_2.clone(); + definition_2.max_transaction_gas_limit = definition_1.max_transaction_gas_limit; + v1_config.set_wasm_lanes(vec![ + definition_1.clone(), + definition_2.clone(), + definition_3.clone(), + ]); + let transaction_config = TransactionConfig { + transaction_v1_config: v1_config, + ..Default::default() + }; + assert!(!validate_transaction_config(&transaction_config)); + } + + #[test] + fn should_fail_when_wasm_lanes_have_reseved_ids() { + fail_validation_with_lane_id(MINT_LANE_ID); + fail_validation_with_lane_id(AUCTION_LANE_ID); + fail_validation_with_lane_id(INSTALL_UPGRADE_LANE_ID); + } + + fn fail_validation_with_lane_id(lane_id: u8) { + let mut v1_config = TransactionV1Config::default(); + let definition_1 = TransactionLimitsDefinition { + id: lane_id, + max_transaction_length: 100, + max_transaction_args_length: 100, + max_transaction_gas_limit: 100, + max_transaction_count: 10, + }; + v1_config.set_wasm_lanes(vec![definition_1.clone()]); + let transaction_config = TransactionConfig { + transaction_v1_config: v1_config.clone(), + ..Default::default() + }; + assert!(!validate_transaction_config(&transaction_config)); + } } diff --git a/types/src/block/test_block_builder/test_block_v2_builder.rs b/types/src/block/test_block_builder/test_block_v2_builder.rs index d2c268a2f1..d23490a585 100644 --- a/types/src/block/test_block_builder/test_block_v2_builder.rs +++ b/types/src/block/test_block_builder/test_block_v2_builder.rs @@ -248,7 +248,7 @@ impl TestBlockV2Builder { // A simplified way of calculating transaction lanes. It doesn't take // into consideration the size of the transaction against the chainspec -// and doesn't take `additional_compufsdetation_factor` into consideration. +// and doesn't take `additional_computation_factor` into consideration. // This is only used for tests purposes. fn simplified_calculate_transaction_lane_from_values( entry_point: &TransactionEntryPoint, diff --git a/types/src/chainspec/transaction_config/transaction_v1_config.rs b/types/src/chainspec/transaction_config/transaction_v1_config.rs index d423bb225d..2e455c198c 100644 --- a/types/src/chainspec/transaction_config/transaction_v1_config.rs +++ b/types/src/chainspec/transaction_config/transaction_v1_config.rs @@ -11,6 +11,7 @@ use serde::{ ser::SerializeSeq, Deserialize, Deserializer, Serialize, Serializer, }; +use tracing::error; #[cfg(any(feature = "testing", test))] use crate::testing::TestRng; @@ -38,7 +39,7 @@ const TRANSACTION_COUNT_INDEX: usize = 4; pub struct TransactionLimitsDefinition { /// The lane identifier pub id: u8, - /// The maximum length of a transaction i bytes + /// The maximum length of a transaction in bytes pub max_transaction_length: u64, /// The maximum number of runtime args pub max_transaction_args_length: u64, @@ -153,7 +154,7 @@ pub struct TransactionV1Config { deserialize_with = "definition_to_wasms" )] /// Lane configurations for Wasm based lanes that are not declared as install/upgrade. - pub wasm_lanes: Vec, + wasm_lanes: Vec, #[cfg_attr(any(all(feature = "std", feature = "once_cell"), test), serde(skip))] #[cfg_attr( all(any(feature = "once_cell", test), feature = "datasize"), @@ -161,6 +162,13 @@ pub struct TransactionV1Config { )] #[cfg(any(feature = "once_cell", test))] wasm_lanes_ordered_by_transaction_size: OnceCell>, + #[cfg_attr(any(all(feature = "std", feature = "once_cell"), test), serde(skip))] + #[cfg_attr( + all(any(feature = "once_cell", test), feature = "datasize"), + data_size(skip) + )] + #[cfg(any(feature = "once_cell", test))] + wasm_lanes_ordered_by_transaction_gas_limit: OnceCell>, } impl PartialEq for TransactionV1Config { @@ -173,6 +181,8 @@ impl PartialEq for TransactionV1Config { wasm_lanes, #[cfg(any(feature = "once_cell", test))] wasm_lanes_ordered_by_transaction_size: _, + #[cfg(any(feature = "once_cell", test))] + wasm_lanes_ordered_by_transaction_gas_limit: _, } = self; *native_mint_lane == other.native_mint_lane && *native_auction_lane == other.native_auction_lane @@ -193,6 +203,10 @@ impl TransactionV1Config { let wasm_lanes_ordered_by_transaction_size = OnceCell::with_value( Self::build_wasm_lanes_ordered_by_transaction_size(wasm_lanes.clone()), ); + #[cfg(any(feature = "once_cell", test))] + let wasm_lanes_ordered_by_transaction_gas_limit = OnceCell::with_value( + Self::build_wasm_lanes_ordered_by_transaction_gas_limit(wasm_lanes.clone()), + ); TransactionV1Config { native_mint_lane, native_auction_lane, @@ -200,6 +214,8 @@ impl TransactionV1Config { wasm_lanes, #[cfg(any(feature = "once_cell", test))] wasm_lanes_ordered_by_transaction_size, + #[cfg(any(feature = "once_cell", test))] + wasm_lanes_ordered_by_transaction_gas_limit, } } @@ -354,13 +370,13 @@ impl TransactionV1Config { /// Returns a wasm lane id based on the transaction size adjusted by /// maybe_additional_computation_factor if necessary. - pub fn get_wasm_lane_id( + pub fn get_wasm_lane_id_by_size( &self, transaction_size: u64, additional_computation_factor: u8, ) -> Option { let mut maybe_adequate_lane_index = None; - let buckets = self.get_wasm_lanes_ordered(); + let buckets = self.get_wasm_lanes_ordered_by_transaction_size(); let number_of_lanes = buckets.len(); for (i, lane) in buckets.iter().enumerate() { let lane_size = lane.max_transaction_length; @@ -378,10 +394,27 @@ impl TransactionV1Config { maybe_adequate_lane_index.map(|index| buckets[index].id) } + pub fn get_wasm_lane_id_by_gas_limit(&self, gas_limit: u64) -> Option { + let mut maybe_adequate_lane_index = None; + let buckets = self.get_wasm_lanes_ordered_by_gas_limit(); + for (i, lane) in buckets.iter().enumerate() { + let lane_size = lane.max_transaction_gas_limit; + if lane_size >= gas_limit { + maybe_adequate_lane_index = Some(i); + break; + } + } + error!( + "maybe_adequate_lane_index maybe_adequate_lane_index: {:?}", + maybe_adequate_lane_index + ); + maybe_adequate_lane_index.map(|index| buckets[index].id) + } + #[allow(unreachable_code)] //We're allowing unreachable code here because there's a possibility that someone might // want to use the types crate without once_cell - fn get_wasm_lanes_ordered(&self) -> &Vec { + fn get_wasm_lanes_ordered_by_transaction_size(&self) -> &Vec { #[cfg(any(feature = "once_cell", test))] return self.wasm_lanes_ordered_by_transaction_size.get_or_init(|| { Self::build_wasm_lanes_ordered_by_transaction_size(self.wasm_lanes.clone()) @@ -389,13 +422,57 @@ impl TransactionV1Config { &Self::build_wasm_lanes_ordered_by_transaction_size(self.wasm_lanes.clone()) } + #[allow(unreachable_code)] + //We're allowing unreachable code here because there's a possibility that someone might + // want to use the types crate without once_cell + fn get_wasm_lanes_ordered_by_gas_limit(&self) -> &Vec { + #[cfg(any(feature = "once_cell", test))] + return self + .wasm_lanes_ordered_by_transaction_gas_limit + .get_or_init(|| { + Self::build_wasm_lanes_ordered_by_transaction_gas_limit(self.wasm_lanes.clone()) + }); + &Self::build_wasm_lanes_ordered_by_transaction_gas_limit(self.wasm_lanes.clone()) + } + + fn build_wasm_lanes_ordered_by_transaction_gas_limit( + wasm_lanes: Vec, + ) -> Vec { + let mut ordered = wasm_lanes; + ordered.sort_by(|a, b| { + a.max_transaction_gas_limit + .cmp(&b.max_transaction_gas_limit) + }); + ordered + } + fn build_wasm_lanes_ordered_by_transaction_size( wasm_lanes: Vec, ) -> Vec { - let mut wasm_lanes_ordered_by_transaction_size = wasm_lanes; - wasm_lanes_ordered_by_transaction_size - .sort_by(|a, b| a.max_transaction_length.cmp(&b.max_transaction_length)); - wasm_lanes_ordered_by_transaction_size + let mut ordered = wasm_lanes; + ordered.sort_by(|a, b| a.max_transaction_length.cmp(&b.max_transaction_length)); + ordered + } + + pub fn wasm_lanes(&self) -> &Vec { + &self.wasm_lanes + } + + #[cfg(any(feature = "testing", test))] + pub fn set_wasm_lanes(&mut self, wasm_lanes: Vec) { + self.wasm_lanes = wasm_lanes; + #[cfg(any(feature = "once_cell", test))] + { + let wasm_lanes_ordered_by_transaction_size = OnceCell::with_value( + Self::build_wasm_lanes_ordered_by_transaction_size(self.wasm_lanes.clone()), + ); + self.wasm_lanes_ordered_by_transaction_size = wasm_lanes_ordered_by_transaction_size; + let wasm_lanes_ordered_by_transaction_gas_limit = OnceCell::with_value( + Self::build_wasm_lanes_ordered_by_transaction_gas_limit(self.wasm_lanes.clone()), + ); + self.wasm_lanes_ordered_by_transaction_gas_limit = + wasm_lanes_ordered_by_transaction_gas_limit; + } } } @@ -579,20 +656,20 @@ mod tests { #[test] fn should_get_configuration_for_wasm() { let config = build_example_transaction_config(); - let got = config.get_wasm_lane_id(100, 0); + let got = config.get_wasm_lane_id_by_size(100, 0); assert_eq!(got, Some(3)); let config = build_example_transaction_config_reverse_wasm_ids(); - let got = config.get_wasm_lane_id(100, 0); + let got = config.get_wasm_lane_id_by_size(100, 0); assert_eq!(got, Some(5)); } #[test] fn given_too_big_transaction_should_return_none() { let config = build_example_transaction_config(); - let got = config.get_wasm_lane_id(100000000, 0); + let got = config.get_wasm_lane_id_by_size(100000000, 0); assert!(got.is_none()); let config = build_example_transaction_config_reverse_wasm_ids(); - let got = config.get_wasm_lane_id(100000000, 0); + let got = config.get_wasm_lane_id_by_size(100000000, 0); assert!(got.is_none()); } @@ -600,61 +677,61 @@ mod tests { fn given_wasm_should_return_first_fit() { let config = build_example_transaction_config(); - let got = config.get_wasm_lane_id(660, 0); + let got = config.get_wasm_lane_id_by_size(660, 0); assert_eq!(got, Some(4)); - let got = config.get_wasm_lane_id(800, 0); + let got = config.get_wasm_lane_id_by_size(800, 0); assert_eq!(got, Some(5)); - let got = config.get_wasm_lane_id(1, 0); + let got = config.get_wasm_lane_id_by_size(1, 0); assert_eq!(got, Some(3)); let config = build_example_transaction_config_reverse_wasm_ids(); - let got = config.get_wasm_lane_id(660, 0); + let got = config.get_wasm_lane_id_by_size(660, 0); assert_eq!(got, Some(4)); - let got = config.get_wasm_lane_id(800, 0); + let got = config.get_wasm_lane_id_by_size(800, 0); assert_eq!(got, Some(3)); - let got = config.get_wasm_lane_id(1, 0); + let got = config.get_wasm_lane_id_by_size(1, 0); assert_eq!(got, Some(5)); } #[test] fn given_additional_computation_factor_should_be_applied() { let config = build_example_transaction_config(); - let got = config.get_wasm_lane_id(660, 1); + let got = config.get_wasm_lane_id_by_size(660, 1); assert_eq!(got, Some(5)); let config = build_example_transaction_config_reverse_wasm_ids(); - let got = config.get_wasm_lane_id(660, 1); + let got = config.get_wasm_lane_id_by_size(660, 1); assert_eq!(got, Some(3)); } #[test] fn given_additional_computation_factor_should_not_overflow() { let config = build_example_transaction_config(); - let got = config.get_wasm_lane_id(660, 2); + let got = config.get_wasm_lane_id_by_size(660, 2); assert_eq!(got, Some(5)); - let got_2 = config.get_wasm_lane_id(660, 20); + let got_2 = config.get_wasm_lane_id_by_size(660, 20); assert_eq!(got_2, Some(5)); let config = build_example_transaction_config_reverse_wasm_ids(); - let got = config.get_wasm_lane_id(660, 2); + let got = config.get_wasm_lane_id_by_size(660, 2); assert_eq!(got, Some(3)); - let got_2 = config.get_wasm_lane_id(660, 20); + let got_2 = config.get_wasm_lane_id_by_size(660, 20); assert_eq!(got_2, Some(3)); } #[test] fn given_no_wasm_lanes_should_return_none() { let config = build_example_transaction_config_no_wasms(); - let got = config.get_wasm_lane_id(660, 2); + let got = config.get_wasm_lane_id_by_size(660, 2); assert!(got.is_none()); - let got = config.get_wasm_lane_id(660, 0); + let got = config.get_wasm_lane_id_by_size(660, 0); assert!(got.is_none()); - let got = config.get_wasm_lane_id(660, 20); + let got = config.get_wasm_lane_id_by_size(660, 20); assert!(got.is_none()); } diff --git a/types/src/transaction/transaction_v1/errors_v1.rs b/types/src/transaction/transaction_v1/errors_v1.rs index afa9589df7..df3f384971 100644 --- a/types/src/transaction/transaction_v1/errors_v1.rs +++ b/types/src/transaction/transaction_v1/errors_v1.rs @@ -200,6 +200,8 @@ pub enum InvalidTransaction { }, /// The transaction is missing a seed field. MissingSeed, + // Pricing mode not implemented yet + PricingModeNotSupported, } impl Display for InvalidTransaction { @@ -389,6 +391,9 @@ impl Display for InvalidTransaction { InvalidTransaction::MissingSeed => { write!(formatter, "missing seed for install or upgrade") } + InvalidTransaction::PricingModeNotSupported => { + write!(formatter, "Pricing mode not supported") + } } } } @@ -439,7 +444,8 @@ impl StdError for InvalidTransaction { }, InvalidTransaction::ExpectedNamedArguments | InvalidTransaction::InvalidTransactionRuntime { .. } - | InvalidTransaction::MissingSeed => None, + | InvalidTransaction::MissingSeed + | InvalidTransaction::PricingModeNotSupported => None, } } }