Conversation
🧪 Network TestsTo run network tests for this PR, use: gh workflow run network-tests.yml -f pr_number=995Available test options:
Test types: Results will be posted as workflow runs in the Actions tab. |
|
❌ CLI reference check failed in CI. Please run |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #995 +/- ##
==========================================
+ Coverage 54.47% 55.80% +1.33%
==========================================
Files 403 418 +15
Lines 68046 70462 +2416
Branches 68046 70462 +2416
==========================================
+ Hits 37066 39320 +2254
- Misses 29120 29262 +142
- Partials 1860 1880 +20 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
0b6fc5d to
2db2f88
Compare
c86fa0a to
35647d3
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces a generic transactional abstraction (including a derive macro) and rewires the collator’s queue / reader / anchors pipelines to support cheap rollback, delayed commit of queue diffs, and richer statistics and metrics around collation.
Changes:
- Add a reusable
tycho_util::transactionalmodule (withTransactionaltrait,TransactionalValue/Option/HashMap/BTreeMap) plus a#[derive(Transactional)]proc macro to compose transactions across complex structs. - Refactor collator internals (reader state, cumulative statistics, queue statistics, anchors cache, message types) to use the transactional abstraction, enabling full undo of collator state and “prepare then commit” semantics for queue diffs, along with new metrics and dashboards.
- Simplify and harden message representation (
ParsedMessage), anchoring logic, and queue statistics, and split collator statistics into dedicated modules (collator::collator::statistics::{queue,cumulative}) with extensive unit tests.
Reviewed changes
Copilot reviewed 50 out of 52 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
util/src/transactional/mod.rs |
Introduces the core Transactional trait used across the repo. |
util/src/transactional/value.rs |
Adds TransactionalValue<T> wrapper with snapshot-based transactional semantics and unit tests. |
util/src/transactional/option.rs |
Adds TransactionalOption<T> to manage Option<T> in transactions, including nested inner Transactional values and snapshotting for presence/absence. |
util/src/transactional/hashmap.rs |
Implements transactional wrapper around FastHashMap tracking added/removed keys for rollback and delegating begin/commit/rollback to V: Transactional. |
util/src/transactional/btreemap.rs |
Same as hashmap variant but for BTreeMap, supporting ordered keys used in queue/range state. |
util/src/lib.rs |
Exposes the new transactional module from tycho_util. |
util-proc/src/lib.rs |
Registers the new #[derive(Transactional)] proc macro entry point. |
util-proc/src/derive_transactional.rs |
Implements macro expansion for #[derive(Transactional)], generating begin/commit/rollback/in_tx that delegate to non-#[tx(skip)] fields and enforce T: tycho_util::transactional::Transactional bounds. |
util-proc/Cargo.toml |
Keeps syn configuration unchanged but reflows, and is used by the new derive macro. |
scripts/gen-dashboard.py |
Adapts collator Grafana dashboard to new metric names for queue diff preparation/commit and state/anchors cache commit/rollback timings. |
collator/src/types.rs |
Re-exports processed_upto types publicly in this crate and converts ParsedMessage into an Arc<ParsedMessageInner> wrapper with an API of accessors instead of public fields. |
collator/src/queue_adapter.rs |
Adds prepare_diff to MessageQueueAdapter and its impl, collecting metrics and returning InternalQueueTransaction instead of writing immediately; adjusts diff-statistics construction for new DiffStatistics::new signature. |
collator/src/internal_queue/types/stats.rs |
Reimplements QueueStatistics as a transactional structure using versioned values (committed/uncommitted), adds StatisticsView/iterator types, moves shard count aggregation into DiffStatistics::new, and adds extensive unit tests. |
collator/src/internal_queue/types/ranges.rs |
Extracts compute_cumulative_stats_ranges helper from old CumulativeStatistics into the queue types module for reuse and testing. |
collator/src/internal_queue/state/storage.rs |
Extends QueueState with prepare_diff returning InternalQueueTransaction and implements it for QueueStateStdImpl; write_diff now delegates to prepare_diff().write(). |
collator/src/internal_queue/queue.rs |
Adds prepare_diff to the queue abstraction to prepare state changes and metadata in one shot and return a transaction, while apply_diff becomes a thin wrapper that times and writes the prepared transaction. |
collator/src/collator/types.rs |
Cleans up a large amount of collator-specific types: moves anchors cache to a dedicated module, replaces inline CumulativeStatistics and ConcurrentQueueStatistics with imports from new statistics modules, and adapts tests and helpers to the new ParsedMessage API. |
collator/src/collator/tests/messages_reader_tests.rs |
Adapts tests to new MessagesReader lifetimes, AnchorsCacheTransaction usage, compute_cumulative_stats_ranges, transactional reader state, and the new ParsedMessage accessor API. |
collator/src/collator/tests/externals_reader_tests.rs |
Updates externals reader tests to use new ext/int state modules, AnchorsCacheTransaction, and transactional skip/processed offsets (TransactionalValue<u32>). |
collator/src/collator/test_utils.rs |
Refactors stub message constructors to build ParsedMessage via ParsedMessage::new instead of constructing the struct directly or boxing it. |
collator/src/collator/statistics/queue.rs |
Introduces TrackedQueueStatistics, a mutex-protected, shard-aware wrapper around QueueStatistics that tracks per-shard totals and itself implements Transactional. |
collator/src/collator/statistics/cumulative.rs |
Reimplements CumulativeStatistics as a transactional type with an internal transaction log, separating “initial stats” from tracked, per-shard “remaining stats”, and adds a thorough test suite for commit/rollback, processed-to updates, and cross-shard semantics. |
collator/src/collator/statistics/mod.rs |
New module wiring that exposes cumulative and queue statistics submodules. |
collator/src/collator/state/mod.rs |
Adapts state display utilities to the new externals partition reader (ext::partition_range_reader) and transactional skip/processed offsets. |
collator/src/collator/mod.rs |
Wires in the new anchors_cache and statistics submodules, uses AnchorsCacheTransaction for temporary operations, and passes transactional anchors cache/readers into collator phases and tests. |
collator/src/collator/messages_reader/state/mod.rs |
Refactors message reader state into ext and int submodules; marks ReaderState as #[derive(Transactional)]; rewrites restoration of ProcessedUptoInfoStuff using transactional fields and the new externals state types. |
collator/src/collator/messages_reader/state/int/{reader,range_reader,partition_reader,mod}.rs |
Splits previous internals state into transactional components: reader, per-partition, and per-seqno-range state, all using the new transactional wrappers and providing the same logical operations (range retention, prev+current iteration, etc.). |
collator/src/collator/messages_reader/state/ext/{reader,range_reader,partition_reader,partition_range_reader,mod}.rs |
Splits previous externals state into reader, per-partition, per-range, and shared key/range types; makes most state transactional while keeping some flags (fully_read) non-transactional; updates conversions to/from ExternalsRangeInfo. |
collator/src/collator/messages_reader/new_messages.rs |
Updates new-messages reader to use the refactored internals range state, the new ParsedMessage API, and StatisticsViewIter from QueueStatistics for partition routing. |
collator/src/collator/messages_reader/mod.rs |
Generalizes MessagesReader to carry both shard and anchors-cache lifetimes, uses AnchorsCacheTransaction, integrates CumulativeStatistics and TrackedQueueStatistics, refactors cumulative-stats loading via compute_cumulative_stats_ranges, and updates logging to the new transactional state layout. |
collator/src/collator/messages_reader/internals_reader.rs |
Refactors internals reader to use transactional partition/range state, TrackedQueueStatistics instead of ConcurrentQueueStatistics, and CumulativeStatistics::apply_diff_stats, while updating collection and logging logic to the new InternalsRangeReaderState API. |
collator/src/collator/messages_reader/internals_range_reader.rs |
Adapts range reader to the new transactional internals state and to the new queue stats API (contains/statistics().iter()), and simplifies iteration over map entries. |
collator/src/collator/messages_reader/externals_reader.rs |
Refactors externals reader to the new ext state modules, wraps AnchorsCache mutations in AnchorsCacheTransaction, moves the skip/processed offset logic to TransactionalValue<u32>, and updates parsing to the new ParsedMessage wrapper. |
collator/src/collator/execution_manager.rs |
Switches message lists and fields from Box<ParsedMessage> to ParsedMessage (sharing via Arc internally) and updates access to message data via the new accessor methods. |
collator/src/collator/do_collate/prepare.rs |
Adjusts prepare phase generics to propagate the anchors-cache lifetime and passes an AnchorsCacheTransaction instead of the raw cache. |
collator/src/collator/do_collate/mod.rs |
Major change to collation pipeline: instantiates an AnchorsCacheTransaction and wraps ReaderState in a single, top-level transaction, moves anchor removal/validation into run, converts queue diff application into a “prepare diff + commit tx” sequence with separate timing, and integrates new metrics for reader state and anchors cache commit/rollback. |
collator/src/collator/do_collate/finalize.rs |
Updates finalize pipeline to use the two-lifetime MessagesReader type. |
collator/src/collator/do_collate/execution_wrapper.rs |
Adapts execution to ParsedMessage by value, updates logging and message construction to use the new accessors, and reworks in-message processing to be agnostic to internal representation. |
collator/src/collator/do_collate/execute.rs |
Propagates the extra lifetime parameter for MessagesReader through execute-phase state and finish logic. |
collator/src/collator/anchors_cache.rs |
Extracts and extends anchors cache into its own module, and adds AnchorsCacheTransaction with an undo log to support transactional modifications and metrics on commit/rollback. |
collator/Cargo.toml / Cargo.lock |
Adds dependencies on dashmap and tycho-util-proc for the new transactional infrastructure and moves criterion into main dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
91316bc to
01200c2
Compare
1d281b8 to
49c2ed4
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 53 out of 55 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
4e181ee to
fa458a5
Compare
Rexagon
left a comment
There was a problem hiding this comment.
Lgtm, but please squash all "fixes" commits before merge
Pull Request Checklist
NODE CONFIGURATION MODEL CHANGES
[None]
BLOCKCHAIN CONFIGURATION MODEL CHANGES
[None]
COMPATIBILITY
FULL
SPECIAL DEPLOYMENT ACTIONS
Not Required
PERFORMANCE IMPACT
Rollback states may take some time (calculating..).
TESTS
dex1500
1-255