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

zcash_address 0.3.2: ZIP 320 TEX address support #1226

Merged
merged 3 commits into from
Mar 6, 2024
Merged

zcash_address 0.3.2: ZIP 320 TEX address support #1226

merged 3 commits into from
Mar 6, 2024

Conversation

str4d
Copy link
Contributor

@str4d str4d commented Mar 5, 2024 •
edited
Loading

Closes #1220.
Closes #1221.

nuttycom
nuttycom approved these changes Mar 5, 2024
View reviewed changes
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

@nuttycom nuttycom self-requested a review March 5, 2024 15:39
nuttycom
nuttycom requested changes Mar 5, 2024
View reviewed changes
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.

This also needs 4992ff7

@str4d
Copy link
Contributor Author

str4d commented Mar 5, 2024

Force-pushed to include the regtest prefix from @daira's review on #1135.

nuttycom
nuttycom requested changes Mar 5, 2024
View reviewed changes
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.

@str4d the location you originally flagged still needs to be fixed.

@str4d
Copy link
Contributor Author

str4d commented Mar 5, 2024

Force-pushed to fix the regtest issue in impl ToAddress for ZcashAddress now that we have regtest prefixes.

@str4d str4d mentioned this pull request Mar 5, 2024
Open
nuttycom
nuttycom previously approved these changes Mar 5, 2024
View reviewed changes
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

@codecov Codecov
Copy link

codecov bot commented Mar 5, 2024 •
edited
Loading

Codecov Report

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

Project coverage is 63.92%. Comparing base (16e3d1b) to head (d539f04).

Files Patch % Lines
components/zcash_address/src/convert.rs 0.00% 12 Missing ⚠️
components/zcash_protocol/src/consensus.rs 57.14% 3 Missing ⚠️
components/zcash_address/src/lib.rs 0.00% 2 Missing ⚠️
components/zcash_address/src/encoding.rs 96.66% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1226      +/-   ##
==========================================
- Coverage   63.97%   63.92%   -0.06%     
==========================================
  Files         115      115              
  Lines       11895    11930      +35     
==========================================
+ Hits         7610     7626      +16     
- Misses       4285     4304      +19

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

@str4d
Copy link
Contributor Author

str4d commented Mar 5, 2024

Force-pushed to fix the other bug after a regtest prefix was defined.

nuttycom
nuttycom previously approved these changes Mar 5, 2024
View reviewed changes
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

daira
daira previously approved these changes Mar 6, 2024
View reviewed changes
Copy link
Contributor

@daira daira 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 dismissed stale reviews from daira and nuttycom via ba3dcd5 March 6, 2024 14:56
@str4d
Copy link
Contributor Author

str4d commented Mar 6, 2024 •
edited
Loading

Rebased on main to fix merge conflicts with #1142 and include its changes in zcash_address 0.3.2, as they are also suitable for a point release (the new dependency on the new zcash_protocol crate doesn't affect the public API).

@str4d
Copy link
Contributor Author

str4d commented Mar 6, 2024

Force-pushed to update the zcash_protocol changelog.

nuttycom
nuttycom previously approved these changes Mar 6, 2024
View reviewed changes
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 9758bee

@str4d
Copy link
Contributor Author

str4d commented Mar 6, 2024

Force-pushed to add a zcash_protocol 0.1.0 release commit prior to the zcash_address 0.3.2 release commit.

nuttycom
nuttycom approved these changes Mar 6, 2024
View reviewed changes
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 8130359 into main Mar 6, 2024
68 of 69 checks passed
@nuttycom nuttycom deleted the tex-addrs branch March 6, 2024 15:40
daira
daira reviewed Mar 6, 2024
View reviewed changes
Copy link
Contributor

@daira daira left a comment

Choose a reason for hiding this comment

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

Post-hoc ACK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daira daira daira left review comments

@nuttycom nuttycom nuttycom approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

zcash_address 0.3.2 zcash_address: Implement ZIP 320
3 participants
@str4d @nuttycom @daira