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

add tx.chainId if tx.common.chainId not provided #4293

Merged
merged 13 commits into from
Sep 22, 2021
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -441,3 +441,4 @@ Released with 1.0.0-beta.37 code base.
- lerna from 3.22.1 to 4.0.0 (#4231)
- Dropped build tests in CI for Node v8 and v10, and added support for Node v14
- Change default value for `maxPriorityFeePerGas` from `1 Gwei` to `2.5 Gwei` (#4284)
- Not considering `tx.chainId` if `tx.common.customChain.chainId` is provided for `web3.eth.accounts.signTransaction` function (#4293)
15 changes: 11 additions & 4 deletions packages/web3-eth-accounts/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,13 @@ Accounts.prototype.signTransaction = function signTransaction(tx, privateKey, ca
return Promise.reject(error);
}

if (!isNot(tx.common) && !isNot(tx.common.customChain.chainId) && !isNot(tx.chainId) && tx.chainId !== tx.common.customChain.chainId) {
jdevcs marked this conversation as resolved.
Show resolved Hide resolved
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);

Expand Down Expand Up @@ -264,18 +271,18 @@ 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)) ? ( isNot(tx.chainId) ? _this._ethereumCall.getChainId() : tx.chainId) : undefined ), //tx.common.customChain.chainId is not optional inside tx.common if tx.common is provided
jdevcs marked this conversation as resolved.
Show resolved Hide resolved
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])) {
if ( (isNot(args[0]) && isNot(tx.common) && isNot(tx.common.customChain.chainId)) || isNot(args[1]) || isNot(args[2]) || isNot(args[3])) {
jdevcs marked this conversation as resolved.
Show resolved Hide resolved
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],
... ((isNot(tx.common) || isNot(tx.common.customChain.chainId) ) ? {chainId: args[0]}:{}), // if common.customChain.chainId is provided no need to add tx.chainId
nonce: args[1],
networkId: args[2],
...args[3] // Will either be gasPrice or maxFeePerGas and maxPriorityFeePerGas
Expand Down
8 changes: 7 additions & 1 deletion test/e2e.method.signing.js
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,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"
};

Expand Down
39 changes: 38 additions & 1 deletion test/eth.accounts.signTransaction.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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:
Expand All @@ -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);
Expand Down