Skip to content

Commit

Permalink
Merge branch 'main' into revert-63-rpc-publish-dry-run
Browse files Browse the repository at this point in the history
  • Loading branch information
Shaptic authored Feb 21, 2024
2 parents a88a7c9 + a3047e5 commit b53ef88
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 16 deletions.
6 changes: 3 additions & 3 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ default-members = ["cmd/crates/soroban-rpc", "cmd/crates/soroban-spec-tools"]
#exclude = ["cmd/crates/soroban-test/tests/fixtures/hello"]

[workspace.package]
version = "20.3.1"
version = "20.3.2"
rust-version = "1.74.0"

[workspace.dependencies.soroban-env-host]
Expand Down Expand Up @@ -47,7 +47,7 @@ git = "https://github.com/stellar/soroban-tools"
rev = "a59f5f421a27bab71472041fc619dd8b0d1cf902"

[workspace.dependencies.soroban-spec-tools]
version = "20.2.0"
version = "20.3.2"
path = "./cmd/crates/soroban-spec-tools"

[workspace.dependencies.soroban-sdk]
Expand All @@ -66,7 +66,7 @@ version = "=20.3.1"
# rev = "4aef54ff9295c2fca4c5b9fbd2c92d0ff99f67de"

[workspace.dependencies.soroban-rpc]
version = "20.3.1"
version = "20.3.2"
path = "cmd/crates/soroban-rpc"

[workspace.dependencies.stellar-xdr]
Expand Down
90 changes: 81 additions & 9 deletions cmd/crates/soroban-rpc/src/txn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,15 +313,25 @@ pub fn assemble(
}
}

// update the fees of the actual transaction to meet the minimum resource fees.
let classic_transaction_fees = crate::DEFAULT_TRANSACTION_FEES;
// Pad the fees up by 15% for a bit of wiggle room.
tx.fee = (tx.fee.max(
classic_transaction_fees
+ u32::try_from(simulation.min_resource_fee)
.map_err(|_| Error::LargeFee(simulation.min_resource_fee))?,
) * 115)
/ 100;
// Update transaction fees to meet the minimum resource fees.
let classic_tx_fee: u64 = crate::DEFAULT_TRANSACTION_FEES.into();

// Choose larger of existing fee or inclusion + resource fee.
let base_tx_fee: u64 = tx
.fee
.max(
u32::try_from(classic_tx_fee + simulation.min_resource_fee)
.map_err(|_| Error::LargeFee(simulation.min_resource_fee + classic_tx_fee))?,
)
.into(); // invariant: base_tx_fee <= u32::MAX

// Pad the total fee by up to 15% for a bit of wiggle room.
//
// We know for a fact because of the .min() call that we will not exceed
// U32_MAX, but we can't get clippy to shut the fuck up for this single
// line, so we have a redundant error check anyway.
tx.fee = u32::try_from((base_tx_fee * 115 / 100).min(u64::from(u32::MAX)))
.map_err(|_| Error::LargeFee(base_tx_fee))?;

tx.operations = vec![op].try_into()?;
tx.ext = TransactionExt::V1(transaction_data);
Expand Down Expand Up @@ -721,4 +731,66 @@ 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.)
//
// 2. Given a large resource fee that WILL exceed on its own with the
// inclusion fee, ensure the overflow is caught with an error rather
// than silently ignored.
//
// 3. Given a max resource fee that exceeds U32_MAX *only with* wiggle
// room, ensure the overflow is ignored and we cap on the max.
//
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 = (u64::from(u32::MAX) * 85 / 100) - 100 - 1;
assert_eq!(resource_fee, 3_650_722_099);
response.min_resource_fee = resource_fee;

match assemble(&txn, &response) {
Ok(asstxn) => {
let expected = (resource_fee + 100) * 115 / 100;
assert_eq!(asstxn.fee, u32::try_from(expected).unwrap());
}
r => panic!("expected success, got: {r:#?}"),
}

// 2: combo overflows, should throw
resource_fee = u64::from(u32::MAX);
assert_eq!(resource_fee, 4_294_967_295);
response.min_resource_fee = resource_fee;

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

// 2: combo works but wiggle room overflows, should cap
resource_fee = u64::from(u32::MAX) * 90 / 100;
assert_eq!(resource_fee, 3_865_470_565);
response.min_resource_fee = resource_fee;

match assemble(&txn, &response) {
Ok(asstxn) => {
assert_eq!(asstxn.fee, u32::MAX);
}
r => panic!("expected success, got: {r:#?}"),
}
}
}
2 changes: 1 addition & 1 deletion cmd/soroban-rpc/lib/preflight/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "preflight"
version = "20.3.1"
version = "20.3.2"
publish = false

[lib]
Expand Down

0 comments on commit b53ef88

Please sign in to comment.