ZIP 225 & ZIP 244 #375

nuttycom · 2021-04-21T21:25:13Z

No description provided.

nuttycom · 2021-05-04T18:34:27Z

A note to reviewers: This PR is best viewed hiding whitespace changes.

conradoplg

Found a possible personalization field mismatch

zcash_primitives/src/transaction/txid.rs

therealyingtong

Flushing comments from partial review.

zcash_primitives/src/transaction/mod.rs

therealyingtong · 2021-05-07T06:53:31Z

zcash_primitives/src/transaction/txid.rs

+    h.finalize()
+}
+
+pub fn to_txid(


Could to_txid() be a method on TransactionData?

zcash_primitives/src/consensus.rs

zcash_primitives/Cargo.toml

zcash_primitives/src/transaction/builder.rs

zcash_primitives/src/transaction/mod.rs

zcash_primitives/src/transaction/components/transparent/builder.rs

zcash_primitives/src/transaction/txid.rs

zcash_primitives/src/transaction/sighash_v5.rs

zcash_primitives/src/transaction/txid.rs

Co-authored-by: str4d <jack@electriccoin.co>

str4d

utACK 1d7c6ea9f9a60a564a7ead4cab9b9e6b55aef241

zcash_primitives/src/transaction/builder.rs

zcash_primitives/src/transaction/mod.rs

daira · 2021-06-06T02:53:58Z

zcash_primitives/src/transaction/sighash_v5.rs

@@ -0,0 +1,182 @@
+use std::io::Write;


I have not checked this file either.

daira

Partial review. I commented which files I have not checked.

Co-authored-by: str4d <jack@electriccoin.co>

daira · 2021-06-08T09:45:38Z

zcash_primitives/src/transaction/builder.rs

-                unauthed_tx,
+        let transparent_bundle = unauthed_tx.transparent_bundle.clone().map(|b| {
+            b.apply_signatures(
+                #[cfg(feature = "transparent-inputs")]


Should this be a separate method for the case with transparent inputs, rather than a method with feature-conditional arguments?

That's going to happen in a subsequent PR, when the Orchard builder is integrated.

daira · 2021-06-08T11:38:23Z

zcash_primitives/src/transaction/mod.rs

+        if actions_without_auth.is_empty() {
+            Ok(None)
+        } else {
+            let flags = orchard_serialization::read_flags(&mut reader)?;


The implementation of read_flags enforces that unused bits are zero. This is as intended but was ambiguous in the spec. The spec has been clarified in version 2021.2.4.

zcash_primitives/src/transaction/mod.rs

daira · 2021-06-08T12:30:10Z

zcash_primitives/src/transaction/mod.rs

        ) -> TransactionData<Authorized> {
            TransactionData {
                version,
+                consensus_branch_id,
                lock_time,
                expiry_height: expiry_height.into(),
                transparent_bundle,
                sprout_bundle: None,


I know we don't support creating v4 transactions with a Sprout bundle, but we do support reading them, so it should probably be prop-tested. Please file a ticket. (Does not block this PR.)

daira · 2021-06-08T12:36:56Z

zcash_primitives/src/transaction/txid.rs

+const ZCASH_SAPLING_SIGS_HASH_PERSONALIZATION: &[u8; 16] = b"ZTxAuthSapliHash";
+const ZCASH_ORCHARD_SIGS_HASH_PERSONALIZATION: &[u8; 16] = b"ZTxAuthOrchaHash";
+#[cfg(feature = "zfuture")]
+const ZCASH_TZE_WITNESSES_HASH_PERSONALIZATION: &[u8; 16] = b"ZTxAuthTZE__Hash";


I checked all of the non-TZE personalizations against the current version of ZIP 244.

daira

utACK with minor comments.

Co-authored-by: Daira Hopwood <daira@jacaranda.org>

str4d

utACK eb3d01a. I confirmed it addressed @daira's comments.

daira

Post-hoc ACK eb3d01a

r3ld3v added this to the Core Sprint 2021-16 milestone Apr 27, 2021

nuttycom force-pushed the feature/zip-225 branch from 4b8ea07 to da7f667 Compare May 3, 2021 20:43

nuttycom mentioned this pull request May 4, 2021

ZIP-244 & ZIP-245: creation of non-malleable txids + TZE signature hashing #319

Closed

nuttycom force-pushed the feature/zip-225 branch from d054bc2 to a6940bf Compare May 4, 2021 18:07

nuttycom marked this pull request as ready for review May 4, 2021 18:31

nuttycom requested review from str4d, daira and therealyingtong May 4, 2021 18:31

nuttycom force-pushed the feature/zip-225 branch 3 times, most recently from 39770cf to aa060fa Compare May 6, 2021 15:42

nuttycom marked this pull request as draft May 7, 2021 20:14

conradoplg reviewed May 7, 2021

View reviewed changes

zcash_primitives/src/transaction/txid.rs Outdated Show resolved Hide resolved

conradoplg reviewed May 7, 2021

View reviewed changes

zcash_primitives/src/transaction/txid.rs Outdated Show resolved Hide resolved

r3ld3v modified the milestones: Core Sprint 2021-16, Core Sprint 2021-18 May 10, 2021

nuttycom marked this pull request as ready for review May 10, 2021 21:47

nuttycom force-pushed the feature/zip-225 branch from ea0ec3a to 9d94466 Compare May 10, 2021 23:34

teor2345 mentioned this pull request May 11, 2021

Add ZIP-0244 TxId Digest support ZcashFoundation/zebra#2129

Merged

2 tasks

nuttycom force-pushed the feature/zip-225 branch from 5b3616f to 12e3370 Compare May 11, 2021 15:05

nuttycom marked this pull request as draft May 11, 2021 15:14

nuttycom force-pushed the feature/zip-225 branch from 12e3370 to 2c24085 Compare May 11, 2021 15:59

therealyingtong reviewed May 13, 2021

View reviewed changes

zcash_primitives/src/transaction/txid.rs Show resolved Hide resolved

therealyingtong reviewed May 13, 2021

View reviewed changes

zcash_primitives/src/transaction/sighash_v5.rs Outdated Show resolved Hide resolved

therealyingtong reviewed May 13, 2021

View reviewed changes

zcash_primitives/src/transaction/txid.rs Show resolved Hide resolved

nuttycom force-pushed the feature/zip-225 branch 2 times, most recently from 3450de4 to 239b125 Compare May 14, 2021 15:37

nuttycom marked this pull request as ready for review May 14, 2021 15:38

Use Vector read/write operations where possible instead of Array

4f764c3

Co-authored-by: str4d <jack@electriccoin.co>

nuttycom force-pushed the feature/zip-225 branch 2 times, most recently from ecc9b68 to 8a9eda7 Compare June 5, 2021 16:18

Deserialize signatures directly into actions.

b93c503

Co-authored-by: str4d <jack@electriccoin.co>

nuttycom force-pushed the feature/zip-225 branch 2 times, most recently from 54dce0e to 1d7c6ea Compare June 5, 2021 16:42

str4d approved these changes Jun 5, 2021

View reviewed changes

daira reviewed Jun 6, 2021

View reviewed changes

zcash_primitives/src/transaction/builder.rs Show resolved Hide resolved

daira reviewed Jun 6, 2021

View reviewed changes

zcash_primitives/src/transaction/mod.rs Outdated Show resolved Hide resolved

daira reviewed Jun 6, 2021

View reviewed changes

zcash_primitives/src/transaction/sighash_v5.rs

@@ -0,0 +1,182 @@

use std::io::Write;

Copy link

Contributor

daira Jun 6, 2021

Choose a reason for hiding this comment

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

I have not checked this file either.

daira reviewed Jun 6, 2021

View reviewed changes

nuttycom and others added 3 commits June 7, 2021 09:27

Be more explicit about empty sapling & orchard hashes.

0253442

Minor error message improvement.

371f84d

Co-authored-by: str4d <jack@electriccoin.co>

Fix error in transaction auth digest.

f70285d

Co-authored-by: str4d <jack@electriccoin.co>

nuttycom force-pushed the feature/zip-225 branch from 0517162 to f70285d Compare June 7, 2021 15:27

r3ld3v modified the milestones: Core Sprint 2021-20, Core Sprint 2021-22 Jun 7, 2021

daira reviewed Jun 8, 2021

View reviewed changes

zcash_primitives/src/transaction/mod.rs Outdated Show resolved Hide resolved

daira reviewed Jun 8, 2021

View reviewed changes

zcash_primitives/src/transaction/mod.rs Show resolved Hide resolved

daira reviewed Jun 8, 2021

View reviewed changes

zcash_primitives/src/transaction/mod.rs Outdated Show resolved Hide resolved

daira reviewed Jun 8, 2021

View reviewed changes

daira approved these changes Jun 8, 2021

View reviewed changes

Apply suggestions from code review

eb3d01a

Co-authored-by: Daira Hopwood <daira@jacaranda.org>

str4d approved these changes Jun 8, 2021

View reviewed changes

str4d merged commit 0bfd1f7 into zcash:master Jun 8, 2021

daira reviewed Jun 8, 2021

View reviewed changes

nuttycom deleted the feature/zip-225 branch June 12, 2021 21:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ZIP 225 & ZIP 244 #375

ZIP 225 & ZIP 244 #375

nuttycom commented Apr 21, 2021

nuttycom commented May 4, 2021

conradoplg left a comment

therealyingtong left a comment

therealyingtong May 7, 2021

str4d left a comment

daira Jun 6, 2021

daira left a comment

daira Jun 8, 2021

str4d Jun 8, 2021

daira Jun 8, 2021

daira Jun 8, 2021

daira Jun 8, 2021 •

edited

Loading

daira left a comment

str4d left a comment

daira left a comment

ZIP 225 & ZIP 244 #375

ZIP 225 & ZIP 244 #375

Conversation

nuttycom commented Apr 21, 2021

nuttycom commented May 4, 2021

conradoplg left a comment

Choose a reason for hiding this comment

therealyingtong left a comment

Choose a reason for hiding this comment

therealyingtong May 7, 2021

Choose a reason for hiding this comment

str4d left a comment

Choose a reason for hiding this comment

daira Jun 6, 2021

Choose a reason for hiding this comment

daira left a comment

Choose a reason for hiding this comment

daira Jun 8, 2021

Choose a reason for hiding this comment

str4d Jun 8, 2021

Choose a reason for hiding this comment

daira Jun 8, 2021

Choose a reason for hiding this comment

daira Jun 8, 2021

Choose a reason for hiding this comment

daira Jun 8, 2021 • edited Loading

Choose a reason for hiding this comment

daira left a comment

Choose a reason for hiding this comment

str4d left a comment

Choose a reason for hiding this comment

daira left a comment

Choose a reason for hiding this comment

daira Jun 8, 2021 •

edited

Loading