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

Handle the case for contract creation on an empty but exist account with storage items #10065

Merged
merged 5 commits into from
Jan 15, 2019
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
2 changes: 1 addition & 1 deletion ethcore/res/ethereum/tests
Submodule tests updated 110 files
44 changes: 3 additions & 41 deletions ethcore/res/ethereum/tests-issues/currents.json
Original file line number Diff line number Diff line change
@@ -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": []
}
7 changes: 6 additions & 1 deletion ethcore/src/externalities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<H256> {
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<H256> {
Expand Down
5 changes: 5 additions & 0 deletions ethcore/src/state/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions ethcore/src/state/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,13 @@ impl<B: Backend> State<B> {
|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<bool> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not just return bool here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Below State::ensure_cached can fail, and in that case I think it's better to bail because it's a DB failure.

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<Option<H256>> {
self.ensure_cached(a, RequireCache::None, true,
Expand Down