testeth touch coinbase in state tests #5024
Conversation
test/tools/libtesteth/ImportTest.cpp
Outdated
| @@ -266,6 +266,10 @@ std::tuple<eth::State, ImportTest::ExecOutput, eth::ChangeLog> ImportTest::execu | |||
| try | |||
| { | |||
| unique_ptr<SealEngineFace> se(ChainParams(genesisInfo(_sealEngineNetwork)).createSealEngine()); | |||
| bool removeEmptyAccounts = m_envInfo->number() >= se->chainParams().EIP158ForkBlock; | |||
| if (!removeEmptyAccounts) | |||
| initialState.addBalance(_env.author(), 0); // touch coinbase no matter what | |||
There was a problem hiding this comment.
Why should the coinbase be touched both before and after the transaction?
test/tools/libtesteth/ImportTest.cpp
Outdated
| { | ||
| // Touch here bacuse coinbase might be suicided above | ||
| initialState.addBalance(_env.author(), 0); // touch coinbase no matter what | ||
| initialState.commit(removeEmptyAccounts ? State::CommitBehaviour::RemoveEmptyAccounts : |
There was a problem hiding this comment.
What if we move this out of the if body? Maybe we can skip the first initialState.commit?
There was a problem hiding this comment.
no. the first commit apply suicide instruction to coinbase. making it dissappear from the post state.
There was a problem hiding this comment.
thus simulating touch and commiting the state again.
test/tools/libtesteth/ImportTest.cpp
Outdated
| @@ -292,7 +291,7 @@ std::tuple<eth::State, ImportTest::ExecOutput, eth::ChangeLog> ImportTest::execu | |||
| if (!removeEmptyAccounts) | |||
| { | |||
| // Touch here bacuse coinbase might be suicided above | |||
| initialState.addBalance(_env.author(), 0); // touch coinbase no matter what | |||
| initialState.addBalance(_env.author(), 0); // imitate mining reward | |||
There was a problem hiding this comment.
Why is this done again? Before, this is a line
initialState.addBalance(_env.author(), 0); // imitate mining reward
There was a problem hiding this comment.
because suicide coibase will delete coinbase account from the state. we need to touch it again after suicide.
Codecov Report
@@ Coverage Diff @@
## develop #5024 +/- ##
===========================================
- Coverage 60.74% 60.56% -0.18%
===========================================
Files 351 351
Lines 28375 28397 +22
Branches 2873 2867 -6
===========================================
- Hits 17235 17200 -35
- Misses 10202 10243 +41
- Partials 938 954 +16 |
test/tools/libtesteth/ImportTest.cpp
Outdated
| { | ||
| // Touch here bacuse coinbase might be suicided above | ||
| initialState.addBalance(_env.author(), 0); // imitate mining reward | ||
| initialState.commit(removeEmptyAccounts ? State::CommitBehaviour::RemoveEmptyAccounts : |
There was a problem hiding this comment.
What happens if you move this line down out of the if body?
There was a problem hiding this comment.
ah, yes it could be simplified. nothing really changes. I just thought that it is not neccessary to perform this step after EIP158
fix testeth to pass state tests as retesteth ethereum/tests#437