ethereumjs-monorepo
ethereumjs-monorepo copied to clipboard
ethereumjs
Add option to disable EIPs
In Common, it would be nice to add an option to disable certain EIPs. The function signature would be disableEIP(eip: number) and disableEIPs(eips: number[]). It is not entirely trivial to just disable them. We should check if other activated EIPs at the current moment depend on the disabled EIP. If that's the case, throw and do not disable any. The user should thus carefully ensure that the list of activated EIPs is in the right order: each time an EIP is deleted, it should at that moment have no dependants.
For instance, if EIP B depends upon EIP A, then disableEIPs([A, B]) would throw, but disableEIPs([B, A]) should not.
Instead of additional methods in common I would rather think this should be added to the common constructor opts, so in addition to the existing eips key we would also add disabledEIPs. I don't think anyone should be disabling EIPs on the fly after the common is already constructed :)
Some of the EIP json definition files in common have a requiredEIPs field, if we ensure they are reliably filled out we can possibly use that for the scenario where other EIPs might depend on a deactivated one.
Yep, you are right, this should indeed go in the constructor :smile:
We have setEIPs though which means that if I have a Common I can add EIPs - might want to reconsider that at some point? I think it is more clean to have a frozen Common? Especially when situations change, such as added EIPs then the VM should update the opcodes (which we do). Might want to freeze Common forcing users to re-instantiate anything which uses Common if it changes (?). But that is rather cumbersome though...
We have setEIPs though which means that if I have a Common I can add EIPs - might want to reconsider that at some point? I think it is more clean to have a frozen Common? Especially when situations change, such as added EIPs then the VM should update the opcodes (which we do). Might want to freeze Common forcing users to re-instantiate anything which uses Common if it changes (?). But that is rather cumbersome though...
Yes - also just wrote a similar suggestion in the chat - I guess this method is not really used anywhere anyhow and I would fully agree with your frozen-Common argumentation.
High level note for eventual implementers: please do not start this task without additional confirmation and/or discussion.