protocol: decode every AlgebraicType from the schema tags #46
No reviewers
Labels
No labels
area:defold
area:docs
area:infra
area:protocol
area:testing
kind:bug
kind:chore
phase:breadth
phase:core
phase:hardening
phase:slice
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
yuris/spacetimedb-defold!46
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/26-decode-types"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #26.
The SDK handed row values through untouched. That is correct for scalars and strings — every column
messagehas — and wrong or unusable for everything else. Nobody had hit it because the only tables under test were three scalar columns plusprofile.New
all_typestable with 31 columns, one per AlgebraicType, plus a payload-carrying enum and reducers that insert a fully-populated row and an all-empty row. Captured in both encodings (subscription readback andTransactionUpdate) intosession.jsonl, 24 → 29 frames, appended so indices 1–24 do not move.Then the decoder written against those bytes:
protocol.decode_typed(encoded, spec, typespace)andprotocol.decode_value(v, tag, typespace).Five wire facts the repo had wrong or did not know
Captured, not reasoned:
vec_u8is HEX TEXT, not a JSON array —"0001feff", and""when empty. Undocumented anywhere in this repo before now.i256/u256are hex text of minimal width, sign-prefixed-0x. Zero is"0x0", not a padded 64-digit string.Optioncolumn is an ordinal-keyed sum[0,v]/[1,{}]— not{"some":v}/{"none":[]}asprotocol.lua's header claimed. The variant-name form is real but belongs to AlgebraicType JSON from the schema endpoint, never to a row value. Header corrected.ScheduleAtvariant names are capitalised (Interval/Time) because they come from the library, whereas module enum variants are lowercased by the server. A decoder resolving ordinals by name has to know that.i128/u128arrive as bare full-precision JSON numbers, a sum is[ordinal,payload]in both encodings, and special types are 1-field products (named wrapper / 1-element positional array).Three representation decisions
i128/u128→ exact decimal digit string, always a string even for narrow values so the type never depends on magnitude. Exactness comes fromquote_wide_ints, which quotes ≥16-digit integer literals in the raw row text beforejson.decode. The scanner steps over string literals, honouring backslash escapes, because a text column may legitimately contain a long digit run.i256/u256→ the server's hex string verbatim. Already exact; decimal conversion would need bignum division in Lua for no gain. The asymmetry withi128is the server's, not the SDK's.i64/u64unchanged — still a Lua double, still rounds past 2^53. Retyping would retypemessage.id, au64pk the cache keys on, which is #35's frozen-surface call. Pinned attest_fixtures.lua:481-482:u64::MAXcompares equal only because both sides rounded, andstring.format("%.0f", …)of it is18446744073709551616— off by one, asserted so nobody reads it as exact.Some(nil)versusNoneis not distinguishable after normalisation — a Lua field holding nil is absent. Settled empirically rather than argued: anOption<Option<i32>>column was added, and a JS module writingnullemits the outernone([1,{}]). Captured and asserted, with the escape hatch documented (read element 1 of the raw column).The rescue mark is unforgeable by construction
A rescued literal is marked with a leading U+0001 so
unrescuecan tell a number this pass stringified from a string the server really sent. An earlier revision of this branch justified that mark by claiming no value could produce it. That was false, and measured false: writing\u0001followed by 18 nines into a string column on a live 2.8.2 server round-trips it verbatim, and rxi's json decodes the escape to the raw byte. Under an uninterpretable tag the string then came back as the number1e+18— silent corruption of a legal server payload.So the mark is no longer unlikely, it is escaped. The scanner already steps over string literals; it now doubles a leading mark while copying one. A rescue emits a single mark followed by digits, so
\1\1…can only be a server string and\1…can only be this pass's own work. Deterministic, no nonce.unrescuetests the doubled form first, and every tag that hands a value back without interpreting it goes throughunrescueorunmark.Verified against bytes a live server actually sent: the forged string survives byte-for-byte (19 bytes, first byte
0x01) underunknown,string,u64,f64,u128,i128,vec_u8,identityandi256, nested inside anunknowntable, in a positional row, and beside a genuine 39-digit rescue in the same row — which still decodes exactly.A 128-bit value that never met the rescue decodes to nil
decode_valuetakes an already-decoded value, so on any path that does not quote the raw text first — a procedure return throughdecode_payload, which the docstring explicitly blesses — a 128-bit number arrives already rounded.string.format("%.0f", v)would then print exact-looking digits for bits that are gone:u128::MAXcame back340282366920938463463374607431768211456, off by one at 39 digits, and a capturedconnection_idcame back nine digits wrong.Past 2^53 the answer is now nil, matching the call #37 already made in
u128_digits.%.0fis kept only below 2^53, where every integer has its own double and the digits are exact. Both sides of the boundary are asserted, including the negative one, and2^53itself returns nil because it shares a double with2^53+1.Opt-in, deliberately
decode_typedis a second entry point, not wired intodecode_row. Per-column coercion on the shared path would change the type of values consumers already receive — an identity column would stop being a table, a timestamp would stop being a wrapper. That is a frozen-surface break and #35's cross-cutting decision, not this PR's.connection_id/uuiddecode to the exact decimal string the server sends andidentityto its0xhex, leaving presentation to #37's accessors — which chains cleanly, since theirscalar()passes non-tables through.What a consumer may do with a 128-bit string
Documented in the README rather than left to be discovered, because Lua 5.1 makes both failures silent. Arithmetic coerces:
row.serial + 1yields a rounded double and raises nothing, so the value survives arrival and is destroyed by first use. And</>are lexicographic, not numeric —"9007199254740993" < "10000000000000000"isfalse,table.sorton a 128-bit column gives10000000000000000 < 9007199254740993 < 999, and fori128the-prefix inverts it again ("-8" < "-9"istrue). Compare with==, which is exact because the representation is canonical, and leave 128-bit arithmetic and ordering to the server.Verification
6 passed, 0 failed— the sixth suite,test_tables.lua, arrived with #45;stylua --checkclean. Fixture-replay test decodes every type from real captured bytes in both encodings. Rebased ontofeeb6c2after #45 landed: clean, no conflicts. #45 touches neitherprotocol.luanortest_protocol.lua, andstdb/tables.luadoes not calldecode_typed.Compatibility.
decode_rowis byte-identical toorigin/main, per-function md5 of the extracted body — as aredecode_server,decode_payload,unwrap,encode_client,quote_big_ints,u128_digitsandscalar. Diffing the whole region above the TYPED VALUE DECODING banner against main gives comment-only hunks: zero code lines changed. All new code is below the banner.Mutation testing, 7 mutations, all 7 caught:
WIDE_DIGITS = 179007199254740993, gotnilunknown→ got1e+18unrescuedoubled-mark check removedunknownrescued_digitsdoubled-mark check removedu64→ gotnilunmarkdropped from pass-through tagsstring%.0fmade unconditional again2^53→ expectednil, got9007199254740992<= MAX_EXACTWIDE_DIGITSwas previously the one mutation the suite did not catch: the fixture's 128-bit values are 0 and 39/40 digits, so nothing crossed 2^53. It is now pinned by four assertions at the boundary.Native conformance PASS, both runs including identity persistence across a process restart, on a private server (port 4200, own state dir); the shared port was untouched. The forged-sentinel round-trip above was captured on that same server.
Independent checks recorded during the run:
chat_schema.lua's["message"]and["profile"]blocks are byte-identical to main, all 7 pre-existing reducers/procedures are still present includingedit_message, andsession.jsonlframe 24 still carries 2updatesentries — so PR #44's blocker citation still holds.Also corrected
gen-schema.sh's "only the third reason has a consumer" claim, in both the prose header and the stamped generated header: it is now false, because this decoder readstypesandtypespace.The SDK handed row values through as json.decode left them. That is correct for the three scalar columns the test module had and unusable for anything else: an enum column reached a consumer as {1,{}}, a zero-based ordinal with no variant name anywhere, and a Timestamp reached it as a one-field wrapper table. Nobody had noticed because no module under test owned a column that was not a u64 or a string. Every claim about how these types are encoded was previously read from server source, and this repo's history is a list of cases where the wire disagreed. So the order here is capture first, decode second. dev/server/index.js gains an `all_types` table with one column of every AlgebraicType a module can declare, plus two reducers -- one filling every column, one emptying every column that can be empty -- and the capture appends steps that read those two rows back in BOTH encodings. Reading them twice is the point: the same bytes arrive positional in a TransactionUpdate and named in a subscription readback, so a decoder can be proved to handle the split per TYPE rather than passing by accident on whichever encoding it happened to see. The capture contradicted three things this repo believed: * A byte array is HEX TEXT ("0001feff"), not a JSON array. A consumer who trusted the `vec_u8` tag and indexed it would have read characters. * i256/u256 are hex text of MINIMAL width -- zero is "0x0", not padded -- so nothing may assume a length. * An Option COLUMN is an ordinal-keyed sum, [0,v] / [1,{}], not the {"some":v} / {"none":[]} the header comment claimed. That form is real but belongs to AlgebraicType JSON, never to a row value. Comment fixed. Three representation choices, because Lua 5.1 has one number type: * i128/u128 decode to an exact decimal digit STRING, narrow values included, so the type never depends on the magnitude. Exactness comes from quoting wide integer literals in the RAW row text before json.decode, which is the only moment the digits still exist. The scanner steps over string literals rather than regexing the whole frame, because a text column may legitimately contain a long digit run and quoting inside it would corrupt the value -- that safety property has its own test, and without it the real capture fails to parse at all. * i256/u256 keep the server's hex string. Already exact; converting to decimal would mean bignum division in Lua for nothing. * i64/u64 keep the double they decode to today, so they still round past 2^53. Retyping them would retype message.id, a u64 primary key the cache keys rows on, and that is #35's cross-cutting call rather than this function's. The mechanism is in place if #35 wants it. Deliberately a second entry point instead of wiring into decode_row. Coercing every column on the shared path would change the type of values consumers already receive -- an identity column would stop being a table -- and that is a frozen-surface break, so it is opt-in and decode_row keeps its contract. Some(None) collapses into None, because a Lua table field holding nil IS absent. Rather than reason about whether that matters, the module now declares an Option<Option<i32>> column: a JS module writing null into it emits the OUTER none, captured and asserted, so the distinction is one nothing on this path can even produce. Also corrects a claim #40 stamped into three files. It said reasons 1 and 2 for column order being load-bearing were dependencies of a typed decoder that does not exist, and that nothing under stdb/ reads `types` or `typespace`. True when written; false now that the decoder reads both.52e032154ccd52f3ce52Review — automated reviewer, posted for the record
Verdict: APPROVE with should-fixes. No blockers. Closes #26 — every acceptance criterion met. All five claimed wire facts reproduce on an independently published server; quote_wide_ints survived every attack I could construct; the compatibility claim is byte-exact; native conformance passes.
Scope: head
cd52f3c, merge-base = origin/main6e37098(rebase confirmed, unlike PR #45). Own server: STDB_PORT=3900, STDB_STATE_DIR=/tmp/stdb-review46b, stopped afterwards. Port 3000 untouched. Repo tree left clean.wire facts
"bytes":"0001feff", positional"0001feff", empty row""in both. Bonus corroboration at envelope level: OneOffQueryResponse"message_id":"01020304"— a Vec is hex text there too."-0x8000000000000000000000000000000000000000000000000000000000000000"(sign-prefixed-0x); u256::MAX = 64 f's; zero ="0x0"— NOT padded. The minimal-width claim is carried entirely by the0x0case and it holds."opt_int":[0,7]/[1,{}]; positional[0,7]/[1,[]]. The variant-NAME form appears exactly where the PR says: schema endpoint ({"some":"Interval"},{"some":"nickname"}) and message envelopes (OneOffQueryResponse"error":{"none":[]}). Never in a row value. protocol.lua:27-34 header correction is accurate."sched":[0,{"__time_duration_micros__":1500000}]/[1,{...}]). Schema endpoint variant-name lists: ScheduleAt = ["Interval","Time"] capitalised; every module enum lowercased — Status ["idle","busy"], Kind ["nothing","tagged","pointy"], Option ["some","none"]. Ordinal order also confirms Interval=0, matching protocol.lua:912-922 and theScheduleAt.interval(1500000n)-> [0,[1500000]] capture.-170141183460469231731687303715884105728and340282366920938463463374607431768211455as bare unquoted JSON numbers. Alsoconn(39 digits) anduid(37 digits) bare.quote wide ints
"\u0001-1234567890123456"1234567890123456.5);1e20,1.5e-9,-2.5E+30,0.1234567890123456all untouched — the literal is consumed whole andlit:match('^%-?(%d+)$')rejects anything with./e["12345678901234567890"]){"1234567890123456":1}, and{"k\"1234567890123456":...})\", empty string then a wide int — all correct\u00e9) and raw non-ASCII UTF-8 bytes in a string column untoucheddecode_value's NUMERIC branch doestonumber(digits). That is IDENTICAL BY CONSTRUCTION, not merely empirically: rxi json.lua:262 parses numbers with the sametonumber(s)on the same substring. Verified for u64::MAX, 2^53, 2^53+1, 2^64, 16/15/18-digit values and the captured 16-digit timestamp: all bitwise equal to plain json.decode, underu64,i64andunknowntags. A u64 column is NOT retyped."(__[%a_]+__)":(%d+)cannot match inside a double-encoded row string because the CLOSING quote is escaped there (\"__connection_id__\":), so a row ByteString exits decode_server unmodified — verified byte-identically on a real captured frame in replay.lua. Double-quoting is impossible.compatibility
scalar(:194-212) untouched — the rebaser's payload_of collapse claim holds; the only call site is the new MAGIC branch at :904. cache.lua and schema.lua untouched, so #44 is intact.message.id(a wide u64) and decode_row still quotes connection_id while decode_typed returns the exact decimal — both paths verified on the same input text.frozen contracts
row.balance + 1silently yields a rounded double with NO error and NO warning. And string comparison is not ordering — I measured"9007199254740993" < "10000000000000000"== false, i.e. sorting a u128 column lexicographically is silently wrong; unpadded digits plus-prefixes make i128 worse ("-1701411834..." < "-7"== true). Equality IS safe (the representation is canonical — either wire digits with no leading zeros, or %.0f). The README's illustrative column is literallyrow.balance(README.md:572), the one case where you most want to add. SHOULD-FIX (docs): state that arithmetic coerces-and-rounds silently and that<is not numeric ordering."0x0"next to a 64-digit value), and the code correctly never assumes one.message.id, which cache.lua keys rows on, and that is #35's call not this PR's; the mechanism is genuinely one line away. The PR body says u64::MAX "round-trips to …211456" — it does not. The actual assertion is test_fixtures.lua:481-482,u64::MAX -> 18446744073709551616.…211456isconnection_id_to_hex("340282366920938463463374607431768211456")at test_protocol.lua:250, an unrelated pre-existing u128-overflow guard. Sharper risk worth documenting: u64::MAX rounds to 2^64, which is OUT of u64 range and therefore detectably bogus — but i64::MIN+1 (-9223372036854775807) rounds to-9223372036854775808, which IS a valid i64 and therefore undetectable by any range check. Pinning the lossy value is the right thing to freeze (it converts a silent surprise into a documented one), but the i64 case is the one to name.opt_opt: null(anOption<Option<i32>>column) returns[1,{}]named /[1,[]]positional — the OUTER none — in both rows and all three encodings. The empirical settlement is right and the negative result is the answer #26 wanted. The escape hatch is ADEQUATE but under-specified:[0, [1,{}]]vs[1,{}]is distinguishable at element 1, so the hatch works — but neither protocol.lua:932-934 nor README.md:624 names HOW to get the raw column (decode_row(encoded, columns)orjson.decode(encoded)on the same ByteString), and afterdecode_typedthe raw column is gone. NIT.decoder correctness
def.variants[at + 1]with a 0-based wire ordinal and a 1-based Lua array — correct. Verified live: ordinal 0 -> "idle"/"nothing", 1 -> "busy"/"tagged", 2 -> "pointy". Mutating todef.variants[at]fails 2 test files.sum_Missingand a nil typespace both pass the value through. Correct.fieldsorder drivesv[i]; positional detection isv[1] ~= nilnot truthiness, so a product whose first field isfalsestill reads positionally — verified with a bool-first product, and mutating tolocal positional = v[1]fails a test file. (My own harness hit exactly thisand/ortrap, which is a good sign the author anticipated it.)unknown/nil/resultreturn exactly what json.decode produced, including inside nested tables.opt_vec_opt_stringcorrect in both:[0,[[0,"a"],[1,{}],[0,"c"]]]and the[1,[]]positional variant both give {"a", nil, "c"} with the inner none collapsed to nil.fixture recapture
updatesentries. PR #44's regression citation still holds.["message"](233 bytes) and["profile"](509 bytes) blocks are BYTE-IDENTICAL to origin/main.sanity
Findings
F1 — should-fix
stdb/protocol.lua:713-715 (comment), :720-721 (SENTINEL), :799-811 (unrescue), :815-820 (rescued_digits)Problem: The SENTINEL is FORGEABLE and the comment asserting otherwise is wrong. protocol.lua:713-715 says U+0001 "is not producible by any value this SDK sends and would need an explicit \u0001 escape to arrive from a server, so a marked string is this pass's own work and never a server payload." I wrote
\u0001+ 18 nines into a string column on a live 2.8.2 server and it round-tripped VERBATIM in both encodings (capture: /tmp/cap46/sentinel.jsonl —"text":"\u0001999999999999999999"in the TransactionUpdate and in the SubscribeApplied readback). rxi json decodes\u0001to the raw byte, so the mark is indistinguishable from a rescue. Measured corruption, all reachable: tagunknown-> the string becomes the number 1e+18; nil tag -> 1e+18;vec_unknownelement -> 1e+18;unknownover a nested table -> the whole tree is walked and the leaf converted;u128/i128-> the mark is stripped and the remaining digits returned as if exact. Also a nil hole: a string that is exactly U+0001 returns nil under a numeric tag (the column silently vanishes) and "" under u128/unknown.string,bool,i256,u256are SAFE (they return v raw), and gen-schema.sh always emitstypesfor sums/products, so realistic reachability is narrow: gen-schema.sh's fourunknownsites (:188, :209, :217, :219 — an anonymous inline sum, or an anonymous product with >=2 elements), theresulttag, and direct calls to the public frozendecode_value. Narrow, but the file's own standard is that a confident-wrong comment is the defect.Fix: The mark must be unforgeable, not merely unlikely. The scanner already steps over string literals, so give it three more lines: when copying a string literal, if it begins with the sentinel, DOUBLE the sentinel; then have unrescue/rescued_digits treat a doubled mark as a literal single one. Deterministic, no nonce, and it makes "only this function produces it" true. At minimum, correct the comment to say the mark is forgeable and name the tags at risk.
F2 — should-fix
stdb/protocol.lua:888-892Problem:
string.format("%.0f", v)FABRICATES exact-looking 128-bit digits on the un-rescued path. Measured:decode_value(json.decode('[340282366920938463463374607431768211455]')[1], "u128")returns "340282366920938463463374607431768211456" — 39 digits, off by one, indistinguishable from an exact value. Worse for connection_id: wire 106265176936864380835577448117692846105 -> "106265176936864376154898381920077348864", nine digits wrong. Unreachable through decode_typed (the rescue catches >=16 digits and <16 digits are exact), but reachable exactly where the docstring blesses it: protocol.lua:852-856 explicitly names "a procedure return through decode_payload, say", and decode_payload never rescues. So a u128-returning procedure hands the consumer fabricated digits. This is the same class the file's own SPECIAL TYPES header exists to prevent, and it contradicts the sibling accessor:connection_id_to_hexreturns NIL for a number past 2^53 rather than "a padded guess" (protocol.lua:380-381, asserted at test_protocol.lua:249). decode_value is FROZEN at v1.0.Fix: Match connection_id_to_hex: if
vis a number andv >= MAX_EXACT(the constant already exists at :317), return nil rather than a %.0f fabrication. Keep %.0f only below 2^53, where it is exact.F3 — should-fix
stdb/protocol.lua:709-711 (WIDE_DIGITS), dev/test/test_fixtures.luaProblem: The WIDE_DIGITS boundary is UNTESTED — the only mutation of nine that the suite did not catch. Setting
WIDE_DIGITS = 17leaves all 5 test files passing, yet it silently breaks exactly what the constant exists for: a 16-digit 128-bit value above 2^53. Measured with the mutant: u128 9007199254740993 -> "9007199254740992", 9999999999999999 -> "10000000000000000". Both are wrong AND exact-looking. The fixture's 128-bit values are 0 and 39/40 digits, so nothing pins the 2^53 crossing. The comment at :709-711 reasons the constant out correctly; nothing defends it.Fix: One assertion in test_fixtures.lua or test_protocol.lua: decode_typed a u128 of 9007199254740993 (2^53+1, 16 digits) and assert the exact string. Hand-written input is fine — no recapture needed, and it makes WIDE_DIGITS load-bearing under mutation.
F4 — should-fix
README.md:586-602 (the per-tag table), :607-614Problem: The 128-bit decimal-string contract is frozen with no warning about what a consumer will actually do with it. Lua 5.1 coerces numeric strings, so
row.balance + 1silently produces a rounded double with no error — the string preserves the value on arrival and then loses it on first use, invisibly. And<on these strings is not numeric ordering: I measured"9007199254740993" < "10000000000000000"== false, and i128's-prefix makes it worse. Sorting or range-filtering a 128-bit column is silently wrong. The README's own illustrative column isrow.balance(:572), the single case where a consumer most wants arithmetic.Fix: Two sentences in the 128-bit paragraph: arithmetic on these strings coerces to a double and rounds silently, and
</>are lexicographic, not numeric — compare with==only. Optionally pick a non-arithmetic example column instead ofbalance.N1 — nit
README.md:600Problem: Unescaped
|inside a code span inside a markdown table:| \schedule_at` | `{ tag = "Interval" | "Time", value = }` |`. GFM does not protect pipes inside code spans in tables, so this row renders as four cells and garbles.Fix: Escape as
\|.N2 — nit
PR body, "Three representation decisions"Problem: "Pinned by an assertion (
u64::MAXround-trips to…211456)" misattributes the assertion. The real pin is test_fixtures.lua:481-482,u64::MAX -> 18446744073709551616.…211456is connection_id_to_hex's 2^128 overflow guard at test_protocol.lua:250, unrelated and pre-existing. The code and tests are right; only the body is wrong — but the body is what the next reader trusts.Fix: Correct the body to the asserted value.
N3 — nit
stdb/protocol.lua:932-934, README.md:624-625Problem: The Some(None) escape hatch says "read element 1 of the raw column" but never names how to obtain the raw column, and decode_typed has already discarded it by the time the consumer holds a row.
Fix: Name it:
protocol.decode_row(encoded, spec.columns)(or json.decode on the same ByteString) on the same encoded row.N4 — nit
stdb/protocol.lua:912-924 vs :251-280Problem: Two ScheduleAt resolvers in one file disagree on input shapes and neither cross-references the other.
schedule_at_to_secondsaccepts the named{Interval=v}form (documented at :255-259 as kept for schema JSON);decode_value's schedule_at branch accepts ordinals only and returns the raw table for{Interval=...}(measured). Defensible — rows are always ordinal — but a reader who found one will assume the other.Fix: One sentence in the decode_value branch noting the named form is deliberately not accepted here and where it is.
N5 — nit
stdb/protocol.lua:896-900Problem:
unitreturns nil, which is right for a variant payload but means aunit-tagged COLUMN silently vanishes from the decoded row. Documented in the README table (unit-> nil) so not a surprise, just worth a word in the code comment, which currently only reasons about the variant case.Fix: Extend the comment to say a unit column is therefore absent, not empty.
Closes #26
YES
typesandtypespace; test_fixtures.lua:601-611 cross-checks all 31 columns in both encodings and asserts no column got anunknowntag.{tag="busy"}/{tag="pointy", value={x=5,y=-6}}. Note the issue's example spells the variant capitalised ("Ready"); the server lowercases module variants, so lowercase is correct, not a miss.Unmet: None. F1-F4 are defects in a deliverable that meets the criteria, not missing criteria.
cd52f3ce523156a2d8113156a2d811be6336f741be6336f7418abfba8475