Skip to content

Commit

Permalink
Add tests for various overflow cases
Browse files Browse the repository at this point in the history
  • Loading branch information
Shaptic committed Feb 21, 2024
1 parent 783dc98 commit f960d0d
Showing 1 changed file with 90 additions and 20 deletions.
110 changes: 90 additions & 20 deletions cmd/crates/soroban-rpc/src/txn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,8 +323,8 @@ pub fn assemble(
+ u32::try_from(simulation.min_resource_fee)
.map_err(|_| Error::LargeFee(simulation.min_resource_fee))?,
),
) / 100
* 115;
) * 115
/ 100;
tx.fee = u32::try_from(padded_fee).map_err(|_| Error::LargeFee(padded_fee))?;

tx.operations = vec![op].try_into()?;
Expand Down Expand Up @@ -612,6 +612,27 @@ mod tests {
}
}

fn non_soroban_transaction() -> Transaction {
let source_bytes = Ed25519PublicKey::from_string(SOURCE).unwrap().0;
Transaction {
source_account: MuxedAccount::Ed25519(Uint256(source_bytes)),
fee: 100,
seq_num: SequenceNumber(0),
cond: Preconditions::None,
memo: Memo::None,
operations: vec![Operation {
source_account: None,
body: OperationBody::ChangeTrust(ChangeTrustOp {
line: ChangeTrustAsset::Native,
limit: 0,
}),
}]
.try_into()
.unwrap(),
ext: TransactionExt::V0,
}
}

#[test]
fn test_assemble_transaction_updates_tx_data_from_simulation_response() {
let sim = simulation_response();
Expand Down Expand Up @@ -669,24 +690,7 @@ mod tests {

#[test]
fn test_assemble_transaction_errors_for_non_invokehostfn_ops() {
let source_bytes = Ed25519PublicKey::from_string(SOURCE).unwrap().0;
let txn = Transaction {
source_account: MuxedAccount::Ed25519(Uint256(source_bytes)),
fee: 100,
seq_num: SequenceNumber(0),
cond: Preconditions::None,
memo: Memo::None,
operations: vec![Operation {
source_account: None,
body: OperationBody::ChangeTrust(ChangeTrustOp {
line: ChangeTrustAsset::Native,
limit: 0,
}),
}]
.try_into()
.unwrap(),
ext: TransactionExt::V0,
};
let txn = non_soroban_transaction();

let result = assemble(
&txn,
Expand Down Expand Up @@ -725,4 +729,70 @@ mod tests {
r => panic!("expected UnexpectedSimulateTransactionResultSize error, got: {r:#?}"),
}
}

#[test]
fn test_assemble_transaction_overflow_behavior() {
// Test three separate cases:
//
// 1. Given a near-max (U32_MAX) resource fee, ensure the "wiggle room"
// doesn't cause an overflow due to correct math order.
// (Specifically, do U32_MAX - 15% - 100 - 1, so the final fee is
// U32_MAX-1.)
//
// 3. Given a near-max (U32_MAX) resource fee that will ONLY exceed
// U32_MAX *with* the wiggle room, ensure the overflow is caught
// with an error rather than silently ignored.
//
// 2. Given a large resource fee that WILL exceed on its own, ensure
// the overflow is caught with an error rather than silently
// ignored.
//
let txn = single_contract_fn_transaction();
let mut response = simulation_response();

// sanity check so these can be adjusted if the above helper changes
assert_eq!(txn.fee, 100, "modified txn.fee: update the math below");

// 1: wiggle room math overflows but result fits
let mut resource_fee = ((u32::MAX as f64) * 0.85).floor() as u64 - 100 - 1;
assert_eq!(resource_fee, 3650722099);
response.min_resource_fee = resource_fee;

match assemble(&txn, &response) {
Ok(_) => {}
r => {
panic!("expected success, got: {r:#?}")
}
}

// 2: combo works but wiggle room overflows
resource_fee = ((u32::MAX as f64) * 0.90).floor() as u64;
assert_eq!(resource_fee, 3865470565);
response.min_resource_fee = resource_fee;

match assemble(&txn, &response) {
Err(Error::LargeFee(fee)) => {
let expected = (((resource_fee + 100) as f64) * 1.15).floor() as u64;
assert_eq!(expected, fee, "expected {} != {} actual", expected, fee);
}
r => {
panic!("expected LargeFee error, got: {r:#?}")
}
}

// 2: combo overflows
resource_fee = (u32::MAX - 100) as u64;
assert_eq!(resource_fee, 4294967195);
response.min_resource_fee = resource_fee;

match assemble(&txn, &response) {
Err(Error::LargeFee(fee)) => {
let expected = (((resource_fee + 100) as f64) * 1.15).floor() as u64;
assert_eq!(expected, fee, "expected {} != {} actual", expected, fee);
}
r => {
panic!("expected LargeFee error, got: {r:#?}")
}
}
}
}

0 comments on commit f960d0d

Please sign in to comment.