From 33f9d6f480b324ecaa5e825d7f17d16a48a0bcdc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Albert=20Oliv=C3=A9?= Date: Fri, 5 May 2023 15:58:40 +0200 Subject: [PATCH] [MMI] Applied code fencing in transactions and pending tx tracker controllers (#17909) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Applied code fencing in trabsactions and pending tx tracker controllers * remove unneeded URL from the comment * Added test * Added useful comment * Fixing code fences --------- Co-authored-by: António Regadas --- app/scripts/controllers/transactions/index.js | 49 +++++++++++++++++-- .../transactions/pending-tx-tracker.js | 17 ++++++- .../transactions/pending-tx-tracker.test.js | 40 +++++++++++++++ 3 files changed, 101 insertions(+), 5 deletions(-) diff --git a/app/scripts/controllers/transactions/index.js b/app/scripts/controllers/transactions/index.js index 5b393963c..032ed4225 100644 --- a/app/scripts/controllers/transactions/index.js +++ b/app/scripts/controllers/transactions/index.js @@ -225,6 +225,10 @@ export default class TransactionController extends EventEmitter { // request state update to finalize initialization this._updatePendingTxsAfterFirstBlock(); this._onBootCleanUp(); + + ///: BEGIN:ONLY_INCLUDE_IN(build-mmi) + this.transactionUpdateController = opts.transactionUpdateController; + ///: END:ONLY_INCLUDE_IN } /** @@ -1359,6 +1363,16 @@ export default class TransactionController extends EventEmitter { // So that we do not increment nonce + resubmit something // that is already being incremented & signed. const txMeta = this.txStateManager.getTransaction(txId); + + ///: BEGIN:ONLY_INCLUDE_IN(build-mmi) + // MMI does not broadcast transactions, as that is the responsibility of the custodian + if (txMeta.custodyStatus) { + this.inProcessOfSigning.delete(txId); + await this.signTransaction(txId); + return; + } + ///: END:ONLY_INCLUDE_IN + if (this.inProcessOfSigning.has(txId)) { return; } @@ -1512,7 +1526,25 @@ export default class TransactionController extends EventEmitter { const fromAddress = txParams.from; const common = await this.getCommonConfiguration(txParams.from); const unsignedEthTx = TransactionFactory.fromTxData(txParams, { common }); - const signedEthTx = await this.signEthTx(unsignedEthTx, fromAddress); + const signedEthTx = await this.signEthTx( + unsignedEthTx, + fromAddress, + ///: BEGIN:ONLY_INCLUDE_IN(build-mmi) + txMeta.custodyStatus ? txMeta : undefined, + ///: END:ONLY_INCLUDE_IN + ); + + ///: BEGIN:ONLY_INCLUDE_IN(build-mmi) + if (txMeta.custodyStatus) { + txMeta.custodyId = signedEthTx.custodian_transactionId; + txMeta.custodyStatus = signedEthTx.transactionStatus; + + this.transactionUpdateController.addTransactionToWatchList( + txMeta.custodyId, + fromAddress, + ); + } + ///: END:ONLY_INCLUDE_IN // add r,s,v values for provider request purposes see createMetamaskMiddleware // and JSON rpc standard for further explanation @@ -1876,9 +1908,18 @@ export default class TransactionController extends EventEmitter { }, }) .forEach((txMeta) => { - // Line below will try to publish transaction which is in - // APPROVED state at the time of controller bootup - this.approveTransaction(txMeta.id); + ///: BEGIN:ONLY_INCLUDE_IN(build-mmi) + // If you create a Tx and its still inside the custodian waiting to be approved we don't want to approve it right away + if (!txMeta.custodyStatus) { + ///: END:ONLY_INCLUDE_IN + + // Line below will try to publish transaction which is in + // APPROVED state at the time of controller bootup + this.approveTransaction(txMeta.id); + + ///: BEGIN:ONLY_INCLUDE_IN(build-mmi) + } + ///: END:ONLY_INCLUDE_IN }); } diff --git a/app/scripts/controllers/transactions/pending-tx-tracker.js b/app/scripts/controllers/transactions/pending-tx-tracker.js index 93b1de252..403c94ace 100644 --- a/app/scripts/controllers/transactions/pending-tx-tracker.js +++ b/app/scripts/controllers/transactions/pending-tx-tracker.js @@ -144,6 +144,13 @@ export default class PendingTransactionTracker extends EventEmitter { return undefined; } + ///: BEGIN:ONLY_INCLUDE_IN(build-mmi) + // Don't ever resubmit custodian transactions + if (txMeta.custodyId) { + return undefined; + } + ///: END:ONLY_INCLUDE_IN + // Only auto-submit already-signed txs: if (!('rawTx' in txMeta)) { return this.approveTransaction(txMeta.id); @@ -180,7 +187,15 @@ export default class PendingTransactionTracker extends EventEmitter { // extra check in case there was an uncaught error during the // signature and submission process - if (!txHash) { + + let hasNoHash = !txHash; + + ///: BEGIN:ONLY_INCLUDE_IN(build-mmi) + // Don't emit noTxHashErr for custodian transactions + hasNoHash ||= !txMeta.custodyId; + ///: END:ONLY_INCLUDE_IN + + if (hasNoHash) { const noTxHashErr = new Error( 'We had an error while submitting this transaction, please try again.', ); diff --git a/app/scripts/controllers/transactions/pending-tx-tracker.test.js b/app/scripts/controllers/transactions/pending-tx-tracker.test.js index 7bbed9687..13d812eeb 100644 --- a/app/scripts/controllers/transactions/pending-tx-tracker.test.js +++ b/app/scripts/controllers/transactions/pending-tx-tracker.test.js @@ -374,6 +374,43 @@ describe('PendingTransactionTracker', function () { 'should NOT try to publish transaction', ); }); + + it('should return undefined if txMeta has custodyId property', async function () { + const txMeta = { + custodyId: 1, + id: 1, + hash: '0x0593ee121b92e10d63150ad08b4b8f9c7857d1bd160195ee648fb9a0f8d00eeb', + status: TransactionStatus.signed, + txParams: { + from: '0x1678a085c290ebd122dc42cba69373b5953b831d', + nonce: '0x1', + value: '0xfffff', + }, + history: [{}], + rawTx: + '0xf86c808504a817c80086a02ec9d3d1d6e176d4d2593dd760e74ccac753e6a0ea0d00cc9789d0d7ff1f471d', + }; + const approveTransaction = sinon.spy(); + const publishTransaction = sinon.spy(); + const pendingTxTracker = new PendingTransactionTracker({ + query: { + getTransactionReceipt: sinon.stub(), + }, + nonceTracker: { + getGlobalLock: sinon.stub().resolves({ + releaseLock: sinon.spy(), + }), + }, + getPendingTransactions: sinon.stub().returns([]), + getCompletedTransactions: sinon.stub().returns([]), + approveTransaction, + publishTransaction, + confirmTransaction: sinon.spy(), + }); + + const result = await pendingTxTracker._resubmitTx(txMeta); + assert.equal(result, undefined); + }); }); describe('#_checkIfTxWasDropped', function () { @@ -576,6 +613,7 @@ describe('PendingTransactionTracker', function () { }, history: [{}], rawTx: '0xf86c808504a817c80082471d', + custodyId: 'testid', }; const nonceBN = new BN(2); const pendingTxTracker = new PendingTransactionTracker({ @@ -720,6 +758,7 @@ describe('PendingTransactionTracker', function () { id: '123', value: '0x02', hash: '0x2a919d2512ec963f524bfd9730fb66b6d5a2e399d1dd957abb5e2b544a12644b', + custodyId: 'testid', }, ]; const pendingTxTracker = new PendingTransactionTracker({ @@ -771,6 +810,7 @@ describe('PendingTransactionTracker', function () { }, history: [{}], rawTx: '0xf86c808504a817c80082471d', + custodyId: 'testid', }; const pendingTxTracker = new PendingTransactionTracker({ query: {