Skip to content
Open
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
1 change: 1 addition & 0 deletions packages/transaction-controller/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Defaults to 100 if not provided.
- Add support for `transactionHistoryLimit` feature flag to configure the maximum number of transactions stored in state ([#7648](https://github.com/MetaMask/core/pull/7648))
- Defaults to 40 if not provided.
- Add callTrace errors in the simulation data ([#7641](https://github.com/MetaMask/core/pull/7641))

### Changed

Expand Down
7 changes: 5 additions & 2 deletions packages/transaction-controller/src/api/simulation-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,13 @@ export type SimulationResponseLog = {
/** Call trace of a single simulated transaction. */
export type SimulationResponseCallTrace = {
/** Nested calls. */
calls: SimulationResponseCallTrace[];
calls?: SimulationResponseCallTrace[] | null;

/** Error message for the call, if any. */
error?: string;

/** Raw event logs created by the call. */
logs: SimulationResponseLog[];
logs?: SimulationResponseLog[] | null;
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need both optional and null?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It might be over "type protective", I have noticed that the property calls can be null and I thought it could be the same with the logs but might not be necessary.

};

/**
Expand Down
3 changes: 3 additions & 0 deletions packages/transaction-controller/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1551,6 +1551,9 @@ export type SimulationData = {
/** Whether the simulation response changed after a security check triggered a re-simulation. */
isUpdatedAfterSecurityCheck?: boolean;

/** Error messages extracted from call traces, if any. */
callTraceErrors?: string[];

/** Data concerning a change to the user's native balance. */
nativeBalanceChange?: SimulationBalanceChange;

Expand Down
141 changes: 141 additions & 0 deletions packages/transaction-controller/src/utils/balance-changes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,7 @@ describe('Balance Change Utils', () => {

expect(result).toStrictEqual({
simulationData: {
callTraceErrors: [],
nativeBalanceChange: {
difference: DIFFERENCE_MOCK,
isDecrease,
Expand All @@ -328,6 +329,7 @@ describe('Balance Change Utils', () => {

expect(result).toStrictEqual({
simulationData: {
callTraceErrors: [],
nativeBalanceChange: undefined,
tokenBalanceChanges: [],
},
Expand All @@ -344,6 +346,7 @@ describe('Balance Change Utils', () => {

expect(result).toStrictEqual({
simulationData: {
callTraceErrors: [],
nativeBalanceChange: {
difference: '0x7',
isDecrease: false,
Expand Down Expand Up @@ -465,6 +468,7 @@ describe('Balance Change Utils', () => {

expect(result).toStrictEqual({
simulationData: {
callTraceErrors: [],
nativeBalanceChange: undefined,
tokenBalanceChanges: [
{
Expand Down Expand Up @@ -515,6 +519,7 @@ describe('Balance Change Utils', () => {

expect(result).toStrictEqual({
simulationData: {
callTraceErrors: [],
nativeBalanceChange: undefined,
tokenBalanceChanges: [
{
Expand Down Expand Up @@ -574,6 +579,7 @@ describe('Balance Change Utils', () => {

expect(result).toStrictEqual({
simulationData: {
callTraceErrors: [],
nativeBalanceChange: undefined,
tokenBalanceChanges: [
{
Expand Down Expand Up @@ -621,6 +627,7 @@ describe('Balance Change Utils', () => {

expect(result).toStrictEqual({
simulationData: {
callTraceErrors: [],
nativeBalanceChange: undefined,
tokenBalanceChanges: [
{
Expand Down Expand Up @@ -732,6 +739,7 @@ describe('Balance Change Utils', () => {
);
expect(result).toStrictEqual({
simulationData: {
callTraceErrors: [],
nativeBalanceChange: undefined,
tokenBalanceChanges: [
{
Expand Down Expand Up @@ -773,6 +781,7 @@ describe('Balance Change Utils', () => {

expect(result).toStrictEqual({
simulationData: {
callTraceErrors: [],
nativeBalanceChange: undefined,
tokenBalanceChanges: [],
},
Expand Down Expand Up @@ -800,6 +809,7 @@ describe('Balance Change Utils', () => {

expect(result).toStrictEqual({
simulationData: {
callTraceErrors: [],
nativeBalanceChange: undefined,
tokenBalanceChanges: [],
},
Expand All @@ -824,6 +834,7 @@ describe('Balance Change Utils', () => {

expect(result).toStrictEqual({
simulationData: {
callTraceErrors: [],
nativeBalanceChange: undefined,
tokenBalanceChanges: [],
},
Expand All @@ -844,6 +855,7 @@ describe('Balance Change Utils', () => {

expect(result).toStrictEqual({
simulationData: {
callTraceErrors: [],
nativeBalanceChange: undefined,
tokenBalanceChanges: [
{
Expand Down Expand Up @@ -902,6 +914,7 @@ describe('Balance Change Utils', () => {

expect(result).toStrictEqual({
simulationData: {
callTraceErrors: [],
nativeBalanceChange: undefined,
tokenBalanceChanges: [
{
Expand All @@ -920,6 +933,128 @@ describe('Balance Change Utils', () => {
});
});

describe('returns call trace errors', () => {
it('from root call trace', async () => {
simulateTransactionsMock.mockResolvedValueOnce({
transactions: [
{
...defaultResponseTx,
callTrace: {
calls: [],
logs: [],
error: 'Root error',
},
},
],
sponsorship: {
isSponsored: false,
error: null,
},
});

const result = await getBalanceChanges(REQUEST_MOCK);

expect(result).toStrictEqual({
simulationData: {
callTraceErrors: ['Root error'],
nativeBalanceChange: undefined,
tokenBalanceChanges: [],
},
gasUsed: undefined,
});
});

it('from nested call traces', async () => {
simulateTransactionsMock.mockResolvedValueOnce({
transactions: [
{
...defaultResponseTx,
callTrace: {
calls: [
{
calls: [
{
calls: [],
logs: [],
error: 'Deeply nested error',
},
],
logs: [],
error: 'Nested error',
},
],
logs: [],
error: 'Root error',
},
},
],
sponsorship: {
isSponsored: false,
error: null,
},
});

const result = await getBalanceChanges(REQUEST_MOCK);

expect(result).toStrictEqual({
simulationData: {
callTraceErrors: [
'Root error',
'Nested error',
'Deeply nested error',
],
nativeBalanceChange: undefined,
tokenBalanceChanges: [],
},
gasUsed: undefined,
});
});

it('as empty array when no errors in call trace', async () => {
simulateTransactionsMock.mockResolvedValueOnce(
createNativeBalanceResponse(BALANCE_1_MOCK, BALANCE_2_MOCK),
);

const result = await getBalanceChanges(REQUEST_MOCK);

expect(result.simulationData.callTraceErrors).toStrictEqual([]);
});

it('in error response when call trace errors exist', async () => {
simulateTransactionsMock.mockResolvedValueOnce({
transactions: [
{
...defaultResponseTx,
error: 'Transaction failed',
callTrace: {
calls: [],
logs: [],
error: 'Call trace error',
},
},
],
sponsorship: {
isSponsored: false,
error: null,
},
});

const result = await getBalanceChanges(REQUEST_MOCK);

expect(result).toStrictEqual({
simulationData: {
callTraceErrors: ['Call trace error'],
tokenBalanceChanges: [],
error: {
code: undefined,
message: 'Transaction failed',
},
},
gasUsed: undefined,
});
});
});

describe('returns error', () => {
it('if API request throws', async () => {
simulateTransactionsMock.mockRejectedValueOnce({
Expand All @@ -931,6 +1066,7 @@ describe('Balance Change Utils', () => {

expect(result).toStrictEqual({
simulationData: {
callTraceErrors: undefined,
tokenBalanceChanges: [],
error: {
code: ERROR_CODE_MOCK,
Expand All @@ -950,6 +1086,7 @@ describe('Balance Change Utils', () => {

expect(result).toStrictEqual({
simulationData: {
callTraceErrors: undefined,
tokenBalanceChanges: [],
error: {
code: ERROR_CODE_MOCK,
Expand All @@ -973,6 +1110,7 @@ describe('Balance Change Utils', () => {

expect(result).toStrictEqual({
simulationData: {
callTraceErrors: [],
tokenBalanceChanges: [],
error: {
code: SimulationErrorCode.InvalidResponse,
Expand Down Expand Up @@ -1001,6 +1139,7 @@ describe('Balance Change Utils', () => {

expect(result).toStrictEqual({
simulationData: {
callTraceErrors: [],
tokenBalanceChanges: [],
error: {
code: SimulationErrorCode.Reverted,
Expand Down Expand Up @@ -1029,6 +1168,7 @@ describe('Balance Change Utils', () => {

expect(result).toStrictEqual({
simulationData: {
callTraceErrors: [],
tokenBalanceChanges: [],
error: {
code: undefined,
Expand All @@ -1049,6 +1189,7 @@ describe('Balance Change Utils', () => {

expect(result).toStrictEqual({
simulationData: {
callTraceErrors: undefined,
tokenBalanceChanges: [],
error: {
code: SimulationErrorCode.Reverted,
Expand Down
30 changes: 28 additions & 2 deletions packages/transaction-controller/src/utils/balance-changes.ts
Copy link
Member

Choose a reason for hiding this comment

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

Unit tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will add

Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,8 @@ export async function getBalanceChanges(
): Promise<{ simulationData: SimulationData; gasUsed?: Hex }> {
log('Request', request);

let callTraceErrors: string[] | undefined;

try {
const response = await baseRequest({
request,
Expand All @@ -130,7 +132,9 @@ export async function getBalanceChanges(
},
});

const transactionError = response.transactions?.[0]?.error;
const transactionResponse = response.transactions?.[0];
callTraceErrors = extractCallTraceErrors(transactionResponse?.callTrace);
const transactionError = transactionResponse?.error;

if (transactionError) {
throw new SimulationError(transactionError);
Expand All @@ -143,8 +147,9 @@ export async function getBalanceChanges(

const tokenBalanceChanges = await getTokenBalanceChanges(request, events);

const gasUsed = response.transactions?.[0]?.gasUsed;
const gasUsed = transactionResponse?.gasUsed;
const simulationData = {
callTraceErrors,
nativeBalanceChange,
tokenBalanceChanges,
};
Expand All @@ -167,6 +172,7 @@ export async function getBalanceChanges(

return {
simulationData: {
callTraceErrors,
tokenBalanceChanges: [],
error: {
code,
Expand Down Expand Up @@ -632,6 +638,26 @@ function extractLogs(
];
}

/**
* Extract all error messages from a call trace tree.
*
* @param call - The root call trace.
* @returns An array of error messages.
*/
function extractCallTraceErrors(call?: SimulationResponseCallTrace): string[] {
if (!call) {
return [];
}

const errors = call.error ? [call.error] : [];
const nestedCalls = call.calls ?? [];
const nestedErrors = nestedCalls.flatMap((nestedCall) =>
extractCallTraceErrors(nestedCall),
);

return [...errors, ...nestedErrors];
}

/**
* Generate balance change data from previous and new balances.
*
Expand Down
Loading