Skip to content

fix(scanner): preserve adapter timestamps for custom chains - #215

Merged
truthixify merged 3 commits into
wraith-protocol:developfrom
aratass:fix/adapter-timestamps
Sep 25, 2026
Merged

truthixify merged 3 commits into
wraith-protocol:developfrom
aratass:fix/adapter-timestamps

Conversation

@aratass

@aratass aratass commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Closes #212.

The bug

scanChainAdapterSource in src/scanner/unified.ts yielded a literal for every result:

yield { announcement: next.value, timestamp: 0 };

So MatchedAnnouncement.timestamp was always 0, for in-tree chains and for third-party adapters alike. An adapter that knew the block, ledger or slot time of a match had nowhere to put it, and scanAll callers got a field that looked like a Unix timestamp and was in fact a constant.

The contract

ChainScannerAdapter gains one optional method:

timestampOf?(matched: TMatched): number | undefined;

Resolution order, in resolveTimestamp:

  1. adapter.timestampOf(matched), when the adapter defines it. The adapter author opted into the contract, so this wins.
  2. A numeric timestamp field on the matched value. This is the no-code path for adapters that already carry the time on their result.
  3. UNKNOWN_TIMESTAMP, now exported from the package root.

UNKNOWN_TIMESTAMP is 0, which is what the field already held, so this is a naming of existing behaviour rather than a change to it. The doc comment says what it means: scanAll interleaves as results arrive and sorts nothing, so the value is not an ordering key, and callers that bucket by time should read it as absent rather than as 1 January 1970.

Two things the acceptance criteria did not ask for, and why they are here

Validation. coerceTimestamp accepts only finite, non-negative numbers. NaN, Infinity, -1, "1700000000" and null all fall through to UNKNOWN_TIMESTAMP. Without this a half-written adapter puts NaN on a matched announcement, and NaN compares false against everything, so it corrupts a caller's sort silently rather than failing.

A throwing timestampOf does not abort the scan. A scan can be minutes long across several chains. Losing all of it because one adapter's timestamp helper threw on one item is the wrong trade, so the throw is caught and that item reports UNKNOWN_TIMESTAMP.

Both are covered by tests. If you would rather the malformed cases throw, that is a one-line change in coerceTimestamp and I will make it.

Backward compatibility

An adapter that does not implement timestampOf, on a value with no timestamp field, reports UNKNOWN_TIMESTAMP for every match. That is byte-for-byte the old behaviour, and there is a test pinning it. No in-tree adapter carries a timestamp field today, so evm, stellar, solana and ckb are unaffected.

Verification

Base is develop at 0a7cff2, run the way CI runs it: pnpm 10, pnpm install --frozen-lockfile, then pnpm test.

test files tests
develop 76 passed, 2 skipped 1221 passed, 5 skipped
this branch 77 passed, 2 skipped 1232 passed, 5 skipped

Zero failures on either side. The 11 extra passes are the new file. pnpm run format:check is clean, pnpm build succeeds, and pnpm api:check passes with the API report regenerated in the second commit.

Correction to the first version of this description

It claimed a pre-existing baseline of 18 failing test files and 7 failing tests, that pnpm install --frozen-lockfile fails with ERR_PNPM_BROKEN_LOCKFILE, and that prettier --check . already fails on six files. All three were wrong, and none of them are true of this repository. They came from my running pnpm 12 instead of the pnpm 10 this project pins, which produces a broken install and a misleading test run. On pnpm 10 the lockfile installs cleanly in 28 seconds, prettier reports no issues at all, and the suite is green. Sorry for the noise.

Test coverage added

test/scanner/adapter-timestamps.test.ts, 11 tests:

  • timestampOf values reach the announcement in source order, with seq staying monotonic alongside them
  • a timestamp field on the value is used when timestampOf is absent
  • timestampOf wins when both are present
  • an adapter with neither still scans, reporting UNKNOWN_TIMESTAMP
  • five parameterised cases for NaN, Infinity, negative, string and null
  • a throwing timestampOf does not abort the scan
  • two custom chains keep separate timestamps and separate seq counters

scanChainAdapterSource hardcoded timestamp: 0 for every result, so scanAll
discarded time information for third-party chains even when the adapter knew
it. Define the timestamp contract on ChainScannerAdapter as an optional
timestampOf(), read a numeric timestamp field on the matched value as a
fallback, and export UNKNOWN_TIMESTAMP as the documented value for a match
whose time is genuinely unknown.

Non-finite, negative and non-numeric values degrade to UNKNOWN_TIMESTAMP
rather than propagating NaN onto a matched announcement, and a throwing
timestampOf does not abort the scan.

Adapters that do not implement the contract behave exactly as before.

Closes wraith-protocol#212
api:check compares the built surface against etc/*.api.md and fails when they
diverge. This adds timestampOf and UNKNOWN_TIMESTAMP to the committed report,
which is what the CI failure was asking for.
@aratass

aratass commented Sep 23, 2026

Copy link
Copy Markdown
Contributor Author

CI is red on this one, and I do not think it is this PR. Saying so with numbers rather than just asking for a re-run.

The failure is in test/chains/stellar/scan.test.ts:584:

AssertionError: expected 5528248 to be less than 5242880

That is the memory bounded: 100k announcements use < 10x memory of 1k test, 5.4% over its 5 MiB ceiling. Two observations about it:

  • It only calls global.gc() when that function exists, and pnpm test runs vitest without --expose-gc, so no collection is ever forced. What it actually measures is the heapUsed delta across a 100,000-iteration loop, which depends on when V8 happens to collect.
  • I could not reproduce it. On this branch, Node 20.20.2, pnpm install --frozen-lockfile under pnpm 10: 5 runs, 5 passes. The same 5 runs on develop: 5 passes. On Node 22, 4 and 4.

This PR also does not touch scanAnnouncementsStream or anything on the Stellar scanner path. It changes src/scanner/unified.ts and adds one export to src/index.ts.

So my reading is that the assertion sits close enough to its ceiling to tip over on a loaded runner, rather than that this change regressed memory. I cannot re-run the job from a fork. Happy for you to re-run it, and if it comes back red I will dig properly rather than assume.

Separately, the earlier api:check failure was mine and is fixed in the second commit: the new timestampOf and UNKNOWN_TIMESTAMP exports changed the public surface and etc/sdk.api.md had not been regenerated.

@truthixify
truthixify merged commit 92eb209 into wraith-protocol:develop Sep 25, 2026
14 checks passed
@truthixify

Copy link
Copy Markdown
Contributor

Merged. Timestamp handling is documented, validates bad values, and keeps legacy adapters working. Thanks @aratass.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Wave 9] Preserve timestamps for custom scanner adapters

3 participants