-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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: remove chain_id in authorization tuple #9152
base: master
Are you sure you want to change the base?
Conversation
File
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense
|
||
```python | ||
assert auth.chain_id < 2**64 | ||
assert auth.unchained == 0 || auth.unchained == 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This specific check looks like the yParity
check (where yParity
is also either 0 or 1).
assert auth.unchained == 0 || auth.unchained == 1 | |
assert auth.unchained < 2**8 |
There should be an added check for each tuple to validate if the auth.unchained
is 0 or 1 (if not, skip the tuple).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think this line should be reverted. This section has to do with decoding txs and I think 1 byte is the highest resolution we want here. I think many languages represent boolean as 1 byte value despite it only needing 1 bit of information.
We would verify unchained
is 0 or 1 in the tuple validation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From Reth side we don't think this makes full sense / is an improvement.
With this idea:
- When I'm signing, I need to sign
auth_chain_id
which I need to pull from the transaction context, depending on whether theunchained
flag is set or not. - This seems more complex for not changing any functionality, or improving security.
I understand that this is a counterproposal to U256 chainId because plausibly U256 as as an auth parameter makes the auth list big in terms of data?
I'm proposing this just because it's weird to duplicate a potentially large chainID in every authorization tuple. From an implementation point of view, the chainID is already an input to the signing function. This does not change with the PR, you still have to supply the chainID when signing an authorization. We just leave out the chainID in the encoded authorization object that goes into the transaction, and replace it with a single bit instead, which says if chainID was zero. |
Would this leave any room for replaying a chain specific authorisation tuple on a different chain? |
No, because the signed-over data still contains the chainID, and it will be verified with the chainID of the executing chain. |
This is a counterproposal to #9143. Since the chain ID is already available in the context of the transaction, there is no need to specify it again in each authorization tuple. To preserve the special case of an authorization which is valid on all chains (
chain_id == 0
), a new boolean fieldunchained
is added to the tuple in place ofchain_id
.