Skip to content

Commit 5107e96

Browse files
authored
feat: trace Bridge transactions and quote fetching (#5768)
## Explanation Draft integration for extension: MetaMask/metamask-extension#32722 Sentry Dashboard: https://metamask.sentry.io/dashboard/131851/?statsPeriod=1d <!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? * Are there any changes whose purpose might not obvious to those unfamiliar with the domain? * If your primary goal was to update one package but you found you had to update another one along the way, why did you do so? * If you had to upgrade a dependency, why did you do so? --> ## References <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? Are there client or consumer pull requests to adopt any breaking changes? For example: * Fixes #12345 * Related to #67890 --> ## Changelog <!-- THIS SECTION IS NO LONGER NEEDED. The process for updating changelogs has changed. Please consult the "Updating changelogs" section of the Contributing doc for more. --> ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs), highlighting breaking changes as necessary - [ ] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes
1 parent 9e46628 commit 5107e96

File tree

12 files changed

+236
-43
lines changed

12 files changed

+236
-43
lines changed

packages/bridge-controller/CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Added
11+
12+
- Sentry traces for `BridgeQuotesFetched` and `SwapQuotesFetched` events ([#5780](https://github.com/MetaMask/core/pull/5780))
13+
- Export `isCrossChain` utility ([#5780](https://github.com/MetaMask/core/pull/5780))
14+
1015
## [23.0.0]
1116

1217
### Changed
1318

19+
- **BREAKING:** Remove `BridgeToken` export ([#5768](https://github.com/MetaMask/core/pull/5768))
1420
- **BREAKING** Rename `QuoteResponse.bridgePriceData` to `priceData` ([#5784](https://github.com/MetaMask/core/pull/5784))
21+
- `traceFn` added to BridgeController constructor to enable clients to pass in a custom sentry trace handler ([#5768](https://github.com/MetaMask/core/pull/5768))
1522

1623
## [22.0.0]
1724

packages/bridge-controller/src/bridge-controller.ts

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import type { BigNumber } from '@ethersproject/bignumber';
22
import { Contract } from '@ethersproject/contracts';
33
import { Web3Provider } from '@ethersproject/providers';
44
import type { StateMetadata } from '@metamask/base-controller';
5-
import type { ChainId } from '@metamask/controller-utils';
5+
import type { ChainId, TraceCallback } from '@metamask/controller-utils';
66
import { SolScope } from '@metamask/keyring-api';
77
import { abiERC20 } from '@metamask/metamask-eth-abis';
88
import type { NetworkClientId } from '@metamask/network-controller';
@@ -20,6 +20,7 @@ import {
2020
REFRESH_INTERVAL_MS,
2121
} from './constants/bridge';
2222
import { CHAIN_IDS } from './constants/chains';
23+
import { TraceName } from './constants/traces';
2324
import { selectIsAssetExchangeRateInState } from './selectors';
2425
import type { QuoteRequest } from './types';
2526
import {
@@ -37,6 +38,7 @@ import { getAssetIdsForToken, toExchangeRates } from './utils/assets';
3738
import { hasSufficientBalance } from './utils/balance';
3839
import {
3940
getDefaultBridgeControllerState,
41+
isCrossChain,
4042
isSolanaChainId,
4143
sumHexes,
4244
} from './utils/bridge';
@@ -151,6 +153,8 @@ export class BridgeController extends StaticIntervalPollingController<BridgePoll
151153
properties: CrossChainSwapsEventProperties<T>,
152154
) => void;
153155

156+
readonly #trace: TraceCallback;
157+
154158
readonly #config: {
155159
customBridgeApiBaseUrl?: string;
156160
};
@@ -163,6 +167,7 @@ export class BridgeController extends StaticIntervalPollingController<BridgePoll
163167
fetchFn,
164168
config,
165169
trackMetaMetricsFn,
170+
traceFn,
166171
}: {
167172
messenger: BridgeControllerMessenger;
168173
state?: Partial<BridgeControllerState>;
@@ -182,6 +187,7 @@ export class BridgeController extends StaticIntervalPollingController<BridgePoll
182187
eventName: T,
183188
properties: CrossChainSwapsEventProperties<T>,
184189
) => void;
190+
traceFn?: TraceCallback;
185191
}) {
186192
super({
187193
name: BRIDGE_CONTROLLER_NAME,
@@ -201,6 +207,7 @@ export class BridgeController extends StaticIntervalPollingController<BridgePoll
201207
this.#fetchFn = fetchFn;
202208
this.#trackMetaMetricsFn = trackMetaMetricsFn;
203209
this.#config = config ?? {};
210+
this.#trace = traceFn ?? (((_request, fn) => fn?.()) as TraceCallback);
204211

205212
// Register action handlers
206213
this.messagingSystem.registerActionHandler(
@@ -453,7 +460,7 @@ export class BridgeController extends StaticIntervalPollingController<BridgePoll
453460
state.quoteFetchError = DEFAULT_BRIDGE_CONTROLLER_STATE.quoteFetchError;
454461
});
455462

456-
try {
463+
const fetchQuotes = async () => {
457464
const quotes = await fetchBridgeQuotes(
458465
updatedQuoteRequest,
459466
// AbortController is always defined by this line, because we assign it a few lines above,
@@ -473,6 +480,24 @@ export class BridgeController extends StaticIntervalPollingController<BridgePoll
473480
state.quotes = quotesWithL1GasFees ?? quotesWithSolanaFees ?? quotes;
474481
state.quotesLoadingStatus = RequestStatus.FETCHED;
475482
});
483+
};
484+
485+
try {
486+
await this.#trace(
487+
{
488+
name: isCrossChain(
489+
updatedQuoteRequest.srcChainId,
490+
updatedQuoteRequest.destChainId,
491+
)
492+
? TraceName.BridgeQuotesFetched
493+
: TraceName.SwapQuotesFetched,
494+
data: {
495+
srcChainId: formatChainIdToCaip(updatedQuoteRequest.srcChainId),
496+
destChainId: formatChainIdToCaip(updatedQuoteRequest.destChainId),
497+
},
498+
},
499+
fetchQuotes,
500+
);
476501
} catch (error) {
477502
const isAbortError = (error as Error).name === 'AbortError';
478503
const isAbortedDueToReset = error === RESET_STATE_ABORT_MESSAGE;
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
export enum TraceName {
2+
BridgeQuotesFetched = 'Bridge Quotes Fetched',
3+
SwapQuotesFetched = 'Swap Quotes Fetched',
4+
}

packages/bridge-controller/src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ export type {
2828
L1GasFees,
2929
SolanaFees,
3030
QuoteMetadata,
31-
BridgeToken,
3231
GasMultiplierByChainId,
3332
FeatureFlagResponse,
3433
BridgeAsset,
@@ -101,6 +100,7 @@ export {
101100
isSolanaChainId,
102101
getNativeAssetForChainId,
103102
getDefaultBridgeControllerState,
103+
isCrossChain,
104104
} from './utils/bridge';
105105

106106
export { isValidQuoteRequest, formatEtaInMinutes } from './utils/quote';

packages/bridge-controller/src/utils/bridge.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import type { Hex } from '@metamask/utils';
66
import {
77
getEthUsdtResetData,
88
getNativeAssetForChainId,
9+
isCrossChain,
910
isEthUsdt,
1011
isSolanaChainId,
1112
isSwapsDefaultTokenAddress,
@@ -202,4 +203,21 @@ describe('Bridge utils', () => {
202203
expect(decimalResult).toStrictEqual(stringifiedDecimalResult);
203204
});
204205
});
206+
207+
describe('isCrossChain', () => {
208+
it('should return false when there is no destChainId', () => {
209+
const result = isCrossChain('0x1');
210+
expect(result).toBe(false);
211+
});
212+
213+
it('should return false when srcChainId is invalid', () => {
214+
const result = isCrossChain('a', '0x1');
215+
expect(result).toBe(false);
216+
});
217+
218+
it('should return false when destChainId is invalid', () => {
219+
const result = isCrossChain('0x1', 'a');
220+
expect(result).toBe(false);
221+
});
222+
});
205223
});

packages/bridge-controller/src/utils/bridge.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,11 @@ import {
2121
SYMBOL_TO_SLIP44_MAP,
2222
type SupportedSwapsNativeCurrencySymbols,
2323
} from '../constants/tokens';
24-
import type { BridgeAsset, BridgeControllerState } from '../types';
24+
import type {
25+
BridgeAsset,
26+
BridgeControllerState,
27+
GenericQuoteRequest,
28+
} from '../types';
2529
import { ChainId } from '../types';
2630

2731
export const getDefaultBridgeControllerState = (): BridgeControllerState => {
@@ -175,3 +179,24 @@ export const isSolanaChainId = (
175179
}
176180
return chainId.toString() === ChainId.SOLANA.toString();
177181
};
182+
183+
/**
184+
* Checks whether the transaction is a cross-chain transaction by comparing the source and destination chainIds
185+
*
186+
* @param srcChainId - The source chainId
187+
* @param destChainId - The destination chainId
188+
* @returns Whether the transaction is a cross-chain transaction
189+
*/
190+
export const isCrossChain = (
191+
srcChainId: GenericQuoteRequest['srcChainId'],
192+
destChainId?: GenericQuoteRequest['destChainId'],
193+
) => {
194+
try {
195+
if (!destChainId) {
196+
return false;
197+
}
198+
return formatChainIdToCaip(srcChainId) !== formatChainIdToCaip(destChainId);
199+
} catch {
200+
return false;
201+
}
202+
};

packages/bridge-controller/src/utils/metrics/properties.ts

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import type { AccountsControllerState } from '../../../../accounts-controller/sr
66
import { DEFAULT_BRIDGE_CONTROLLER_STATE } from '../../constants/bridge';
77
import type { BridgeControllerState, QuoteResponse, TxData } from '../../types';
88
import { type GenericQuoteRequest, type QuoteRequest } from '../../types';
9-
import { getNativeAssetForChainId } from '../bridge';
9+
import { getNativeAssetForChainId, isCrossChain } from '../bridge';
1010
import {
1111
formatAddressToAssetId,
1212
formatChainIdToCaip,
@@ -49,11 +49,7 @@ export const getActionType = (
4949
srcChainId?: GenericQuoteRequest['srcChainId'],
5050
destChainId?: GenericQuoteRequest['destChainId'],
5151
) => {
52-
if (
53-
srcChainId &&
54-
formatChainIdToCaip(srcChainId) ===
55-
formatChainIdToCaip(destChainId ?? srcChainId)
56-
) {
52+
if (srcChainId && !isCrossChain(srcChainId, destChainId ?? srcChainId)) {
5753
return MetricsActionType.SWAPBRIDGE_V1;
5854
}
5955
return MetricsActionType.CROSSCHAIN_V1;
@@ -69,11 +65,7 @@ export const getSwapType = (
6965
srcChainId?: GenericQuoteRequest['srcChainId'],
7066
destChainId?: GenericQuoteRequest['destChainId'],
7167
) => {
72-
if (
73-
srcChainId &&
74-
formatChainIdToCaip(srcChainId) ===
75-
formatChainIdToCaip(destChainId ?? srcChainId)
76-
) {
68+
if (srcChainId && !isCrossChain(srcChainId, destChainId ?? srcChainId)) {
7769
return MetricsSwapType.SINGLE;
7870
}
7971
return MetricsSwapType.CROSSCHAIN;

packages/bridge-status-controller/CHANGELOG.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Added
11+
12+
- Sentry traces for Swap and Bridge `TransactionApprovalCompleted` and `TransactionCompleted` events ([#5780](https://github.com/MetaMask/core/pull/5780))
13+
14+
### Changed
15+
16+
- `traceFn` added to BridgeStatusController constructor to enable clients to pass in a custom sentry trace handler ([#5768](https://github.com/MetaMask/core/pull/5768))
17+
1018
## [20.0.0]
1119

1220
### Changed

packages/bridge-status-controller/src/__snapshots__/bridge-status-controller.test.ts.snap

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -532,6 +532,31 @@ Array [
532532
]
533533
`;
534534

535+
exports[`BridgeStatusController submitTx: EVM should delay after submitting linea approval 4`] = `
536+
Array [
537+
Array [
538+
Object {
539+
"data": Object {
540+
"srcChainId": "eip155:59144",
541+
"stxEnabled": false,
542+
},
543+
"name": "Bridge Transaction Approval Completed",
544+
},
545+
[Function],
546+
],
547+
Array [
548+
Object {
549+
"data": Object {
550+
"srcChainId": "eip155:59144",
551+
"stxEnabled": false,
552+
},
553+
"name": "Bridge Transaction Completed",
554+
},
555+
[Function],
556+
],
557+
]
558+
`;
559+
535560
exports[`BridgeStatusController submitTx: EVM should handle smart accounts (4337) 1`] = `
536561
Object {
537562
"chainId": "0xa4b1",

packages/bridge-status-controller/src/bridge-status-controller.test.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,7 @@ const addTransactionFn = jest.fn();
523523
const estimateGasFeeFn = jest.fn();
524524
const addUserOperationFromTransactionFn = jest.fn();
525525

526-
const getController = (call: jest.Mock) => {
526+
const getController = (call: jest.Mock, traceFn?: jest.Mock) => {
527527
const controller = new BridgeStatusController({
528528
messenger: {
529529
call,
@@ -536,6 +536,7 @@ const getController = (call: jest.Mock) => {
536536
addTransactionFn,
537537
estimateGasFeeFn,
538538
addUserOperationFromTransactionFn,
539+
traceFn,
539540
});
540541

541542
jest.spyOn(controller, 'startPolling').mockImplementation(jest.fn());
@@ -1837,12 +1838,17 @@ describe('BridgeStatusController', () => {
18371838
const handleLineaDelaySpy = jest
18381839
.spyOn(transactionUtils, 'handleLineaDelay')
18391840
.mockResolvedValueOnce();
1841+
const mockTraceFn = jest
1842+
.fn()
1843+
.mockImplementation((_p, callback) => callback());
18401844

18411845
setupApprovalMocks();
18421846
setupBridgeMocks();
18431847

1844-
const { controller, startPollingForBridgeTxStatusSpy } =
1845-
getController(mockMessengerCall);
1848+
const { controller, startPollingForBridgeTxStatusSpy } = getController(
1849+
mockMessengerCall,
1850+
mockTraceFn,
1851+
);
18461852

18471853
const lineaQuoteResponse = {
18481854
...mockEvmQuoteResponse,
@@ -1853,6 +1859,7 @@ describe('BridgeStatusController', () => {
18531859
const result = await controller.submitTx(lineaQuoteResponse, false);
18541860
controller.stopAllPolling();
18551861

1862+
expect(mockTraceFn).toHaveBeenCalledTimes(2);
18561863
expect(handleLineaDelaySpy).toHaveBeenCalledTimes(1);
18571864
expect(result).toMatchSnapshot();
18581865
expect(startPollingForBridgeTxStatusSpy).toHaveBeenCalledTimes(1);
@@ -1866,6 +1873,7 @@ describe('BridgeStatusController', () => {
18661873
1234567890,
18671874
);
18681875
expect(mockMessengerCall.mock.calls).toMatchSnapshot();
1876+
expect(mockTraceFn.mock.calls).toMatchSnapshot();
18691877
});
18701878
});
18711879
});

0 commit comments

Comments
 (0)