fix(validate): handle non-conway utxos in conway txs

[scarmuega]

Summary by CodeRabbit

• Refactor
  • Improved validation logic to support additional output types, significantly enhancing system robustness and reliability throughout the validation process.
  • Optimized witness processing and evaluation order for better performance and correctness.
  • Enhanced validation handling to ensure comprehensive and accurate processing across various scenarios and conditions.

[coderabbitai]

📝 Walkthrough

Walkthrough

This pull request refines witness validation logic in Conway validation by improving address extraction to support multiple output types (Conway through Byron), reordering tx_hash extraction, and enhancing the witness verification flow with explicit conditional handling and remaining witness checks.

Changes

Cohort / File(s)	Summary
Witness Validation Logic pallas-validate/src/phase1/conway.rs	Expanded address extraction in check_vkey_input_wits to handle Conway, Alonzo/Mary/Allegra/Shelley/Babbage, and Byron outputs. Added conditional payment-part handling, refined vk_wits iteration with iter_mut and direct vkey hash computation, and integrated check_remaining_vk_wits for non-covered witness validation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A witness hops through outputs grand,
From Conway down through Byron's land,
Each address found, each payment weighed,
Verification marks are laid,
Now every signature is read! 🔐

🚥 Pre-merge checks | ✅ 2 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Merge Conflict Detection	⚠️ Warning	❌ Merge conflicts detected (23 files): ⚔️ pallas-addresses/Cargo.toml (content) ⚔️ pallas-codec/Cargo.toml (content) ⚔️ pallas-configs/Cargo.toml (content) ⚔️ pallas-configs/README.md (content) ⚔️ pallas-configs/src/alonzo.rs (content) ⚔️ pallas-configs/src/cost_models.rs (content) ⚔️ pallas-configs/src/shelley.rs (content) ⚔️ pallas-crypto/Cargo.toml (content) ⚔️ pallas-hardano/Cargo.toml (content) ⚔️ pallas-math/Cargo.toml (content) ⚔️ pallas-math/src/math_dashu.rs (content) ⚔️ pallas-network/Cargo.toml (content) ⚔️ pallas-network/src/miniprotocols/localmsgsubmission/codec.rs (content) ⚔️ pallas-network/src/miniprotocols/localmsgsubmission/protocol.rs (content) ⚔️ pallas-network/tests/protocols.rs (content) ⚔️ pallas-primitives/Cargo.toml (content) ⚔️ pallas-traverse/Cargo.toml (content) ⚔️ pallas-traverse/src/input.rs (content) ⚔️ pallas-txbuilder/Cargo.toml (content) ⚔️ pallas-utxorpc/Cargo.toml (content) ⚔️ pallas-validate/Cargo.toml (content) ⚔️ pallas-validate/src/phase1/conway.rs (content) ⚔️ pallas/Cargo.toml (content) These conflicts must be resolved before merging into main.	Resolve conflicts locally and push changes to this branch.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title clearly and specifically describes the main change: enabling handling of non-Conway UTXOs in Conway transactions, which aligns with the expanded address extraction logic in the changeset.

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

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/pre-conway-utxos-on-conway-tx

⚔️ Resolve merge conflicts (beta)

• Auto-commit resolved conflicts to branch fix/pre-conway-utxos-on-conway-tx
• Create stacked PR with resolved conflicts
• Post resolved changes as copyable diffs in a comment

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Caution

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

⚠️ Outside diff range comments (2)

pallas-validate/src/phase1/conway.rs (2)

296-329: ⚠️ Potential issue | 🟠 Major

Byron UTxO inputs will always fail with InputDecoding.

The comment on line 316 says "Byron outputs are handled separately below," but the else branch at lines 327–328 simply returns Err(PostAlonzo(InputDecoding)). There is no separate Byron handling — any Conway transaction spending a Byron UTxO will be rejected.

If Byron inputs are intentionally unsupported (e.g., bootstrap witnesses aren't implemented), remove the misleading comment and add an explicit note. If they should be supported, Byron inputs need dedicated handling (bootstrap witness verification or a skip-and-delegate path).

Proposed fix (if Byron is intentionally unsupported)

                 } else {
-                    // Byron outputs are handled separately below
+                    // Byron outputs are not currently supported in validation;
+                    // they would require bootstrap witness verification.
                     None
                 };

---

1356-1370: ⚠️ Potential issue | 🔴 Critical

check_remaining_vk_wits only validates the first uncovered witness.

The return Ok(()) on line 1363 causes the function to exit after verifying the first uncovered witness, skipping signature verification for all subsequent uncovered witnesses. Every uncovered witness should be checked.

Proposed fix

 fn check_remaining_vk_wits(
     wits: &mut [(bool, VKeyWitness)],
     data_to_verify: &[u8],
 ) -> ValidationResult {
     for (covered, vkey_wit) in wits {
         if !*covered {
-            if verify_signature(vkey_wit, data_to_verify) {
-                return Ok(());
-            } else {
+            if !verify_signature(vkey_wit, data_to_verify) {
                 return Err(PostAlonzo(VKWrongSignature));
             }
         }
     }
     Ok(())
 }