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 initial tracing to transaction controller #4575

Merged
merged 8 commits into from
Aug 16, 2024
166 changes: 120 additions & 46 deletions packages/transaction-controller/src/TransactionController.ts
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
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;
matthewwalsh0 marked this conversation as resolved.
Show resolved Hide resolved

/** 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>;
Loading