Skip to content

Commit

Permalink
feat: add initial tracing to transaction controller (#4575)
Browse files Browse the repository at this point in the history 
Add an optional `trace` constructor callback to the `TransactionController`.
Add an optional `traceContext` argument to `addTransaction`.
Add initial instrumentation of the transaction lifecycle.
  • Loading branch information
@matthewwalsh0
matthewwalsh0 authored Aug 16, 2024
1 parent d5e5c62 commit b4506ba
Show file tree
Hide file tree
Showing 2 changed files with 157 additions and 46 deletions.
166 changes: 120 additions & 46 deletions packages/transaction-controller/src/TransactionController.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ import type {
SimulationData,
GasFeeEstimates,
GasFeeFlowResponse,
TraceCallback,
TraceContext,
} from './types';
import {
TransactionEnvelopeType,
Expand Down Expand Up @@ -313,6 +315,7 @@ export type TransactionControllerOptions = {
) => Promise<TypedTransaction>;
state?: Partial<TransactionControllerState>;
testGasFeeFlows?: boolean;
trace?: TraceCallback;
transactionHistoryLimit: number;
hooks: {
afterSign?: (
Expand Down Expand Up @@ -634,6 +637,8 @@ export class TransactionController extends BaseController<

private readonly signAbortCallbacks: Map<string, () => void> = new Map();

#trace: TraceCallback;

#transactionHistoryLimit: number;

#isSimulationEnabled: () => boolean;
Expand Down Expand Up @@ -743,6 +748,7 @@ export class TransactionController extends BaseController<
* @param options.sign - Function used to sign transactions.
* @param options.state - Initial state to set on this controller.
* @param options.testGasFeeFlows - Whether to use the test gas fee flow.
* @param options.trace - Callback to generate trace information.
* @param options.transactionHistoryLimit - Transaction history limit.
* @param options.hooks - The controller hooks.
*/
Expand Down Expand Up @@ -770,6 +776,7 @@ export class TransactionController extends BaseController<
sign,
state,
testGasFeeFlows,
trace,
transactionHistoryLimit = 40,
hooks,
}: TransactionControllerOptions) {
Expand Down Expand Up @@ -807,6 +814,7 @@ export class TransactionController extends BaseController<
this.#transactionHistoryLimit = transactionHistoryLimit;
this.sign = sign;
this.#testGasFeeFlows = testGasFeeFlows === true;
this.#trace = trace ?? (((_request, fn) => fn?.()) as TraceCallback);

this.afterSign = hooks?.afterSign ?? (() => true);
this.beforeApproveOnInit = hooks?.beforeApproveOnInit ?? (() => true);
Expand Down Expand Up @@ -976,6 +984,7 @@ export class TransactionController extends BaseController<
* @param opts.swaps.hasApproveTx - Whether the transaction has an approval transaction.
* @param opts.swaps.meta - Metadata for swap transaction.
* @param opts.networkClientId - The id of the network client for this transaction.
* @param opts.traceContext - The parent context for any new traces.
* @returns Object containing a promise resolving to the transaction hash if approved.
*/
async addTransaction(
Expand All @@ -989,6 +998,7 @@ export class TransactionController extends BaseController<
securityAlertResponse,
sendFlowHistory,
swaps = {},
traceContext,
type,
networkClientId: requestNetworkClientId,
}: {
Expand All @@ -1003,6 +1013,7 @@ export class TransactionController extends BaseController<
hasApproveTx?: boolean;
meta?: Partial<TransactionMeta>;
};
traceContext?: unknown;
type?: TransactionType;
networkClientId?: NetworkClientId;
} = {},
Expand Down Expand Up @@ -1074,7 +1085,13 @@ export class TransactionController extends BaseController<
networkClientId,
};

await this.updateGasProperties(addedTransactionMeta);
await this.#trace(
{ name: 'Estimate Gas Properties', parentContext: traceContext },
(context) =>
this.updateGasProperties(addedTransactionMeta, {
traceContext: context,
}),
);

// Checks if a transaction already exists with a given actionId
if (!existingTransactionMeta) {
Expand Down Expand Up @@ -1110,7 +1127,9 @@ export class TransactionController extends BaseController<
this.addMetadata(addedTransactionMeta);

if (requireApproval !== false) {
this.#updateSimulationData(addedTransactionMeta).catch((error) => {
this.#updateSimulationData(addedTransactionMeta, {
traceContext,
}).catch((error) => {
log('Error while updating simulation data', error);
throw error;
});
Expand All @@ -1129,6 +1148,7 @@ export class TransactionController extends BaseController<
isExisting: Boolean(existingTransactionMeta),
requireApproval,
actionId,
traceContext,
}),
transactionMeta: addedTransactionMeta,
};
Expand Down Expand Up @@ -2442,7 +2462,10 @@ export class TransactionController extends BaseController<
});
}

private async updateGasProperties(transactionMeta: TransactionMeta) {
private async updateGasProperties(
transactionMeta: TransactionMeta,
{ traceContext }: { traceContext?: TraceContext } = {},
) {
const isEIP1559Compatible =
transactionMeta.txParams.type !== TransactionEnvelopeType.legacy &&
(await this.getEIP1559Compatibility(transactionMeta.networkClientId));
Expand All @@ -2461,27 +2484,40 @@ export class TransactionController extends BaseController<
chainId,
});

await updateGas({
ethQuery,
chainId,
isCustomNetwork,
txMeta: transactionMeta,
});
await this.#trace(
{ name: 'Update Gas', parentContext: traceContext },
async () => {
await updateGas({
ethQuery,
chainId,
isCustomNetwork,
txMeta: transactionMeta,
});
},
);

await updateGasFees({
eip1559: isEIP1559Compatible,
ethQuery,
gasFeeFlows: this.gasFeeFlows,
getGasFeeEstimates: this.getGasFeeEstimates,
getSavedGasFees: this.getSavedGasFees.bind(this),
txMeta: transactionMeta,
});
await this.#trace(
{ name: 'Update Gas Fees', parentContext: traceContext },
async () =>
await updateGasFees({
eip1559: isEIP1559Compatible,
ethQuery,
gasFeeFlows: this.gasFeeFlows,
getGasFeeEstimates: this.getGasFeeEstimates,
getSavedGasFees: this.getSavedGasFees.bind(this),
txMeta: transactionMeta,
}),
);

await updateTransactionLayer1GasFee({
layer1GasFeeFlows: this.layer1GasFeeFlows,
provider,
transactionMeta,
});
await this.#trace(
{ name: 'Update Layer 1 Gas Fees', parentContext: traceContext },
async () =>
await updateTransactionLayer1GasFee({
layer1GasFeeFlows: this.layer1GasFeeFlows,
provider,
transactionMeta,
}),
);
}

private onBootCleanup() {
Expand Down Expand Up @@ -2513,11 +2549,13 @@ export class TransactionController extends BaseController<
requireApproval,
shouldShowRequest = true,
actionId,
traceContext,
}: {
isExisting?: boolean;
requireApproval?: boolean | undefined;
shouldShowRequest?: boolean;
actionId?: string;
traceContext?: TraceContext;
},
): Promise<string> {
const transactionId = transactionMeta.id;
Expand All @@ -2530,9 +2568,15 @@ export class TransactionController extends BaseController<
if (meta && !isExisting && !isCompleted) {
try {
if (requireApproval !== false) {
const acceptResult = await this.requestApproval(transactionMeta, {
shouldShowRequest,
});
const acceptResult = await this.#trace(
{ name: 'Await Approval', parentContext: traceContext },
(context) =>
this.requestApproval(transactionMeta, {
shouldShowRequest,
traceContext: context,
}),
);

resultCallbacks = acceptResult.resultCallbacks;

const approvalValue = acceptResult.value as
Expand Down Expand Up @@ -2560,7 +2604,10 @@ export class TransactionController extends BaseController<
this.isTransactionCompleted(transactionId);

if (!isTxCompleted) {
const approvalResult = await this.approveTransaction(transactionId);
const approvalResult = await this.approveTransaction(
transactionId,
traceContext,
);
if (
approvalResult === ApprovalState.SkippedViaBeforePublishHook &&
resultCallbacks
Expand Down Expand Up @@ -2627,8 +2674,12 @@ export class TransactionController extends BaseController<
* A `<tx.id>:finished` hub event is fired after success or failure.
*
* @param transactionId - The ID of the transaction to approve.
* @param traceContext - The parent context for any new traces.
*/
private async approveTransaction(transactionId: string) {
private async approveTransaction(
transactionId: string,
traceContext?: unknown,
) {
const cleanupTasks = new Array<() => void>();
cleanupTasks.push(await this.mutex.acquire());

Expand Down Expand Up @@ -2690,9 +2741,9 @@ export class TransactionController extends BaseController<

this.onTransactionStatusChange(transactionMeta);

const rawTx = await this.signTransaction(
transactionMeta,
transactionMeta.txParams,
const rawTx = await this.#trace(
{ name: 'Sign', parentContext: traceContext },
() => this.signTransaction(transactionMeta, transactionMeta.txParams),
);

if (!this.beforePublish(transactionMeta)) {
Expand Down Expand Up @@ -2727,14 +2778,21 @@ export class TransactionController extends BaseController<

log('Publishing transaction', transactionMeta.txParams);

let { transactionHash: hash } = await this.publish(
transactionMeta,
rawTx,
);
let hash: string | undefined;

if (hash === undefined) {
hash = await this.publishTransaction(ethQuery, rawTx);
}
await this.#trace(
{ name: 'Publish', parentContext: traceContext },
async () => {
({ transactionHash: hash } = await this.publish(
transactionMeta,
rawTx,
));

if (hash === undefined) {
hash = await this.publishTransaction(ethQuery, rawTx);
}
},
);

log('Publish successful', hash);

Expand Down Expand Up @@ -2904,13 +2962,22 @@ export class TransactionController extends BaseController<

private async requestApproval(
txMeta: TransactionMeta,
{ shouldShowRequest }: { shouldShowRequest: boolean },
{
shouldShowRequest,
traceContext,
}: { shouldShowRequest: boolean; traceContext?: TraceContext },
): Promise<AddResult> {
const id = this.getApprovalId(txMeta);
const { origin } = txMeta;
const type = ApprovalType.Transaction;
const requestData = { txId: txMeta.id };

await this.#trace({
name: 'Notification Display',
id,
parentContext: traceContext,
});

return (await this.messagingSystem.call(
'ApprovalController:addRequest',
{
Expand Down Expand Up @@ -3724,7 +3791,10 @@ export class TransactionController extends BaseController<
}
}

async #updateSimulationData(transactionMeta: TransactionMeta) {
async #updateSimulationData(
transactionMeta: TransactionMeta,
{ traceContext }: { traceContext?: TraceContext } = {},
) {
const { id: transactionId, chainId, txParams } = transactionMeta;
const { from, to, value, data } = txParams;

Expand All @@ -3744,13 +3814,17 @@ export class TransactionController extends BaseController<
},
);

simulationData = await getSimulationData({
chainId,
from: from as Hex,
to: to as Hex,
value: value as Hex,
data: data as Hex,
});
simulationData = await this.#trace(
{ name: 'Simulate', parentContext: traceContext },
() =>
getSimulationData({
chainId,
from: from as Hex,
to: to as Hex,
value: value as Hex,
data: data as Hex,
}),
);
}

const finalTransactionMeta = this.getTransaction(transactionId);
Expand Down
37 changes: 37 additions & 0 deletions packages/transaction-controller/src/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1299,3 +1299,40 @@ export type SimulationData = {
/** Data concerning a change to the user's token balances. */
tokenBalanceChanges: SimulationTokenBalanceChange[];
};

/** A context in which to execute a trace, in order to generate nested timings. */
export type TraceContext = unknown;

/** Request to trace an operation. */
export type TraceRequest = {
/** Additional data to include in the trace. */
data?: Record<string, number | string | boolean>;

/** Name of the operation. */
name: string;

/**
* Unique identifier for the trace.
* Required if starting a trace and not providing a callback.
*/
id?: string;

/** Trace context in which to execute the operation. */
parentContext?: TraceContext;

/** Additional tags to include in the trace to filter results. */
tags?: Record<string, number | string | boolean>;
};

/** Callback that traces the performance of an operation. */
export type TraceCallback = <ReturnType>(
/** Request to trace the performance of an operation. */
request: TraceRequest,

/**
* Callback to trace.
* Thrown errors will not be caught, but the trace will still be recorded.
* @param context - The context in which the operation is running.
*/
fn?: (context?: TraceContext) => ReturnType,
) => Promise<ReturnType>;

0 comments on commit b4506ba

Please sign in to comment.