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.

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

Murisi/sequential payment addresses #4417

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

murisi
Copy link
Collaborator

@murisi murisi commented Feb 25, 2025

Describe your changes

Implemented deterministic shielded payment address generation replacing the former method of randomly generating payment addresses. Determinism is achieved by maintaining a diversifier index for each viewing key in the wallet store and incrementing it each time a payment address is generated. Specifically, the following changes have been made:

  • Extended the wallet store with a diversifier index field
  • Implemented migration so the wallet can read older Store versions that do not contain diversifier indices
  • Extended the gen-payment-addr CLI command with a flag to set the current diversifier index for a viewing key
  • Altered payment address generation to use stored/specified diversifier index and increment it afterwards

A drawback of the approach taken is that gen-payment-addr must now take the alias of a viewing key as its argument instead of a literal viewing key. This is because it needs to have an alias to write the updated diversifier index back to. But under the assumption that end-users were not calling gen-payment-addr with literal keys, the CLI command can continue to be used in exactly the same way as before.

Checklist before merging

  • If this PR has some consensus breaking changes, I added the corresponding breaking:: labels
    • This will require 2 reviewers to approve the changes
  • If this PR requires changes to the docs or specs, a corresponding PR is opened in the namada-docs repo
    • Relevant PR if applies:
  • If this PR affects services such as namada-indexer or namada-masp-indexer, a corresponding PR is opened in that repo
    • Relevant PR if applies:

Copy link

codecov bot commented Feb 25, 2025

Codecov Report

Attention: Patch coverage is 25.86207% with 86 lines in your changes missing coverage. Please review.

Project coverage is 74.48%. Comparing base (b5f4a7d) to head (a64db62).
Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
crates/core/src/masp.rs 0.00% 47 Missing ⚠️
crates/wallet/src/store.rs 55.55% 24 Missing ⚠️
crates/wallet/src/lib.rs 0.00% 15 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4417      +/-   ##
==========================================
- Coverage   74.52%   74.48%   -0.04%     
==========================================
  Files         339      339              
  Lines      110504   110615     +111     
==========================================
+ Hits        82351    82395      +44     
- Misses      28153    28220      +67     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@murisi murisi marked this pull request as ready for review February 26, 2025 12:51
@github-actions github-actions bot added the breaking:api public API breaking change label Feb 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking:api public API breaking change MASP wallet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant