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

Use signed 256-bit integer for sstore gas refund substate #9746

Merged
merged 6 commits into from
Oct 15, 2018
Merged
Show file tree
Hide file tree
Changes from all 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
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);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i128::max_value() as u64 == u64::max_value(), and we checked above that it's always non-negative.

// 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);
cheme marked this conversation as resolved.
Show resolved Hide resolved
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