From 64704c456fffb6043671692e65486f6daf529d58 Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Tue, 15 Jan 2019 10:21:44 +0100 Subject: [PATCH] Handle the case for contract creation on an empty but exist account with storage items (#10065) * Add is_base_storage_root_unchanged * Fix compile, use a shortcut for check, and remove ignored tests * Add a warn! * Update ethereum/tests to v6.0.0-beta.2 * grumble: use {:#x} instead of 0x{:x} Co-Authored-By: sorpaas --- ethcore/res/ethereum/tests | 2 +- .../res/ethereum/tests-issues/currents.json | 44 ++----------------- ethcore/src/externalities.rs | 7 ++- ethcore/src/state/account.rs | 5 +++ ethcore/src/state/mod.rs | 7 +++ 5 files changed, 22 insertions(+), 43 deletions(-) diff --git a/ethcore/res/ethereum/tests b/ethcore/res/ethereum/tests index 2cd62aeec..420f44347 160000 --- a/ethcore/res/ethereum/tests +++ b/ethcore/res/ethereum/tests @@ -1 +1 @@ -Subproject commit 2cd62aeec11da29766b30d500f2b9a96f1f28cf0 +Subproject commit 420f443477caa8516f1f9ee8122fafc3415c0f34 diff --git a/ethcore/res/ethereum/tests-issues/currents.json b/ethcore/res/ethereum/tests-issues/currents.json index 2ece034d0..d4d3f5e3a 100644 --- a/ethcore/res/ethereum/tests-issues/currents.json +++ b/ethcore/res/ethereum/tests-issues/currents.json @@ -1,42 +1,4 @@ -{ "block": - [ - { - "reference": "None", - "comment": "This failing test is deemed skippable. Could not happen on a mainnet.", - "failing": "GeneralStateTest_stCreate2", - "subtests": ["RevertInCreateInInitCreate2_d0g0v0_Constantinople"] - }, - { - "reference": "None", - "comment": "This failing test is deemed skippable. Could not happen on a mainnet.", - "failing": "GeneralStateTest_stRevertTest", - "subtests": ["RevertInCreateInInit_d0g0v0_Constantinople"] - } - ], - "state": - [ - { - "reference": "None", - "comment": "This failing test is deemed skippable. Could not happen on a mainnet.", - "failing": "stCreate2Test", - "subtests": { - "RevertInCreateInInitCreate2": { - "subnumbers": ["1"], - "chain": "Constantinople (test)" - } - } - }, - { - "reference": "None", - "comment": "This failing test is deemed skippable. Could not happen on a mainnet.", - "failing": "stRevertTest", - "subtests": { - "RevertInCreateInInit": { - "subnumbers": ["1"], - "chain": "Constantinople (test)" - } - } - } - - ] +{ + "block": [], + "state": [] } diff --git a/ethcore/src/externalities.rs b/ethcore/src/externalities.rs index 0cda8ac87..a2f35b6de 100644 --- a/ethcore/src/externalities.rs +++ b/ethcore/src/externalities.rs @@ -116,7 +116,12 @@ 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 { - self.state.checkpoint_storage_at(0, &self.origin_info.address, key).map(|v| v.unwrap_or(H256::zero())).map_err(Into::into) + if self.state.is_base_storage_root_unchanged(&self.origin_info.address)? { + self.state.checkpoint_storage_at(0, &self.origin_info.address, key).map(|v| v.unwrap_or(H256::zero())).map_err(Into::into) + } else { + warn!(target: "externalities", "Detected existing account {:#x} where a forced contract creation happened.", self.origin_info.address); + Ok(H256::zero()) + } } fn storage_at(&self, key: &H256) -> vm::Result { diff --git a/ethcore/src/state/account.rs b/ethcore/src/state/account.rs index d4a42dd31..e4553b906 100644 --- a/ethcore/src/state/account.rs +++ b/ethcore/src/state/account.rs @@ -451,6 +451,11 @@ impl Account { } } + /// Whether the base storage root of this account is unchanged. + pub fn is_base_storage_root_unchanged(&self) -> bool { + self.original_storage_cache.is_none() + } + /// Storage root where the account changes are based upon. pub fn base_storage_root(&self) -> H256 { self.storage_root diff --git a/ethcore/src/state/mod.rs b/ethcore/src/state/mod.rs index 8a46c7e1d..2939c9729 100644 --- a/ethcore/src/state/mod.rs +++ b/ethcore/src/state/mod.rs @@ -539,6 +539,13 @@ impl State { |a| a.as_ref().map_or(self.account_start_nonce, |account| *account.nonce())) } + /// Whether the base storage root of an account remains unchanged. + pub fn is_base_storage_root_unchanged(&self, a: &Address) -> TrieResult { + Ok(self.ensure_cached(a, RequireCache::None, true, + |a| a.as_ref().map(|account| account.is_base_storage_root_unchanged()))? + .unwrap_or(true)) + } + /// Get the storage root of account `a`. pub fn storage_root(&self, a: &Address) -> TrieResult> { self.ensure_cached(a, RequireCache::None, true,