Skip to content

Commit

Permalink
Do not do gas price check on transactions arriving from peers
Browse files Browse the repository at this point in the history
  • Loading branch information
Rafał Chabowski committed Aug 14, 2024
1 parent 56b82f4 commit 86a098c
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 9 deletions.
16 changes: 9 additions & 7 deletions node/src/components/transaction_acceptor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -351,17 +351,19 @@ impl TransactionAcceptor {
maybe_next_upgrade: Option<NextUpgrade>,
event_metadata: Box<EventMetadata>,
) -> Effects<Event> {
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();
Expand Down
32 changes: 30 additions & 2 deletions node/src/components/transaction_acceptor/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,8 +236,10 @@ enum TestScenario {
DeployWithoutTransferAmount,
BalanceCheckForDeploySentByPeer,
InvalidPricingModeForTransactionV1,
FromPeerTooLowGasPriceToleranceForTransactionV1,
TooLowGasPriceToleranceForTransactionV1,
TooLowGasPriceToleranceForDeploy,
FromPeerUnreachableGasPriceToleranceForTransactionV1,
UnreachableGasPriceToleranceForTransactionV1,
UnreachableGasPriceToleranceForDeploy,
}
Expand All @@ -248,6 +250,8 @@ impl TestScenario {
TestScenario::FromPeerInvalidTransaction(_)
| TestScenario::FromPeerExpired(_)
| TestScenario::FromPeerValidTransaction(_)
| TestScenario::FromPeerTooLowGasPriceToleranceForTransactionV1
| TestScenario::FromPeerUnreachableGasPriceToleranceForTransactionV1
| TestScenario::FromPeerRepeatedValidTransaction(_)
| TestScenario::BalanceCheckForDeploySentByPeer
| TestScenario::FromPeerMissingAccount(_)
Expand Down Expand Up @@ -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)
Expand All @@ -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.

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -831,6 +839,8 @@ impl reactor::Reactor for Reactor {
| TestScenario::FromClientCustomPaymentContractPackage(
ContractPackageScenario::MissingContractVersion,
)
| TestScenario::FromPeerTooLowGasPriceToleranceForTransactionV1
| TestScenario::FromPeerUnreachableGasPriceToleranceForTransactionV1
| TestScenario::FromClientSessionContractPackage(
_,
ContractPackageScenario::MissingContractVersion,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down

0 comments on commit 86a098c

Please sign in to comment.