Skip to content

Shielded voting feature#22

Closed
greg0x wants to merge 32 commits into
mainfrom
shielded-vote-2.4.10
Closed

Shielded voting feature#22
greg0x wants to merge 32 commits into
mainfrom
shielded-vote-2.4.10

Conversation

@greg0x
Copy link
Copy Markdown
Collaborator

@greg0x greg0x commented Apr 15, 2026

Summary

Rebuilds the shielded voting feature from the shielded-vote branch onto MOB-1011-Synchronizer-rescanFromHeight, which is the SDK base required by zodl-ios release/3.4.0.

  • Rust FFI: 43 extern "C" functions in voting.rs — ballot construction, proof generation, share delegation, note snapshots, progress callbacks
  • Swift wrappers: VotingRustBackend (1485 lines) + VotingTypes (426 lines) + getTreeState(height:) on Synchronizer protocol
  • Dependencies: librustvoting, voting-circuits, serde — with [patch.crates-io] redirecting orchard and librustzcash to valargroup forks
  • Package.swift: binary target pointing at 0.6.0-voting xcframework release

Blocking: dependency alignment

This branch won't compile until:

  1. voting-circuits fork is rebased onto orchard 0.12 (currently 0.11)
  2. valargroup/librustzcash PR Bump zcash_voting for PIR proof checks #34 (shielded-vote-main) lands — rebases the governance extensions onto the rev MOB-1011 uses
  3. librustvoting is updated for the above
  4. Cargo.toml rev pins are updated and Cargo.lock regenerated
  5. New xcframework release is cut

@greg0x greg0x force-pushed the shielded-vote-2.4.10 branch 5 times, most recently from bb7d8b7 to 7030596 Compare April 24, 2026 16:46
Comment thread Cargo.toml Outdated
Comment on lines +103 to +121
# librustzcash: pinned to valargroup fork containing the new
# `TransparentOutputFilter` and `ReceiverRequirementError` APIs that the
# SDK rust code adapts to.
pczt = { git = "https://github.com/valargroup/librustzcash.git", rev = "6534482c1cc595f38e0aaa63c4c7c4689360d766" }
transparent = { package = "zcash_transparent", git = "https://github.com/valargroup/librustzcash.git", rev = "6534482c1cc595f38e0aaa63c4c7c4689360d766" }
zcash_address = { git = "https://github.com/valargroup/librustzcash.git", rev = "6534482c1cc595f38e0aaa63c4c7c4689360d766" }
zcash_client_backend = { git = "https://github.com/valargroup/librustzcash.git", rev = "6534482c1cc595f38e0aaa63c4c7c4689360d766" }
zcash_client_sqlite = { git = "https://github.com/valargroup/librustzcash.git", rev = "6534482c1cc595f38e0aaa63c4c7c4689360d766" }
zcash_keys = { git = "https://github.com/valargroup/librustzcash.git", rev = "6534482c1cc595f38e0aaa63c4c7c4689360d766" }
zcash_primitives = { git = "https://github.com/valargroup/librustzcash.git", rev = "6534482c1cc595f38e0aaa63c4c7c4689360d766" }
zcash_proofs = { git = "https://github.com/valargroup/librustzcash.git", rev = "6534482c1cc595f38e0aaa63c4c7c4689360d766" }
zcash_protocol = { git = "https://github.com/valargroup/librustzcash.git", rev = "6534482c1cc595f38e0aaa63c4c7c4689360d766" }
# sapling-crypto: bumped to match the rev required by the librustzcash fork
# above (which transitively depends on sapling-crypto 0.6.2).
sapling = { package = "sapling-crypto", git = "https://github.com/zcash/sapling-crypto.git", rev = "b8a81c22f034d68f9bbd6cba728aab807b9ba2ea" }
# No `orchard` patch: both the SDK and the librustzcash fork use
# `valar-orchard` (renamed via `package = "valar-orchard"`), which resolves
# directly from crates.io. Patching `orchard` here would create two
# incompatible orchard implementations in the dependency graph.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

p0mvn added a commit that referenced this pull request Apr 25, 2026
…brustzcash zcash/main

Companion to the previous "Migrate libzcashlc to orchard 0.13 +
librustzcash zcash/main" commit (which only flipped the Cargo.toml
version pins and `[patch.crates-io]` entries). Regenerate Cargo.lock
from scratch against the live git URL pins so a fresh `cargo build`
reproduces the exact dependency graph that the migration was tested
against and that the workspace `mise run wire:local` overrides resolve
to:

  Cargo.lock pins
    orchard          7fa213ae   valargroup/orchard         valar/0.13-spend-auth-g
                                = orchard 0.13.0
                                + cherry-picks of zcash/orchard zcash#489
                                  (SpendAuthG fixed-base multiplication)
                                + zcash/orchard zcash#495
                                  (NoteValue::ZERO public associated constant)
                                (tracked by valargroup/orchard PR #19)
    voting-circuits  84726847   valargroup/voting-circuits valar/orchard-0.13
                                = valargroup/voting-circuits PR #20 head:
                                  - Drop vendored valar-orchard 0.12 fork
                                  - Migrate to orchard 0.13 public APIs
                                    (NoteValue::ZERO, Nullifier accessors,
                                     local assign_constant)
    zcash_voting     214163da   valargroup/zcash_voting    valar/orchard-0.13
                                = valargroup/zcash_voting PR #22 head
                                  (Migrate to orchard 0.13 + librustzcash
                                   zcash/main + lock against PR #19/#20)
    pczt / transparent / zcash_address / zcash_client_backend
    / zcash_client_sqlite / zcash_encoding / zcash_keys / zcash_primitives
    / zcash_proofs / zcash_protocol
                     976efa76ca zcash/librustzcash         main (commit-pinned)

The lockfile diff is large because this is the first lockfile commit
since the major version bumps in `96849be7` (zcash_address 0.10→0.11,
zcash_protocol 0.7→0.8, zcash_primitives 0.26→0.27, transparent
0.6→0.7, sapling-crypto 0.6.2→0.7, orchard 0.12 valar-fork → 0.13).
The whole `valargroup/librustzcash` fork branch and the previous
sapling-crypto git pin are gone from the resolved graph — sapling
is back on crates.io 0.7.0 and every governance librustzcash PR
(#2281, #2283, #2284) is merged into zcash/main.

Verification:
* `cargo check --tests --all-features` clean.
* No drift between Cargo.toml [patch.crates-io] entries and the
  resolved [[package]] sources in Cargo.lock.
* Downstream consumer (zodl-ios xcframework build) verified earlier
  in this migration against this exact orchard / voting-circuits /
  zcash_voting stack.

Made-with: Cursor
greg0x added a commit that referenced this pull request Apr 25, 2026
@greg0x greg0x force-pushed the shielded-vote-2.4.10 branch from fa5ea28 to 1514f71 Compare April 25, 2026 23:01
p0mvn added a commit that referenced this pull request Apr 26, 2026
…port

When PR #25 ("Migrate libzcashlc to orchard 0.13 + librustzcash zcash/main")
was force-ported into `shielded-vote-2.4.10` instead of being merged, two
of the three voting-stack `[patch.crates-io]` entries were dropped and the
`orchard` patch was re-pointed at the wrong source. Result: the FFI build
fails on every PR against this base because the dep graph contains
**three different `orchard` versions simultaneously**:

  orchard 0.11.0 (crates.io)  ← pulled in by `zcash_voting 0.1.0`
                                (crates.io, no patch)
  orchard 0.12.0 (zcash/orchard rev 6b12c77)
                              ← from the broken `[patch.crates-io] orchard`
                                entry pointing at upstream `zcash/orchard`
                                instead of `valargroup/orchard`
  orchard 0.13.0 (crates.io)  ← libzcashlc's direct dep (no patch matched
                                because the patch resolves to 0.12)

This produces:

    error[E0277]: the trait bound
        `zcash_primitives::consensus::TestNetwork:
         zcash_protocol::consensus::Parameters`
        is not satisfied
    error[E0308]: mismatched types
        expected `orchard::keys::SpendingKey`,
        found a different `orchard::keys::SpendingKey`
    note: there are multiple different versions of crate `orchard` in the
          dependency graph

inside `zcash_voting-0.1.0/src/zkp2.rs` and `orchard_compat.rs`.

This commit restores the `[patch.crates-io]` block to the form PR #25's
body specified, and regenerates `Cargo.lock` so the resolver collapses
to a single-version graph:

  orchard          7fa213ae    valargroup/orchard          valar/0.13-spend-auth-g
                               = orchard 0.13.0 + cherry-picks of
                                 zcash/orchard zcash#489 (SpendAuthG fixed-base)
                                 + zcash#495 (NoteValue::ZERO)
                                 (tracked by valargroup/orchard PR #19)
  voting-circuits  84726847    valargroup/voting-circuits  valar/orchard-0.13
                               = drop vendored valar-orchard, port to
                                 orchard 0.13 public APIs
                                 (= valargroup/voting-circuits PR #20)
  zcash_voting     d351e9dc    valargroup/zcash_voting     main (rev-pinned)
                               = merge of valargroup/zcash_voting PR #22
                                 (zcash_voting / vote-commitment-tree[-client]
                                 on orchard 0.13 + librustzcash zcash/main)

`zcash_voting` is rev-pinned (rather than branch-tracked) at the merged
`main` tip `d351e9dc13baa623b95883e0815b194358604c54` so the resolved
dep graph is reproducible across rebuilds even as `main` advances.

The previous `[patch.crates-io] orchard` entry pointing at `zcash/orchard`
rev `6b12c77` is also dropped, since plain upstream `orchard 0.12.0` is
incompatible with the rest of the orchard-0.13 voting stack.

Verification:

* `cargo check --target aarch64-apple-darwin` clean.
* `Cargo.lock` resolves to **one** version each of `orchard`,
  `voting-circuits`, `zcash_voting`, `zcash_protocol`, `zcash_primitives`
  (no stale `valar-orchard` rename left in the graph either).
* No drift between `Cargo.toml` `[patch.crates-io]` entries and the
  resolved `[[package]]` sources in `Cargo.lock`.
* Matches the workspace `.wiring/states/current/zcash-swift-wallet-sdk.patch`'s
  assumed upstream state for the orchard / voting-circuits / zcash_voting
  patch entries (modulo the rev pin), so `mise run wire:local` against a
  fresh checkout will apply cleanly again.

Made-with: Cursor
p0mvn added a commit that referenced this pull request Apr 26, 2026
…port

When PR #25 ("Migrate libzcashlc to orchard 0.13 + librustzcash zcash/main")
was force-ported into `shielded-vote-2.4.10` instead of being merged, two
of the three voting-stack `[patch.crates-io]` entries were dropped and the
`orchard` patch was re-pointed at the wrong source. Result: the FFI build
fails on every PR against this base because the dep graph contains
**three different `orchard` versions simultaneously**:

  orchard 0.11.0 (crates.io)  ← pulled in by `zcash_voting 0.1.0`
                                (crates.io, no patch)
  orchard 0.12.0 (zcash/orchard rev 6b12c77)
                              ← from the broken `[patch.crates-io] orchard`
                                entry pointing at upstream `zcash/orchard`
                                instead of `valargroup/orchard`
  orchard 0.13.0 (crates.io)  ← libzcashlc's direct dep (no patch matched
                                because the patch resolves to 0.12)

Producing:

    error[E0277]: the trait bound
        `zcash_primitives::consensus::TestNetwork:
         zcash_protocol::consensus::Parameters`
        is not satisfied
    error[E0308]: mismatched types
        expected `orchard::keys::SpendingKey`,
        found a different `orchard::keys::SpendingKey`
    note: there are multiple different versions of crate `orchard` in the
          dependency graph

inside `zcash_voting-0.1.0/src/zkp2.rs` and `orchard_compat.rs`.

This commit restores the `[patch.crates-io]` block to a self-consistent
single-version graph, with all three voting-stack crates rev-pinned at
the post-merge `main` tips (so the resolved dep graph is reproducible
across rebuilds even as `main` advances):

  orchard          c26a6ec6  valargroup/orchard          main
                             = orchard 0.13.0 + cherry-picks of
                               zcash/orchard zcash#489 (SpendAuthG fixed-base)
                               + zcash#495 (NoteValue::ZERO)
                               (= valargroup/orchard PR #19, merged)
  voting-circuits  66777e27  valargroup/voting-circuits  main
                             = drop vendored valar-orchard, port to
                               orchard 0.13 public APIs
                               (= valargroup/voting-circuits PR #20, merged;
                                internally pins orchard to c26a6ec6)
  zcash_voting     d351e9dc  valargroup/zcash_voting     main
                             = zcash_voting / vote-commitment-tree[-client]
                               on orchard 0.13 + librustzcash zcash/main
                               (= valargroup/zcash_voting PR #22, merged)

The orchard rev (c26a6ec6) is chosen to match what voting-circuits at
66777e27 internally pins to (its `05f98aa` "Pin orchard to valargroup/
orchard main" commit), so cargo collapses libzcashlc's direct orchard
dep and voting-circuits' transitive orchard dep into a single source.

The previous `[patch.crates-io] orchard` entry pointing at `zcash/orchard`
rev `6b12c77` is dropped — plain upstream `orchard 0.12.0` is incompatible
with the rest of the orchard-0.13 voting stack.

Verification:

* `cargo check --target aarch64-apple-darwin` clean.
* `Cargo.lock` resolves to **one** version each of `orchard`,
  `voting-circuits`, `zcash_voting`, `zcash_protocol`, `zcash_primitives`
  (no stale `valar-orchard` rename left in the graph either).
* No drift between `Cargo.toml` `[patch.crates-io]` entries and the
  resolved `[[package]]` sources in `Cargo.lock`.

Made-with: Cursor
@greg0x greg0x force-pushed the shielded-vote-2.4.10 branch from 6678f6e to 8ccd2f9 Compare April 26, 2026 14:33
@p0mvn p0mvn force-pushed the shielded-vote-2.4.10 branch from 8ccd2f9 to 8ee7106 Compare April 26, 2026 22:00
@greg0x greg0x force-pushed the shielded-vote-2.4.10 branch from 0818f8d to 0321168 Compare April 29, 2026 13:11
@greg0x greg0x changed the title Shielded voting feature on top of MOB-1011 Shielded voting feature Apr 30, 2026
@greg0x greg0x force-pushed the shielded-vote-2.4.10 branch 5 times, most recently from 5e6b4de to bc0c3ef Compare May 1, 2026 23:07
Copy link
Copy Markdown

@p0mvn p0mvn left a comment

Choose a reason for hiding this comment

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

  • Why are we targeting this branch MOB-1011-Synchronizer-rescanFromHeight?
  • Can we rebase against latest base?
  • Can we ensure that our diff is focused without any superfluous scripts, docs etc?

Additionally, I think that we should split up the APIs in voting.rs more by functional area. For example, delegation in one file, casting a vote in another, and common utils is a separate file. This would help make it more human reviewable.

Comment thread .github/workflows/codeql.yml
Comment thread rust/src/eip681.rs
Comment thread rust/src/voting.rs Outdated
Comment on lines +802 to +811
/// Initialize a voting round.
///
/// Returns 0 on success, -1 on error.
///
/// # Safety
///
/// - `db` must be a valid, non-null `VotingDatabaseHandle` pointer.
/// - String/byte parameters must be valid for their stated lengths.
#[unsafe(no_mangle)]
pub unsafe extern "C" fn zcashlc_voting_init_round(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can we improve the spec and add inline comments here?

All functions would benefit from the same improvement IMO

Comment thread rust/src/voting.rs Outdated
}

// =============================================================================
// B. VotingDatabase methods — Delegation proof
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What does B. mean? Looks like an AI comment

Comment thread CLAUDE.md
Comment thread .gitignore
Comment thread Sources/ZcashLightClientKit/Synchronizer.swift
Comment thread Scripts/release.sh
@greg0x greg0x changed the base branch from MOB-1011-Synchronizer-rescanFromHeight to main May 3, 2026 14:57
@greg0x
Copy link
Copy Markdown
Collaborator Author

greg0x commented May 3, 2026

@p0mvn this is our internal PR that wasn't kept up to date. Most of you questions are for the stale state.
zcash#1687 - this is the upstream PR that I was paying attention to.

I have synced the fork and changed the target branch. The diff is correct now.

I will go through your comments, close those that came from the stale state and fix those that are still relevant.

--- Edit

Why are we targeting this branch MOB-1011-Synchronizer-rescanFromHeight?

The internal PR wasn't maintained

Can we rebase against latest base?

The branch is good, now the fork and the interanl PR also reflects the upstream state

Can we ensure that our diff is focused without any superfluous scripts, docs etc?

This is the case.

Additionally, I think that we should split up the APIs in voting.rs more by functional area. For example, delegation in one file, casting a vote in another, and common utils is a separate file. This would help make it more human reviewable.

This would help with review but break the existing pattern for the lib.rs.

@p0mvn
Copy link
Copy Markdown

p0mvn commented May 3, 2026

This would help with review but break the existing pattern for the lib.rs.

What are the existing patterns for lib.rs?

Comment thread rust/build.rs Outdated
Comment on lines +6 to +12
// Re-run when any Rust source changes so cbindgen regenerates the C header.
for entry in std::fs::read_dir("rust/src").expect("rust/src should exist") {
let path = entry.expect("readable dir entry").path();
if path.extension().is_some_and(|ext| ext == "rs") {
println!("cargo:rerun-if-changed={}", path.display());
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Note: we should probably remove this

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Without this the voting.rs didn't produce the necessary bindings

@p0mvn p0mvn force-pushed the shielded-vote-2.4.10 branch 5 times, most recently from f67f2a7 to de4c609 Compare May 5, 2026 15:10
nullcopy and others added 5 commits May 8, 2026 12:36
Adds `zcashlc_voting_generate_note_witnesses`, the next FFI step in the
shielded voting wiring (zcash#1661). It generates Orchard Merkle inclusion
witnesses for the notes in a voting bundle, anchored at the round's
snapshot height, caches them in the voting DB via
`VotingDb::store_witnesses`, and returns them JSON-encoded as
`Vec<WitnessData>` in a `*mut FfiBoxedSlice`.

Implementation notes:
- Loads the cached `TreeState` and `RoundParams` from the voting DB
  inside a scoped block, decodes the protobuf, and takes the Orchard
  frontier from the snapshot.
- Rejects stale cached `TreeState` values whose decoded height or
  Orchard root do not match the round's `snapshot_height` and `nc_root`.
- Opens the wallet DB with the caller-supplied `network_id` (parsed
  via `parse_network`) and calls
  `WalletDb::generate_orchard_witnesses_at_historical_height` for the
  bundle's note positions.
- `RoundParams.snapshot_height` is `u64`; `BlockHeight` is `u32`-backed.
  Uses a checked `u32::try_from` rather than silently truncating.
- Empty `notes_json` is treated as the empty notes list (no JSON
  parsing), matching the contract of
  `zcashlc_voting_precompute_delegation_pir`.
- `# Safety` block documents every `(ptr, len)` pair, the empty-input
  rule, and the `network_id` enum.

Adds `incrementalmerkletree 0.8` as a direct Rust dependency (used for
`Position` and the `MerklePath` returned by the wallet DB). Adds a
`JsonWitnessData` shape and `From<voting::WitnessData>` impl in
`voting/json.rs`.

Tests: `generate_note_witnesses_rejects_null_db`,
`generate_note_witnesses_rejects_invalid_network_id`, and cached
`TreeState` validation coverage for matching state, height mismatch, and
root mismatch. The focused `cargo test voting::delegation` suite passes.

No Swift API surface is added in this step; the Swift wrapper will land
in a follow-up PR, matching the per-step pattern used for the other
voting FFI symbols on this branch.
@p0mvn p0mvn force-pushed the shielded-vote-2.4.10 branch from edcd7e0 to b48febd Compare May 8, 2026 18:03
p0mvn and others added 6 commits May 8, 2026 16:01
Adds FFI-level coverage for multi-note witnesses, stale TreeState rejection, empty-frontier handling, and empty notes input so witness generation cannot silently regress.
…ote-witnesses

[zcash#1661] Add voting generate-note-witnesses FFI
Expose persisted voting round, recovery, and share-delegation tracking APIs through libzcashlc.

Validate voting round and recovery byte lengths, verify recovery writes persist, consolidate test helpers, and cover defensive and Keystone signature happy paths.
@p0mvn p0mvn force-pushed the shielded-vote-2.4.10 branch from b48febd to 33af326 Compare May 8, 2026 19:51
Comment thread .github/workflows/testflight-release.yml Fixed
Comment thread .github/workflows/testflight-release.yml Fixed
Comment thread .github/workflows/testflight-release.yml Fixed
Comment thread .github/workflows/testflight-release.yml Fixed
Comment thread .github/workflows/testflight-release.yml Fixed
Comment thread .github/workflows/testflight-release.yml Fixed
Comment thread .github/workflows/testflight-release.yml Fixed
Comment thread .github/workflows/testflight-release.yml Fixed
Comment thread .github/workflows/testflight-release.yml Fixed
@p0mvn p0mvn force-pushed the shielded-vote-2.4.10 branch 2 times, most recently from a559096 to b94b806 Compare May 9, 2026 00:47
@p0mvn p0mvn closed this May 9, 2026
@p0mvn p0mvn deleted the shielded-vote-2.4.10 branch May 9, 2026 00:48
@p0mvn p0mvn restored the shielded-vote-2.4.10 branch May 9, 2026 00:48
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.

5 participants