Skip to content

Commit 9d287d0

Browse files
authored
fix: debug_traceTransaction validation for default tracer (#4404)
Signed-off-by: Simeon Nakov <[email protected]>
1 parent 9f9c6d9 commit 9d287d0

File tree

7 files changed

+233
-21
lines changed

7 files changed

+233
-21
lines changed

docs/openrpc.json

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1176,7 +1176,7 @@
11761176
"schema": {
11771177
"$ref": "#/components/schemas/TracerConfigWrapper"
11781178
},
1179-
"description": "Configuration object for the tracer."
1179+
"description": "Configuration object for the tracer. Supports two formats: 1) Standard nested format with 'tracer' and 'tracerConfig' properties, 2) Simplified format with opcodeLogger config properties directly at top level (when no explicit 'tracer' is specified, defaults to opcodeLogger)."
11801180
}
11811181
],
11821182
"result": {
@@ -1665,6 +1665,10 @@
16651665
},
16661666
"disableStorage": {
16671667
"type": "boolean"
1668+
},
1669+
"fullStorage": {
1670+
"type": "boolean",
1671+
"description": "Non-standard parameter sometimes sent by Remix - accepted for compatibility but ignored in implementation"
16681672
}
16691673
}
16701674
},
@@ -1694,6 +1698,22 @@
16941698
},
16951699
"tracerConfig": {
16961700
"$ref": "#/components/schemas/TracerConfig"
1701+
},
1702+
"enableMemory": {
1703+
"type": "boolean",
1704+
"description": "Top-level opcodeLogger config property. Only valid when no explicit 'tracer' is specified (defaults to opcodeLogger)."
1705+
},
1706+
"disableStack": {
1707+
"type": "boolean",
1708+
"description": "Top-level opcodeLogger config property. Only valid when no explicit 'tracer' is specified (defaults to opcodeLogger)."
1709+
},
1710+
"disableStorage": {
1711+
"type": "boolean",
1712+
"description": "Top-level opcodeLogger config property. Only valid when no explicit 'tracer' is specified (defaults to opcodeLogger)."
1713+
},
1714+
"fullStorage": {
1715+
"type": "boolean",
1716+
"description": "Top-level opcodeLogger config property. Non-standard parameter for Remix compatibility - accepted but ignored. Only valid when no explicit 'tracer' is specified."
16971717
}
16981718
}
16991719
},

packages/relay/src/lib/debug.ts

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,32 @@ export class DebugImpl implements Debug {
122122
//we use a wrapper since we accept a transaction where a second param with tracer/tracerConfig may not be provided
123123
//and we will still default to opcodeLogger
124124
const tracer = tracerObject?.tracer ?? TracerType.OpcodeLogger;
125-
const tracerConfig = tracerObject?.tracerConfig ?? {};
125+
126+
// Extract tracer config from either nested tracerConfig or top-level properties
127+
let tracerConfig = tracerObject?.tracerConfig ?? {};
128+
129+
// If no nested tracerConfig is provided AND no tracer is explicitly set,
130+
// check for top-level opcodeLogger config properties (defaults to opcodeLogger)
131+
if (!tracerObject?.tracerConfig && !tracerObject?.tracer && tracerObject) {
132+
const topLevelConfig = Object.fromEntries(
133+
Object.entries(tracerObject).filter(([key]) => key !== 'tracer' && key !== 'tracerConfig'),
134+
);
135+
// Only include valid opcodeLogger config properties
136+
const validOpcodeLoggerKeys = ['enableMemory', 'disableStack', 'disableStorage', 'fullStorage'];
137+
const filteredConfig = Object.keys(topLevelConfig)
138+
.filter((key) => validOpcodeLoggerKeys.includes(key))
139+
.reduce((obj, key) => {
140+
// Filter out non-standard parameters that shouldn't be passed to the actual tracer
141+
if (key !== 'fullStorage') {
142+
obj[key] = topLevelConfig[key];
143+
}
144+
return obj;
145+
}, {} as any);
146+
147+
if (Object.keys(filteredConfig).length > 0) {
148+
tracerConfig = filteredConfig;
149+
}
150+
}
126151

127152
try {
128153
DebugImpl.requireDebugAPIEnabled();
@@ -131,7 +156,7 @@ export class DebugImpl implements Debug {
131156
}
132157

133158
if (tracer === TracerType.PrestateTracer) {
134-
const onlyTopCall = (tracerObject?.tracerConfig as ICallTracerConfig)?.onlyTopCall ?? false;
159+
const onlyTopCall = (tracerConfig as ICallTracerConfig)?.onlyTopCall ?? false;
135160
return await this.prestateTracer(transactionIdOrHash, onlyTopCall, requestDetails);
136161
}
137162

packages/relay/src/lib/types/ITracerConfig.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ export interface IOpcodeLoggerConfig {
88
enableMemory?: boolean;
99
disableStack?: boolean;
1010
disableStorage?: boolean;
11+
fullStorage?: boolean; // Non-standard parameter sometimes sent by Remix - ignored in implementation
1112
}
1213

1314
export type ITracerConfig = ICallTracerConfig | IOpcodeLoggerConfig;
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
// SPDX-License-Identifier: Apache-2.0
22

33
import { TracerType } from '../constants';
4-
import { ITracerConfig } from './ITracerConfig';
4+
import { IOpcodeLoggerConfig, ITracerConfig } from './ITracerConfig';
55

6-
export interface ITracerConfigWrapper {
6+
export interface ITracerConfigWrapper extends Partial<IOpcodeLoggerConfig> {
77
tracer?: TracerType;
88
tracerConfig?: ITracerConfig;
99
}

packages/relay/src/lib/validators/objectTypes.ts

Lines changed: 60 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,11 @@ export const OBJECTS_VALIDATIONS: { [key: string]: IObjectSchema } = {
106106
nullable: false,
107107
required: false,
108108
},
109+
fullStorage: {
110+
type: 'boolean',
111+
nullable: false,
112+
required: false,
113+
},
109114
},
110115
},
111116
tracerConfigWrapper: {
@@ -123,6 +128,27 @@ export const OBJECTS_VALIDATIONS: { [key: string]: IObjectSchema } = {
123128
nullable: false,
124129
required: false,
125130
},
131+
// OpcodeLogger config properties at top level
132+
enableMemory: {
133+
type: 'boolean',
134+
nullable: false,
135+
required: false,
136+
},
137+
disableStack: {
138+
type: 'boolean',
139+
nullable: false,
140+
required: false,
141+
},
142+
disableStorage: {
143+
type: 'boolean',
144+
nullable: false,
145+
required: false,
146+
},
147+
fullStorage: {
148+
type: 'boolean',
149+
nullable: false,
150+
required: false,
151+
},
126152
},
127153
},
128154
transaction: {
@@ -236,32 +262,52 @@ export function validateTracerConfigWrapper(param: any): boolean {
236262
const valid = validateSchema(schema, param);
237263
const { tracer, tracerConfig } = param;
238264

239-
if (!tracerConfig) {
240-
return valid;
241-
}
242-
243265
const callTracerKeys = Object.keys(OBJECTS_VALIDATIONS.callTracerConfig.properties);
244266
const opcodeLoggerKeys = Object.keys(OBJECTS_VALIDATIONS.opcodeLoggerConfig.properties);
245267

246-
const configKeys = Object.keys(tracerConfig);
247-
const hasCallTracerKeys = configKeys.some((k) => callTracerKeys.includes(k));
248-
const hasOpcodeLoggerKeys = configKeys.some((k) => opcodeLoggerKeys.includes(k));
268+
// Check for opcodeLogger config properties at the top level
269+
const topLevelKeys = Object.keys(param);
270+
const hasTopLevelOpcodeLoggerKeys = topLevelKeys.some((k) => opcodeLoggerKeys.includes(k));
271+
272+
// Check for tracer config properties in nested tracerConfig
273+
let hasNestedCallTracerKeys = false;
274+
let hasNestedOpcodeLoggerKeys = false;
275+
if (tracerConfig) {
276+
const configKeys = Object.keys(tracerConfig);
277+
hasNestedCallTracerKeys = configKeys.some((k) => callTracerKeys.includes(k));
278+
hasNestedOpcodeLoggerKeys = configKeys.some((k) => opcodeLoggerKeys.includes(k));
279+
}
249280

250-
// we want to accept ICallTracerConfig only if the tracer is callTracer
251-
// this config is not valid for opcodeLogger and vice versa
252-
// accept only IOpcodeLoggerConfig with opcodeLogger tracer
253-
if (hasCallTracerKeys && tracer === TracerType.OpcodeLogger) {
281+
// Don't allow both top-level and nested config properties at the same time
282+
if (hasTopLevelOpcodeLoggerKeys && tracerConfig) {
254283
throw predefined.INVALID_PARAMETER(
255284
1,
256-
`callTracer 'tracerConfig' for ${schema.name} is only valid when tracer=${TracerType.CallTracer}`,
285+
`Cannot specify tracer config properties both at top level and in 'tracerConfig' for ${schema.name}`,
257286
);
258287
}
259288

260-
if (hasOpcodeLoggerKeys && tracer !== TracerType.OpcodeLogger) {
289+
// Don't allow top-level config properties when tracer is explicitly specified
290+
if (hasTopLevelOpcodeLoggerKeys && tracer) {
261291
throw predefined.INVALID_PARAMETER(
262292
1,
263-
`opcodeLogger 'tracerConfig' for ${schema.name} is only valid when tracer=${TracerType.OpcodeLogger}`,
293+
`Cannot specify tracer config properties at top level when 'tracer' is explicitly set for ${schema.name}`,
264294
);
265295
}
296+
297+
// Validate nested config properties with tracer types (existing logic)
298+
if (hasNestedCallTracerKeys && tracer === TracerType.OpcodeLogger) {
299+
throw predefined.INVALID_PARAMETER(
300+
1,
301+
`callTracer config properties for ${schema.name} are only valid when tracer=${TracerType.CallTracer}`,
302+
);
303+
}
304+
305+
if (hasNestedOpcodeLoggerKeys && tracer !== TracerType.OpcodeLogger) {
306+
throw predefined.INVALID_PARAMETER(
307+
1,
308+
`opcodeLogger config properties for ${schema.name} are only valid when tracer=${TracerType.OpcodeLogger}`,
309+
);
310+
}
311+
266312
return valid;
267313
}

packages/relay/tests/lib/validators/validators.spec.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -687,6 +687,13 @@ describe('Validator', async () => {
687687
{ tracer: Constants.TracerType.OpcodeLogger, tracerConfig: {} },
688688
{ tracer: Constants.TracerType.OpcodeLogger },
689689
{ tracerConfig: { enableMemory: true, disableStack: false, disableStorage: true } },
690+
// Top level opcodeLogger config without explicit tracer (defaults to opcodeLogger)
691+
{ enableMemory: true, disableStack: false, disableStorage: true },
692+
{ enableMemory: true },
693+
{ disableStack: false },
694+
{ disableStorage: true },
695+
{ fullStorage: false }, // Non-standard but accepted for Remix compatibility
696+
{ enableMemory: true, disableStack: false, disableStorage: true, fullStorage: false },
690697
// Empty object
691698
{},
692699
],
@@ -695,6 +702,37 @@ describe('Validator', async () => {
695702
input: { tracer: 'invalid', tracerConfig: {} },
696703
error: expectInvalidParam("'tracer' for TracerConfigWrapper", TYPES.tracerType.error, 'invalid'),
697704
},
705+
// Config properties with explicit tracer (not allowed in simplified model)
706+
{
707+
input: { tracer: Constants.TracerType.CallTracer, enableMemory: true },
708+
error: expectInvalidParam(
709+
1,
710+
"Cannot specify tracer config properties at top level when 'tracer' is explicitly set for TracerConfigWrapper",
711+
),
712+
},
713+
{
714+
input: { tracer: Constants.TracerType.CallTracer, disableStack: false },
715+
error: expectInvalidParam(
716+
1,
717+
"Cannot specify tracer config properties at top level when 'tracer' is explicitly set for TracerConfigWrapper",
718+
),
719+
},
720+
// Both top-level and nested config
721+
{
722+
input: { enableMemory: true, tracerConfig: { disableStack: false } },
723+
error: expectInvalidParam(
724+
1,
725+
"Cannot specify tracer config properties both at top level and in 'tracerConfig' for TracerConfigWrapper",
726+
),
727+
},
728+
// Top-level config with explicit tracer (not allowed)
729+
{
730+
input: { tracer: Constants.TracerType.OpcodeLogger, enableMemory: true },
731+
error: expectInvalidParam(
732+
1,
733+
"Cannot specify tracer config properties at top level when 'tracer' is explicitly set for TracerConfigWrapper",
734+
),
735+
},
698736
{
699737
input: { tracer: Constants.TracerType.CallTracer, tracerConfig: { onlyTopCall: 'invalid' } },
700738
error: expectInvalidParam(

packages/server/tests/acceptance/debug.spec.ts

Lines changed: 84 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -605,7 +605,7 @@ describe('@debug API Acceptance Tests', function () {
605605
[createChildTx.hash, invalidTracerConfig],
606606
predefined.INVALID_PARAMETER(
607607
1,
608-
"callTracer 'tracerConfig' for TracerConfigWrapper is only valid when tracer=callTracer",
608+
'callTracer config properties for TracerConfigWrapper are only valid when tracer=callTracer',
609609
),
610610
);
611611
});
@@ -620,7 +620,89 @@ describe('@debug API Acceptance Tests', function () {
620620
[createChildTx.hash, invalidTracerConfig],
621621
predefined.INVALID_PARAMETER(
622622
1,
623-
"opcodeLogger 'tracerConfig' for TracerConfigWrapper is only valid when tracer=opcodeLogger",
623+
'opcodeLogger config properties for TracerConfigWrapper are only valid when tracer=opcodeLogger',
624+
),
625+
);
626+
});
627+
628+
it('should support both nested and top-level tracer config parameter formats', async function () {
629+
// Test the nested format (original format)
630+
const nestedFormat = {
631+
tracer: TracerType.OpcodeLogger,
632+
tracerConfig: {
633+
enableMemory: true,
634+
disableStack: false,
635+
disableStorage: true,
636+
},
637+
};
638+
639+
const nestedResult = await relay.call(DEBUG_TRACE_TRANSACTION, [createChildTx.hash, nestedFormat]);
640+
expect(nestedResult).to.exist;
641+
expect(nestedResult.structLogs).to.exist;
642+
643+
// Test the top-level format (new format from our fix)
644+
const topLevelFormat = {
645+
enableMemory: true,
646+
disableStack: false,
647+
disableStorage: true,
648+
};
649+
650+
// Test with fullStorage parameter (Remix compatibility)
651+
const topLevelWithFullStorage = {
652+
enableMemory: true,
653+
disableStack: false,
654+
disableStorage: true,
655+
fullStorage: false, // Non-standard parameter sent by Remix
656+
};
657+
658+
const topLevelResult = await relay.call(DEBUG_TRACE_TRANSACTION, [createChildTx.hash, topLevelFormat]);
659+
expect(topLevelResult).to.exist;
660+
expect(topLevelResult.structLogs).to.exist;
661+
662+
// Test that fullStorage parameter is accepted (Remix compatibility)
663+
const fullStorageResult = await relay.call(DEBUG_TRACE_TRANSACTION, [
664+
createChildTx.hash,
665+
topLevelWithFullStorage,
666+
]);
667+
expect(fullStorageResult).to.exist;
668+
expect(fullStorageResult.structLogs).to.exist;
669+
670+
// All formats should produce similar results
671+
expect(topLevelResult.structLogs).to.have.length.greaterThan(0);
672+
expect(nestedResult.structLogs).to.have.length.greaterThan(0);
673+
expect(fullStorageResult.structLogs).to.have.length.greaterThan(0);
674+
});
675+
676+
it('should reject mixed format (top-level and nested config)', async function () {
677+
const mixedFormat = {
678+
enableMemory: true,
679+
tracerConfig: {
680+
disableStack: false,
681+
},
682+
};
683+
684+
await relay.callFailing(
685+
DEBUG_TRACE_TRANSACTION,
686+
[createChildTx.hash, mixedFormat],
687+
predefined.INVALID_PARAMETER(
688+
1,
689+
"Cannot specify tracer config properties both at top level and in 'tracerConfig' for TracerConfigWrapper",
690+
),
691+
);
692+
});
693+
694+
it('should reject top-level config when tracer is explicitly set', async function () {
695+
const invalidFormat = {
696+
tracer: TracerType.OpcodeLogger,
697+
enableMemory: true, // Not allowed - tracer is explicitly set
698+
};
699+
700+
await relay.callFailing(
701+
DEBUG_TRACE_TRANSACTION,
702+
[createChildTx.hash, invalidFormat],
703+
predefined.INVALID_PARAMETER(
704+
1,
705+
"Cannot specify tracer config properties at top level when 'tracer' is explicitly set for TracerConfigWrapper",
624706
),
625707
);
626708
});

0 commit comments

Comments
 (0)