Support Dao Fork and other state.fork options for t8n
#737
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #737 +/- ##
==========================================
+ Coverage 82.47% 84.91% +2.44%
==========================================
Files 570 570
Lines 31568 31571 +3
==========================================
+ Hits 26036 26810 +774
+ Misses 5532 4761 -771
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 35 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
gurukamath
force-pushed
the
t8n-dao-fork
branch
2 times, most recently
from
2af23de
to
c7afa88
Compare
| if t8n.fork_module == "dao_fork": | ||
| t8n.fork.apply_dao(state) |
There was a problem hiding this comment.
Shouldn't we run apply_fork for every fork, and not just the DAO fork?
There was a problem hiding this comment.
The t8n inputs do not have enough details to re-create the entire chain which is why we decided to use the apply_body function as the point of entry for the tool. That is why I have also separated out the state changes in dao_fork into a different function apply_dao and used that instead.
There was a problem hiding this comment.
I got that, I'm just thinking generally. I'd like to avoid the situation where every state modifying fork (all one of them 🤣) needs to be mentioned explicitly in this tool. Do we need to change apply_fork so it's better suited?
There was a problem hiding this comment.
I think forks that modify the state directly (like the DAO Fork) should be looked upon as more of an exception than a rule. But we can discuss this.
There was a problem hiding this comment.
I have created an issue for this for now (see #740 ). Would like to implement the next few steps in the evm tools and re-visit this at a later point
Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com> The go-ethereum version of t8n supports various options for --state.fork which involve transition for one fork to the next at a certain block number. E.g - FrontierToHomesteadAt5. This commit adds support for these options. It also adds support for the Dao Fork.
gurukamath
force-pushed
the
t8n-dao-fork
branch
from
c7afa88
to
374e485
Compare
| @@ -12,6 +14,39 @@ | |||
|
|
|||
| W = TypeVar("W", Uint, U64, U256) | |||
|
|
|||
| EXCEPTION_MAPS = { | |||
| "FrontierToHomesteadAt5": { | |||
There was a problem hiding this comment.
this logic can be done also on testers side. if tool is transition only it does not keep blockchain data.
so the testing tool can run it with Frontier or Homestead to build a transition chain
What was wrong?
The go-ethereum version of t8n supports various options for
--state.forkwhich involve transition for one fork to the next at a certain block number. E.g -FrontierToHomesteadAt5. The t8n tool also doesn't support the DAO Fork.Related to Issue #625
How was it fixed?
This commit adds support for the transition options. It also adds support for the Dao Fork.
Cute Animal Picture