Fix broken bumpfee method

[rajarshimaitra]

Description

This fixes the issue I pointed at bitcoindevkit/bdk#427. I found way around to get the script_pubkey for and output without a need of get_tx() function.

Added one extra argument to bump_fee to select the utxo whose output is to be reduced.

Notes to the reviewers

Please check the bump_fee function manually. I have checked them locally and it works. But there is no unit tests for these.

Checklists

All Submissions:

• I've signed all my commits
• I followed the contribution guidelines
• I ran cargo fmt and cargo clippy before committing

[rajarshimaitra]

Rebased on master

[notmandatory]

LGTM but I still need to manually test. Found a small typo that should be fixed.

[rajarshimaitra]

Thanks. Updated..

[notmandatory]

I've tried manually testing and need to confirm my scenario is right,

1. create a spending all transaction with create_tx --enable_rbf -a --to tb1ql7w62elx9ucw4pj5lgw4l028hmuw80sndtntxt:0
2. broadcast the transaction and sync my wallet
3. now I see it in the mempool
   https://mempool.space/testnet/tx/35aab0d0213f8996f9e236a28630319b93109754819e8abf48a0835708d33506
4. I tried to bump my single output with bump_fee --vout 0 -t 35aab0d0213f8996f9e236a28630319b93109754819e8abf48a0835708d33506 -f 6

But this gives me an error UnknownUtxo. If I DO have an unspent change output I'm able to specify it and then the bump fee works. Is the purpose of this PR is to be able to bump a single output TX or a TX with a change output?

[rajarshimaitra]

But this gives me an error UnknownUtxo. If I DO have an unspent change output I'm able to specify it and then the bump fee works. Is the purpose of this PR is to be able to bump a single output TX or a TX with a change output?

I think what is happening here is tb1ql7w62elx9ucw4pj5lgw4l028hmuw80sndtntxt is an outside wallet address, so wallet.get_utxo() will not see that utxo in its own list.

That's the intended behavior though. The wallet should not be able to reduce an outsider recipient's amount by bumping fee.

It can only bumfee if its has a change output, or a single output with wallet internal address (in case of sweeping funds).

Can you check the single output case with an address derived from the wallet? It worked for me as far as I remember.

But this gives me an error UnknownUtxo

I think the error message can be improved here. It could say "not allowed to reduce amount of non wallet address" or something like that.

[notmandatory]

Thanks, ok yes it makes sense now that I couldn't reduce the output of an address I don't control. Is the purpose of this sort of transaction with all sats spent to one of my own wallet addresses be for some sort of UTXO consolidation?

[rajarshimaitra]

Yes mostly for utxo consolidation in the same wallet, or shifting the full balance from one wallet to another.

But in the second case we can't bumpfee, because such sweeping transactions won't have a change. There the option is to CPFP the transaction from the recipient wallet to increase mining priority.

I think I have an idea for selecting the wallet utxo automatically, now that we have get_tx() available in bdk. The user should not have to specify the utxo to reduce amount. The wallet has all the data to figure that out. Maybe I will do that one a subsequent PR, and we can get rid of the change_vout field introduced here.

In most of the cases the tx will have a change. And we can handle the special case of "send all funds to an outside address" with a dedicated error message.

But not sure if that's too fancy for the bdk-cli tool. 😅

[notmandatory]

Sorry now that I'm looking at it closer, why can't we reduce the output amount for a single output to an address we can't control? In the pre-0.10.0 code it looks like that's what we were doing. I'm going to play around with it a little and see if I can get what I want to work. :-)

[notmandatory]

I think I have a better approach in #45, please take a look and let me know what you think.

[rajarshimaitra]

Closed in favor of #45