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: adding TransactionTimeToConfirmation event for confirmations #9

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import getProvidedUtxos from '../utils/getProvidedUtxos';
import { AnalyticsServicePosthog } from '../../analytics/AnalyticsServicePosthog';
import { ChainId } from '@avalabs/core-chains-sdk';
import { openApprovalWindow } from '@src/background/runtime/openApprovalWindow';
import { measureTransactionTime } from '@src/background/services/wallet/utils/measureTransactionTime';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
import { measureTransactionTime } from '@src/background/services/wallet/utils/measureTransactionTime';


type TxParams = {
transactionHex: string;
Expand Down Expand Up @@ -231,6 +232,7 @@ export class AvalancheSendTransactionHandler extends DAppRequestHandler<
const usedNetwork = this.#getChainIdForVM(vm);

try {
measureTransactionTime().startMeasure();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥

Suggested change
measureTransactionTime().startMeasure();

// Parse the json into a tx object
const unsignedTx =
vm === EVM
Expand Down Expand Up @@ -272,6 +274,18 @@ export class AvalancheSendTransactionHandler extends DAppRequestHandler<
},
});

measureTransactionTime().endMeasure(async (duration) => {
this.analyticsServicePosthog.captureEvent({
name: 'TransactionTimeToConfirmation',
windowId: crypto.randomUUID(),
properties: {
duration,
txType: 'txType',
chainId: usedNetwork,
},
});
});
Comment on lines +277 to +287
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥

Suggested change
measureTransactionTime().endMeasure(async (duration) => {
this.analyticsServicePosthog.captureEvent({
name: 'TransactionTimeToConfirmation',
windowId: crypto.randomUUID(),
properties: {
duration,
txType: 'txType',
chainId: usedNetwork,
},
});
});


// If we already have the transaction hash (i.e. it was dispatched by WalletConnect),
// we just return it to the caller.
onSuccess(txHash);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ describe('src/background/services/wallet/handlers/bitcoin_sendTransaction.ts', (
const signMock = jest.fn();
const sendTransactionMock = jest.fn();
const getBalancesForNetworksMock = jest.fn();
const captureEventMock = jest.fn();

const getBitcoinNetworkMock = jest.fn();
const activeAccountMock = {
Expand All @@ -51,6 +52,9 @@ describe('src/background/services/wallet/handlers/bitcoin_sendTransaction.ts', (
const balanceAggregatorServiceMock = {
getBalancesForNetworks: getBalancesForNetworksMock,
};
const analyticsServiceMock = {
captureEvent: captureEventMock,
};

beforeEach(() => {
jest.resetAllMocks();
Expand Down Expand Up @@ -91,6 +95,7 @@ describe('src/background/services/wallet/handlers/bitcoin_sendTransaction.ts', (
{} as any,
{} as any,
{} as any,
{} as any,
{} as any
);
const result = await handler.handleUnauthenticated(buildRpcCall(request));
Expand All @@ -113,7 +118,8 @@ describe('src/background/services/wallet/handlers/bitcoin_sendTransaction.ts', (
type: AccountType.PRIMARY,
},
} as any,
balanceAggregatorServiceMock as any
balanceAggregatorServiceMock as any,
analyticsServiceMock as any
);

it('returns error if the active account is imported via WalletConnect', async () => {
Expand All @@ -128,7 +134,8 @@ describe('src/background/services/wallet/handlers/bitcoin_sendTransaction.ts', (
type: AccountType.WALLET_CONNECT,
},
} as any,
balanceAggregatorServiceMock as any
balanceAggregatorServiceMock as any,
analyticsServiceMock as any
);

const result = await sendHandler.handleAuthenticated(
Expand All @@ -153,7 +160,8 @@ describe('src/background/services/wallet/handlers/bitcoin_sendTransaction.ts', (
addressC: 'abcd1234',
},
} as any,
balanceAggregatorServiceMock as any
balanceAggregatorServiceMock as any,
analyticsServiceMock as any
);

const result = await sendHandler.handleAuthenticated(
Expand Down Expand Up @@ -230,7 +238,8 @@ describe('src/background/services/wallet/handlers/bitcoin_sendTransaction.ts', (
walletServiceMock as any,
networkServiceMock as any,
{} as any,
balanceAggregatorServiceMock as any
balanceAggregatorServiceMock as any,
analyticsServiceMock as any
);
const result = await sendHandler.handleAuthenticated({ request } as any);
expect(result).toEqual({
Expand Down Expand Up @@ -260,7 +269,8 @@ describe('src/background/services/wallet/handlers/bitcoin_sendTransaction.ts', (
walletServiceMock as any,
networkServiceMock as any,
accountsServiceMock as any,
balanceAggregatorServiceMock as any
balanceAggregatorServiceMock as any,
analyticsServiceMock as any
);

const result = await sendHandler.handleAuthenticated(
Expand Down Expand Up @@ -298,7 +308,8 @@ describe('src/background/services/wallet/handlers/bitcoin_sendTransaction.ts', (
},
} as any,
accountsServiceMock as any,
balanceAggregatorServiceMock as any
balanceAggregatorServiceMock as any,
analyticsServiceMock as any
);

const result = await sendHandler.handleAuthenticated(
Expand Down Expand Up @@ -329,7 +340,8 @@ describe('src/background/services/wallet/handlers/bitcoin_sendTransaction.ts', (
walletServiceMock as any,
networkServiceMock as any,
accountsServiceMock as any,
balanceAggregatorServiceMock as any
balanceAggregatorServiceMock as any,
analyticsServiceMock as any
);

getBitcoinNetworkMock.mockResolvedValue({
Expand Down Expand Up @@ -362,7 +374,8 @@ describe('src/background/services/wallet/handlers/bitcoin_sendTransaction.ts', (
walletServiceMock as any,
networkServiceMock as any,
accountsServiceMock as any,
balanceAggregatorServiceMock as any
balanceAggregatorServiceMock as any,
analyticsServiceMock as any
);

getBitcoinNetworkMock.mockResolvedValue({
Expand Down
43 changes: 32 additions & 11 deletions src/background/services/wallet/handlers/bitcoin_sendTransaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { DAppRequestHandler } from '@src/background/connections/dAppConnection/D
import { Action } from '../../actions/models';
import { DEFERRED_RESPONSE } from '@src/background/connections/middlewares/models';
import { NetworkService } from '@src/background/services/network/NetworkService';
import { AnalyticsServicePosthog } from '@src/background/services/analytics/AnalyticsServicePosthog';
import { ethErrors } from 'eth-rpc-errors';
import {
DisplayData_BitcoinSendTx,
Expand All @@ -33,6 +34,7 @@ import { resolve } from '@avalabs/core-utils-sdk';

import { openApprovalWindow } from '@src/background/runtime/openApprovalWindow';
import { runtime } from 'webextension-polyfill';
import { measureTransactionTime } from '@src/background/services/wallet/utils/measureTransactionTime';

type BitcoinTxParams = [
address: string,
Expand All @@ -52,7 +54,8 @@ export class BitcoinSendTransactionHandler extends DAppRequestHandler<
private walletService: WalletService,
private networkService: NetworkService,
private accountService: AccountsService,
private balanceAggregatorService: BalanceAggregatorService
private balanceAggregatorService: BalanceAggregatorService,
private analyticsServicePosthog: AnalyticsServicePosthog
) {
super();
}
Expand Down Expand Up @@ -256,8 +259,12 @@ export class BitcoinSendTransactionHandler extends DAppRequestHandler<
frontendTabId?: number
) => {
try {
measureTransactionTime().startMeasure();
const { address, amount, from, feeRate, balance } =
pendingAction.displayData;
const btcChainID = this.networkService.isMainnet()
? ChainId.BITCOIN
: ChainId.BITCOIN_TESTNET;

const [network, networkError] = await resolve(
this.networkService.getBitcoinNetwork()
Expand All @@ -266,16 +273,14 @@ export class BitcoinSendTransactionHandler extends DAppRequestHandler<
throw new Error('Bitcoin network not found');
}

const { inputs, outputs } = await buildBtcTx(
from,
getProviderForNetwork(network) as BitcoinProvider,
{
amount,
address,
token: balance,
feeRate,
}
);
const provider = getProviderForNetwork(network) as BitcoinProvider;

const { inputs, outputs } = await buildBtcTx(from, provider, {
amount,
address,
token: balance,
feeRate,
});

if (!inputs || !outputs) {
throw new Error('Unable to create transaction');
Expand All @@ -293,7 +298,23 @@ export class BitcoinSendTransactionHandler extends DAppRequestHandler<
if (this.#isSupportedAccount(this.accountService.activeAccount)) {
this.#getBalance(this.accountService.activeAccount);
}

onSuccess(hash);

// eslint-disable-next-line @typescript-eslint/no-unused-vars
provider.waitForTx(hash).then((_tx) => {
measureTransactionTime().endMeasure(async (duration) => {
this.analyticsServicePosthog.captureEvent({
name: 'TransactionTimeToConfirmation',
windowId: crypto.randomUUID(),
properties: {
duration,
txType: 'txType',
chainId: btcChainID,
},
});
});
});
} catch (e) {
onError(e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,17 @@ jest.mock('@src/background/services/analytics/utils/encryptAnalyticsData');
jest.mock('./contracts/contractParsers/parseWithERC20Abi');
jest.mock('./utils/getTxDescription');
jest.mock('./contracts/contractParsers/utils/parseBasicDisplayValues');
jest.mock(
'@src/background/services/wallet/utils/measureTransactionTime',
() => ({
measureTransactionTime: function () {
return {
startMeasure: jest.fn(),
endMeasure: jest.fn(),
};
},
})
);
jest.mock('./contracts/contractParsers/contractParserMap', () => ({
contractParserMap: new Map([['function', jest.fn()]]),
}));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import { openApprovalWindow } from '@src/background/runtime/openApprovalWindow';
import { EnsureDefined } from '@src/background/models';
import { caipToChainId } from '@src/utils/caipConversion';
import { TxDisplayOptions } from '../models';
import { measureTransactionTime } from '@src/background/services/wallet/utils/measureTransactionTime';

type TxPayload = EthSendTransactionParams | ContractTransaction;
type Params = [TxPayload] | [TxPayload, TxDisplayOptions];
Expand Down Expand Up @@ -207,6 +208,7 @@ export class EthSendTransactionHandler extends DAppRequestHandler<
tabId?: number | undefined
) => {
try {
measureTransactionTime().startMeasure();
const network = await getTargetNetworkForTx(
pendingAction.displayData.txParams,
this.networkService,
Expand All @@ -223,6 +225,7 @@ export class EthSendTransactionHandler extends DAppRequestHandler<
const nonce = await provider.getTransactionCount(
pendingAction.displayData.txParams.from
);
const chainId = pendingAction.displayData.chainId;

const {
maxFeePerGas,
Expand All @@ -237,7 +240,7 @@ export class EthSendTransactionHandler extends DAppRequestHandler<
const signingResult = await this.walletService.sign(
{
nonce,
chainId: Number(BigInt(pendingAction.displayData.chainId)),
chainId: Number(BigInt(chainId)),
maxFeePerGas,
maxPriorityFeePerGas,
gasLimit: gasLimit,
Expand Down Expand Up @@ -296,11 +299,26 @@ export class EthSendTransactionHandler extends DAppRequestHandler<
address: this.accountsService.activeAccount?.addressC,
txHash,
method: pendingAction.method,
chainId: pendingAction.displayData.chainId,
chainId,
},
});

onSuccess(txHash);

// eslint-disable-next-line @typescript-eslint/no-unused-vars
provider.waitForTransaction(txHash).then((_tx) => {
measureTransactionTime().endMeasure(async (duration) => {
this.analyticsServicePosthog.captureEvent({
name: 'TransactionTimeToConfirmation',
windowId: crypto.randomUUID(),
properties: {
duration,
txType: 'txType',
chainId,
},
});
});
});
} catch (err: any) {
const errorMessage: string =
err instanceof Error ? err.message : err.toString();
Expand Down
32 changes: 32 additions & 0 deletions src/background/services/wallet/utils/measureTransactionTime.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
enum TransactionTimeEvents {
TRANSACTION_TIMED = 'transaction-timed',
TRANSACTION_SUCCEEDED = 'transaction-succeeded',
TRANSACTION_APPROVED = 'transaction-approved',
}

export const measureTransactionTime = (): {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this util is handy, it is introducing some issues when there are multiple concurrent transactions waiting for confirmations. Since you are using the same marks for all measurements, the start time can be overwritten or cleared by an upcoming transaction.

startMeasure: () => void;
endMeasure: (callback: (duration: number) => void) => void;
} => {
const startMeasure = () => {
performance.mark(TransactionTimeEvents.TRANSACTION_APPROVED);
};

const endMeasure = (callback: (duration: number) => void) => {
performance.mark(TransactionTimeEvents.TRANSACTION_SUCCEEDED);

const measurement = performance.measure(
TransactionTimeEvents.TRANSACTION_TIMED,
TransactionTimeEvents.TRANSACTION_APPROVED,
TransactionTimeEvents.TRANSACTION_SUCCEEDED
);

performance.clearMarks(TransactionTimeEvents.TRANSACTION_APPROVED);
performance.clearMarks(TransactionTimeEvents.TRANSACTION_SUCCEEDED);
performance.clearMarks(TransactionTimeEvents.TRANSACTION_TIMED);

callback(measurement.duration);
};

return { startMeasure, endMeasure };
};
2 changes: 2 additions & 0 deletions src/tests/setupTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ Object.defineProperty(global.document, 'prerendering', {
value: false,
});

performance.mark = jest.fn();

global.chrome = {
runtime: {
id: 'testid',
Expand Down
Loading