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

chore: emit cross-chain swaps metrics + validation error logic #28713

Merged
merged 35 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
d5d592c
fix: validate srcTokenAmount
micaelae Nov 26, 2024
14a87cd
chore: insufficientBalance and noQuotes validation errors
micaelae Nov 26, 2024
73fb9d6
chore: isInsufficientGasBalance validation logic
micaelae Nov 13, 2024
3ab0382
chore: isEstimatedReturnLow logic
micaelae Nov 26, 2024
5947f00
chore: move BRIDGE_DEFAULT_SLIPPAGE to shared constants
micaelae Nov 29, 2024
1fd81bf
chore: event property types
micaelae Nov 21, 2024
370d1b0
chore: type-safe cross chain swaps event tracker
micaelae Nov 21, 2024
d2dd9d1
refactor: extract bridge token exchange rate fetching to a hook
micaelae Nov 27, 2024
6a287c1
chore: emit ActionOpened event
micaelae Nov 21, 2024
54ea558
chore: emit InputSourceDestinationFlipped event
micaelae Nov 21, 2024
f369cf1
chore: emit InputChanged events
micaelae Nov 21, 2024
44e49ee
chore: emit QuotesRequested and QuoteError events
micaelae Nov 21, 2024
861a6ef
chore: add TODO for ActionFailed
micaelae Nov 22, 2024
0bd266a
chore: emit AllQuotesOpened + AllQuotesSorted events
micaelae Nov 27, 2024
63237bc
chore: emit ActionSubmitted event
micaelae Nov 22, 2024
4b4ce2a
chore: emit QuoteSelected event
micaelae Nov 22, 2024
99c79c8
chore: emit QuotesReceived event
micaelae Nov 22, 2024
5c89f70
refactor: move useQuoteFetchEvents hook
micaelae Nov 29, 2024
6fb2fb1
refactor: rename balanceAmount
micaelae Nov 29, 2024
248ee32
fix: selector unit tests (refactor)
micaelae Nov 29, 2024
04df7c4
fix: bridge page unit tests
micaelae Nov 30, 2024
abbcee9
fix: lint error
micaelae Nov 30, 2024
f854d27
fix: sentry e2e test
micaelae Nov 30, 2024
dbb94d9
fix: default test sort order
micaelae Nov 30, 2024
7246c3b
fix: unit tests
micaelae Dec 2, 2024
7832a64
refactor: rename conversion rate selectors
micaelae Dec 2, 2024
1f9f1b7
chore: add comment
micaelae Dec 2, 2024
946aa59
fix: use toString for readability
micaelae Dec 3, 2024
0d75389
refactor: rename exchange rates in selector
micaelae Dec 3, 2024
d731910
chore: add comments for exchangeRate utils
micaelae Dec 3, 2024
8a74dcc
refactor: rename fiat -> valueInCurrency
micaelae Dec 3, 2024
375b3e2
refactor: rename Fiat -> Currency
micaelae Dec 3, 2024
edef303
chore: add cross-chain swaps page to route constants
micaelae Dec 6, 2024
83860d3
chore: rename duplicate events
micaelae Dec 10, 2024
b80879e
fix: unit tests
micaelae Dec 10, 2024
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
2 changes: 2 additions & 0 deletions app/scripts/constants/sentry-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,10 @@ export const SENTRY_BACKGROUND_STATE = {
srcTokenAmount: true,
},
quotes: [],
quotesInitialLoadTime: true,
quotesLastFetched: true,
quotesLoadingStatus: true,
quoteFetchError: true,
quotesRefreshCount: true,
},
},
Expand Down
12 changes: 9 additions & 3 deletions app/scripts/controllers/bridge/bridge-controller.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,7 @@ describe('BridgeController', function () {
...mockBridgeQuotesNativeErc20Eth,
],
quotesLoadingStatus: 1,
quoteFetchError: undefined,
quotesRefreshCount: 2,
}),
);
Expand All @@ -433,12 +434,14 @@ describe('BridgeController', function () {
...mockBridgeQuotesNativeErc20Eth,
],
quotesLoadingStatus: 2,
quoteFetchError: 'Network error',
quotesRefreshCount: 3,
}),
);
expect(bridgeController.state.bridgeState.quotesLastFetched).toStrictEqual(
secondFetchTime,
);
secondFetchTime &&
expect(
bridgeController.state.bridgeState.quotesLastFetched,
).toBeGreaterThan(secondFetchTime);

expect(hasSufficientBalanceSpy).toHaveBeenCalledTimes(1);
expect(getLayer1GasFeeMock).not.toHaveBeenCalled();
Expand Down Expand Up @@ -507,6 +510,7 @@ describe('BridgeController', function () {
quoteRequest: { ...quoteRequest, walletAddress: undefined },
quotes: DEFAULT_BRIDGE_CONTROLLER_STATE.quotes,
quotesLastFetched: DEFAULT_BRIDGE_CONTROLLER_STATE.quotesLastFetched,
quotesInitialLoadTime: undefined,
quotesLoadingStatus:
DEFAULT_BRIDGE_CONTROLLER_STATE.quotesLoadingStatus,
}),
Expand Down Expand Up @@ -544,6 +548,7 @@ describe('BridgeController', function () {
quotes: mockBridgeQuotesNativeErc20Eth,
quotesLoadingStatus: 1,
quotesRefreshCount: 1,
quotesInitialLoadTime: 11000,
}),
);
const firstFetchTime =
Expand All @@ -560,6 +565,7 @@ describe('BridgeController', function () {
quotes: mockBridgeQuotesNativeErc20Eth,
quotesLoadingStatus: 1,
quotesRefreshCount: 1,
quotesInitialLoadTime: 11000,
}),
);
const secondFetchTime =
Expand Down
51 changes: 36 additions & 15 deletions app/scripts/controllers/bridge/bridge-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ export default class BridgeController extends StaticIntervalPollingController<Br
> {
#abortController: AbortController | undefined;

#quotesFirstFetched: number | undefined;

#getLayer1GasFee: (params: {
transactionParams: TransactionParams;
chainId: ChainId;
Expand Down Expand Up @@ -150,11 +152,15 @@ export default class BridgeController extends StaticIntervalPollingController<Br
quotesLastFetched: DEFAULT_BRIDGE_CONTROLLER_STATE.quotesLastFetched,
quotesLoadingStatus:
DEFAULT_BRIDGE_CONTROLLER_STATE.quotesLoadingStatus,
quoteFetchError: DEFAULT_BRIDGE_CONTROLLER_STATE.quoteFetchError,
quotesRefreshCount: DEFAULT_BRIDGE_CONTROLLER_STATE.quotesRefreshCount,
quotesInitialLoadTime:
DEFAULT_BRIDGE_CONTROLLER_STATE.quotesInitialLoadTime,
};
});

if (isValidQuoteRequest(updatedQuoteRequest)) {
this.#quotesFirstFetched = Date.now();
const walletAddress = this.#getSelectedAccount().address;
const srcChainIdInHex = add0x(
decimalToHex(updatedQuoteRequest.srcChainId),
Expand Down Expand Up @@ -242,36 +248,23 @@ export default class BridgeController extends StaticIntervalPollingController<Br
...bridgeState,
quotesLoadingStatus: RequestStatus.LOADING,
quoteRequest: updatedQuoteRequest,
quoteFetchError: DEFAULT_BRIDGE_CONTROLLER_STATE.quoteFetchError,
};
});
const { maxRefreshCount } =
bridgeState.bridgeFeatureFlags[BridgeFeatureFlagsKey.EXTENSION_CONFIG];
const newQuotesRefreshCount = bridgeState.quotesRefreshCount + 1;

try {
const quotes = await fetchBridgeQuotes(
updatedQuoteRequest,
this.#abortController.signal,
);

// Stop polling if the maximum number of refreshes has been reached
if (
(updatedQuoteRequest.insufficientBal && newQuotesRefreshCount >= 1) ||
(!updatedQuoteRequest.insufficientBal &&
newQuotesRefreshCount >= maxRefreshCount)
) {
this.stopAllPolling();
}

const quotesWithL1GasFees = await this.#appendL1GasFees(quotes);

this.update((_state) => {
_state.bridgeState = {
..._state.bridgeState,
quotes: quotesWithL1GasFees,
quotesLastFetched: Date.now(),
quotesLoadingStatus: RequestStatus.FETCHED,
quotesRefreshCount: newQuotesRefreshCount,
};
});
} catch (error) {
Expand All @@ -284,11 +277,39 @@ export default class BridgeController extends StaticIntervalPollingController<Br
this.update((_state) => {
_state.bridgeState = {
...bridgeState,
quoteFetchError:
error instanceof Error ? error.message : 'Unknown error',
quotesLoadingStatus: RequestStatus.ERROR,
quotesRefreshCount: newQuotesRefreshCount,
};
});
console.log('Failed to fetch bridge quotes', error);
} finally {
const { maxRefreshCount } =
bridgeState.bridgeFeatureFlags[BridgeFeatureFlagsKey.EXTENSION_CONFIG];

const updatedQuotesRefreshCount = bridgeState.quotesRefreshCount + 1;
// Stop polling if the maximum number of refreshes has been reached
if (
updatedQuoteRequest.insufficientBal ||
(!updatedQuoteRequest.insufficientBal &&
updatedQuotesRefreshCount >= maxRefreshCount)
) {
this.stopAllPolling();
}

// Update quote fetching stats
const quotesLastFetched = Date.now();
this.update((_state) => {
_state.bridgeState = {
..._state.bridgeState,
quotesInitialLoadTime:
updatedQuotesRefreshCount === 1 && this.#quotesFirstFetched
? quotesLastFetched - this.#quotesFirstFetched
: bridgeState.quotesInitialLoadTime,
quotesLastFetched,
quotesRefreshCount: updatedQuotesRefreshCount,
};
});
}
};

Expand Down
10 changes: 7 additions & 3 deletions app/scripts/controllers/bridge/constants.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
import { zeroAddress } from 'ethereumjs-util';
import { Hex } from '@metamask/utils';
import { METABRIDGE_ETHEREUM_ADDRESS } from '../../../../shared/constants/bridge';
import {
BRIDGE_DEFAULT_SLIPPAGE,
METABRIDGE_ETHEREUM_ADDRESS,
} from '../../../../shared/constants/bridge';
import { CHAIN_IDS } from '../../../../shared/constants/network';
import { BridgeControllerState, BridgeFeatureFlagsKey } from './types';

export const BRIDGE_CONTROLLER_NAME = 'BridgeController';
export const REFRESH_INTERVAL_MS = 30 * 1000;
const DEFAULT_MAX_REFRESH_COUNT = 5;
const DEFAULT_SLIPPAGE = 0.5;

export enum RequestStatus {
LOADING,
Expand All @@ -31,11 +33,13 @@ export const DEFAULT_BRIDGE_CONTROLLER_STATE: BridgeControllerState = {
quoteRequest: {
walletAddress: undefined,
srcTokenAddress: zeroAddress(),
slippage: DEFAULT_SLIPPAGE,
slippage: BRIDGE_DEFAULT_SLIPPAGE,
},
quotesInitialLoadTime: undefined,
quotes: [],
quotesLastFetched: undefined,
quotesLoadingStatus: undefined,
quoteFetchError: undefined,
quotesRefreshCount: 0,
};

Expand Down
2 changes: 2 additions & 0 deletions app/scripts/controllers/bridge/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,10 @@ export type BridgeControllerState = {
destTopAssets: { address: string }[];
quoteRequest: Partial<QuoteRequest>;
quotes: (QuoteResponse & L1GasFees)[];
quotesInitialLoadTime?: number;
quotesLastFetched?: number;
quotesLoadingStatus?: RequestStatus;
quoteFetchError?: string;
quotesRefreshCount: number;
};

Expand Down
1 change: 1 addition & 0 deletions shared/constants/bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,4 @@ export const BRIDGE_QUOTE_MAX_ETA_SECONDS = 60 * 60; // 1 hour
export const BRIDGE_QUOTE_MAX_RETURN_DIFFERENCE_PERCENTAGE = 0.8; // if a quote returns in x times less return than the best quote, ignore it

export const BRIDGE_PREFERRED_GAS_ESTIMATE = 'medium';
export const BRIDGE_DEFAULT_SLIPPAGE = 0.5;
15 changes: 14 additions & 1 deletion shared/constants/metametrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -861,7 +861,6 @@ export enum MetaMetricsEventName {
NotificationsActivated = 'Notifications Activated',
PushNotificationReceived = 'Push Notification Received',
PushNotificationClicked = 'Push Notification Clicked',

// Send
sendAssetSelected = 'Send Asset Selected',
sendFlowExited = 'Send Flow Exited',
Expand All @@ -870,6 +869,19 @@ export enum MetaMetricsEventName {
sendSwapQuoteRequested = 'Send Swap Quote Requested',
sendSwapQuoteReceived = 'Send Swap Quote Received',
sendTokenModalOpened = 'Send Token Modal Opened',
// Cross Chain Swaps
ActionCompleted = 'Action Completed',
ActionFailed = 'Action Failed',
ActionOpened = 'Action Opened',
ActionSubmitted = 'Action Submitted',
AllQuotesOpened = 'All Quotes Opened',
AllQuotesSorted = 'All Quotes Sorted',
InputChanged = 'Input Changed',
InputSourceDestinationFlipped = 'Source and Destination Flipped',
CrossChainSwapsQuoteError = 'Cross-chain Quote Error',
QuoteSelected = 'Quote Selected',
CrossChainSwapsQuotesReceived = 'Cross-chain Quotes Received',
CrossChainSwapsQuotesRequested = 'Cross-chain Quotes Requested',
}

export enum MetaMetricsEventAccountType {
Expand Down Expand Up @@ -931,6 +943,7 @@ export enum MetaMetricsEventCategory {
///: BEGIN:ONLY_INCLUDE_IF(build-mmi)
MMI = 'Institutional',
///: END:ONLY_INCLUDE_IF
CrossChainSwaps = 'Cross Chain Swaps',
}

export enum MetaMetricsEventLinkType {
Expand Down
2 changes: 2 additions & 0 deletions test/e2e/tests/metrics/errors.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -877,6 +877,8 @@ describe('Sentry errors', function () {
quotesLastFetched: true,
quotesLoadingStatus: true,
quotesRefreshCount: true,
quoteFetchError: true,
quotesInitialLoadTime: true,
},
currentPopupId: false, // Initialized as undefined
// Part of transaction controller store, but missing from the initial
Expand Down
2 changes: 1 addition & 1 deletion test/jest/mock-store.js
Original file line number Diff line number Diff line change
Expand Up @@ -722,7 +722,7 @@ export const createBridgeMockStore = (
...swapsStore,
bridge: {
toChainId: null,
sortOrder: 0,
sortOrder: 'cost_ascending',
...bridgeSliceOverrides,
},
metamask: {
Expand Down
8 changes: 4 additions & 4 deletions ui/ducks/bridge/bridge.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ describe('Ducks - Bridge', () => {
fromToken: null,
toToken: null,
fromTokenInputValue: null,
sortOrder: 0,
sortOrder: 'cost_ascending',
toTokenExchangeRate: null,
fromTokenExchangeRate: null,
});
Expand Down Expand Up @@ -213,7 +213,7 @@ describe('Ducks - Bridge', () => {
fromTokenExchangeRate: null,
fromTokenInputValue: null,
selectedQuote: null,
sortOrder: 0,
sortOrder: 'cost_ascending',
toChainId: null,
toToken: null,
toTokenExchangeRate: null,
Expand Down Expand Up @@ -260,7 +260,7 @@ describe('Ducks - Bridge', () => {
expect(newState).toStrictEqual({
toChainId: null,
toTokenExchangeRate: 0.356628,
sortOrder: 0,
sortOrder: 'cost_ascending',
});
});

Expand Down Expand Up @@ -305,7 +305,7 @@ describe('Ducks - Bridge', () => {
expect(newState).toStrictEqual({
toChainId: null,
toTokenExchangeRate: 0.999881,
sortOrder: 0,
sortOrder: 'cost_ascending',
});
});
});
Expand Down
10 changes: 8 additions & 2 deletions ui/ducks/bridge/bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ export type BridgeState = {
fromToken: SwapsTokenObject | SwapsEthToken | null;
toToken: SwapsTokenObject | SwapsEthToken | null;
fromTokenInputValue: string | null;
fromTokenExchangeRate: number | null;
toTokenExchangeRate: number | null;
fromTokenExchangeRate: number | null; // Exchange rate from selected token to the default currency (can be fiat or crypto)
toTokenExchangeRate: number | null; // Exchange rate from the selected token to the default currency (can be fiat or crypto)
sortOrder: SortOrder;
selectedQuote: (QuoteResponse & QuoteMetadata) | null; // Alternate quote selected by user. When quotes refresh, the best match will be activated.
};
Expand Down Expand Up @@ -70,6 +70,12 @@ const bridgeSlice = createSlice({
},
},
extraReducers: (builder) => {
builder.addCase(setDestTokenExchangeRates.pending, (state) => {
state.toTokenExchangeRate = null;
});
builder.addCase(setSrcTokenExchangeRates.pending, (state) => {
state.fromTokenExchangeRate = null;
});
builder.addCase(setDestTokenExchangeRates.fulfilled, (state, action) => {
state.toTokenExchangeRate = action.payload ?? null;
});
Expand Down
Loading
Loading