diff --git a/CHANGELOG.md b/CHANGELOG.md index 5fcf9a55d50..40d41785d22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -450,3 +450,5 @@ Released with 1.0.0-beta.37 code base. ## [Unreleased] ## [1.5.4] +### Changed +- Not considering `tx.chainId` if `tx.common.customChain.chainId` is provided for `web3.eth.accounts.signTransaction` function (#4293) diff --git a/docs/web3-eth-accounts.rst b/docs/web3-eth-accounts.rst index c98cd8ee124..2cb66760495 100644 --- a/docs/web3-eth-accounts.rst +++ b/docs/web3-eth-accounts.rst @@ -151,7 +151,7 @@ Parameters 1. ``tx`` - ``Object``: The transaction object as follows: - ``nonce`` - ``String``: (optional) The nonce to use when signing this transaction. Default will use :ref:`web3.eth.getTransactionCount() `. - - ``chainId`` - ``String``: (optional) The chain id to use when signing this transaction. Default will use :ref:`web3.eth.net.getId() `. + - ``chainId`` - ``String``: (optional) The chain id to use when signing this transaction. Default will use :ref:`web3.eth.net.getId() `. Web3 will ignore this field if `common.customChain.chainId` is provided. - ``to`` - ``String``: (optional) The recevier of the transaction, can be empty when deploying a contract. - ``data`` - ``String``: (optional) The call data of the transaction, can be empty for simple value transfers. - ``value`` - ``String``: (optional) The value of the transaction in wei. diff --git a/packages/web3-eth-accounts/src/index.js b/packages/web3-eth-accounts/src/index.js index f3fbf7ad5da..c86d6ee5423 100644 --- a/packages/web3-eth-accounts/src/index.js +++ b/packages/web3-eth-accounts/src/index.js @@ -39,6 +39,10 @@ var isNot = function(value) { return (typeof value === 'undefined') || value === null; }; +var isExist = function(value) { + return (typeof value !== 'undefined') && value !== null; +}; + var Accounts = function Accounts() { var _this = this; @@ -154,6 +158,27 @@ Accounts.prototype.signTransaction = function signTransaction(tx, privateKey, ca return Promise.reject(error); } + if (isExist(tx.common) && isNot(tx.common.customChain)) { + error = new Error('If tx.common is provided it must have tx.common.customChain'); + + callback(error); + return Promise.reject(error); + } + + if (isExist(tx.common) && isNot(tx.common.customChain.chainId)) { + error = new Error('If tx.common is provided it must have tx.common.customChain and tx.common.customChain.chainId'); + + callback(error); + return Promise.reject(error); + } + + if (isExist(tx.common) && isExist(tx.common.customChain.chainId) && isExist(tx.chainId) && tx.chainId !== tx.common.customChain.chainId) { + error = new Error('Chain Id doesnt match in tx.chainId tx.common.customChain.chainId'); + + callback(error); + return Promise.reject(error); + } + function signed(tx) { const error = _validateTransactionForSigning(tx); @@ -264,21 +289,25 @@ Accounts.prototype.signTransaction = function signTransaction(tx, privateKey, ca // Otherwise, get the missing info from the Ethereum Node return Promise.all([ - isNot(tx.chainId) ? _this._ethereumCall.getChainId() : tx.chainId, + ((isNot(tx.common) || isNot(tx.common.customChain.chainId)) ? //tx.common.customChain.chainId is not optional inside tx.common if tx.common is provided + ( isNot(tx.chainId) ? _this._ethereumCall.getChainId() : tx.chainId) + : undefined ), isNot(tx.nonce) ? _this._ethereumCall.getTransactionCount(_this.privateKeyToAccount(privateKey).address) : tx.nonce, isNot(hasTxSigningOptions) ? _this._ethereumCall.getNetworkId() : 1, _handleTxPricing(_this, tx) ]).then(function(args) { - if (isNot(args[0]) || isNot(args[1]) || isNot(args[2]) || isNot(args[3])) { + const [txchainId, txnonce, txnetworkId, txgasInfo] = args; + + if ( (isNot(txchainId) && isNot(tx.common) && isNot(tx.common.customChain.chainId)) || isNot(txnonce) || isNot(txnetworkId) || isNot(txgasInfo)) { throw new Error('One of the values "chainId", "networkId", "gasPrice", or "nonce" couldn\'t be fetched: ' + JSON.stringify(args)); } - + return signed({ ...tx, - chainId: args[0], - nonce: args[1], - networkId: args[2], - ...args[3] // Will either be gasPrice or maxFeePerGas and maxPriorityFeePerGas + ... ((isNot(tx.common) || isNot(tx.common.customChain.chainId) ) ? {chainId: txchainId}:{}), // if common.customChain.chainId is provided no need to add tx.chainId + nonce: txnonce, + networkId: txnetworkId, + ...txgasInfo // Will either be gasPrice or maxFeePerGas and maxPriorityFeePerGas }); }); }; diff --git a/test/e2e.method.signing.js b/test/e2e.method.signing.js index bfc2e838c1d..228839e6433 100644 --- a/test/e2e.method.signing.js +++ b/test/e2e.method.signing.js @@ -281,7 +281,13 @@ describe('transaction and message signing [ @E2E ]', function() { gasLimit: web3.utils.toHex(21000), gasPrice: web3.utils.toHex(web3.utils.toWei('10', 'gwei')), chain: "ropsten", - common: {}, + common: { + customChain: { + name: 'custom-network', + networkId: 1, + chainId: 1, + } + }, hardfork: "istanbul" }; diff --git a/test/eth.accounts.signTransaction.js b/test/eth.accounts.signTransaction.js index eb13322c23c..369814399cb 100644 --- a/test/eth.accounts.signTransaction.js +++ b/test/eth.accounts.signTransaction.js @@ -878,7 +878,42 @@ describe("eth", function () { }); - it("signTransaction will call for chainId", function(done) { + it("signTransaction should not call for chainId if common.customChain.chainId provided", function(done) { + var provider = new FakeHttpProvider(); + var web3 = new Web3(provider); + + provider.injectResult( + test.transaction.common.hardfork === 'london' ? + postEip1559Block: + preEip1559Block + ); + provider.injectValidation(function (payload) { + assert.equal(payload.jsonrpc, '2.0'); + assert.equal(payload.method, 'eth_getBlockByNumber'); + assert.deepEqual(payload.params, ['latest', false]); + }); + + var ethAccounts = new Accounts(web3); + + var testAccount = ethAccounts.privateKeyToAccount(test.privateKey); + assert.equal(testAccount.address, test.address); + + var transaction = clone(test.transaction); + delete transaction.chainId; + testAccount.signTransaction(transaction) + .then(function (tx) { + assert.isObject(tx); + assert.isString(tx.rawTransaction); + + done(); + }) + .catch(e => { + console.log(i, e) + done(e); + }); + }); + + it("signTransaction should call for chainId if common.customChain.chainId not provided", function(done) { var provider = new FakeHttpProvider(); var web3 = new Web3(provider); @@ -888,6 +923,7 @@ describe("eth", function () { assert.equal(payload.method, 'eth_chainId'); assert.deepEqual(payload.params, []); }); + provider.injectResult( test.transaction.common.hardfork === 'london' ? postEip1559Block: @@ -906,6 +942,7 @@ describe("eth", function () { var transaction = clone(test.transaction); delete transaction.chainId; + delete transaction.common; testAccount.signTransaction(transaction) .then(function (tx) { assert.isObject(tx);