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

Update EIP-7702: Reorder authorization fields for authority recovery #8679

Merged
merged 1 commit into from
Jun 24, 2024

Conversation

onbjerg
Copy link
Contributor

@onbjerg onbjerg commented Jun 20, 2024

Currently items in the authorization list are ordered:

  • Chain ID
  • Address
  • (Optional) nonce

but in order to recover the authority, we need to reorder the fields to

  • Chain ID
  • (Optional) Nonce
  • Address

which in some cases necessitates a secondary type to encode the authorization for recovery. Using the same order for the fields simplifies implementation slightly.

@onbjerg onbjerg requested a review from eth-bot as a code owner June 20, 2024 18:25
@github-actions github-actions bot added c-update Modifies an existing proposal s-review This EIP is in Review t-core labels Jun 20, 2024
@onbjerg onbjerg changed the title Reorder authorization fields for authority recovery EIP-7702: Reorder authorization fields for authority recovery Jun 20, 2024
@eth-bot
Copy link
Collaborator

eth-bot commented Jun 20, 2024

✅ All reviewers have approved.

@onbjerg onbjerg changed the title EIP-7702: Reorder authorization fields for authority recovery Update EIP-7702: Reorder authorization fields for authority recovery Jun 20, 2024
@eth-bot eth-bot added the a-review Waiting on author to review label Jun 20, 2024
@lightclient
Copy link
Member

This ordering was my mistake, they should be ordered the same throughout. I would like to know from other teams if it's okay to update this for devnet-1? Or will we just update this for future devnets?

@onbjerg
Copy link
Contributor Author

onbjerg commented Jun 20, 2024

Speaking for reth we're ok with changing it for devnet-1:)

@Scamreno
Copy link

#8682 #6976 #8546

@sudeepdino008
Copy link
Contributor

This ordering was my mistake, they should be ordered the same throughout. I would like to know from other teams if it's okay to update this for devnet-1? Or will we just update this for future devnets?

Erigon is okay too.

@ak88
Copy link

ak88 commented Jun 24, 2024

This ordering was my mistake, they should be ordered the same throughout. I would like to know from other teams if it's okay to update this for devnet-1? Or will we just update this for future devnets?

Nethermind is okay with this too

@eth-bot eth-bot enabled auto-merge (squash) June 24, 2024 17:01
Copy link
Collaborator

@eth-bot eth-bot left a comment

Choose a reason for hiding this comment

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

All Reviewers Have Approved; Performing Automatic Merge...

@eth-bot eth-bot merged commit 5c2aedb into ethereum:master Jun 24, 2024
14 of 17 checks passed
onbjerg added a commit to alloy-rs/alloy that referenced this pull request Jul 1, 2024
onbjerg added a commit to alloy-rs/alloy that referenced this pull request Jul 1, 2024
ben186 pushed a commit to ben186/alloy that referenced this pull request Jul 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-review Waiting on author to review c-update Modifies an existing proposal s-review This EIP is in Review t-core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants