wallet: GetEffectiveBalance #26365

pull willcl-ark wants to merge 3 commits into bitcoin:master from willcl-ark:effective_balance changing 8 files +100 −0

willcl-ark commented at 3:12 PM on October 21, 2022: contributor
This is a rough draft of what a GetEffectiveBalance function on the wallet might look like, along with RPC call and wallet interface. Related to #15767

Still todo and open questions:
- RPCHelp wording is just a rough draft and needs amending
- Should the result contain additional info such as: fee rate used, num utxos, num excluded utxos etc. (this would better satisfy #15767 too)
- Should confirm target be configurable? Currently fixed to a 3 blocks.
  
  In this case the response should likely include the confirm target used to generate the effective value.
- Should this be added to QT wallet? Interface is present. I think it would be nice but don't know much about QT.
- ~Required adding a lot of headers to receive.cpp which seems sub-optimal. Perhaps the function belongs somewhere else...~
- Doesn't add a dummy recipient to the tx, so fee calculation is slightly off
- ~Doesn't respect avoid_reuse~
- Doesn't respect watch_only
- RBF just uses wallet default
DrahtBot added the label Wallet on Oct 21, 2022
willcl-ark force-pushed on Oct 21, 2022
DrahtBot commented at 1:49 PM on October 22, 2022: contributor


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #26756 (wallet: Replace GetBalance() logic with AvailableCoins() by w0xlt)
- #26699 (wallet, gui: bugfix, getAvailableBalance skips selected coins by furszy)
- #25659 (wallet: simplify ListCoins implementation by furszy)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
wallet: create GetEffectiveBalance 68ee33076b
wallet: add getEffectiveBalance interface bcb45d2ef3
wallet: add geteffectivebalance RPC d6517427d9
in src/wallet/rpc/coins.cpp:221 in 3778f9d70d outdated
```
 212 | @@ -213,6 +213,59 @@ RPCHelpMan getbalance()
 213 |      };
 214 |  }
 215 |  
 216 | +RPCHelpMan geteffectivebalance()
 217 | +{
 218 | +    return RPCHelpMan{"geteffectivebalance",
 219 | +                "\nReturns the total effective balance.\n"
 220 | +                "The effective balance is what the wallet considers currently spendable at a\n"
 221 | +                "confirmation target of 3 blocks at current feerate.\n",
```
luke-jr commented at 5:05 PM on October 27, 2022:

Why? This seems quite arbitrary.

willcl-ark commented at 6:43 PM on October 27, 2022:

Correct, it's totally arbitrary for now but I think making it configurable woudl make it more interesting/useful.

The idea (from #15767 ) is that Bitcoin-qt (primarily) could display some estimate of the actual value that can be sent, after fees. In this case, we could either present the GUI with this "estimate" (3 confirmations seems estimate-ish to me), or there could perhaps be an option/drop-down for user to select which fee style they would like the estimate to use.

For an RPC, user could specify the fees they wanted used.

Anyway, for now I am just working out whether it makes much sense at all, and the best way to architect it (e.g. I started with it in spend.cpp as this made logical sense, but ended up introducing circular dependencies so moved into receive.cpp ...)
willcl-ark force-pushed on Oct 31, 2022
murchandamus commented at 4:22 PM on November 1, 2022: contributor

Concept ACK

I think it's useful to be able to tell users how much they could actually spend. A three-block target or six-block target seems like a reasonable default, but I'd expect this RPC call to be more useful if it could also take a custom feerate.
aureleoules commented at 10:46 AM on November 2, 2022: member

Concept ACK

I agree that there should be a configurable fee rate and/or configurable block target. How about adding an RPC arg effective to getbalance instead of creating a separate RPC?
DrahtBot added the label Needs rebase on Jan 18, 2023
DrahtBot commented at 8:43 PM on January 18, 2023: contributor



🐙 This pull request conflicts with the target branch and needs rebase.
willcl-ark commented at 12:12 PM on March 14, 2023: contributor

Going to close this as there are other, cleaner and better-maintained version(s) of it being worked on now.
willcl-ark closed this on Mar 14, 2023
bitcoin locked this on Mar 13, 2024

Contributors

Labels

Linked (view graph)

#15767 Display sendable (effective) balance

wallet: GetEffectiveBalance #26365

Conflicts