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

[Bug] encode_with_signature_fields encodes an invalid transaction when the signature Parity is unexpected #1510

Closed
kayabaNerve opened this issue Oct 17, 2024 · 0 comments · Fixed by #1496
Assignees
Labels
bug Something isn't working

Comments

@kayabaNerve
Copy link
Contributor

Component

consensus, eips, genesis

What version of Alloy are you on?

84dfc7a

Operating System

None

Describe the bug

TxLegacy::encode_with_signature_fields with a Signature constructed with From<Signature<Secp256k1>>::from used to work in my setup until that (its own bug) was patched with #1305. #1428 corrected this for such signatures when TxLegacy::into_signed is used yet encode_with_signature_fields remains an issue.

TxLegacy::encode_with_signature_fields should either return an error when an incorrect parity is passed or also coerce it as into_signed did. I'm happy to implement either solution once informed which would be preferable. I'd assume (and personally recommend) coercion. The other transaction types may have similar issues but I have yet to look.

I'll also note I initially raised/misdiagnosed this in foundry-rs/foundry#9046.

@kayabaNerve kayabaNerve added the bug Something isn't working label Oct 17, 2024
kayabaNerve added a commit to serai-dex/serai that referenced this issue Oct 17, 2024
Mainly corrects for alloy-rs/alloy#1510 yet also
corrects a missing machete ignore.
@prestwich prestwich linked a pull request Oct 19, 2024 that will close this issue
3 tasks
mattsse pushed a commit that referenced this issue Oct 28, 2024
* fix: make a sensible encoding api

* fix: k256 test

* fix: single doc hidden

* fix: missing doc

* nit: undo changed list

* feat: optimize hash calculation

* fix: chain rlp_decode_signed

* fix: restore remaining check

* refactor: helper trait

* feat: add API to Signed for convenience

* refactor: add header shortcut functions

* feat: impl for some other tx types

* refctor: impl RlpEcdsaTx for TxLegacy

* lint: clippy

* fix: import vec

* fix: import Vec

* fix: rebase artifacts

* fix: correct handling of legacy signatures when encoding (#1510)

* lint: clippy

* fix: use corrected impls

* nit: remove bug comments

* refactor: use length_with_payload

* fix: legacy network headers are transparent

* fix: restore signature v check

* fix: check for leftoverbytes

* fix: corrected RLP decoding of eip4844 variant

* fix: list check in 4844 decoding

* fix: add map or
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants