Add `UnifiedIncomingViewingKey` struct #1245

AArnott · 2024-03-09T06:08:35Z

Remove some fn implementations in the UFVK type and have it instead simply forward to the UIVK type. For example address generation needn't be implemented twice.
Update zcash_client_sqlite where it left placeholders for using the new UIVK type.
~~Consider adding ZIP-316 rev. 1 support to both UFVK and UIVK types.~~
Add tests

zcash_keys/src/keys.rs

AArnott · 2024-03-09T14:52:19Z

@nuttycom Why is #[doc(hidden)] applied to the existing UnifiedFullViewingKey struct? Should I remove it at this point, or add it to the new UnifiedIncomingViewingKey struct?

zcash_keys/src/keys.rs

nuttycom

This looks great, it looks to me like the hdwallet AddressGenerationError and keeping the decode and encode convenience methods (to limit API breakage slightly) are the only blocking issues.

nuttycom · 2024-03-09T16:28:37Z

Also, add tests that verify against https://github.com/zcash/zcash-test-vectors/blob/master/test-vectors/rust/unified_incoming_viewing_keys.rs

nuttycom · 2024-03-09T16:31:14Z

@nuttycom Why is #[doc(hidden)] applied to the existing UnifiedFullViewingKey struct? Should I remove it at this point, or add it to the new UnifiedIncomingViewingKey struct?

Oh, yeah, we should definitely remove that and have both of these publicly documented.

zcash_keys/src/keys.rs

@daira

Unblocking for others to review, since I don't have time at the moment. @daira?

Also update sqlite to utilize the new struct

Co-authored-by: Kris Nuttycombe <[email protected]>

zcash_client_sqlite/src/wallet.rs

zcash_keys/src/keys.rs

nuttycom

One minor naming fix, otherwise looks good!

…nstruction.

nuttycom

utACK 4d9927b

zcash_keys/src/keys.rs

nuttycom

utACK b4171ca

zcash_client_sqlite/src/wallet.rs

zcash_keys/src/address.rs

nuttycom

re-utACK 25045b3

nuttycom

re-utACK 6102e83

daira · 2024-03-15T23:15:21Z

zcash_client_sqlite/src/wallet.rs

@@ -206,7 +206,7 @@ impl Account {
    ) -> Result<(UnifiedAddress, DiversifierIndex), AddressGenerationError> {


This method can be public. (It will need a changelog entry.)

In general we would like to avoid encouraging users to use the default_address method. In any case, this is not a change that should be made in this PR.

daira · 2024-03-18T03:19:52Z

zcash_keys/src/keys.rs

+        let _encoded_no_t = "uivk1020vq9j5zeqxh303sxa0zv2hn9wm9fev8x0p8yqxdwyzde9r4c90fcglc63usj0ycl2scy8zxuhtser0qrq356xfy8x3vyuxu7f6gas75svl9v9m3ctuazsu0ar8e8crtx7x6zgh4kw8xm3q4rlkpm9er2wefxhhf9pn547gpuz9vw27gsdp6c03nwlrxgzhr2g6xek0x8l5avrx9ue9lf032tr7kmhqf3nfdxg7ldfgx6yf09g";
+
+        // We test the full roundtrip only with the `sapling` and `orchard` features enabled,
+        // because we will not generate these parts of the encoding if the UIVK does not have an


Suggested change

// because we will not generate these parts of the encoding if the UIVK does not have an

// because we will not generate these parts of the encoding if the UIVK does not have

Non-blocking.

daira

utACK

nuttycom reviewed Mar 9, 2024

View reviewed changes

zcash_keys/src/keys.rs Outdated Show resolved Hide resolved

nuttycom reviewed Mar 9, 2024

View reviewed changes

zcash_keys/src/keys.rs Show resolved Hide resolved

nuttycom reviewed Mar 9, 2024

View reviewed changes

zcash_keys/src/keys.rs Show resolved Hide resolved

nuttycom reviewed Mar 9, 2024

View reviewed changes

zcash_keys/src/keys.rs Outdated Show resolved Hide resolved

nuttycom reviewed Mar 9, 2024

View reviewed changes

zcash_keys/src/keys.rs Outdated Show resolved Hide resolved

nuttycom reviewed Mar 9, 2024

View reviewed changes

zcash_keys/src/keys.rs Show resolved Hide resolved

nuttycom reviewed Mar 9, 2024

View reviewed changes

zcash_keys/src/keys.rs Outdated Show resolved Hide resolved

nuttycom reviewed Mar 9, 2024

View reviewed changes

zcash_keys/src/keys.rs Outdated Show resolved Hide resolved

nuttycom reviewed Mar 9, 2024

View reviewed changes

zcash_keys/src/keys.rs Outdated Show resolved Hide resolved

nuttycom reviewed Mar 9, 2024

View reviewed changes

zcash_keys/src/keys.rs Outdated Show resolved Hide resolved

nuttycom previously requested changes Mar 9, 2024

View reviewed changes

AArnott force-pushed the uivk branch from b24d6f7 to a57ab54 Compare March 9, 2024 20:06

AArnott marked this pull request as ready for review March 9, 2024 20:06

nuttycom reviewed Mar 10, 2024

View reviewed changes

zcash_keys/src/keys.rs Outdated Show resolved Hide resolved

AArnott and others added 2 commits March 12, 2024 06:37

Add UnifiedIncomingViewingKey struct

071d7c5

Also update sqlite to utilize the new struct

Apply updated style for key discovery

2b3060d

Co-authored-by: Kris Nuttycombe <[email protected]>

AArnott force-pushed the uivk branch from ae59799 to 2b3060d Compare March 12, 2024 12:37

Merge remote-tracking branch 'upstream/main' into uivk

27eba4e

AArnott force-pushed the uivk branch from a22d5e4 to 27eba4e Compare March 13, 2024 15:43

nuttycom reviewed Mar 13, 2024

View reviewed changes

zcash_client_sqlite/src/wallet.rs Outdated Show resolved Hide resolved

nuttycom reviewed Mar 13, 2024

View reviewed changes

zcash_keys/src/keys.rs Outdated Show resolved Hide resolved

nuttycom reviewed Mar 13, 2024

View reviewed changes

zcash_keys/src/keys.rs Outdated Show resolved Hide resolved

nuttycom reviewed Mar 13, 2024

View reviewed changes

zcash_keys/src/keys.rs Outdated Show resolved Hide resolved

nuttycom reviewed Mar 13, 2024

View reviewed changes

zcash_keys/src/keys.rs Outdated Show resolved Hide resolved

nuttycom requested changes Mar 13, 2024

View reviewed changes

nuttycom mentioned this pull request Mar 13, 2024

Account APIs needed for the mobile SDKs #1267

Merged

nuttycom force-pushed the uivk branch 6 times, most recently from 04798e4 to f3fb7db Compare March 14, 2024 17:24

zcash_keys: Verify the ability to derive addresses at USK and UFVK co…

4d9927b

…nstruction.

nuttycom force-pushed the uivk branch from f3fb7db to 4d9927b Compare March 14, 2024 17:26

nuttycom previously approved these changes Mar 14, 2024

View reviewed changes

zcash_keys/src/keys.rs Outdated Show resolved Hide resolved

zcash_keys: Fix no-default-features compilation.

b4171ca

nuttycom dismissed their stale review via b4171ca March 14, 2024 18:00

nuttycom previously approved these changes Mar 14, 2024

View reviewed changes

daira reviewed Mar 14, 2024

View reviewed changes

zcash_client_sqlite/src/wallet.rs Show resolved Hide resolved

daira reviewed Mar 14, 2024

View reviewed changes

zcash_keys/src/address.rs Show resolved Hide resolved

Implement todo! line

9ddbf1e

AArnott dismissed nuttycom’s stale review via 9ddbf1e March 14, 2024 21:21

nuttycom force-pushed the uivk branch from 9434b80 to 25045b3 Compare March 14, 2024 21:29

nuttycom previously approved these changes Mar 14, 2024

View reviewed changes

zcash_keys: Keep the Ufvk and Uivk encodings private.

9e1a432

nuttycom dismissed their stale review via 9e1a432 March 14, 2024 23:03

nuttycom force-pushed the uivk branch from 25045b3 to 9e1a432 Compare March 14, 2024 23:03

Merge remote-tracking branch 'upstream/main' into uivk

6102e83

nuttycom approved these changes Mar 15, 2024

View reviewed changes

nuttycom mentioned this pull request Mar 15, 2024

with this change, zcash_keys compiles without sapling enabled #1278

Closed

daira reviewed Mar 15, 2024

View reviewed changes

daira reviewed Mar 18, 2024

View reviewed changes

daira approved these changes Mar 18, 2024

View reviewed changes

nuttycom merged commit 273712b into zcash:main Mar 18, 2024
22 of 25 checks passed

AArnott deleted the uivk branch March 18, 2024 15:14

AArnott mentioned this pull request Mar 20, 2024

remove doc hidden from some keys #1296

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `UnifiedIncomingViewingKey` struct #1245

Add `UnifiedIncomingViewingKey` struct #1245

AArnott commented Mar 9, 2024 •

edited

Loading

AArnott commented Mar 9, 2024 •

edited

Loading

nuttycom left a comment

nuttycom commented Mar 9, 2024

nuttycom commented Mar 9, 2024

nuttycom left a comment

nuttycom left a comment

nuttycom left a comment

nuttycom left a comment

nuttycom left a comment

daira Mar 15, 2024 •

edited

Loading

nuttycom Mar 15, 2024

daira Mar 18, 2024

daira left a comment

		@@ -206,7 +206,7 @@ impl Account {
		) -> Result<(UnifiedAddress, DiversifierIndex), AddressGenerationError> {

	// because we will not generate these parts of the encoding if the UIVK does not have an
	// because we will not generate these parts of the encoding if the UIVK does not have

Add UnifiedIncomingViewingKey struct #1245

Add UnifiedIncomingViewingKey struct #1245

Conversation

AArnott commented Mar 9, 2024 • edited Loading

AArnott commented Mar 9, 2024 • edited Loading

nuttycom left a comment

Choose a reason for hiding this comment

nuttycom commented Mar 9, 2024

nuttycom commented Mar 9, 2024

nuttycom left a comment

Choose a reason for hiding this comment

nuttycom left a comment

Choose a reason for hiding this comment

nuttycom left a comment

Choose a reason for hiding this comment

nuttycom left a comment

Choose a reason for hiding this comment

nuttycom left a comment

Choose a reason for hiding this comment

daira Mar 15, 2024 • edited Loading

Choose a reason for hiding this comment

nuttycom Mar 15, 2024

Choose a reason for hiding this comment

daira Mar 18, 2024

Choose a reason for hiding this comment

daira left a comment

Choose a reason for hiding this comment

Add `UnifiedIncomingViewingKey` struct #1245

Add `UnifiedIncomingViewingKey` struct #1245

AArnott commented Mar 9, 2024 •

edited

Loading

AArnott commented Mar 9, 2024 •

edited

Loading

daira Mar 15, 2024 •

edited

Loading