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

ethclient: use 'input', not 'data' as field for transaction input #28078

Merged
merged 1 commit into from
Sep 8, 2023

Conversation

holiman
Copy link
Contributor

@holiman holiman commented Sep 8, 2023

Closes #28077

This makes ethclient / gethclient use input instead of data for passing input with a transaction. input is the preferred way.

See #15628, and https://github.com/ethereum/go-ethereum/blob/master/signer/core/apitypes/types.go#L93

@holiman holiman requested a review from fjl September 8, 2023 08:34
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

@smartprogrammer93
Copy link

Just a heads up, guys. This will break with the current implementation of prysm since they use eth_call with the data field currently.

@fjl fjl merged commit 5cf53f5 into ethereum:master Sep 8, 2023
1 check passed
@indanielo indanielo mentioned this pull request Sep 26, 2023
schmir added a commit to shutter-network/rolling-shutter that referenced this pull request Oct 5, 2023
Unfortunately we cannot use v1.13.0, as that breaks our tests. See
ethereum/go-ethereum#28078 and
NomicFoundation/hardhat#4438
@holiman holiman deleted the ethclient_input branch October 11, 2023 07:24
@peterbitfly
Copy link
Contributor

FYI, this breaks calling contracts with ethclient against an erigon node, as erigon still expects the data to be passed in the data field.

@holiman
Copy link
Contributor Author

holiman commented Oct 11, 2023

as erigon still expects the data to be passed in the data field.

That is very odd, https://github.com/ledgerwatch/erigon/blob/devel/core/types/transaction_marshalling.go#L29

	Data     *hexutility.Bytes  `json:"input"`

Are you quite certain?

@peterbitfly
Copy link
Contributor

peterbitfly commented Oct 11, 2023

Not sure if their RPC implementation uses a different decoding routine. But our tests against a node show that behavior: erigontech/erigon#8437

Edit: I believe erigon uses the following definition in this case https://github.com/ledgerwatch/erigon/blob/fe263ae02e08df8119cae7eaa54e49e75141762b/interfaces.go#L118

@holiman
Copy link
Contributor Author

holiman commented Oct 11, 2023

That strange too, because we have the same struct: https://github.com/ethereum/go-ethereum/blob/master/interfaces.go#L142

@holiman
Copy link
Contributor Author

holiman commented Oct 11, 2023

Hm, looking closer, this PR description talks about "transaction input", but in fact it addresses a parameter used in eth_call, not (directly) related to the transaction type. So maybe the spec (https://github.com/ethereum/execution-apis/blob/main/src/schemas/transaction.yaml#L25) posted in #28077 does not concern eth_call at all.

?

@fjl
Copy link
Contributor

fjl commented Oct 11, 2023

The spec object is for both. I think what happened in erigon might be, they used CallMsg instead of our TransactionArgs to represent the input. The ethereum.CallArgs type does not have an UnmarshalJSON method, it's just a Go struct containing the call parameters. We have custom serialization code for it in ethclient.

@holiman
Copy link
Contributor Author

holiman commented Oct 12, 2023

https://hivetests2.ethdevops.io/suite.html?suiteid=1697083845-025eee594dcbf6ac1c1aea97aa153fe3.json&suitename=rpc#test-2

>>  {"jsonrpc":"2.0","id":1,"method":"eth_call","params":[{"from":"0x0000000000000000000000000000000000000000","input":"0xa223e05d","to":"0x0000000000000000000000000000000000000314"},"pending"]}
<<  {"jsonrpc":"2.0","id":1,"result":"0x0000000000000000000000000000000000000000000000000000000000001234"}
>>  {"jsonrpc":"2.0","id":2,"method":"eth_call","params":[{"from":"0x0000000000000000000000000000000000000000","input":"0xabd1a0cf000000000000000000000000391694e7e0b0cce554cb130d723a9d27458f9298","to":"0x0000000000000000000000000000000000000314"},"pending"]}
<<  {"jsonrpc":"2.0","id":2,"result":"0x0000000000000000000000000000000000000000000000000000000000000001"}
>>  {"jsonrpc":"2.0","id":3,"method":"eth_call","params":[{"from":"0x0000000000000000000000000000000000000000","input":"0xe6768b45000000000000000000000000000000000000000000000000000000000000045700000000000000000000000000000000000000000000000000000000000008ae0000000000000000000000000000000000000000000000000000000000000d05","to":"0x0000000000000000000000000000000000000314"},"pending"]}
<<  {"jsonrpc":"2.0","id":3,"result":"0x000000000000000000000000000000000000000000000000000000000000045700000000000000000000000000000000000000000000000000000000000008ae0000000000000000000000000000000000000000000000000000000000000d05"}

The hive-tests indicate that erigon does support input. cc @AskAlexSharov any thoughts?

devopsbo3 pushed a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
raulk added a commit to filecoin-project/lotus that referenced this pull request Dec 1, 2023
The correct name for this field is 'input' according to the Ethereum specs [0].
However, for the longest time, clients have been using 'data' and servers have been
lenient to accept both, preferring 'input' over 'data' when both appear.

Our lack of support for 'input' had gone unnoticed until go-ethereum decided
to adjust their ethclient implementation to issue eth_call and eth_estimateGas
requests with the 'input' field instead of 'data' [1]. This suddenly broke apps
using this client against Lotus' Eth API.

[0]: https://github.com/ethereum/execution-apis/blob/main/src/schemas/transaction.yaml#L33-L35
[1]: ethereum/go-ethereum#28078
Stebalien added a commit to filecoin-project/lotus that referenced this pull request Dec 9, 2023
The correct name for this field is 'input' according to the Ethereum specs [0].
However, for the longest time, clients have been using 'data' and servers have been
lenient to accept both, preferring 'input' over 'data' when both appear.

Our lack of support for 'input' had gone unnoticed until go-ethereum decided
to adjust their ethclient implementation to issue eth_call and eth_estimateGas
requests with the 'input' field instead of 'data' [1]. This suddenly broke apps
using this client against Lotus' Eth API.

[0]: https://github.com/ethereum/execution-apis/blob/main/src/schemas/transaction.yaml#L33-L35
[1]: ethereum/go-ethereum#28078

Co-authored-by: raulk <[email protected]>
Stebalien added a commit to filecoin-project/lotus that referenced this pull request Jan 6, 2024
…#11505)

The correct name for this field is 'input' according to the Ethereum specs [0].
However, for the longest time, clients have been using 'data' and servers have been
lenient to accept both, preferring 'input' over 'data' when both appear.

Our lack of support for 'input' had gone unnoticed until go-ethereum decided
to adjust their ethclient implementation to issue eth_call and eth_estimateGas
requests with the 'input' field instead of 'data' [1]. This suddenly broke apps
using this client against Lotus' Eth API.

[0]: https://github.com/ethereum/execution-apis/blob/main/src/schemas/transaction.yaml#L33-L35
[1]: ethereum/go-ethereum#28078

Co-authored-by: raulk <[email protected]>
justin-crypted added a commit to justin-crypted/go-ethereum that referenced this pull request Jan 30, 2024
justin-crypted added a commit to kstadium/go-ethereum that referenced this pull request Jan 30, 2024
dumikau pushed a commit to protofire/lotus that referenced this pull request Feb 22, 2024
The correct name for this field is 'input' according to the Ethereum specs [0].
However, for the longest time, clients have been using 'data' and servers have been
lenient to accept both, preferring 'input' over 'data' when both appear.

Our lack of support for 'input' had gone unnoticed until go-ethereum decided
to adjust their ethclient implementation to issue eth_call and eth_estimateGas
requests with the 'input' field instead of 'data' [1]. This suddenly broke apps
using this client against Lotus' Eth API.

[0]: https://github.com/ethereum/execution-apis/blob/main/src/schemas/transaction.yaml#L33-L35
[1]: ethereum/go-ethereum#28078

Co-authored-by: raulk <[email protected]>
dumikau pushed a commit to protofire/lotus that referenced this pull request Feb 22, 2024
The correct name for this field is 'input' according to the Ethereum specs [0].
However, for the longest time, clients have been using 'data' and servers have been
lenient to accept both, preferring 'input' over 'data' when both appear.

Our lack of support for 'input' had gone unnoticed until go-ethereum decided
to adjust their ethclient implementation to issue eth_call and eth_estimateGas
requests with the 'input' field instead of 'data' [1]. This suddenly broke apps
using this client against Lotus' Eth API.

[0]: https://github.com/ethereum/execution-apis/blob/main/src/schemas/transaction.yaml#L33-L35
[1]: ethereum/go-ethereum#28078

Co-authored-by: raulk <[email protected]>
@fengzie
Copy link

fengzie commented Mar 28, 2024

It caused me more than a half day for troubleshooting a chain about "execution reverted" issue. The go-ethereum v1.13.8 uses "input" in eth_call instead of "data".

Dominator008 added a commit to celer-network/endpoint-proxy that referenced this pull request Sep 25, 2024
ethereum/go-ethereum#28078 changed geth to use
"input" instead of "data", which causes issues with multiple chain RPC
endpoints. Temporarily revert that for those chains.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

eth_call sent with field not in the API specification
6 participants