Add state.reward processing #558
Conversation
rodiazet force-pushed the t8n-state-reward branch from 390393f to 3337517 Compare 
Codecov Report
Additional details and impacted files@@ Coverage Diff @@ ## master #558 +/- ## ========================================== + Coverage 97.00% 97.03% +0.03% ========================================== Files 66 66 Lines 6134 6134 ========================================== + Hits 5950 5952 +2 + Misses 184 182 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
test/t8n/t8n.cpp Outdated
| state.touch(block.coinbase); | ||
| else if (state_reward == std::numeric_limits<intx::uint256>::max()) | ||
| { | ||
| if (state.get_or_insert(block.coinbase).is_empty()) |
There was a problem hiding this comment.
Hm, cleaning up empty accounts is just an EIP-161 rule. Don't we have that implemented?
There was a problem hiding this comment.
For <= EIP158 coinbase account is touched even it tx is rejected. retesteth passing -1 as a stare.reward requires zero state to be removed.
test/t8n/t8n.cpp Outdated
| else | ||
| state.get_or_insert(block.coinbase).balance += *block_reward; | ||
| } | ||
| else if (rev <= EVMC_TANGERINE_WHISTLE) |
There was a problem hiding this comment.
Don't we do the same in transaction execution?
There was a problem hiding this comment.
no, we are touching there here we are erasing.
rodiazet force-pushed the t8n-state-reward branch from 0d06cfd to 6dae3ed Compare test/t8n/t8n.cpp Outdated
| if (block_reward.has_value()) | ||
| state.touch(block.coinbase).balance += *block_reward; | ||
| else if (rev <= EVMC_TANGERINE_WHISTLE) // This behaviour is required by retesteth | ||
| state.get_accounts().erase(block.coinbase); |
There was a problem hiding this comment.
Hm, this will remove it even when other transactions touched this account. Is that desired?
There was a problem hiding this comment.
Sorry. For some reason I removed also checking if it's empty. Fixed now
There was a problem hiding this comment.
I'm still not convinced by this quirk. It looks it replicates a bug in some client implementation. Can you provide a list of failing tests?
rodiazet force-pushed the t8n-state-reward branch from 6dae3ed to 677692e Compare test/t8n/t8n.cpp Outdated
| else if (arg == "--output.alloc" && ++i < argc) | ||
| output_alloc_file = argv[i]; | ||
| else if (arg == "--state.reward" && ++i < argc) | ||
| if (std::string_view(argv[i]) != "-1") |
There was a problem hiding this comment.
You have to combine all 3 conditions. You can be fancy by doing argv[i] != "-1"sv.
There was a problem hiding this comment.
error: no matching literal operator for call to 'operator""sv' with arguments of types 'const char *' and 'unsigned long', and no matching literal operator template
else if (arg == "--state.reward" && ++i < argc && argv[i] != "-1"sv)
test/t8n/t8n.cpp Outdated
| if (block_reward.has_value()) | ||
| state.touch(block.coinbase).balance += *block_reward; | ||
| else if (rev <= EVMC_TANGERINE_WHISTLE) // This behaviour is required by retesteth | ||
| state.get_accounts().erase(block.coinbase); |
There was a problem hiding this comment.
I'm still not convinced by this quirk. It looks it replicates a bug in some client implementation. Can you provide a list of failing tests?
rodiazet force-pushed the t8n-state-reward branch from 677692e to 02de26a Compare 
No description provided.