-
Notifications
You must be signed in to change notification settings - Fork 20.3k
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
internal/ethapi: support "input" in transaction args #15640
Conversation
There is still a problem that raw transactions are decoded to core.types.Transaction which uses the 'input' field name for payload. from Geth/internal/ethapi/api.go:
The current javascript tooling to construct and sign transactions uses the 'data' field: ex web3-eth-accounts:
Am I missing something? |
@arrivets Yes, I think you are.. For the rlp-decoding, it doesn't matter what field is used internally. This PR would still accept current tooliing using |
I think this PR makes sense, but instead of failing if both are non-nil, it could check if they are equal first. |
ah yes of course.. thanks @holiman |
internal/ethapi/api.go
Outdated
@@ -1093,14 +1094,23 @@ func (args *SendTxArgs) setDefaults(ctx context.Context, b Backend) error { | |||
} | |||
args.Nonce = (*hexutil.Uint64)(&nonce) | |||
} | |||
if args.Input != nil && args.Data != nil { | |||
return errors.New(`"data" and "input" are mutually exclusive`) |
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.
The error message is a bit obfuscated. It's not entirely obvious from the error message that it will fail if the user supplies both. When two entities are mutually exclusive, it does not require that one be nil.
Perhaps it makes sense to have the error message something on the lines of: "Please supply either data or input."
PTAL |
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.
LGTM (have not yet tested, only looked at the code)
The tx data field is called "input" in returned objects and "data" in argument objects. Make it so "input" can be used in args, but bail if both "input" and "data" are set. Fixes ethereum#15628
Rebased again to remove lint failure |
The tx data field is called "input" in returned objects and "data" in argument objects. Make it so "input" can be used, but bail if both are set.
The tx data field is called "input" in returned objects and "data" in argument objects. Make it so "input" can be used, but bail if both are set.
The tx data field is called "input" in returned objects and "data" in
argument objects. Make it so "input" can be used, but bail if both are
set.
Fixes #15628