-
Notifications
You must be signed in to change notification settings - Fork 754
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add HF consistency checks #969
Comments
From my perspective this is neither a bug nor an inconsistency but is a by-design choice. Both the VM and e.g. a block are independent entities. If you create a VM with common set to Then you can decide how to use these independent entities. If you then run this Therefore you've got these two "operation modes".
|
Gave this some thought, good points. Do you think we should add explicit checks in the VM and Block in the future? If you add Txs to a Block with the wrong common, it will throw. Same if you try to run a Block with a different Common HF in the VM? |
Haven't given this much thought yet, but - yeah - such checks might be worth to add I would say on first thought. |
Have updated the title to reflect the latest state of the discussion. |
I guess these consistency checks Jochem mentioned would still make sense. Good first issue since very local changes. |
In this specific test I noticed that I had to specifically pass on the Common object which is used by the VM in order to get the right results. I would assume that any package which is used downstream (tx/block) would use the Common of the VM in case it is ran. But this is not the case. One can create a VM which points to
Istanbul
, and then instantiate a block which usesChainstart
. This leads to the very weird situation where the EVM executes rules according toIstanbul
(e.g. gas calculations), but the block will validate the rules againstChainstart
. This sounds like a very bug-prone feature and it seems very inconsistent to me.I'd say a way to fix this is to call
setHardfork
on the tx/block before we run it and thus set the common to the same hardfork as the one the VM is using. But this would be a breaking change as now suddenly the Common of tx/block can switch forks when you run it. We could also explicitly communicate that if you want to ensure "correctness", then you need to always pass on the common of the VM in case you create a tx/block.One other nasty problem here is that in the constructor of block/tx there are some validations/calculations which explicitly depend upon the fork. In
block
it checks DAO extra data, and ifcalcDifficultyFromHeader
is set totrue
then it also calculates difficulty (which could thus change if you are on a different fork due to the difficulty bomb). Intx
it also verifies that thev
value of the signature is correct.The text was updated successfully, but these errors were encountered: