Repository navigation
CI rehearsal only: p036-rfc3339-expires-vectors at aed26381 (do not merge) - #1
Open
michael-moffett wants to merge 16 commits into
Open
michael-moffett wants to merge 16 commits into
michael-moffett wants to merge 16 commits into
Conversation
…ss loader Issue solana-foundation#111 asks for RFC 3339 conformance coverage on the MPP `expires` field. This repo has no vector corpus for it: the canonical files in harness/vectors/mpp-protocol cover challenge, header, receipt, base64url and challenge.id, but nothing exercises the date-time grammar that `expires` is specified in. This commit adds that corpus and the loader that reads it. expires-rfc3339-corpus.json carries 228 scenarios, 116 of them for date-time, 74 for full-date and 38 for full-time, merged from four upstream sources. Every scenario records a provenance object naming where the input came from and what authority decided its verdict, so a reviewer can tell an input imported verbatim from a suite that already ships the answer apart from one whose verdict was derived here against the grammar in RFC 3339. Scenarios also carry `applies_to`, which is load-bearing rather than decorative: a verdict only means what it says inside its own production, and MPP `expires` holds a date-time, so a consumer that wants `expires` behaviour has to filter on that field instead of reading the file as one flat list. The corpus arrives under two upstream licences, added beside it as LICENSE.json-schema-test-suite, LICENSE.go-stdlib and PATENTS.go-stdlib. The Go standard library grant is split across a LICENSE and a PATENTS file and shipping one without the other would be an incomplete grant, so both are included. Both README files in the affected directories are updated to say which files came from which upstream, because the vector directory previously had a single provenance and now has two. vectors.ts gains loadExpiresRfc3339() and collectExpiresCases() on the same shape as the loaders already there, plus an `expires.parse` operation for the corpus to hang from. That operation is not in upstream conformance/operations.json, which is not vendored here, so it is proposed rather than asserted; the divergence is called out in a comment at the point of definition and the name is trivially changeable if you would rather it land upstream or read differently.
The corpus added in the previous commit is inert until something reads it. This commit points seven SDK trees at it, covering Go, Lua, PHP, Python, Ruby, Rust and TypeScript, so every tree asserts the same accept and reject verdicts against the same vectors. A divergence between two SDKs then shows up as a failing test in exactly one of them rather than as silence. Each test appends to the file in that tree which already covers the neighbouring behaviour, so it runs under the suite runner the tree already uses and no new entrypoint, fixture directory or dependency is introduced. Each drives that tree's own shipped parsing path rather than a bare library call, since the question is what the SDK does with an untrusted `expires` string, not what the language's date library does with it. Every tree selects the `applies_to == "date-time"` slice of the corpus before asserting, because that is the production MPP `expires` is specified in; `1963-06-19` is a valid RFC 3339 full-date and no date-time parser should accept it. Selection is on the first-class `applies_to` field rather than on a name prefix or a description string, and each tree also guards the filter itself, so a regression that silently empties or widens the slice fails the suite instead of passing vacuously. On divergence a test reports the offending input, the expected verdict and the scenario name, so a failure names the string that broke rather than only a count.
The comment above collectExpiresCases() in harness/src/protocol/vectors.ts said that per-language `expires` tests call the collector directly, and harness/src/protocol/README.md repeated the claim. That is not what the code does. Nothing calls the collector: each SDK test loads expires-rfc3339-corpus.json itself and applies the same `applies_to == "date-time"` filter independently, which is why the collector appears nowhere outside the harness. Both places now say so, and say that the collector is the TypeScript side of the same contract, kept for the day an adapter implements `expires.parse`. The surrounding rationale is unchanged and still holds: the cases stay out of collectProtocolCases() because folding them in would add 116 red cases to a green suite, and absorbing them later is still a one-line spread. That comment could go wrong unnoticed because nothing exercised the collector. Its `applies_to` filter and its accept/reject mapping could both regress without a test going red. test/conformance.test.ts gains a block that drives collectExpiresCases() directly and asserts that it returns exactly the date-time scenarios and strictly fewer than all of them, that every case carries the `expires.parse` op, and that a scenario whose parse expectation is an error yields expectSuccess false with that scenario's own error type while a success scenario yields expectSuccess true. Every count is derived from the corpus at runtime, from the scenario list and from the corpus's own counts block, so a corpus edit moves the expectation along with it. The block reads the vendored corpus and nothing else: it spawns no runner and contributes no case to the driver in that file, so the cross-SDK run is unchanged. It lives there rather than in a new file because that is the harness test file the workflows in this repo actually execute. The corpus is byte-for-byte untouched.
The corpus added in this branch's first commit shipped as
expires-rfc3339-corpus.json in a shape of its own: 228 scenarios spanning
three RFC 3339 productions, a provenance object on every row, and an
`applies_to` field that each consumer had to filter on before the
verdicts meant anything. That is not how the five files beside it read,
and a vector directory whose files do not share a shape is one a reader
has to learn twice.
This commit replaces it with expires.json, in the shape the canonical
files already use: { version, spec_ref, description, commands, scenarios }
at the top level, { name, description, tags, input, tests } per row, and
the same verdict encoding those files use:
"tests": { "parse": true }
"tests": { "parse": { "success": false, "error_type": "parse_error" } }
The file is 124 scenarios, 54 ACCEPT and 70 REJECT, and every one of them
is an `expires` verdict. Because the file holds nothing else, `applies_to`
is gone and the filtering step with it: there is no slice to select and no
scenario a consumer should skip, so there is no longer a way to read the
file as a flat list and get a wrong answer. Eight of the rejections are
well-formed full-date and full-time strings, kept because a parser that
accepts `1963-06-19` for `expires` is the bug solana-foundation#111 is about; the other
wrong-production rows are excluded rather than shipped and skipped, since
eleven month-length variants are one failure shape to an `expires` parser
and not eleven.
No `object` golden is supplied for an ACCEPT. That is deliberate and the
README says why: solana-foundation#111 is a verdict-alignment problem rather than an
instant-normalisation one, and leap seconds have no representable instant
in most date libraries.
Per-row provenance moves to the directory README, which now records the
three upstreams, their pinned commits, their licences, the counts per
provenance class, the dedupe rule and the rows it dropped. Recording it
once there rather than on every row is what lets the vector file read
like its neighbours.
The harness loader and the seven SDK tests follow the file to its new
path and shape and drop the filter they no longer need. No verdict
changed in this re-encode.
The vectors this branch adds carry verdicts and the seven SDK tests assert them, but what each tree actually does with an `expires` string was written down nowhere. A reader who wanted to know which parse paths diverge, and on which inputs, had to run the suites and work it out. This commit records that result under docs/, beside mpp-protocol-conformance-results.md, where the equivalent result for the rest of the protocol already lives. The document drives all eight `expires` parse paths, seven trees with two call sites in one of them, across the 124 date-time scenarios in expires.json: 992 cells, of which 120 diverge from the corpus. It names the parse path per tree by file and line, the runtime each number was measured on, and the count that tree failed, then nine findings, each with the input that produced it, the verdicts the trees returned and the RFC 3339 section the expected verdict derives from. Several of those findings are behaviours no single accept-or-reject vector can express: an offsetless date-time whose instant depends on the host, a path that evaluates ECMA-262 date-time strings rather than RFC 3339 ones, an empty `expires` that returns not-expired without being parsed, and two trees that accept the same input and denote different instants. One vector is separated out because RFC 3339 does not decide it, so no tree can be called wrong about it. A limitations section states what was not measured, and a reproduce section gives the command per tree with its prerequisites and expected exit status, so every number here can be rerun rather than taken. The vectors README ended its provenance section by pointing at a file that is not in this repository. It now points at this document.
Two files this branch added were committed without the repo's formatters having been run over them, and CI is red on both checks. typescript/packages/mpp/src/__tests__/client-charge-validation.test.ts fails `pnpm format:check`: the corpus URL constant was hand-wrapped across four lines where prettier's print width puts it on one. rust/crates/kit/src/mpp/protocol/core/challenge.rs fails `cargo fmt --check` with four diffs, at the corpus read, the description field extraction, the non-empty assertion and the Rfc3339 parse call. That crate's 842 library tests pass; the job is red on formatting alone. Both fixes are the formatter's own output, produced by `prettier --write` on the one file and `cargo fmt` from rust/, not by hand-editing toward what the formatter was expected to want. No other file in the tree was reformatted. Nothing in either file changed except layout. Stripping all whitespace from both versions leaves, in the TypeScript file, one removed trailing comma and nothing else; and in the Rust file, one removed trailing comma plus the braces rustfmt wraps a closure body in when it moves that body onto its own line. No identifier, literal, operator or call site differs. The Rust library suite is 842 passed, 0 failed, before and after. The prettier failure is the one named in review. The cargo fmt failure is the same defect from the same cause in the other tree, so it is fixed here rather than left for a second round.
Finding 2 claimed every one of the 22 typescript divergences was an over-acceptance and that not one was a parse failure. Counted over the 124 scenarios in harness/vectors/mpp-protocol/expires.json with the same two expressions the call sites evaluate, the split is 16 over-acceptances and 6 the other way. The 6 are the leap-second date-times -- jsts_date_time_005, jsts_date_time_006, leap_second_june_month_end, leap_second_offset_rolls_local_date_forward, rfc_5_8_example_leap_second_offset, rfc_5_8_example_leap_second_z -- which the corpus accepts under RFC 3339 section 5.7 and which V8 rejects, having no representable 61st second. The document already carried the correction elsewhere and contradicted itself: finding 6's table records leap_second_offset_rolls_local_date_forward as corpus ACCEPT / typescript REJECT, and finding 6's prose states that go, python and typescript reject every :60. Finding 2 now agrees with both. Docs only. The corpus, the vectors and every test are unchanged.
…ile the php carry F1: the Reproduce block's only typescript command drives parseRfc3339 under solana-foundation#286 and returns 6, while the headline rows are the merged tree's 22 each, so the sentence binding the two was false for 44 of 120 cells. Adds the direct-evaluation command that prints the 22, rather than only disclosing the gap: no harness adapter implements expires.parse. F2-F4: php was not re-executed this pass. Three statements had lost the clauses that said so - exit 127 read as a correct result, the command comment claimed exit 1 against a delimiter block recording 127, and the prerequisites table asserted a probe that was never run.
IsExpired treats an empty expires as no expiry, so the runner scored empty_string as ACCEPT without parsing it. The verdict now comes from time.Parse(time.RFC3339, ...), as in the other runners; empty_string passes and go reports 15 of 124.
solana-foundation#315 (python) and solana-foundation#320 (php) reject year 0 on purpose. year_0001 is the smallest year every SDK represents, so the year-boundary case stays covered and the corpus stays at 124 scenarios.
Every suite re-run on this branch: python 8, go 15, php 16 (now executed, no longer carried); total 117 of 992. New corpus hash 613fdb3f.
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.
Fork-internal pull request opened to fire the repository workflows on this branch. Not a contribution and not for merge.
Head is
aed2638147a957f422835361c7d3a0f5639e1dd8, the same commit as the head of solana-foundation#282. Base is16777d465f4446c7ac519378663d6d4bbf394877, level with upstreammain.Purpose is to observe workflow conclusions on infrastructure we control, because the upstream runs have been held at
action_requiredpending first-contribution approval since 2026-08-12.Do not merge. Merging moves this fork's
mainoff16777d46and breaks base parity with upstream.