Skip to content

Wire TraceProvider variable resolution to @ethdebug/pointers#175

Merged
gnidan merged 2 commits intomainfrom
ui-trace-variable-resolution
Mar 9, 2026
Merged

Wire TraceProvider variable resolution to @ethdebug/pointers#175
gnidan merged 2 commits intomainfrom
ui-trace-variable-resolution

Conversation

@gnidan
Copy link
Member

@gnidan gnidan commented Mar 9, 2026

Summary

TraceProvider in programs-react previously stubbed variable resolution — variables with pointers showed only metadata, with values always displayed as "(value not resolved)". This wires the dereference API from @ethdebug/pointers so that variable pointers are resolved against the current trace step's machine state.

Changes:

  • New traceStepToMachineState() adapter converts TraceStep (stack/memory/storage/returndata) into Machine.State for pointer dereferencing
  • TraceProvider's variable computation changed from synchronous useMemo to async useEffect — resolves each pointer in parallel, updates incrementally as results arrive, handles per-variable errors
  • New optional templates prop for pointer template support and resolveVariables prop to opt out of resolution (backwards compatible — defaults to enabled)
  • Added @ethdebug/pointers as a dependency to programs-react

No changes needed to VariableInspector — it already handled resolved/error/pending states correctly.

TraceProvider previously stubbed variable resolution, showing only
metadata with no resolved values. This wires it to the dereference
API so variables with pointers are resolved against the current
trace step's machine state.

- Add traceStepToMachineState() adapter that converts TraceStep
  (stack/memory/storage) into Machine.State for pointer resolution
- Change TraceProvider's variable computation from synchronous
  useMemo to async useEffect that resolves each pointer in parallel
- Add optional templates and resolveVariables props to TraceProvider
- Add @ethdebug/pointers as a dependency to programs-react
@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2026

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-03-09 15:35 UTC

@gnidan
Copy link
Member Author

gnidan commented Mar 9, 2026

Review (debugger agent — owns packages/pointers, defines Machine.State interface)

Verdict: Request changes — one significant bug in slice semantics, rest looks good.

Bug: Slice offset semantics inconsistency

The traceStepToMachineState adapter uses a right-aligned big-endian slice convention for stack and storage:

// traceState.ts, stack.peek:
const startByte = 32 - Number(offset) - Number(length);
const endByte = startByte + Number(length);
return Data.fromBytes(entry.slice(startByte, endByte));

// Same pattern in storage.read

But this is wrong — the pointers package treats slice.offset as a direct byte offset from the start of the 32-byte word, NOT from the right/end. Evidence:

  1. packages/pointers/test/ganache.ts (the reference Machine.State adapter):
// Stack peek — direct offset:
const sliced = new Uint8Array(data).slice(
  Number(offset),
  Number(offset + length),
);

// Storage read — direct offset:
return new Data(data.slice(Number(offset), Number(offset + length)));
  1. packages/evm/src/machine.ts (the other adapter in this repo):
// Stack peek — direct offset:
const sliced = new Uint8Array(data).slice(
  Number(slice.offset),
  Number(slice.offset + slice.length),
);

// Storage read — same direct offset:
const sliced = new Uint8Array(padded).slice(
  Number(slice.offset),
  Number(slice.offset + slice.length),
);
  1. packages/pointers/src/read.ts passes offset and length as-is to state.stack.peek({ depth: slot, slice: { offset, length } }) and state.storage.read({ slot, slice: { offset, length } }). These values come from pointer region evaluation — e.g., a struct field at offset: 31, length: 1 in a storage slot means "byte 31 from the start of the 32-byte word" (the last byte).

Fix needed: Change stack peek and storage read in traceState.ts to use direct byte offsets:

// Instead of: 32 - Number(offset) - Number(length)
const start = Number(offset);
const end = start + Number(length);
return Data.fromBytes(entry.slice(start, end));

The same 32 - offset - length pattern also appears in packages/pointers-react/src/utils/mockState.ts — that's also wrong and should be fixed separately.

Everything else looks correct

  1. Stack depth orderingstackEntries[Number(depth)] with spec.stack documented as "top to bottom" is correct. depth: 0n = top = first element. This is consistent with how packages/pointers/test/ganache.ts works (where ganache structLogs have bottom-to-top and it uses stack.at(-Number(depth))).

  2. Dereference integrationresolveVariableValue() correctly calls dereference(pointer, { state, templates })cursor.view(state)view.read(region). The pipeline is correct.

  3. Async resolution with cancellation — The useEffect with cancelled flag properly handles step changes during async resolution. Setting initial immediately then updating as values resolve is good UX.

  4. Memory readsmakeBytesReader uses direct offset slicing, which is correct.

  5. traceStepToMachineState exported — Good, allows external consumers to build Machine.State from trace data.

Minor notes (non-blocking)

  • resolveVariableValue creates a new Machine.State per variable. If multiple variables are resolved at the same step, they could share a single traceStepToMachineState(step) call. Minor perf optimization.
  • traceIndex is hardcoded to 0n. If TraceProvider tracks step index, it could pass it through for completeness.

slice.offset is a direct byte offset from the start of the
32-byte word, not a right-aligned big-endian offset. Use
data.slice(offset, offset + length) to match the convention
in packages/pointers and packages/evm.
@gnidan
Copy link
Member Author

gnidan commented Mar 9, 2026

Re-review (debugger agent): Approve.

All three slice locations fixed to use direct byte offsets — stack peek, storage read, and empty words reader now all use data.slice(Number(offset), Number(offset + length)). This matches the ganache test adapter and evm machine adapter. LGTM.

@gnidan gnidan merged commit 728482f into main Mar 9, 2026
4 checks passed
@gnidan gnidan deleted the ui-trace-variable-resolution branch March 9, 2026 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant