feat: decode composite (vec/map) call arguments; raise recursion limit - #52
Merged
Conversation
komet-node rejected any Vec/Map — and thus any user enum/struct/tuple —
contract call argument at admission: scval_to_json raised NotImplementedError
on SCV_VEC/SCV_MAP and #decodeArg had only scalar rules, so the transaction
never ran. Add recursive vec/map support on both sides so composite arguments
are decoded and executed:
- scval.py: scval_to_json emits {"type":"vec","value":[...]} and
{"type":"map","value":[{"key":..,"val":..},..]}, recursing element-wise.
- node.md: #decodeArg vec/map rules — ScVec(#decodeArgList(...)) and
ScMap(#decodeMapEntries(...)). Enums, structs, and tuples all reduce to
vec/map at the XDR level, so these cover every composite call argument.
- args.wat / test_server.py: a call carrying flat, nested (Vec<(enum,i128)>
with an Address variant and a negative i128), map, and map-in-vec arguments
reaches SUCCESS and its trace's callContract frame round-trips the exact
SCVals sent.
Also raise the Python recursion limit — large real contracts produce a KORE
world-state term far deeper than CPython's default 1000, which surfaced as a
RecursionError mid-request during pyk parsing / config traversal:
- __init__.py: sys.setrecursionlimit(10**7), matching the rest of the K
tooling (pyk sets 10**7; komet sets its own limit at import).
- server.py: run the blocking serve loop on a worker thread with a 512 MB
stack, so a deep term raises a catchable RecursionError instead of
overflowing the 8 MB default stack into a SIGSEGV.
- test_scval.py: unit tests pinning the vec/map JSON shape (order-sensitive,
since #decodeArg matches on member order) and that a deeply nested value
encodes without hitting the recursion limit.
…ract Reassembling the trace array in the semantics recursively copied the whole remaining tail once per line — O(n^2) time and memory that OOM-killed the interpreter on multi-hundred-MB traces. Serve traceTransaction directly from traces/trace_<hash>.jsonl in one linear pass instead, bypassing the interpreter. Each served record is stamped with an executingContract field — the contract executing at that record, reconstructed from the callContract/endWasm call-boundary markers — so a consumer can map each pos against the right contract binary. The field is named executingContract, not contract, to avoid clobbering the contractData record's own documented contract field.
#traceLedger writes the ledger scalars and every account's balance as the trace's first line, before any step runs, so a debugger can seed its view of chain state and replay the per-operation events on top of it instead of seeing only what a contract happened to touch. Balances are gathered one per rewrite step by #collectAccounts, since <accounts> is a cell collection that no function can take as an argument. Also records the executing module's globals on each instruction record. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`generateLedgerTrace` and `AccountBalances2JSONs` lived in komet's `tracing.md` but had no caller there -- `collectAccounts-done` below is the only one. Keeping them upstream meant they were untested and undocumented where they lived, and made this module depend on a komet newer than the v0.1.86 it pins. `imports JSON-UTILS` is now explicit, for `Address2JSON`. It was previously reachable only through KASMER's import chain into `TRACING`, which sits behind komet's `k-tracing` md selector -- so relying on it would have made this module silently require a tracing-enabled komet build. Also corrects the rationale in the surrounding prose, which had it backwards: `<ledgerSequenceNumber>`, `<ledgerTimestamp>` and `<accounts>` are all declared in komet's `configuration.md`, not here. What belongs to komet-node is the record, not the cells. The same passage referred to komet's `#collectGlobals`, which no longer exists; it now points at `moduleGlobals` and notes that reading cells as function context would remove these rewrite steps here too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…format
komet v0.1.87 ("Clean up the trace format", #122) replaced the `pos`/`instr`
tagging of trace records with a top-level `kind` field and flattened each
record's own fields, and v0.1.88 (#126) added `globals` to every instruction
record. Bumping the pin alone would have broken the serve path silently, so
this migrates komet-node with it.
- pyproject.toml / uv.lock: v0.1.86 -> v0.1.88. pykwasm is unchanged (v0.1.155).
- server.py: `_annotate_trace_lines` keyed its call-boundary stack on
`instr[0]`, which no longer exists on `callContract`/`endWasm` records — it
would have tagged every served record `executingContract: null` without
raising. It now dispatches on `kind`, and the cheap substring prefilter
matches `"endWasm"` exactly rather than the `"endWasm` prefix: the trap
spelling it was guarding against (`endWasm-error`) is a K rule name, never a
record kind. komet emits one `endWasm` record for both outcomes, telling them
apart by `success`, so the pop keys on `kind` alone.
- node.md: `generateLedgerTrace` emits `{"kind": "ledger", ...}`, dropping the
`pos`/`instr` pair, so the baseline record komet-node contributes matches the
format of the komet records around it.
- README.md / docs: the trace format, record-by-record. The README trace
section also gains the `ledger` record and the `executingContract` tag, both
of which it predated.
- test_server.py: trace assertions and synthetic record fixtures move to
`kind`. Also fixes two lint errors that already failed `make check` on this
branch (an unused local and a quote-escaping warning).
Verified with `make check`, `make test-unit` (9 passed) and, against a kdist
rebuild of the v0.1.88 semantics, `make test-integration` (101 passed).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
traceTransaction stamped every served record with an executingContract field naming the contract running at it, reconstructed by walking the trace's callContract/endWasm boundaries. That was the wrong layer. komet-node is a thin stateful wrapper around komet, and the field carried no information the trace did not already have: a callContract names its callee and an endWasm closes it, so a consumer folds it out of records it is walking anyway. Doing it here cost three things. It duplicated ~87 bytes of derivable data per record on traces that run to hundreds of megabytes — added by the very code path that exists to keep memory proportional to the trace. It made the served array differ from the stored file, so the RPC and the on-disk format disagreed about what a record is. And it coupled komet-node to komet's record semantics: the v0.1.87 format change broke exactly this function and nothing else, because it is the only place here that looks inside a record. The serve path is now a linear join of the file's lines with no JSON parsing at all. The debug adapter derives the executing contract itself (simbolik-komet, src/komet/executingContract.ts), where it also has the call-frame stack it needs for its own Ledger view. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every RPC call parsed state.kore into a pyk Pattern and immediately re-serialized
it, twice — once on the way in, once on the way out — whether or not the request
had any reason to look at the configuration. Nothing did: requests reach the
semantics through request.json, which insert-handleRequestFile picks up off disk,
so the only configuration edit Python ever makes is splicing an uploaded module
into the <program> cell.
The cost was entirely a function of world-state size, and world-state size is
~40x the wasm code section a chain has accumulated. At 3 MB of state, pyk's KORE
parser needed 1.8s where the interpreter needs 0.5s, and a getHealth — which
returns a constant — took 4.9s against 0.01s on an empty chain. A 16.9 MB state
cost 30s of marshalling per call.
So exchange KORE as text and let the interpreter read the state file itself:
- _llvm_interpret becomes _run_interpreter, returning stdout verbatim. A Path
config is passed as a path; only a str config goes on stdin.
- run() hands state.kore to the interpreter and writes its output straight
back, with no parse in either direction.
- _inject_program becomes splice_program, a textual substitution into the idle
<program> cell. A saved configuration is always idle, so the marker occurs
exactly once; anything else raises rather than silently dropping the module.
- the module's KORE is cached on disk, keyed by wasm content hash and a stamp
of the compiled semantics, so re-uploading an unchanged contract is a file
read. The cache lives outside the io-dir, which is a fresh temp directory per
debug session.
Replaying a 31-request debug session: 177.2s -> 13.0s. Traces and receipts are
byte-identical, and the resulting state.kore is the same KORE term (1.7% smaller
as text, since the interpreter's printer is more compact than Pattern.text).
Converting an uploaded module to KORE was the dominant remaining cost of a debug session: 6.5s for an optimized 70 KB contract, 40.2s for the 389 KB an unoptimized build with debug info produces. kast_to_kore is general, and pays for it — it runs six whole-term normalization passes, builds a KORE term, and leaves the caller to serialize that, so every stage rebuilds every node. A 499,701-node module drove 401,844 KApply allocations, and resolve_sorts was called about twice per node, uncached, over a few hundred distinct labels. Five of the six passes cannot change a term built by wasm2kast, which has no variables, no K sequences and no cells — measured as changed=False on every real module, and 20.1s of the 40.2s. kast_to_kore_text walks a plain term once and writes KORE text straight into a buffer, memoizing each lookup by label, sort, or token. Plain means a tree of KApply and KToken with no sequences, variables, rewrites, ML connectives or cells, and with every parametric label's sort parameters already resolved — exactly the features the skipped passes exist to rewrite, which is what makes skipping them sound and not merely faster. A term that is not plain falls back to kast_to_kore. optimized 70 KB module 6.49s -> 0.38s -O0+DWARF 389 KB module 42.70s -> 2.54s and end to end, an upload of that 389 KB module goes 51.2s -> 12.2s, while the 31-request session replay goes 13.0s -> 9.7s. The emitted KORE is byte-identical to kast_to_kore's output, which the tests assert against real contract modules rather than trusting the reasoning above; traces and receipts are unchanged. Nothing here is Soroban- or wasm-specific. This is generic pyk.konvert material and belongs upstream, where komet's own kasmer paths would get it too; it lives here until it does. The two pyk helpers it reuses are private, so the byte-equality tests are what will catch a pyk bump changing them — and the kframework pin is exact, so such a bump is deliberate.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Four fixes from one debugging session against a real Soroban contract.
Composite call arguments.
Vec/Maparguments — and so any user enum, struct, or tuple — were rejected astxMALFORMEDbefore the transaction ran. Both the Python encoder and the K decoder now recurse, covering every composite argument rather than a list of special cases.traceTransactionno longer OOM-kills the interpreter. Traces were reassembled inside the semantics, copying the whole remainder once per line. The server now joins the stored file's lines in one pass with no JSON parsing and returns them verbatim; anything derivable is left to the consumer.Every trace opens with a ledger baseline. Sequence, timestamp, and all account balances as the transaction found them, so a debugger can show chain state it did not watch being written.
Large contracts no longer crash mid-request. Deep world-state terms blew CPython's recursion limit; it is raised to match the rest of the K tooling, with a large serve-thread stack so a deep term raises rather than segfaults.
komet v0.1.86 → v0.1.88 — breaking trace format change
Needed for the globals the debugger uses to resolve local variables. v0.1.87 also reorganised trace records: each now names itself with a
kindfield instead of encoding its type insidepos/instr, with type-specific fields spelled out. So{"instr": ["contractData", "put", "temporary"], …}becomes{"kind": "contractData", "operation": "put", "durability": "temporary", …}.komet-node is migrated to match, including the
ledgerrecord it emits itself.Downstream
The VS Code debugger must land after this, not before: it is migrated on its own branch and now requires v0.1.87 or newer, so either half released alone will not run. That branch also flips its composite-argument test from asserting komet-node cannot encode a
Vecto tracing one end to end.