feat(commitment-tree): add cv_net to the stored note item for OVK recovery - #761
Merged
Merged
Conversation
…overy The shielded-pool note item stored cmx ‖ rho ‖ payload, omitting the note's value commitment cv_net. cv_net (32 bytes) is required for outgoing-note (OVK) recovery — it is an input to Orchard's derive_ock(ovk, cv, cmx, epk) key that decrypts out_ciphertext, and it cannot be recomputed from the note. Insert cv_net as a new unencrypted protocol-level field between rho and the ciphertext payload, exactly as rho already sits between cmx and the payload: new item = cmx(32) ‖ rho(32) ‖ cv_net(32) ‖ payload (312 B for DashMemo) API changes (grovedb-commitment-tree): - CommitmentTree::append / append_raw gain a cv_net: [u8; 32] param. - append_many_raw entries become (cmx, rho, cv_net, payload) 4-tuples. - Item layout, payload-size check, and docs updated. The 216-byte ciphertext check is unchanged; cv_net is validated by its [u8; 32] type. The Sinsemilla frontier (commitment_frontier) is deliberately untouched: it only ever sees cmx and produces the Orchard anchor. cv_net never enters it, so for any fixed cmx sequence the anchor — against which existing zk spend proofs verify cmx-membership — is byte-for-byte identical. A new pinned-anchor test locks this in, alongside a cv_net round-trip test (cv_net at [64..96], ciphertext from [96..]). Threaded through grovedb's CommitmentTree ops + batch op (GroveOp:: CommitmentTreeInsert gains cv_net), op builders, execution, and estimated costs. BREAKING: the BulkAppendTree (item-data) state root changes, so consumers must rebuild state — no in-place migration. The Orchard anchor is unchanged. Platform's shielded pool is pre-release, so this is acceptable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
Warning Review limit reached
More reviews will be available in 16 minutes and 56 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughDocs, APIs, and implementation now carry ChangesCommitmentTree
Sequence Diagram(s)sequenceDiagram
participant GroveDb
participant CommitmentTree
participant BulkAppendTree
participant CommitmentFrontier
GroveDb->>CommitmentTree: commitment_tree_insert(..., cv_net, ...)
CommitmentTree->>BulkAppendTree: append cmx||rho||cv_net||ciphertext
CommitmentTree->>CommitmentFrontier: append cmx
CommitmentFrontier-->>GroveDb: updated anchor/root
Estimated Code Review Effort🎯 5 (Critical) | ⏱️ ~90+ minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Sync the mdBook English source with the cv_net addition: - commitment-tree.md: stored-record layout (280 → 312 B, cv_net at offset 64), field-by-field breakdown for cv_net (incl. the OVK/ock derivation rationale), insert/append API signatures, batch op variant + constructors, get_value / preprocessing / proof-item layouts, size-comparison and trial-decryption flow. - element-system.md: note-data layout + commitment_tree_insert signature. - batch-operations.md: GroveOp::CommitmentTreeInsert variant fields. - quantum-cryptography.md: stored-record read + post-quantum size estimates rebased on the 312-byte record. Docs-only; English source. Translations were intentionally left untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
✅ Review complete (commit c2c5729) |
thepastaclaw
reviewed
Jun 8, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Contributor
There was a problem hiding this comment.
Code Review
The PR correctly threads cv_net through the Rust implementation and tests without an in-scope blocking correctness issue. Two in-scope suggestions remain: the checked-in translated book pages still document the old 280-byte layout, and the new raw batch API exposes three adjacent [u8; 32] fields that can be silently misordered by callers.
🟡 2 suggestion(s)
1 additional finding(s) omitted (not in diff).
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/book/translations/es/src/commitment-tree.md`:
- [SUGGESTION] docs/book/translations/es/src/commitment-tree.md:521-536: Update translated commitment-tree docs for the new record layout
The English commitment-tree page now documents the stored item as `cmx || rho || cv_net || payload`, with `cv_net` at `[64..96]` and DashMemo records totaling 312 bytes, but the checked-in translated books still publish the old 280-byte layout and place `epk_bytes` at offset 64. These translation directories are built by the book CI, and the same stale API examples/layout text appears across the translated `commitment-tree.md` files, so non-English docs would tell consumers to deserialize the ciphertext from the wrong offset and omit the `cv_net` field required for OVK recovery. Update or regenerate the translated commitment-tree pages as part of this layout/API change.
In `grovedb-commitment-tree/src/commitment_tree/mod.rs`:
- [SUGGESTION] grovedb-commitment-tree/src/commitment_tree/mod.rs:397-402: Use a named raw entry type for batched commitment inserts
`append_many_raw` now accepts entries as `([u8; 32], [u8; 32], [u8; 32], Vec<u8>)`, which puts `cmx`, `rho`, and `cv_net` next to each other with identical Rust types. A caller that swaps `rho` and `cv_net` will still compile and still produce an internally consistent tree, but the stored metadata will be wrong for nullifier association and OVK recovery. A small named raw-entry struct, or domain newtypes for these 32-byte fields, would make the new public batch boundary preserve the protocol meaning instead of relying on tuple position.
Address review feedback (thepastaclaw): the batch API took entries as
`([u8; 32], [u8; 32], [u8; 32], Vec<u8>)`, putting the three type-identical
32-byte protocol fields (cmx, rho, cv_net) adjacent and positional. A caller
that transposed rho and cv_net would still compile and build an internally
consistent tree while silently corrupting nullifier association and OVK
recovery.
Replace the tuple with a named `CommitmentEntry { cmx, rho, cv_net, payload }`
struct so the public batch boundary preserves protocol meaning by field name
rather than tuple position. Update `append_many_raw`, re-export the struct,
and update the bench + crate-test callers.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address review feedback (thepastaclaw): the checked-in translated books still documented the old 280-byte item layout (epk_bytes at offset 64), so non-English readers would deserialize the ciphertext from the wrong offset and miss the cv_net field required for OVK recovery. Apply the same language-neutral corrections made to the English source across all 16 translations (commitment-tree, element-system, batch-operations, quantum-cryptography): stored-record layout/offset tables (280 → 312 B, cv_net at [64..96]), API signatures, the GroveOp::CommitmentTreeInsert variant + op constructors, record-read flows, size-comparison totals, and the post-quantum size box. The byte offsets, code blocks, layout strings, and signatures are byte-identical English across translations; translated explanatory prose (the new cv_net field description) is left for the localization pass. All 16 translated books build cleanly via build-translations.sh. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Member
Author
|
This is Claude — both in-scope suggestions from the review are addressed: 1. Named raw entry type for batched inserts ( 2. Stale translated commitment-tree docs ( The third finding was flagged as "not in diff," so I have not acted on it — happy to look if you can point me at it. Branch tip is now |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
docs/book/translations/es/src/commitment-tree.md (2)
406-406:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate stored-value tuple to include
cv_net.Line 406 still documents
cmx || rho || carga util; it should includecv_netto match the new record format (cmx || rho || cv_net || payload).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/book/translations/es/src/commitment-tree.md` at line 406, Update the documented stored-value tuple text to reflect the new record format by including cv_net: change the phrase listing "cmx || rho || carga util" to "cmx || rho || cv_net || payload" (or the Spanish equivalent "cmx || rho || cv_net || carga útil") so the documentation matches the updated stored-value tuple fields (`cmx`, `rho`, `cv_net`, `payload`).
876-876:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winV1 proof item tuple is stale (missing
cv_net).Line 876 still says
cmx || rho || carga util; this conflicts with the updated stored-record contract and can mislead proof consumers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/book/translations/es/src/commitment-tree.md` at line 876, The line listing V1 proof item tuple is outdated: replace "cmx || rho || carga util" with the full tuple including the new field "cv_net" (e.g., "cmx || rho || carga util || cv_net") so the documentation matches the updated stored-record contract and avoids misleading proof consumers.docs/book/translations/fr/src/commitment-tree.md (2)
878-878:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winV1 item tuple must include
cv_net.Line 878 documents
cmx || rho || charge utile, which now conflicts with the record schema used elsewhere in this chapter.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/book/translations/fr/src/commitment-tree.md` at line 878, The documentation line listing tuple fields is missing the required `cv_net` field for V1 item tuples; update the sentence that currently reads "cmx || rho || charge utile" to include `cv_net` (e.g., "cmx || rho || cv_net || charge utile") so it matches the record schema used elsewhere in this chapter and the V1 item tuple definition.
405-405:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStored value description is outdated (missing
cv_net).Line 405 still says
cmx || rho || charge utile; it should becmx || rho || cv_net || payloadto match the updated storage format.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/book/translations/fr/src/commitment-tree.md` at line 405, Update the stored-value description phrase so it reflects the new storage format: replace the fragment "cmx || rho || charge utile" with "cmx || rho || cv_net || payload" (or in French "cmx || rho || cv_net || charge utile") so the documentation now lists cmx, rho, cv_net and payload explicitly; locate the sentence containing the symbols cmx, rho and charge utile in commitment-tree.md and adjust the wording accordingly.docs/book/translations/id/src/quantum-cryptography.md (1)
278-283:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAlign hybrid-size column header with the documented 1,432-byte layout.
The table header uses
Hibrid (1.400 B), but this section defines hybrid records as 1,432 bytes (Lines 262 and 272). Please make the header consistent to avoid calculation ambiguity.Suggested doc fix
-| Notes | Saat ini (312 B) | Hibrid (1.400 B) | Delta | +| Notes | Saat ini (312 B) | Hibrid (1.432 B) | Delta |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/book/translations/id/src/quantum-cryptography.md` around lines 278 - 283, The table header currently reads "Hibrid (1.400 B)" but hybrid records are defined elsewhere as 1,432 bytes; update the header text to "Hibrid (1.432 B)" and then verify and correct the table values (columns for Hibrid and Delta) so they reflect calculations using 1,432 bytes per record; look for the table row containing "| 100.000 | 26,7 MB | 133 MB | +106 MB |" and the header text "Hibrid (1.400 B)" to make the edits.docs/book/translations/vi/src/commitment-tree.md (1)
365-370:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
GroveOp::CommitmentTreeInsertsnippet is missingcv_netfield.This block still documents
{ cmx, rho, payload }, but the same file now uses constructors and preprocess tuples that requirecv_net, so the API description is inconsistent.Suggested doc fix
GroveOp::CommitmentTreeInsert { cmx: [u8; 32], // cam kết note đã trích xuất rho: [u8; 32], // nullifier của note đã chi tiêu + cv_net: [u8; 32], // value commitment (for outgoing/OVK recovery) payload: Vec<u8>, // ciphertext tuần tự hóa (216 byte cho DashMemo) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/book/translations/vi/src/commitment-tree.md` around lines 365 - 370, The documentation for GroveOp::CommitmentTreeInsert is missing the cv_net field; update the snippet and surrounding docs to include cv_net (e.g., add "cv_net: [u8; 32]" to the documented structure) so it matches the constructors and preprocess tuples that expect cv_net; ensure any explanatory text references cv_net's purpose consistently with the code paths that construct GroveOp::CommitmentTreeInsert.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/book/translations/ar/src/element-system.md`:
- Around line 287-291: Reword the awkward Arabic phrase around "يتدفق كتجزئة
Merk الابن" to a more idiomatic construction in the architecture bullet: keep
the same technical references (BulkAppendTree, Platform, Sinsemilla, GroveDB)
but replace that fragment with a clearer phrasing such as "يُمثّل كتجزئة ابن في
تسلسل تجزئة GroveDB" or "يُخزّن/يُمرّر كتجزئة ابن عبر تسلسل تجزئة GroveDB" so
the sentence reads smoothly and preserves the intended meaning.
In `@docs/book/translations/cs/src/quantum-cryptography.md`:
- Line 277: Update the table header string that currently reads "| Notes |
Soucasne (312 B) | Hybridni (1 400 B) | Delta |" to match the computed per-note
value "1,432 bytes" used elsewhere (and in the diagram); change "Hybridni (1 400
B)" to "Hybridni (1 432 B)" so the displayed header aligns with the computed
1,432 B value.
In `@docs/book/translations/fr/src/element-system.md`:
- Line 287: The sentence "Les données réelles des notes (`cmx || rho || cv_net
|| ciphertext`) sont stockées via un **BulkAppendTree**" uses an inconsistent
apostrophe character in the prose; update the prose to use the document's
standard French apostrophe (typographic right single quote ’) while leaving the
code span (`cmx || rho || cv_net || ciphertext`) and the bold token
(**BulkAppendTree**) unchanged so only the surrounding plain-text apostrophe
characters are normalized.
In `@docs/book/translations/pl/src/quantum-cryptography.md`:
- Line 279: The storage table header currently shows "Hybrydowe (1 400 B)" while
the content uses "1,432 bytes" in the same section; pick one canonical
representation (either "1 432 B" to match Polish spacing or "1,432 bytes") and
update the header string "Hybrydowe (1 400 B)" and the two in-section
occurrences of "1,432 bytes" so they all match exactly, keeping the chosen
numeric value and formatting consistent across the table and surrounding text.
In `@docs/book/translations/pt/src/element-system.md`:
- Line 289: Fix the Portuguese accenting: update the sentence containing
"Ancoras historicas sao rastreadas pela Platform em uma arvore provavel
separada" by correcting "arvore" to "árvore" (and, if present, consider adding
accents to other words like "Ancoras" -> "Âncoras" and "historicas" ->
"históricas" for full correctness) so the line reads with proper accents.
In `@docs/book/translations/pt/src/quantum-cryptography.md`:
- Line 285: Update the table header string "Notas | Atual (312 B) | Híbrido
(1.400 B) | Delta" so the hybrid size matches the rest of the section: replace
"Híbrido (1.400 B)" with "Híbrido (1.432 B)" (to match the "1,432 bytes" value
and the totals/deltas). Ensure any other occurrences of "1.400 B" in this
section are updated to "1.432 B" for consistency.
In `@docs/book/translations/vi/src/quantum-cryptography.md`:
- Around line 219-223: The table's "Lai (1,432 B)" column and the "Chênh lệch"
column must be recomputed to use exactly 1,432 bytes per note: for each row
compute hybrid_size = count * 1,432 bytes, convert to human-readable units (use
same units style as existing rows: MB for ~10^6, GB for ~10^9, round to two
decimal places or match existing precision), then set delta = hybrid_size -
current_size (show as +/− and use same unit as hybrid column); update the three
rows for counts 100,000, 1,000,000, and 10,000,000 accordingly so the table is
internally consistent and formatted like the other rows.
In `@docs/book/translations/zh/src/quantum-cryptography.md`:
- Around line 255-259: The storage-size table in the markdown (the rows starting
with "| 票据数量 | 当前（312 B）...") mixes decimal MB/GB labels with values that appear
to be binary MiB/GiB; update the table so units are consistent: either relabel
the headers to "MiB/GiB" if the numbers are binary, or recompute each cell to
decimal "MB/GB" (e.g., use 1 MB = 1,000,000 B) and replace the values
accordingly; ensure the header text "(312 B)" and the column headings reflect
the chosen unit system so readers aren’t misled.
---
Outside diff comments:
In `@docs/book/translations/es/src/commitment-tree.md`:
- Line 406: Update the documented stored-value tuple text to reflect the new
record format by including cv_net: change the phrase listing "cmx || rho ||
carga util" to "cmx || rho || cv_net || payload" (or the Spanish equivalent "cmx
|| rho || cv_net || carga útil") so the documentation matches the updated
stored-value tuple fields (`cmx`, `rho`, `cv_net`, `payload`).
- Line 876: The line listing V1 proof item tuple is outdated: replace "cmx ||
rho || carga util" with the full tuple including the new field "cv_net" (e.g.,
"cmx || rho || carga util || cv_net") so the documentation matches the updated
stored-record contract and avoids misleading proof consumers.
In `@docs/book/translations/fr/src/commitment-tree.md`:
- Line 878: The documentation line listing tuple fields is missing the required
`cv_net` field for V1 item tuples; update the sentence that currently reads "cmx
|| rho || charge utile" to include `cv_net` (e.g., "cmx || rho || cv_net ||
charge utile") so it matches the record schema used elsewhere in this chapter
and the V1 item tuple definition.
- Line 405: Update the stored-value description phrase so it reflects the new
storage format: replace the fragment "cmx || rho || charge utile" with "cmx ||
rho || cv_net || payload" (or in French "cmx || rho || cv_net || charge utile")
so the documentation now lists cmx, rho, cv_net and payload explicitly; locate
the sentence containing the symbols cmx, rho and charge utile in
commitment-tree.md and adjust the wording accordingly.
In `@docs/book/translations/id/src/quantum-cryptography.md`:
- Around line 278-283: The table header currently reads "Hibrid (1.400 B)" but
hybrid records are defined elsewhere as 1,432 bytes; update the header text to
"Hibrid (1.432 B)" and then verify and correct the table values (columns for
Hibrid and Delta) so they reflect calculations using 1,432 bytes per record;
look for the table row containing "| 100.000 | 26,7 MB | 133 MB | +106 MB |" and
the header text "Hibrid (1.400 B)" to make the edits.
In `@docs/book/translations/vi/src/commitment-tree.md`:
- Around line 365-370: The documentation for GroveOp::CommitmentTreeInsert is
missing the cv_net field; update the snippet and surrounding docs to include
cv_net (e.g., add "cv_net: [u8; 32]" to the documented structure) so it matches
the constructors and preprocess tuples that expect cv_net; ensure any
explanatory text references cv_net's purpose consistently with the code paths
that construct GroveOp::CommitmentTreeInsert.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a299aa78-17b3-4aab-855f-eced5f663fc2
📒 Files selected for processing (81)
docs/book/src/batch-operations.mddocs/book/src/commitment-tree.mddocs/book/src/element-system.mddocs/book/src/quantum-cryptography.mddocs/book/translations/ar/src/batch-operations.mddocs/book/translations/ar/src/commitment-tree.mddocs/book/translations/ar/src/element-system.mddocs/book/translations/ar/src/quantum-cryptography.mddocs/book/translations/cs/src/batch-operations.mddocs/book/translations/cs/src/commitment-tree.mddocs/book/translations/cs/src/element-system.mddocs/book/translations/cs/src/quantum-cryptography.mddocs/book/translations/de/src/batch-operations.mddocs/book/translations/de/src/commitment-tree.mddocs/book/translations/de/src/element-system.mddocs/book/translations/de/src/quantum-cryptography.mddocs/book/translations/es/src/batch-operations.mddocs/book/translations/es/src/commitment-tree.mddocs/book/translations/es/src/element-system.mddocs/book/translations/es/src/quantum-cryptography.mddocs/book/translations/fr/src/batch-operations.mddocs/book/translations/fr/src/commitment-tree.mddocs/book/translations/fr/src/element-system.mddocs/book/translations/fr/src/quantum-cryptography.mddocs/book/translations/id/src/batch-operations.mddocs/book/translations/id/src/commitment-tree.mddocs/book/translations/id/src/element-system.mddocs/book/translations/id/src/quantum-cryptography.mddocs/book/translations/it/src/batch-operations.mddocs/book/translations/it/src/commitment-tree.mddocs/book/translations/it/src/element-system.mddocs/book/translations/it/src/quantum-cryptography.mddocs/book/translations/ja/src/batch-operations.mddocs/book/translations/ja/src/commitment-tree.mddocs/book/translations/ja/src/element-system.mddocs/book/translations/ja/src/quantum-cryptography.mddocs/book/translations/ko/src/batch-operations.mddocs/book/translations/ko/src/commitment-tree.mddocs/book/translations/ko/src/element-system.mddocs/book/translations/ko/src/quantum-cryptography.mddocs/book/translations/pl/src/batch-operations.mddocs/book/translations/pl/src/commitment-tree.mddocs/book/translations/pl/src/element-system.mddocs/book/translations/pl/src/quantum-cryptography.mddocs/book/translations/pt/src/batch-operations.mddocs/book/translations/pt/src/commitment-tree.mddocs/book/translations/pt/src/element-system.mddocs/book/translations/pt/src/quantum-cryptography.mddocs/book/translations/ru/src/batch-operations.mddocs/book/translations/ru/src/commitment-tree.mddocs/book/translations/ru/src/element-system.mddocs/book/translations/ru/src/quantum-cryptography.mddocs/book/translations/th/src/batch-operations.mddocs/book/translations/th/src/commitment-tree.mddocs/book/translations/th/src/element-system.mddocs/book/translations/th/src/quantum-cryptography.mddocs/book/translations/tr/src/batch-operations.mddocs/book/translations/tr/src/commitment-tree.mddocs/book/translations/tr/src/element-system.mddocs/book/translations/tr/src/quantum-cryptography.mddocs/book/translations/vi/src/batch-operations.mddocs/book/translations/vi/src/commitment-tree.mddocs/book/translations/vi/src/element-system.mddocs/book/translations/vi/src/quantum-cryptography.mddocs/book/translations/zh/src/batch-operations.mddocs/book/translations/zh/src/commitment-tree.mddocs/book/translations/zh/src/element-system.mddocs/book/translations/zh/src/quantum-cryptography.mdgrovedb-commitment-tree/benches/seeding.rsgrovedb-commitment-tree/src/commitment_tree/mod.rsgrovedb-commitment-tree/src/commitment_tree/tests.rsgrovedb-commitment-tree/src/lib.rsgrovedb/src/batch/estimated_costs/average_case_costs.rsgrovedb/src/batch/estimated_costs/worst_case_costs.rsgrovedb/src/batch/mod.rsgrovedb/src/operations/commitment_tree.rsgrovedb/src/tests/batch_delete_tree_tests.rsgrovedb/src/tests/batch_rejection_tests.rsgrovedb/src/tests/batch_unit_tests.rsgrovedb/src/tests/commitment_tree_tests.rsgrovedb/src/tests/proof_coverage_tests.rs
Address CodeRabbit: the quantum-cryptography post-quantum scale table in cs, id, pl, and pt still showed the pre-cv_net hybrid size (1 400 / 1.400 B) and the old storage estimates (26,7 MB / 133 MB / 267 MB / 2,67 GB), because these locales use space/period thousands separators and comma decimals that the comma-format replacements didn't match. Recompute the hybrid header to 1 432 / 1.432 B and the three rows to the 312-byte/1,432-byte values, keeping each locale's number format. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address CodeRabbit language findings on element-system.md: - ar: reword the awkward "يتدفق كتجزئة Merk الابن" to the clearer "يُمرَّر كتجزئة العقدة الابن في Merk". - pt: restore diacritics on the historical-anchors bullet (Âncoras históricas são … árvore provável). - fr: normalize prose apostrophes to the typographic ’ (U+2019) throughout the file for internal consistency; code spans left untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Merged
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Dash Platform's shielded pool stores each Orchard note's scanning data in the commitment-tree item so light wallets can retrieve it. The item held
cmx ‖ rho ‖ payloadbut omitted the note's value commitmentcv_net(32 bytes).cv_netis required for outgoing-note (OVK) recovery — it is an input to Orchard'sderive_ock(ovk, cv, cmx, epk)key that decryptsout_ciphertext, and it cannot be recomputed from the note.This inserts
cv_netas a new unencrypted protocol-level field betweenrhoand the ciphertext payload, exactly asrhoalready sits betweencmxand the payload:API changes (
grovedb-commitment-tree)CommitmentTree::append/append_rawgain acv_net: [u8; 32]param.append_many_rawentries become(cmx, rho, cv_net, payload)4-tuples.cv_netis validated by its[u8; 32]type.commitment_tree_insert{,_raw}, the batch opGroveOp::CommitmentTreeInsert(gainscv_net), op builders, batch execution, and estimated costs.Safety: the Orchard anchor is unchanged
The Sinsemilla frontier (
commitment_frontier) is deliberately untouched — it only ever seescmxand produces the Orchard anchor.cv_netnever enters it. For any fixedcmxsequence the anchor (against which existing zk spend proofs verify cmx-membership) is byte-for-byte identical.New tests lock this in:
append_does_not_change_sinsemilla_anchor— for a fixedcmxvector, asserts the tree root equals (a) a pureCommitmentFrontierbuilt from cmx-only, and (b) a hard-coded pinned anchor constant.append_preserves_cv_net_and_ciphertext_at_fixed_offsets— round-trips a note, assertingcv_netat[64..96]and the ciphertext deserializing from[96..].Client side
No change needed — the client
ShardTree(memory + sqlite) stores onlycmxfor witness generation and does not mirror the note item, so there is no client-side layout to update. Verified.The BulkAppendTree (item-data) state root changes, so consumers must rebuild state — there is no in-place migration. The Orchard anchor is unchanged. Platform's shielded pool is pre-release, so this is acceptable.
Verification
cargo test -p grovedb-commitment-tree --features server,sqlite→ 110 passed, 0 failedcargo build --workspace --all-targets+ server-gated bench → cleancargo fmt --check→ clean🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Documentation