From a4b8477dbb4ef1daeafeab115fba0595afd78d66 Mon Sep 17 00:00:00 2001 From: Goktug Poyraz Date: Fri, 4 Aug 2023 11:15:44 +0200 Subject: [PATCH 1/6] fix: add options bag to addTransaction fn --- .../src/TransactionController.ts | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 22ad692a32..ddbf742a7e 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -124,6 +124,18 @@ export interface TransactionState extends BaseState { methodData: { [key: string]: MethodData }; } +/** + * @type AddTransactionOptions + * + * Transaction controller addTransaction method options + * @param origin - The domain origin to append to the generated TransactionMeta. + * @param deviceConfirmedOn - An enum to indicate what device the transaction was confirmed to append to the generated TransactionMeta. + */ +type AddTransactionOptions = { + origin?: string; + deviceConfirmedOn?: WalletDevice; +}; + /** * Multiplier used to determine a transaction's increased gas fee during cancellation */ @@ -335,14 +347,12 @@ export class TransactionController extends BaseController< * if not provided. If A `:unapproved` hub event will be emitted once added. * * @param transaction - The transaction object to add. - * @param origin - The domain origin to append to the generated TransactionMeta. - * @param deviceConfirmedOn - An enum to indicate what device the transaction was confirmed to append to the generated TransactionMeta. + * @param opts - The options bag for addTransaction. * @returns Object containing a promise resolving to the transaction hash if approved. */ async addTransaction( transaction: Transaction, - origin?: string, - deviceConfirmedOn?: WalletDevice, + opts: AddTransactionOptions = {}, ): Promise { const { providerConfig, networkId } = this.getNetworkState(); const { transactions } = this.state; @@ -353,11 +363,11 @@ export class TransactionController extends BaseController< id: random(), networkID: networkId ?? undefined, chainId: providerConfig.chainId, - origin, + origin: opts.origin, status: TransactionStatus.unapproved as TransactionStatus.unapproved, time: Date.now(), transaction, - deviceConfirmedOn, + deviceConfirmedOn: opts.deviceConfirmedOn, verifiedOnBlockchain: false, }; From f4f51a6c5daad3eecff6f2db1d0e125ffa39fff4 Mon Sep 17 00:00:00 2001 From: Goktug Poyraz Date: Mon, 14 Aug 2023 09:33:58 +0200 Subject: [PATCH 2/6] remove type and put inline variables --- .../src/TransactionController.ts | 22 +++++-------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index ddbf742a7e..222175bc97 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -124,18 +124,6 @@ export interface TransactionState extends BaseState { methodData: { [key: string]: MethodData }; } -/** - * @type AddTransactionOptions - * - * Transaction controller addTransaction method options - * @param origin - The domain origin to append to the generated TransactionMeta. - * @param deviceConfirmedOn - An enum to indicate what device the transaction was confirmed to append to the generated TransactionMeta. - */ -type AddTransactionOptions = { - origin?: string; - deviceConfirmedOn?: WalletDevice; -}; - /** * Multiplier used to determine a transaction's increased gas fee during cancellation */ @@ -347,12 +335,14 @@ export class TransactionController extends BaseController< * if not provided. If A `:unapproved` hub event will be emitted once added. * * @param transaction - The transaction object to add. - * @param opts - The options bag for addTransaction. + * @param opts - The options bag for addTransaction + * @param opts.deviceConfirmedOn - An enum to indicate what device the transaction was confirmed. + * @param opts.origin - The domain origin * @returns Object containing a promise resolving to the transaction hash if approved. */ async addTransaction( transaction: Transaction, - opts: AddTransactionOptions = {}, + { deviceConfirmedOn, origin } = {}, ): Promise { const { providerConfig, networkId } = this.getNetworkState(); const { transactions } = this.state; @@ -363,11 +353,11 @@ export class TransactionController extends BaseController< id: random(), networkID: networkId ?? undefined, chainId: providerConfig.chainId, - origin: opts.origin, + origin, status: TransactionStatus.unapproved as TransactionStatus.unapproved, time: Date.now(), transaction, - deviceConfirmedOn: opts.deviceConfirmedOn, + deviceConfirmedOn, verifiedOnBlockchain: false, }; From 749d3c5c271e31d19a270df95bca1a94b1edcf21 Mon Sep 17 00:00:00 2001 From: Goktug Poyraz Date: Mon, 14 Aug 2023 09:46:18 +0200 Subject: [PATCH 3/6] add test for options bag --- .../src/TransactionController.test.ts | 23 +++++++++++++++---- .../src/TransactionController.ts | 8 ++++++- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index 305168da70..632d015c64 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -25,7 +25,7 @@ import type { } from './TransactionController'; import { TransactionController, HARDFORK } from './TransactionController'; import type { TransactionMeta } from './types'; -import { TransactionStatus } from './types'; +import { WalletDevice, TransactionStatus } from './types'; import { ESTIMATE_GAS_ERROR } from './utils'; import { FakeBlockTracker } from '../../../tests/fake-block-tracker'; import { mockNetwork } from '../../../tests/mock-network'; @@ -654,10 +654,19 @@ describe('TransactionController', () => { it('adds unapproved transaction to state', async () => { const controller = newController(); - await controller.addTransaction({ - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }); + const mockDeviceConfirmedOn = WalletDevice.OTHER; + const mockOrigin = 'origin'; + + await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + deviceConfirmedOn: mockDeviceConfirmedOn, + origin: mockOrigin, + }, + ); expect(controller.state.transactions[0].transaction.from).toBe( ACCOUNT_MOCK, @@ -668,6 +677,10 @@ describe('TransactionController', () => { expect(controller.state.transactions[0].chainId).toBe( MOCK_NETWORK.state.providerConfig.chainId, ); + expect(controller.state.transactions[0].deviceConfirmedOn).toBe( + mockDeviceConfirmedOn, + ); + expect(controller.state.transactions[0].origin).toBe(mockOrigin); expect(controller.state.transactions[0].status).toBe( TransactionStatus.unapproved, ); diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 222175bc97..5e79205722 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -342,7 +342,13 @@ export class TransactionController extends BaseController< */ async addTransaction( transaction: Transaction, - { deviceConfirmedOn, origin } = {}, + { + deviceConfirmedOn, + origin, + }: { + deviceConfirmedOn?: WalletDevice; + origin?: string; + } = {}, ): Promise { const { providerConfig, networkId } = this.getNetworkState(); const { transactions } = this.state; From b5b195c23610ce99409fcb7adc37d55b078a58b0 Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Mon, 14 Aug 2023 12:58:47 +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 5e79205722..c0d80e0811 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -337,7 +337,7 @@ export class TransactionController extends BaseController< * @param transaction - The transaction object to add. * @param opts - The options bag for addTransaction * @param opts.deviceConfirmedOn - An enum to indicate what device the transaction was confirmed. - * @param opts.origin - The domain origin + * @param opts.origin - The origin of the transaction request, such as a dApp hostname. * @returns Object containing a promise resolving to the transaction hash if approved. */ async addTransaction( From 6b22cc9b3cb5eaaaa097b095586e3efbdd898260 Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Mon, 14 Aug 2023 13:06:18 +0200 Subject: [PATCH 5/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 704e6cf187..42c884e8a7 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -336,7 +336,7 @@ export class TransactionController extends BaseController< * * @param transaction - The transaction object to add. * @param opts - The options bag for addTransaction - * @param opts.deviceConfirmedOn - An enum to indicate what device the transaction was confirmed. + * @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. * @returns Object containing a promise resolving to the transaction hash if approved. */ From 0995d269c19798c603c73e3af0e7b634ff7d5dae Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Mon, 14 Aug 2023 13:06:25 +0200 Subject: [PATCH 6/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 42c884e8a7..4c089ffeec 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -335,7 +335,7 @@ export class TransactionController extends BaseController< * if not provided. If A `:unapproved` hub event will be emitted once added. * * @param transaction - The transaction object to add. - * @param opts - The options bag for addTransaction + * @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. * @returns Object containing a promise resolving to the transaction hash if approved.