pr15557-notes.md

My process:

1. Open PR in browser
2. Checkout PR in dev environment
3. Build
4. Run integration tests
5. During and after 2-4, make notes on what I need to review
6. Answer what questions I can by looking at the code
7. Start regtest node (or testnet or mainnet if necessary) and review actual operation matches my expectation from the code

Notes:

Build ok; tests passed: ALL | ✓ Passed | 1590 s (accumulated)
Runtime: 406 s

• "important to reuse all previous inputs"

  • Added tests don't seem to cover this (don't see it in previous tests either)

• "cannot source new unconfirmed inputs"

  • Added tests don't seem to cover this (don't see it in previous tests either)

• seems to use a new key for each bump in some cases? If I bump 2,000 times, does that mean I go beyond Bitcoin Core's default gap limit and could end up missing funds on a restore the wallet seed? Related to discussion at: https://github.com/bitcoin/bitcoin/pull/15557/files#r271956393

• RPC documentation says that it will add a change output if one doesn't exist. Will it also remove a change output if it's not neccessary per Bitcoin Core's usual remove-if-not-economical policy? (Probably unrelated to this specific PR.)

• PR title refers to bumpfee RPC; how does this affect the bump fee option in the GUI wallet? (Code changes seem to be in functions that are shared between the two, but I can test this later.)

• test_small_output_with_feerate_succeeds only tests the addition of a single input (1 vin -> 2 vins); could manually test to 2->3 or more.

• What happens if I call bumpfee with an old txid? E.g. I hit the up arrow in my terminal and accidentally repeat an old bumpfee command from an earlier incarnation that had fewer inputs than the later version? When it rolls over from n to n+1 inputs again, is there a risk that it'll select a different input the second time? Whatever the repeat behavior is (fail, success, warning), it was probably set when this call was first designed and isn't redefined with this change, however this change has additional risk if the same inputs aren't always used.

• What happens if I have spend zero conf change on and I spend the change output from a tx that I later try to bumpfee? (Not related to this PR, just curious.)