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

EIP98 - Remove state root hash from transaction receipt #4035

Merged
merged 7 commits into from
May 4, 2017
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
1 change: 1 addition & 0 deletions libethcore/Exceptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ DEV_SIMPLE_EXCEPTION(InvalidParentHash);
DEV_SIMPLE_EXCEPTION(InvalidUncleParentHash);
DEV_SIMPLE_EXCEPTION(InvalidNumber);
DEV_SIMPLE_EXCEPTION(InvalidZeroSignatureTransaction);
DEV_SIMPLE_EXCEPTION(InvalidTransactionReceiptFormat);
DEV_SIMPLE_EXCEPTION(BlockNotFound);
DEV_SIMPLE_EXCEPTION(UnknownParent);

Expand Down
9 changes: 2 additions & 7 deletions libethereum/Block.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -829,15 +829,10 @@ bool Block::sealBlock(bytesConstRef _header)
return true;
}

State Block::fromPending(unsigned _i) const
h256 Block::stateRootBeforeTx(unsigned _i) const
{
State ret = m_state;
_i = min<unsigned>(_i, m_transactions.size());
if (!_i)
ret.setRoot(m_previousBlock.stateRoot());
else
ret.setRoot(receipt(_i - 1).stateRoot());
return ret;
return (_i > 0 ? receipt(_i - 1).stateRoot() : m_previousBlock.stateRoot());
}

LogBloom Block::logBloom() const
Expand Down
5 changes: 3 additions & 2 deletions libethereum/Block.h
Original file line number Diff line number Diff line change
Expand Up @@ -202,10 +202,11 @@ class Block
/// Get the bloom filter of a particular transaction that happened in the block.
LogBloom const& logBloom(unsigned _i) const { return receipt(_i).bloom(); }

/// Get the State immediately after the given number of pending transactions have been applied.
/// Get the State root hash immediately after all previous transactions before transaction @a _i have been applied.
/// If (_i == 0) returns the initial state of the block.
/// If (_i == pending().size()) returns the final state of the block, prior to rewards.
State fromPending(unsigned _i) const;
/// Returns zero hash if intermediate state root is not available in the receipt (the case after EIP98)
h256 stateRootBeforeTx(unsigned _i) const;

// State-change operations

Expand Down
22 changes: 0 additions & 22 deletions libethereum/Client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -775,28 +775,6 @@ Block Client::block(h256 const& _blockHash, PopulationStatistics* o_stats) const
}
}

State Client::state(unsigned _txi, h256 const& _blockHash) const
Copy link
Member Author

Choose a reason for hiding this comment

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

These were not used

{
try
{
return block(_blockHash).fromPending(_txi);
}
catch (Exception& ex)
{
ex << errinfo_block(bc().block(_blockHash));
onBadBlock(ex);
return State(chainParams().accountStartNonce);
}
}

eth::State Client::state(unsigned _txi) const
{
DEV_READ_GUARDED(x_postSeal)
return m_postSeal.fromPending(_txi);
assert(false);
return State(chainParams().accountStartNonce);
}

void Client::flushTransactions()
{
doWork();
Expand Down
6 changes: 0 additions & 6 deletions libethereum/Client.h
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,6 @@ class Client: public ClientBase, protected Worker
// [PRIVATE API - only relevant for base clients, not available in general]
/// Get the block.
dev::eth::Block block(h256 const& _blockHash, PopulationStatistics* o_stats) const;
/// Get the state of the given block part way through execution, immediately before transaction
/// index @a _txi.
dev::eth::State state(unsigned _txi, h256 const& _block) const;
/// Get the state of the currently pending block part way through execution, immediately before
/// transaction index @a _txi.
dev::eth::State state(unsigned _txi) const;

/// Get the object representing the current state of Ethereum.
dev::eth::Block postState() const { ReadGuard l(x_postSeal); return m_postSeal; }
Expand Down
4 changes: 2 additions & 2 deletions libethereum/Executive.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,8 @@ Executive::Executive(Block& _s, LastHashes const& _lh, unsigned _level):
{
}

Executive::Executive(State& _s, Block const& _block, unsigned _txIndex, BlockChain const& _bc, unsigned _level):
m_s(_s = _block.fromPending(_txIndex)),
Executive::Executive(State& io_s, Block const& _block, unsigned _txIndex, BlockChain const& _bc, unsigned _level):
m_s(createIntermediateState(io_s, _block, _txIndex, _bc)),
m_envInfo(_block.info(), _bc.lastHashes(_block.info().parentHash()), _txIndex ? _block.receipt(_txIndex - 1).gasUsed() : 0),
m_depth(_level),
m_sealEngine(*_bc.sealEngine())
Expand Down
2 changes: 1 addition & 1 deletion libethereum/Executive.h
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ class Executive
* populating environment info from the given Block and the LastHashes portion from the BlockChain.
* State is assigned the resultant value, but otherwise unused.
*/
Executive(State& _s, Block const& _block, unsigned _txIndex, BlockChain const& _bc, unsigned _level = 0);
Executive(State& io_s, Block const& _block, unsigned _txIndex, BlockChain const& _bc, unsigned _level = 0);

Executive(Executive const&) = delete;
void operator=(Executive) = delete;
Expand Down
57 changes: 49 additions & 8 deletions libethereum/State.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@
#include <libethcore/Exceptions.h>
#include <libevm/VMFactory.h>
#include "BlockChain.h"
#include "Block.h"
#include "CodeSizeCache.h"
#include "Defaults.h"
#include "ExtVM.h"
#include "Executive.h"
#include "BlockChain.h"
#include "TransactionQueue.h"

using namespace std;
Expand All @@ -49,6 +49,20 @@ const char* StateDetail::name() { return EthViolet "⚙" EthWhite " ◌"; }
const char* StateTrace::name() { return EthViolet "⚙" EthGray " ◎"; }
const char* StateChat::name() { return EthViolet "⚙" EthWhite " ◌"; }

namespace
{

void executeTransaction(Executive& _e, Transaction const& _t, OnOpFunc const& _onOp)
{
_e.initialize(_t);

if (!_e.execute())
_e.go(_onOp);
_e.finalize();
}

}

State::State(u256 const& _accountStartNonce, OverlayDB const& _db, BaseState _bs):
m_db(_db),
m_state(&m_db),
Expand Down Expand Up @@ -523,13 +537,9 @@ std::pair<ExecutionResult, TransactionReceipt> State::execute(EnvInfo const& _en
Executive e(*this, _envInfo, _sealEngine);
ExecutionResult res;
e.setResultRecipient(res);
e.initialize(_t);

// OK - transaction looks valid - execute.
u256 startGasUsed = _envInfo.gasUsed();
if (!e.execute())
e.go(onOp);
e.finalize();
u256 const startGasUsed = _envInfo.gasUsed();
executeTransaction(e, _t, onOp);

if (_p == Permanence::Reverted)
m_cache.clear();
Expand All @@ -539,7 +549,24 @@ std::pair<ExecutionResult, TransactionReceipt> State::execute(EnvInfo const& _en
commit(removeEmptyAccounts ? State::CommitBehaviour::RemoveEmptyAccounts : State::CommitBehaviour::KeepEmptyAccounts);
}

return make_pair(res, TransactionReceipt(rootHash(), startGasUsed + e.gasUsed(), e.logs()));
TransactionReceipt const receipt = _envInfo.number() >= _sealEngine.chainParams().u256Param("metropolisForkBlock") ?
TransactionReceipt(startGasUsed + e.gasUsed(), e.logs()) :
TransactionReceipt(rootHash(), startGasUsed + e.gasUsed(), e.logs());
return make_pair(res, receipt);
}

void State::executeBlockTransactions(Block const& _block, unsigned _txCount, LastHashes const& _lastHashes, SealEngineFace const& _sealEngine)
{
u256 gasUsed = 0;
for (unsigned i = 0; i < _txCount; ++i)
{
Copy link
Contributor

Choose a reason for hiding this comment

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

The whole for-loop body should be a helper function also used elsewhere - or are the cases there more complicated?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, the other case - State::execute needs to get gas, logs, and return value from Executive...
But I've moved the common part (initialize-execute-go-finalize) to separate helper method

Copy link
Member

Choose a reason for hiding this comment

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

I wanted to move it to a separate function for a long time.

EnvInfo envInfo(_block.info(), _lastHashes, gasUsed);

Executive e(*this, envInfo, _sealEngine);
executeTransaction(e, _block.pending()[i], OnOpFunc());

gasUsed += e.gasUsed();
}
}

std::ostream& dev::eth::operator<<(std::ostream& _out, State const& _s)
Expand Down Expand Up @@ -617,3 +644,17 @@ std::ostream& dev::eth::operator<<(std::ostream& _out, State const& _s)
}
return _out;
}

State& dev::eth::createIntermediateState(State& o_s, Block const& _block, unsigned _txIndex, BlockChain const& _bc)
{
o_s = _block.state();
u256 const rootHash = _block.stateRootBeforeTx(_txIndex);
if (rootHash)
o_s.setRoot(rootHash);
else
{
o_s.setRoot(_block.stateRootBeforeTx(0));
o_s.executeBlockTransactions(_block, _txIndex, _bc.lastHashes(_block.info().parentHash()), *_bc.sealEngine());
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need a check for dereferencing sealEngine here?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think so. I've spot it before, it should not be a pointer type.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, it's always non-null in practice

}
return o_s;
}
8 changes: 7 additions & 1 deletion libethereum/State.h
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ DEV_SIMPLE_EXCEPTION(InvalidAccountStartNonceInState);
DEV_SIMPLE_EXCEPTION(IncorrectAccountStartNonceInState);

class SealEngineFace;

class Executive;

namespace detail
{
Expand Down Expand Up @@ -207,6 +207,10 @@ class State
/// This will change the state accordingly.
std::pair<ExecutionResult, TransactionReceipt> execute(EnvInfo const& _envInfo, SealEngineFace const& _sealEngine, Transaction const& _t, Permanence _p = Permanence::Committed, OnOpFunc const& _onOp = OnOpFunc());

/// Execute @a _txCount transactions of a given block.
/// This will change the state accordingly.
void executeBlockTransactions(Block const& _block, unsigned _txCount, LastHashes const& _lastHashes, SealEngineFace const& _sealEngine);

/// Check if the address is in use.
bool addressInUse(Address const& _address) const;

Expand Down Expand Up @@ -337,6 +341,8 @@ class State

std::ostream& operator<<(std::ostream& _out, State const& _s);

State& createIntermediateState(State& o_s, Block const& _block, unsigned _txIndex, BlockChain const& _bc);

template <class DB>
AddressHash commit(AccountMap const& _cache, SecureTrieDB<Address, DB>& _state)
{
Expand Down
26 changes: 21 additions & 5 deletions libethereum/TransactionReceipt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,21 @@ using namespace dev::eth;
TransactionReceipt::TransactionReceipt(bytesConstRef _rlp)
{
RLP r(_rlp);
m_stateRoot = (h256)r[0];
m_gasUsed = (u256)r[1];
m_bloom = (LogBloom)r[2];
for (auto const& i: r[3])
if (!r.isList() || r.itemCount() < 3 || r.itemCount() > 4)
DEV_SIMPLE_EXCEPTION(InvalidTransactionReceiptFormat);

size_t gasUsedIndex = 0;
if (r.itemCount() == 4)
{
m_stateRoot = (h256)r[0];
gasUsedIndex = 1;
}

m_gasUsed = (u256)r[gasUsedIndex];
m_bloom = (LogBloom)r[gasUsedIndex + 1];
for (auto const& i : r[gasUsedIndex + 2])
m_log.emplace_back(i);

}

TransactionReceipt::TransactionReceipt(h256 _root, u256 _gasUsed, LogEntries const& _log):
Expand All @@ -44,7 +54,13 @@ TransactionReceipt::TransactionReceipt(h256 _root, u256 _gasUsed, LogEntries con

void TransactionReceipt::streamRLP(RLPStream& _s) const
{
_s.appendList(4) << m_stateRoot << m_gasUsed << m_bloom;
if (m_stateRoot)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you document this is the header? Perhaps best to put that into the class description.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a comment

_s.appendList(4) << m_stateRoot;
else
_s.appendList(3);

_s << m_gasUsed << m_bloom;

_s.appendList(m_log.size());
for (LogEntry const& l: m_log)
l.streamRLP(_s);
Expand Down
4 changes: 4 additions & 0 deletions libethereum/TransactionReceipt.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,15 @@ namespace dev
namespace eth
{

/// Transaction receipt, constructed either from RLP representation or from individual values.
/// State Root is optional, m_stateRoot is h256() when it is empty (for transactions after Metropolis)
/// Empty state root is not included into RLP-encoding.
class TransactionReceipt
{
public:
TransactionReceipt(bytesConstRef _rlp);
TransactionReceipt(h256 _root, u256 _gasUsed, LogEntries const& _log);
TransactionReceipt(u256 _gasUsed, LogEntries const& _log) : TransactionReceipt(h256(), _gasUsed, _log) {}

h256 const& stateRoot() const { return m_stateRoot; }
u256 const& gasUsed() const { return m_gasUsed; }
Expand Down
13 changes: 10 additions & 3 deletions libweb3jsonrpc/Debug.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,19 @@ Json::Value Debug::traceTransaction(Executive& _e, Transaction const& _t, Json::

Json::Value Debug::traceBlock(Block const& _block, Json::Value const& _json)
{
State s(_block.state());
s.setRoot(_block.stateRootBeforeTx(0));

Json::Value traces(Json::arrayValue);
for (unsigned k = 0; k < _block.pending().size(); k++)
{
Transaction t = _block.pending()[k];
State s(State::Null);

u256 const gasUsed = k ? _block.receipt(k - 1).gasUsed() : 0;
EnvInfo envInfo(_block.info(), m_eth.blockChain().lastHashes(_block.info().parentHash()), gasUsed);
Executive e(s, envInfo, *m_eth.blockChain().sealEngine());

eth::ExecutionResult er;
Executive e(s, _block, k, m_eth.blockChain());
e.setResultRecipient(er);
traces.append(traceTransaction(e, t, _json));
}
Expand Down Expand Up @@ -136,7 +142,8 @@ Json::Value Debug::debug_storageRangeAt(string const& _blockHashOrNumber, int _t
Block block = m_eth.block(blockHash(_blockHashOrNumber));

unsigned const i = ((unsigned)_txIndex < block.pending().size()) ? (unsigned)_txIndex : block.pending().size();
State state = block.fromPending(i);
State state(State::Null);
createIntermediateState(state, block, i, m_eth.blockChain());

map<h256, pair<u256, u256>> const storage(state.storage(Address(_address)));

Expand Down
5 changes: 3 additions & 2 deletions test/tools/libtesteth/BlockChainHelper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -501,8 +501,9 @@ bool TestBlockChain::addBlock(TestBlock const& _block)
Block block = (m_blockChain.get()->genesisBlock(genesisDB));
block.sync(*m_blockChain.get());

//BOOST_REQUIRE(m_lastBlock.blockHeader().hash() == BlockHeader(block.blockData()).hash());
m_lastBlock.setState(block.fromPending(10000));
State st(block.state());
st.setRoot(block.info().stateRoot());
m_lastBlock.setState(st);
return true;
}

Expand Down
18 changes: 8 additions & 10 deletions test/unittests/libethereum/Block.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,14 @@ BOOST_AUTO_TEST_CASE(bStructures)

BOOST_AUTO_TEST_CASE(bStates)
{
if (!dev::test::Options::get().quadratic)
return;
try
{
TestBlockChain testBlockchain(TestBlockChain::defaultGenesisBlock());
TestBlock const& genesisBlock = testBlockchain.testGenesis();
OverlayDB const& genesisDB = genesisBlock.state().db();
BlockChain const& blockchain = testBlockchain.interface();

State stateBofore = testBlockchain.topBlock().state();
h256 stateRootBefore = testBlockchain.topBlock().state().rootHash();

TestBlock testBlock;
TestTransaction transaction1 = TestTransaction::defaultTransaction(1);
Expand All @@ -76,15 +74,15 @@ BOOST_AUTO_TEST_CASE(bStates)

Block block2 = blockchain.genesisBlock(genesisDB);
block2.populateFromChain(blockchain, testBlock.blockHeader().hash());
State stateAfterInsert = block2.fromPending(0); //get the state of blockchain on previous block
BOOST_REQUIRE(ImportTest::compareStates(stateBofore, stateAfterInsert) == 0);
h256 stateRootAfterInsert = block2.stateRootBeforeTx(0); //get the state of blockchain on previous block
BOOST_REQUIRE_EQUAL(stateRootBefore, stateRootAfterInsert);

State stateAfterInsert1 = block2.fromPending(1); //get the state of blockchain on current block executed
BOOST_REQUIRE(ImportTest::compareStates(stateAfterInsert, stateAfterInsert1, eth::AccountMaskMap(), WhenError::DontThrow) == 1);
h256 stateRootAfterInsert1 = block2.stateRootBeforeTx(1); //get the state of blockchain on current block executed
BOOST_REQUIRE(stateRootAfterInsert != stateRootAfterInsert1);

State stateAfterInsert2 = block2.fromPending(2); //get the state of blockchain on current block executed
BOOST_REQUIRE(ImportTest::compareStates(stateBofore, stateAfterInsert2, eth::AccountMaskMap(), WhenError::DontThrow) == 1);
BOOST_REQUIRE(ImportTest::compareStates(stateAfterInsert1, stateAfterInsert2, eth::AccountMaskMap(), WhenError::DontThrow) == 1);
h256 stateRootAfterInsert2 = block2.stateRootBeforeTx(2); //get the state of blockchain on current block executed
BOOST_REQUIRE(stateRootBefore != stateRootAfterInsert2);
BOOST_REQUIRE(stateRootAfterInsert1 != stateRootAfterInsert2);

//Block2 will start a new block on top of blockchain
BOOST_REQUIRE(block1.info() == block2.info());
Expand Down