-
Notifications
You must be signed in to change notification settings - Fork 249
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
zcash_client_backend: Move min_confirmations
into Proposal
#1019
Conversation
Codecov ReportAttention:
... and 1 file with indirect coverage changes 📢 Thoughts on this report? Let us know!. |
4dbd6e7
to
0fb65a5
Compare
0fb65a5
to
c084174
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main changes in the PR LGTM.
@@ -81,12 +81,39 @@ pub struct Proposal<FeeRuleT, NoteRef> { | |||
sapling_inputs: Vec<ReceivedSaplingNote<NoteRef>>, | |||
balance: TransactionBalance, | |||
fee_rule: FeeRuleT, | |||
min_confirmations: NonZeroU32, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-blocking: this propagates the mistake I made in suggesting to use a NonZeroU32
for min_confirmations
, when we actually need the ability to set this to zero for shielding transactions. Non-blocking because it just moves around the problem, and #873 is about fixing it.
c084174
to
e1892b7
Compare
This fixes an API issue whereby it was possible to execute a `Proposal` with a different value of `min_confirmations` than that with which the `Proposal` was constructed.
e1892b7
to
4cd26b7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
Builds upon #1018
This fixes an API issue whereby it was possible to execute a
Proposal
with a different value of
min_confirmations
than that with which theProposal
was constructed.