EIP 1283: Net gas metering for SSTORE without dirty maps (#9319)

* Implement last_checkpoint_storage_at

* Add reverted_storage_at for externalities

* sstore_clears_count -> sstore_clears_refund

* Implement eip1283 for evm

* Add eip1283Transition params

* evm: fix tests

* jsontests: fix test

* Return checkpoint index when creating

* Comply with spec Version II

* Fix docs

* Fix jsontests feature compile

* Address grumbles

* Fix no-checkpoint-entry case

* Remove unnecessary expect

* Add test for State::checkpoint_storage_at

* Add executive level test for eip1283

* Hard-code transaction_checkpoint_index to 0

* Fix jsontests

* Add tests for checkpoint discard/revert

* Require checkpoint to be empty for kill_account and commit

* Get code coverage

* Use saturating_add/saturating_sub

* Fix issues in insert_cache

* Clear the state again

* Fix original_storage_at

* Early return for empty RLP trie storage

* Update comments

* Fix borrow_mut issue

* Simplify checkpoint_storage_at if branches

* Better commenting for gas handling code

* Address naming grumbles

* More tests

* Fix an issue in overwrite_with

* Add another test

* Fix comment

* Remove unnecessary bracket

* Move orig to inner if

* Remove test coverage for this PR

* Add tests for executive original value

* Add warn! for an unreachable cause

Author: sorpaas

ethcore/evm/src/interpreter/gasometer.rs

@@ -125,12 +125,17 @@ impl<Gas: evm::CostType> Gasometer<Gas> {
 				let newval = stack.peek(1);
 				let val = U256::from(&*ext.storage_at(&address)?);
 
-				let gas = if val.is_zero() && !newval.is_zero() {
-					schedule.sstore_set_gas
+				let gas = if schedule.eip1283 {
+					let orig = U256::from(&*ext.initial_storage_at(&address)?);
+					calculate_eip1283_sstore_gas(schedule, &orig, &val, &newval)
 				} else {
-					// Refund for below case is added when actually executing sstore
-					// !is_zero(&val) && is_zero(newval)
-					schedule.sstore_reset_gas
+					if val.is_zero() && !newval.is_zero() {
+						schedule.sstore_set_gas
+					} else {
+						// Refund for below case is added when actually executing sstore
+						// !is_zero(&val) && is_zero(newval)
+						schedule.sstore_reset_gas
+					}
 				};
 				Request::Gas(Gas::from(gas))
 			},
@@ -342,6 +347,89 @@ fn add_gas_usize<Gas: evm::CostType>(value: Gas, num: usize) -> (Gas, bool) {
 	value.overflow_add(Gas::from(num))
 }
 
+#[inline]
+fn calculate_eip1283_sstore_gas<Gas: evm::CostType>(schedule: &Schedule, original: &U256, current: &U256, new: &U256) -> Gas {
+	Gas::from(
+		if current == new {
+			// 1. If current value equals new value (this is a no-op), 200 gas is deducted.
+			schedule.sload_gas
+		} else {
+			// 2. If current value does not equal new value
+			if original == current {
+				// 2.1. If original value equals current value (this storage slot has not been changed by the current execution context)
+				if original.is_zero() {
+					// 2.1.1. If original value is 0, 20000 gas is deducted.
+					schedule.sstore_set_gas
+				} else {
+					// 2.1.2. Otherwise, 5000 gas is deducted.
+					schedule.sstore_reset_gas
+
+					// 2.1.2.1. If new value is 0, add 15000 gas to refund counter.
+				}
+			} else {
+				// 2.2. If original value does not equal current value (this storage slot is dirty), 200 gas is deducted. Apply both of the following clauses.
+				schedule.sload_gas
+
+				// 2.2.1. If original value is not 0
+				// 2.2.1.1. If current value is 0 (also means that new value is not 0), remove 15000 gas from refund counter. We can prove that refund counter will never go below 0.
+				// 2.2.1.2. If new value is 0 (also means that current value is not 0), add 15000 gas to refund counter.
+
+				// 2.2.2. If original value equals new value (this storage slot is reset)
+				// 2.2.2.1. If original value is 0, add 19800 gas to refund counter.
+				// 2.2.2.2. Otherwise, add 4800 gas to refund counter.
+			}
+		}
+	)
+}
+
+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);
+
+	if current == new {
+		// 1. If current value equals new value (this is a no-op), 200 gas is deducted.
+	} else {
+		// 2. If current value does not equal new value
+		if original == current {
+			// 2.1. If original value equals current value (this storage slot has not been changed by the current execution context)
+			if original.is_zero() {
+				// 2.1.1. If original value is 0, 20000 gas is deducted.
+			} else {
+				// 2.1.2. Otherwise, 5000 gas is deducted.
+				if new.is_zero() {
+					// 2.1.2.1. If new value is 0, add 15000 gas to refund counter.
+					ext.add_sstore_refund(sstore_clears_schedule);
+				}
+			}
+		} else {
+			// 2.2. If original value does not equal current value (this storage slot is dirty), 200 gas is deducted. Apply both of the following clauses.
+
+			if !original.is_zero() {
+				// 2.2.1. If original value is not 0
+				if current.is_zero() {
+					// 2.2.1.1. If current value is 0 (also means that new value is not 0), remove 15000 gas from refund counter. We can prove that refund counter will never go below 0.
+					ext.sub_sstore_refund(sstore_clears_schedule);
+				} else if new.is_zero() {
+					// 2.2.1.2. If new value is 0 (also means that current value is not 0), add 15000 gas to refund counter.
+					ext.add_sstore_refund(sstore_clears_schedule);
+				}
+			}
+
+			if original == new {
+				// 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);
+					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);
+					ext.add_sstore_refund(refund);
+				}
+			}
+		}
+	}
+}
+
 #[test]
 fn test_mem_gas_cost() {
 	// given

ethcore/evm/src/interpreter/mod.rs

@@ -515,8 +515,14 @@ impl<Cost: CostType> Interpreter<Cost> {
 
 				let current_val = U256::from(&*ext.storage_at(&address)?);
 				// Increase refund for clear
-				if !self.is_zero(&current_val) && self.is_zero(&val) {
-					ext.inc_sstore_clears();
+				if ext.schedule().eip1283 {
+					let original_val = U256::from(&*ext.initial_storage_at(&address)?);
+					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);
+						ext.add_sstore_refund(sstore_clears_schedule);
+					}
 				}
 				ext.set_storage(address, H256::from(&val))?;
 			},

ethcore/evm/src/tests.rs

@@ -716,7 +716,7 @@ fn test_jumps(factory: super::Factory) {
 		test_finalize(vm.exec(params, &mut ext)).unwrap()
 	};
 
-	assert_eq!(ext.sstore_clears, 1);
+	assert_eq!(ext.sstore_clears, U256::from(ext.schedule.sstore_refund_gas));
 	assert_store(&ext, 0, "0000000000000000000000000000000000000000000000000000000000000000"); // 5!
 	assert_store(&ext, 1, "0000000000000000000000000000000000000000000000000000000000000078"); // 5!
 	assert_eq!(gas_left, U256::from(54_117));

ethcore/res/ethereum/constantinople_test.json

@@ -33,7 +33,8 @@
 		"eip211Transition": "0x0",
 		"eip214Transition": "0x0",
 		"eip155Transition": "0x0",
-		"eip658Transition": "0x0"
+		"eip658Transition": "0x0",
+		"eip1283Transition": "0x0"
 	},
 	"genesis": {
 		"seal": {

ethcore/src/executive.rs

@@ -560,9 +560,9 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> {
 		let prev_bal = self.state.balance(&params.address)?;
 		if let ActionValue::Transfer(val) = params.value {
 			self.state.sub_balance(&params.sender, &val, &mut substate.to_cleanup_mode(&schedule))?;
-			self.state.new_contract(&params.address, val + prev_bal, nonce_offset);
+			self.state.new_contract(&params.address, val + prev_bal, nonce_offset)?;
 		} else {
-			self.state.new_contract(&params.address, prev_bal, nonce_offset);
+			self.state.new_contract(&params.address, prev_bal, nonce_offset)?;
 		}
 
 		let trace_info = tracer.prepare_trace_create(&params);
@@ -613,7 +613,7 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> {
 		let schedule = self.machine.schedule(self.info.number);
 
 		// refunds from SSTORE nonzero -> zero
-		let sstore_refunds = U256::from(schedule.sstore_refund_gas) * substate.sstore_clears_count;
+		let sstore_refunds = substate.sstore_clears_refund;
 		// 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;
@@ -1587,6 +1587,65 @@ mod tests {
 		assert_eq!(state.storage_at(&contract_address, &H256::from(&U256::zero())).unwrap(), H256::from(&U256::from(0)));
 	}
 
+	evm_test!{test_eip1283: test_eip1283_int}
+	fn test_eip1283(factory: Factory) {
+		let x1 = Address::from(0x1000);
+		let x2 = Address::from(0x1001);
+		let y1 = Address::from(0x2001);
+		let y2 = Address::from(0x2002);
+		let operating_address = Address::from(0);
+		let k = H256::new();
+
+		let mut state = get_temp_state_with_factory(factory.clone());
+		state.new_contract(&x1, U256::zero(), U256::from(1)).unwrap();
+		state.init_code(&x1, "600160005560006000556001600055".from_hex().unwrap()).unwrap();
+		state.new_contract(&x2, U256::zero(), U256::from(1)).unwrap();
+		state.init_code(&x2, "600060005560016000556000600055".from_hex().unwrap()).unwrap();
+		state.new_contract(&y1, U256::zero(), U256::from(1)).unwrap();
+		state.init_code(&y1, "600060006000600061100062fffffff4".from_hex().unwrap()).unwrap();
+		state.new_contract(&y2, U256::zero(), U256::from(1)).unwrap();
+		state.init_code(&y2, "600060006000600061100162fffffff4".from_hex().unwrap()).unwrap();
+
+		let info = EnvInfo::default();
+		let machine = ::ethereum::new_constantinople_test_machine();
+
+		assert_eq!(state.storage_at(&operating_address, &k).unwrap(), H256::from(U256::from(0)));
+		// Test a call via top-level -> y1 -> x1
+		let (FinalizationResult { gas_left, .. }, refund, gas) = {
+			let gas = U256::from(0xffffffffffu64);
+			let mut params = ActionParams::default();
+			params.code = Some(Arc::new("6001600055600060006000600061200163fffffffff4".from_hex().unwrap()));
+			params.gas = gas;
+			let mut substate = Substate::new();
+			let mut ex = Executive::new(&mut state, &info, &machine);
+			let res = ex.call(params, &mut substate, BytesRef::Fixed(&mut []), &mut NoopTracer, &mut NoopVMTracer).unwrap();
+
+			(res, substate.sstore_clears_refund, gas)
+		};
+		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!(state.storage_at(&operating_address, &k).unwrap(), H256::from(U256::from(1)));
+		// Test a call via top-level -> y2 -> x2
+		let (FinalizationResult { gas_left, .. }, refund, gas) = {
+			let gas = U256::from(0xffffffffffu64);
+			let mut params = ActionParams::default();
+			params.code = Some(Arc::new("6001600055600060006000600061200263fffffffff4".from_hex().unwrap()));
+			params.gas = gas;
+			let mut substate = Substate::new();
+			let mut ex = Executive::new(&mut state, &info, &machine);
+			let res = ex.call(params, &mut substate, BytesRef::Fixed(&mut []), &mut NoopTracer, &mut NoopVMTracer).unwrap();
+
+			(res, substate.sstore_clears_refund, gas)
+		};
+		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));
+	}
+
 	fn wasm_sample_code() -> Arc<Vec<u8>> {
 		Arc::new(
 			"0061736d01000000010d0360027f7f0060017f0060000002270303656e7603726574000003656e760673656e646572000103656e76066d656d6f727902010110030201020404017000000501000708010463616c6c00020901000ac10101be0102057f017e4100410028020441c0006b22043602042004412c6a41106a220041003602002004412c6a41086a22014200370200200441186a41106a22024100360200200441186a41086a220342003703002004420037022c2004410036021c20044100360218200441186a1001200020022802002202360200200120032903002205370200200441106a2002360200200441086a200537030020042004290318220537022c200420053703002004411410004100200441c0006a3602040b0b0a010041040b0410c00000"

ethcore/src/externalities.rs

@@ -111,6 +111,10 @@ impl<'a, T: 'a, V: 'a, B: 'a> Externalities<'a, T, V, B>
 impl<'a, T: 'a, V: 'a, B: 'a> Ext for Externalities<'a, T, V, B>
 	where T: Tracer, V: VMTracer, B: StateBackend
 {
+	fn initial_storage_at(&self, key: &H256) -> vm::Result<H256> {
+		self.state.checkpoint_storage_at(0, &self.origin_info.address, key).map(|v| v.unwrap_or(H256::zero())).map_err(Into::into)
+	}
+
 	fn storage_at(&self, key: &H256) -> vm::Result<H256> {
 		self.state.storage_at(&self.origin_info.address, key).map_err(Into::into)
 	}
@@ -394,8 +398,12 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for Externalities<'a, T, V, B>
 		self.depth
 	}
 
-	fn inc_sstore_clears(&mut self) {
-		self.substate.sstore_clears_count = self.substate.sstore_clears_count + U256::one();
+	fn add_sstore_refund(&mut self, value: U256) {
+		self.substate.sstore_clears_refund = self.substate.sstore_clears_refund.saturating_add(value);
+	}
+
+	fn sub_sstore_refund(&mut self, value: U256) {
+		self.substate.sstore_clears_refund = self.substate.sstore_clears_refund.saturating_sub(value);
 	}
 
 	fn trace_next_instruction(&mut self, pc: usize, instruction: u8, current_gas: U256) -> bool {

ethcore/src/json_tests/executive.rs

@@ -111,6 +111,10 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for TestExt<'a, T, V, B>
 		self.ext.storage_at(key)
 	}
 
+	fn initial_storage_at(&self, key: &H256) -> vm::Result<H256> {
+		self.ext.initial_storage_at(key)
+	}
+
 	fn set_storage(&mut self, key: H256, value: H256) -> vm::Result<()> {
 		self.ext.set_storage(key, value)
 	}
@@ -205,8 +209,12 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for TestExt<'a, T, V, B>
 		false
 	}
 
-	fn inc_sstore_clears(&mut self) {
-		self.ext.inc_sstore_clears()
+	fn add_sstore_refund(&mut self, value: U256) {
+		self.ext.add_sstore_refund(value)
+	}
+
+	fn sub_sstore_refund(&mut self, value: U256) {
+		self.ext.sub_sstore_refund(value)
 	}
 }
 

ethcore/src/spec/spec.rs

@@ -115,6 +115,8 @@ pub struct CommonParams {
 	pub eip145_transition: BlockNumber,
 	/// Number of first block where EIP-1052 rules begin.
 	pub eip1052_transition: BlockNumber,
+	/// Number of first block where EIP-1283 rules begin.
+	pub eip1283_transition: BlockNumber,
 	/// Number of first block where EIP-1014 rules begin.
 	pub eip1014_transition: BlockNumber,
 	/// Number of first block where dust cleanup rules (EIP-168 and EIP169) begin.
@@ -181,6 +183,7 @@ impl CommonParams {
 		schedule.have_return_data = block_number >= self.eip211_transition;
 		schedule.have_bitwise_shifting = block_number >= self.eip145_transition;
 		schedule.have_extcodehash = block_number >= self.eip1052_transition;
+		schedule.eip1283 = block_number >= self.eip1283_transition;
 		if block_number >= self.eip210_transition {
 			schedule.blockhash_gas = 800;
 		}
@@ -284,6 +287,10 @@ impl From<ethjson::spec::Params> for CommonParams {
 				BlockNumber::max_value,
 				Into::into,
 			),
+			eip1283_transition: p.eip1283_transition.map_or_else(
+				BlockNumber::max_value,
+				Into::into,
+			),
 			eip1014_transition: p.eip1014_transition.map_or_else(
 				BlockNumber::max_value,
 				Into::into,