Skip to content

Commit

Permalink
Improve transactor acceptor logic.
Browse files Browse the repository at this point in the history
  • Loading branch information
mpapierski committed Dec 12, 2024
1 parent c881b11 commit 54bfafb
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 3 deletions.
41 changes: 38 additions & 3 deletions node/src/components/transaction_acceptor/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ use casper_types::{
Block, BlockV2, CLValue, Chainspec, ChainspecRawBytes, Contract, Deploy, EraId, HashAddr,
InvalidDeploy, InvalidTransaction, InvalidTransactionV1, Package, PricingHandling, PricingMode,
ProtocolVersion, PublicKey, SecretKey, StoredValue, TestBlockBuilder, TimeDiff, Timestamp,
Transaction, TransactionConfig, TransactionRuntimeParams, TransactionV1, URef, U512,
Transaction, TransactionArgs, TransactionConfig, TransactionRuntimeParams, TransactionV1, URef,
U512,
};

use super::*;
Expand Down Expand Up @@ -216,6 +217,7 @@ enum TestScenario {
TooLowGasPriceToleranceForDeploy,
InvalidFields,
InvalidFieldsFromPeer,
InvalidArgumentsKind,
}

impl TestScenario {
Expand Down Expand Up @@ -260,7 +262,8 @@ impl TestScenario {
| TestScenario::InvalidPricingModeForTransactionV1
| TestScenario::TooLowGasPriceToleranceForTransactionV1
| TestScenario::TooLowGasPriceToleranceForDeploy
| TestScenario::InvalidFields => Source::Client,
| TestScenario::InvalidFields
| TestScenario::InvalidArgumentsKind => Source::Client,
}
}

Expand Down Expand Up @@ -623,6 +626,25 @@ impl TestScenario {
.unwrap();
Transaction::from(txn)
}
TestScenario::InvalidArgumentsKind => {
let timestamp = Timestamp::now()
+ Config::default().timestamp_leeway
+ TimeDiff::from_millis(100);
let ttl = TimeDiff::from_seconds(300);
let txn = TransactionV1Builder::new_session(
false,
Bytes::from(vec![1]),
TransactionRuntimeParams::VmCasperV1,
)
.with_transaction_args(TransactionArgs::Bytesrepr(Bytes::from(vec![1, 2, 3])))
.with_chain_name("casper-example")
.with_timestamp(timestamp)
.with_ttl(ttl)
.with_secret_key(&secret_key)
.build()
.unwrap();
Transaction::from(txn)
}
}
}

Expand Down Expand Up @@ -681,6 +703,7 @@ impl TestScenario {
TestScenario::TooLowGasPriceToleranceForDeploy => false,
TestScenario::InvalidFields => false,
TestScenario::InvalidFieldsFromPeer => false,
TestScenario::InvalidArgumentsKind => false,
}
}

Expand Down Expand Up @@ -1240,7 +1263,8 @@ async fn run_transaction_acceptor_without_timeout(
| TestScenario::FromClientExpired(_)
| TestScenario::TooLowGasPriceToleranceForTransactionV1
| TestScenario::TooLowGasPriceToleranceForDeploy
| TestScenario::InvalidFields => {
| TestScenario::InvalidFields
| TestScenario::InvalidArgumentsKind => {
matches!(
event,
Event::TransactionAcceptorAnnouncement(
Expand Down Expand Up @@ -2511,3 +2535,14 @@ async fn should_reject_transaction_from_peer_with_unexpected_fields() {
)))
))
}

#[tokio::test]
async fn should_reject_transaction_with_invalid_transaction_args() {
let result = run_transaction_acceptor(TestScenario::InvalidArgumentsKind).await;
assert!(matches!(
result,
Err(super::Error::InvalidTransaction(InvalidTransaction::V1(
InvalidTransactionV1::ExpectedNamedArguments
)))
));
}
27 changes: 27 additions & 0 deletions node/src/types/transaction/meta_transaction/meta_transaction_v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,12 @@ impl MetaTransactionV1 {
expected: expected_runtime,
});
}

if !self.args.is_named() {
// VmCasperV1 runtime expected named arguments and does not support bytes
// variant.
return Err(InvalidTransactionV1::ExpectedNamedArguments);
}
}
Some(expected_runtime @ ContractRuntimeTag::VmCasperV2) => {
if !transaction_config.runtime_config.vm_casper_v2 {
Expand All @@ -331,6 +337,27 @@ impl MetaTransactionV1 {
expected: expected_runtime,
});
}

if !self.args.is_bytesrepr() {
// VmCasperV2 runtime expected bytes arguments and does not support named
// variant.
return Err(InvalidTransactionV1::ExpectedBytesArguments);
}

match self.pricing_mode {
PricingMode::PaymentLimited {
standard_payment, ..
} => {
if !standard_payment {
// V2 runtime expects standard payment in the payment limited mode.
return Err(InvalidTransactionV1::InvalidPricingMode {
price_mode: self.pricing_mode.clone(),
});
}
}
PricingMode::Fixed { .. } => {}
PricingMode::Prepaid { .. } => {}
}
}
None => {
// Native transactions are config compliant by default
Expand Down
9 changes: 9 additions & 0 deletions node/src/types/transaction/transaction_v1_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,15 @@ impl<'a> TransactionV1Builder<'a> {
self
}

/// Sets the transaction args in the transaction.
///
/// NOTE: this overwrites any existing transaction_args args.
#[cfg(test)]
pub fn with_transaction_args(mut self, args: TransactionArgs) -> Self {
self.args = args;
self
}

/// Returns the new transaction, or an error if non-defaulted fields were not set.
///
/// For more info, see [the `TransactionBuilder` documentation](TransactionV1Builder).
Expand Down
8 changes: 8 additions & 0 deletions types/src/transaction/transaction_v1/errors_v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,8 @@ pub enum InvalidTransaction {
UnexpectedTransactionFieldEntries,
/// The transaction requires named arguments.
ExpectedNamedArguments,
/// The transaction required bytes arguments.
ExpectedBytesArguments,
/// The transaction runtime is invalid.
InvalidTransactionRuntime {
/// The expected runtime as specified by the chainspec.
Expand Down Expand Up @@ -380,6 +382,9 @@ impl Display for InvalidTransaction {
InvalidTransaction::ExpectedNamedArguments => {
write!(formatter, "transaction requires named arguments")
}
InvalidTransaction::ExpectedBytesArguments => {
write!(formatter, "transaction requires bytes arguments")
}
InvalidTransaction::InvalidTransactionRuntime { expected } => {
write!(
formatter,
Expand All @@ -389,6 +394,8 @@ impl Display for InvalidTransaction {
InvalidTransaction::MissingSeed => {
write!(formatter, "missing seed for install or upgrade")
}


}
}
}
Expand Down Expand Up @@ -438,6 +445,7 @@ impl StdError for InvalidTransaction {
FieldDeserializationError::FromBytesError { error, .. } => Some(error),
},
InvalidTransaction::ExpectedNamedArguments
| InvalidTransaction::ExpectedBytesArguments
| InvalidTransaction::InvalidTransactionRuntime { .. }
| InvalidTransaction::MissingSeed => None,
}
Expand Down
16 changes: 16 additions & 0 deletions types/src/transaction/transaction_v1/transaction_args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,22 @@ impl TransactionArgs {
}
}
}

/// Returns `true` if the transaction args is [`Named`].
///
/// [`Named`]: TransactionArgs::Named
#[must_use]
pub fn is_named(&self) -> bool {
matches!(self, Self::Named(..))
}

/// Returns `true` if the transaction args is [`Bytesrepr`].
///
/// [`Bytesrepr`]: TransactionArgs::Bytesrepr
#[must_use]
pub fn is_bytesrepr(&self) -> bool {
matches!(self, Self::Bytesrepr(..))
}
}

impl FromBytes for TransactionArgs {
Expand Down

0 comments on commit 54bfafb

Please sign in to comment.