From 86a098c7eaf16ea57a1f1ecae705e17dd3a8cec5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Chabowski?= Date: Wed, 14 Aug 2024 12:43:12 +0200 Subject: [PATCH] Do not do gas price check on transactions arriving from peers --- node/src/components/transaction_acceptor.rs | 16 ++++++---- .../components/transaction_acceptor/tests.rs | 32 +++++++++++++++++-- 2 files changed, 39 insertions(+), 9 deletions(-) diff --git a/node/src/components/transaction_acceptor.rs b/node/src/components/transaction_acceptor.rs index 1320970839..81ce72abb3 100644 --- a/node/src/components/transaction_acceptor.rs +++ b/node/src/components/transaction_acceptor.rs @@ -351,17 +351,19 @@ impl TransactionAcceptor { maybe_next_upgrade: Option, event_metadata: Box, ) -> Effects { - if let Err(error) = self.should_reject_due_to_gas_price( - &event_metadata.transaction, - self.chainspec.vacancy_config.min_gas_price, - maybe_next_upgrade, - ) { - return self.reject_transaction(effect_builder, *event_metadata, error.into()); + if event_metadata.source.is_client() { + if let Err(error) = self.should_reject_due_to_gas_price( + &event_metadata.transaction, + self.chainspec.vacancy_config.min_gas_price, + maybe_next_upgrade, + ) { + return self.reject_transaction(effect_builder, *event_metadata, error.into()); + } } // We only perform expiry checks on transactions received from the client. let current_node_timestamp = event_metadata.verification_start_timestamp; - if event_metadata.source.is_client() // TODO[RC]: We should also do this for the gas price checks + if event_metadata.source.is_client() && event_metadata.transaction.expired(current_node_timestamp) { let expiry_timestamp = event_metadata.transaction.expires(); diff --git a/node/src/components/transaction_acceptor/tests.rs b/node/src/components/transaction_acceptor/tests.rs index 0ea5898a7f..c983016988 100644 --- a/node/src/components/transaction_acceptor/tests.rs +++ b/node/src/components/transaction_acceptor/tests.rs @@ -236,8 +236,10 @@ enum TestScenario { DeployWithoutTransferAmount, BalanceCheckForDeploySentByPeer, InvalidPricingModeForTransactionV1, + FromPeerTooLowGasPriceToleranceForTransactionV1, TooLowGasPriceToleranceForTransactionV1, TooLowGasPriceToleranceForDeploy, + FromPeerUnreachableGasPriceToleranceForTransactionV1, UnreachableGasPriceToleranceForTransactionV1, UnreachableGasPriceToleranceForDeploy, } @@ -248,6 +250,8 @@ impl TestScenario { TestScenario::FromPeerInvalidTransaction(_) | TestScenario::FromPeerExpired(_) | TestScenario::FromPeerValidTransaction(_) + | TestScenario::FromPeerTooLowGasPriceToleranceForTransactionV1 + | TestScenario::FromPeerUnreachableGasPriceToleranceForTransactionV1 | TestScenario::FromPeerRepeatedValidTransaction(_) | TestScenario::BalanceCheckForDeploySentByPeer | TestScenario::FromPeerMissingAccount(_) @@ -598,7 +602,8 @@ impl TestScenario { .expect("must create classic mode transaction"); Transaction::from(classic_mode_transaction) } - TestScenario::TooLowGasPriceToleranceForTransactionV1 => { + TestScenario::TooLowGasPriceToleranceForTransactionV1 + | TestScenario::FromPeerTooLowGasPriceToleranceForTransactionV1 => { const TOO_LOW_GAS_PRICE_TOLERANCE: u8 = 0; let fixed_mode_transaction = TransactionV1Builder::new_random(rng) @@ -616,7 +621,8 @@ impl TestScenario { let deploy = Deploy::random_with_gas_price(rng, TOO_LOW_GAS_PRICE_TOLERANCE); Transaction::from(deploy) } - TestScenario::UnreachableGasPriceToleranceForTransactionV1 => { + TestScenario::UnreachableGasPriceToleranceForTransactionV1 + | TestScenario::FromPeerUnreachableGasPriceToleranceForTransactionV1 => { // For this scenario, network will start with "current gas price" set to 20. // It'll not be able to reach the gas price tolerance of 10 within 5 minutes. @@ -659,6 +665,8 @@ impl TestScenario { TestScenario::FromPeerRepeatedValidTransaction(_) | TestScenario::FromPeerExpired(_) | TestScenario::FromPeerValidTransaction(_) + | TestScenario::FromPeerTooLowGasPriceToleranceForTransactionV1 // gas price check skipped if from peer + | TestScenario::FromPeerUnreachableGasPriceToleranceForTransactionV1 // gas price check skipped if from peer | TestScenario::FromPeerMissingAccount(_) // account check skipped if from peer | TestScenario::FromPeerAccountWithInsufficientWeight(_) // account check skipped if from peer | TestScenario::FromPeerAccountWithInvalidAssociatedKeys(_) // account check skipped if from peer @@ -831,6 +839,8 @@ impl reactor::Reactor for Reactor { | TestScenario::FromClientCustomPaymentContractPackage( ContractPackageScenario::MissingContractVersion, ) + | TestScenario::FromPeerTooLowGasPriceToleranceForTransactionV1 + | TestScenario::FromPeerUnreachableGasPriceToleranceForTransactionV1 | TestScenario::FromClientSessionContractPackage( _, ContractPackageScenario::MissingContractVersion, @@ -1390,6 +1400,8 @@ async fn run_transaction_acceptor_without_timeout( | TestScenario::FromPeerMissingAccount(_) | TestScenario::FromPeerAccountWithInvalidAssociatedKeys(_) | TestScenario::FromPeerAccountWithInsufficientWeight(_) + | TestScenario::FromPeerTooLowGasPriceToleranceForTransactionV1 + | TestScenario::FromPeerUnreachableGasPriceToleranceForTransactionV1 | TestScenario::FromPeerExpired(_) => { matches!( event, @@ -2548,6 +2560,14 @@ async fn should_reject_transaction_v1_with_invalid_pricing_mode() { )) } +#[tokio::test] +async fn should_accept_transaction_v1_with_too_low_gas_price_tolerance_from_peer() { + let test_scenario = TestScenario::FromPeerTooLowGasPriceToleranceForTransactionV1; + let result = + run_transaction_acceptor_with_gas_price(test_scenario, PriceInEra::new(1.into(), 20)).await; + assert!(result.is_ok()) +} + #[tokio::test] async fn should_reject_transaction_v1_with_too_low_gas_price_tolerance() { let test_scenario = TestScenario::TooLowGasPriceToleranceForTransactionV1; @@ -2572,6 +2592,14 @@ async fn should_reject_deploy_with_too_low_gas_price_tolerance() { )) } +#[tokio::test] +async fn should_accept_transaction_v1_with_unreachable_gas_price_tolerance_from_peer() { + let test_scenario = TestScenario::FromPeerUnreachableGasPriceToleranceForTransactionV1; + let result = + run_transaction_acceptor_with_gas_price(test_scenario, PriceInEra::new(1.into(), 20)).await; + assert!(result.is_ok()) +} + #[tokio::test] async fn should_reject_transaction_v1_with_unreachable_gas_price_tolerance() { let test_scenario = TestScenario::UnreachableGasPriceToleranceForTransactionV1;