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

feat: integrate signature with boolean parity #1540

Merged
merged 24 commits into from
Nov 5, 2024
Merged

Conversation

klkvr
Copy link
Member

@klkvr klkvr commented Oct 20, 2024

Motivation

Integrates signature from alloy-rs/core#776

Depends on alloy-rs/eips#12 and alloy-rs/core#796, needs minor releases for core and eips

Solution

Changes in this PR:

  • Transaction encoding is made a bit more verbose as signature.v() is now encoded and accounted in length separately
  • Validation/adjustments of signature parity value are removed
  • Legacy transaction decoding now handles decoding of parity manually by decoding it into u64 and extracting chain_id when possible. Encoding manually encodes chain_id into boolean parity as per EIP-155 spec.
  • sign_transaction_with_chain_id macro no more adjusts the signature. as long as signer signed the correct payload with chain_id we are fine with parity being boolean, and legacy transaction encoding methods will figure out how to encode it into integer
  • signed_legacy_serde module is added which should be used for serialization of Signed<TxLegacy> to respect the specification requiring parity being serialized as EIP-155 compliant v key.

This is not perfect from our pov because we now have more boilerplate logic for parity encoding. However, this makes encoding API safer without any changes to it.

One limitation here is that Signed<TxLegacy> on its own would be serialized incorrectly with yParity key. We can't implement serialize directly on individual Signed<_> types because this would make it impossible to implement serde for Signed<_> with custom transaction. For now I've added with = "signed_legacy_serde" to envelope legacy variants which should ensure that enveloped transactions are always (de)serialized correctly

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

@klkvr klkvr force-pushed the klkvr/wip-bool-parity branch from 6e67c32 to 4ad8918 Compare October 31, 2024 12:45
@klkvr klkvr changed the title [wip] integrate signature with boolean parity feat: integrate signature with boolean parity Oct 31, 2024
@klkvr klkvr marked this pull request as ready for review October 31, 2024 19:29
@klkvr klkvr enabled auto-merge (squash) November 1, 2024 00:00
@klkvr klkvr disabled auto-merge November 1, 2024 00:52
@klkvr klkvr force-pushed the klkvr/wip-bool-parity branch 3 times, most recently from 7a56d8a to ede6173 Compare November 4, 2024 03:08
@klkvr klkvr force-pushed the klkvr/wip-bool-parity branch from ede6173 to 6408184 Compare November 5, 2024 14:07
@klkvr klkvr merged commit 2323ddb into main Nov 5, 2024
26 checks passed
@klkvr klkvr deleted the klkvr/wip-bool-parity branch November 5, 2024 14:37
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 this pull request may close these issues.

2 participants