Skip to content

Make ChangeStrategy values usable outside of the context of zcash_client_backend#2094

Merged
nuttycom merged 4 commits into
mainfrom
feature/change_strategy_external_usability
Dec 19, 2025
Merged

Make ChangeStrategy values usable outside of the context of zcash_client_backend#2094
nuttycom merged 4 commits into
mainfrom
feature/change_strategy_external_usability

Conversation

@nuttycom
Copy link
Copy Markdown
Collaborator

This exposes types and trait implementations from zcash_transparent and zcash_client_backend to make it easier to use the ChangeStrategy type in contexts outside of zcash_client_backend proposal construction.

This trait is introduced in order to make it possible to implement
`ChangeStrategy` for a type where the `fetch_wallet_meta` method
provably ignores its `meta_source` argument. This is useful for
circumstances when no input source is available.
@nuttycom nuttycom force-pushed the feature/change_strategy_external_usability branch from bf05408 to 3c96bd2 Compare December 19, 2025 20:30
Copy link
Copy Markdown
Contributor

@str4d str4d left a comment

Choose a reason for hiding this comment

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

Reviewed 3c96bd2.

Comment thread zcash_transparent/src/builder.rs Outdated
In order to use zcash_client_backend::fees::ChangeStrategy when
preparing a spend of transparent coins, it is necessary to be able to
accurately compute fees. At present, the fee computation logic in
`zcash_client_backend` only functions in the context of spending p2pkh
inputs. Instead of reimplementing script size computation externally,
this commit exposes enough information from the `zcash_transparent`
builder to enable the script size computation used by the builder to
also be used externally.
@nuttycom nuttycom force-pushed the feature/change_strategy_external_usability branch from 3c96bd2 to 2303fa4 Compare December 19, 2025 22:05
@nuttycom
Copy link
Copy Markdown
Collaborator Author

force-pushed and commit added to address comments from code review.

str4d
str4d previously approved these changes Dec 19, 2025
Copy link
Copy Markdown
Contributor

@str4d str4d left a comment

Choose a reason for hiding this comment

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

utACK 77c422f with nit

Comment thread zcash_transparent/src/builder.rs Outdated
Co-authored-by: Jack Grigg <jack@electriccoin.co>
Copy link
Copy Markdown
Contributor

@str4d str4d 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 9303994

@nuttycom nuttycom merged commit ade8b67 into main Dec 19, 2025
46 checks passed
@nuttycom nuttycom deleted the feature/change_strategy_external_usability branch December 19, 2025 22:54
@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 20, 2025

Codecov Report

❌ Patch coverage is 46.00000% with 27 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.83%. Comparing base (f7254cf) to head (9303994).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
zcash_transparent/src/builder.rs 43.18% 25 Missing ⚠️
zcash_primitives/src/transaction/builder.rs 50.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2094      +/-   ##
==========================================
- Coverage   73.84%   73.83%   -0.02%     
==========================================
  Files         222      222              
  Lines       36704    36716      +12     
==========================================
+ Hits        27103    27108       +5     
- Misses       9601     9608       +7     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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