refactor: replace wildcard match arms with explicit enum variants#10440
refactor: replace wildcard match arms with explicit enum variants#10440zmanian wants to merge 6 commits into
Conversation
Add comprehensive unit tests for policy functions added in ZcashFoundation#10314: - count_script_push_ops: PUSHDATA1/2/4, mixed types, truncated scripts - extract_p2sh_redeemed_script: existing + edge cases - script_sig_args_expected: P2PK, multisig via script classification - are_inputs_standard: P2PKH accept/reject, wrong stack depth, non-standard spent outputs, P2SH with standard/non-standard redeemed scripts, sigops boundary, multi-input rejection - p2sh_sigop_count: P2SH/non-P2SH/multi-input summation Also adds defensive improvements: - saturating_add for sigop count arithmetic - debug_assert_eq for input/spent_output alignment - Expanded doc comments with correctness notes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove unnecessary Hash derive from NonStandardTransactionError - Move p2pkh_lock_script, p2sh_lock_script, p2pk_lock_script to module level in policy.rs as #[cfg(test)] pub(super) functions, eliminating duplication between policy::tests and tests::vectors Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace `_ =>` wildcard patterns with explicit enum variant listings across the workspace to improve safety. When new enum variants are added, the compiler will now produce errors instead of silently falling through to a default case. - 42 files modified across zebra-chain, zebra-consensus, zebra-network, zebra-rpc, zebra-state, and zebrad - Feature-gated variants (ReadResponse::TransactionId behind `indexer`, Transaction::V6 behind `nu7`/`tx_v6`) handled with #[cfg] arms - halo2::plonk::Error retains wildcard with #[allow] due to upstream #[non_exhaustive] - Pre-existing println! lint fix in zebra-state test (-> tracing::warn!) Closes ZcashFoundation#10211
|
@conradoplg This is ready for review — CI workflows need maintainer approval to run (fork PR). Could you approve the Actions runs when you get a chance? Thanks! |
…ard-match-arms # Conflicts: # zebra-network/src/peer/connection/tests/vectors.rs # zebra-rpc/src/methods.rs
|
Thanks for working on this @zmanian — the refactor aligns with #10211 and conradoplg's acknowledgment there. I did a detailed review of commit Two issues to fix and a rebase needed before we can merge: Bug 1 — Missing The non-test code correctly includes the cfg-gated V6 arm, but these test files don't:
Each needs: #[cfg(all(zcash_unstable = "nu7", feature = "tx_v6"))]
Transaction::V6 { .. } => // same body as the other armsWithout this, the build breaks under Bug 2 — Missing
Each needs: #[cfg(zcash_unstable = "zfuture")]
NetworkUpgrade::ZFuture => // same body as the other armsRebase needed The branch has 6 commits but only
Timing note: This PR touches files that overlap with other in-flight work, so we may need to coordinate merge timing. Getting the rebase and fixes done now means it's ready when the window opens. |
Motivation
Closes #10211
Wildcard match arms (
_ =>) silently swallow new enum variants, which can hide bugs when new variants are added. Replacing them with explicit variant listings ensures the compiler flags any match that needs updating.Solution
_ =>patterns across 42 files with explicit enum variant listingsReadResponse::TransactionIdbehindindexer,Transaction::V6behindnu7/tx_v6,NetworkUpgrade::ZFuturebehindzfuture) handled with#[cfg]armshalo2::plonk::Errorretains wildcard with#[allow(clippy::wildcard_enum_match_arm)]due to upstream#[non_exhaustive]indexerfeature propagation fromzebrad->zebra-rpc->zebra-stateprintln!clippy lint inzebra-state/src/service/read/tests/vectors.rs(changed totracing::warn!)Verification
cargo fmt --all -- --checkpassescargo clippy --workspace --all-targets -- -D warningspasses (default features)cargo clippy --workspace --all-targets --features indexer -- -D warningspassesAI Disclosure
Used Claude Code for initial enumeration of wildcard patterns and mechanical replacement. All changes were reviewed and verified by the contributor.