Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

UIVK carries full viewing key for transparent #264

Closed
AArnott opened this issue Mar 8, 2024 · 2 comments · Fixed by #265
Closed

UIVK carries full viewing key for transparent #264

AArnott opened this issue Mar 8, 2024 · 2 comments · Fixed by #265
Assignees
@AArnott
Labels
invalid This doesn't seem right

Comments

@AArnott
Copy link
Collaborator

AArnott commented Mar 8, 2024

Nerdbank.Cryptocurrencies/src/Nerdbank.Zcash/Transparent/PublicKey.cs

Line 69 in 5b968e0

public IIncomingViewingKey IncomingViewingKey => this;

Per ZIP-316, a UFVK to UIVK conversion should not contain the full public key for transparent accounts. It should instead contain the external scope derivation. But based on the code show above, I'd say this is missing.
@AArnott AArnott self-assigned this Mar 8, 2024
@AArnott
Copy link
Collaborator Author

AArnott commented Mar 8, 2024

A related conversation, where I introduced a similar bug, is here: zcash/librustzcash#1175 (review)

@AArnott
Copy link
Collaborator Author

AArnott commented Mar 9, 2024

This doesn't actually repro as I thought it would. Not an issue. The code I found was not relevant to UFVK conversions.

@AArnott AArnott added the invalid This doesn't seem right label Mar 9, 2024
AArnott added a commit that referenced this issue Mar 9, 2024
@AArnott
Add test proving that UFVK->UIVK conversions for transparent keys are…
e0bfae2 
… correct

Closes #264
@AArnott AArnott mentioned this issue Mar 9, 2024
Merged
AArnott pushed a commit that referenced this issue Mar 28, 2024
@dependabot
Bump dotnet-coverage from 17.10.3 to 17.10.4 (#264)
3271c78
@AArnott AArnott mentioned this issue Mar 28, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@AArnott AArnott

Labels
invalid This doesn't seem right
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add UFVK->UIVK transparent key derivation test nerdcash/Nerdbank.Cryptocurrencies
1 participant
@AArnott