refactor(cli): refactor messaging-related CLI args#565
Merged
Conversation
--messaging json, add --messaging.enabled opt-in
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.30.
| Benchmark suite | Current: 088c24e | Previous: 2aa5b3e | Ratio |
|---|---|---|---|
StorageEntry/compress |
161 ns/iter (± 6) |
123 ns/iter (± 3) |
1.31 |
StorageEntry/decompress |
159 ns/iter (± 4) |
118 ns/iter (± 3) |
1.35 |
ContractNonceChange/compress |
160 ns/iter (± 2) |
122 ns/iter (± 4) |
1.31 |
ContractNonceChange/decompress |
277 ns/iter (± 7) |
205 ns/iter (± 3) |
1.35 |
GenericContractInfo/decompress |
114 ns/iter (± 2) |
86 ns/iter (± 2) |
1.33 |
Felt/decompress |
64 ns/iter (± 4) |
47 ns/iter (± 7) |
1.36 |
BlockHash/decompress |
63 ns/iter (± 12) |
47 ns/iter (± 4) |
1.34 |
TxHash/decompress |
63 ns/iter (± 7) |
47 ns/iter (± 6) |
1.34 |
ClassHash/decompress |
63 ns/iter (± 6) |
46 ns/iter (± 4) |
1.37 |
CompiledClassHash/decompress |
63 ns/iter (± 8) |
46 ns/iter (± 6) |
1.37 |
TypedTransactionExecutionInfo/decompress |
3819 ns/iter (± 103) |
2560 ns/iter (± 72) |
1.49 |
MigratedCompiledClassHash/decompress |
162 ns/iter (± 8) |
119 ns/iter (± 5) |
1.36 |
ReceiptEnvelope/decompress |
6698 ns/iter (± 267) |
4899 ns/iter (± 314) |
1.37 |
TrieHistoryEntry/decompress |
278 ns/iter (± 13) |
190 ns/iter (± 14) |
1.46 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @kariy
Codec benchmark diff vs
|
| Benchmark | Baseline (ns) | Current (ns) | Δ |
|---|---|---|---|
CompiledClass(fixture)/compress |
2714068 | 2546979 | -6.16% |
CompiledClass(fixture)/decompress |
2932598 | 2743536 | -6.45% |
ExecutionCheckpoint/compress |
32 | 29 | -9.38% |
ExecutionCheckpoint/decompress |
26 | 23 | -11.54% |
PruningCheckpoint/compress |
32 | 29 | -9.38% |
PruningCheckpoint/decompress |
26 | 23 | -11.54% |
VersionedHeader/compress |
655 | 664 | +1.37% |
VersionedHeader/decompress |
815 | 731 | -10.31% |
StoredBlockBodyIndices/compress |
78 | 71 | -8.97% |
StoredBlockBodyIndices/decompress |
36 | 36 | +0.00% |
StorageEntry/compress |
148 | 119 | -19.59% |
StorageEntry/decompress |
139 | 119 | -14.39% |
ContractNonceChange/compress |
148 | 120 | -18.92% |
ContractNonceChange/decompress |
230 | 205 | -10.87% |
ContractClassChange/compress |
220 | 165 | -25.00% |
ContractClassChange/decompress |
250 | 218 | -12.80% |
ContractStorageEntry/compress |
159 | 127 | -20.13% |
ContractStorageEntry/decompress |
316 | 276 | -12.66% |
GenericContractInfo/compress |
137 | 122 | -10.95% |
GenericContractInfo/decompress |
112 | 86 | -23.21% |
Felt/compress |
83 | 70 | -15.66% |
Felt/decompress |
55 | 46 | -16.36% |
BlockHash/compress |
83 | 71 | -14.46% |
BlockHash/decompress |
55 | 46 | -16.36% |
TxHash/compress |
83 | 69 | -16.87% |
TxHash/decompress |
55 | 46 | -16.36% |
ClassHash/compress |
82 | 69 | -15.85% |
ClassHash/decompress |
54 | 46 | -14.81% |
CompiledClassHash/compress |
83 | 69 | -16.87% |
CompiledClassHash/decompress |
55 | 45 | -18.18% |
BlockNumber/compress |
47 | 42 | -10.64% |
BlockNumber/decompress |
26 | 23 | -11.54% |
TxNumber/compress |
47 | 42 | -10.64% |
TxNumber/decompress |
25 | 23 | -8.00% |
FinalityStatus/compress |
1 | 0 | -100.00% |
FinalityStatus/decompress |
12 | 10 | -16.67% |
TypedTransactionExecutionInfo/compress |
18662 | 14583 | -21.86% |
TypedTransactionExecutionInfo/decompress |
3591 | 2528 | -29.60% |
VersionedContractClass/compress |
365 | 350 | -4.11% |
VersionedContractClass/decompress |
770 | 662 | -14.03% |
MigratedCompiledClassHash/compress |
147 | 120 | -18.37% |
MigratedCompiledClassHash/decompress |
139 | 118 | -15.11% |
ContractInfoChangeList/compress |
1590 | 1517 | -4.59% |
ContractInfoChangeList/decompress |
2442 | 2259 | -7.49% |
BlockChangeList/compress |
667 | 641 | -3.90% |
BlockChangeList/decompress |
964 | 886 | -8.09% |
ReceiptEnvelope/compress |
30098 | 25026 | -16.85% |
ReceiptEnvelope/decompress |
6209 | 5010 | -19.31% |
TrieDatabaseValue/compress |
166 | 139 | -16.27% |
TrieDatabaseValue/decompress |
227 | 244 | +7.49% |
TrieHistoryEntry/compress |
292 | 257 | -11.99% |
TrieHistoryEntry/decompress |
254 | 190 | -25.20% |
|
…lement.* Re-apply CLI surface for messaging configuration on top of current main (which merged the messaging crate refactor in #564). The CLI now builds `Config.messaging: Option<MessagingConfig>` from `--messaging.enabled` plus chain-spec settlement plus optional `--settlement.*` override flags, instead of consuming an opaque JSON file via `--messaging`. Changes: - `crates/cli/src/options.rs`: add `MessagingOptions`, `ChainSpec`, `SettlementOptions`, `SettlementChainKind`; remove `chain_id` from `EnvironmentOptions` (now lives on `ChainSpec`). - `crates/cli/src/args.rs`: flatten `chain: ChainSpecArgs` and `messaging: MessagingOptions` onto `SequencerNodeArgs`; drop the top-level `--chain` and `--messaging` JSON fields; add `build_messaging_config` and the `apply_chain_overrides` / `build_settlement_from_overrides` / `apply_partial_overrides` helpers that fold `--settlement.*` overrides into the chain spec. - `crates/cli/src/file.rs`: swap `messaging: Option<MessagingConfig>` for `messaging: Option<MessagingOptions>` and add `chain: Option<ChainSpec>` to the TOML config file shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
088c24e to
fd5abcf
Compare
--messaging json, add --messaging.enabled opt-in
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #565 +/- ##
==========================================
- Coverage 73.32% 68.27% -5.05%
==========================================
Files 209 320 +111
Lines 23132 44772 +21640
==========================================
+ Hits 16961 30569 +13608
- Misses 6171 14203 +8032 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
A zero polling interval panics deep inside `tokio::time::interval` construction in the messaging trigger. Reject it at both entry points: - CLI: `value_parser` range on `--messaging.interval` so clap fails with a standard out-of-range message. - TOML: explicit guard in `build_messaging_config` (the CLI parser doesn't run on values that came from a config file). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds unit tests for the settlement override resolver: short-circuit on no inputs, fresh construction (Ethereum + Starknet) with partial-input rejection, per-field patching on existing layers (Ethereum + Starknet), chain-kind swaps (and their all-three requirement), Sovereign replacement, and core-contract parse failures on both the fresh and partial paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Summary
Replaces the standalone
--messaging foo.jsonJSON config with chain-spec-driven settlement plus an opt-in--messaging.enabledCLI flag. Stacked on top ofrefactor/messaging.The chain spec is now the canonical source of truth for settlement layer configuration. The CLI surface controls operational concerns (does this node consume L1 messages, how often, from where).
Why
Today there are two sources of settlement config:
--chain rollup.json, derives messaging viaMessagingConfig::from_chain_spec)--messaging foo.json)Source 2 was a leak — it duplicated info that already lives in the chain spec, required its own loader, and tied messaging to "is this JSON file present" rather than to an explicit operator decision. The previous review flagged this; the in-tree comment on
args.rsalready telegraphed removal ("temporary and the messaging config will eventually be removed slowly").This PR delivers on that.
Behavior changes
Settlement is now read from
chain.settlement()only. Existing rollup chain spec workflows continue to work — settlement details come from the chain spec.Messaging is opt-in. Even if the chain spec defines a settlement layer, the messaging service does not run unless
--messaging.enabledis passed. The chain spec describes the chain; the flag describes what this node does with it.Dev / ad-hoc workflow (anvil + Katana, no chain spec file): use the new
--chain.settlement-*override flags to inject settlement into the dev chain spec at startup.New CLI flags
--messaging.enabled--messaging.interval <SECS>--messaging.from-block <N>--chain.settlement-chain <ethereum|starknet>--chain.settlement-rpc-url <URL>--chain.settlement-contract <ADDR>Removed
--messaging <PATH>JSON config flag.MessagingConfig::load,MessagingConfig::parse,MessagingConfig::from_chain_spec.SettlementChainConfigenum (the messaging crate now useskatana_chain_spec::SettlementLayerdirectly).Migration
Rollup operators need to add
--messaging.enabledto existing commands. Existing JSON config files are no longer loaded.Example workflows
If
--messaging.enabledis set but the chain spec has no settlement layer, the node fails fast at startup with a clear error message pointing to the override flags.Test plan
cargo build --workspace --all-targets— cleancargo nextest run -p katana-messaging— 5/5 passcargo nextest run -p katana-db -p katana-provider— 159/159 passcargo nextest run -p katana-rpc-server --test messaging— 4/4 pass (skipping anvil-onlytest_messaging)cargo nextest run -p katana-cli— 17/17 passkatana --helpshows the 6 new flags and no longer shows--messaging <PATH>tests/messaging.rs) updated to mutate the dev chain spec settlement directlyNotes / decisions
idfield, when constructing settlement from--chain.settlement-*overrides on a chain spec without existing settlement, defaults to1(mainnet). For non-mainnet chain ids, use a chain spec file. No fourth--chain.settlement-idflag added in this pass.--chain.settlement-from-blockwas intentionally not added;--messaging.from-blockis the operational override and the chain spec'sblockfield stays a deployment-time fact.🤖 Generated with Claude Code