Conversation
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.
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.
Drop redundant optional-undefined on lookup evidence limits, extract parseEvidence and Chaintracks header-retry helpers below S3776, and rename coordinator catch params to error_.
…e-c02 # Conflicts: # docs/reference/package-api-migrations.md # governance/package-release-notes.json
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.
Health requires overrideRemovalReview.retainedCount to match the live registry.
LookupResolver and evidence helpers pushed the UMD/Vite/Metro payloads over the previous raw-size gates.
Message-box UMD, wallet Vite gzip, and Hermes bytecode now cover the evidence/discovery helper payload growth.
…ervices Contact discovery still works without services. forceRefresh bypasses contacts and still requires a chain tracker, matching the existing test.
S3776: discoverOverlayCertificates was 17 after the forceRefresh services check. The missing-services path is now requireOverlayChainTracker.
sirdeggen
added
the
overlay-reliability
|
Linked to org project Make Overlays Great Again and tracker https://github.com/bsv-blockchain/make-overlays-great-again/issues/1. This PR stays draft until the overlay stack is proven locally across |
4ad5f3e to
f9423d1
Compare
Merge origin/main (bdaebe6) into prep/517 and resolve the metadata drift this branch carried. - Restore pnpm-workspace.yaml, governance/repository-health/exceptions.json, governance/dependency-release-policy.json and pnpm-lock.yaml to main's copies. Main already ships the js-yaml 3.15.2 override and registers it as js-yaml-jest-istanbul-advisory-override, so this branch's relocation and its duplicate js-yaml-jest-advisory-override entry were redundant and produced a duplicated overrides key in the merged lockfile (ERR_PNPM_BROKEN_LOCKFILE). - Raise the bundle and platform budgets to the wave ceilings, which an earlier main merge on this branch had silently reverted. - Drop the stray .claude/tasks/context_session_c02.md working note. - Regenerate the ts2md reference docs for the overlay-tools and transaction modules so they match the source in this branch. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…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>
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>
| operation: (signal: AbortSignal) => Promise<T>, | ||
| settled?: () => void | ||
| ): Promise<T> { | ||
| if (this.activeAttempts >= this.limits.concurrentTransactions) { |
There was a problem hiding this comment.
[bug] A same-txid alternate that was already admitted to a job can be discarded without being tried. run() shift()s the next candidate, then attempt() throws 'limit' and invokes settled() if activeAttempts is already at concurrentTransactions. That is treated as a candidate failure (catch at line 366), so the receipt is never put back on job.candidates. attempt().finally calls pump(), which can start a waiting job and consume the last slot in the gap after a failed first proof and before the queued good proof runs. Under concurrent load this violates V3: a later valid receipt that joined the in-flight txid job can be dropped, and the coalesced consumers fail with 'limit' instead of verifying the remaining proof. Identity parseResults happens to be sequential so it is less exposed; the public Promise.all coordinator path that the V3 tests use is not.
Suggestion: Do not treat attempt-pool exhaustion as a candidate verdict. Either unshift the candidate and wait for a slot (the job is already running), or admit the follow-up attempt for an already-running job without competing with newly pumped work. Add a test that fills concurrentTransactions with other txids, then overlapping-verifies a bad receipt and a later good receipt for one txid.
| this.throwIfAborted(signal) | ||
| if (this.getVerificationContext() !== marker) | ||
| throw new Error('Chaintracks provider changed during event registration') | ||
| const tip = await chaintracks.findChainTipHash() |
There was a problem hiding this comment.
[bug] Every coordinator attempt now fetches getVerificationContextToken(), which requires chaintracks.findChainTipHash(). BHServiceClient still implements that as throw new Error('Not implemented') (it already has a working findChainTipHeader() at /api/v1/chain/tip/longest). Any wallet that uses BHS as options.chaintracks, or as a LocalChainTracker participating source (including remote-only fallbacks), will fail token acquisition, map it to 'invalid-evidence', and return no identities — even though header reads and the new non-poisoning isValidRootForHeight work. This PR introduced the token callers; the adapter was not updated to match.
Suggestion: Implement findChainTipHash() as (await this.findChainTipHeader()).hash, matching GoChaintracksServiceClient. Cover remote-only LocalChainTracker token lookup with a BHS fallback so this cannot regress to the stub.
| return { outpoint: verified.outpoint, certificate: this.clone(json) } | ||
| } catch (error) { | ||
| if (error instanceof TransactionEvidenceError && (error.code === 'limit' || error.code === 'timeout')) throw error | ||
| return null |
There was a problem hiding this comment.
[suggestion] IdentityEvidenceVerifier.parse rethrows only 'limit' and 'timeout'. 'context-changed' (and 'cancelled' / 'disposed') become null, which parseResults treats as a malformed overlay output. ChaintracksChainTracker.getVerificationContextToken includes the live tip hash, and the first verify() captures that token at attempt start then compares it after SPV. A new block during script/header work therefore omits a valid certificate rather than retrying. Wallet then deletes the overlay cache because certificates.length !== outputs.length, so the next discover refetches, but the current call can return an empty identity set.
Suggestion: Retry once on 'context-changed', or propagate it so discoverOverlayCertificates can refetch/re-verify instead of dropping the cert. Keep swallowing true 'invalid-evidence' parse failures.
| throw error | ||
| } | ||
|
|
||
| private attemptSources(): ChaintracksClientApi[] { |
There was a problem hiding this comment.
[suggestion] getVerificationContextToken documents a token for “providers participating in this attempt’s canonical authority”, but attemptSources() is mode-based: local-primary is always [local]. isValidRootForHeight (line 216) still falls back to remotes when local throws and fallbackOnLocalError is true (the default). A fallback-validated root can therefore be published under a local-only token. Recheck will not notice a fallback identity change, and after local recovers enough to serve a tip hash but still throw on root lookup, the coordinator can keep a remote-validated positive in a local-primary context.
Suggestion: Bind the token to the sources that actually answered isValidRootForHeight / currentHeight on this attempt (or fail the attempt if validation had to leave the participating set). If fallback-on-exception must stay, include those fallbacks in the token for that attempt.
| throw error | ||
| } | ||
| // Failed evidence must allow another fetch, including after temporary chain unavailability. | ||
| if (certificates.length !== cached.value.outputs.length) this._overlayEvidenceCache.delete(cacheKey) |
There was a problem hiding this comment.
[suggestion] Any identity lookup whose parsed certificate count does not equal the raw evidence count deletes the 2-minute overlay cache. That is correct for total chain failure, but it also fires when one host’s invalid alternate proof sits beside a valid one — the V3 path this PR exists to keep. Wallet.identityVerification.test.ts already shows the cost (lookup called 6 times for 3 discovers). A permanently mixed overlay answer will never cache and will re-hit every identity host on every call.
Suggestion: Cache the owned evidence snapshots regardless of parse yield, and only force-refresh on 'limit' / 'timeout' / tracker unavailability. Deduplicate by computed outpoint after successful verify, not by raw output count.
… 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>
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>
|
1 participant
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.tsand must not merge independently.Program and scope
b012b33a6b1891d8635b16bf48c3c83ae441a7feImpact
Affected packages:
@bsv/sdk,@bsv/wallet-toolbox(intended patch/minor at release workflow; not published from this PR)Verification
Security and dependencies
Release and operations
Notes for reviewers
C04 is a sibling PR that also touches
LookupResolver.ts. Do not merge both without a follow-up merge. C05 remains parked.