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

Insert blob versioned hashes in signature payload for hashing #209

Merged
merged 1 commit into from
May 2, 2024

Conversation

Nashtare
Copy link
Collaborator

@Nashtare Nashtare commented May 2, 2024

We were not increasing the RLP blob length with the blob versioned hashes, which were then being ignored when processing type 3 txn hashing.

cc @wborgeaud, while looking at this, I noticed something concerning. The txn hash was invalid, but the return value of ECRECOVER wasn't U256::MAX (i.e. failure signal), but instead an arbitrary (wrong) recovered sender address.

@github-actions github-actions bot added the crate: evm_arithmetization Anything related to the evm_arithmetization crate. label May 2, 2024
@Nashtare Nashtare self-assigned this May 2, 2024
@Nashtare Nashtare added this to the Cancun - Q2 2024 milestone May 2, 2024
@wborgeaud
Copy link
Contributor

cc @wborgeaud, while looking at this, I noticed something concerning. The txn hash was invalid, but the return value of ECRECOVER wasn't U256::MAX (i.e. failure signal), but instead an arbitrary (wrong) recovered sender address.

We return U256::MAX only when the signature is invalid, which wouldn't be the case here, right?

@Nashtare
Copy link
Collaborator Author

Nashtare commented May 2, 2024

Ah yes, in this case it wouldn't trigger the flag.

Copy link
Contributor

@wborgeaud wborgeaud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Nashtare Nashtare merged commit b1d0416 into feat/cancun May 2, 2024
6 checks passed
@Nashtare Nashtare deleted the fix-type3-hashing branch May 2, 2024 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate: evm_arithmetization Anything related to the evm_arithmetization crate.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants