fix(utxorpc): convert block timestamp from seconds to ms in u5c mapping - #793
fix(utxorpc): convert block timestamp from seconds to ms in u5c mapping#793stnly-btcdefi wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThe shared Cardano block mapper now converts ledger-provided UNIX timestamps from seconds to milliseconds before populating ChangesBlock timestamp mapping
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
3da2662 to
ba053d3
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pallas-utxorpc/src/shared.rs (1)
464-471: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd regression coverage for both timestamp paths.
Please add tests asserting seconds-to-milliseconds conversion and that a missing ledger timestamp still maps to
0; this protects the wire-unit contract from future regressions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pallas-utxorpc/src/shared.rs` around lines 464 - 471, Add regression tests for the timestamp construction around get_slot_timestamp: cover a present ledger timestamp and assert UNIX seconds are converted to milliseconds, then cover an absent ledger timestamp and assert the resulting Block.timestamp is 0. Reuse the existing test setup and inspect the produced timestamp field to preserve both wire-unit paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@pallas-utxorpc/src/shared.rs`:
- Around line 464-471: Add regression tests for the timestamp construction
around get_slot_timestamp: cover a present ledger timestamp and assert UNIX
seconds are converted to milliseconds, then cover an absent ledger timestamp and
assert the resulting Block.timestamp is 0. Reuse the existing test setup and
inspect the produced timestamp field to preserve both wire-unit paths.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 62be5a6e-3c8f-4b11-aa9e-b2bacf9497d8
📒 Files selected for processing (1)
pallas-utxorpc/src/shared.rs
A sixth site of the seconds-vs-milliseconds timestamp mismatch fixed in txpipe/dolos#1003: the u5c
MapperwritesLedgerContext::get_slot_timestamp's seconds-typed value straight intoBlock.timestamp, which the utxorpc spec declares as milliseconds.What the proto says
utxorpc/specdeclarescardano.Block.timestampas milliseconds (// Block ms timestamp), same as the fourChainPoint/BlockRefsites dolos#1003 covered.What the mapper actually writes
pallas-utxorpc/src/shared.rs(map_block, shared by the v1alpha and v1beta mappers):LedgerContext::get_slot_timestampis documented as UNIX seconds (pallas-utxorpc/src/lib.rs), and implementations follow that contract — so every parsed block flowing through the mapper (DolosSyncService.FollowTip/DumpHistoryamong others) carries a seconds value in a ms field.Observed downstream against Dolos
sha-c469c7fon preprod: a block at slot ~125.54M arrived withtimestamp ≈ 1_781_253_xxx. Read as seconds that decodes to 2026-06-12, which matches the slot arithmetic (preprod system start 2022-06-01, 20-day Byron era at 20s slots, 1s slots after) to the day; read as ms per spec it decodes to 1970-01-21, which is how our chain-follower's e2e caught it.The fix
Same principle as dolos#1003: convert at the proto write boundary, keep the trait untouched — its seconds contract is documented and correctly implemented. One functional line:
A missing timestamp stays
0(proto3 default), unchanged.Verification
cargo test -p pallas-utxorpcgreen. With the multiplication in place the observed preprod values decode to the same wall-clock dates as the slot math — the conversion promotes units, it does not reinterpret anything.Summary by CodeRabbit