Skip to content
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

yParity missing from EIP-2930 and 1559 transactions. #3276

Closed
MicahZoltu opened this issue Aug 4, 2021 · 2 comments · Fixed by #3635
Closed

yParity missing from EIP-2930 and 1559 transactions. #3276

MicahZoltu opened this issue Aug 4, 2021 · 2 comments · Fixed by #3635
Assignees

Comments

@MicahZoltu
Copy link
Contributor

MicahZoltu commented Aug 4, 2021

Describe the bug

V = transaction.Type == TxType.Legacy ? (UInt256?)signature.V : (UInt256?)signature.RecoveryId;

yParity should be returned for 2930 and 1559 transactions, not v. It would not be unreasonable to continue filling in v for backward compatibility since a version has already been released with that, but transactions should be returning yParity as well.

See ethereum/execution-specs#293 for details.

Additional context
It is unfortunate that no one noticed this discrepancy prior to launch of Berlin and then London, but it is better to fix it now than to continue incorrectly populating only a v when we don't actually have a v.

@kjazgar
Copy link
Contributor

kjazgar commented Aug 10, 2021

We agree that yParity should be returned in these cases, however we checked that geth returns v as well. So the question is whether changing this would not cause any problems? Do we have consensus between all client teams?

@MicahZoltu
Copy link
Contributor Author

At the least, yParity should be returned along side v. I can appreciate also returning v for compatibility with Geth, if they put yParity into v incorrectly, but even in that case we should include a yParity field I think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants