feat: decode composite (vec/map) call arguments; raise recursion limit - #52
Open
RaoulSchaffranek wants to merge 1 commit into
Open
feat: decode composite (vec/map) call arguments; raise recursion limit#52RaoulSchaffranek wants to merge 1 commit into
RaoulSchaffranek wants to merge 1 commit into
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.
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
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
Two fixes that surfaced while debugging a real Soroban contract through the komet-node-backed debugger.
1. Composite (Vec/Map) call arguments
komet-node rejected any
Vec/Map— and therefore any user enum, struct, or tuple — contract call argument at admission:scval_to_jsonraisedNotImplementedErroronSCV_VEC/SCV_MAP, and the#decodeArgrules had only scalar cases, sosendTransactionfailed withtxMALFORMEDand the invocation never ran.This adds recursive vec/map support on both sides:
scval.py—scval_to_jsonemits{"type":"vec","value":[…]}and{"type":"map","value":[{"key":…,"val":…},…]}, recursing element-wise. -node.md—#decodeArgvec/map rules:ScVec(#decodeArgList(…))andScMap(#decodeMapEntries(…)). Enums, structs, and tuples all reduce to vec/map at the XDR level, so these two rules cover every composite argument.Verified end-to-end (
test_call_tx_with_composite_args): a contract call carrying a flat vec, a nestedVec<(enum, i128)>(with anAddress-carrying variant and a negativei128), a struct-shaped map, and a map-nested-in-vec reachesSUCCESS, and the arguments echoed in the trace'scallContractframe round-trip back to the exactSCVals that were sent — so a decoding bug is caught even when the transaction still succeeds (same style as the existingtest_call_tx_with_args).2. Recursion limit for large real contracts
Large real contracts produce a KORE world-state term far deeper than CPython's default recursion limit (1000), which surfaced as a
RecursionErrormid-request during pyk's recursive-descent KORE parsing / config traversal.__init__.py—sys.setrecursionlimit(10**7), matching the rest of theK tooling (pyk sets
10**7; komet sets its own limit at import).server.py— run the blocking serve loop on a worker thread with a512 MB stack, so a deep term raises a catchable
RecursionErrorrather thanoverflowing the 8 MB default stack into a SIGSEGV (keeping the server's
"never take down the serve thread" guarantee intact).
test_scval.py— unit tests pinning the vec/map JSON shape(order-sensitive, since
#decodeArgmatches on JSON member order) and that adeeply nested value encodes without hitting the recursion limit.
Testing
kdistrebuild: 97 passed.make checkclean: flake8, mypy (24 files), black, isort.Notes
The two changes are independent and can be split into separate PRs if preferred; they are bundled here because both came out of the same debugging session. Developed test-first (red proven before green).