feat: noncanonical fixtures (PBNode field order) - #212
Merged
Conversation
New noncanonical-fixtures category for blocks that decode successfully but are not in canonical form, so they cannot round-trip byte-for-byte: runners assert decoding succeeds, matches the canonical decode, and canonically re-encodes to canonicalCid. First case: the dag-pb Data-before-Links order from IPIP-550 (readers accept both orders, links-first stays canonical), paired with positive fixture dagpb_1namedlink+data holding the canonical twin. - all four runners (js, go, rust, python) learn the category - js/test.js: drop leftover describe.only that skipped the whole positive suite in CI Refs ipfs/specs#550, ipld/ipld#383
Prevents text diffs and line-ending normalization (e.g. on Windows checkouts) from corrupting byte-exact fixture payloads whose CIDs must match their content.
lidel
marked this pull request as ready for review
September 1, 2026 16:15
IPIP-550 no longer defines a profile; the field order is a low-level opt-in.
Member
|
seems fine to me |
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
3 participants
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.
Problem
Review of ipld/ipld#383 asked for codec-fixtures coverage so implementations can verify they decode the opt-in
Data-first PBNode order from IPIP-550. But positive fixtures here must round-trip byte-for-byte, and every encoder is canonical (Links-first), so aData-first block can never live infixtures/.Fix
noncanonical-fixtures/<codec>/decode/*.jsoncategory, mirroring negative-fixtures: decoding must succeed, match the canonical decode, and canonically re-encode tocanonicalCidData-before-Linksblock from IPIP-550, paired with a new positive fixturedagpb_1namedlink+dataholding the canonical twin of the same logical node (the block from feat: add data-first dag-pb block #211, under its canonical name)// TODOs note these can become full round-trip fixtures once codecs expose opt-in encoders like feat: allow encoding Data fields before Links in PBNode messages js-dag-pb#111describe.onlyinjs/test.jsthat was skipping the entire positive suite in CI (JS leg now runs 219 tests, all green, as are Go, Rust, and Python)cc @achingbrain as #211 may not be enough to be meaningful (protect from read-capability regressions)
@vmx does this look like a sensible way to wire up this style of fixture, so read-interop stays covered even while writes remain canonical (legacy behavior, no breaking changes, maximized interop of reads)?