ethereum / execution-specs

Specification for the Execution Layer. Tracking network upgrades.
Creative Commons Zero v1.0 Universal
810 stars 225 forks source link

2930 and 1559 transactions do not have a `v`. #814

Closed MicahZoltu closed 10 months ago

MicahZoltu commented 11 months ago

https://github.com/ethereum/execution-specs/blob/2da1ad34bb0f17bf61d41bdbad6fa8c814286dcf/src/ethereum/berlin/fork_types.py#L81 https://github.com/ethereum/execution-specs/blob/2da1ad34bb0f17bf61d41bdbad6fa8c814286dcf/src/ethereum/london/fork_types.py#L101

These should both be named yParity. v has a very well defined meaning in legacy transactions and EIP-155 transactions, but it is not mentioned anywhere in the EIP-2930 or EIP-1559 specification. What is mentioned in both of those specifications is a yParity field that sits in a similar place in the RLP structure, but it's definition doesn't match that of the v value defined in either legacy transactions or EIP-155 transactions.

Glancing at the code here, it appears you are just putting yParity into a property called v, but this will lead to a great amount of confusion since it doesn't actually match the definition of v anywhere (neither in the yellow paper nor in any EIP).

Hopefully the fix for this is as simple as a rename, but I'm not confident enough in Python being able to do rename refactoring to want to attempt a PR myself.