V6/Nu7 implementation by alexeykoren · Pull Request #73 · QED-it/librustzcash

alexeykoren · 2024-09-22T09:37:46Z

ZSA is implemented across all the codebase, including:

TransactionData now contains 2 extra components - OrchardZsaBundle and IssueBundle

Serialization/deserialization of the new components

Digests of the new components are taken into account during calculation of sighash/txid.
TODO: we are waiting for test vectors and small ordering fixes to be merged to be able to check txid calculation

Nu7 network upgrade is introduced, although currently "nu6" zcash_unstable flag is used to separate ZSA from regular build. All nu7-related "nu6" flags are annotated with /* TODO nu7 */ comment

Builder methods for ZSAs

Wallet-related functionality (zcash_client_backend, zcash_client_sqlite) are disabled since they do not support ZSAs

…te (which supports Vanilla and ZSA) - but for now zcash_primitives is not generalised itself and pinned to Vanilla only

This PR updates the `zcash_primitives` crate to align with the recent changes in the `sapling-crypto` crate, which was updated to sync with the latest changes in `zcash_note_encryption`. ### Changes: - Updated the usage of `enc_ciphertext` to call `.as_ref()` to match the new return type. - Adjusted the import of the `ENC_CIPHERTEXT_SIZE` constant to use the definition from `sapling-crypto` instead of `zcash_note_encryption`, as the constant has been moved to `sapling-crypto`. Co-authored-by: Dmitry Demin <dmitry@qed-it.com>

This updates the test vectors to be of version 6 (as opposed to version 7). It also provides a new file for the V6 test vectors, an improvement to having them in the same file but behind feature flags. It provides a error for when the asset description string is not a UTF8 encoding.

PaulLaux

added some comments

PaulLaux · 2024-09-22T19:55:14Z

-        let tx = Transaction::read(&tv.tx[..], BranchId::Nu5).unwrap();
+        let tx = Transaction::read(
+            &tv.tx[..],
+            #[cfg(not(zcash_unstable = "nu6"))] /* TODO nu7 */ BranchId::Nu5,


#[cfg(not(zcash_unstable = "nu6")) /* TODO nu7 */ ] + new line

PaulLaux · 2024-09-22T20:13:42Z

        sapling_bundle: Option<sapling::Bundle<A::SaplingAuth, Amount>>,
-        orchard_bundle: Option<orchard::Bundle<A::OrchardAuth, Amount>>,
+        orchard_bundle: Option<orchard::Bundle<A::OrchardAuth, Amount, OrchardVanilla>>,
+        #[cfg(zcash_unstable = "nu6" /* TODO nu7 */ )] orchard_zsa_bundle: Option<


PaulLaux · 2024-09-22T20:13:51Z

+        #[cfg(zcash_unstable = "nu6" /* TODO nu7 */ )] orchard_zsa_bundle: Option<
+            orchard::Bundle<A::OrchardZsaAuth, Amount, OrchardZSA>,
+        >,
+        #[cfg(zcash_unstable = "nu6" /* TODO nu7 */ )] issue_bundle: Option<


PaulLaux · 2024-09-22T20:14:14Z

+        ) -> Option<
+            orchard::bundle::Bundle<B::OrchardAuth, Amount, OrchardVanilla>,
+        >,
+        #[cfg(zcash_unstable = "nu6" /* TODO nu7 */ )] f_zsa_orchard: impl FnOnce(


PaulLaux · 2024-09-24T18:14:29Z

            version in Just(version)
        ) -> TransactionData<Authorized> {
-            TransactionData {
+            TransactionData::<Authorized> {


I don't think this is needed.

PaulLaux · 2024-09-24T18:15:37Z

+use byteorder::WriteBytesExt;
+
+#[cfg(zcash_unstable = "zfuture")]
+use zcash_encoding::{CompactSize, Vector};
+
+#[cfg(zcash_unstable = "zfuture")]
+use crate::transaction::{components::tze, TzeDigests};


Change not required

indeed, reverted

PaulLaux · 2024-09-24T18:17:01Z

+) -> Blake2bHash {
+    // Currently to_hash is designed in a way that it supports both v5 and v6 signature hash
+    v5_v6_signature_hash(tx, signable_input, txid_parts)
+}


this file is not connected in mod.rs and function is unused.

not sure we need it

alexeykoren · 2024-10-01T14:40:04Z

+    }
+
+    #[cfg(zcash_unstable = "nu6" /* TODO nu7 */ )]
+    fn digest_orchard_zsa(


see comment wrt digest_orchard

PaulLaux · 2024-09-24T18:25:00Z

        transparent_digest: Self::TransparentDigest,
        sapling_digest: Self::SaplingDigest,
        orchard_digest: Self::OrchardDigest,
+        #[cfg(zcash_unstable = "nu6" /* TODO nu7 */ )] issue_digest: Self::IssueDigest,


alexeykoren · 2024-10-01T14:40:04Z

+    }
+
+    #[cfg(zcash_unstable = "nu6" /* TODO nu7 */ )]
+    fn digest_orchard_zsa(


see comment wrt digest_orchard

PaulLaux

Good overall, added final fixes.

PaulLaux · 2024-10-01T19:04:10Z

+    let asset_descr: String = String::from_utf8(asset_descr_bytes)
+        .map_err(|_| Error::new(ErrorKind::InvalidData, "Asset Description not valid UTF-8"))?;
+    let notes = Vector::read(&mut reader, |r| read_note(r))?;
+    let finalize = reader.read_u8()? != 0;


If Muhammad won't go to the mountain, then the mountain must come to Muhammad.

let finalize = match reader.read_u8()? { 0 => false, 1 => true, _ => return Err(Error::new(ErrorKind::InvalidData, "Invalid value for finalize")), };

PaulLaux · 2024-10-01T19:24:26Z

+    Ok(())
+}
+
+fn write_action<W: Write>(action: &IssueAction, mut writer: W) -> io::Result<()> {


Consider changing the parameters order to be compatible with existing code (ie CompactSize::write())
also it will be more compatible with the order in Vector::write(&mut writer, action.notes(), |w, note| write_note(note, w))?; and will allow to omit the parameters completely in some places (same for write_note())

PaulLaux · 2024-10-01T19:26:20Z

+    Ok(())
+}
+
+pub fn write_note<W: Write>(note: &Note, writer: &mut W) -> io::Result<()> {


replacing the order will allow:

fn write_action<W: Write>(action: &IssueAction, mut writer: W) -> io::Result<()> { ... Vector::write(&mut writer, action.notes(), write_note)?; ... Ok(()) } pub fn write_note<W: Write>( writer: &mut W,note: &Note) -> io::Result<()> { ... }

PaulLaux · 2024-10-01T19:33:22Z

+    }
+}
+
+/// Writes an [`orchard::Bundle`] in the v6 transaction format.


/// Writes an [orchard::Bundle] in the appropriate transaction format

PaulLaux · 2024-10-06T10:03:27Z

        transparent_digest: Self::TransparentDigest,
        sapling_digest: Self::SaplingDigest,
        orchard_digest: Self::OrchardDigest,
+        #[cfg(zcash_unstable = "nu6" /* TODO nu7 */ )] issue_digest: Self::IssueDigest,


new line plz

PaulLaux · 2024-10-07T11:17:37Z

    /// spend or output was added.
-    OrchardBuilderNotAvailable,
+    OrchardBundleNotAvailable,
+    ///  The issuance bundle not initialized.


remove one space after ///

PaulLaux · 2024-10-07T11:29:35Z

+            .init_issuance_bundle::<FeeError>(iak, asset.clone(), None)
+            .unwrap();
+
+        let binding = builder.mock_build_no_fee(OsRng).unwrap().into_transaction();


let tx = might be a better name

PaulLaux · 2024-10-07T11:48:14Z

+    orchard_bundle: Option<orchard::bundle::Bundle<A::OrchardAuth, Amount, OrchardVanilla>>,
+    #[cfg(zcash_unstable = "nu6" /* TODO nu7 */ )]
+    orchard_zsa_bundle: Option<orchard::bundle::Bundle<A::OrchardZsaAuth, Amount, OrchardZSA>>,


For safety we should make them mutually exclusive using Enum as discussed.

PaulLaux · 2024-10-10T08:20:01Z

    if let Some(bundle) = bundle {
        Vector::write_nonempty(&mut writer, bundle.actions(), |w, action| {
-            write_action(action, w)
+            write_action(w, action)


Vector::write_nonempty(&mut writer, bundle.actions(), write_action)?;

PaulLaux and others added 30 commits July 24, 2024 11:45

minor update

d58ac5b

updated version

f34dd02

commented out test code

b890183

updated test and cleanup

f62a230

New orchard version

a56c5da

Add TODO for burn reading/writing

d670033

fixed zcash_client_backend and simplified cargo.toml

c6175b8

updated Cargo.toml

af4d4bb

Fix post-rebase problems

fbbd655

Make BlockHeader from_data public

424bf1a

Make txid bytes public

b0b0f65

Note serialization

dac8d1f

Update zcash_primitives to use the generalised version of orchard cra…

cd8524b

…te (which supports Vanilla and ZSA) - but for now zcash_primitives is not generalised itself and pinned to Vanilla only

Fix tests accordingly to modifications in orchard

3c83b72

Minor fix or Rust code format (because of cargo fmt error)

aac4840

Fix post-rebase problems

6da0474

Note serialization

0049a3b

Update zcash_primitives to use the generalised version of orchard cra…

98f4747

…te (which supports Vanilla and ZSA) - but for now zcash_primitives is not generalised itself and pinned to Vanilla only

Introduce issue bundle

2365c2b

Orchard domain interface update

7e87fa5

Work in progress

2bd5e45

Add ZSA

2704c8a

Add issuance

5eec964

Fix fmt

3a18a4d

Fix clippy

35f02fb

Fix transparent-inputs

2ab8e16

Remove unproven enum

c841df1

Change methods visibility

fb2b73a

Fix post-rebase compilation errors

cdd8afa

Revert test vector changes for Nu5

5733e8d

alexeykoren and others added 11 commits July 30, 2024 15:20

Fmt

3ac2618

Fix double import

ea1770e

Use updated zsa1

fe88163

Fmt

fc38730

Minor changes

dc0a815

Change V7 to v6

a434849

Make ciphertext reading generic

9f52b1b

PR fixes

f79962b

PR fixes

738ed17

alexeykoren changed the title ~~Txv6 separate bundles rebased~~ V6/Nu7 implementation Sep 22, 2024

alexeykoren mentioned this pull request Sep 22, 2024

Nu7 implementation #65

Closed

PaulLaux reviewed Sep 24, 2024

View reviewed changes

alexeykoren added 6 commits September 25, 2024 13:30

Uncomment CI

25c6cd1

Add PR fixes

655d961

PR Fixes

fc12237

Check build config in issuance

8a40922

Add tx building to tests

12a39c2

Add test for burn

9c51a46

alexeykoren commented Oct 1, 2024

View reviewed changes

alexeykoren added 4 commits October 1, 2024 16:42

Fmt

6283d33

Simplify upper_bound calculation

75e3b46

Fix clippy and missing lines

4101a96

Simplify imports

a0a7d38

PaulLaux reviewed Oct 7, 2024

View reviewed changes

alexeykoren and others added 2 commits October 8, 2024 09:24

PR fixes

b2d7b18

updated syntax

798630a

PaulLaux approved these changes Oct 10, 2024

View reviewed changes

PaulLaux merged commit 7301f78 into zsa1 Oct 10, 2024

Conversation

alexeykoren commented Sep 22, 2024

Uh oh!

PaulLaux left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PaulLaux left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants