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

Commit

Permalink
Use signed 256-bit integer for sstore gas refund substate (#9746)
Browse files Browse the repository at this point in the history
* Add signed refund

* Use signed 256-bit integer for sstore gas refund substate

* Fix tests

* Remove signed mod and use i128 directly

* Fix evm test case casting

* Fix jsontests ext signature
  • Loading branch information
sorpaas authored Oct 15, 2018
1 parent 702311b commit 5319d33
Show file tree
Hide file tree
Showing 10 changed files with 26 additions and 25 deletions.
6 changes: 3 additions & 3 deletions ethcore/evm/src/interpreter/gasometer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ fn calculate_eip1283_sstore_gas<Gas: evm::CostType>(schedule: &Schedule, origina
}

pub fn handle_eip1283_sstore_clears_refund(ext: &mut vm::Ext, original: &U256, current: &U256, new: &U256) {
let sstore_clears_schedule = U256::from(ext.schedule().sstore_refund_gas);
let sstore_clears_schedule = ext.schedule().sstore_refund_gas;

if current == new {
// 1. If current value equals new value (this is a no-op), 200 gas is deducted.
Expand Down Expand Up @@ -438,11 +438,11 @@ pub fn handle_eip1283_sstore_clears_refund(ext: &mut vm::Ext, original: &U256, c
// 2.2.2. If original value equals new value (this storage slot is reset)
if original.is_zero() {
// 2.2.2.1. If original value is 0, add 19800 gas to refund counter.
let refund = U256::from(ext.schedule().sstore_set_gas - ext.schedule().sload_gas);
let refund = ext.schedule().sstore_set_gas - ext.schedule().sload_gas;
ext.add_sstore_refund(refund);
} else {
// 2.2.2.2. Otherwise, add 4800 gas to refund counter.
let refund = U256::from(ext.schedule().sstore_reset_gas - ext.schedule().sload_gas);
let refund = ext.schedule().sstore_reset_gas - ext.schedule().sload_gas;
ext.add_sstore_refund(refund);
}
}
Expand Down
2 changes: 1 addition & 1 deletion ethcore/evm/src/interpreter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -720,7 +720,7 @@ impl<Cost: CostType> Interpreter<Cost> {
gasometer::handle_eip1283_sstore_clears_refund(ext, &original_val, &current_val, &val);
} else {
if !current_val.is_zero() && val.is_zero() {
let sstore_clears_schedule = U256::from(ext.schedule().sstore_refund_gas);
let sstore_clears_schedule = ext.schedule().sstore_refund_gas;
ext.add_sstore_refund(sstore_clears_schedule);
}
}
Expand Down
2 changes: 1 addition & 1 deletion ethcore/evm/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -716,7 +716,7 @@ fn test_jumps(factory: super::Factory) {
test_finalize(vm.exec(&mut ext).ok().unwrap()).unwrap()
};

assert_eq!(ext.sstore_clears, U256::from(ext.schedule().sstore_refund_gas));
assert_eq!(ext.sstore_clears, ext.schedule().sstore_refund_gas as i128);
assert_store(&ext, 0, "0000000000000000000000000000000000000000000000000000000000000000"); // 5!
assert_store(&ext, 1, "0000000000000000000000000000000000000000000000000000000000000078"); // 5!
assert_eq!(gas_left, U256::from(54_117));
Expand Down
7 changes: 4 additions & 3 deletions ethcore/src/executive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1091,7 +1091,8 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> {
let schedule = self.schedule;

// refunds from SSTORE nonzero -> zero
let sstore_refunds = substate.sstore_clears_refund;
assert!(substate.sstore_clears_refund >= 0, "On transaction level, sstore clears refund cannot go below zero.");
let sstore_refunds = U256::from(substate.sstore_clears_refund as u64);
// refunds from contract suicides
let suicide_refunds = U256::from(schedule.suicide_refund_gas) * U256::from(substate.suicides.len());
let refunds_bound = sstore_refunds + suicide_refunds;
Expand Down Expand Up @@ -2095,7 +2096,7 @@ mod tests {
let gas_used = gas - gas_left;
// sstore: 0 -> (1) -> () -> (1 -> 0 -> 1)
assert_eq!(gas_used, U256::from(41860));
assert_eq!(refund, U256::from(19800));
assert_eq!(refund, 19800);

assert_eq!(state.storage_at(&operating_address, &k).unwrap(), H256::from(U256::from(1)));
// Test a call via top-level -> y2 -> x2
Expand All @@ -2113,7 +2114,7 @@ mod tests {
let gas_used = gas - gas_left;
// sstore: 1 -> (1) -> () -> (0 -> 1 -> 0)
assert_eq!(gas_used, U256::from(11860));
assert_eq!(refund, U256::from(19800));
assert_eq!(refund, 19800);
}

fn wasm_sample_code() -> Arc<Vec<u8>> {
Expand Down
8 changes: 4 additions & 4 deletions ethcore/src/externalities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -394,12 +394,12 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for Externalities<'a, T, V, B>
self.depth
}

fn add_sstore_refund(&mut self, value: U256) {
self.substate.sstore_clears_refund = self.substate.sstore_clears_refund.saturating_add(value);
fn add_sstore_refund(&mut self, value: usize) {
self.substate.sstore_clears_refund += value as i128;
}

fn sub_sstore_refund(&mut self, value: U256) {
self.substate.sstore_clears_refund = self.substate.sstore_clears_refund.saturating_sub(value);
fn sub_sstore_refund(&mut self, value: usize) {
self.substate.sstore_clears_refund -= value as i128;
}

fn trace_next_instruction(&mut self, pc: usize, instruction: u8, current_gas: U256) -> bool {
Expand Down
4 changes: 2 additions & 2 deletions ethcore/src/json_tests/executive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,11 +218,11 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for TestExt<'a, T, V, B>
false
}

fn add_sstore_refund(&mut self, value: U256) {
fn add_sstore_refund(&mut self, value: usize) {
self.ext.add_sstore_refund(value)
}

fn sub_sstore_refund(&mut self, value: U256) {
fn sub_sstore_refund(&mut self, value: usize) {
self.ext.sub_sstore_refund(value)
}
}
Expand Down
6 changes: 3 additions & 3 deletions ethcore/src/state/substate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

//! Execution environment substate.
use std::collections::HashSet;
use ethereum_types::{U256, Address};
use ethereum_types::Address;
use log_entry::LogEntry;
use evm::{Schedule, CleanDustMode};
use super::CleanupMode;
Expand All @@ -35,7 +35,7 @@ pub struct Substate {
pub logs: Vec<LogEntry>,

/// Refund counter of SSTORE.
pub sstore_clears_refund: U256,
pub sstore_clears_refund: i128,

/// Created contracts.
pub contracts_created: Vec<Address>,
Expand All @@ -52,7 +52,7 @@ impl Substate {
self.suicides.extend(s.suicides);
self.touched.extend(s.touched);
self.logs.extend(s.logs);
self.sstore_clears_refund = self.sstore_clears_refund + s.sstore_clears_refund;
self.sstore_clears_refund += s.sstore_clears_refund;
self.contracts_created.extend(s.contracts_created);
}

Expand Down
4 changes: 2 additions & 2 deletions ethcore/vm/src/ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,10 @@ pub trait Ext {
fn depth(&self) -> usize;

/// Increments sstore refunds counter.
fn add_sstore_refund(&mut self, value: U256);
fn add_sstore_refund(&mut self, value: usize);

/// Decrements sstore refunds counter.
fn sub_sstore_refund(&mut self, value: U256);
fn sub_sstore_refund(&mut self, value: usize);

/// Decide if any more operations should be traced. Passthrough for the VM trace.
fn trace_next_instruction(&mut self, _pc: usize, _instruction: u8, _current_gas: U256) -> bool { false }
Expand Down
10 changes: 5 additions & 5 deletions ethcore/vm/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ pub struct FakeExt {
pub store: HashMap<H256, H256>,
pub suicides: HashSet<Address>,
pub calls: HashSet<FakeCall>,
pub sstore_clears: U256,
pub sstore_clears: i128,
pub depth: usize,
pub blockhashes: HashMap<U256, H256>,
pub codes: HashMap<Address, Arc<Bytes>>,
Expand Down Expand Up @@ -231,12 +231,12 @@ impl Ext for FakeExt {
self.is_static
}

fn add_sstore_refund(&mut self, value: U256) {
self.sstore_clears = self.sstore_clears + value;
fn add_sstore_refund(&mut self, value: usize) {
self.sstore_clears += value as i128;
}

fn sub_sstore_refund(&mut self, value: U256) {
self.sstore_clears = self.sstore_clears - value;
fn sub_sstore_refund(&mut self, value: usize) {
self.sstore_clears -= value as i128;
}

fn trace_next_instruction(&mut self, _pc: usize, _instruction: u8, _gas: U256) -> bool {
Expand Down
2 changes: 1 addition & 1 deletion ethcore/wasm/src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ impl<'a> Runtime<'a> {
self.ext.set_storage(key, val).map_err(|_| Error::StorageUpdateError)?;

if former_val != H256::zero() && val == H256::zero() {
let sstore_clears_schedule = U256::from(self.schedule().sstore_refund_gas);
let sstore_clears_schedule = self.schedule().sstore_refund_gas;
self.ext.add_sstore_refund(sstore_clears_schedule);
}

Expand Down

0 comments on commit 5319d33

Please sign in to comment.