From f9ae7d8b51a995a739d26958666e6e0ab7316134 Mon Sep 17 00:00:00 2001 From: Goktug Poyraz Date: Mon, 14 Aug 2023 14:56:00 +0200 Subject: [PATCH 1/6] add requireApproval option to addTransaction method option bag --- .../src/TransactionController.test.ts | 16 ++++++++++++++++ .../src/TransactionController.ts | 19 +++++++++++++------ 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index 1f4649a808..ab85e28b1c 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -809,6 +809,22 @@ describe('TransactionController', () => { ); }); + it('skip request for an approval using the approval controller', async () => { + const controller = newController(); + + await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + requireApproval: false, + }, + ); + + expect(delayMessengerMock.call).toHaveBeenCalledTimes(0); + }); + it.each([ ['mainnet', MOCK_MAINNET_NETWORK], ['custom network', MOCK_CUSTOM_NETWORK], diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 2fe455791b..f0674bade9 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -340,6 +340,7 @@ export class TransactionController extends BaseController< * @param opts - Additional options to control how the transaction is added. * @param opts.deviceConfirmedOn - An enum to indicate what device confirmed the transaction. * @param opts.origin - The origin of the transaction request, such as a dApp hostname. + * @param opts.requireApproval - Whether to skip creating approval for transactions. * @returns Object containing a promise resolving to the transaction hash if approved. */ async addTransaction( @@ -347,9 +348,11 @@ export class TransactionController extends BaseController< { deviceConfirmedOn, origin, + requireApproval = true, }: { deviceConfirmedOn?: WalletDevice; origin?: string; + requireApproval?: boolean; } = {}, ): Promise { const { chainId, networkId } = this.getChainAndNetworkId(); @@ -383,7 +386,9 @@ export class TransactionController extends BaseController< this.hub.emit(`unapprovedTransaction`, transactionMeta); return { - result: this.processApproval(transactionMeta), + result: this.processApproval(transactionMeta, { + requireApproval: Boolean(requireApproval), + }), transactionMeta, }; } @@ -886,16 +891,18 @@ export class TransactionController extends BaseController< private async processApproval( transactionMeta: TransactionMeta, - { shouldShowRequest = true } = {}, + { requireApproval, shouldShowRequest = true }: { requireApproval: boolean }, ): Promise { const transactionId = transactionMeta.id; let resultCallbacks: AcceptResultCallbacks | undefined; try { - const acceptResult = await this.requestApproval(transactionMeta, { - shouldShowRequest, - }); - resultCallbacks = acceptResult.resultCallbacks; + if (requireApproval) { + const acceptResult = await this.requestApproval(transactionMeta, { + shouldShowRequest, + }); + resultCallbacks = acceptResult.resultCallbacks; + } const { meta, isCompleted } = this.isTransactionCompleted(transactionId); From 80ebab7704ea1c872fec3be4ebdea34e36d39bb8 Mon Sep 17 00:00:00 2001 From: Goktug Poyraz Date: Tue, 15 Aug 2023 09:17:44 +0200 Subject: [PATCH 2/6] fix test case --- .../transaction-controller/src/TransactionController.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index ab85e28b1c..ef5c73ab6f 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -809,7 +809,7 @@ describe('TransactionController', () => { ); }); - it('skip request for an approval using the approval controller', async () => { + it('skips request for an approval using the approval controller', async () => { const controller = newController(); await controller.addTransaction( From 09f31adcb86e4f7176ef15822ea24fe711a87111 Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Wed, 16 Aug 2023 11:13:50 +0200 Subject: [PATCH 3/6] Update packages/transaction-controller/src/TransactionController.test.ts Co-authored-by: Matthew Walsh --- .../transaction-controller/src/TransactionController.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index ef5c73ab6f..e597c13e34 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -809,7 +809,7 @@ describe('TransactionController', () => { ); }); - it('skips request for an approval using the approval controller', async () => { + it('skips approval if option explicitly false', async () => { const controller = newController(); await controller.addTransaction( From 42dd9b8b4e12bd98a2d224a0d6dc8acba58a1170 Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Wed, 16 Aug 2023 11:13:58 +0200 Subject: [PATCH 4/6] Update packages/transaction-controller/src/TransactionController.ts Co-authored-by: Matthew Walsh --- packages/transaction-controller/src/TransactionController.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index f0674bade9..ca53287f50 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -340,7 +340,7 @@ export class TransactionController extends BaseController< * @param opts - Additional options to control how the transaction is added. * @param opts.deviceConfirmedOn - An enum to indicate what device confirmed the transaction. * @param opts.origin - The origin of the transaction request, such as a dApp hostname. - * @param opts.requireApproval - Whether to skip creating approval for transactions. + * @param opts.requireApproval - Whether the transaction requires approval by the user, defaults to true unless explicitly disabled. * @returns Object containing a promise resolving to the transaction hash if approved. */ async addTransaction( From 9dff14568e376567bcb16efabd6eb7c321e9401a Mon Sep 17 00:00:00 2001 From: Goktug Poyraz Date: Wed, 16 Aug 2023 11:31:20 +0200 Subject: [PATCH 5/6] fix suggestion --- .../src/TransactionController.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index ca53287f50..a78c3613bd 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -348,11 +348,11 @@ export class TransactionController extends BaseController< { deviceConfirmedOn, origin, - requireApproval = true, + requireApproval, }: { deviceConfirmedOn?: WalletDevice; origin?: string; - requireApproval?: boolean; + requireApproval?: boolean | undefined; } = {}, ): Promise { const { chainId, networkId } = this.getChainAndNetworkId(); @@ -387,7 +387,7 @@ export class TransactionController extends BaseController< return { result: this.processApproval(transactionMeta, { - requireApproval: Boolean(requireApproval), + requireApproval, }), transactionMeta, }; @@ -891,13 +891,13 @@ export class TransactionController extends BaseController< private async processApproval( transactionMeta: TransactionMeta, - { requireApproval, shouldShowRequest = true }: { requireApproval: boolean }, + { requireApproval, shouldShowRequest = true }: { requireApproval: boolean | undefined }, ): Promise { const transactionId = transactionMeta.id; let resultCallbacks: AcceptResultCallbacks | undefined; try { - if (requireApproval) { + if (requireApproval !== false) { const acceptResult = await this.requestApproval(transactionMeta, { shouldShowRequest, }); From f7b006dbe1799f98a1280e927f68d880b5244f67 Mon Sep 17 00:00:00 2001 From: Goktug Poyraz Date: Wed, 16 Aug 2023 11:46:29 +0200 Subject: [PATCH 6/6] fix suggestion --- .../transaction-controller/src/TransactionController.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index a78c3613bd..51f018a268 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -891,7 +891,13 @@ export class TransactionController extends BaseController< private async processApproval( transactionMeta: TransactionMeta, - { requireApproval, shouldShowRequest = true }: { requireApproval: boolean | undefined }, + { + requireApproval, + shouldShowRequest = true, + }: { + requireApproval?: boolean | undefined; + shouldShowRequest?: boolean; + }, ): Promise { const transactionId = transactionMeta.id; let resultCallbacks: AcceptResultCallbacks | undefined;