Repository navigation
refactor(protocol)!: rename Update.block_number_or_timestamp to block_number - #1562
Open
die-herdplatte wants to merge 1 commit into
Open
die-herdplatte wants to merge 1 commit into
die-herdplatte wants to merge 1 commit into
Conversation
…_number The field was named for the two things it could hold. `HeaderLike` had two implementors: `BlockHeader`, which returns a block number, and the RFQ path's `TimestampHeader`, which returned a unix timestamp because a book has no block and still had to travel through `TychoStreamDecoder` to reach a consumer. With the book feeds publishing their own snapshots, that header is gone and `BlockHeader` is the only implementor left, so every `Update` on every path now carries a block number. BREAKING CHANGE: `Update.block_number_or_timestamp` is `Update.block_number`, with no serde alias — a market recording saved under the old name fails to load with `missing field block_number` and needs the key renamed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Oct 7, 2026
die-herdplatte
added this pull request to stack #1556
October 7, 2026 12:56
die-herdplatte
removed this pull request from stack #1556
October 8, 2026 11:43
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Last of four PRs replacing #1440 — Stack #1556 (#1554, #1555, #1561, #1562). One field is renamed; nothing else changes.
Why now, and not earlier
Update.block_number_or_timestampwas named for the two things it could hold.HeaderLikehad two implementors:BlockHeader, which returns a block number, and the RFQ path'sTimestampHeader, which returned a unix timestamp — it existed only because an off-chain book has no block and still had to travel throughTychoStreamDecoderto reach a consumer.That was honest while it lasted, and it cost a real bug downstream: a consumer that evicts simulation overlays by block number treated a venue's unix timestamp as one, and evicted everything.
#1561 deletes
TimestampHeaderwith the round trip that needed it, leavingBlockHeaderas the only implementor. EveryUpdateon every path now carries a block number, so the field can say so. Doing this before the port would have renamed a field that was still carrying timestamps.What changes
37 reference sites across 10 files, all mechanical.
HeaderLike::block_number_or_timestamp()in tycho-client keeps its name, and the two decoder call sites that read it with it: it is the accessor on the header trait, and the trait is what the follow-up below removes.Breaking change
No serde alias. A serialized
Update— a saved market recording — written under the old name fails to load withmissing field block_numberand needs the key renamed. An alias was considered and left out: it would keep a field name that no longer describes anything readable in the type forever, for the sake of recordings that #1561 already invalidates in other ways (every pair-keyed component id and every book state's serialized form changed there), so a recording that survives this rename does not replay anyway.Follow-up, not here
With one implementor left, the header generic itself is dead weight:
HeaderLike,FeedMessage<H>andStateSyncMessage<H>in tycho-client, andTychoStreamDecoder<H>andTryFromWithBlock<T, H>(23 implementations) in tycho-simulation, all exist so aTimestampHeadercould stand in for a block header. Removing the parameter is mechanical but breaks the public API of both crates, so it is its own PR.Verification
cargo clippy --workspace --all-targets --all-featuresclean,cargo +nightly fmt --all --checkclean.block_number_or_timestampsurvives only asHeaderLike's accessor in tycho-client and the two decoder call sites that read it.--lib: 1668 pass, the 17 failures theRPC_URL-gated EVM tests that fail on main too; tycho-integration-test 34 pass; tycho-execution 198 pass with its 4RPC_URL-gated failures.🤖 Generated with Claude Code