fix(scanner): preserve adapter timestamps for custom chains - #215
Conversation
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.
|
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 That is the
This PR also does not touch 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 |
|
Merged. Timestamp handling is documented, validates bad values, and keeps legacy adapters working. Thanks @aratass. |
Closes #212.
The bug
scanChainAdapterSourceinsrc/scanner/unified.tsyielded a literal for every result:So
MatchedAnnouncement.timestampwas always0, 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, andscanAllcallers got a field that looked like a Unix timestamp and was in fact a constant.The contract
ChainScannerAdaptergains one optional method:Resolution order, in
resolveTimestamp:adapter.timestampOf(matched), when the adapter defines it. The adapter author opted into the contract, so this wins.timestampfield on the matched value. This is the no-code path for adapters that already carry the time on their result.UNKNOWN_TIMESTAMP, now exported from the package root.UNKNOWN_TIMESTAMPis0, 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:scanAllinterleaves 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.
coerceTimestampaccepts only finite, non-negative numbers.NaN,Infinity,-1,"1700000000"andnullall fall through toUNKNOWN_TIMESTAMP. Without this a half-written adapter putsNaNon a matched announcement, andNaNcompares false against everything, so it corrupts a caller's sort silently rather than failing.A throwing
timestampOfdoes 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 reportsUNKNOWN_TIMESTAMP.Both are covered by tests. If you would rather the malformed cases throw, that is a one-line change in
coerceTimestampand I will make it.Backward compatibility
An adapter that does not implement
timestampOf, on a value with notimestampfield, reportsUNKNOWN_TIMESTAMPfor every match. That is byte-for-byte the old behaviour, and there is a test pinning it. No in-tree adapter carries atimestampfield today, so evm, stellar, solana and ckb are unaffected.Verification
Base is
developat0a7cff2, run the way CI runs it: pnpm 10,pnpm install --frozen-lockfile, thenpnpm test.developZero failures on either side. The 11 extra passes are the new file.
pnpm run format:checkis clean,pnpm buildsucceeds, andpnpm api:checkpasses 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-lockfilefails withERR_PNPM_BROKEN_LOCKFILE, and thatprettier --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:timestampOfvalues reach the announcement in source order, withseqstaying monotonic alongside themtimestampfield on the value is used whentimestampOfis absenttimestampOfwins when both are presentUNKNOWN_TIMESTAMPNaN,Infinity, negative, string andnulltimestampOfdoes not abort the scanseqcounters