Skip to content

fix(ci): restore green CI under current stable clippy and cddl 0.10.7 - #795

Merged
scarmuega merged 3 commits into
mainfrom
fix/ci-drift-clippy-cddl
Aug 25, 2026
Merged

fix(ci): restore green CI under current stable clippy and cddl 0.10.7#795
scarmuega merged 3 commits into
mainfrom
fix/ci-drift-clippy-cddl

Conversation

@scarmuega

@scarmuega scarmuega commented Aug 25, 2026

Copy link
Copy Markdown
Member

CI on main is red, and has been since the last green run on 2026-07-01 (cdee91d). The workspace does not commit Cargo.lock and CI resolves dtolnay/rust-toolchain@stable, so both failures are external drift rather than anything in a PR. Reproduced on a clean checkout of origin/main with no local changes.

Surfaced while triaging #793, which only touches pallas-utxorpc/src/shared.rs and is unaffected by either failure.

1. Clippy — new lints on current stable (1.98.0)

cargo clippy --workspace --all-targets -- -D warnings fails on four for_kv_map hits in pallas-validate:

error: you seem to want to iterate on a map's keys
   --> pallas-validate/src/phase1/alonzo.rs:912:32
912 |             for (policy, _) in minted_value.iter() {

…plus the same in babbage.rs, conway.rs and shelley_ma.rs. The CI log stops there because the build bails, but more failures hide behind it — each one only becomes visible once the crate ahead of it compiles:

  • useless_borrows_in_formatting on &-prefixed format!/assert_eq!/panic! arguments in pallas-math tests and pallas-hardano's ValidityInterval display.
  • chunks_exact_to_as_chunks (new in clippy 1.98) on the constant-size chunks_exact(4) in pallas-hardano's AsIPv6. slice::as_chunks is stable since 1.88.0, which is exactly the workspace MSRV, and .0 drops the same (always empty here) remainder.

All mechanical; .keys() is already the idiom elsewhere in the same files. No behaviour change.

2. cddl 0.10.7 regressed map keys that reference a type rule

cargo test -p pallas-network -p pallas-network2 --features blueprint fails on all three OSes:

`msgLeiosBlockTxs` does not conform to CDDL:
error validating at cbor location /2: unexpected key Integer(Integer(0))

cddl 0.10.7 was published 2026-08-10, after the last green run; 0.10.6 (2026-06-29) is what that run resolved. The leios-fetch blueprint schema declares

bitmaps = { * base.word16 => base.word64 }

and 0.10.7 rejects every key of a map whose key is a named type rule rather than an inline type. Reduced repro:

schema CBOR 0.10.6 0.10.7
{ * uint => uint } {0: 1} ok ok
{ * k => uint }, k = uint {0: 1} ok unexpected key Integer(0)
{ * k => uint }, k = tstr {"a": 10} ok unexpected key Text("a")
{ * uint => v }, v = uint {0: 1} ok ok

Only the key position is affected; a named value rule still resolves. Definite- and indefinite-length maps behave identically, so this is not about our encoder. Still reproduces against cddl master as of 2026-08-25, so this is pinned to =0.10.6 rather than waiting on a patch release.

cddl is optional and reached only through the test-only blueprint feature, so the pin does not affect default builds or downstream consumers who leave the feature off. The comment in Cargo.toml records when to relax it.

Verification

Every CI job run locally on rustc/clippy 1.98.0 — the same stable CI resolves — against a freshly resolved lockfile (rm Cargo.lock):

  • cargo clippy --workspace --all-targets -- -D warnings — clean
  • cargo fmt --all --check — clean
  • cargo test --workspace — no failures
  • cargo test -p pallas-network -p pallas-network2 --features blueprint — all suites green
  • RUSTDOCFLAGS=-D warnings cargo doc --workspace --no-deps — clean
  • cargo check --workspace --all-targets with --all-features and --no-default-features — clean
  • cargo check -p pallas-primitives --no-default-features — clean

Follow-ups (not in this PR)

  • The cddl map-key regression is unreported upstream; worth an issue on anweiss/cddl.
  • Both failures reached main because nothing runs CI between merges and Cargo.lock is uncommitted. A scheduled run would catch drift closer to when it lands, instead of surprising the next contributor to open a PR.
  • connection_sustained_over_time (pallas-network2) is flaky on the macOS runner: it timed out at wait_for_intersection with Events collected: [], then passed on re-run of the same commit, having also passed on macOS one commit earlier. The 10s DEFAULT_TIMEOUT over a real loopback connection looks tight for a contended runner. Same family as 0353bf4.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Pinned the optional CDDL parser to a stable version, preventing failures when processing maps with named key types.
  • Refactor

    • Simplified internal validation, display, and mathematical formatting logic while preserving existing behavior and generated output.
    • Improved consistency and maintainability across transaction validation and data presentation without introducing user-facing changes.

Recent stable clippy releases widened two lints that the workspace now trips
under `-D warnings`:

- `for_kv_map` on the four `check_minting` loops that bind `(policy, _)` over
  `minted_value.iter()`; the same files already use `.keys()` elsewhere.
- `useless_borrows_in_formatting` on `&`-prefixed arguments to `format!`,
  `assert_eq!` and `panic!` in pallas-math tests and pallas-hardano's
  `ValidityInterval` display.

All mechanical; no behaviour change.
cddl 0.10.7 regressed map entries whose key is a type-rule reference rather
than an inline type. The leios-fetch blueprint schema uses exactly that shape

    bitmaps = { * base.word16 => base.word64 }

and 0.10.7 rejects every key in such a map as unexpected, failing
`block_txs_conforms` and `block_txs_request_conforms`. Reduced repro:

    { * uint => uint }            with {0: 1}  -> validates
    { * k => uint }  k = uint     with {0: 1}  -> "unexpected key Integer(0)"

Confirmed still broken on cddl master as of 2026-08-25. `cddl` is optional and
only pulled in by the test-only `blueprint` feature, so the pin does not reach
default builds. Relax once a release carries the fix.
@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f7bfcdd6-b975-4f3f-a04d-fc6eb1b2abb4

📥 Commits

Reviewing files that changed from the base of the PR and between 84cb57c and 35c90c7.

📒 Files selected for processing (1)
  • pallas-hardano/src/display/haskell_display.rs

Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The changes pin the optional cddl dependency, pass formatting values without references, and update minting validation loops to iterate directly over policy keys. Runtime behavior remains unchanged.

Changes

Cleanup and compatibility updates

Layer / File(s) Summary
Pin cddl compatibility version
pallas-network2/Cargo.toml
The optional cddl dependency is pinned to =0.10.6, with a comment describing a regression in later versions.
Pass formatting values directly
pallas-hardano/src/display/haskell_display.rs, pallas-math/src/math.rs
Formatting calls pass owned strings and values directly instead of references.
Iterate over minted policy keys
pallas-validate/src/phase1/alonzo.rs, pallas-validate/src/phase1/babbage.rs, pallas-validate/src/phase1/conway.rs, pallas-validate/src/phase1/shelley_ma.rs
Minting validation loops use map keys directly while preserving witness checks and error handling.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to 35c90

The PR applies localized CI compatibility fixes and pins an optional test-only dependency; no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 6 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ⚠️ Warning The title correctly identifies the CI and Clippy focus, but it states that the change supports cddl 0.10.7. The pull request instead pins cddl to 0.10.6 because 0.10.7 causes a regression. Update the title to state that cddl is pinned to 0.10.6 while CI is restored under current stable Clippy, for example: "fix(ci): restore green CI and pin cddl to 0.10.6".
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/ci-drift-clippy-cddl

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

clippy 1.98 adds `chunks_exact_to_as_chunks`, which fires on the constant-size
`chunks_exact(4)` in `AsIPv6`. `slice::as_chunks` is stable since 1.88.0, which
is the workspace MSRV, and `.0` drops the same (always empty here) remainder
`chunks_exact` did.
@scarmuega
scarmuega merged commit afc39b0 into main Aug 25, 2026
20 of 21 checks passed
@scarmuega
scarmuega deleted the fix/ci-drift-clippy-cddl branch August 25, 2026 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant