Skip to content

ethclient: allow passing AuthorizationList to calls #31198

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Apr 23, 2025
Merged

ethclient: allow passing AuthorizationList to calls #31198

merged 7 commits into from
Apr 23, 2025
+62 −3

Conversation

colinlyguo
Copy link
Contributor

@colinlyguo colinlyguo commented Feb 17, 2025 •
edited
Loading

This PR adds the AuthList field in toCallArg to support eth_call and eth_estimateGas of SetCodeTx.

Some unit tests are added in this PR (containing uncovered cases, e.g., ErrSetCodeTxCreate).

@fjl
Copy link
Contributor

fjl commented Feb 17, 2025

We have been discussing this. At the moment, it doesn't feel very useful since you need a signed authorization in order to invoke the call. It's not super convenient. And it's not really necessary to override an account with code via EIP-7702 in eth_call because it already supports arbitrary state overrides in Geth. And the new method eth_simulateV1 supports overrides in all clients.

@colinlyguo
Copy link
Contributor Author

colinlyguo commented Feb 17, 2025 •
edited
Loading

We have been discussing this. At the moment, it doesn't feel very useful since you need a signed authorization in order to invoke the call. It's not super convenient. And it's not really necessary to override an account with code via EIP-7702 in eth_call because it already supports arbitrary state overrides in Geth. And the new method eth_simulateV1 supports overrides in all clients.

aha. I see and thanks. IMHO the main intention for this PR is not to overwrite accounts but to return an accurate gas estimation. e.g. bundlers may need to estimate gas (if the bundle transactions have some delegation submissions. I learned this from here. Though they might be written in other languages, too). In the meantime, it doesn't make it worse to support AuthList here, haven't thought of any side effects.

btw, do you think the added unit tests are useful here? might tweak some if duplicated.

@s1na s1na changed the title client: add AuthList field in toCallArg ethclient: add AuthList field in toCallArg Feb 18, 2025
s1na
s1na reviewed Feb 18, 2025
View reviewed changes
@colinlyguo colinlyguo requested a review from s1na February 24, 2025 12:20
@colinlyguo
Copy link
Contributor Author

added some small fixes, pls take a look again. cc @fjl @s1na

@buddh0
Copy link
Contributor

buddh0 commented Mar 7, 2025

I think it's an useful feature

@colinlyguo
Copy link
Contributor Author

colinlyguo commented Mar 14, 2025 •
edited
Loading

Asking for reviews (maybe further updates are needed or some things are unnecessary?). @s1na @fjl

@clabby
Copy link

clabby commented Mar 25, 2025

What's the status here? This is useful to have - we've resorted to a shim above eth_estimateGas to manually include auth list costs in the meantime.

@colinlyguo
Copy link
Contributor Author

bump. @s1na @fjl

s1na
s1na reviewed Apr 9, 2025
View reviewed changes
colinlyguo
colinlyguo commented Apr 16, 2025
View reviewed changes
s1na
s1na previously approved these changes Apr 17, 2025
View reviewed changes
Copy link
Contributor

@s1na s1na left a comment

Choose a reason for hiding this comment

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

LGTM

@s1na
Copy link
Contributor

s1na commented Apr 23, 2025

Renamed the field to AuthorizationList to keep it consistent with the API field.

s1na
s1na approved these changes Apr 23, 2025
View reviewed changes
Copy link
Contributor

@s1na s1na left a comment

Choose a reason for hiding this comment

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

LGTM

@s1na s1na added this to the 1.15.10 milestone Apr 23, 2025
@s1na s1na changed the title ethclient: add AuthList field in toCallArg ethclient: allow passing AuthorizationList to calls Apr 23, 2025
@s1na s1na merged commit 846c14e into ethereum:master Apr 23, 2025
4 checks passed
@buddh0 buddh0 mentioned this pull request Apr 24, 2025
Merged
0g-wh pushed a commit to 0g-wh/0g-geth that referenced this pull request May 8, 2025
@colinlyguo @s1na @0g-wh
ethclient: allow passing AuthorizationList to calls (ethereum#31198)
8bfd96d 
This PR adds the `AuthorizationList` field to the `CallMsg` interface to support `eth_call`
and `eth_estimateGas` of set-code transactions.

---------

Co-authored-by: Sina Mahmoodi <[email protected]>
Rampex1 pushed a commit to streamingfast/go-ethereum that referenced this pull request May 15, 2025
@colinlyguo @s1na @Rampex1
ethclient: allow passing AuthorizationList to calls (ethereum#31198)
58e35c4 
This PR adds the `AuthorizationList` field to the `CallMsg` interface to support `eth_call`
and `eth_estimateGas` of set-code transactions.

---------

Co-authored-by: Sina Mahmoodi <[email protected]>
sduchesneau pushed a commit to streamingfast/go-ethereum that referenced this pull request May 22, 2025
@buddh0 @colinlyguo @s1na
ethclient: allow passing AuthorizationList to calls (ethereum#31198) (e…
9e5d766 
…thereum#3048)

This PR adds the `AuthorizationList` field to the `CallMsg` interface to support `eth_call`
and `eth_estimateGas` of set-code transactions.

---------

Co-authored-by: colin <[email protected]>
Co-authored-by: Sina Mahmoodi <[email protected]>
@colinlyguo colinlyguo deleted the client-add-AuthList-field-in-toCallArg branch June 7, 2025 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@s1na s1na s1na approved these changes

@fjl fjl Awaiting requested review from fjl fjl is a code owner

@lightclient lightclient Awaiting requested review from lightclient lightclient is a code owner

@holiman holiman Awaiting requested review from holiman

@rjl493456442 rjl493456442 Awaiting requested review from rjl493456442 rjl493456442 is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.15.10
Development

Successfully merging this pull request may close these issues.
5 participants
@colinlyguo @fjl @buddh0 @clabby @s1na