Update EIP-7702: Add decoding limits for AuthList items #8746
Conversation
|
✅ All reviewers have approved. |
eth-bot
changed the title
| @@ -43,6 +43,14 @@ rlp([chain_id, nonce, max_priority_fee_per_gas, max_fee_per_gas, gas_limit, dest | |||
| authorization_list = [[chain_id, address, nonce, y_parity, r, s], ...] | |||
| ``` | |||
|
|
|||
| Transaction is considered invalid if authorization list items can't be decoded as: | |||
There was a problem hiding this comment.
Invalid, as in should be rejected by txpool? (If yes I think this should be clarified - and I also think this should be the case 😄 )
There was a problem hiding this comment.
It's invalid as it can't be added to the block. And if invalid tx is found in the block, the block is considered invalid.
Valid/Invalid should be the correct term used to spec out this.
It is assumed that txpool rejects invalid transactions.
There was a problem hiding this comment.
Alright, yes sorry about this, I sometimes get confused about what terminology maps to what, since "invalid" could also be interpreted as the tx itself is valid but will be reverted immediately. (It would be great at some point to have a glossary EIP of the terminology we use to avoid confusion :) )
There was a problem hiding this comment.
(It would be great at some point to have a glossary EIP of the terminology we use to avoid confusion :) )
true, this would be helpful
EIPS/eip-7702.md
Outdated
| @@ -43,6 +43,14 @@ rlp([chain_id, nonce, max_priority_fee_per_gas, max_fee_per_gas, gas_limit, dest | |||
| authorization_list = [[chain_id, address, nonce, y_parity, r, s], ...] | |||
| ``` | |||
|
|
|||
| Transaction is considered invalid if authorization list items can't be decoded as: | |||
|
|
|||
| * `chain_id` and `nonce`: unsigned 64-bit integer. | |||
There was a problem hiding this comment.
chain_id should be a uint256
There was a problem hiding this comment.
Why should it be uint256?
there are no chain IDs that are bigger than u64: https://chainid.network/
There was a problem hiding this comment.
Both reth and alloy uses u64 for ChainId
There was a problem hiding this comment.
It hasn't been specified as a uint64 anywhere. To be fair, an explicit bound hasn't been given afaict. But in these cases we basically always assume it is bound by uint256. Reth /alloy can definitely impl as uint64 if you prefer but I wouldn't put that in the spec.
There was a problem hiding this comment.
To be fair, an explicit bound hasn't been given afaict true, but imo it is always better to use a native number than u256, if the size is not the problem and u64 is quite big.
We can't use u64 for AuthList, we would fail to decode tx in that way. Mentioned Alloy and Reth as popular libs that didn't have a problem with chain-Id as u64.
Not that big of a problem to change this field to u256, but for ChainId would prefer if we talk about limiting it to u64.
There was a problem hiding this comment.
I don't mind limiting the chain id, but I would want that to be a separate proposal that limits the chain id across all instances in the protocol at one time.
There was a problem hiding this comment.
It seems people responded that they want chain_id U256, so will make a change for that.
github-actions
bot
added
c-update
EIPS/eip-7702.md
Outdated
| * `nonce`: unsigned 64-bit integer. | ||
| * `address`: 20 bytes array. | ||
| * `y_parity`: Value 0 or 1. | ||
| * `r`: 32 bytes array. |
There was a problem hiding this comment.
I believe this should also be an unsigned 256-bit integer for RLP purposes?
There was a problem hiding this comment.
@lightclient it is updated
Fields inside the Authorization List are accessed (validated) differently than items inside the Transaction. A very big nonce will invalidate the transaction while if the nonce is big in AuthorizationList (By current spec) tx will still be valid. This makes decoding a special case.
Introducing expected limits on items invalidates this edge case.