feat(network2): add Leios mini-protocols (LeiosNotify, LeiosFetch)#788
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Leios notify and fetch support across protocol types, handshake gating, initiator and responder behaviors, emulation replies, and harness handling. ChangesLeios overlay support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pallas-network2/tests/harness/node.rs (1)
94-95: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider matching the Leios variants explicitly to preserve exhaustiveness.
The catch-all
_ => {}silently absorbs any futureResponderEventvariant, so a newly added event that should be exercised here would be ignored without a compiler error. Listing the known Leios variants (EbNotificationRequested,EbRequested,EbTxsRequested,LeiosVotesRequested) keeps the match exhaustive while still no-op'ing them.♻️ Proposed explicit no-op arms
- // Leios responder events are not exercised by this harness. - _ => {} + // Leios responder events are not exercised by this harness. + ResponderEvent::EbNotificationRequested(..) + | ResponderEvent::EbRequested(..) + | ResponderEvent::EbTxsRequested(..) + | ResponderEvent::LeiosVotesRequested(..) => {}🤖 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-network2/tests/harness/node.rs` around lines 94 - 95, The match on ResponderEvent in the harness is using a catch-all arm that hides future Leios variants from compiler exhaustiveness checks. Replace the generic no-op branch with explicit no-op arms for the known Leios responder events in this match so that Node harness behavior stays unchanged while new variants will surface as compile errors. Use the ResponderEvent match in the test harness and list the existing Leios variants explicitly instead of relying on _ => {}.
🤖 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.
Inline comments:
In `@pallas-network2/src/behavior/initiator/leiosfetch.rs`:
- Around line 103-106: The fetch request is being removed from the queue too
early in visit_housekeeping, before InterfaceEvent::Sent updates
state.leios_fetch, which can allow duplicate fetches or drop an unsent request.
Update the flow around visit_housekeeping and send_request so the request stays
queued until the send completes, either by deferring removal to the send hook or
by tracking a per-peer pending send state, using the existing self.requests and
self.send_request logic to keep one in-flight fetch per peer.
---
Nitpick comments:
In `@pallas-network2/tests/harness/node.rs`:
- Around line 94-95: The match on ResponderEvent in the harness is using a
catch-all arm that hides future Leios variants from compiler exhaustiveness
checks. Replace the generic no-op branch with explicit no-op arms for the known
Leios responder events in this match so that Node harness behavior stays
unchanged while new variants will surface as compile errors. Use the
ResponderEvent match in the test harness and list the existing Leios variants
explicitly instead of relying on _ => {}.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c2a2f0f6-4aa4-4c28-946e-ac4f3f5a483c
📒 Files selected for processing (15)
pallas-network2/benches/harness/node.rspallas-network2/src/behavior/initiator/leiosfetch.rspallas-network2/src/behavior/initiator/leiosnotify.rspallas-network2/src/behavior/initiator/mod.rspallas-network2/src/behavior/mod.rspallas-network2/src/behavior/responder/leiosfetch.rspallas-network2/src/behavior/responder/leiosnotify.rspallas-network2/src/behavior/responder/mod.rspallas-network2/src/emulation/happy.rspallas-network2/src/protocol/common.rspallas-network2/src/protocol/handshake/n2n.rspallas-network2/src/protocol/leiosfetch.rspallas-network2/src/protocol/leiosnotify.rspallas-network2/src/protocol/mod.rspallas-network2/tests/harness/node.rs
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@examples/p2p-responder/src/main.rs`:
- Around line 119-130: The new Leios request event arms in ResponderEvent
handling only log and never respond, so the example stalls. Update the match in
main to dispatch EbNotificationRequested, EbRequested, EbTxsRequested, and
LeiosVotesRequested to the existing Provide* command handlers used elsewhere in
the responder so these requests are actually served instead of just traced.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a33e5f0d-268b-4dbf-bd0f-f5e8a03beb68
📒 Files selected for processing (8)
examples/p2p-discovery/src/node.rsexamples/p2p-responder/src/main.rspallas-network2/src/behavior/initiator/leiosfetch.rspallas-network2/src/behavior/initiator/leiosnotify.rspallas-network2/src/behavior/initiator/mod.rspallas-network2/src/behavior/responder/leiosfetch.rspallas-network2/src/emulation/happy.rspallas-network2/src/protocol/leiosfetch.rs
🚧 Files skipped from review as they are similar to previous changes (6)
- pallas-network2/src/behavior/responder/leiosfetch.rs
- pallas-network2/src/emulation/happy.rs
- pallas-network2/src/behavior/initiator/leiosnotify.rs
- pallas-network2/src/behavior/initiator/leiosfetch.rs
- pallas-network2/src/protocol/leiosfetch.rs
- pallas-network2/src/behavior/initiator/mod.rs
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
examples/leios-testnet/src/main.rs (1)
47-55: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMake the relay and magic overridable.
Lines 49-51 already note that this devnet rotates. Keeping both values hardcoded means the example goes stale until someone edits source. Prefer env vars or CLI flags, with these constants only as defaults.
Also applies to: 201-208
🤖 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 `@examples/leios-testnet/src/main.rs` around lines 47 - 55, Make the Leios relay address and network magic configurable instead of relying on hardcoded defaults in main and any related setup code. Update the constants LEIOS_RELAY and LEIOS_TESTNET_MAGIC to act only as fallback values, and thread env var or CLI flag parsing through the startup path so the example can use updated relay/magic values without source edits. Also apply the same override handling wherever the relay/magic are reused later in the file, including the sections referenced in the comment.
🤖 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.
Inline comments:
In `@examples/leios-testnet/src/main.rs`:
- Around line 161-179: Handle the closed-stream case in tick() by checking the
None branch from self.network.poll_next() and taking a clear action instead of
doing nothing. Update tick() and/or run_forever() so a disconnected network
either returns an error, exits the loop, or triggers reconnect logic, rather
than letting run_forever() spin forever on repeated None results.
---
Nitpick comments:
In `@examples/leios-testnet/src/main.rs`:
- Around line 47-55: Make the Leios relay address and network magic configurable
instead of relying on hardcoded defaults in main and any related setup code.
Update the constants LEIOS_RELAY and LEIOS_TESTNET_MAGIC to act only as fallback
values, and thread env var or CLI flag parsing through the startup path so the
example can use updated relay/magic values without source edits. Also apply the
same override handling wherever the relay/magic are reused later in the file,
including the sections referenced in the comment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 35790c04-ab12-4d49-b252-38dd5706ab84
📒 Files selected for processing (3)
Cargo.tomlexamples/leios-testnet/Cargo.tomlexamples/leios-testnet/src/main.rs
- Add Bitmaps::all / from_indices (MSB-first 64-tx window convention) and slim
Response::BlockTxs to { txs }; the EB id now rides on EbFetched(PeerId, EbId,
Response) via leiosfetch::State::Idle(Option<(EbId, Response)>).
- leios-testnet: offer-gated EB transaction fetch (only on BlockTxsOffer), sized
from the EB body's tx count; intersect near a fixed tip point.
- Dedup accepted_version/supports_leios into shared behavior helpers; purge the
leios-fetch request queue on disconnect/error; drop the unused CertCbor alias.
Member
Author
|
Update: pushed follow-up work since the original review. Spec-strict refactor ( EB transaction fetch + ergonomic primitives (
Review-followup cleanups (in Verified: Known caveat: the live prototype relay resets on |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pallas-network2/src/behavior/responder/mod.rs (1)
694-733: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGate outbound Leios commands on the negotiated version.
These arms bypass
ResponderState::supports_leios()and send Leios messages unconditionally. That lets a normal API call put unsupported mini-protocol traffic on the wire before handshake completion or after a pre-v15 negotiation, which can turn into a disconnect instead of a no-op.Suggested direction
+ fn send_leios_msg(&mut self, pid: &PeerId, msg: AnyMessage) { + let Some(state) = self.peers.get(pid) else { + return; + }; + + if !state.is_initialized() || !state.supports_leios() { + tracing::debug!(%pid, "dropping leios command before Leios negotiation"); + return; + } + + self.send_msg(pid, msg); + } + fn execute(&mut self, cmd: Self::Command) { match cmd { ... ResponderCommand::ProvideEbAnnouncement(pid, header) => { - self.send_msg( + self.send_leios_msg( &pid, AnyMessage::LeiosNotify(proto::leiosnotify::Message::BlockAnnouncement(header)), ); }Apply the same helper to the other Leios-specific command arms.
🤖 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-network2/src/behavior/responder/mod.rs` around lines 694 - 733, The Leios-specific arms in ResponderCommand handling send outbound protocol messages unconditionally, bypassing ResponderState::supports_leios() and risking unsupported traffic on the wire. Update the matching logic in responder/mod.rs so the ProvideEbAnnouncement, ProvideEbOffer, ProvideEbTxsOffer, ProvideVotes, ProvideEb, and ProvideEbTxs paths are gated through the same version-check helper used for other Leios commands, and make unsupported cases no-op instead of sending messages before negotiation completes or on pre-v15 peers.
🤖 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.
Inline comments:
In `@pallas-network2/src/protocol/common.rs`:
- Around line 200-227: Bitmaps::all and Bitmaps::from_indices can overflow the
u16 window index and silently wrap/truncate when the selector spans more than
65536 windows. Add an explicit bounds check before incrementing or casting the
window index, and fail fast with an error/guard when the limit is exceeded.
Update both constructors so the window calculation in Bitmaps::all and the
offset-to-window conversion in Bitmaps::from_indices use the same overflow-safe
validation.
In `@pallas-network2/src/protocol/leiosfetch.rs`:
- Around line 95-101: The AwaitingBlockTxs branch in leiosfetch::State handling
accepts any Message::BlockTxs without checking the echoed point and bitmaps,
which can misattribute a response to the wrong EB request. Update the
Message::BlockTxs match to validate the echoed point/bitmaps against the pending
request state before returning State::Idle and constructing Response::BlockTxs,
using the existing State::AwaitingBlockTxs and Message::BlockTxs symbols to keep
the request/response pair aligned.
---
Outside diff comments:
In `@pallas-network2/src/behavior/responder/mod.rs`:
- Around line 694-733: The Leios-specific arms in ResponderCommand handling send
outbound protocol messages unconditionally, bypassing
ResponderState::supports_leios() and risking unsupported traffic on the wire.
Update the matching logic in responder/mod.rs so the ProvideEbAnnouncement,
ProvideEbOffer, ProvideEbTxsOffer, ProvideVotes, ProvideEb, and ProvideEbTxs
paths are gated through the same version-check helper used for other Leios
commands, and make unsupported cases no-op instead of sending messages before
negotiation completes or on pre-v15 peers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bbe46582-ac3a-40f7-a21c-2abfc841643d
📒 Files selected for processing (15)
examples/leios-testnet/Cargo.tomlexamples/leios-testnet/src/main.rsexamples/p2p-discovery/src/node.rsexamples/p2p-responder/src/main.rspallas-network2/benches/harness/node.rspallas-network2/src/behavior/initiator/leiosfetch.rspallas-network2/src/behavior/initiator/mod.rspallas-network2/src/behavior/mod.rspallas-network2/src/behavior/responder/leiosfetch.rspallas-network2/src/behavior/responder/mod.rspallas-network2/src/emulation/happy.rspallas-network2/src/protocol/common.rspallas-network2/src/protocol/leiosfetch.rspallas-network2/src/protocol/leiosnotify.rspallas-network2/tests/harness/node.rs
💤 Files with no reviewable changes (1)
- examples/p2p-responder/src/main.rs
🚧 Files skipped from review as they are similar to previous changes (6)
- examples/leios-testnet/Cargo.toml
- pallas-network2/tests/harness/node.rs
- examples/p2p-discovery/src/node.rs
- pallas-network2/benches/harness/node.rs
- pallas-network2/src/emulation/happy.rs
- examples/leios-testnet/src/main.rs
Move single-protocol Leios wire types out of `protocol::common` into the modules that own them: `Bitmaps`/`EndorserBlockCbor`/`TxCbor` to leiosfetch, `VoteCbor` to leiosnotify. Only the genuinely shared `RawCbor`/`EbId` stay in common; all references are re-qualified. Add CBOR-vs-CDDL conformance tests: bump the cardano-blueprint submodule to the leios-prototype tip (and track that branch) for the vendored Leios CDDLs, and introduce a shared `protocol::cddl` test helper (the `conforms!` macro, CDDL preprocessing, and the `cddl`-crate validation kernel) so each mini-protocol contributes only a `self_contained()` schema builder plus a per-message table. Gated behind the `blueprint` feature (new optional `cddl` dependency).
The Leios codecs introduced a `RawCbor` newtype that duplicated `pallas_codec::utils::AnyCbor` exactly (verbatim encode, skip-and-capture decode). Drop it and alias the Leios payload types — `EndorserBlockCbor`, `TxCbor`, `VoteCbor` — to `AnyCbor`. `AnyCbor` previously only had `from_encode` (which encodes a value), so add `from_raw_bytes`/`From<Vec<u8>>` to wrap already-encoded wire bytes, matching what `Bytes` already provides. Also drop the stray Go reference-implementation mention from the `EbId` doc.
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
1 participant
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
Adds Ouroboros Linear Leios (CIP-0164) networking support to
pallas-network2. Leios overlays Praos with an endorsement layer: producers announce Endorser Blocks (EBs) that are diffused via two new N2N mini-protocols — LeiosNotify (server-push announcements/offers) and LeiosFetch (client-pull EB bodies/txs/votes). Legacypallas-networkis untouched.Leios is added as two ordinary mini-protocols following the existing flat per-protocol conventions (no new bundling layer): a single behavior per connection, with the Leios sub-behaviors self-gated on the negotiated handshake version so non-Leios sessions are unaffected.
What's included
protocol/common.rs, next toPoint):EbId,VoteId,Bitmaps(indefinite-length CBOR map — definite encoding resets the prototype), and aRawCbornewtype for embedded CBOR payloads (EndorserBlockCbor/TxCbor/VoteCbor/CertCbor). Leios uses raw embedded CBOR (Gocbor.RawMessage), not the tag-24 wrapping chainsync/blockfetch use.protocol/leiosnotify.rs, channel 18) and LeiosFetch (protocol/leiosfetch.rs, channel 19): message codecs + state machines, including the dualVotesOffer/BlockTxswire shapes for prototype interop.AnyMessagevariants + channel dispatch inbehavior/mod.rs;bearer.rsunchanged.LEIOS_MIN_VERSIONandsupports_leios()gates. ChainSync/BlockFetch wire formats unchanged.leios_notify/leios_fetchstate +apply_msgarms on bothInitiatorState/ResponderState; initiator and responder sub-behaviors with new events (EbNotification/EbFetched, responderEb*Requested) and commands (FetchEb/FetchEbTxs/FetchVotes, responderProvide*).Authority / ground truth
Codecs and protocol numbers follow the gouroboros reference implementation and the
leios-prototypeblueprints (the CIP-0164 network chapter has drifted). Notable confirmed details: protocol ids 18/19, NtN v15, indefinite-length bitmaps, dualBlockTxsshapes, andLast=7/Next=8range-tag ordering (opposite of the CIP — flagged in comments).Testing
cargo test -p pallas-network2 --all-features— 84 lib + 6 integration tests pass (codec round-trips incl. indefinite bitmap & both BlockTxs shapes; state transitions; a synchronous end-to-end flow: handshake → v15 → notify offer → fetch →EbFetched).cargo clippy -p pallas-network2 --all-targets --all-features— clean.pallascrate builds with thenetwork2feature.Caveats / follow-ups
BlockRangeRequest/Next/Last) is encoded for forward-compat but not surfaced (not yet live in the prototype).LeiosVotes(ch 20), structural EB/vote/cert decoding (→pallas-primitives).🤖 Generated with Claude Code
Summary by CodeRabbit