zcash_client_sqlite: Add note version to Orchard received notes#2532
zcash_client_sqlite: Add note version to Orchard received notes#2532dannywillems wants to merge 2 commits into
Conversation
Ironwood notes are stored in `orchard_received_notes` alongside Orchard notes, and an Orchard action and an Ironwood action in the same transaction can share an action index. Add a `note_version` column and widen the received-note uniqueness constraint from `(transaction_id, action_index)` to `(transaction_id, action_index, note_version)` so the two pools no longer collide. Orchard notes are version 2 and Ironwood notes are version 3, matching the Orchard protocol revision; existing rows are backfilled as version 2. The Orchard received-note upsert now writes `ORCHARD_NOTE_VERSION` and targets the widened constraint. A reusable proptest strategy for synthetic Orchard note payloads is added to the shared migration-test module, and a property test checks that a wallet's pre-NU6.3 Orchard notes survive the migration as version 2 with their payloads intact, that an Ironwood note may reuse an action index, and that a duplicate Orchard note is still rejected. Adapted from valargroup/librustzcash@0555be5234 and @d94102c3ea (adam/qleak-pr44-orchard-dummy-ciphertexts). Co-Authored-By: Adam Tucker <adam@osmosis.team> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Adam Tucker <adam@osmosis.team> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
e29fb03 to
225c487
Compare
| // The two note versions must differ so that an Orchard and an Ironwood action in the same | ||
| // transaction can share an action index under the widened uniqueness constraint. | ||
| const _: () = assert!(ORCHARD_NOTE_VERSION != IRONWOOD_NOTE_VERSION); |
There was a problem hiding this comment.
We need a pool identifier; the note version is too fragile for disambiguation - even though it does disambiguate orchard vs ironwood, an ironwood2 pool could have the same note plaintext version.
There was a problem hiding this comment.
I've been thinking about this more and I'm not sure which the best way to go is. At very least, I do not want the note plaintext version to be used as a pool identifier. Keeping everything in the orchard_received_notes table might be okay, but we should definitely populate the note version from the note itself, and have an explicit pool identifier column (and make the uniqueness constraint cover that instead of the note plaintext version) if we want to do so.
Reusing orchard_received_notes probably ultimately results in less churn, but I'm concerned about the confusion that results from the balance for two separate pools being mixed together in orchard_received_notes. I opened #2538 to explore the alternative of adding a separate ironwood_received_notes table.
There was a problem hiding this comment.
I agree with your approach; I was also not convinced by the approach in this patch (which follows ValarGroup's). I will have a look at 2538.
nuttycom
left a comment
nuttycom
left a comment
There was a problem hiding this comment.
If we're going to make orchard_received_notes mean notes received using the orchard protocol instead of the orchard pool, we need to disambiguate by something more than the note plaintext version. But my inclination here would be to have a separate ironwood_received_notes table instead.
|
Closing in favor of #2538 |
2 participants
No description provided.