Skip to content
This repository was archived by the owner on Mar 5, 2025. It is now read-only.

Auto fetch tx signing option values when not set by user #3143

Merged
nivida merged 11 commits into from
Oct 21, 2019
Merged

Auto fetch tx signing option values when not set by user #3143

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
7788a8f
Auto fetch tx signing option values when not set by user
cgewecke Oct 18, 2019
cc58a9c
Fix (most) failing tests
cgewecke Oct 18, 2019
c056c17
Transfer sendTransaction gasPrice tests to e2e (because broken)
cgewecke Oct 18, 2019
26e1a47
Fix/unskip eth.sendTransaction tests, remove e2e substitute
cgewecke Oct 19, 2019
1115cb5
Cleanup line diff
cgewecke Oct 19, 2019
261776f
Cleanup line diff
cgewecke Oct 19, 2019
b97587f
Cleanup diff
cgewecke Oct 19, 2019
642d357
More diff cleanup
cgewecke Oct 19, 2019
98a5812
Add missing common case to accounts.sign tests / hoist hasTxSigningOp…
cgewecke Oct 19, 2019
5355b95
public api simplified if someone wants to pass custom chain configura…
nivida Oct 21, 2019
e1be1f3
default properties added (chain, hardfork, and common) and related ty…
nivida Oct 21, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion docs/web3-eth.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1062,7 +1062,8 @@ Parameters

2. ``callback`` - ``Function``: (optional) Optional callback, returns an error object as first parameter and the result as second.

.. note:: The ``from`` property can also be an address or index from the :ref:`web3.eth.accounts.wallet <eth_accounts_wallet>`. It will then sign locally using the private key of that account, and send the transaction via :ref:`web3.eth.sendSignedTransaction() <eth-sendsignedtransaction>`. The properties ``chain`` and ``hardfork`` or ``common`` are required if you are not connected to the ``mainnet``.
.. note:: The ``from`` property can also be an address or index from the :ref:`web3.eth.accounts.wallet <eth_accounts_wallet>`. It will then sign locally using the private key of that account, and send the transaction via :ref:`web3.eth.sendSignedTransaction() <eth-sendsignedtransaction>`. If the properties ``chain`` and ``hardfork`` or ``common`` are not set, Web3 will try to set appropriate values by
querying the network for its chainId and networkId.

.. _eth-sendtransaction-return:

Expand Down
14 changes: 7 additions & 7 deletions packages/web3-eth-accounts/package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions packages/web3-eth-accounts/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,15 @@
"any-promise": "1.3.0",
"crypto-browserify": "3.12.0",
"eth-lib": "0.2.7",
"ethereumjs-common": "^1.3.2",
"ethereumjs-tx": "^2.1.1",
"scrypt-shim": "github:web3-js/scrypt-shim",
"underscore": "1.9.1",
"uuid": "3.3.2",
"web3-core": "1.2.1",
"web3-core-helpers": "1.2.1",
"web3-core-method": "1.2.1",
"web3-utils": "1.2.1",
"ethereumjs-tx": "^2.1.1"
"web3-utils": "1.2.1"
},
"devDependencies": {
"definitelytyped-header-parser": "^1.0.1",
Expand Down
70 changes: 50 additions & 20 deletions packages/web3-eth-accounts/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ var uuid = require('uuid');
var utils = require('web3-utils');
var helpers = require('web3-core-helpers');
var Transaction = require('ethereumjs-tx').Transaction;
var Common = require('ethereumjs-common').default;


var isNot = function(value) {
Expand All @@ -54,7 +55,13 @@ var Accounts = function Accounts() {

var _ethereumCall = [
new Method({
name: 'getId',
name: 'getNetworkId',
call: 'net_version',
params: 0,
outputFormatter: parseInt
}),
new Method({
name: 'getChainId',
call: 'eth_chainId',
params: 0,
outputFormatter: utils.hexToNumber
Expand Down Expand Up @@ -138,6 +145,13 @@ Accounts.prototype.signTransaction = function signTransaction(tx, privateKey, ca
);
}

if ((tx.chain && !tx.hardfork) || (tx.hardfork && !tx.chain)){
error = new Error(
'When specifying chain and hardfork, both values must be defined. ' +
'Received "chain": ' + tx.chain + ', "hardfork": ' + tx.hardfork
);
}

if (!tx.gas && !tx.gasLimit) {
error = new Error('"gas" is missing');
}
Expand All @@ -161,19 +175,33 @@ Accounts.prototype.signTransaction = function signTransaction(tx, privateKey, ca
transaction.value = transaction.value || '0x';
transaction.chainId = utils.numberToHex(transaction.chainId);

if (transaction.common) {
transactionOptions['common'] = transaction.common;
delete transaction.common;
}
// Because tx has no ethereumjs-tx signing options we use fetched vals.
if (!(tx.chain && tx.hardfork) && !tx.common){
transactionOptions['common'] = Common.forCustomChain(
'mainnet',
{
name: 'custom-network',
networkId: transaction.networkId,
chainId: transaction.chainId,
},
'petersburg'
);
delete transaction.networkId;
} else {
if (transaction.common) {
transactionOptions['common'] = transaction.common;
delete transaction.common;
}

if (transaction.chain) {
transactionOptions['chain'] = transaction.chain;
delete transaction.chain;
}
if (transaction.chain) {
transactionOptions['chain'] = transaction.chain;
delete transaction.chain;
}

if (transaction.hardfork) {
transactionOptions['hardfork'] = transaction.hardfork;
delete transaction.hardfork;
if (transaction.hardfork) {
transactionOptions['hardfork'] = transaction.hardfork;
delete transaction.hardfork;
}
}

if (privateKey.startsWith('0x')) {
Expand Down Expand Up @@ -212,22 +240,24 @@ Accounts.prototype.signTransaction = function signTransaction(tx, privateKey, ca
return result;
}

// Resolve immediately if nonce, chainId and price are provided
if (tx.nonce !== undefined && tx.chainId !== undefined && tx.gasPrice !== undefined) {
var hasTxSigningOptions = ((tx.chain && tx.hardfork) || tx.common) ? true : false;

// Resolve immediately if nonce, chainId, price and signing options are provided
if (tx.nonce !== undefined && tx.chainId !== undefined && tx.gasPrice !== undefined && hasTxSigningOptions) {
return Promise.resolve(signed(tx));
}


// Otherwise, get the missing info from the Ethereum Node
return Promise.all([
isNot(tx.chainId) ? _this._ethereumCall.getId() : tx.chainId,
isNot(tx.chainId) ? _this._ethereumCall.getChainId() : tx.chainId,
isNot(tx.gasPrice) ? _this._ethereumCall.getGasPrice() : tx.gasPrice,
isNot(tx.nonce) ? _this._ethereumCall.getTransactionCount(_this.privateKeyToAccount(privateKey).address) : tx.nonce
isNot(tx.nonce) ? _this._ethereumCall.getTransactionCount(_this.privateKeyToAccount(privateKey).address) : tx.nonce,
isNot(hasTxSigningOptions) ? _this._ethereumCall.getNetworkId() : 1,
]).then(function (args) {
if (isNot(args[0]) || isNot(args[1]) || isNot(args[2])) {
throw new Error('One of the values "chainId", "gasPrice", or "nonce" couldn\'t be fetched: '+ JSON.stringify(args));
if (isNot(args[0]) || isNot(args[1]) || isNot(args[2]) || isNot(args[3]) ) {
throw new Error('One of the values "chainId", "networkId", "gasPrice", or "nonce" couldn\'t be fetched: '+ JSON.stringify(args));
}
return signed(_.extend(tx, {chainId: args[0], gasPrice: args[1], nonce: args[2]}));
return signed(_.extend(tx, {chainId: args[0], gasPrice: args[1], nonce: args[2], networkId: args[3]}));
});
};

Expand Down
4 changes: 4 additions & 0 deletions test/contract.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3031,6 +3031,8 @@ describe('typical usage', function() {
gasPrice: '0xbb8',
chainId: '0x1',
nonce: '0x1',
chain: 'mainnet',
hardfork: 'petersburg'
}).then(function (tx) {
const expected = tx.rawTransaction;
assert.equal(payload.method, 'eth_sendRawTransaction');
Expand Down Expand Up @@ -3090,6 +3092,8 @@ describe('typical usage', function() {
gasPrice: 3000,
chainId: 1,
nonce: 1,
chain: 'mainnet',
hardfork: 'petersburg'
})
.on('transactionHash', function (value) {
assert.equal('0x5550000000000000000000000000000000000000000000000000000000000032', value);
Expand Down
93 changes: 92 additions & 1 deletion test/e2e.method.signing.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ describe('transaction and message signing [ @E2E ]', function() {
assert(receipt.status === true);
});

it('sendSignedTransaction (with eth.accounts.signTransaction)', async function(){
it('sendSignedTransaction (accounts.signTransaction with signing options)', async function(){
const source = wallet[0].address;
const destination = wallet[1].address;

Expand Down Expand Up @@ -80,6 +80,97 @@ describe('transaction and message signing [ @E2E ]', function() {
assert(receipt.status === true);
});

it('sendSignedTransaction (accounts.signTransaction / without signing options)', async function(){
const source = wallet[0].address;
const destination = wallet[1].address;

const txCount = await web3.eth.getTransactionCount(source);

const txObject = {
nonce: web3.utils.toHex(txCount),
to: destination,
value: web3.utils.toHex(web3.utils.toWei('0.1', 'ether')),
gasLimit: web3.utils.toHex(21000),
gasPrice: web3.utils.toHex(web3.utils.toWei('10', 'gwei')),
};

const signed = await web3.eth.accounts.signTransaction(txObject, wallet[0].privateKey);
const receipt = await web3.eth.sendSignedTransaction(signed.rawTransaction);

assert(receipt.status === true);
});

it('accounts.signTransaction errors when common, chain and hardfork all defined', async function(){
const source = wallet[0].address;
const destination = wallet[1].address;

const txCount = await web3.eth.getTransactionCount(source);

const txObject = {
nonce: web3.utils.toHex(txCount),
to: destination,
value: web3.utils.toHex(web3.utils.toWei('0.1', 'ether')),
gasLimit: web3.utils.toHex(21000),
gasPrice: web3.utils.toHex(web3.utils.toWei('10', 'gwei')),
chain: "ropsten",
common: {},
hardfork: "istanbul"
};

try {
await web3.eth.accounts.signTransaction(txObject, wallet[0].privateKey);
assert.fail()
} catch (err) {
assert(err.message.includes('common object or the chain and hardfork'));
}
});

it('accounts.signTransaction errors when chain specified without hardfork', async function(){
const source = wallet[0].address;
const destination = wallet[1].address;

const txCount = await web3.eth.getTransactionCount(source);

const txObject = {
nonce: web3.utils.toHex(txCount),
to: destination,
value: web3.utils.toHex(web3.utils.toWei('0.1', 'ether')),
gasLimit: web3.utils.toHex(21000),
gasPrice: web3.utils.toHex(web3.utils.toWei('10', 'gwei')),
chain: "ropsten"
};

try {
await web3.eth.accounts.signTransaction(txObject, wallet[0].privateKey);
assert.fail()
} catch (err) {
assert(err.message.includes('both values must be defined'));
}
});

it('accounts.signTransaction errors when hardfork specified without chain', async function(){
const source = wallet[0].address;
const destination = wallet[1].address;

const txCount = await web3.eth.getTransactionCount(source);

const txObject = {
nonce: web3.utils.toHex(txCount),
to: destination,
value: web3.utils.toHex(web3.utils.toWei('0.1', 'ether')),
gasLimit: web3.utils.toHex(21000),
gasPrice: web3.utils.toHex(web3.utils.toWei('10', 'gwei')),
hardfork: "istanbul"
};

try {
await web3.eth.accounts.signTransaction(txObject, wallet[0].privateKey);
assert.fail()
} catch (err) {
assert(err.message.includes('both values must be defined'));
}
});

it('eth.personal.sign', async function(){
// ganache does not support eth_sign
if (process.env.GANACHE) return;
Expand Down
Loading