From ae2c6e107e6bff182e9c922f3006af250272f31a Mon Sep 17 00:00:00 2001 From: Karan Dhareshwar Date: Wed, 26 Feb 2025 08:31:09 -0600 Subject: [PATCH 1/3] Fix calculation of gas cost in payment limited --- types/src/transaction/pricing_mode.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/types/src/transaction/pricing_mode.rs b/types/src/transaction/pricing_mode.rs index 73bde157fa..ac49498161 100644 --- a/types/src/transaction/pricing_mode.rs +++ b/types/src/transaction/pricing_mode.rs @@ -214,10 +214,12 @@ impl PricingMode { ) -> Result { let gas_limit = self.gas_limit(chainspec, entry_point, lane_id)?; let motes = match self { - PricingMode::PaymentLimited { .. } | PricingMode::Fixed { .. } => { - Motes::from_gas(gas_limit, gas_price) + PricingMode::PaymentLimited { payment_amount, .. } => { + Motes::from_gas(Gas::from(*payment_amount), gas_price) .ok_or(PricingModeError::UnableToCalculateGasCost)? } + PricingMode::Fixed { .. } => Motes::from_gas(gas_limit, gas_price) + .ok_or(PricingModeError::UnableToCalculateGasCost)?, PricingMode::Prepaid { .. } => { Motes::zero() // prepaid } From 0bfac6ca0d7616b594a1996498d99b9dfd295611 Mon Sep 17 00:00:00 2001 From: Karan Dhareshwar Date: Wed, 26 Feb 2025 10:00:19 -0600 Subject: [PATCH 2/3] Add additional regression test --- .../main_reactor/tests/transactions.rs | 120 ++++++++++++++++++ 1 file changed, 120 insertions(+) diff --git a/node/src/reactor/main_reactor/tests/transactions.rs b/node/src/reactor/main_reactor/tests/transactions.rs index f87d6b9642..db554f9c25 100644 --- a/node/src/reactor/main_reactor/tests/transactions.rs +++ b/node/src/reactor/main_reactor/tests/transactions.rs @@ -82,6 +82,52 @@ async fn transfer_to_account>( ) } +async fn send_add_bid>( + fixture: &mut TestFixture, + amount: A, + signing_key: &SecretKey, + pricing: PricingMode, +) -> (TransactionHash, u64, ExecutionResult) { + let chain_name = fixture.chainspec.network_config.name.clone(); + let public_key = PublicKey::from(signing_key); + + let mut txn = Transaction::from( + TransactionV1Builder::new_add_bid(public_key.clone(), 10, amount, None, None, None) + .unwrap() + .with_initiator_addr(public_key) + .with_pricing_mode(pricing) + .with_chain_name(chain_name) + .build() + .unwrap(), + ); + + txn.sign(signing_key); + let txn_hash = txn.hash(); + + fixture.inject_transaction(txn).await; + + info!("transfer_to_account starting run_until_executed_transaction"); + fixture + .run_until_executed_transaction(&txn_hash, TEN_SECS) + .await; + + info!("transfer_to_account finished run_until_executed_transaction"); + let (_node_id, runner) = fixture.network.nodes().iter().next().unwrap(); + let exec_info = runner + .main_reactor() + .storage() + .read_execution_info(txn_hash) + .expect("Expected transaction to be included in a block."); + + ( + txn_hash, + exec_info.block_height, + exec_info + .execution_result + .expect("Exec result should have been stored."), + ) +} + async fn send_wasm_transaction( fixture: &mut TestFixture, from: &SecretKey, @@ -763,6 +809,80 @@ async fn transfer_cost_classic_price_no_fee_no_refund() { ); } +#[tokio::test] +async fn add_bid_with_classic_pricing_no_fee_no_refund() { + let initial_stakes = InitialStakes::FromVec(vec![u128::MAX, 1]); + + let config = SingleTransactionTestCase::default_test_config() + .with_pricing_handling(PricingHandling::Classic) + .with_refund_handling(RefundHandling::NoRefund) + .with_fee_handling(FeeHandling::NoFee); + + let mut fixture = TestFixture::new(initial_stakes, Some(config)).await; + + let alice_secret_key = Arc::clone(&fixture.node_contexts[0].secret_key); + let alice_public_key = PublicKey::from(&*alice_secret_key); + + fixture.run_until_consensus_in_era(ERA_ONE, ONE_MIN).await; + + let alice_initial_balance = *get_balance(&mut fixture, &alice_public_key, None, true) + .available_balance() + .expect("Expected Alice to have a balance."); + + const BID_PAYMENT_AMOUNT: u64 = 1_000_000_000; + + let bid_amount = fixture.chainspec.core_config.minimum_bid_amount + 1; + // This transaction should be included since the tolerance is above the min gas price. + let (_txn_hash, block_height, exec_result) = send_add_bid( + &mut fixture, + bid_amount, + &alice_secret_key, + PricingMode::PaymentLimited { + payment_amount: BID_PAYMENT_AMOUNT, + gas_price_tolerance: MIN_GAS_PRICE + 1, + standard_payment: true, + }, + ) + .await; + + let expected_add_bid_cost = BID_PAYMENT_AMOUNT * MIN_GAS_PRICE as u64; + + assert!(exec_result_is_success(&exec_result)); // transaction should have succeeded. + assert_exec_result_cost( + exec_result, + expected_add_bid_cost.into(), + Gas::new(BID_PAYMENT_AMOUNT), + ); + + let alice_available_balance = + get_balance(&mut fixture, &alice_public_key, Some(block_height), false); + let alice_total_balance = + get_balance(&mut fixture, &alice_public_key, Some(block_height), true); + + // since FeeHandling is set to NoFee, we expect that there's a hold on Alice's balance for the + // cost of the transfer. The total balance of Alice now should be the initial balance - the + // amount transferred to Charlie. + let alice_expected_total_balance = alice_initial_balance - bid_amount; + // The available balance is the initial balance - the amount transferred to Charlie - the hold + // for the transfer cost. + let alice_expected_available_balance = alice_expected_total_balance - expected_add_bid_cost; + + assert_eq!( + alice_total_balance + .available_balance() + .expect("Expected Alice to have a balance") + .clone(), + alice_expected_total_balance + ); + assert_eq!( + alice_available_balance + .available_balance() + .expect("Expected Alice to have a balance") + .clone(), + alice_expected_available_balance + ); +} + #[tokio::test] #[should_panic = "within 10 seconds"] async fn transaction_with_low_threshold_should_not_get_included() { From 1a6baf60118bfaa1af137fa22dd1083a0a610ab2 Mon Sep 17 00:00:00 2001 From: Karan Dhareshwar Date: Wed, 26 Feb 2025 11:55:38 -0600 Subject: [PATCH 3/3] Address lint issues --- types/src/chainspec.rs | 7 +++++++ types/src/transaction/deploy.rs | 4 ++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/types/src/chainspec.rs b/types/src/chainspec.rs index 92b50effdd..b78a18312a 100644 --- a/types/src/chainspec.rs +++ b/types/src/chainspec.rs @@ -257,6 +257,13 @@ impl Chainspec { .transaction_v1_config .get_max_transaction_count(lane) } + + /// Returns the max payment defined by the wasm lanes. + pub fn get_max_payment_limit_for_wasm(&self) -> u64 { + self.transaction_config + .transaction_v1_config + .get_max_payment_limit_for_wasm() + } } #[cfg(any(feature = "testing", test))] diff --git a/types/src/transaction/deploy.rs b/types/src/transaction/deploy.rs index 782749fc84..9f74d9bb90 100644 --- a/types/src/transaction/deploy.rs +++ b/types/src/transaction/deploy.rs @@ -60,7 +60,7 @@ use crate::{ }; #[cfg(any(feature = "std", test))] -use crate::{chainspec::PricingHandling, Chainspec, LARGE_WASM_LANE_ID}; +use crate::{chainspec::PricingHandling, Chainspec}; #[cfg(any(feature = "std", test))] use crate::{system::auction::ARG_AMOUNT, transaction::GasLimited, Gas, Motes, U512}; pub use deploy_hash::DeployHash; @@ -1387,7 +1387,7 @@ impl GasLimited for Deploy { let computation_limit = if self.is_transfer() { costs.mint_costs().transfer as u64 } else { - chainspec.get_max_gas_limit_by_category(LARGE_WASM_LANE_ID) + chainspec.get_max_payment_limit_for_wasm() }; Gas::new(computation_limit) } // legacy deploys do not support prepaid