Skip to content
This repository has been archived by the owner on Oct 28, 2021. It is now read-only.

Coming to agreement with geth on RIPEMD touch revert - another alternative solution #5664

Merged
merged 1 commit into from
Jul 11, 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 CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
20 changes: 12 additions & 8 deletions libethereum/Executive.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ using namespace dev::eth;

namespace
{
Address const c_RipemdPrecompiledAddress{0x03};

std::string dumpStackAndMemory(LegacyVM const& _vm)
{
ostringstream o;
Expand Down Expand Up @@ -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
Expand Down
14 changes: 14 additions & 0 deletions libethereum/State.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{}

Expand Down Expand Up @@ -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)
Expand All @@ -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;
}
Expand Down Expand Up @@ -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();
Expand Down
5 changes: 5 additions & 0 deletions libethereum/State.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -361,6 +364,8 @@ class State
mutable std::set<Address> 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;

Expand Down
2 changes: 1 addition & 1 deletion test/jsontests
Submodule jsontests updated 6294 files