From 4ba565e7196635caf63d803cab830e03bab85684 Mon Sep 17 00:00:00 2001 From: Brad Decker Date: Tue, 29 Jun 2021 15:08:16 -0500 Subject: [PATCH] track dapp suggested gas fees (#11410) --- app/scripts/controllers/transactions/index.js | 3 +- .../transactions/tx-state-manager.js | 36 ++++- .../transactions/tx-state-manager.test.js | 131 ++++++++++++++++++ 3 files changed, 167 insertions(+), 3 deletions(-) diff --git a/app/scripts/controllers/transactions/index.js b/app/scripts/controllers/transactions/index.js index 18d625f3e..60ad40f91 100644 --- a/app/scripts/controllers/transactions/index.js +++ b/app/scripts/controllers/transactions/index.js @@ -298,6 +298,7 @@ export default class TransactionController extends EventEmitter { */ let txMeta = this.txStateManager.generateTxMeta({ txParams: normalizedTxParams, + origin, }); if (origin === 'metamask') { @@ -321,8 +322,6 @@ export default class TransactionController extends EventEmitter { } } - txMeta.origin = origin; - const { type, getCodeResponse } = await this._determineTransactionType( txParams, ); diff --git a/app/scripts/controllers/transactions/tx-state-manager.js b/app/scripts/controllers/transactions/tx-state-manager.js index eed1f6d79..19155d1a2 100644 --- a/app/scripts/controllers/transactions/tx-state-manager.js +++ b/app/scripts/controllers/transactions/tx-state-manager.js @@ -72,12 +72,45 @@ export default class TransactionStateManager extends EventEmitter { * overwriting default keys of the TransactionMeta * @returns {TransactionMeta} the default txMeta object */ - generateTxMeta(opts) { + generateTxMeta(opts = {}) { const netId = this.getNetwork(); const chainId = this.getCurrentChainId(); if (netId === 'loading') { throw new Error('MetaMask is having trouble connecting to the network'); } + + let dappSuggestedGasFees = null; + + // If we are dealing with a transaction suggested by a dapp and not + // an internally created metamask transaction, we need to keep record of + // the originally submitted gasParams. + if ( + opts.txParams && + typeof opts.origin === 'string' && + opts.origin !== 'metamask' + ) { + if (typeof opts.txParams.gasPrice !== 'undefined') { + dappSuggestedGasFees = { + gasPrice: opts.txParams.gasPrice, + }; + } else if ( + typeof opts.txParams.maxFeePerGas !== 'undefined' || + typeof opts.txParams.maxPriorityFeePerGas !== 'undefined' + ) { + dappSuggestedGasFees = { + maxPriorityFeePerGas: opts.txParams.maxPriorityFeePerGas, + maxFeePerGas: opts.txParams.maxFeePerGas, + }; + } + + if (typeof opts.txParams.gas !== 'undefined') { + dappSuggestedGasFees = { + ...dappSuggestedGasFees, + gas: opts.txParams.gas, + }; + } + } + return { id: createId(), time: new Date().getTime(), @@ -85,6 +118,7 @@ export default class TransactionStateManager extends EventEmitter { metamaskNetworkId: netId, chainId, loadingDefaults: true, + dappSuggestedGasFees, ...opts, }; } diff --git a/app/scripts/controllers/transactions/tx-state-manager.test.js b/app/scripts/controllers/transactions/tx-state-manager.test.js index 895fb3626..e8a391faf 100644 --- a/app/scripts/controllers/transactions/tx-state-manager.test.js +++ b/app/scripts/controllers/transactions/tx-state-manager.test.js @@ -10,6 +10,7 @@ import { RINKEBY_CHAIN_ID, KOVAN_NETWORK_ID, } from '../../../../shared/constants/network'; +import { GAS_LIMITS } from '../../../../shared/constants/gas'; import TxStateManager from './tx-state-manager'; import { snapshotFromTxMeta } from './lib/tx-state-history-helpers'; @@ -1074,6 +1075,136 @@ describe('TransactionStateManager', function () { }); }); + describe('#generateTxMeta', function () { + it('generates a txMeta object when supplied no parameters', function () { + // There are currently not safety checks for missing 'opts' but we should + // at least enforce txParams. This is done in the transaction controller + // before *calling* this method, but we should perhaps ensure that + // txParams is provided and validated in this method. + // TODO: this test should fail. + const generatedTransaction = txStateManager.generateTxMeta(); + assert.ok(generatedTransaction); + }); + + it('generates a txMeta object with txParams specified', function () { + const txParams = { + gas: GAS_LIMITS.SIMPLE, + from: '0x0000', + to: '0x000', + value: '0x0', + gasPrice: '0x0', + }; + const generatedTransaction = txStateManager.generateTxMeta({ + txParams, + }); + assert.ok(generatedTransaction); + assert.strictEqual(generatedTransaction.txParams, txParams); + }); + + it('generates a txMeta object with txParams specified using EIP-1559 fields', function () { + const txParams = { + gas: GAS_LIMITS.SIMPLE, + from: '0x0000', + to: '0x000', + value: '0x0', + maxFeePerGas: '0x0', + maxPriorityFeePerGas: '0x0', + }; + const generatedTransaction = txStateManager.generateTxMeta({ + txParams, + }); + assert.ok(generatedTransaction); + assert.strictEqual(generatedTransaction.txParams, txParams); + }); + + it('records dappSuggestedGasFees when origin is provided and is not "metamask"', function () { + const eip1559GasFeeFields = { + maxFeePerGas: '0x0', + maxPriorityFeePerGas: '0x0', + gas: GAS_LIMITS.SIMPLE, + }; + + const legacyGasFeeFields = { + gasPrice: '0x0', + gas: GAS_LIMITS.SIMPLE, + }; + + const eip1559TxParams = { + from: '0x0000', + to: '0x000', + value: '0x0', + ...eip1559GasFeeFields, + }; + + const legacyTxParams = { + from: '0x0000', + to: '0x000', + value: '0x0', + ...legacyGasFeeFields, + }; + const eip1559GeneratedTransaction = txStateManager.generateTxMeta({ + txParams: eip1559TxParams, + origin: 'adappt.com', + }); + const legacyGeneratedTransaction = txStateManager.generateTxMeta({ + txParams: legacyTxParams, + origin: 'adappt.com', + }); + assert.ok( + eip1559GeneratedTransaction, + 'generated EIP1559 transaction should be truthy', + ); + assert.deepStrictEqual( + eip1559GeneratedTransaction.dappSuggestedGasFees, + eip1559GasFeeFields, + 'generated EIP1559 transaction should have appropriate dappSuggestedGasFees', + ); + + assert.ok( + legacyGeneratedTransaction, + 'generated legacy transaction should be truthy', + ); + assert.deepStrictEqual( + legacyGeneratedTransaction.dappSuggestedGasFees, + legacyGasFeeFields, + 'generated legacy transaction should have appropriate dappSuggestedGasFees', + ); + }); + + it('does not record dappSuggestedGasFees when transaction origin is "metamask"', function () { + const txParams = { + gas: GAS_LIMITS.SIMPLE, + from: '0x0000', + to: '0x000', + value: '0x0', + maxFeePerGas: '0x0', + maxPriorityFeePerGas: '0x0', + }; + const generatedTransaction = txStateManager.generateTxMeta({ + txParams, + origin: 'metamask', + }); + assert.ok(generatedTransaction); + assert.strictEqual(generatedTransaction.dappSuggestedGasFees, null); + }); + + it('does not record dappSuggestedGasFees when transaction origin is not provided', function () { + const txParams = { + gas: GAS_LIMITS.SIMPLE, + from: '0x0000', + to: '0x000', + value: '0x0', + maxFeePerGas: '0x0', + maxPriorityFeePerGas: '0x0', + }; + const generatedTransaction = txStateManager.generateTxMeta({ + txParams, + }); + assert.ok(generatedTransaction); + assert.strictEqual(generatedTransaction.dappSuggestedGasFees, null); + }); + }); + describe('#clearUnapprovedTxs', function () { it('removes unapproved transactions', function () { const txMetas = [