feat: added bumpfee rpc

[sander2]

This adds the bumpfee rpc. Since versions < 21 interpret the fee_rate field differently, the serialization got slightly more complicated.

[tcharding]

ACK e4a5f71

[stevenroose]

Looks good, left one comment regarding the FeeRate type.

[sander2]

Thanks for the review guys. @stevenroose I updated the PR according to your comment

[sander2]

Thanks @tcharding. Sorry for the slow response, I was on holiday. I updated the PR now. I think this PR is ready to merge now - could someone merge please?

[sander2]

Actually the tests are failing because bumpfee changed from version 0.18. See the rpc doc for 0.18 and 0.19. There were 2 changes:

• change from a totalFee argument to a fee_rate argument
• change in behavior: 0.18 can only bump fees by decreasing the return-to-self output, whereas from 0.19 on, new inputs will be added if required.

These changes make it very difficult to provide a uniform interface that works in all versions of bitcoin core. I would propose to only support bumpfee for versions 0.19 and up by returning an error in the rpc for versions < 0.19. Thoughts?

[tcharding]

I would propose to only support bumpfee for versions 0.19 and up by returning an error in the rpc for versions < 0.19. Thoughts?

I'm don't feel confident to answer this, will wait for @stevenroose for ideas.

[stevenroose]

Actually the tests are failing because bumpfee changed from version 0.18. See the rpc doc for 0.18 and 0.19. There were 2 changes:

* change from a `totalFee` argument to a `fee_rate` argument

* change in behavior: 0.18 can only bump fees by decreasing the return-to-self output, whereas from 0.19 on, new inputs will be added if required.

These changes make it very difficult to provide a uniform interface that works in all versions of bitcoin core. I would propose to only support bumpfee for versions 0.19 and up by returning an error in the rpc for versions < 0.19. Thoughts?

It should be possible to write a custom deserializer using a stub struct that has both field optionally and then build the real return struct from that? I think we do that in some other methods.

[sander2]

Actually the tests are failing because bumpfee changed from version 0.18. See the rpc doc for 0.18 and 0.19. There were 2 changes:

* change from a `totalFee` argument to a `fee_rate` argument

* change in behavior: 0.18 can only bump fees by decreasing the return-to-self output, whereas from 0.19 on, new inputs will be added if required.

These changes make it very difficult to provide a uniform interface that works in all versions of bitcoin core. I would propose to only support bumpfee for versions 0.19 and up by returning an error in the rpc for versions < 0.19. Thoughts?

It should be possible to write a custom deserializer using a stub struct that has both field optionally and then build the real return struct from that? I think we do that in some other methods.

The problem is not to build the return struct - the problem is that these 2 versions of the rpc behave differently.

What we'd need to do in version 0.18, is to convert the fee_rate argument to a total_fee based on size of the transaction, and to call bumpfee with that. That's relatively straightforward. But this rpc will fail if the return-to-self output can not be reduced sufficiently to achieve that fee, since 0.18 will not add new inputs (unlike 0.19 and up). I assumed that you'd want the library to behave identically for all bitcoin versions. But maybe this is not the case?