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

core/types: change SetCodeTx.ChainID to uint256 #30982

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fjl
Copy link
Contributor

@fjl fjl commented Jan 3, 2025

We still need to decide how to handle non-specfic chainId in the JSON encoding of authorizations. With chainId being a uint64, the previous implementation just used value zero. However, it might actually be more correct to use the value null for this case.

@fjl fjl requested review from lightclient and removed request for holiman and rjl493456442 January 3, 2025 13:58
@fjl fjl requested a review from s1na as a code owner January 3, 2025 14:07
core/types/tx_setcode.go Outdated Show resolved Hide resolved
@fjl
Copy link
Contributor Author

fjl commented Jan 3, 2025

Hmm. So now it's a bit inconsistent with SetCodeAuthorization having ChainID of type uint256.Int and the transaction type having a ChainID of type *uint256.Int. I think the non-pointer version is more correct since we treat the value zero as 'valid for all chains' for authorizations.

@lightclient
Copy link
Member

Why do we use *uint256.Int for other instances of chain id? I always assumed it was because the pointer is just less data for shuttle around. AFAIK the chain id is required for every tx anyways.

We still need to decide how to handle non-specfic chainId in the JSON encoding of authorizations. With chainId being a uint64, the previous implementation just used value zero. However, it might actually be more correct to use the value null for this case.

In terms of this, it seems like encoding as 0 is the right way to represent valid on all chains since that is the specified value in the EIP.

@lightclient lightclient force-pushed the core-types-7702-chainid branch from b2d69de to cd0c444 Compare January 10, 2025 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants