Summary

Harden WalletSeed, KeyPair, and address-related types in the Midnight node ledger by implementing redacted Debug, removing Copy (required for ZeroizeOnDrop), adding Zeroize/ZeroizeOnDrop for automatic secret material cleanup, removing unused Clone from Keypair, replacing index-based panics in add_addresses with zip iteration, and enforcing input-size validation in try_from_lazy_hex — addressing Least Authority audit Suggestion 3 (A2).

🎫 Ticket 📐 Engineering

Motivation

The Least Authority Node DIFF audit identified informational findings in ledger/helpers/src/versions/common/types.rs where cryptographic types derive traits that expose secret material. WalletSeed derives Debug, Clone, Copy, Hash, PartialEq, and Eq, enabling accidental logging of seed bytes, invisible copies via Copy, and non-constant-time comparisons. KeyPair derives Clone, encouraging copies of secret key material. Address generation code uses unchecked indexing that can panic, and try_from_lazy_hex allocates memory from untrusted input before validating size.

Key technical insight: ZeroizeOnDrop generates a Drop impl, and Rust prohibits Copy + Drop. Therefore Copy removal is a technical requirement for zeroize support, not merely a style choice. Hash/PartialEq/Eq must remain because WalletSeed serves as a HashMap key in LedgerContext.

Changes

• WalletSeed type — Removed Copy; added Zeroize + ZeroizeOnDrop via the zeroize crate; implemented redacted fmt::Debug (prints WalletSeed(Medium(***)) instead of raw bytes)
• Keypair type — Removed unused Clone derivation (zero callers — pure parsing wrapper)
• Address generation — Rewrote add_addresses to use zip with &[MaintenanceCounter], eliminating unchecked indexing panics
• Hex decoding — Added pre-validation of hex string length in try_from_lazy_hex before hex::decode allocation
• Copy→Clone fixups — ~100 call sites across ledger/helpers and util/toolkit updated to use explicit .clone() where WalletSeed previously relied on implicit Copy (see scope note below)
• Tests — Added tests for redacted Debug output and hex length validation; all existing tests pass

Note

The Least Authority finding has two related intents: (1) zeroize secret material on drop, and (2) minimize the number of plaintext copies of WalletSeed that coexist in memory. This PR fully addresses (1) — every clone is now independently zeroized via ZeroizeOnDrop — but only partially addresses (2). Removing Copy makes each duplication explicit (~100 .clone() insertions across ledger/helpers and util/toolkit), but the resolution preserves the original duplication count rather than reducing it; the implicit bitwise copies are now explicit owned buffers, all of which get zeroized.

A fuller fix would convert BuildInput / BuildOutput / UtxoOutputInfo / wallet_from_seed and similar APIs to borrow &WalletSeed instead of owning it, so the seed lives in one canonical location for the duration of tx construction. That refactor requires threading lifetimes through trait objects (Box<dyn BuildInput<DefaultDB>>, etc.) and is deferred as follow-up scope.

📌 Submission Checklist

• Changes are backward-compatible (or flagged if breaking)
• Pull request description explains why the change is needed
• Self-reviewed the diff
• I have included a change file, or skipped for this reason: code-quality improvement, no functional behavior change
• If the changes introduce a new feature, I have bumped the node minor version
• Update documentation (if relevant)
• No new todos introduced

🔱 Fork Strategy

• Node Runtime Update
• Node Client Update
• Other
• N/A

🗹 TODO before merging

• Ready for review