Compilance fix 5 (#221)

* Fix issues with SUICIDE testing and retval/retbuf

* Fix gas cost calculation related to address access cost

* Revert "Fix gas cost calculation related to address access cost"

This reverts commit 5406dab.

* Fix issue with call stipend

* Enable more jsontests

Author: sorpaas

.github/workflows/rust.yml

@@ -36,10 +36,12 @@ jobs:
     - name: Run tests
       run: |
         cargo run --release --verbose -p jsontests -- \
+        jsontests/res/ethtests/GeneralStateTests/stCodeCopyTest/ \
         jsontests/res/ethtests/GeneralStateTests/stExample/ \
         jsontests/res/ethtests/GeneralStateTests/stSLoadTest/ \
         jsontests/res/ethtests/GeneralStateTests/VMTests/vmArithmeticTest/ \
         jsontests/res/ethtests/GeneralStateTests/VMTests/vmBitwiseLogicOperation/ \
         jsontests/res/ethtests/GeneralStateTests/VMTests/vmIOandFlowOperations/ \
-        jsontests/res/ethtests/GeneralStateTests/VMTests/vmLogTest/
+        jsontests/res/ethtests/GeneralStateTests/VMTests/vmLogTest/ \
+        jsontests/res/ethtests/GeneralStateTests/VMTests/vmTests/
 

interpreter/src/eval/misc.rs

@@ -207,6 +207,7 @@ pub fn ret<S, Tr>(state: &mut Machine<S>) -> Control<Tr> {
 	pop_u256!(state, start, len);
 	try_or_fail!(state.memory.resize_offset(start, len));
 	state.memory.resize_to_range(start..(start + len));
+	state.memory.swap_and_clear(&mut state.retval);
 	Control::Exit(ExitSucceed::Returned.into())
 }
 
@@ -215,5 +216,6 @@ pub fn revert<S, Tr>(state: &mut Machine<S>) -> Control<Tr> {
 	pop_u256!(state, start, len);
 	try_or_fail!(state.memory.resize_offset(start, len));
 	state.memory.resize_to_range(start..(start + len));
+	state.memory.swap_and_clear(&mut state.retval);
 	Control::Exit(ExitError::Reverted.into())
 }

interpreter/src/eval/system.rs

@@ -1,7 +1,7 @@
 use super::Control;
 use crate::{
 	ExitException, ExitFatal, ExitSucceed, Log, Machine, RuntimeBackend, RuntimeEnvironment,
-	RuntimeState,
+	RuntimeState, Transfer,
 };
 use alloc::vec::Vec;
 use primitive_types::{H256, U256};
@@ -47,7 +47,7 @@ pub fn balance<S: AsRef<RuntimeState>, H: RuntimeEnvironment + RuntimeBackend, T
 	handler: &mut H,
 ) -> Control<Tr> {
 	pop!(machine, address);
-	try_or_fail!(handler.mark_hot(address.into(), None));
+	handler.mark_hot(address.into(), None);
 	push_u256!(machine, handler.balance(address.into()));
 
 	Control::Continue
@@ -127,7 +127,7 @@ pub fn extcodesize<S: AsRef<RuntimeState>, H: RuntimeEnvironment + RuntimeBacken
 	handler: &mut H,
 ) -> Control<Tr> {
 	pop!(machine, address);
-	try_or_fail!(handler.mark_hot(address.into(), None));
+	handler.mark_hot(address.into(), None);
 	let code_size = handler.code_size(address.into());
 	push_u256!(machine, code_size);
 
@@ -139,7 +139,7 @@ pub fn extcodehash<S: AsRef<RuntimeState>, H: RuntimeEnvironment + RuntimeBacken
 	handler: &mut H,
 ) -> Control<Tr> {
 	pop!(machine, address);
-	try_or_fail!(handler.mark_hot(address.into(), None));
+	handler.mark_hot(address.into(), None);
 	let code_hash = handler.code_hash(address.into());
 	push!(machine, code_hash);
 
@@ -153,7 +153,7 @@ pub fn extcodecopy<S: AsRef<RuntimeState>, H: RuntimeEnvironment + RuntimeBacken
 	pop!(machine, address);
 	pop_u256!(machine, memory_offset, code_offset, len);
 
-	try_or_fail!(handler.mark_hot(address.into(), None));
+	handler.mark_hot(address.into(), None);
 	try_or_fail!(machine.memory.resize_offset(memory_offset, len));
 
 	let code = handler.code(address.into());
@@ -265,7 +265,7 @@ pub fn sload<S: AsRef<RuntimeState>, H: RuntimeEnvironment + RuntimeBackend, Tr>
 	handler: &mut H,
 ) -> Control<Tr> {
 	pop!(machine, index);
-	try_or_fail!(handler.mark_hot(machine.state.as_ref().context.address, Some(index)));
+	handler.mark_hot(machine.state.as_ref().context.address, Some(index));
 	let value = handler.storage(machine.state.as_ref().context.address, index);
 	push!(machine, value);
 
@@ -277,7 +277,7 @@ pub fn sstore<S: AsRef<RuntimeState>, H: RuntimeEnvironment + RuntimeBackend, Tr
 	handler: &mut H,
 ) -> Control<Tr> {
 	pop!(machine, index, value);
-	try_or_fail!(handler.mark_hot(machine.state.as_ref().context.address, Some(index)));
+	handler.mark_hot(machine.state.as_ref().context.address, Some(index));
 
 	match handler.set_storage(machine.state.as_ref().context.address, index, value) {
 		Ok(()) => Control::Continue,
@@ -335,12 +335,23 @@ pub fn suicide<S: AsRef<RuntimeState>, H: RuntimeEnvironment + RuntimeBackend, T
 	machine: &mut Machine<S>,
 	handler: &mut H,
 ) -> Control<Tr> {
-	pop!(machine, target);
+	let address = machine.state.as_ref().context.address;
 
-	match handler.mark_delete(machine.state.as_ref().context.address, target.into()) {
-		Ok(()) => (),
-		Err(e) => return Control::Exit(e.into()),
-	}
+	match machine.stack.perform_pop1_push0(|target| {
+		let balance = handler.balance(address);
+
+		handler.transfer(Transfer {
+			source: address,
+			target: (*target).into(),
+			value: balance,
+		})?;
 
-	Control::Exit(ExitSucceed::Suicided.into())
+		handler.mark_delete(address);
+		handler.reset_balance(address);
+
+		Ok(((), ()))
+	}) {
+		Ok(()) => Control::Exit(ExitSucceed::Suicided.into()),
+		Err(e) => Control::Exit(Err(e)),
+	}
 }

interpreter/src/lib.rs

@@ -40,6 +40,10 @@ pub struct Machine<S> {
 	position: usize,
 	/// Code validity maps.
 	valids: Valids,
+	/// Return value. Note the difference between `retbuf`.
+	/// A `retval` holds what's returned by the current machine, with `RETURN` or `REVERT` opcode.
+	/// A `retbuf` holds the buffer of returned value by sub-calls.
+	pub retval: Vec<u8>,
 	/// Memory.
 	pub memory: Memory,
 	/// Stack.
@@ -69,6 +73,7 @@ impl<S> Machine<S> {
 			code,
 			position: 0,
 			valids,
+			retval: Vec::new(),
 			memory: Memory::new(memory_limit),
 			stack: Stack::new(stack_limit),
 			state,
@@ -93,11 +98,6 @@ impl<S> Machine<S> {
 		self.position = self.code.len();
 	}
 
-	/// Return value of the machine.
-	pub fn into_retbuf(self) -> Vec<u8> {
-		self.memory.into_data()
-	}
-
 	/// Inspect the machine's next opcode and current stack.
 	pub fn inspect(&self) -> Option<(Opcode, &Stack)> {
 		self.code

interpreter/src/memory.rs

@@ -1,7 +1,7 @@
 use crate::{ExitException, ExitFatal};
 use alloc::vec::Vec;
-use core::cmp::min;
 use core::ops::{BitAnd, Not, Range};
+use core::{cmp::min, mem};
 use primitive_types::U256;
 
 /// A sequencial memory. It uses Rust's `Vec` for internal
@@ -48,9 +48,10 @@ impl Memory {
 		&self.data
 	}
 
-	/// Into the full data.
-	pub fn into_data(self) -> Vec<u8> {
-		self.data
+	pub(crate) fn swap_and_clear(&mut self, other: &mut Vec<u8>) {
+		mem::swap(&mut self.data, other);
+		self.data = Vec::new();
+		self.effective_len = U256::zero();
 	}
 
 	/// Resize the memory, making it cover the memory region of `offset..(offset

interpreter/src/runtime.rs

@@ -133,13 +133,13 @@ pub trait RuntimeBackend: RuntimeBaseBackend {
 	}
 
 	/// Mark an address or (address, index) pair as hot.
-	fn mark_hot(&mut self, address: H160, index: Option<H256>) -> Result<(), ExitError>;
+	fn mark_hot(&mut self, address: H160, index: Option<H256>);
 	/// Set storage value of address at index.
 	fn set_storage(&mut self, address: H160, index: H256, value: H256) -> Result<(), ExitError>;
 	/// Create a log owned by address with given topics and data.
 	fn log(&mut self, log: Log) -> Result<(), ExitError>;
-	/// Mark an address to be deleted, with funds transferred to target.
-	fn mark_delete(&mut self, address: H160, target: H160) -> Result<(), ExitError>;
+	/// Mark an address to be deleted.
+	fn mark_delete(&mut self, address: H160);
 	/// Fully delete storages of an account.
 	fn reset_storage(&mut self, address: H160);
 	/// Set code of an account.

interpreter/tests/performance.rs

@@ -15,7 +15,7 @@ macro_rules! ret_test {
 				vm.run(&mut (), &ETABLE),
 				Capture::Exit(Ok(ExitSucceed::Returned.into()))
 			);
-			assert_eq!(vm.into_retbuf(), hex::decode($ret).unwrap());
+			assert_eq!(vm.retval, hex::decode($ret).unwrap());
 		}
 	};
 }

interpreter/tests/usability.rs

@@ -25,7 +25,7 @@ fn etable_wrap() {
 	let mut vm = Machine::new(Rc::new(code), Rc::new(data), 1024, 10000, ());
 	let result = vm.run(&mut (), &wrapped_etable);
 	assert_eq!(result, Capture::Exit(Ok(ExitSucceed::Returned.into())));
-	assert_eq!(vm.into_retbuf(), hex::decode(&RET1).unwrap());
+	assert_eq!(vm.retval, hex::decode(&RET1).unwrap());
 }
 
 #[test]
@@ -125,7 +125,7 @@ impl<'a> RuntimeBackend for UnimplementedHandler {
 		unimplemented!()
 	}
 
-	fn mark_hot(&mut self, _address: H160, _index: Option<H256>) -> Result<(), ExitError> {
+	fn mark_hot(&mut self, _address: H160, _index: Option<H256>) {
 		unimplemented!()
 	}
 
@@ -135,7 +135,7 @@ impl<'a> RuntimeBackend for UnimplementedHandler {
 	fn log(&mut self, _log: Log) -> Result<(), ExitError> {
 		unimplemented!()
 	}
-	fn mark_delete(&mut self, _address: H160, _target: H160) -> Result<(), ExitError> {
+	fn mark_delete(&mut self, _address: H160) {
 		unimplemented!()
 	}
 
@@ -194,5 +194,5 @@ fn etable_runtime() {
 
 	let res = vm.run(&mut handler, &RUNTIME_ETABLE).exit().unwrap();
 	assert_eq!(res, Ok(ExitSucceed::Returned.into()));
-	assert_eq!(vm.into_retbuf(), hex::decode(&RET1).unwrap());
+	assert_eq!(vm.retval, hex::decode(&RET1).unwrap());
 }

jsontests/src/run.rs

@@ -94,6 +94,7 @@ pub fn run_test(_filename: &str, _test_name: &str, test: Test, debug: bool) -> R
 		&invoker,
 		&etable,
 	);
+	run_backend.layers[0].clear_pending();
 
 	// Step
 	if debug {
@@ -116,6 +117,7 @@ pub fn run_test(_filename: &str, _test_name: &str, test: Test, debug: bool) -> R
 				break result;
 			}
 		});
+		step_backend.layers[0].clear_pending();
 	}
 
 	let state_root = crate::hash::state_root(&run_backend);

src/backend/in_memory.rs

@@ -4,6 +4,7 @@ use crate::{
 };
 use alloc::collections::{BTreeMap, BTreeSet};
 use primitive_types::{H160, H256, U256};
+use std::mem;
 
 #[derive(Clone, Debug)]
 pub struct InMemoryEnvironment {
@@ -30,7 +31,6 @@ pub struct InMemoryAccount {
 #[derive(Clone, Debug)]
 pub struct InMemorySuicideInfo {
 	pub address: H160,
-	pub target: H160,
 }
 
 #[derive(Clone, Debug)]
@@ -41,6 +41,18 @@ pub struct InMemoryLayer {
 	pub hots: BTreeSet<(H160, Option<H256>)>,
 }
 
+impl InMemoryLayer {
+	pub fn clear_pending(&mut self) {
+		self.hots.clear();
+
+		let mut suicides = Vec::new();
+		mem::swap(&mut suicides, &mut self.suicides);
+		for suicide in suicides {
+			self.state.remove(&suicide.address);
+		}
+	}
+}
+
 #[derive(Clone, Debug)]
 pub struct InMemoryBackend {
 	pub environment: InMemoryEnvironment,
@@ -168,9 +180,8 @@ impl RuntimeBackend for InMemoryBackend {
 		!self.current_layer().hots.contains(&(address, index))
 	}
 
-	fn mark_hot(&mut self, address: H160, index: Option<H256>) -> Result<(), ExitError> {
+	fn mark_hot(&mut self, address: H160, index: Option<H256>) {
 		self.current_layer_mut().hots.insert((address, index));
-		Ok(())
 	}
 
 	fn set_storage(&mut self, address: H160, index: H256, value: H256) -> Result<(), ExitError> {
@@ -189,11 +200,10 @@ impl RuntimeBackend for InMemoryBackend {
 		Ok(())
 	}
 
-	fn mark_delete(&mut self, address: H160, target: H160) -> Result<(), ExitError> {
+	fn mark_delete(&mut self, address: H160) {
 		self.current_layer_mut()
 			.suicides
-			.push(InMemorySuicideInfo { address, target });
-		Ok(())
+			.push(InMemorySuicideInfo { address });
 	}
 
 	fn reset_storage(&mut self, address: H160) {