Skip to content

Commit

Permalink
Merge #5059
Browse files Browse the repository at this point in the history
5059: Amend config compliance to check limit of largest wasm lane r=darthsiroftardis a=darthsiroftardis

CHANGELOG:

- Fixed the config compliance check for Deploys to ensure the payment amount does not exceed the limit of the largest WASM lane


Co-authored-by: Karan Dhareshwar <karan@casperlabs.io>
  • Loading branch information
2 parents 52af64a + 4ecdfdc commit cd6a880
Show file tree
Hide file tree
Showing 4 changed files with 107 additions and 5 deletions.
22 changes: 20 additions & 2 deletions node/src/components/transaction_acceptor/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ enum TestScenario {
InvalidFieldsFromPeer,
InvalidArgumentsKind,
WasmTransactionWithTooBigPayment,
WasmDeployWithTooBigPayment,
}

impl TestScenario {
Expand Down Expand Up @@ -265,7 +266,8 @@ impl TestScenario {
| TestScenario::TooLowGasPriceToleranceForDeploy
| TestScenario::InvalidFields
| TestScenario::InvalidArgumentsKind
| TestScenario::WasmTransactionWithTooBigPayment => Source::Client,
| TestScenario::WasmTransactionWithTooBigPayment
| TestScenario::WasmDeployWithTooBigPayment => Source::Client,
}
}

Expand Down Expand Up @@ -671,6 +673,9 @@ impl TestScenario {
.unwrap();
Transaction::from(txn)
}
TestScenario::WasmDeployWithTooBigPayment => {
Transaction::from(Deploy::random_with_oversized_payment_amount(rng))
}
}
}

Expand Down Expand Up @@ -731,6 +736,7 @@ impl TestScenario {
TestScenario::InvalidFieldsFromPeer => false,
TestScenario::InvalidArgumentsKind => false,
TestScenario::WasmTransactionWithTooBigPayment => false,
TestScenario::WasmDeployWithTooBigPayment => false,
}
}

Expand Down Expand Up @@ -1294,7 +1300,8 @@ async fn run_transaction_acceptor_without_timeout(
| TestScenario::TooLowGasPriceToleranceForDeploy
| TestScenario::InvalidFields
| TestScenario::InvalidArgumentsKind
| TestScenario::WasmTransactionWithTooBigPayment => {
| TestScenario::WasmTransactionWithTooBigPayment
| TestScenario::WasmDeployWithTooBigPayment => {
matches!(
event,
Event::TransactionAcceptorAnnouncement(
Expand Down Expand Up @@ -2587,3 +2594,14 @@ async fn should_reject_wasm_transaction_with_limited_too_big_payment() {
)))
));
}

#[tokio::test]
async fn should_reject_deploy_with_payment_amount_larger_than_max_wasm_lane_limit() {
let result = run_transaction_acceptor(TestScenario::WasmDeployWithTooBigPayment).await;
assert!(matches!(
result,
Err(super::Error::InvalidTransaction(
InvalidTransaction::Deploy(InvalidDeploy::ExceededWasmLaneGasLimit { .. })
))
))
}
16 changes: 16 additions & 0 deletions types/src/chainspec/transaction_config/transaction_v1_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,22 @@ impl TransactionV1Config {
maybe_adequate_lane_index.map(|index| buckets[index].id)
}

pub fn get_max_serialized_length_for_wasm(&self) -> u64 {
self.get_wasm_lanes_ordered_by_transaction_size()
.iter()
.map(|lane_def| lane_def.max_transaction_length)
.max()
.unwrap_or(0u64)
}

pub fn get_max_payment_limit_for_wasm(&self) -> u64 {
self.get_wasm_lanes_ordered_by_gas_limit()
.iter()
.map(|lane_def| lane_def.max_transaction_gas_limit)
.max()
.unwrap_or(0u64)
}

pub fn get_wasm_lane_id_by_payment_limited(
&self,
gas_limit: u64,
Expand Down
51 changes: 50 additions & 1 deletion types/src/transaction/deploy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ impl Deploy {
// InstallUpgrade doesn't rely on the size of transaction
let max_transaction_size = config
.transaction_v1_config
.get_max_serialized_length(LARGE_WASM_LANE_ID);
.get_max_serialized_length_for_wasm();
self.is_valid_size(max_transaction_size as u32)?;

let header = self.header();
Expand Down Expand Up @@ -482,6 +482,22 @@ impl Deploy {
});
}

let wasm_lane_limit = config
.transaction_v1_config
.get_max_payment_limit_for_wasm();
let wasm_lane_limit_as_gas = Gas::new(wasm_lane_limit);
if gas_limit > wasm_lane_limit_as_gas {
debug!(
payment_amount = %gas_limit,
%block_gas_limit,
"transaction gas limit exceeds wasm lane limit"
);
return Err(InvalidDeploy::ExceededWasmLaneGasLimit {
wasm_lane_gas_limit: wasm_lane_limit,
got: Box::new(gas_limit.value()),
});
}

let payment_args_length = self.payment().args().serialized_length();
if payment_args_length > config.deploy_config.payment_args_max_length as usize {
debug!(
Expand Down Expand Up @@ -712,6 +728,39 @@ impl Deploy {
Self::random_transfer_with_payment(rng, payment)
}

/// Returns a random invalid `Deploy` with an invalid value for the payment amount.
#[cfg(any(all(feature = "std", feature = "testing"), test))]
pub fn random_with_oversized_payment_amount(rng: &mut TestRng) -> Self {
let payment_args = runtime_args! {
"amount" => U512::from(400_000_000_000u64)
};
let payment = ExecutableDeployItem::ModuleBytes {
module_bytes: Bytes::new(),
args: payment_args,
};

let session = ExecutableDeployItem::StoredContractByName {
name: "Test".to_string(),
entry_point: "call".to_string(),
args: Default::default(),
};

let deploy = Self::random_valid_native_transfer(rng);
let secret_key = SecretKey::random(rng);

Deploy::new_signed(
deploy.header.timestamp(),
deploy.header.ttl(),
deploy.header.gas_price(),
deploy.header.dependencies().clone(),
deploy.header.chain_name().to_string(),
payment,
session,
&secret_key,
None,
)
}

/// Returns a random `Deploy` with custom payment specified as a stored contract by name.
#[cfg(any(all(feature = "std", feature = "testing"), test))]
pub fn random_with_valid_custom_payment_contract_by_name(rng: &mut TestRng) -> Self {
Expand Down
23 changes: 21 additions & 2 deletions types/src/transaction/deploy/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,16 @@ pub enum InvalidDeploy {
/// Invalid runtime.
InvalidRuntime,

//Invalida chainspec configuration - seems that chainspec has no wasm lanes defined
/// Invalid chainspec configuration - seems that chainspec has no wasm lanes defined
ChainspecHasNoWasmLanesDefined,

/// The payment amount associated with the deploy exceeds the block gas limit.
ExceededWasmLaneGasLimit {
/// Configured block gas limit.
wasm_lane_gas_limit: u64,
/// The payment amount received.
got: Box<U512>,
},
}

impl Display for InvalidDeploy {
Expand Down Expand Up @@ -270,6 +278,16 @@ impl Display for InvalidDeploy {
write!(formatter, "invalid runtime",)
}
InvalidDeploy::ChainspecHasNoWasmLanesDefined => write!(formatter, "chainspec didnt have any wasm lanes defined which is required for wasm based deploys",),
InvalidDeploy::ExceededWasmLaneGasLimit {
wasm_lane_gas_limit,
got,
} => {
write!(
formatter,
"payment amount of {} exceeds the largest wasm lane gas limit of {}",
got, wasm_lane_gas_limit
)
}
}
}
}
Expand Down Expand Up @@ -307,7 +325,8 @@ impl StdError for InvalidDeploy {
| InvalidDeploy::UnableToCalculateGasCost
| InvalidDeploy::GasPriceToleranceTooLow { .. }
| InvalidDeploy::InvalidRuntime
| InvalidDeploy::ChainspecHasNoWasmLanesDefined => None,
| InvalidDeploy::ChainspecHasNoWasmLanesDefined
| InvalidDeploy::ExceededWasmLaneGasLimit { .. } => None,
}
}
}
Expand Down

0 comments on commit cd6a880

Please sign in to comment.