Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add firstTimeInteraction to transactionMeta #4895

Merged
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions packages/transaction-controller/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ module.exports = merge(baseConfig, {
// An object that configures minimum threshold enforcement for coverage results
coverageThreshold: {
global: {
branches: 93.74,
functions: 97.51,
lines: 98.34,
statements: 98.35,
branches: 93.4,
functions: 97.32,
lines: 98.26,
statements: 98.27,
},
},

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import {
buildCustomNetworkClientConfiguration,
buildMockGetNetworkClientById,
} from '../../network-controller/tests/helpers';
import { getAccountAddressRelationship } from './api/accounts-api';
import { CHAIN_IDS } from './constants';
import { DefaultGasFeeFlow } from './gas-flows/DefaultGasFeeFlow';
import { LineaGasFeeFlow } from './gas-flows/LineaGasFeeFlow';
Expand Down Expand Up @@ -103,6 +104,7 @@ const MOCK_V1_UUID = '9b1deb4d-3b7d-4bad-9bdd-2b0d7b3dcb6d';
const TRANSACTION_HASH_MOCK = '0x123456';

jest.mock('@metamask/eth-query');
jest.mock('./api/accounts-api');
jest.mock('./gas-flows/DefaultGasFeeFlow');
jest.mock('./gas-flows/LineaGasFeeFlow');
jest.mock('./gas-flows/TestGasFeeFlow');
Expand Down Expand Up @@ -488,6 +490,9 @@ describe('TransactionController', () => {
);
const getGasFeeFlowMock = jest.mocked(getGasFeeFlow);
const shouldResimulateMock = jest.mocked(shouldResimulate);
const getAccountAddressRelationshipMock = jest.mocked(
getAccountAddressRelationship,
);

let mockEthQuery: EthQuery;
let getNonceLockSpy: jest.Mock;
Expand Down Expand Up @@ -870,6 +875,11 @@ describe('TransactionController', () => {
updateSwapsTransactionMock.mockImplementation(
(transactionMeta) => transactionMeta,
);

getAccountAddressRelationshipMock.mockResolvedValue({
isFirstTimeInteraction: undefined,
isFirstTimeInteractionDisabled: false,
});
});

describe('constructor', () => {
Expand Down Expand Up @@ -1378,6 +1388,11 @@ describe('TransactionController', () => {
it('adds unapproved transaction to state', async () => {
const { controller } = setupController();

getAccountAddressRelationshipMock.mockResolvedValueOnce({
isFirstTimeInteraction: true,
isFirstTimeInteractionDisabled: false,
});

const mockDeviceConfirmedOn = WalletDevice.OTHER;
const mockOrigin = 'origin';
const mockSecurityAlertResponse = {
Expand Down Expand Up @@ -1412,6 +1427,8 @@ describe('TransactionController', () => {
},
);

await flushPromises();

const transactionMeta = controller.state.transactions[0];

expect(updateSwapsTransactionMock).toHaveBeenCalledTimes(1);
Expand All @@ -1426,6 +1443,58 @@ describe('TransactionController', () => {
expect(controller.state.transactions[0].sendFlowHistory).toStrictEqual(
mockSendFlowHistory,
);
expect(controller.state.transactions[0].isFirstTimeInteraction).toBe(
true,
);
});

it('does not check account address relationship if a transaction with the same from, to, and chainId exists', async () => {
const { controller } = setupController({
options: {
state: {
transactions: [
{
id: '1',
chainId: MOCK_NETWORK.chainId,
status: TransactionStatus.confirmed as const,
time: 123456789,
txParams: {
from: ACCOUNT_MOCK,
to: ACCOUNT_MOCK,
},
isFirstTimeInteraction: false, // Ensure this is set
},
],
},
},
});

// Add second transaction with the same from, to, and chainId
await controller.addTransaction({
from: ACCOUNT_MOCK,
to: ACCOUNT_MOCK,
});

await flushPromises();

expect(controller.state.transactions[1].isFirstTimeInteraction).toBe(
false,
);
});

it('does not update first time interaction properties if disabled', async () => {
const { controller } = setupController({
options: { isFirstTimeInteractionEnabled: () => false },
});

await controller.addTransaction({
from: ACCOUNT_MOCK,
to: ACCOUNT_MOCK,
});

await flushPromises();

expect(getAccountAddressRelationshipMock).not.toHaveBeenCalled();
});

describe('networkClientId exists in the MultichainTrackingHelper', () => {
Expand Down Expand Up @@ -1512,6 +1581,8 @@ describe('TransactionController', () => {
dappSuggestedGasFees: undefined,
deviceConfirmedOn: undefined,
id: expect.any(String),
isFirstTimeInteraction: false,
isFirstTimeInteractionDisabled: false,
networkClientId: MOCK_NETWORK.state.selectedNetworkClientId,
origin: undefined,
securityAlertResponse: undefined,
Expand Down Expand Up @@ -2211,7 +2282,7 @@ describe('TransactionController', () => {

const mockActionId = 'mockActionId';

const { result, transactionMeta } = await controller.addTransaction(
const { result } = await controller.addTransaction(
{
from: ACCOUNT_MOCK,
to: ACCOUNT_MOCK,
Expand All @@ -2231,10 +2302,14 @@ describe('TransactionController', () => {
await finishedPromise;

expect(rejectedEventListener).toHaveBeenCalledTimes(1);
expect(rejectedEventListener).toHaveBeenCalledWith({
transactionMeta: { ...transactionMeta, status: 'rejected' },
actionId: mockActionId,
});
expect(rejectedEventListener).toHaveBeenCalledWith(
expect.objectContaining({
transactionMeta: expect.objectContaining({
status: 'rejected',
}),
actionId: mockActionId,
}),
);
});
});

Expand Down
104 changes: 100 additions & 4 deletions packages/transaction-controller/src/TransactionController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,17 @@ import type {
import { NonceTracker } from '@metamask/nonce-tracker';
import { errorCodes, rpcErrors, providerErrors } from '@metamask/rpc-errors';
import type { Hex } from '@metamask/utils';
import { add0x } from '@metamask/utils';
import { add0x, hexToNumber } from '@metamask/utils';
import { Mutex } from 'async-mutex';
import { MethodRegistry } from 'eth-method-registry';
import { EventEmitter } from 'events';
import { cloneDeep, mapValues, merge, pickBy, sortBy } from 'lodash';
import { v1 as random } from 'uuid';

import {
getAccountAddressRelationship,
type GetAccountAddressRelationshipRequest,
} from './api/accounts-api';
import { DefaultGasFeeFlow } from './gas-flows/DefaultGasFeeFlow';
import { LineaGasFeeFlow } from './gas-flows/LineaGasFeeFlow';
import { OptimismLayer1GasFeeFlow } from './gas-flows/OptimismLayer1GasFeeFlow';
Expand Down Expand Up @@ -123,6 +127,7 @@ import {
normalizeGasFeeValues,
} from './utils/utils';
import {
validateAccountAddressRelationshipRequest,
validateTransactionOrigin,
validateTxParams,
} from './utils/validation';
Expand Down Expand Up @@ -297,6 +302,7 @@ export type TransactionControllerOptions = {
etherscanApiKeysByChainId?: Record<Hex, string>;
};
isMultichainEnabled: boolean;
isFirstTimeInteractionEnabled?: () => boolean;
isSimulationEnabled?: () => boolean;
messenger: TransactionControllerMessenger;
onNetworkStateChange: (listener: (state: NetworkState) => void) => void;
Expand Down Expand Up @@ -638,6 +644,8 @@ export class TransactionController extends BaseController<

#transactionHistoryLimit: number;

#isFirstTimeInteractionEnabled: () => boolean;

#isSimulationEnabled: () => boolean;

#testGasFeeFlows: boolean;
Expand Down Expand Up @@ -756,6 +764,7 @@ export class TransactionController extends BaseController<
* @param options.getSavedGasFees - Gets the saved gas fee config.
* @param options.incomingTransactions - Configuration options for incoming transaction support.
* @param options.isMultichainEnabled - Enable multichain support.
* @param options.isFirstTimeInteractionEnabled - Whether first time interaction checks are enabled.
* @param options.isSimulationEnabled - Whether new transactions will be automatically simulated.
* @param options.messenger - The controller messenger.
* @param options.onNetworkStateChange - Allows subscribing to network controller state changes.
Expand Down Expand Up @@ -784,6 +793,7 @@ export class TransactionController extends BaseController<
getSavedGasFees,
incomingTransactions = {},
isMultichainEnabled = false,
isFirstTimeInteractionEnabled,
isSimulationEnabled,
messenger,
onNetworkStateChange,
Expand Down Expand Up @@ -812,6 +822,8 @@ export class TransactionController extends BaseController<
this.isSendFlowHistoryDisabled = disableSendFlowHistory ?? false;
this.isHistoryDisabled = disableHistory ?? false;
this.isSwapsDisabled = disableSwaps ?? false;
this.#isFirstTimeInteractionEnabled =
isFirstTimeInteractionEnabled ?? (() => true);
this.#isSimulationEnabled = isSimulationEnabled ?? (() => true);
// @ts-expect-error the type in eth-method-registry is inappropriate and should be changed
this.registry = new MethodRegistry({ provider });
Expand Down Expand Up @@ -1090,15 +1102,17 @@ export class TransactionController extends BaseController<
dappSuggestedGasFees,
deviceConfirmedOn,
id: random(),
isFirstTimeInteraction: false,
isFirstTimeInteractionDisabled: false,
networkClientId,
origin,
securityAlertResponse,
status: TransactionStatus.unapproved as const,
time: Date.now(),
txParams,
type: transactionType,
userEditedGasLimit: false,
verifiedOnBlockchain: false,
type: transactionType,
networkClientId,
};

await this.#trace(
Expand Down Expand Up @@ -1149,8 +1163,16 @@ export class TransactionController extends BaseController<
log('Error while updating simulation data', error);
throw error;
});

this.#updateFirstInteractionProperties(addedTransactionMeta, {
traceContext,
}).catch((error) => {
log('Error while updating first interaction properties', error);
});
} else {
log('Skipping simulation as approval not required');
log(
'Skipping simulation & first interaction update as approval not required',
);
}

this.messagingSystem.publish(
Expand Down Expand Up @@ -3614,6 +3636,80 @@ export class TransactionController extends BaseController<
return transactionMeta;
}

async #updateFirstInteractionProperties(
transactionMeta: TransactionMeta,
{
traceContext,
}: {
traceContext?: TraceContext;
} = {},
) {
if (!this.#isFirstTimeInteractionEnabled()) {
return;
}

const {
chainId,
id: transactionId,
txParams: { to, from },
} = transactionMeta;

const request: GetAccountAddressRelationshipRequest = {
chainId: hexToNumber(chainId),
to: to as string, // This is validated in validateAccountAddressRelationshipRequest
from,
};

validateAccountAddressRelationshipRequest(request);

const existingTransaction = this.state.transactions.find(
(tx) =>
tx.chainId === chainId &&
tx.txParams.from === from &&
tx.txParams.to === to &&
tx.id !== transactionId,
);

// Check if there is an existing transaction with the same from, to, and chainId
// else we continue to check the account address relationship from API
if (existingTransaction) {
return;
}

const { isFirstTimeInteractionDisabled, isFirstTimeInteraction } =
await this.#trace(
{ name: 'Account Address Relationship', parentContext: traceContext },
() => getAccountAddressRelationship(request),
);

const finalTransactionMeta = this.getTransaction(transactionId);

/* istanbul ignore if */
if (!finalTransactionMeta) {
log(
'Cannot update first time interaction properties as transaction not found',
transactionId,
);
return;
}

this.#updateTransactionInternal(
{
transactionId,
note: 'TransactionController#updateFirstInteraction - Update first time interaction',
},
(txMeta) => {
txMeta.isFirstTimeInteraction = isFirstTimeInteraction;
txMeta.isFirstTimeInteractionDisabled = isFirstTimeInteractionDisabled;
},
);

log('Updated first time interaction properties', transactionId, {
isFirstTimeInteractionDisabled,
isFirstTimeInteraction,
});
}

async #updateSimulationData(
transactionMeta: TransactionMeta,
{
Expand Down
Loading
Loading