Skip to content

Harden BASM protocol validation and Merkle path offsets - #520

Open
sirdeggen wants to merge 141 commits into
mainfrom
codex/basm-protocol-hardening
Open

sirdeggen wants to merge 141 commits into
mainfrom
codex/basm-protocol-hardening

Conversation

@sirdeggen

@sirdeggen sirdeggen commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Overlay reliability program

This change is one slice of Make Overlays Great Again.

Do not merge until the stacked TypeScript and Go overlay branches have been proven together locally. Isolated CI green is not integration acceptance. Keep this PR draft. No npm publication, live database migration, or Mongo/BASM default activation.

C05 (verified progressive API) is parked. C02 (ts-stack#517) and C04 (ts-stack#518) both edit LookupResolver.ts and must not merge independently.

Keep this pull request in draft until local validation is complete.

Program and scope

  • Tracker or issue: Overlay reliability plan B01 TS protocol/conformance slice
  • Why this change is needed: BASM peer inputs must be bound to local canonical headers and admitted-list indices; MerklePath offsets must not wrap at 2^31.
  • Explicitly out of scope: B02 durable recovery jobs, automatic BASM sync enablement, W01 shared fixture package (full B01 still needs those)
  • Exact head SHA reviewed: 752464dff9d91ee4d0ee4abdd1ad6997a4141912

Impact

  • Public package source or manifest changed; affected packages are listed below
  • Security-sensitive boundary changed
  • Documentation or examples changed

Affected packages: @bsv/sdk (MerklePath), @bsv/overlay, @bsv/overlay-express

Verification

  • Local commands and results: MerklePath 46 passed; overlay BASM 91 passed including local Go five-method HTTP; OverlayExpress 113 passed (implementer evidence)
  • Hosted CI run: not yet (draft)
  • All applicable checks are terminal and successful on the exact head

Security and dependencies

  • No dependency or lockfile change

Release and operations

  • No npm publication was performed from a workstation or from this PR
  • enableBASMSync remains false

…duling

Subscribe to SLAP tracker updates during an active query so later
advertised hosts can join a fair, concurrency-bounded queue. Read lookup
bodies incrementally with honest resource-limit accounting, keep
trackers as routing sources, and leave raw query$ unverified while
exposing the C02 onEvidence intake seam.

Preserves 2s/5s delays, reputation/backoff, query/query$/freeform APIs,
and existing CORS/public lookup request headers.
Add replica-set schema bootstrap, content-addressed payload publication,
transactionally guarded references/GC, and majority-commit outcome helpers
behind an optional mongodb peer. This is not an AdmissionStorage adapter
and does not change Engine or Knex defaults.
Preserve per-entry script results so a rejected sibling cannot poison a
shared ancestor. LocalChainTracker tokens use participating sources only
and fail closed on missing identity. Reset ownership is rechecked after
dispose before destructive hooks. Built-in remote ChainTracks clients
advertise reorg-event capability explicitly. Services.getChainTracker
publishes one coalesced wrapper before yielding.
Validate untrusted BASM peer JSON, bind remote anchors to local canonical
headers, and independently check admitted-list block indices before
historical admission. Keep automatic BASM sync disabled and leave durable
recovery jobs for B02.

Include independent BRC-136 vector checks and localhost TS client tests
against the Go five-method read/serving surface.
BHServiceClient no longer caches the queried merkle root. Validity is
decided from a freshly read header, so a false result cannot invert on
retry and a reorg cannot reuse a stale positive.
Store discovery bounds on the SLAP host cache so a tighter-limit query
cannot freeze a truncated host set for a later broader attempt. Throw
the historical no-competent-hosts error from query()/queryDetailed when
a deadline expires before any host is admitted; query$ still emits the
deadline snapshot.
Collection validators used lexicographic $lte on unpadded outputIndex
strings, which rejected legal indexes such as "9". Bound uint32 fields
with $toLong on outputs, consumption edges, GASP nodes, and SHIP/SLAP.
Verify claimed admitted-list block indices against the compound Merkle
path even when every remote txid is already local. Check inclusion with
the chain-tracker root at the claimed height instead of MerklePath.verify,
which also enforces coinbase 100-block spendability.
LookupResolver is a BRC-100 JSON boundary file, so the in-flight
discovery key cannot use JSON.stringify. stringifyBRC100 preserves the
same service-plus-limit tuple without changing cache cover semantics.
Drop redundant optional-undefined on lookup evidence limits, extract
parseEvidence and Chaintracks header-retry helpers below S3776, and
rename coordinator catch params to error_.
Extract BASM reconcile helpers so cognitive complexity stays at or below 15,
prefer optional chaining on nullable anchors, and invert the advertised
response-length comparison.
…e-c02

# Conflicts:
#	docs/reference/package-api-migrations.md
#	governance/package-release-notes.json
…ardening

# Conflicts:
#	docs/reference/package-api-migrations.md
#	docs/reference/stack-facts.md
#	governance/package-release-notes.json
#	governance/repository-health/baselines.json
Extract query/session and host-tracking helpers so cognitive complexity
stays within Sonar's limit, and clear the remaining overlay-tools
findings without changing lookup behavior.
Brand StorageUint64, extract high-complexity helpers, and apply
Sonar-preferred optional chaining, regex, clone, and sort APIs
without changing payload identity or uint32 $toLong validators.
# Conflicts:
#	docs/reference/package-api-migrations.md
#	docs/reference/stack-facts.md
#	governance/repository-health/baselines.json
S7746 on LookupResolver: prefer throw over return Promise.reject in the facilitator start chain.
discoverOverlayCertificates called getServices(), which throws on wallets
constructed without services and broke BRC-100 discoverBy* conformance.
Missing chain context now yields no identities instead of throwing.
Jest's istanbul graph still resolved 3.15.1, which fails pnpm audit --audit-level=high.
Jest's istanbul graph still resolved 3.15.1, which fails pnpm audit --audit-level=high.
Merge from main left baselines.json on 2.4.2 while package.json is 2.5.0.
Jest's istanbul graph still resolved 3.15.1, which fails pnpm audit --audit-level=high.
Jest's istanbul graph still resolved 3.15.1, which fails pnpm audit --audit-level=high.
Health requires overrideRemovalReview.retainedCount to match the live registry.
Health requires overrideRemovalReview.retainedCount to match the live registry.
Health requires overrideRemovalReview.retainedCount to match the live registry.
sirdeggen and others added 6 commits September 18, 2026 15:13
…ling

The main integration merge raised the SDK esbuild raw ceiling to 600000,
a value that belongs to the lookup discovery change (#518). This branch
measured 590000; restore it so the diff carries only its own budget.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Restore the eleven ts2md reference pages for modules this change does
not touch to the incoming copies; the generator run had imported
unrelated drift that already exists on main. Keep the regenerated
overlay-tools page. Reinstate this change's own 600000 esbuild raw
ceiling after restacking on the corrected evidence branch.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ery branch

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…n branch

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The ordered-merge integration took the incoming @bsv/sdk release-note
entry verbatim and dropped this change's sentences. Append them to the
cumulative candidate text and regenerate the package migration table.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
sirdeggen added a commit that referenced this pull request Sep 18, 2026
Real merge (no rebase/squash) so the criss-cross history between
#519 and #525 reconciles correctly: prep/520's MongoPayloadStore
addReference() idempotent-reference-readiness guard, its byteLength
$cond on reclaim, and all of #519's mongo/** tests (including the
uint64 payload-length boundary case) survive alongside #525's own
Mongo AdmissionStorage adapter and Engine.submit() admission-commit
path. Engine.ts merges cleanly as a pure superposition of #520's BASM
guard/mode handling and #525's assertSupportedTopics /
shouldSkipPropagation / acknowledgeOverlayAdmission refactor; the
three-value SubmissionMode is threaded through consistently end to
end.

Resolves governance/repository-health/baselines.json,
governance/package-release-notes.json, the overlay-topics docs/
package metadata, and the message-box-client browser budget by
taking prep/520's newer values and layering #525's own additions on
top; deduplicates the js-yaml override assertion in
scripts/dependency-release-governance.test.mjs now that prep/520
already carries it. docs/reference/package-api-migrations.md and
docs/reference/stack-facts.md are taken from prep/520 pending
regeneration in the metadata commit.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
sirdeggen added a commit that referenced this pull request Sep 18, 2026
Bumps the overlay family's own version fields on top of what #517-#520
already carry: @bsv/overlay 2.6.0, @bsv/overlay-express 2.7.1,
@bsv/overlay-discovery-services 2.2.4, @bsv/overlay-topics 1.8.3.
No @bsv/sdk or @bsv/wallet-toolbox version bump is included; that
release decision stays with the maintainer.

Updates governance/repository-health/baselines.json's
publicPackageVersions for the four bumped packages, and regenerates
docs/reference/package-api-migrations.md and docs/reference/stack-facts.md
(pnpm docs:facts) plus each package's docs page frontmatter so they
match. governance/package-release-notes.json's cumulative summary and
migration prose for @bsv/overlay already carries this PR's own opt-in
Mongo AdmissionStorage adapter / Engine.submit() admission-commit
sentence, layered onto #519/#520's BASM prose during the predecessor
merge; no further edit is needed here.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
sirdeggen and others added 21 commits September 18, 2026 15:36
query() and queryDetailed() flatten query$ to its first snapshot. The
terminal gate only threw when hostCount was 0, and explicitly skipped
the throw for terminalReason 'cancelled', so a caller abort resolved as
{ type: 'output-list', outputs: [] }. At any host count an aborted
attempt was indistinguishable from an authoritative "this service has
no outputs" answer, which is exactly the completion-vs-empty confusion
queryDetailed exists to prevent.

Throw lookupAbortError() before the host-count gate whenever the
terminal snapshot reports 'cancelled'. query$ is unchanged and still
emits its terminalReason: 'cancelled' snapshot for progressive callers.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
chargeLookupDiscoveryBytes can exhaust maxTotalBytes while SLAP trackers
are still being read, before any host is admitted. That leaves the
terminal snapshot with hostCount 0, terminalReason 'resource-limit' and
the limit recorded in limitsHit. queryDetailed()'s hostCount gate did not
look at terminalReason, so it threw the historical "No competent <network>
hosts found by the SLAP trackers" Error and discarded both fields.
Security-sensitive Promise callers could not tell "the trackers named no
hosts" from "this attempt ran out of its own byte budget".

Branch the gate on terminalReason: 'resource-limit' throws
LookupResourceLimitError carrying the first limit that was hit, while
'deadline' and a settled empty discovery keep the historical error and
message unchanged.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A merge on this branch widened the submit() SPV guard from
`mode !== 'historical-tx-no-spv'` to `mode === 'current-tx'`. That removed
`tx.verify(this.chainTracker)` from every `historical-tx` submission, not just
from the BASM admission path that needed it. `historical-tx` is a public
submission mode, so any caller could get an unverified transaction applied to
a topic.

Root cause: the guard was widened to work around a symptom. BASM admission
proves inclusion independently (`chainTracker.isValidRootForHeight` plus the
canonical-header binding) and must not re-apply `MerklePath.verify`'s coinbase
100-block spendability rule, but the correct lever for that is the existing
`historical-tx-no-spv` mode on the BASM call site, not a relaxed global guard.

Restores the original guard and switches the BASM admission in
`fetchBASMMissingTransactions` to `historical-tx-no-spv`. The other effects of
that mode are correct here: propagation is skipped (already the case for
`historical-tx`), and although the broadcast block is entered, every
BASM-admitted transaction carries its extracted Merkle path, so
`broadcastAcceptedSubmission` synthesises a local success and never reaches the
network. Topic managers now observe `historical-tx-no-spv` for BASM
admissions; no bundled manager branches on the mode.

Tests: the coinbase fixture no longer mocks `Engine.submit`, so the young
offset-0 proof is admitted through the production path, and a new test proves a
GASP-style `historical-tx` submission with an invalid proof is still rejected.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
performLookupRequest built its RequestInit without a redirect field, so
fetch defaulted to 'follow'. normalizeLookupHost and the https: guard in
lookup() validate the advertised URL only; neither runs again on the
final URL. A SLAP-advertised HTTPS host could therefore answer the
lookup POST with a 307/308 and have the serialized
stringifyBRC100({ service, query }) body re-sent to http:, loopback or a
link-local address. This PR queries every eligible advertised host, so
the hop sat on the hot path for untrusted SLAP domains, and the SLAP
tracker discovery requests reach the network through this same method.

Set redirect: 'error' so the transport fails closed. fetch rejects, and
the existing failure path records the advertised host as an ordinary
availability failure instead of crashing the query or following the hop.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`resolveGoWorktree()` tried the `BASM_GO_OVERLAY_SERVICES` environment variable
and then fell back to the literal path
`/Users/personal/git/go/worktrees/go-overlay-services-basm`. That fallback made
the suite run on exactly one workstation and `describe.skip` everywhere else,
including CI, so the B01 protocol was never actually spoken by any job that
claimed to cover it, and the skip was silent.

Root cause: the checkout location was treated as a discovery problem with a
convenient default instead of an explicit input. Resolution is now the
environment variable alone. When it is unset the suite is skipped under a title
that names the variable; when it is set but does not resolve to a
go-overlay-services checkout the resolver throws, so a job configured for Go
interop fails closed rather than quietly skipping.

Tests: a new BASMGoInteropGating suite asserts the gate from the interop
source, which is the only machine-independent way to catch this class of
regression - a behavioural assertion agrees with a workstation fallback on
every machine that lacks that path, which is precisely where the fallback
hides. The fixtures README now states that Go interop does not run by default
and records the fixture provenance as a repository-relative path.

Verified locally: with
BASM_GO_OVERLAY_SERVICES=/Users/personal/git/go/worktrees/go-overlay-services-basm
the interop suite builds the Go host and passes (1/1); with a non-checkout path
the suite fails to run with the explanatory error; unset, it skips.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
addReference's idempotent re-add branch only re-validated the payload
was still ready; it never touched the existing (ownerKind, ownerId,
slot) pin row's expiresAt. Extending a pin therefore silently kept the
stale expiry, and claimGarbage counted an already-expired pin as
not-live via $expr {$gt:['$expiresAt','$$NOW']} but never deleted that
expired pin document. The unique slot row survived expiry and kept
short-circuiting future addReference calls on that slot, so a caller
could be told content stayed pinned while GC moved the payload
ready -> deleting underneath it.

Add refreshPinExpiry: called after the existing {_id: payloadId,
state: 'ready'} CAS matches (so it stays serialized against a
concurrent GC claim in the same transaction), it reactivates an
expired pin with the caller's new expiry, allows extending a still-
live pin to a later expiry, and rejects an attempt to shorten a live
pin's expiry rather than silently ignoring it. claimGarbage now
deletes every expired pin reference row for the payload in the same
transaction as its ready -> deleting claim, so a stale slot cannot
outlive the payload it named.

Document the re-add/extend/reactivate/shorten rule in
specs/overlay/mongo-v1.md next to the existing pin semantics.

Regression tests (mongodb-memory-server replica fixture) added to
src/__tests/mongo/MongoPayloadStore.test.ts:
- re-adding a pin slot extends its expiry but never shortens it
  silently
- re-adding an expired pin slot reactivates it and makes the payload
  live again
- claimGarbage deletes the expired pin reference row inside the same
  transaction as its claim

All three were confirmed to fail against the pre-fix source for the
reason described above (verified by temporarily reverting this file
and re-running the suite) before the fix was applied.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Follow-up to 7584d4e, which moved BASM admission from the public
`historical-tx` submission mode to `historical-tx-no-spv` so that restoring the
SPV guard does not re-apply MerklePath.verify's coinbase 100-block spendability
rule to an independently proven admission.

The submission mode is passed through to `TopicManager.identifyAdmissibleOutputs`,
so it is observable by every topic manager that branches on it. The release
prose described the inclusion rule but not the mode a manager now sees, which
left implementors without the one externally visible consequence of the change.

Adds that sentence to the @bsv/overlay release note and the package CHANGELOG
entry, and regenerates docs/reference/package-api-migrations.md from the
governance source. No code or behaviour change.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The reviewer asked for a regression test on the reclaim path taken when
publish fails because the caller declared a too-small byteLength:
consumePayloadChunk rejects the stream once it exceeds the declared
length, abandonUpload marks the payload row 'deleted', and a retry
with the correct digest and length must reclaim that row through
tryClaimUpload (which already recomputes byteLength from the new input
when the prior state is 'deleted') and finish 'ready' with the
corrected byteLength stored.

This path was already implemented and covered indirectly (a sibling
test manually sets state: 'deleted' with a wrong byteLength via direct
DB mutation), but there was no test driving the actual publish/fail/
retry sequence end to end. This is coverage-only: the reclaim logic in
tryClaimUpload predates this change and needed no fix, so there is no
meaningful pre-fix state to fail against in this worktree; the new
test passes against the current code, which is the expected outcome
for confirming existing, correct behavior.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… them

TransactionEvidenceCoordinator.run() shifts the next candidate off a job's
queue before calling attempt(). If every concurrency slot is already in use
(including slots still reserved by non-abortable, already-finished attempts;
see the "does not free a non-abortable backend slot..." test), attempt()
threw a plain TransactionEvidenceError('limit') and called the caller's
`settled` callback. run()'s catch treated that exactly like a genuine
verification failure: the candidate was gone (already shift()ed) and never
pushed back, so a same-txid alternate candidate that had already been
admitted to the job could be silently discarded without ever being tried,
even though it was perfectly valid.

Fix: attempt() now throws a dedicated ConcurrencyLimitSignal when the
operation never started, and does not invoke `settled` (nothing was
consumed, so no byte/candidate bookkeeping should be released). run()'s
candidate loop catches that signal specifically, unshifts the candidate
back onto job.candidates, marks the job not-running, and returns without
looping or retrying itself. Re-admission is driven solely by pump() being
invoked again when some other active attempt settles, so this cannot spin
or busy-wait for a slot.

Added a regression test that builds a genuine concurrency race: a
non-abortable "ghost" attempt (cancelled while its script verification is
still in flight) holds one of two slots, a job with two same-txid
candidates (one invalid, one valid) takes the other, and a third waiting
job is admitted into the slot freed by the first candidate's failure before
the job's own retry can reclaim it. Before the fix this made the valid
alternate candidate reject with 'limit'; the test fails for that reason on
the old code and passes with the fix.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Every TransactionEvidenceCoordinator attempt calls
ChaintracksChainTracker.getVerificationContextToken(), which requires
chaintracks.findChainTipHash(). BHServiceClient implemented that method
as `throw new Error('Not implemented')` even though it already exposes a
working findChainTipHeader() against /api/v1/chain/tip/longest. Any wallet
using a BHServiceClient as options.chaintracks, or as a LocalChainTracker
participating source, broke on every verification attempt.

Fix: implement findChainTipHash() by delegating to the existing
findChainTipHeader() and returning its hash, matching the same pattern
GoChaintracksServiceClient already uses for the same interface method.
No other ChaintracksClientApi contract surface changes.

Added a regression test on BHServiceClient confirming findChainTipHash()
resolves to findChainTipHeader()'s hash instead of throwing, and a
regression test on ChaintracksChainTracker confirming
getVerificationContextToken() succeeds end-to-end when backed by a
BHServiceClient. Both fail with "Not implemented" on the old code and
pass with the fix.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Document the TransactionEvidenceCoordinator candidate-requeue fix and the
BHServiceClient.findChainTipHash implementation in their packages'
CHANGELOG.md, matching this repo's convention of recording behavior-
affecting fixes there alongside the commit history.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The concurrency-limit signal is handled only in the candidate loop. The
cached-positive recheck does not need the same handling: run() is
reached solely from pump(), which checks the attempt limit synchronously
before starting the job, and no await separates that check from the
recheck attempt. Record the invariant next to the call so a later
refactor of pump() or run() revisits it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…dles

The ceilings carried on this branch were authored before main grew
(#534, #547), so the merged wallet bundles exceeded them and CI stopped
at the first over-budget dimension. Measured from the exact packed
dependency graph at 83c0670 (JS bundle sizes verified byte-identical
between macOS and hosted Linux; Hermes gzip estimated at the +1.3% Linux
ratio recorded in #547):

  client  Vite    1,759,717 raw / 416,284 gzip / 324,287 brotli
  client  esbuild 1,372,320 raw / 378,741 gzip / 302,937 brotli
  mobile  Metro   1,811,581 raw / 461,355 gzip / 355,871 brotli
  mobile  Hermes  3,688,871 raw / 1,502,972 gzip / 1,166,821 brotli

Only the dimensions that exceeded are raised, with about 0.25% headroom
(1% on the Hermes gzip estimate, 0.5% on Hermes brotli run variance).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Bounded discovery, the host queue and the streaming response reader grow
every bundle that embeds @bsv/sdk. Measured from the exact packed
dependency graph at the top of the ordered overlay wave (max of 5eef0fc
and 9077394; JS bundle sizes verified byte-identical between macOS and
hosted Linux; Hermes gzip estimated at the +1.3% Linux ratio from #547):

  @bsv/sdk UMD         591,705 raw / 168,531 gzip / 140,485 brotli
  @bsv/simple Vite     752,122 raw / 182,787 gzip / 150,699 brotli
  wallet client Vite   1,782,018 raw / 421,917 gzip / 328,724 brotli
  wallet client esbuild 1,389,928 raw / 383,620 gzip / 306,783 brotli
  wallet mobile Metro  1,834,234 raw / 466,804 gzip / 360,591 brotli
  wallet mobile Hermes 3,725,120 raw / 1,520,644 gzip / 1,181,384 brotli

Only the dimensions that exceeded are raised, with about 0.25% headroom
(1% on the Hermes gzip estimate, 0.5% on Hermes brotli run variance).
Later branches in the wave move these bundles by under 150 bytes, so
they inherit these ceilings unchanged.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
GASP finalization already submits with historical-tx-no-spv; its
historical-tx calls are dry-run admissibility checks. The restored guard
protects public Engine.submit callers, so say that in the test title.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

overlay-reliability Part of org overlay reliability program (project 19)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant