From fcda8d2a144166c6e3a24a032a62a556c9a4e777 Mon Sep 17 00:00:00 2001 From: Andrei Maiboroda Date: Wed, 10 Jul 2019 12:38:42 +0200 Subject: [PATCH] Change the logic around reverting touched Precompiled contracts to agree with geth. Now precompile stays touched even after revert in the calling frame, but only for one RIPEMD precompile. --- CHANGELOG.md | 2 +- libethereum/Executive.cpp | 20 ++++++++++++-------- libethereum/State.cpp | 14 ++++++++++++++ libethereum/State.h | 5 +++++ test/jsontests | 2 +- 5 files changed, 33 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 32095ac6706..25de7d41055 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,7 +32,7 @@ - Fixed: [#5627](https://github.com/ethereum/aleth/pull/5627) Correct testeth --help log output indentation. - Fixed: [#5644](https://github.com/ethereum/aleth/pull/5644) Avoid attempting to sync with disconnected peers. - Fixed: [#5647](https://github.com/ethereum/aleth/pull/5647) test_importRawBlock RPC method correctly fails in case of import failure. - +- Fixed: [#5664](https://github.com/ethereum/aleth/pull/5664) Behavior in corner case tests about touching empty Precompiles now agrees with geth's results. ## [1.6.0] - 2019-04-16 diff --git a/libethereum/Executive.cpp b/libethereum/Executive.cpp index 5b66e7fadae..3aae475d181 100644 --- a/libethereum/Executive.cpp +++ b/libethereum/Executive.cpp @@ -35,6 +35,8 @@ using namespace dev::eth; namespace { +Address const c_RipemdPrecompiledAddress{0x03}; + std::string dumpStackAndMemory(LegacyVM const& _vm) { ostringstream o; @@ -304,19 +306,21 @@ bool Executive::call(CallParameters const& _p, u256 const& _gasPrice, Address co if (m_sealEngine.isPrecompiled(_p.codeAddress, m_envInfo.number())) { + // Empty RIPEMD contract needs to be deleted even in case of OOG + // because of the anomaly on the main net caused by buggy behavior by both Geth and Parity + // https://github.com/ethereum/go-ethereum/pull/3341/files#diff-2433aa143ee4772026454b8abd76b9dd + // https://github.com/ethereum/EIPs/issues/716 + // https://github.com/ethereum/aleth/pull/5664 + // We mark the account as touched here, so that is can be removed among other touched empty + // accounts (after tx finalization) + if (_p.receiveAddress == c_RipemdPrecompiledAddress) + m_s.unrevertableTouch(_p.codeAddress); + bigint g = m_sealEngine.costOfPrecompiled(_p.codeAddress, _p.data, m_envInfo.number()); if (_p.gas < g) { m_excepted = TransactionException::OutOfGasBase; // Bail from exception. - - // Empty precompiled contracts need to be deleted even in case of OOG - // because the bug in both Geth and Parity led to deleting RIPEMD precompiled in this case - // see https://github.com/ethereum/go-ethereum/pull/3341/files#diff-2433aa143ee4772026454b8abd76b9dd - // We mark the account as touched here, so that is can be removed among other touched empty accounts (after tx finalization) - if (m_envInfo.number() >= m_sealEngine.chainParams().EIP158ForkBlock) - m_s.addBalance(_p.codeAddress, 0); - return true; // true actually means "all finished - nothing more to be done regarding go(). } else diff --git a/libethereum/State.cpp b/libethereum/State.cpp index 063872eca67..f6fe6824386 100644 --- a/libethereum/State.cpp +++ b/libethereum/State.cpp @@ -53,6 +53,7 @@ State::State(State const& _s): m_unchangedCacheEntries(_s.m_unchangedCacheEntries), m_nonExistingAccountsCache(_s.m_nonExistingAccountsCache), m_touched(_s.m_touched), + m_unrevertablyTouched(_s.m_unrevertablyTouched), m_accountStartNonce(_s.m_accountStartNonce) {} @@ -126,6 +127,13 @@ void State::removeEmptyAccounts() for (auto& i: m_cache) if (i.second.isDirty() && i.second.isEmpty()) i.second.kill(); + + for (auto const& _address : m_unrevertablyTouched) + { + Account* a = account(_address); + if (a && a->isEmpty()) + a->kill(); + } } State& State::operator=(State const& _s) @@ -139,6 +147,7 @@ State& State::operator=(State const& _s) m_unchangedCacheEntries = _s.m_unchangedCacheEntries; m_nonExistingAccountsCache = _s.m_nonExistingAccountsCache; m_touched = _s.m_touched; + m_unrevertablyTouched = _s.m_unrevertablyTouched; m_accountStartNonce = _s.m_accountStartNonce; return *this; } @@ -564,6 +573,11 @@ u256 State::version(Address const& _a) const return a ? a->version() : 0; } +void State::unrevertableTouch(Address const& _address) +{ + m_unrevertablyTouched.insert(_address); +} + size_t State::savepoint() const { return m_changeLog.size(); diff --git a/libethereum/State.h b/libethereum/State.h index 659248a968f..a9bad752be2 100644 --- a/libethereum/State.h +++ b/libethereum/State.h @@ -318,6 +318,9 @@ class State u256 const& requireAccountStartNonce() const; void noteAccountStartNonce(u256 const& _actual); + /// Mark account as touched and keep it touched even in case of rollback + void unrevertableTouch(Address const& _addr); + /// Create a savepoint in the state changelog. /// @return The savepoint index that can be used in rollback() function. size_t savepoint() const; @@ -361,6 +364,8 @@ class State mutable std::set
m_nonExistingAccountsCache; /// Tracks all addresses touched so far. AddressHash m_touched; + /// Tracks addresses that were touched and should stay touched in case of rollback + AddressHash m_unrevertablyTouched; u256 m_accountStartNonce; diff --git a/test/jsontests b/test/jsontests index 5018b8f30e9..2b199c4b99e 160000 --- a/test/jsontests +++ b/test/jsontests @@ -1 +1 @@ -Subproject commit 5018b8f30e9e57a0d136ed40bc6ceab51b61e144 +Subproject commit 2b199c4b99edeb5ef2e43ef0f773e6fcd7538ae3