feat(air-gap): add @bsv/air-gap one-directional optical transport v0.1.1 (wire v1, BRC-141) - #404
Conversation
02b4747 to
f4d92b6
Compare
Adds a zero-dependency, payload-agnostic one-directional optical air-gap
transport: arbitrary bytes in, a deterministic endless sequence of
fountain-coded wire parts out, reassembled from a camera feed with no
back-channel.
Wire format: `air-gap:` + unpadded base64url of a 14-byte big-endian header
(seq u32, K u16, msgLen u32, crc32 u32) and exactly one fixed-size block. The
first K parts are the source blocks verbatim, so one clean camera cycle decodes
with zero overhead; later parts are interchangeable XOR mixes chosen by an
xorshift32 RNG seeded from seq with an ideal-soliton degree, so any K + epsilon
distinct parts reconstruct the message and a missed frame costs almost nothing.
The coding is ported bit-for-bit from bsv-browser's `bsvpayf2:` fountain so a
later migration and any cross-language port stay interoperable; the frozen part
strings in tests/vectors.test.ts are the conformance contract. The legacy
prefix is not accepted.
Decoder guarantees, because the input is a camera:
- accept() never throws; stray reads are ordinary { ok: false } no-ops
- no partial or unverified bytes ever escape; message() is CRC32-gated
- a CRC mismatch discards the assembly and self-resets, so a looping sender
refills it without the application managing a retry
- the session block size is pinned by the first accepted part, which is what
stops two senders, or one padded frame, from being assembled together
Encoder guards empty and oversize messages, non-integer block sizes, block
counts past the u16 K field, and sequence numbers outside u32.
Out of scope by design, and deliberately absent: QR rendering, camera capture,
display cadence (no frame-interval constant is exported), payment or BRC-100
schemas, compression, encryption.
Tests: 100 unit and vector cases plus a fast-check property suite over the wire
boundary; 100% statement, branch and line coverage of src; 89% mutation score
against an 85 floor.
Governance: registered in repository-health, package-release-notes, test-quality
property suites, mutation targets and npm supply chain; package count baselines
and the docs package page and indexes updated to 31 public packages.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Wave 39 (#403) landed after this branch was cut and now requires a generated root-policy pointer at every governed project, so add packages/helpers/air-gap/AGENTS.md and bump the scoped-pointer count to 44. Also clears the ten new SonarCloud findings the zero-findings gate reported: - S8786 in src/base64url.ts and tests/helpers.ts — `=+$` backtracks super-linearly on a long run of '='. btoa pads to a multiple of four, so a bounded `={0,2}$` strips the same padding in linear time. - S7749 in src/constants.ts — uneven numeric separator groups in `0x1_0000_0000`; `2 ** 32` states the u32 bound directly. - S5906 across four test files — `toHaveLength` reports better than comparing a raw `.length`. No behavior change: the frozen conformance vectors and all 100 tests pass unchanged, src coverage stays at 100%, and the mutation score is 89.77%. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adding the first package whose coverage comes only from the "Coverage / other affected packages" lane exposed two gaps that made the 90% patch-coverage gate unsatisfiable for it. 1. That lane flattened report paths into `packages_helpers_air-gap_coverage_lcov.info`, but both the aggregate lane's Codecov glob and scripts/patch-coverage.mjs collect `*.lcov.info` — a dot, not an underscore. Every coverage-other package's LCOV was therefore invisible to the gate and never uploaded to Codecov. The artifact is now named `packages_helpers_air-gap.lcov.info`. 2. patch-coverage.mjs counted `jest.config.cjs` and similar as governed production source. Configuration is never instrumented, so a package that adds or edits a test config could never clear "changed production files absent from LCOV". Config files are now excluded, with a regression test. Verified locally against origin/main: the gate reports 100.00% (276/276 changed line/branch points). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
3a7d9a0 to
8ada948
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
ty-everett
left a comment
ty-everett
left a comment
There was a problem hiding this comment.
Deggen, this is a strong package contribution, and it already incorporates a lot of the repository doctrine we just established: the package-local AGENTS.md is the correct root pointer, the docs/release evidence and governance inventory are present, strict lint/type/build/artifact checks pass, the property/coverage/mutation work is substantial, and the implementation is admirably small and dependency-free.
I am requesting changes because this PR also creates a new public wire contract at an untrusted camera and offline-wallet boundary. A few details that look small in TypeScript become permanent interoperability or resource-safety problems once another implementation ships. The inline comments identify the must-fix code paths: cross-language u32 seed divergence, a degree sampler that is not the documented ideal-soliton distribution, an absolute recovery guarantee that LT coding cannot provide, missing wire version/session identity, unbounded decoder work/memory, and a Node-only governance declaration for a browser-facing package.
There are four cross-cutting items to address with those changes:
packages/helpers/air-gap/tests/decoder.test.tscontains a literal NUL byte at line 128. GitHub consequently treats the 315-line test file as binary and hides its patch from review tooling. Please express that hostile input without embedding byte 0 in the source (for example, construct it withString.fromCodePoint(0)) so the file remains ordinary reviewable UTF-8.- Base64url contains lowercase letters and
_, so these strings use QR byte mode, not QR alphanumeric mode. Please correctbase64url.ts,helpers.ts,constants.ts, and the package/docs capacity language, then validate the documented default against byte-mode capacity (ideally with a real render/scan sizing fixture). - This is a portable protocol, but its normative definition and vectors currently live only in package prose/Jest. Add a versioned specification under
specs/and implementation-neutral vectors under the rootconformance/corpus (including boundary/high-sequence vectors), with the package tests consuming the same fixtures. Until the protocol and cross-implementation behavior are settled, the docs should sayexperimental, notstable. - Please keep
@bsv/air-gappayload-agnostic, but design its versioning and conformance with the adjacent systems in view. PiWalletSV PR #1 now has versioned gzip+CBOR envelopes withstateSync,stateReceipt, state revision/hash, proposal IDs, and Atomic BEEF over its indexedPW1transport. Vault Manager has its ownCHUNKtransport plus an Atomic-BEEF/coin-state and audit model. The clean convergence boundary is: this package owns a shared, versioned optical transport; a separate common wallet-state envelope owns capabilities, network/wallet identity, Atomic BEEF, revision/hash, proposal/idempotency and receipt/replay semantics. ExistingPW1,CHUNK, andbsvpayf2should get explicit adapters/migration paths rather than silent multi-format decoding. Please capture that coordination as a linked design/tracking item and avoid calling the transport stable until shared fixtures can exercise at least one independent implementation.
For orientation to the new contribution contract: the root AGENTS.md, CONTRIBUTING.md, and SECURITY.md are authoritative; package files should only point there. Runtime/criticality claims in the repository inventory must match docs and exact packed-consumer evidence. Protocol work belongs in specs/ plus root conformance fixtures. Untrusted parsers must be resource-bounded. Docs, changelog/migration notes, and tests change with the code. Before the next review handoff, run the strict local equivalents and then wait for every check on the exact pushed head; new Sonar or CodeQL findings are blockers, never waivable.
Validation on commit 8ada94853cf4657ce8d7fcd8b806b254850c3e22: frozen install; package typecheck, zero-warning lint, formatting, build, property tests, 100-test coverage suite (100% src coverage), exact-tarball pack/consumer checks, repository health contract, and local mutation testing (89.77%) all pass. Every GitHub check is terminal green as well, including the repository-owned 90% patch gate, Sonar zero-new-findings gate, CodeQL (zero open PR alerts), dependency review, browser/mobile lanes, and the 8m13s air-gap mutation lane. So the request is about correctness and durable protocol quality rather than unresolved CI.
| * @param k - Source block count. | ||
| */ | ||
| export function blocksForPart(seq: number, k: number): number[] { | ||
| const rng = makeRng((seq * 0x9e3779b1) >>> 0) |
There was a problem hiding this comment.
[P1] Preserve the intended u32 multiplication across languages. JavaScript Number multiplication begins losing low bits once seq * 0x9e3779b1 exceeds 2^53 (from about seq 3,393,265). For example, at seq = 0x7fffffff this expression seeds 3,788,015,616, while 32-bit modular multiplication (Math.imul) yields 3,788,015,183; a normal Go/Rust/Python u32 port will therefore select different blocks. Use Math.imul(seq, 0x9e3779b1) >>> 0 (or an equivalently specified operation) and add frozen vectors around the precision threshold plus 0x7fffffff and 0xffffffff before this mapping is declared immutable.
| let degree: number | ||
| if (k === 1) degree = 1 | ||
| else if (open01() <= 1 / k) degree = 1 | ||
| else degree = Math.min(k, Math.ceil(1 / open01())) |
There was a problem hiding this comment.
[P1] Sample the distribution that the protocol claims. The degree-1 test consumes one uniform draw and the non-1 branch consumes a second independent draw, so the final probabilities are not ideal soliton. With K=5, for example, degree 5 occurs about 20% here instead of the ideal 5%. Because this choice is wire format, settle an explicitly normalized distribution (ideal soliton or another justified/tested choice), sample it deterministically, quantify recovery overhead/loss percentiles, and freeze language-neutral vectors; preserving the current accidental distribution while calling it ideal would make every future port reproduce the bug.
| if (typeof text !== 'string' || !text.startsWith(AIR_GAP_PREFIX)) return this.rejected() | ||
| let bytes: Uint8Array | ||
| try { | ||
| bytes = fromB64url(text.slice(AIR_GAP_PREFIX.length)) |
There was a problem hiding this comment.
[P1] Bound the input before allocating it. A caller-controlled string is currently copied/replaced, decoded by atob, copied into a binary string, and copied again into a Uint8Array before any length ceiling is checked. Moreover, the shape test allows an arbitrarily large payload when K=1. Define a maximum encoded-part/block length, reject on text.length before base64 decoding, and enforce the same block ceiling in the encoder. Add adversarial boundary tests; MAX_MESSAGE_BYTES alone does not make this untrusted camera parser resource-safe.
| else if (payload.length !== this.blockBytes) return this.rejected() | ||
|
|
||
| if (this.seen.has(seq)) return this.accepted() | ||
| this.seen.add(seq) |
There was a problem hiding this comment.
[P1] Put finite bounds on per-session memory and work, and stop mutating a completed session. seen can grow across all unique u32 sequence values, pending retains a payload plus index set for every unsolved equation, and blocksForPart allocates an array of K entries where K may be 65,535. Even after done, additional unique frames keep growing seen. Establish practical K/seen/pending/work limits with defined reject/evict/reset semantics, return immediately once complete, and test adversarial streams so a camera callback cannot be used for memory or CPU exhaustion.
| // not talking about the same message shape. | ||
| if (Math.ceil(msgLen / payload.length) !== total) return this.rejected() | ||
|
|
||
| const key = `${total}:${msgLen}:${crc}` |
There was a problem hiding this comment.
[P1] Give the new wire protocol an explicit version and a real stream identity before freezing it. (K,msgLen,crc32) is a shape plus accidental-corruption checksum, not a collision-resistant session identifier; and automatically adopting any different well-formed tuple lets one foreign frame erase useful progress. Include a version/capability discriminator and a sufficiently strong message/session ID, retain CRC only for accidental integrity, and make session locking/switching explicit. Cover simultaneous senders, collisions, replay, and foreign-frame behavior now, while changing the header/prefix is still inexpensive.
| * | ||
| * `seq < blockCount` returns source block `seq` verbatim — the systematic | ||
| * prefix, so an unlucky-free receiver finishes in exactly `blockCount` reads. | ||
| * Past that, parts are XOR mixes and are interchangeable: any `blockCount + ε` |
There was a problem hiding this comment.
[P1] Replace this absolute recovery guarantee with a probabilistic one. Distinct LT equations can be linearly dependent: with K=3, the six distinct sequence values [4, 27, 38, 56, 63, 72] produced by this implementation all select block 0, so K+3 reads still leave progress at 1/3. Document that receivers keep scanning until recovery, add a deterministic stall-then-recovery regression plus measured recovery percentiles, and update every README/JSDoc occurrence. Also, seq is u32 and therefore finite, not “without bound.”
| "name": "@bsv/air-gap", | ||
| "owner": "ts-stack-maintainers", | ||
| "area": "helpers", | ||
| "profile": "node-library", |
There was a problem hiding this comment.
[P1] Declare and verify the runtime this package actually promises. Both READMEs say browser support, the implementation deliberately uses browser globals, and the primary use case is browser/mobile camera exchange, but node-library plus Node-only targets means the health contract does not require exact packed browser/bundler consumers or the corresponding bundle evidence. Use the browser-capable profile and consumer targets (browser-bundler, browser-esm, plus Node CJS/ESM), add exact-tarball browser tests/budget evidence, and reconsider Tier 1 given the signing/offline-wallet trust boundary. Under the new policy, inventory, documentation, and runtime evidence must agree.
Wire format: versioned 23-byte header (ver u8 = 1, 8-byte session identity, seq u32, K u16, msgLen u32, crc32 u32). The fountain seed is the exact u32 product Math.imul(seq, 0x9e3779b1) - float multiplication diverged from seq 3,393,265 - and the degree draw is a single exact-integer inverse-CDF sample of the ideal soliton distribution, replacing the accidental two-draw sampler. Both are pinned by conformance vectors at the precision boundary, 0x7fffffff and 0xffffffff. No bsvpayf2 bit-compatibility is claimed; the legacy coding's defects are corrected, not reproduced. Decoder: rejects oversize strings before base64 work, enforces the new MAX_BLOCK_BYTES = 2048 ceiling (QR v40-L byte mode) on both sides, locks onto the first session and switches only after 3 consecutive parts of one foreign session, acknowledges completed sessions without mutation, and bounds duplicate tracking (65,536 seqs) and buffered mixes (1,024 parts / 4,096 indices) with liveness preserved. All bounds have adversarial tests; coverage stays 100% statements/branches/lines on src. Recovery language is now probabilistic everywhere, with a pinned K=3 stall-then-recovery regression (seqs 4,27,38,56,63,72 all reduce to block 0) and measured percentiles in the spec. QR capacity language corrected to byte mode with an assertion test. The literal NUL byte is gone from decoder.test.ts so the diff renders as text. Spec and conformance: normative spec at specs/transport/air-gap-optical.md (experimental), BRC-141 revised in place in bsv-blockchain/BRCs, and 31 implementation-neutral vectors at conformance/vectors/transport/ executed by both the package suite and the TS conformance runner via a new transport dispatcher (corpus 74->75 files, 6650->6681 vectors). Convergence with PW1/CHUNK/bsvpayf2 is tracked in #408. Governance: browser-library profile with browser-bundler/browser-esm consumers, tier-1 criticality, test:browser lane with browser-budget.json and a browser-artifact-policy entry; version rolled to 0.1.1 with updated release notes, baselines and experimental docs status. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013VPatJS7hMnrmHJG9iMRQQ
sirdeggen
changed the title
|
Review addressed on head Inline P1s
Cross-cutting
Local gates on this head: 139 package tests green, 100% statements/branches/lines on Generated by Claude Code |
…operty Sonar flagged AirGapDecoder.accept at cognitive complexity 18 (limit 15) after the session-locking and budget logic landed; the session routing and peeling ingestion are now extracted into enterSession and ingest with identical behavior, keeping accept at a straight-line read. Coverage stays 100% statements/branches/lines on src. The frame-loss property let the sender's 4K-part loop resonate with the repeating keep/drop mask - CI's seed found a mask keeping exactly one tick in eight against an 8-part cycle, so the receiver saw one seq forever. The loop length is now the smallest prime above both 4K and the mask period, which by CRT guarantees every part is kept within mask.length * cycle ticks and makes the property deterministically terminating. The failing CI seed replays green, plus a 2,000-run soak. Jest now ignores .stryker-tmp so local runs cannot pick up mutation sandboxes. Local Stryker on air-gap-codec: 87.50% against the 85 floor (269 killed, 32 timeouts, 43 survivors of 344 mutants). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013VPatJS7hMnrmHJG9iMRQQ
…ks it conformance/runner/ts now depends on @bsv/air-gap as a devDependency so it can execute the shared transport vectors, but the 'Build TS runner dependencies' step only ever built @bsv/wallet-toolbox's dependency graph. air-gap isn't in that graph, so its dist/ (and therefore its type declarations) never existed when the runner's typecheck ran, failing with 'Cannot find module @bsv/air-gap'. Add it to the build filter. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013VPatJS7hMnrmHJG9iMRQQ
Empty commit to re-trigger CI so "Require complete dependency review evidence" evaluates the updated PR body (GitHub job re-runs reuse the original pull_request event payload, so a body-only edit does not take effect until a new run starts).
|
3 participants
What
Adds
@bsv/air-gapv0.1.1 atpackages/helpers/air-gap— a zero-dependency, payload-agnostic one-directional optical air-gap transport: arbitrary bytes in, a deterministic sequence of fountain-coded wire parts out, reassembled from a camera feed with no back-channel of any kind.This revision implements wire protocol v1 (BRC-141) and addresses every item in the change-requesting review: the wire format is now versioned with a real session identity, the fountain mapping is exactly specified in cross-language integer arithmetic, the decoder is resource-bounded against hostile input, recovery claims are honest and measured, the protocol has a normative spec plus implementation-neutral root conformance vectors, and the governance declaration matches the browser-facing runtime the package actually promises.
Wire format (v1)
ver ≠ 1, so the header can evolve without silent mis-decoding.SESSION_SWITCH_PARTS = 3consecutive parts of the same new session switch it.Math.imul(seq, 0x9e3779b1); the old float multiplication silently diverged fromseq = 3,393,265(at0x7fffffff: 3,788,015,616 vs the correct 3,788,015,183). Conformance vectors pin both sides of the boundary plus0xffffffff.d = ceil(2^23/(r+1)), overflow → 1) that matches ideal soliton to 4 decimal places and is bit-exact in any language. A frozen distribution test pins it.MAX_BLOCK_BYTES = 2048(sized to QR v40-L byte-mode capacity), enforced by encoder and decoder; scanned strings longer than any legal part (2,770 chars) are rejected before base64 decoding.bsvpayf2, PiWalletSVPW1, and Vault ManagerCHUNKare tracked in Air-gap optical transport (BRC-141): convergence plan for PiWalletSV PW1, Vault Manager CHUNK, and legacy bsvpayf2 #408.Decoder guarantees
accept()never throws; every stray read is an ordinary{ ok: false }no-opmessage()is CRC32-gated; a mismatch discards the assembly and self-resetsHonest recovery semantics
"Any K + ε distinct parts reconstruct the message" was false and is gone everywhere. Distinct parts can be linearly dependent — for K=3, parts
4, 27, 38, 56, 63, 72all reduce to block 0 (now a pinned stall-then-recovery regression and a conformance vector). Measured percentiles (400 deterministic trials/cell): repair-only late joiner completes at ≈1.4–1.5 K parts median, ≈3.8–4.6 K at p99; with a sender looping an 8K-wide window, ≈1.5 K median bounded by the next systematic pass. Docs now tell senders to loopseq(a finite u32, no longer "unbounded") and receivers to keep scanning.Spec, conformance, and coordination
specs/transport/air-gap-optical.md(Experimental), registered in the specs inventory and docs siteconformance/vectors/transport/air-gap-optical.json— 31 implementation-neutral vectors (encode incl. boundary seqs, decode, session lock/switch, stall regression, hostile rejects, part lengths, CRC check value). The package test suite and the cross-language runner (conformance/runner/ts, newtransportdispatcher) execute the same file; META/PARITY_MATRIX/baselines updated (74→75 files, 6,650→6,681 vectors, 6,439→6,470 passed)experimentaleverywhere (docs front-matter, spec, BRC) until an independent second implementation passes the shared vectorsGovernance and runtime truth
projects.json:browser-library, consumersbrowser-bundler, browser-esm, node-cjs, node-esm, runtime targetsbrowser, node, criticality tier-1 (signing/offline-wallet trust boundary)test:browserlane (exact-tarball Vite + esbuild consumers viacheck-browser-package.mjs),browser-budget.json(measured 8.9 kB raw / 3.1 kB gzip vite; budgets ≈2× measured), and agovernance/browser-artifact-policy.jsonentrypublicPackageVersionsupdated; docs page front-matterversion: 0.1.1,status: experimentaltests/decoder.test.tsno longer contains a literal NUL byte (String.fromCodePoint(0)), so GitHub renders the diff as textDependency evidence
@bsv/conformance-runner-tsgains one new devDependency,@bsv/air-gap: workspace:^, so the cross-language conformance runner can execute the same shared vector file as the package's own tests (the whole point of putting fixtures in rootconformance/, per repository policy). No third-party dependency is added anywhere;@bsv/air-gapis a first-party workspace package already part of this PR..github/workflows/conformance.yml's "Build TS runner dependencies" step is updated to also build@bsv/air-gap(previously scoped only to@bsv/wallet-toolbox's dependency graph), which is what makes the new devDependency'sdist/available to the runner's typecheck.@bsv/air-gapitself still ships zero runtime dependencies and zero peer dependencies (verified bypack:check).pnpm-lock.yamlwas regenerated by a plainpnpm installwith no--forceand no override edits. The new importer entry isconformance/runner/ts→@bsv/air-gap: workspace:^resolving tolink:../../../packages/helpers/air-gap— a workspace symlink, not a registry package, so there is no new entry in the resolved dependency graph and nothing to deduplicate.pnpm auditor CodeQL to flag; both were already clean on the prior revision's package changes, which are otherwise unchanged.pnpm --filter @bsv/conformance-runner-ts typecheck/lint/format:check/testall pass locally once@bsv/air-gapis built (confirmed after correcting the CI build-dependency filter above); the TS conformance runner reports 6,470 passed / 211 skipped across 75 vector files, including the newtransport.air-gap-optical.*vectors via the newtransportdispatcher.@bsv/conformance-runner-tsis a private, unpublished test harness ("private": true), so it has no consumer bundle.@bsv/air-gap's own published bundle impact is unchanged from this PR's earlier revision (≈8.9 kB raw / 3.1 kB gzip via Vite, verified bytest:browser).@bsv/air-gap@0.1.1only, a first release still pending publication. No other package's version, dependency range, or published surface changes;pnpm check-versionsconfirms all cross-package references are current.Verification
testtest:coveragesrctest:propertyair-gap-codec)test:browser/browser-budgetpack:checkhealth:checktest:governance/docs:facts:checktypecheck/lint/format:checkNotes for review
AirGapDecoder.accept(session routing and ingestion extracted intoenterSession/ingest, behavior identical); a genuinely flaky frame-loss property where the sender's loop could resonate with the keep/drop mask (loop length is now a prime above the mask period, making completion deterministic by CRT); and thets-runner/ dependency-governance failures addressed by this section and theconformance.ymlbuild-filter fix.pnpm-lock.yamlmoves because@bsv/conformance-runner-tsnow depends on@bsv/air-gap(workspace link only, see Dependency evidence above).bsv-blockchain/BRCs@master.🤖 Generated with Claude Code