feat(network): prep for van Rossem hard fork (NodeToClient v17–v23)#747
Conversation
Catches the Node-to-Client handshake up to NodeToClientV_23 (the cardano-node 10.7.x release line targeted for the van Rossem hard fork) and aligns the LocalStateQuery and LocalTxSubmission codecs with the matching upstream additions. * handshake/n2c: add PROTOCOL_V17..V23 constants, extend v1_and_above and v10_and_above so newer cardano-node peers negotiate above V_16. * localstate BlockQuery: add GetPoolDistr2 (v21), GetStakeDistribution2 (v21), GetDRepsDelegations (v23), and a new GetLedgerPeerSnapshot(kind) variant that encodes the v15+ two-element form of tag 34 (legacy GetBigLedgerPeerSnapshot is preserved for older peers and disambiguated by array length on decode). Replace the unreachable!() catch-all with a proper decode error. * localtxsubmission ConwayLedgerFailure: replace the U8(u8) backward-compat placeholder at tag 8 with the real WithdrawalsMissingAccounts variant the ledger 10.7 line introduced, and add IncompleteWithdrawals at tag 9. * pallas-hardano haskell_display: cover the two new ConwayLedgerFailure variants in the Haskell-style formatter. LedgerPeerSnapshot stays typed as AnyCbor, so the v22 SRV-record additions and v23 encoding change pass through opaquely. pallas-primitives and pallas-traverse need no changes for this hard fork (intra-Conway, no new transaction shape, no PlutusV4 — that lands in Dijkstra). Wire shapes are derived from the upstream Haskell encoders (encodeShelleyQuery, ConwayLedgerPredFailure EncCBOR). Roundtrip and wire-shape unit tests assert codec self-consistency; byte-for-byte agreement with a live 10.7 node should be confirmed at integration time. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe PR extends Cardano protocol support by adding withdrawal failure variants to ChangesWithdrawal Variants & Display
Ledger Queries & Protocol Versions
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Review rate limit: 0/1 reviews remaining, refill in 29 minutes and 18 seconds.Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pallas-network/src/miniprotocols/localstate/queries_v16/codec.rs`:
- Around line 903-965: Add explicit wire-shape assertions for the missing tags:
add a test in test_post_v16_block_queries_wire_shape that encodes
BlockQuery::GetPoolDistr2 and asserts the hex equals the exact CBOR bytes for
tag 36 (match the style used for 34/37, expected "811824"), and add another
assertion that encodes
BlockQuery::GetDRepsDelegations(TaggedSet::from(BTreeSet::new())) and compares
it to the exact expected hex for tag 39 with an empty TaggedSet payload (follow
the same pattern as the GetLedgerPeerSnapshot and GetStakeDistribution2
assertions to pin the discriminant). Ensure you reference
BlockQuery::GetPoolDistr2, BlockQuery::GetDRepsDelegations and TaggedSet when
adding these two assertions.
In `@pallas-network/src/miniprotocols/localstate/queries_v16/mod.rs`:
- Around line 77-90: The module still exposes legacy helper functions
(get_big_ledger_snapshot, get_pool_distr, get_stake_distribution) but the enum
now has new variants GetLedgerPeerSnapshot(LedgerPeerSnapshotKind),
GetPoolDistr2(SMaybe<Pools>), GetStakeDistribution2 and
GetDRepsDelegations(TaggedSet<DRep>) that callers need; update the public API in
queries_v16 to either (a) add new wrapper constructors named e.g.
get_ledger_peer_snapshot(kind: LedgerPeerSnapshotKind),
get_pool_distr_v2(payload: SMaybe<Pools>), get_stake_distribution_v2(),
get_dreps_delegations(set: TaggedSet<DRep>) that build
BlockQuery::GetLedgerPeerSnapshot / GetPoolDistr2 / GetStakeDistribution2 /
GetDRepsDelegations, or (b) rewire the existing helpers (get_big_ledger_snapshot
-> GetLedgerPeerSnapshot with LedgerPeerSnapshotKind::Big, get_pool_distr -> map
to GetPoolDistr2, get_stake_distribution -> GetStakeDistribution2) and mark old
names deprecated; implement one of these approaches so callers use the new
protocol variants and ensure the helper signatures use the correct payload types
(LedgerPeerSnapshotKind, SMaybe<Pools>, TaggedSet<DRep>).
🪄 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: 433e8944-2094-4bc4-b05d-f4b963da205f
📒 Files selected for processing (5)
pallas-hardano/src/display/haskell_display.rspallas-network/src/miniprotocols/handshake/n2c.rspallas-network/src/miniprotocols/localstate/queries_v16/codec.rspallas-network/src/miniprotocols/localstate/queries_v16/mod.rspallas-network/src/miniprotocols/localtxsubmission/protocol.rs
* fmt: rewrap test imports / call chains to satisfy `cargo fmt --check` (CI Lints job was failing on the previously hand-formatted blocks). * test: pin the wire shape of `GetPoolDistr2` (tag 36) and `GetDRepsDelegations` (tag 39) explicitly, so an off-by-one tag in encode/decode can no longer slip past the suite by virtue of using the same wrong constant on both sides. * api: add the four post-V_16 query helpers that mirror the existing `block_query_*!` macro pattern: `get_ledger_peer_snapshot(kind)`, `get_pool_distr_v2(maybe_pools)`, `get_stake_distribution_v2()`, and `get_dreps_delegations(set)`. Without these wrappers callers hitting a v23 node still had to construct the new `BlockQuery` variants by hand. Legacy helpers (`get_big_ledger_snapshot`, `get_pool_distr`, `get_stake_distribution`) are kept untouched for back-compat with pre-v15/v21 peers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pallas-network2 ships its own N2C handshake table (the crate is a from-scratch P2P-focused rewrite, not a re-export of pallas-network), and that table was byte-for-byte the same one pallas-network had before this branch — capped at PROTOCOL_V16. Without the mirror, a pallas-network2 client talking to a cardano-node 10.7.x peer would negotiate down to v16 instead of v23. Add PROTOCOL_V17..PROTOCOL_V23 (32785..32791) and extend v1_and_above and v10_and_above to advertise them, matching the pallas-network change in this PR. The semantic-layer changes from this PR (new BlockQuery variants, ConwayLedgerFailure tag-8/9 additions) do not apply here: pallas-network2 has no LocalStateQuery and no LocalTxSubmission modules — only the multiplexer/transport plus chainsync, blockfetch, keepalive, peersharing, txsubmission (n2n flavor). The N2N table is also stale (caps at v14 while upstream is at v16 with SRV / Peras support) but that gap is from Plomin / Peras work, not van Rossem, so it stays out of scope here. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Backports the handshake half of #747 to the 0.18.x line so downstream consumers can negotiate NodeToClient sessions with cardano-node 10.x peers, including the 10.7.x release that targets the van Rossem hard fork (protocol version 11). The remaining changes from #747 (queries_v16 additions, the new ConwayLedgerFailure tags, and the pallas-network2 / pallas-hardano counterparts) depend on modules introduced after this branch diverged and are intentionally out of scope.
Summary
BlockQueryvariants introduced between v17 and v23:GetPoolDistr2,GetStakeDistribution2,GetDRepsDelegations, andGetLedgerPeerSnapshot(kind)(with legacy single-elementGetBigLedgerPeerSnapshotpreserved and disambiguated by array length on decode).ConwayLedgerFailurewith the cardano-ledger 10.7 line: replaces theU8(u8)backward-compat placeholder at tag 8 with the realWithdrawalsMissingAccountsvariant and addsIncompleteWithdrawalsat tag 9.pallas-hardanoHaskell-display formatter updated to match.Van Rossem is intra-Conway, so this is a network-stack-only change.
pallas-primitivesandpallas-traverseneed nothing —CostModels(Vec<i64>+ extensibleunknown) absorbs longer V1/V2/V3 cost vectors automatically, the Conway CDDL already acceptsmajor_protocol_version = 11, and Plutus V4 is a Dijkstra-era concern, not van Rossem.LedgerPeerSnapshotstays typed asAnyCbor, so the v22 SRV-record additions and the v23 snapshot CBOR encoding change pass through opaquely.Wire shapes were derived from the upstream Haskell encoders (
encodeShelleyQuery,ConwayLedgerPredFailureEncCBOR).Test plan
cargo check --workspacecleancargo test -p pallas-network --lib(240 passed)cargo test -p pallas-hardano --lib(13 passed)test_post_v16_block_queries_roundtrip— encode/decode self-consistency for the four newBlockQueryvariantstest_post_v16_block_queries_wire_shape— asserts the exact CBOR envelope (e.g.[34, 0]=82182200,[37]=811825) against the upstreamencodeShelleyQueryshape🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests