Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Commit

Permalink
Fix/div by zero (#5041)
Browse files Browse the repository at this point in the history
* Handle gas_price being zero separately

* Bump spec_version

* Add a unit & integration tests for gas price = 0
  • Loading branch information
cowboy-bebug authored Feb 26, 2020
1 parent 0f4f9d7 commit 5b41575
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 4 deletions.
4 changes: 2 additions & 2 deletions bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
// and set impl_version to 0. If only runtime
// implementation changes and behavior does not, then leave spec_version as
// is and increment impl_version.
spec_version: 225,
impl_version: 1,
spec_version: 226,
impl_version: 0,
apis: RUNTIME_API_VERSIONS,
};

Expand Down
26 changes: 24 additions & 2 deletions frame/contracts/src/gas.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,11 @@ pub fn refund_unused_gas<T: Trait>(
pub fn approx_gas_for_balance<Balance>(gas_price: Balance, balance: Balance) -> Gas
where Balance: AtLeast32Bit
{
(balance / gas_price).saturated_into::<Gas>()
if gas_price.is_zero() {
Zero::zero()
} else {
(balance / gas_price).saturated_into::<Gas>()
}
}

/// A simple utility macro that helps to match against a
Expand Down Expand Up @@ -294,7 +298,7 @@ macro_rules! match_tokens {
#[cfg(test)]
mod tests {
use super::{GasMeter, Token};
use crate::tests::Test;
use crate::{tests::Test, gas::approx_gas_for_balance};

/// A trivial token that charges the specified number of gas units.
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
Expand Down Expand Up @@ -382,4 +386,22 @@ mod tests {
let mut gas_meter = GasMeter::<Test>::with_limit(25, 10);
assert!(!gas_meter.charge(&(), SimpleToken(25)).is_out_of_gas());
}

// A unit test for `fn approx_gas_for_balance()`, and makes
// sure setting gas_price 0 does not cause `div by zero` error.
#[test]
fn approx_gas_for_balance_works() {
let tests = vec![
(approx_gas_for_balance(0_u64, 123), 0),
(approx_gas_for_balance(0_u64, 456), 0),
(approx_gas_for_balance(1_u64, 123), 123),
(approx_gas_for_balance(1_u64, 456), 456),
(approx_gas_for_balance(100_u64, 900), 9),
(approx_gas_for_balance(123_u64, 900), 7),
];

for (lhs, rhs) in tests {
assert_eq!(lhs, rhs);
}
}
}
16 changes: 16 additions & 0 deletions frame/contracts/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2079,6 +2079,22 @@ fn deploy_and_call_other_contract() {
});
}

#[test]
fn deploy_works_without_gas_price() {
let (wasm, code_hash) = compile_module::<Test>(CODE_GET_RUNTIME_STORAGE).unwrap();
ExtBuilder::default().existential_deposit(50).gas_price(0).build().execute_with(|| {
Balances::deposit_creating(&ALICE, 1_000_000);
assert_ok!(Contracts::put_code(Origin::signed(ALICE), 100_000, wasm));
assert_ok!(Contracts::instantiate(
Origin::signed(ALICE),
100,
100_000,
code_hash.into(),
vec![],
));
});
}

const CODE_SELF_DESTRUCT: &str = r#"
(module
(import "env" "ext_scratch_size" (func $ext_scratch_size (result i32)))
Expand Down

0 comments on commit 5b41575

Please sign in to comment.