[ECTM-212] Fix block preparation (taking into account EIP-161)#741
[ECTM-212] Fix block preparation (taking into account EIP-161)#741
Conversation
KonradStaniec
left a comment
KonradStaniec
left a comment
There was a problem hiding this comment.
As we touched such general config and consensus maybe you cold run ets tests and report if every thing is fine ?
Otherwise lgtm.
| blockchainConfig.eip160BlockNumber -> (4, PostEIP160ConfigBuilder), | ||
| blockchainConfig.eip161BlockNumber -> (5, PostEIP161ConfigBuilder), | ||
| blockchainConfig.byzantiumBlockNumber -> (6, ByzantiumConfigBuilder), | ||
| blockchainConfig.atlantisBlockNumber -> (6, AtlantisConfigBuilder), |
There was a problem hiding this comment.
As i understand that it is also not 100% deterministic as in case of same block and priority the chooses fork will be random one. Maybe leave some comment about it and create and leave some ticket to refactor our blockchain configs ? (we have probably been postponing it long enough)
There was a problem hiding this comment.
Not random but first checked. So in our test config, when we set all numbers to 0 expecting to use the newest but we are getting the first in the map (so it is the oldest fork)
| @@ -456,18 +473,20 @@ class BlockchainImpl( | |||
| ) | |||
|
|
|||
| //FIXME Maybe we can use this one in regular execution too and persist underlying storage when block execution is successful | |||
There was a problem hiding this comment.
maybe create ticket for this FIXME ? as it seems having two separate methods to retrieve execution world was a bit error prone
|
@KonradStaniec I will run ETS and let you know |
|
After margin [ETCM-141] scalafmt . This PR can be updated by git merge |
4 participants
Description
Fix block generation.
BlockPreparator didn't work correctly in case of an empty account (with value == 0). The change was introduced in EIP-161