Skip to content

chore: fix clippy warnings across workspace#748

Merged
scarmuega merged 1 commit into
mainfrom
chore/fix-clippy-warnings
May 4, 2026
Merged

chore: fix clippy warnings across workspace#748
scarmuega merged 1 commit into
mainfrom
chore/fix-clippy-warnings

Conversation

@scarmuega
Copy link
Copy Markdown
Member

@scarmuega scarmuega commented May 4, 2026

Summary

  • Resolves all clippy warnings under cargo clippy --all-targets --all-features (only an external chumsky v1.0.0-alpha.7 future-incompat note remains, untouchable from this repo).
  • Lib clippy as CI runs it (cargo clippy -- -D warnings) is green.
  • Notable refactors:
    • pallas-validate/src/phase1/alonzo.rs: collateral check now extracts the shared lovelace comparison across Coin and Multiasset arms (eliminates collapsible_match).
    • pallas-primitives/src/conway/script_data.rs: const LazyLock<…>static LazyLock<…> (interior mutability lint).
    • pallas-validate/tests/common.rs: existing tuple type aliases (BabbageTxOutInfo, ConwayCollateralInfoMut, …) exposed as pub; babbage.rs / conway.rs now use them instead of duplicating 4–5-field tuple types inline.
    • assert!(false, msg)panic!(msg) in pallas-math and validate tests.
    • Misc small fixes: sort_bysort_by_key w/ Reverse, removed needless returns, removed redundant .cloned(), useless Error::from conversion, redundant &-then-deref, collapsed nested matches into matches!, dropped a -> () return.

Test plan

  • cargo clippy --all-targets --all-features — clean (modulo external chumsky note)
  • cargo clippy -- -D warnings (CI invocation) — passes
  • cargo fmt --all -- --check — clean
  • cargo test -p pallas-math --lib — 19 passed
  • cargo test -p pallas-network --lib — 240 passed
  • cargo test -p pallas-validate --test babbage --test conway — all pass
  • cargo check --all-targets --all-features — builds

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Refactor

    • Simplified code structure across validation, encoding, and networking modules for improved maintainability
    • Optimized version handling and data processing in network communications
    • Improved type organization and error handling patterns
  • Tests

    • Enhanced test assertions and error reporting
    • Updated test data initialization and structure for better performance

Resolves all clippy warnings under `cargo clippy --all-targets
--all-features` (modulo an external `chumsky` future-incompat note),
covering the lib invocation CI runs (`cargo clippy -- -D warnings`)
and test targets.

Notable changes:
- alonzo phase1 collateral check refactored to share the lovelace
  comparison across `Coin` and `Multiasset`
- script_data tests: `const LazyLock` → `static LazyLock` to satisfy
  the interior-mutability lint
- exposed existing tuple type aliases in tests/common.rs as `pub` and
  used them in babbage.rs / conway.rs in place of inline tuples
- replaced `assert!(false, msg)` with `panic!(msg)` in validate tests
  and pallas-math

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 4, 2026

📝 Walkthrough

Walkthrough

This PR applies targeted refactoring and cleanup across multiple crates: core validation logic is simplified via pattern-matching consolidation and matches! macro adoption; test infrastructure is modernized through type-alias exports and adoption of formal types instead of tuples; and minor syntax improvements are made across test helpers and codec utilities.

Changes

Core Validation Logic Refactoring

Layer / File(s) Summary
Collateral Validation
pallas-validate/src/phase1/alonzo.rs
check_collaterals_assets refactors asset value pattern-matching to destructure both Value::Coin and Value::Multiasset together for min-lovelace comparison, then separately verifies that multiasset collateral contains no non-lovelace assets.
Script Context & Plutus Data
pallas-validate/src/phase2/script_context.rs
get_data_info removes intermediate .cloned() iterator step, instead cloning only within the mapping expression for more direct iteration.
Script Execution
pallas-validate/src/phase2/tx.rs
execute_script simplifies script decoding to direct decode(script_bytes)? and refactors ScriptContext::V3 success evaluation to use a single matches! pattern for Ok(Term::Constant(Constant::Unit)).

Test Infrastructure, Type Exports & Test Migrations

Layer / File(s) Summary
Type Alias Exports
pallas-validate/tests/common.rs
Ten internal tuple type aliases covering per-era transaction output/ref-input info and collateral info variants for Babbage and Conway are made publicly exported; no functional changes to helper logic.
Test Type Usage & Assertion Style
pallas-validate/tests/babbage.rs, pallas-validate/tests/conway.rs
Successful tests adopt BabbageTxOutInfo and BabbageCollateralInfo / ConwayTxOutInfoMut typed aliases instead of raw tuple element specs; negative tests replace assert!(false, ...) with panic!(...) while preserving expected error-variant matching.
Minor Syntax & Helper Cleanups
pallas-codec/src/lib.rs, pallas-math/src/math.rs, pallas-network/src/miniprotocols/handshake/server.rs, pallas-network/src/miniprotocols/localstate/queries_v16/codec.rs, pallas-primitives/src/conway/script_data.rs
Test helpers and utility code simplified: roundtrip_codec signature compressed; golden_tests refactored to use local threshold variable and panic! instead of print-then-assert; version sorting changed to sort_by_key with Reverse; codec match arms converted from explicit return to tail expressions; COST_MODEL_PLUTUS_V1 and TEST_VECTORS changed from const to static with LazyLock.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 Hops through validation code with glee,
Type aliases now run wild and free,
Panic replaces assert's old way,
Matches! macros brighten the day,
Cleaner, swifter—refactored with care!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 15.79% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'chore: fix clippy warnings across workspace' accurately and concisely summarizes the main objective of the pull request, which is to resolve Clippy linting warnings throughout the codebase.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/fix-clippy-warnings

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
Review rate limit: 0/1 reviews remaining, refill in 60 minutes.

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
pallas-validate/tests/conway.rs (1)

879-890: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Wire the mutated collateral params into env.

conway_prot_params is updated here, but env.prot_params still comes from a fresh mk_mainnet_params_epoch_380() value. As written, this test never exercises the reduced-collateral scenario it describes.

Suggested fix
-        let env: Environment = Environment {
-            prot_params: MultiEraProtocolParameters::Conway(mk_mainnet_params_epoch_380()),
-            prot_magic: 764824073,
-            block_slot: 149807950,
-            network_id: 1,
-            acnt: Some(acnt),
-        };
         let mut conway_prot_params: ConwayProtParams = mk_mainnet_params_epoch_380();
         conway_prot_params.collateral_percentage = 10;
+        let env: Environment = Environment {
+            prot_params: MultiEraProtocolParameters::Conway(conway_prot_params),
+            prot_magic: 764824073,
+            block_slot: 149807950,
+            network_id: 1,
+            acnt: Some(acnt),
+        };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pallas-validate/tests/conway.rs` around lines 879 - 890, The test mutates
conway_prot_params.collateral_percentage but never wires that into
env.prot_params, so the reduced-collateral scenario is not exercised; after
updating conway_prot_params, assign it into env.prot_params (e.g. set
env.prot_params = MultiEraProtocolParameters::Conway(conway_prot_params) or
construct Environment using conway_prot_params) before calling validate_txs so
the validate_txs call uses the mutated ConwayProtParams.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@pallas-validate/tests/conway.rs`:
- Around line 879-890: The test mutates conway_prot_params.collateral_percentage
but never wires that into env.prot_params, so the reduced-collateral scenario is
not exercised; after updating conway_prot_params, assign it into env.prot_params
(e.g. set env.prot_params =
MultiEraProtocolParameters::Conway(conway_prot_params) or construct Environment
using conway_prot_params) before calling validate_txs so the validate_txs call
uses the mutated ConwayProtParams.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a213e8ac-b5c4-459b-a6b2-2566928bed35

📥 Commits

Reviewing files that changed from the base of the PR and between 3781f63 and feb4dc2.

📒 Files selected for processing (11)
  • pallas-codec/src/lib.rs
  • pallas-math/src/math.rs
  • pallas-network/src/miniprotocols/handshake/server.rs
  • pallas-network/src/miniprotocols/localstate/queries_v16/codec.rs
  • pallas-primitives/src/conway/script_data.rs
  • pallas-validate/src/phase1/alonzo.rs
  • pallas-validate/src/phase2/script_context.rs
  • pallas-validate/src/phase2/tx.rs
  • pallas-validate/tests/babbage.rs
  • pallas-validate/tests/common.rs
  • pallas-validate/tests/conway.rs
💤 Files with no reviewable changes (1)
  • pallas-validate/src/phase2/script_context.rs

@scarmuega scarmuega merged commit a9e538a into main May 4, 2026
15 checks passed
@scarmuega scarmuega deleted the chore/fix-clippy-warnings branch May 4, 2026 11: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