feat: make memo size generic via MemoSize trait

[QuantumExplorer]

Summary

• Introduces a MemoSize trait and generic TransmittedNoteCiphertext<M: MemoSize> to allow variable memo sizes instead of the hardcoded 512-byte Zcash memo
• Makes Action, Bundle, Builder, and note encryption types generic over MemoSize, with ZcashMemo as the default for backwards compatibility
• Switches zcash_note_encryption dependency to the dashpay fork that supports generic memo sizes
• Re-exports zcash_note_encryption from the crate root for downstream consumers

Changes

• New src/memo.rs module with MemoSize trait and ZcashMemo default implementation
• Action<A> → Action<A, M: MemoSize = ZcashMemo>, Bundle and Builder similarly parameterized
• TransmittedNoteCiphertext generic over memo size affecting enc_ciphertext length
• Updated note encryption/decryption (CompactAction, OrchardDomain) for generic memo support
• PCZT parse/extract updated to work with generic memo sizes

Test plan

• cargo test --verbose passes
• cargo clippy -- -D warnings clean
• Downstream crates using default ZcashMemo continue to work without changes

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features
  • Added MemoSize abstraction to support protocol-specific memos (e.g., Zcash 512‑byte, Dash 36‑byte)
  • End-to-end memo support: actions, bundles, note encryption, and PCZT/Orchard flows now accept configurable memo types
  • Exposes compact-note constant for memo-aware encoding and parsing

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a MemoSize abstraction and threads a generic memo parameter M through note encryption, Action, Bundle, Builder, PCZT, and related modules, replacing fixed 512-byte memos with trait-driven memo types (e.g., ZcashMemo, DashMemo).

Changes

Cohort / File(s)	Summary
Dependency & crate root Cargo.toml, src/lib.rs	Switched zcash_note_encryption to a git source with a pinned rev; added pub mod memo and re-exported zcash_note_encryption.
Memo abstraction src/memo.rs	New MemoSize trait, ZcashMemo and DashMemo implementations, COMPACT_NOTE_SIZE constant, and helpers for memo/NoteBytes types.
Note ciphertext & types src/note.rs, src/note_encryption.rs	Made TransmittedNoteCiphertext and Orchard domain generic over M: MemoSize; added from_parts constructors, PhantomData, compact/plaintext/memo splitting, and memo-aware associated types.
Action & PCZT public types src/action.rs, src/pczt.rs, src/pczt/parse.rs	Action, Output, and Bundle (PCZT) are now generic ...<M: MemoSize = ZcashMemo>; constructors, accessors, parse logic, and tests updated to use TransmittedNoteCiphertext<M> and NoteBytes helpers.
Bundle core & utilities src/bundle.rs, src/bundle/batch.rs, src/bundle/commitments.rs	Generalized Bundle<T,V,M> and related impls to thread M; adjusted actions, decryption/recovery return memo type M::Memo; replaced hardcoded ciphertext slice indices with dynamic offsets using COMPACT_NOTE_SIZE and AEAD tag computations.
Builder & construction pipeline src/builder.rs	Made Builder, OutputInfo, ActionInfo, UnauthorizedBundle, and build paths generic over M; memo fields now M::Memo; dummy/empty memos use M::empty_memo().
PCZT flow & tooling src/pczt/io_finalizer.rs, src/pczt/prover.rs, src/pczt/signer.rs, src/pczt/updater.rs, src/pczt/tx_extractor.rs, src/pczt/verify.rs, src/pczt/tx_extractor.rs	Threaded M: MemoSize through impl blocks, extraction, binding, finalization, proving, signing, updating, and verification code; adjusted signatures and Fn bounds to accept Action<M>/Bundle<...,M>.
Tests & misc src/* (tests, helpers)	Updated tests and scaffolding to construct memo-aware types (use NoteBytesData, TransmittedNoteCiphertext::from_parts, and explicit ZcashMemo where needed).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 I hopped through bytes both short and grand,
Grafted a trait to each orchard hand.
From Zcash fields to Dash's small note,
M guides the memos wherever they float —
One carrot-coded crate, now cross-protocol planned. 🥕

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'feat: make memo size generic via MemoSize trait' directly and accurately summarizes the main change: introducing a MemoSize trait to parameterize memo sizes across the codebase, making the previously hardcoded memo size (512 bytes for Zcash) generic and configurable.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feat/makeMemoGeneric

---

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]

Actionable comments posted: 2

🧹 Nitpick comments (1)

src/action.rs (1)

159-163: Extract duplicated dummy ciphertext setup into a local helper.

Both generators build the same placeholder TransmittedNoteCiphertext; a helper avoids drift when ciphertext shape changes again.

♻️ Proposed refactor

+    fn dummy_encrypted_note() -> TransmittedNoteCiphertext {
+        TransmittedNoteCiphertext::from_parts(
+            [0u8; 32],
+            NoteBytesData([0u8; 580]),
+            [0u8; 80],
+        )
+    }
+
     prop_compose! {
@@
-            let encrypted_note = TransmittedNoteCiphertext::from_parts(
-                [0u8; 32],
-                NoteBytesData([0u8; 580]),
-                [0u8; 80],
-            );
+            let encrypted_note = dummy_encrypted_note();
@@
-            let encrypted_note = TransmittedNoteCiphertext::from_parts(
-                [0u8; 32],
-                NoteBytesData([0u8; 580]),
-                [0u8; 80],
-            );
+            let encrypted_note = dummy_encrypted_note();

Also applies to: 191-195

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/action.rs` around lines 159 - 163, Duplicate construction of the
placeholder TransmittedNoteCiphertext via TransmittedNoteCiphertext::from_parts
and NoteBytesData([0u8; ...]) should be extracted into a small local helper
(e.g., fn make_dummy_transmitted_note_ciphertext() -> TransmittedNoteCiphertext)
that returns the same initialized value; replace both call sites currently
creating the [0u8; 32], NoteBytesData([0u8; 580]), [0u8; 80] triple with a call
to that helper so future changes to the ciphertext shape only need to be updated
in one place.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Cargo.toml`:
- Line 45: Cargo.toml currently lists the dependency zcash_note_encryption as a
git-only dependency which prevents publishing; update the crate metadata to
either mark the crate as non-publishable by adding publish = false in the
crate’s Cargo.toml, or replace the git-only entry for zcash_note_encryption with
Cargo’s multiple-location syntax by adding a proper version = "X.Y.Z" alongside
git and rev (or alternatively publish the forked zcash_note_encryption to
crates.io and reference that version), ensuring the dependency line for
zcash_note_encryption is changed accordingly.

In `@src/pczt/parse.rs`:
- Around line 231-241: The parser currently hardcodes Zcash memo size by calling
NoteBytes::from_slice without memo-specific type args, causing
NoteBytes::from_slice to default to ZcashMemo and fail for other sizes; change
the types to be generic over a MemoSize parameter (e.g., make Output<M:
MemoSize> and propagate M through Action and related types), update the parse
method signature to accept M and use the memo-aware instantiation when parsing
(call NoteBytes and TransmittedNoteCiphertext parameterized by M's
NoteCiphertextBytes / TransmittedNoteCiphertext<M>), and replace the current
error mapping (ParseError::InvalidEncCiphertext) usage with the same error but
for the memo-parameterized parsing path so non-default memo sizes parse
correctly.

---

Nitpick comments:
In `@src/action.rs`:
- Around line 159-163: Duplicate construction of the placeholder
TransmittedNoteCiphertext via TransmittedNoteCiphertext::from_parts and
NoteBytesData([0u8; ...]) should be extracted into a small local helper (e.g.,
fn make_dummy_transmitted_note_ciphertext() -> TransmittedNoteCiphertext) that
returns the same initialized value; replace both call sites currently creating
the [0u8; 32], NoteBytesData([0u8; 580]), [0u8; 80] triple with a call to that
helper so future changes to the ciphertext shape only need to be updated in one
place.

---

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 17f835d and 41c8f71.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (12)

• Cargo.toml
• src/action.rs
• src/builder.rs
• src/bundle.rs
• src/bundle/batch.rs
• src/bundle/commitments.rs
• src/lib.rs
• src/memo.rs
• src/note.rs
• src/note_encryption.rs
• src/pczt/parse.rs
• src/pczt/tx_extractor.rs

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -e
echo "Check publish policy (missing key means publish allowed by default):"
rg -n '^\s*publish\s*=' Cargo.toml || echo "No explicit publish setting found"

echo ""
echo "Check runtime git dependencies:"
rg -n '^\s*zcash_note_encryption\s*=\s*\{[^}]*git\s*=' Cargo.toml || echo "No git dependency found"

echo ""
echo "Show the full zcash_note_encryption entry in dependencies:"
rg -A 1 '^\s*zcash_note_encryption' Cargo.toml | head -20

Repository: dashpay/orchard

Length of output: 839

---

Release blocker: git-only dependency in publishable crate.

Line 45 has zcash_note_encryption as a git-only dependency in [dependencies], and no publish = false is set. Crates with git-only dependencies cannot be published to crates.io. Either:

• Set publish = false if this is a workspace-only crate, OR
• Use Cargo's multiple-location syntax: version = "X.Y.Z" with git and rev, OR
• Publish the forked crate to crates.io first

Reference: https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Cargo.toml` at line 45, Cargo.toml currently lists the dependency
zcash_note_encryption as a git-only dependency which prevents publishing; update
the crate metadata to either mark the crate as non-publishable by adding publish
= false in the crate’s Cargo.toml, or replace the git-only entry for
zcash_note_encryption with Cargo’s multiple-location syntax by adding a proper
version = "X.Y.Z" alongside git and rev (or alternatively publish the forked
zcash_note_encryption to crates.io and reference that version), ensuring the
dependency line for zcash_note_encryption is changed accordingly.

[coderabbitai]

🧹 Nitpick comments (1)

src/builder.rs (1)

553-708: Add one non-default memo-size test on the builder path.

The generic API is wired well, but this critical path should include a regression test that instantiates Builder<DashMemo> (or another non-default MemoSize) and exercises add_output + build_for_pczt (and ideally full signing flow) to prevent future fixed-size regressions.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/builder.rs` around lines 553 - 708, The test-suite is missing a
regression test for non-default memo sizes; add a unit/integration test that
constructs a Builder<DashMemo> (using Builder::new or the same constructor used
elsewhere), calls add_output with a DashMemo instance, and then calls
build_for_pczt to exercise the PCZT build path (optionally following with the
signing flow: create_proof/apply_signatures or equivalent functions used
elsewhere) to ensure Builder<M>, add_output, and build_for_pczt work for
non-default MemoSize; assert the build succeeds and the returned
crate::pczt::Bundle contains the expected action count, flags, and value_sum to
prevent regressions.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/builder.rs`:
- Around line 553-708: The test-suite is missing a regression test for
non-default memo sizes; add a unit/integration test that constructs a
Builder<DashMemo> (using Builder::new or the same constructor used elsewhere),
calls add_output with a DashMemo instance, and then calls build_for_pczt to
exercise the PCZT build path (optionally following with the signing flow:
create_proof/apply_signatures or equivalent functions used elsewhere) to ensure
Builder<M>, add_output, and build_for_pczt work for non-default MemoSize; assert
the build succeeds and the returned crate::pczt::Bundle contains the expected
action count, flags, and value_sum to prevent regressions.

---

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 41c8f71 and 5a96ca4.

📒 Files selected for processing (9)

• src/builder.rs
• src/pczt.rs
• src/pczt/io_finalizer.rs
• src/pczt/parse.rs
• src/pczt/prover.rs
• src/pczt/signer.rs
• src/pczt/tx_extractor.rs
• src/pczt/updater.rs
• src/pczt/verify.rs