sendtoaddress ignores conf_target (spending whole UTxO with subtractfeefromamount=true) #16072

issue whitslack openend this issue on May 21, 2019
  1. whitslack commented at 10:53 pm on May 21, 2019: contributor
     0$ bitcoind --version | head -n1
     1Bitcoin Core Daemon version v0.18.0.0-gentoo
     2
     3$ bitcoin-cli estimatesmartfee 288 ECONOMICAL
     4{
     5  "feerate": 0.00001015,
     6  "blocks": 288
     7}
     8
     9$ bitcoin-cli sendtoaddress bc1… ##amount## '' '' true true 288 ECONOMICAL
    1037############################################################3e
    11
    12$ bitcoin-cli gettransaction 37…3e | fgrep fee | head -n1
    13  "fee": -0.00007924,
    

    The redacted ##amount## above was the exact amount of a single unspent output in my wallet (shown by listunspent). The resulting transaction (37…3e) was 191 bytes in length (virtual size of 110 vbytes or 437 weight units — whatever that means).

    So estimatesmartfee told me I should have paid a fee of 1.015 sat/byte for confirmation in 288 blocks, but then sendtoaddress went ahead and paid 41.487 sat/byte. Not surprisingly, the transaction confirmed in the very next block. I was aiming for it to confirm closer to 288 blocks later, to save on fees.

    This is not the first time this has happened to me. My usual routine is that I check the fee needed for a given confirmation target with estimatesmartfee, observe that the fee is reasonable, then specify the same confirmation target to sendtoaddress. In this case and others, I end up paying an exorbitant fee for an immediate confirmation.

    I am not certain whether this behavior always occurs with subtractfeefromamount=true or whether it ever occurs with subtractfeefromamount=false. I do know that I have almost never had BitcoinD choose a fee rate that has resulted in a confirmation time anywhere close to as long as I requested.

  2. VoxR4710 commented at 10:43 am on May 27, 2019: none

    I have also observed strange behaviour from this parameter and i am losing my freaking mind by now ! I made a post about it on stack here : https://bitcoin.stackexchange.com/questions/88001/subtractfeefromamount-not-consistently-working

    I am starting to think after reading your post that it’s just fundamentally broken and that we will have no choice but to do it the long way.

  3. MarcoFalke added this to the milestone 0.18.1 on May 27, 2019
  4. MarcoFalke removed this from the milestone 0.18.1 on May 27, 2019
  5. MarcoFalke added this to the milestone 0.19.0 on May 27, 2019
  6. MarcoFalke added the label Bug on May 27, 2019
  7. MarcoFalke added the label Wallet on May 27, 2019
  8. laanwj commented at 1:48 pm on September 26, 2019: member

    At a cursory glance it looks like the argument is at least used, it’s passed into the coin control object:

    0    if (!request.params[6].isNull()) {
    1        coin_control.m_confirm_target = ParseConfirmTarget(request.params[6], pwallet->chain().estimateMaxBlocks());
    2    }
    

    It doesn’t look like there is any functional test that tests, or even uses this functionality, though, so it might be bugged and simply going untested.

  9. instagibbs commented at 7:15 pm on September 26, 2019: member
    I’ll take a look later today. Nothing stands out in code, time to dig into tests.
  10. instagibbs commented at 8:15 pm on September 26, 2019: member

    @whitslack @VoxR4710 would either of you have detailed debug.log recordings that you could privately share with me?

    I can not replicate in test environment that I set up in feature_fee_estimation.py to have non-minimum relay feerate values.

  11. whitslack commented at 8:28 pm on September 26, 2019: contributor

    @instagibbs: No, sorry. My debug.log begins on 11 June, which is after I opened this issue and switched to using bitcoin-cli settxfee "$(bitcoin-cli estimatesmartfee …)" to manage my fee rate rather than trying to use the buggy conf_target parameter of sendtoaddress.

    It should be pretty easy to reproduce on mainnet whenever there’s a backlog. Just pick any UTxO from your listunspent and do a sendtoaddress with the exact amount of the UTxO, passing true for subtractfeefromamount and some very long conf_target like 144 blocks. Compare the fee rate you actually paid against the fee rate reported by estimatesmartfee 144.

  12. instagibbs commented at 8:36 pm on September 26, 2019: member
    Ah I missed the “spend whole coin” condition from the description, let me try again.
  13. instagibbs commented at 8:57 pm on September 26, 2019: member
    @whitslack notice anything off about the test I wrote? #16972 I am unable to reproduce
  14. whitslack commented at 2:14 am on September 27, 2019: contributor
    @instagibbs: Your test case isn’t exactly the case I described. I spent only one UTxO, not my entire balance. (I didn’t check the rest of the code to see whether your test wallet would have only one UTxO at that point.) Also, I spent a confirmed UTxO, whereas you appear to be spending unconfirmed UTxOs. I don’t know if either of those discrepancies are relevant.
  15. instagibbs commented at 2:21 am on September 27, 2019: member
    @whitslack the 2nd and 3rd send are spending a single utxo each, which is coincidentally the whole balance. I can change it to mine a block each time as well but I don’t expect this to change anything(unconfirmedness doesn’t do anything but possibly disqualify it from coin selection in some cases/settings)
  16. whitslack commented at 2:31 am on September 27, 2019: contributor

    @instagibbs: Oh, right; the entire balance is merged into one UTxO by the first spend.

    Is there a code-path difference between spending the entire wallet balance and spending the exact amount of only one UTxO among many? I would think that the coin selection algorithm might take a different path to pick one UTxO of exactly the requested amount versus all UTxOs in the wallet.

    Let’s lay this on the table for now. Next time I have a need to move a whole UTxO to another wallet, I’ll try to use conf_target instead of settxfee and see if I end up overpaying again.

  17. Sjors commented at 3:29 pm on September 27, 2019: member

    #11413 by @kallewoof adds a whole of bunch of tests for fee handling in sendtoaddress. Perhaps those can be adjusted to cover the behavior on master.

    That said, @instagibbs’s test in #16972 seems to cover the above scenario. I tried it on top of the v0.18.0 release tag, and it passes (using size instead of vsize). I also tried running it with --usecli (add self.supports_cli = True) in case something was wrong with:

    https://github.com/bitcoin/bitcoin/blob/2472733a24a9364e4c6233ccd04166a26a68cc65/src/rpc/client.cpp#L37-L40

    The fee rates it ends up using vary per test run:

    • target 288: 0.00024384-0.00033942 (16-19 blocks)
    • target 2: 0.00116006-0.00161520 (2 blocks)

    I find it strange that estimatesmartfee doesn’t find a lower fee when asked for a 288 block target. I suppose it’s because the test only mines 40 blocks.

    I find it hard to wrap my head around how coin selection (branch & bound, or knapsack) interacts with fees. But that’s probably where we’d have to look, having ruled out most other explanations.

  18. whitslack commented at 6:05 pm on September 29, 2019: contributor

    I just tested this with my main wallet on v0.18.0, and the behavior was correct. Possibly this issue has already been resolved. I’ll switch back to relying on conf_target rather than settxfee in my regular operations and see if I run into any more occurrences of the misbehavior.

    That said, fee estimation has some seemingly bizarre cusps. I’ve seen cases where estimatesmartfee 48 returned a feerate of 1 sat/vB but estimatesmartfee 47 returned a feerate of 20 sat/vB. The function does not seem to be very smooth at all, and it’s far from realistic. But that’s a whole separate issue.

  19. instagibbs commented at 6:30 pm on September 29, 2019: member

    It’s using bucket estimation, it doesn’t try to smooth it out.

    On Sun, Sep 29, 2019, 2:08 PM Matt Whitlock notifications@github.com wrote:

    I just tested this with my main wallet on v0.18.0, and the behavior was correct. Possibly this issue has already been resolved. I’ll switch back to relying on conf_target rather than settxfee in my regular operations and see if I run into any more occurrences of the misbehavior.

    That said, fee estimation has some seemingly bizarre cusps. I’ve seen cases where estimatesmartfee 48 returned a feerate of 1 sat/vB but estimatesmartfee 47 returned a feerate of 20 sat/vB. The function does not seem to be very smooth at all, and it’s far from realistic. But that’s a whole separate issue.

    — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bitcoin/bitcoin/issues/16072?email_source=notifications&email_token=ABMAFU6XM4KO4W2PTLK7SF3QMDVJVA5CNFSM4HOPKMLKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7326LY#issuecomment-536325935, or mute the thread https://github.com/notifications/unsubscribe-auth/ABMAFUZKTU76NEXLANP37ODQMDVJVANCNFSM4HOPKMLA .

  20. MarcoFalke removed this from the milestone 0.19.0 on Sep 30, 2019
  21. MarcoFalke commented at 11:28 pm on May 8, 2020: member

    Possibly this issue has already been resolved.

    Ok, closing for now. Please let us know if this happens again.

  22. MarcoFalke closed this on May 8, 2020

  23. whitslack commented at 1:15 am on August 11, 2020: contributor

    @MarcoFalke: Please reopen. This is still an issue, and it’s still costing me money that I didn’t want to spend.

    Using Bitcoin Core 0.20.0…

    0$ bitcoin-cli estimatesmartfee 720 ECONOMICAL
    1{
    2  "feerate": 0.00001000,
    3  "blocks": 720
    4}
    5
    6$ bitcoin-cli sendmany '' '{"##redacted_A##":#.########,"##redacted_B##":#.########,"##redacted_C##":#.########,"##redacted_D##":#.########}' 0 '' '["##redacted_D##"]' true 720 ECONOMICAL
    7d…##redacted_txid##…0
    

    The resulting transaction had a virtual size of 228 vbytes and paid 2244 satoshis as a fee, which is a rate of 9.9 sat/vB, about ten times the fee rate I expected/intended!

    Of note:

    • I specified exactly one of the output addresses in the subtractfeefrom array.
    • The sum of my specified output amounts was exactly equal to the value of the single unlocked UTxO in my wallet.
  24. fanquake reopened this on Aug 11, 2020

  25. instagibbs commented at 1:18 pm on August 11, 2020: member

    @whitslack as a stop-gap you can try using the explicit feerate options now available in master, will be available in 0.21 release: #11413

    I’ll take another crack at making a test that fails.

  26. instagibbs commented at 1:53 pm on August 11, 2020: member
    Wrote another test case for sendmany, it’s not triggering anything sorry. If you really want to get this fixed I think you’re going to have to send me(or someone that does wallet stuff) detailed logs.
  27. whitslack commented at 4:41 pm on August 11, 2020: contributor

    as a stop-gap you can try using the explicit feerate options now available in master, will be available in 0.21 release: #11413 @instagibbs: The new options look promising, though I have a sneaking suspicion that they’ll be ignored by the same code path that presently ignores the estimator-based fee options.

    Wrote another test case for sendmany, it’s not triggering anything sorry.

    Thanks for your efforts. It’s strange that this is so trivially reproducible for me using my production mainnet node. This recent occurrence with sendmany is not the only time it’s happened for me on 0.20.0.

    If you really want to get this fixed I think you’re going to have to send me(or someone that does wallet stuff) detailed logs.

    Sure. Would I collect detailed enough logs with -debug=walletdb -debug=estimatefee -debug=selectcoins? Would I need to add -debug=coindb too?

  28. instagibbs commented at 4:44 pm on August 11, 2020: member

    I think most of the wallet/fee related stuff is printed always. Look for Fee Calculation string in debug.log? Might be a starting point. My e-mail is in github profile.

    Alternative-alternative for you is to use raw interface and then cross-check that the feerate is right(ugly yes, but wrapping it means you can avoid bugs like this)

  29. whitslack commented at 5:12 pm on August 11, 2020: contributor

    I think most of the wallet/fee related stuff is printed always. Look for Fee Calculation string in debug.log? Might be a starting point. My e-mail is in github profile.

    I found the Fee Calculation for the transaction in question:

    02020-08-11T01:02:57Z [default wallet] Fee Calculation: Fee:2244 Bytes:228 Needed:2244 Tgt:720 (requested 720) Reason:"Mempool Min Fee" Decay 0.99931: Estimation: (0 - 1000) 100.00% 3390.8/(3390.8 0 mem 0.0 out) Fail: (-1 - -1) -nan% 0.0/(0.0 0 mem 0.0 out)
    

    The Tgt:720 would seem to have come from my conf_target parameter, but I don’t know what the numbers in Estimation: mean. I did show the output from estimatesmartfee 720 ECONOMICAL, which showed an estimated fee rate of 1 sat/vB, so I don’t know why the fee calculation would have calculated 2244 satoshis for a 228-vB transaction.

    Alternative-alternative for you is to use raw interface and then cross-check that the feerate is right(ugly yes, but wrapping it means you can avoid bugs like this)

    I wasn’t aware of the subtractFeeFromOutputs and replaceable arguments to fundrawtransaction. Thanks for the pointer. I will most likely use this interface in the future, as it looks easier than locking every UTxO in the wallet and then unlocking the one I want to spend with sendtoaddress/sendmany.

  30. instagibbs commented at 7:24 pm on August 11, 2020: member

    Reason:“Mempool Min Fee”

    errr are you sure you aren’t using a really small mempool or something? The wallet will definitely give you something higher if what you give doesn’t hit your mempool minimum.

    getmempoolinfo <— results of this?

  31. whitslack commented at 7:32 pm on August 11, 2020: contributor

    Current values…

    0$ bitcoin-cli getmempoolinfo
    1{
    2  "loaded": true,
    3  "size": 18232,
    4  "bytes": 8777962,
    5  "usage": 35987504,
    6  "maxmempool": 64000000,
    7  "mempoolminfee": 0.00003503,
    8  "minrelaytxfee": 0.00001000
    9}
    

    Regardless of my own mempool’s configuration, sendtoaddress/sendmany ought not to silently inflate the fees they’re paying when I have explicitly specified a fee rate. Expected behavior would be to pay the actual fee rate I specified and always relay my own transactions to my peers, even if my transactions can’t stick around in my mempool due to memory constraints. An alternative, acceptable (though not desirable) behavior would be to reject my transaction with an error saying it doesn’t pay a great enough fee rate to be accepted into my mempool. The current behavior of silently paying many times my explicitly specified fee rate is unexpected and unacceptable and undermines my trust in the software to keep my money safe.

  32. instagibbs commented at 7:36 pm on August 11, 2020: member

    Maybe, that’s an API break. I believe the explicit feerate option actually fails in this case. I’d suggest doing manual transaction creation(walletcreatefundedpsbt) or use explicit feerate.

    “maxmempool”: 64000000

    64 MB mempool could easily fill during these periods. Mystery solved.

  33. whitslack commented at 7:37 pm on August 11, 2020: contributor
    Thanks. Looking forward to the new explicit fee-rate parameters.
  34. whitslack closed this on Aug 11, 2020

  35. whitslack commented at 7:43 pm on August 11, 2020: contributor
    For what it’s worth, there is no “safe” maximum mempool size. If you say 64 MB is too small, then I say 1 GB is also “too small,” and indeed any size is too small. Under an eclipse attack, an attacker could easily fill a victim node’s mempool of any size with valid, high-fee-rate transactions, which pose no risk of confirming and costing the attacker money since the victim won’t be able to relay them to any miners due to having been eclipsed from the network.
  36. instagibbs commented at 7:51 pm on August 11, 2020: member
    The “fix” I’d look at is simply having the block estimator return the max of min mempool entry and it’s estimation. That actually seems better than current and won’t break anyone’s usage?
  37. whitslack commented at 7:54 pm on August 11, 2020: contributor

    That could be good. Then front-ends that query the estimator to show a fee rate to the user would have a more accurate idea about what will happen when they actually submit a transaction using sendtoaddress/sendmany.

    My concern would be for Lightning nodes, which need to agree with their peers about the current state of the fee market. If the responses from Bitcoin Core’s fee estimator are being influenced by mempool memory constraints, this could trigger mass unilateral channel closures, which is VERY expensive.

  38. kallewoof commented at 3:03 am on August 12, 2020: member
    I don’t believe the explicit fee rate option cares about current mempool limits, it only cares about the hard coded minimum 1 satoshi/byte limit. (I could be mistaken here – we went back and forth on this during review.)
  39. instagibbs commented at 2:10 am on August 13, 2020: member

    Looks like it will simply fail: https://github.com/bitcoin/bitcoin/blob/master/src/wallet/wallet.cpp#L2782

    though I don’t see a test for it necessarily

  40. rebroad commented at 11:30 am on April 16, 2021: contributor

    Reason:“Mempool Min Fee”

    errr are you sure you aren’t using a really small mempool or something? The wallet will definitely give you something higher if what you give doesn’t hit your mempool minimum.

    getmempoolinfo <— results of this?

    Surely the mempool minimum shouldn’t come into the equation at all…

  41. whitslack commented at 11:39 am on April 16, 2021: contributor

    Surely the mempool minimum shouldn’t come into the equation at all… @rebroad: Sadly, it does, and the reason is one of privacy. If your node isn’t relaying any transactions paying fees less than X sat/vB because its mempool is full, but then it relays one transaction that pays a fee of X/2 sat/vB, then it would be obvious to your peers that your node originated that transaction.

    Personally, I wish there were an option to say, “I care more about cost than I care about privacy; transmit my cheap transaction anyway,” but such an option does not exist.

  42. meshcollider referenced this in commit b55232a337 on Sep 28, 2021
  43. sidhujag referenced this in commit efce452c82 on Sep 28, 2021
  44. DrahtBot locked this on Aug 18, 2022

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-22 03:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me