Skip to content

Conversation

@jonatack
Copy link
Member

@jonatack jonatack commented Nov 24, 2020

Continuing work on #19543 and following the plan outlined in #20484 (comment), deprecate the feeRate option in fundrawtransaction and walletcreatefundedpsbt to avoid confusion with fee_rate, as the two options have different units (BTC/kvB and sat/vB) and similar spellings.

This PR currently targets deprecation for 0.21 and so no release notes are added here, as the wiki would be edited instead. Will update if tagged for 0.22 instead.

The last commit, "test: add feeRate tests to rpc_deprecate.py," is best reviewed with -w.

Related to #20391.

@DrahtBot
Copy link
Contributor

DrahtBot commented Nov 25, 2020

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@maflcko
Copy link
Member

maflcko commented Nov 26, 2020

Do we need to be aggressive in deprecating this? The estimatesmartfee is still in BTC/kB, so merging this would force all users to do the conversion themselves (or force to pass -deprecatedrpc).

@jonatack
Copy link
Member Author

jonatack commented Nov 26, 2020

Several reviewers gave feedback that this would be good to fix. If that includes updating estimatesmartfee for a coherent whole, then, like this patch, it's fairly trivial to write and review. If reviewers prefer to hold off, I'll update this patch for 0.22.

Update: currently following the plan outlined in #20484 (comment).

@maflcko
Copy link
Member

maflcko commented Nov 30, 2020

Several reviewers gave feedback that this would be good to fix

Where? I am the first one to comment on this pull.

Regardless, the feature freeze of 0.21 has been a month ago, so I stand by my NACK (at least for 0.21). For 22.0, this could be reconsidered.

@jonatack
Copy link
Member Author

Where? I am the first one to comment on this pull.

In #20305.

I agree it makes sense to do this fix as part of the described larger plan, but I hope users won't find it too confusing between now and when the fix arrives in a release.

@maflcko maflcko removed this from the 0.21.0 milestone Dec 3, 2020
@maflcko
Copy link
Member

maflcko commented Dec 3, 2020

Removed milestone for now

@jonatack
Copy link
Member Author

jonatack commented Jan 3, 2021

Update: currently following the plan outlined in #20484 (comment).

This pull proposed to deprecate the feeRate option in fundrawtransaction and walletcreatefundedpsbt to avoid user confusion with fee_rate, as the two params have different units (BTC/kvB and sat/vB) and very similar spellings. However, review has been negative, the plan is currently somewhat stalled, and there has been no interest in resolving this. I think it was necessary to at least propose a solution, can re-open if/when there is demand.

@jonatack jonatack closed this Jan 3, 2021
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants