Skip to content

internal/ethapi: support "input" in transaction args#15640

Merged
fjl merged 1 commit into
ethereum:masterfrom
fjl:ethapi-tx-input
Dec 18, 2017
Merged

internal/ethapi: support "input" in transaction args#15640
fjl merged 1 commit into
ethereum:masterfrom
fjl:ethapi-tx-input

Conversation

@fjl
Copy link
Copy Markdown
Contributor

@fjl fjl commented Dec 9, 2017

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

@fjl fjl force-pushed the ethapi-tx-input branch from c201473 to 83ed325 Compare December 9, 2017 23:05
@fjl fjl changed the title internal/ethapi: support "input" in transaction objects internal/ethapi: support "input" in transaction args Dec 9, 2017
@arrivets
Copy link
Copy Markdown

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:

func (s *PublicTransactionPoolAPI) SendRawTransaction(ctx context.Context, encodedTx hexutil.Bytes) (string, error) {
	tx := new(types.Transaction)
	if err := rlp.DecodeBytes(encodedTx, tx); err != nil {
		return "", err
	}

The current javascript tooling to construct and sign transactions uses the 'data' field:

ex web3-eth-accounts:

Accounts.prototype.signTransaction = function signTransaction(tx, privateKey, callback) {
    var _this = this;

    function signed (tx) {

        if (!tx.gas && !tx.gasLimit) {
            throw new Error('"gas" is missing');
        }

        var transaction = {
            nonce: utils.numberToHex(tx.nonce),
            to: tx.to ? helpers.formatters.inputAddressFormatter(tx.to) : '0x',
            data: tx.data || '0x',
            value: tx.value ? utils.numberToHex(tx.value) : "0x",
            gas: utils.numberToHex(tx.gasLimit || tx.gas),
            gasPrice: utils.numberToHex(tx.gasPrice),
            chainId: utils.numberToHex(tx.chainId)
        };

Am I missing something?

@holiman
Copy link
Copy Markdown
Contributor

holiman commented Dec 12, 2017

@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 data in the supplied json object.

@holiman
Copy link
Copy Markdown
Contributor

holiman commented Dec 12, 2017

I think this PR makes sense, but instead of failing if both are non-nil, it could check if they are equal first.

@arrivets
Copy link
Copy Markdown

ah yes of course.. thanks @holiman

Comment thread internal/ethapi/api.go Outdated
Copy link
Copy Markdown

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."

@fjl
Copy link
Copy Markdown
Contributor Author

fjl commented Dec 12, 2017

PTAL

Copy link
Copy Markdown
Contributor

@holiman holiman left a 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
@fjl
Copy link
Copy Markdown
Contributor Author

fjl commented Dec 14, 2017

Rebased again to remove lint failure

@fjl fjl merged commit 8c33ac1 into ethereum:master Dec 18, 2017
@karalabe karalabe added this to the 1.8.0 milestone Dec 20, 2017
b00ris pushed a commit to b00ris/go-ethereum that referenced this pull request Jan 19, 2018
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.
mariameda pushed a commit to NiluPlatform/go-nilu that referenced this pull request Aug 23, 2018
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.
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.

5 participants