Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions prdoc/pr_10567.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
title: '[Revive] Fix construction of negative zero SignedGas'
doc:
- audience: Runtime Dev
description: |-
Closes https://github.com/paritytech-secops/srlabs_findings/issues/603

This fixes an issue where a zero `SignedGas` value can be constructed that uses the `Negative` variant. The rest of the code relies on the invariant that zero `SignedGas` has always the `Positive` variant and this is also documented in the code.
crates:
- name: pallet-revive
bump: patch
10 changes: 8 additions & 2 deletions substrate/frame/revive/src/metering/gas.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,14 @@ impl<T: Config> SignedGas<T> {

match deposit {
StorageDeposit::Charge(amount) => Positive(multiplier.saturating_mul_int(*amount)),
StorageDeposit::Refund(amount) if *amount == Default::default() => Positive(*amount),
StorageDeposit::Refund(amount) => Negative(multiplier.saturating_mul_int(*amount)),
StorageDeposit::Refund(amount) => {
let scaled_amount = multiplier.saturating_mul_int(*amount);
if scaled_amount == Default::default() {
Positive(scaled_amount)
} else {
Negative(scaled_amount)
}
},
}
}

Expand Down
18 changes: 18 additions & 0 deletions substrate/frame/revive/src/metering/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,24 @@ enum Charge {
D(i64),
}

#[test]
fn test_deposit_calculation() {
use super::SignedGas;

ExtBuilder::default()
.with_next_fee_multiplier(FixedU128::from_rational(2, 1))
.build()
.execute_with(|| {
let deposit1 = StorageDeposit::Refund(10);
let gas_result1 = SignedGas::<Test>::from_adjusted_deposit_charge(&deposit1);
assert_eq!(gas_result1, SignedGas::Negative(BalanceOf::<Test>::from(5u32)));

let deposit2 = StorageDeposit::Refund(1);
let gas_result2 = SignedGas::<Test>::from_adjusted_deposit_charge(&deposit2);
assert_eq!(gas_result2, SignedGas::Positive(BalanceOf::<Test>::from(0u32)));
});
}

#[test_case(FixtureType::Solc , "DepositPrecompile" ; "solc precompiles")]
#[test_case(FixtureType::Resolc , "DepositPrecompile" ; "resolc precompiles")]
#[test_case(FixtureType::Solc , "DepositDirect" ; "solc direct")]
Expand Down
Loading