Page MenuHomePhabricator

finish backporting PR9602 : remove ineffective parameter from `prioritisetransaction` RPC
AbandonedPublic

Authored by markblundeberg on Dec 31 2019, 03:41.

Details

Reviewers
deadalnix
Fabien
Group Reviewers
Restricted Project
Summary
Test Plan
ninja check-all

Diff Detail

Repository
rABC Bitcoin ABC
Branch
cleanup6
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 8716
Build 15417: Default Diff Build & Tests
Build 15416: arc lint + arc unit

Event Timeline

Fabien requested changes to this revision.Jan 2 2020, 12:50
Fabien added a subscriber: Fabien.
Fabien added inline comments.
doc/release-notes.md
9 ↗(On Diff #15096)

Can you make it a bit more explicit ? e.g. by adding something like:

It is no longer possible to pass a `priority_delta` value, only `fee_delta` is supported.
This revision now requires changes to proceed.Jan 2 2020, 12:50
deadalnix requested changes to this revision.Jan 3 2020, 06:16
deadalnix added inline comments.
doc/release-notes.md
10

Core release note say:

The `prioritisetransaction` RPC no longer takes a `priority_delta` argument, which is replaced by a `dummy` argument for backwards compatibility with clients using positional arguments. The RPC is still used to change the apparent fee-rate of the transaction by using the `fee_delta` argument.

I think we should probably keep the dummy around for a while, and add a deprecated flag to handle the transition.

This revision now requires changes to proceed.Jan 3 2020, 06:16
doc/release-notes.md
10

This will break people's code using the node, which we don't want to do without giving them a fair warning and long enough to adapt.

Yeah fair enough, I was half-expecting this review. Actually the original PR 9602 did fully remove the argument, and I had no idea they restored the parameter as a dummy argument. I just invented the intermediate dummy step myself (the parent Diff) since it was a no-brainer.

The parameter was actually restored in https://github.com/bitcoin/bitcoin/pull/10252/files -- should we replicate its functionality in that "Not supported, so must be zero or null." ?