From 514be408f8eef60b27b79743d6525bf60a354ce4 Mon Sep 17 00:00:00 2001 From: Frankie Date: Wed, 30 Oct 2019 12:15:54 -1000 Subject: [PATCH] I#6704 eth_getTransactionByHash will now check metamask's local history for pending transactions (#7327) * tests - create tests for pending middlewares * transactions - add r,s,v values to the txMeta to match the JSON rpc response * network - add new middleware for eth_getTransactionByHash that the checks pending tx's for a response value * transactions/pending - use getTransactionReceipt for checking if tx is in a block * meta - file rename --- .../network/createMetamaskMiddleware.js | 15 +- .../controllers/network/middleware/pending.js | 28 +++ app/scripts/controllers/network/util.js | 21 ++ app/scripts/controllers/transactions/index.js | 9 + .../transactions/pending-tx-tracker.js | 4 +- app/scripts/metamask-controller.js | 1 + .../network-controller-test.js} | 4 +- .../network/pending-middleware-test.js | 81 +++++++ test/unit/app/controllers/network/stubs.js | 225 ++++++++++++++++++ 9 files changed, 372 insertions(+), 16 deletions(-) create mode 100644 app/scripts/controllers/network/middleware/pending.js rename test/unit/app/controllers/{network-contoller-test.js => network/network-controller-test.js} (95%) create mode 100644 test/unit/app/controllers/network/pending-middleware-test.js create mode 100644 test/unit/app/controllers/network/stubs.js diff --git a/app/scripts/controllers/network/createMetamaskMiddleware.js b/app/scripts/controllers/network/createMetamaskMiddleware.js index 5dcd3a895..58ccb95a1 100644 --- a/app/scripts/controllers/network/createMetamaskMiddleware.js +++ b/app/scripts/controllers/network/createMetamaskMiddleware.js @@ -1,8 +1,7 @@ const mergeMiddleware = require('json-rpc-engine/src/mergeMiddleware') const createScaffoldMiddleware = require('json-rpc-engine/src/createScaffoldMiddleware') -const createAsyncMiddleware = require('json-rpc-engine/src/createAsyncMiddleware') const createWalletSubprovider = require('eth-json-rpc-middleware/wallet') - +const { createPendingNonceMiddleware, createPendingTxMiddleware } = require('./middleware/pending') module.exports = createMetamaskMiddleware function createMetamaskMiddleware ({ @@ -15,6 +14,7 @@ function createMetamaskMiddleware ({ processTypedMessageV4, processPersonalMessage, getPendingNonce, + getPendingTransactionByHash, }) { const metamaskMiddleware = mergeMiddleware([ createScaffoldMiddleware({ @@ -32,16 +32,7 @@ function createMetamaskMiddleware ({ processPersonalMessage, }), createPendingNonceMiddleware({ getPendingNonce }), + createPendingTxMiddleware({ getPendingTransactionByHash }), ]) return metamaskMiddleware } - -function createPendingNonceMiddleware ({ getPendingNonce }) { - return createAsyncMiddleware(async (req, res, next) => { - if (req.method !== 'eth_getTransactionCount') return next() - const address = req.params[0] - const blockRef = req.params[1] - if (blockRef !== 'pending') return next() - res.result = await getPendingNonce(address) - }) -} diff --git a/app/scripts/controllers/network/middleware/pending.js b/app/scripts/controllers/network/middleware/pending.js new file mode 100644 index 000000000..542d8bde6 --- /dev/null +++ b/app/scripts/controllers/network/middleware/pending.js @@ -0,0 +1,28 @@ +const { formatTxMetaForRpcResult } = require('../util') +const createAsyncMiddleware = require('json-rpc-engine/src/createAsyncMiddleware') + +function createPendingNonceMiddleware ({ getPendingNonce }) { + return createAsyncMiddleware(async (req, res, next) => { + const {method, params} = req + if (method !== 'eth_getTransactionCount') return next() + const [param, blockRef] = params + if (blockRef !== 'pending') return next() + res.result = await getPendingNonce(param) + }) +} + +function createPendingTxMiddleware ({ getPendingTransactionByHash }) { + return createAsyncMiddleware(async (req, res, next) => { + const {method, params} = req + if (method !== 'eth_getTransactionByHash') return next() + const [hash] = params + const txMeta = getPendingTransactionByHash(hash) + if (!txMeta) return next() + res.result = formatTxMetaForRpcResult(txMeta) + }) +} + +module.exports = { + createPendingTxMiddleware, + createPendingNonceMiddleware, +} diff --git a/app/scripts/controllers/network/util.js b/app/scripts/controllers/network/util.js index a6f848864..829c62582 100644 --- a/app/scripts/controllers/network/util.js +++ b/app/scripts/controllers/network/util.js @@ -29,6 +29,27 @@ const networkToNameMap = { const getNetworkDisplayName = key => networkToNameMap[key] +function formatTxMetaForRpcResult (txMeta) { + return { + 'blockHash': txMeta.txReceipt ? txMeta.txReceipt.blockHash : null, + 'blockNumber': txMeta.txReceipt ? txMeta.txReceipt.blockNumber : null, + 'from': txMeta.txParams.from, + 'gas': txMeta.txParams.gas, + 'gasPrice': txMeta.txParams.gasPrice, + 'hash': txMeta.hash, + 'input': txMeta.txParams.data || '0x', + 'nonce': txMeta.txParams.nonce, + 'to': txMeta.txParams.to, + 'transactionIndex': txMeta.txReceipt ? txMeta.txReceipt.transactionIndex : null, + 'value': txMeta.txParams.value || '0x0', + 'v': txMeta.v, + 'r': txMeta.r, + 's': txMeta.s, + } +} + + module.exports = { getNetworkDisplayName, + formatTxMetaForRpcResult, } diff --git a/app/scripts/controllers/transactions/index.js b/app/scripts/controllers/transactions/index.js index 408bed283..df9fb6502 100644 --- a/app/scripts/controllers/transactions/index.js +++ b/app/scripts/controllers/transactions/index.js @@ -423,6 +423,15 @@ class TransactionController extends EventEmitter { const fromAddress = txParams.from const ethTx = new Transaction(txParams) await this.signEthTx(ethTx, fromAddress) + + // add r,s,v values for provider request purposes see createMetamaskMiddleware + // and JSON rpc standard for further explanation + txMeta.r = ethUtil.bufferToHex(ethTx.r) + txMeta.s = ethUtil.bufferToHex(ethTx.s) + txMeta.v = ethUtil.bufferToHex(ethTx.v) + + this.txStateManager.updateTx(txMeta, 'transactions#signTransaction: add r, s, v values') + // set state to signed this.txStateManager.setTxStatusSigned(txMeta.id) const rawTx = ethUtil.bufferToHex(ethTx.serialize()) diff --git a/app/scripts/controllers/transactions/pending-tx-tracker.js b/app/scripts/controllers/transactions/pending-tx-tracker.js index 1ef3be36e..8f4076f45 100644 --- a/app/scripts/controllers/transactions/pending-tx-tracker.js +++ b/app/scripts/controllers/transactions/pending-tx-tracker.js @@ -174,7 +174,7 @@ class PendingTransactionTracker extends EventEmitter { // get latest transaction status try { - const { blockNumber } = await this.query.getTransactionByHash(txHash) || {} + const { blockNumber } = await this.query.getTransactionReceipt(txHash) || {} if (blockNumber) { this.emit('tx:confirmed', txId) } @@ -196,7 +196,7 @@ class PendingTransactionTracker extends EventEmitter { async _checkIftxWasDropped (txMeta) { const { txParams: { nonce, from }, hash } = txMeta const nextNonce = await this.query.getTransactionCount(from) - const { blockNumber } = await this.query.getTransactionByHash(hash) || {} + const { blockNumber } = await this.query.getTransactionReceipt(hash) || {} if (!blockNumber && parseInt(nextNonce) > parseInt(nonce)) { return true } diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index e3f332d4c..eac6d1e81 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -353,6 +353,7 @@ module.exports = class MetamaskController extends EventEmitter { processTypedMessageV4: this.newUnsignedTypedMessage.bind(this), processPersonalMessage: this.newUnsignedPersonalMessage.bind(this), getPendingNonce: this.getPendingNonce.bind(this), + getPendingTransactionByHash: (hash) => this.txController.getFilteredTxList({ hash, status: 'submitted' })[0], } const providerProxy = this.networkController.initializeProvider(providerOpts) return providerProxy diff --git a/test/unit/app/controllers/network-contoller-test.js b/test/unit/app/controllers/network/network-controller-test.js similarity index 95% rename from test/unit/app/controllers/network-contoller-test.js rename to test/unit/app/controllers/network/network-controller-test.js index 32f7b337d..b63a23a4f 100644 --- a/test/unit/app/controllers/network-contoller-test.js +++ b/test/unit/app/controllers/network/network-controller-test.js @@ -1,9 +1,9 @@ const assert = require('assert') const nock = require('nock') -const NetworkController = require('../../../../app/scripts/controllers/network') +const NetworkController = require('../../../../../app/scripts/controllers/network') const { getNetworkDisplayName, -} = require('../../../../app/scripts/controllers/network/util') +} = require('../../../../../app/scripts/controllers/network/util') describe('# Network Controller', function () { let networkController diff --git a/test/unit/app/controllers/network/pending-middleware-test.js b/test/unit/app/controllers/network/pending-middleware-test.js new file mode 100644 index 000000000..838395b0b --- /dev/null +++ b/test/unit/app/controllers/network/pending-middleware-test.js @@ -0,0 +1,81 @@ +const assert = require('assert') +const { createPendingNonceMiddleware, createPendingTxMiddleware } = require('../../../../../app/scripts/controllers/network/middleware/pending') +const txMetaStub = require('./stubs').txMetaStub +describe('#createPendingNonceMiddleware', function () { + const getPendingNonce = async () => '0x2' + const address = '0xF231D46dD78806E1DD93442cf33C7671f8538748' + const pendingNonceMiddleware = createPendingNonceMiddleware({ getPendingNonce }) + + it('should call next if not a eth_getTransactionCount request', (done) => { + const req = {method: 'eth_getBlockByNumber'} + const res = {} + pendingNonceMiddleware(req, res, () => done()) + }) + it('should call next if not a "pending" block request', (done) => { + const req = { method: 'eth_getTransactionCount', params: [address] } + const res = {} + pendingNonceMiddleware(req, res, () => done()) + }) + it('should fill the result with a the "pending" nonce', (done) => { + const req = { method: 'eth_getTransactionCount', params: [address, 'pending'] } + const res = {} + pendingNonceMiddleware(req, res, () => { done(new Error('should not have called next')) }, () => { + assert(res.result === '0x2') + done() + }) + }) +}) + +describe('#createPendingTxMiddleware', function () { + let returnUndefined = true + const getPendingTransactionByHash = () => returnUndefined ? undefined : txMetaStub + const address = '0xF231D46dD78806E1DD93442cf33C7671f8538748' + const pendingTxMiddleware = createPendingTxMiddleware({ getPendingTransactionByHash }) + const spec = { + 'blockHash': null, + 'blockNumber': null, + 'from': '0xf231d46dd78806e1dd93442cf33c7671f8538748', + 'gas': '0x5208', + 'gasPrice': '0x1e8480', + 'hash': '0x2cc5a25744486f7383edebbf32003e5a66e18135799593d6b5cdd2bb43674f09', + 'input': '0x', + 'nonce': '0x4', + 'to': '0xf231d46dd78806e1dd93442cf33c7671f8538748', + 'transactionIndex': null, + 'value': '0x0', + 'v': '0x2c', + 'r': '0x5f973e540f2d3c2f06d3725a626b75247593cb36477187ae07ecfe0a4db3cf57', + 's': '0x0259b52ee8c58baaa385fb05c3f96116e58de89bcc165cb3bfdfc708672fed8a', + } + it('should call next if not a eth_getTransactionByHash request', (done) => { + const req = {method: 'eth_getBlockByNumber'} + const res = {} + pendingTxMiddleware(req, res, () => done()) + }) + + it('should call next if no pending txMeta is in history', (done) => { + const req = { method: 'eth_getTransactionByHash', params: [address] } + const res = {} + pendingTxMiddleware(req, res, () => done()) + }) + + it('should fill the result with a the "pending" tx the result should match the rpc spec', (done) => { + returnUndefined = false + const req = { method: 'eth_getTransactionByHash', params: [address, 'pending'] } + const res = {} + pendingTxMiddleware(req, res, () => { done(new Error('should not have called next')) }, () => { + /* + // uncomment this section for debugging help with non matching keys + const coppy = {...res.result} + Object.keys(spec).forEach((key) => { + console.log(coppy[key], '===', spec[key], coppy[key] === spec[key], key) + delete coppy[key] + }) + console.log(coppy) + */ + assert.deepStrictEqual(res.result, spec, new Error('result does not match the spec object')) + done() + }) + }) + +}) diff --git a/test/unit/app/controllers/network/stubs.js b/test/unit/app/controllers/network/stubs.js new file mode 100644 index 000000000..1551cd581 --- /dev/null +++ b/test/unit/app/controllers/network/stubs.js @@ -0,0 +1,225 @@ +/* + this file is for all my big stubs because i don't want to + to mingle with my tests +*/ + +module.exports = {} + +// for pending middlewares test +module.exports.txMetaStub = { + 'estimatedGas': '0x5208', + 'firstRetryBlockNumber': '0x51a402', + 'gasLimitSpecified': true, + 'gasPriceSpecified': true, + 'hash': '0x2cc5a25744486f7383edebbf32003e5a66e18135799593d6b5cdd2bb43674f09', + 'history': [ + { + 'id': 405984854664302, + 'loadingDefaults': true, + 'metamaskNetworkId': '4', + 'status': 'unapproved', + 'time': 1572395156620, + 'transactionCategory': 'sentEther', + 'txParams': { + 'from': '0xf231d46dd78806e1dd93442cf33c7671f8538748', + 'gas': '0x5208', + 'gasPrice': '0x1e8480', + 'to': '0xf231d46dd78806e1dd93442cf33c7671f8538748', + 'value': '0x0', + }, + 'type': 'standard', + }, + [ + { + 'op': 'replace', + 'path': '/loadingDefaults', + 'timestamp': 1572395156645, + 'value': false, + }, + { + 'op': 'add', + 'path': '/gasPriceSpecified', + 'value': true, + }, + { + 'op': 'add', + 'path': '/gasLimitSpecified', + 'value': true, + }, + { + 'op': 'add', + 'path': '/estimatedGas', + 'value': '0x5208', + }, + ], + [ + { + 'note': '#newUnapprovedTransaction - adding the origin', + 'op': 'add', + 'path': '/origin', + 'timestamp': 1572395156645, + 'value': 'MetaMask', + }, + ], + [], + [ + { + 'note': 'txStateManager: setting status to approved', + 'op': 'replace', + 'path': '/status', + 'timestamp': 1572395158240, + 'value': 'approved', + }, + ], + [ + { + 'note': 'transactions#approveTransaction', + 'op': 'add', + 'path': '/txParams/nonce', + 'timestamp': 1572395158261, + 'value': '0x4', + }, + { + 'op': 'add', + 'path': '/nonceDetails', + 'value': { + 'local': { + 'details': { + 'highest': 4, + 'startPoint': 4, + }, + 'name': 'local', + 'nonce': 4, + }, + 'network': { + 'details': { + 'baseCount': 4, + 'blockNumber': '0x51a401', + }, + 'name': 'network', + 'nonce': 4, + }, + 'params': { + 'highestLocallyConfirmed': 0, + 'highestSuggested': 4, + 'nextNetworkNonce': 4, + }, + }, + }, + ], + [ + { + 'note': 'transactions#signTransaction: add r, s, v values', + 'op': 'add', + 'path': '/r', + 'timestamp': 1572395158280, + 'value': '0x5f973e540f2d3c2f06d3725a626b75247593cb36477187ae07ecfe0a4db3cf57', + }, + { + 'op': 'add', + 'path': '/s', + 'value': '0x0259b52ee8c58baaa385fb05c3f96116e58de89bcc165cb3bfdfc708672fed8a', + }, + { + 'op': 'add', + 'path': '/v', + 'value': '0x2c', + }, + ], + [ + { + 'note': 'transactions#publishTransaction', + 'op': 'replace', + 'path': '/status', + 'timestamp': 1572395158281, + 'value': 'signed', + }, + { + 'op': 'add', + 'path': '/rawTx', + 'value': '0xf86204831e848082520894f231d46dd78806e1dd93442cf33c7671f853874880802ca05f973e540f2d3c2f06d3725a626b75247593cb36477187ae07ecfe0a4db3cf57a00259b52ee8c58baaa385fb05c3f96116e58de89bcc165cb3bfdfc708672fed8a', + }, + ], + [], + [ + { + 'note': 'transactions#setTxHash', + 'op': 'add', + 'path': '/hash', + 'timestamp': 1572395158570, + 'value': '0x2cc5a25744486f7383edebbf32003e5a66e18135799593d6b5cdd2bb43674f09', + }, + ], + [ + { + 'note': 'txStateManager - add submitted time stamp', + 'op': 'add', + 'path': '/submittedTime', + 'timestamp': 1572395158571, + 'value': 1572395158570, + }, + ], + [ + { + 'note': 'txStateManager: setting status to submitted', + 'op': 'replace', + 'path': '/status', + 'timestamp': 1572395158576, + 'value': 'submitted', + }, + ], + [ + { + 'note': 'transactions/pending-tx-tracker#event: tx:block-update', + 'op': 'add', + 'path': '/firstRetryBlockNumber', + 'timestamp': 1572395168972, + 'value': '0x51a402', + }, + ], + ], + 'id': 405984854664302, + 'loadingDefaults': false, + 'metamaskNetworkId': '4', + 'nonceDetails': { + 'local': { + 'details': { + 'highest': 4, + 'startPoint': 4, + }, + 'name': 'local', + 'nonce': 4, + }, + 'network': { + 'details': { + 'baseCount': 4, + 'blockNumber': '0x51a401', + }, + 'name': 'network', + 'nonce': 4, + }, + 'params': { + 'highestLocallyConfirmed': 0, + 'highestSuggested': 4, + 'nextNetworkNonce': 4, + }, + }, + 'origin': 'MetaMask', + 'r': '0x5f973e540f2d3c2f06d3725a626b75247593cb36477187ae07ecfe0a4db3cf57', + 'rawTx': '0xf86204831e848082520894f231d46dd78806e1dd93442cf33c7671f853874880802ca05f973e540f2d3c2f06d3725a626b75247593cb36477187ae07ecfe0a4db3cf57a00259b52ee8c58baaa385fb05c3f96116e58de89bcc165cb3bfdfc708672fed8a', + 's': '0x0259b52ee8c58baaa385fb05c3f96116e58de89bcc165cb3bfdfc708672fed8a', + 'status': 'submitted', + 'submittedTime': 1572395158570, + 'time': 1572395156620, + 'transactionCategory': 'sentEther', + 'txParams': { + 'from': '0xf231d46dd78806e1dd93442cf33c7671f8538748', + 'gas': '0x5208', + 'gasPrice': '0x1e8480', + 'nonce': '0x4', + 'to': '0xf231d46dd78806e1dd93442cf33c7671f8538748', + 'value': '0x0', + }, + 'type': 'standard', + 'v': '0x2c', +}