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

Distinguish seed relevance when no derived accounts are present #1287

Merged
merged 2 commits into from
Mar 19, 2024

Conversation

str4d
Copy link
Contributor

@str4d str4d commented Mar 19, 2024

During wallet migration in particular, the absence of any accounts is expected, and all seeds should be treated as relevant (because accounts cannot be added before a wallet is initialized).

@str4d str4d added this to the Librustzcash Zashi 1.0 milestone Mar 19, 2024
@str4d str4d force-pushed the zcs-distinguish-seed-relevance-with-no-accounts branch from 860e286 to 03f072c Compare March 19, 2024 03:55
@str4d str4d force-pushed the zcs-distinguish-seed-relevance-with-no-accounts branch from 03f072c to adaa2b5 Compare March 19, 2024 03:59
Copy link

codecov bot commented Mar 19, 2024

Codecov Report

Attention: Patch coverage is 81.81818% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 63.48%. Comparing base (5b3ebca) to head (3c1e82a).

Files Patch % Lines
zcash_client_backend/src/data_api.rs 0.00% 2 Missing ⚠️
zcash_client_sqlite/src/lib.rs 92.85% 1 Missing ⚠️
zcash_client_sqlite/src/wallet/init.rs 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1287      +/-   ##
==========================================
+ Coverage   63.46%   63.48%   +0.02%     
==========================================
  Files         121      121              
  Lines       13890    13904      +14     
==========================================
+ Hits         8815     8827      +12     
- Misses       5075     5077       +2     

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

nuttycom
nuttycom previously approved these changes Mar 19, 2024
Copy link
Contributor

@nuttycom nuttycom left a comment

Choose a reason for hiding this comment

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

utACK

@str4d str4d force-pushed the zcs-distinguish-seed-relevance-with-no-accounts branch from adaa2b5 to cbfc3b8 Compare March 19, 2024 17:44
@str4d
Copy link
Contributor Author

str4d commented Mar 19, 2024

Force-pushed to address comments from review and a subsequent pairing.

nuttycom
nuttycom previously approved these changes Mar 19, 2024
Copy link
Contributor

@nuttycom nuttycom left a comment

Choose a reason for hiding this comment

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

utACK with nonblocking suggestions.

zcash_client_sqlite/src/lib.rs Outdated Show resolved Hide resolved
zcash_client_sqlite/src/lib.rs Outdated Show resolved Hide resolved
zcash_client_sqlite/src/lib.rs Outdated Show resolved Hide resolved
zcash_client_sqlite/src/lib.rs Outdated Show resolved Hide resolved
str4d added 2 commits March 19, 2024 17:53
During wallet migration in particular, the absence of _any_ accounts is
expected, and all seeds should be treated as relevant (because accounts
cannot be added before a wallet is initialized).
@str4d str4d force-pushed the zcs-distinguish-seed-relevance-with-no-accounts branch from cbfc3b8 to 3c1e82a Compare March 19, 2024 17:54
@str4d
Copy link
Contributor Author

str4d commented Mar 19, 2024

Force-pushed to address second review comments.

Copy link
Contributor

@nuttycom nuttycom left a comment

Choose a reason for hiding this comment

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

re-utACK

@nuttycom nuttycom merged commit 35a60ab into main Mar 19, 2024
23 of 26 checks passed
@nuttycom nuttycom deleted the zcs-distinguish-seed-relevance-with-no-accounts branch March 19, 2024 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants