From 82cf63fcf41412c1a8dd25d567c037bb8f6ba6b7 Mon Sep 17 00:00:00 2001 From: ryanml Date: Thu, 8 Sep 2022 13:01:52 -0700 Subject: [PATCH 1/2] Ensuring Blockies Icon is used in nickname popup when enabled (#15768) --- .../ui/update-nickname-popover/update-nickname-popover.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/components/ui/update-nickname-popover/update-nickname-popover.js b/ui/components/ui/update-nickname-popover/update-nickname-popover.js index 1f6fd2d2280a..2d775603d170 100644 --- a/ui/components/ui/update-nickname-popover/update-nickname-popover.js +++ b/ui/components/ui/update-nickname-popover/update-nickname-popover.js @@ -8,7 +8,7 @@ import TextField from '../text-field'; import { I18nContext } from '../../../contexts/i18n'; -import Identicon from '../identicon/identicon.component'; +import Identicon from '../identicon'; import { getTokenList } from '../../../selectors'; export default function UpdateNicknamePopover({ From 216f76646ed39d2f10e2a3d710a2f29a607c756d Mon Sep 17 00:00:00 2001 From: Jyoti Puri Date: Fri, 9 Sep 2022 17:20:31 +0530 Subject: [PATCH 2/2] Make cancel transaction idempotent (#15675) --- app/scripts/controllers/transactions/index.js | 23 ++-- .../controllers/transactions/index.test.js | 100 ++++++++++++++++++ app/scripts/metamask-controller.js | 10 +- ui/store/actions.js | 10 +- 4 files changed, 123 insertions(+), 20 deletions(-) diff --git a/app/scripts/controllers/transactions/index.js b/app/scripts/controllers/transactions/index.js index 5e78309ef40d..2b18bfe2f6f9 100644 --- a/app/scripts/controllers/transactions/index.js +++ b/app/scripts/controllers/transactions/index.js @@ -1161,13 +1161,23 @@ export default class TransactionController extends EventEmitter { * params instead of allowing this method to generate them * @param options * @param options.estimatedBaseFee + * @param options.actionId * @returns {txMeta} */ async createCancelTransaction( originalTxId, customGasSettings, - { estimatedBaseFee } = {}, + { estimatedBaseFee, actionId } = {}, ) { + // If transaction is found for same action id, do not create a new cancel transaction. + if (actionId) { + const existingTxMeta = + this.txStateManager.getTransactionWithActionId(actionId); + if (existingTxMeta) { + return existingTxMeta; + } + } + const originalTxMeta = this.txStateManager.getTransaction(originalTxId); const { txParams } = originalTxMeta; const { from, nonce } = txParams; @@ -1195,6 +1205,7 @@ export default class TransactionController extends EventEmitter { loadingDefaults: false, status: TRANSACTION_STATUSES.APPROVED, type: TRANSACTION_TYPES.CANCEL, + actionId, }); if (estimatedBaseFee) { @@ -1293,6 +1304,7 @@ export default class TransactionController extends EventEmitter { // we need to keep track of what is currently being signed, // So that we do not increment nonce + resubmit something // that is already being incremented & signed. + const txMeta = this.txStateManager.getTransaction(txId); if (this.inProcessOfSigning.has(txId)) { return; } @@ -1302,8 +1314,6 @@ export default class TransactionController extends EventEmitter { // approve this.txStateManager.setTxStatusApproved(txId); // get next nonce - const txMeta = this.txStateManager.getTransaction(txId); - const fromAddress = txMeta.txParams.from; // wait for a nonce let { customNonceValue } = txMeta; @@ -1793,10 +1803,9 @@ export default class TransactionController extends EventEmitter { }, }) .forEach((txMeta) => { - const txSignError = new Error( - 'Transaction found as "approved" during boot - possibly stuck during signing', - ); - this._failTransaction(txMeta.id, txSignError); + // Line below will try to publish transaction which is in + // APPROVED state at the time of controller bootup + this.approveTransaction(txMeta); }); } diff --git a/app/scripts/controllers/transactions/index.test.js b/app/scripts/controllers/transactions/index.test.js index 4bbb3046d2c6..b1db9e697384 100644 --- a/app/scripts/controllers/transactions/index.test.js +++ b/app/scripts/controllers/transactions/index.test.js @@ -467,6 +467,106 @@ describe('Transaction Controller', function () { }); }); + describe('#createCancelTransaction', function () { + const selectedAddress = '0x1678a085c290ebd122dc42cba69373b5953b831d'; + const recipientAddress = '0xc42edfcc21ed14dda456aa0756c153f7985d8813'; + + let getSelectedAddress, + getPermittedAccounts, + getDefaultGasFees, + getDefaultGasLimit; + beforeEach(function () { + const hash = + '0x2a5523c6fa98b47b7d9b6c8320179785150b42a16bcff36b398c5062b65657e8'; + providerResultStub.eth_sendRawTransaction = hash; + + getSelectedAddress = sinon + .stub(txController, 'getSelectedAddress') + .returns(selectedAddress); + getDefaultGasFees = sinon + .stub(txController, '_getDefaultGasFees') + .returns({}); + getDefaultGasLimit = sinon + .stub(txController, '_getDefaultGasLimit') + .returns({}); + getPermittedAccounts = sinon + .stub(txController, 'getPermittedAccounts') + .returns([selectedAddress]); + }); + + afterEach(function () { + getSelectedAddress.restore(); + getPermittedAccounts.restore(); + getDefaultGasFees.restore(); + getDefaultGasLimit.restore(); + }); + + it('should add an cancel transaction and return a valid txMeta', async function () { + const txMeta = await txController.addUnapprovedTransaction({ + from: selectedAddress, + to: recipientAddress, + }); + await txController.approveTransaction(txMeta.id); + const cancelTxMeta = await txController.createCancelTransaction( + txMeta.id, + {}, + { actionId: 12345 }, + ); + assert.equal(cancelTxMeta.type, TRANSACTION_TYPES.CANCEL); + const memTxMeta = txController.txStateManager.getTransaction( + cancelTxMeta.id, + ); + assert.deepEqual(cancelTxMeta, memTxMeta); + }); + + it('should add only 1 cancel transaction when called twice with same actionId', async function () { + const txMeta = await txController.addUnapprovedTransaction({ + from: selectedAddress, + to: recipientAddress, + }); + await txController.approveTransaction(txMeta.id); + await txController.createCancelTransaction( + txMeta.id, + {}, + { actionId: 12345 }, + ); + const transactionCount1 = + txController.txStateManager.getTransactions().length; + await txController.createCancelTransaction( + txMeta.id, + {}, + { actionId: 12345 }, + ); + const transactionCount2 = + txController.txStateManager.getTransactions().length; + assert.equal(transactionCount1, transactionCount2); + }); + + it('should add multiple transactions when called with different actionId', async function () { + const txMeta = await txController.addUnapprovedTransaction({ + from: selectedAddress, + to: recipientAddress, + }); + await txController.approveTransaction(txMeta.id); + await txController.createCancelTransaction( + txMeta.id, + {}, + undefined, + 12345, + ); + const transactionCount1 = + txController.txStateManager.getTransactions().length; + await txController.createCancelTransaction( + txMeta.id, + {}, + { actionId: 11111 }, + ); + const transactionCount2 = + txController.txStateManager.getTransactions().length; + assert.equal(transactionCount1 + 1, transactionCount2); + }); + }); + describe('#addTxGasDefaults', function () { it('should add the tx defaults if their are none', async function () { txController.txStateManager._addTransactionsToState([ diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 4b115d83455b..b4e1e4a49647 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -3278,18 +3278,14 @@ export default class MetamaskController extends EventEmitter { * './controllers/transactions' * ).CustomGasSettings} [customGasSettings] - overrides to use for gas params * instead of allowing this method to generate them - * @param newTxMetaProps + * @param options * @returns {object} MetaMask state */ - async createCancelTransaction( - originalTxId, - customGasSettings, - newTxMetaProps, - ) { + async createCancelTransaction(originalTxId, customGasSettings, options) { await this.txController.createCancelTransaction( originalTxId, customGasSettings, - newTxMetaProps, + options, ); const state = await this.getState(); return state; diff --git a/ui/store/actions.js b/ui/store/actions.js index a05abf7257a4..25e07483e3b7 100644 --- a/ui/store/actions.js +++ b/ui/store/actions.js @@ -1974,19 +1974,16 @@ export function clearPendingTokens() { }; } -export function createCancelTransaction( - txId, - customGasSettings, - newTxMetaProps, -) { +export function createCancelTransaction(txId, customGasSettings, options) { log.debug('background.cancelTransaction'); let newTxId; return (dispatch) => { + const actionId = Date.now() + Math.random(); return new Promise((resolve, reject) => { callBackgroundMethod( 'createCancelTransaction', - [txId, customGasSettings, newTxMetaProps], + [txId, customGasSettings, { ...options, actionId }], (err, newState) => { if (err) { dispatch(displayWarning(err.message)); @@ -1999,6 +1996,7 @@ export function createCancelTransaction( newTxId = id; resolve(newState); }, + actionId, ); }) .then((newState) => dispatch(updateMetamaskState(newState)))