Allow negative effective value inputs when subtracting fee from outputs (
wallet) Jan 5, 2022
The PR branch HEAD was a26a64c63d at the time of this review club meeting.
allows users to pass in
a transaction with outputs filled in, and the wallet “funds” the payment and transaction fees by
selecting UTXOs from the wallet’s UTXO pool to spend in the transaction’s inputs. The process of
selecting which UTXOs to use is known as
While transaction outputs have an amount,
each added input increases the fee required to keep the transaction at a given feerate. As such, the
effective value (
nValue reduced by the UTXO’s input size multiplied by the given feerate) is
used during coin selection instead.
We have discussed Coin Selection and effective values in a
previous review club.
subtractFeeFromOutputs argument in
fundrawtransaction causes the payment amounts in
the transaction to be reduced by the fee amount. We have discussed the behavior around subtracting
fee from recipients in a
previous review club.
PR #23534 changes coin selection behavior to
allow UTXOs with negative effective values to be selected when
subtractFeeFromOutputs is on.
Did you review the PR? What was your review approach?
What does the
subtractFeeFromOutputs argument do in
fundrawtransaction; how do you use it?
Why might a user opt to subtract fees from outputs?
What does it mean for a coin to have
negative effective value? Why does it usually not make
sense to select negative effective value UTXOs?
What does this PR do? What are the advantages of disadvantages of its changes?
Since this is a wallet PR: could this change cause users to pay extreme fees, end up with dust
outputs, or leak privacy? Would this be confusing to you as a user if you are using the default settings?
What does the
set change fee only if not subtracting fee from outputs, do? Does it change behavior?
AttemptSelection() know that the user wants to subtract fees from recipient outputs?
fundrawtransaction call; how does it eventually end up calling
What is the type of
srd_result a few lines below)? How does it resolve as a boolean expression in the
7a. (Bonus) Why might we use
auto instead of declaring the type explicitly here?
7b. (Bonus 2) Can you find other examples of this calling pattern in the codebase?
Why does the test need to call
Can you think of any logic that isn’t tested by the functional test added in this PR? Do you
have any suggestions to better test the changes added in this PR?
1 17:00 <glozow> #startmeeting
2 17:00 <glozow> Happy New Year! Welcome to PR review club!
3 17:00 <b10c__> hi (lurking today)
12 17:00 <glozow> Feel free to say hi, and let us know if it's your first time
14 17:01 <kalpa> first time for me
15 17:01 <rahim> hi (lurking), first time!
16 17:01 <turkycat> hi, first time. will observe
17 17:01 <janoside> Hi, first time
18 17:01 <erik> It's my first time, here
19 17:01 <stickies-v> hiya
20 17:01 <kodyl> first time (lurking)
21 17:01 <Julianmrodri> hi, first time
22 17:01 <Loris> hi, first time :)
23 17:01 <glozow> kalpa: rahim: turkycat: janoside: erik: kodyl: Julianmrodri: Loris: Welcome!
24 17:01 <tim14> first time (lurking)
25 17:01 <glozow> wow so many first-timers :D
26 17:01 <fqxjkdtbwnm5> Hi, first time :-)
27 17:01 <turkycat> just discovered via twitter
28 17:01 <glozow> Quick poll: have you reviewed the PR or read the notes? y/n?
33 17:02 <turkycat> y and y
35 17:02 <michaelfolkson> hi
36 17:02 <stickies-v> 0.5y, glossed over it
39 17:02 <glozow> For those of you who reviewed, what was your review approach?
41 17:02 <svav> y read the notes
42 17:03 <glozow> tim14: fqxjkdtbwnm5: Welcome as well
43 17:03 <tarun> read the notes reviewed the questions
44 17:03 <sipa> Looks correct to me.
45 17:03 <glozow> awesome, let's dive into the questions
46 17:03 <glozow> What does the `subtractFeeFromOutputs` argument do in fundrawtransaction; how do you use it? Why might a user opt to subtract fees from outputs?
47 17:04 <kouloumos> It subtracts the fee from the outputs. You specify from which outputs you want that fee to be subtracted and it will be equally deducted from the amount of each one.
48 17:04 <glozow> kouloumos: yes!
49 17:05 <erik> It subtracts the fee from the sending amount. It's useful for self transfers, like during wallet migration
50 17:05 <glozow> Can anyone tell us a use case for this?
52 17:05 <stickies-v> It is especially useful when sweeping a wallet, because you don't know how much fees you'll have to pay before iterating over all the inputs
53 17:05 <glozow> erik: right! since the payment amount will be reduced, we'd imagine that you wouldn't use this to pay a merchant for example
54 17:06 <glozow> stickies-v: yes! what does "sweeping a wallet" mean?
56 17:07 <stickies-v> Sending all your UTXOs to one or multiple new addresses. Breaking the piggy bank and putting everything in a shiny new one :)
57 17:07 <glozow> stickies-v: perfect
58 17:08 <glozow> next question: What does it mean for a coin to have negative effective value? Why does it usually not make sense to select negative effective value UTXOs?
59 17:08 <larryruane> the fee increase that would be required to include that input is greater than the value of the input
60 17:09 <kalpa> Do negative effective value UTXOs even exist?
61 17:09 <glozow> larryruane: correct
62 17:09 <brunoerg> the cost to move this coin is greater than its value?
63 17:09 <glozow> kalpa: yes they do! it also depends on the feerate at which you're trying to construct this transaction
64 17:09 <glozow> perhaps it would be helpful for someone to tell us what effective value means :)
65 17:09 <glozow> brunoerg: correct
66 17:09 <turkycat> utxo value - fee per utxo
67 17:09 <kalpa> oh ok understood
68 17:10 <stickies-v> effective value is the nValue of the UTXO minus (tx size * current feerate)
69 17:11 <callebtc> what's a typical threshold value for a too-small utxo (i.e. negative effective value)?
70 17:11 <glozow> stickies-v: yep exactly
71 17:11 <glozow> callebtc: good question, i think it'd be a few hundred - a few thousand sats
72 17:13 <glozow> Alright next question: What does this PR do? What are the advantages of disadvantages of its changes?
73 17:13 <stickies-v> a 2-input 2-output p2wpkh tx is 208.5 vbytes, at a feerate of 5 sats/vbyte you'd get roughly 1000sats?
74 17:13 <callebtc> btw, if I understand correctly, the problem would also propagate to the next utxo: if your effective value is just marginally positive, the output (the input of the next tx) could then have a negative effective value, correct?
75 17:14 <larryruane> stickies-v: "tx size * current feerate" - is it tx size or input size?
76 17:14 <glozow> callebtc: hm, i don't think anything would propagate to the next utxo. the problem here is, when we spend this output in our transaction (i.e. we create an input that refers to this UTXO), the size of the input * feerate is more than the UTXO's nValue
77 17:14 yeah i just realized i've been confusing the two. that should be input size, so my previous example is off too
78 17:15 <glozow> larryruane: stickies-v: oh you're right, i didn't look closely enough. yes it's the size of the input.
79 17:16 <glozow> yeah so an input to spend a p2wpkh is ~68vB, and at 5sat/vB that's about 340 sats
80 17:18 <George[m]1> What’s very strange to me is that I joined IRC as the ‘zenlo’ user which shows up here.
81 17:18 <glozow> are we okay to move on to the next question?
84 17:19 <glozow> okay: what does this PR do?
85 17:19 <callebtc> glozow: > What does this PR do? I think it allows one to spend negative EV utxo's which might be desirable if the user wants to, for example, clear out the entire wallet.
86 17:20 <kalpa> it allows us to consolidate small UTXO values?
87 17:20 <glozow> callebtc: yes, it allows us to include the negative effective value UTXOs in a transaction when `subtractFeeFromOutputs` is on
88 17:20 <glozow> kalpa: yes that's what the effect is!
89 17:20 <callebtc> Understood! Can I ask, why it wasn't possible in the first place?
90 17:21 <glozow> stickies-v mentioned sweeping wallets, so let's consider an alternative approach. Why don't we just have a `sweepwallet` RPC?
91 17:22 <glozow> callebtc: because selecting negative effective value inputs would mean you're not getting closer to funding your payment amount, just throwing away extra money to fees
92 17:22 <stickies-v> I don't think it's a very common use case, so maybe no need to clutter the RPC with an extra method when the fundrawtransaction method can already get the job done?
93 17:23 <lightlike> why is it desirable to sweep the wallet when we are actually donating fees to the miners? why not just forget about these utxos instead and move only those coins with a positive effective value?
94 17:24 <kalpa> well fundrawtransaction is the method to create a transaction so it wouldn't make sense to create another one
95 17:24 <brunoerg> lightlike: privacy?
96 17:24 <stickies-v> lightlike yeah that's my number one mystery with this entire PR, not sure why you wouldn't just leave the negative EVs behind
97 17:24 <m011> I also think fundrawtransaction method gets the job done. This only needs a parameter that use the wallet balance instead of specifying a value.
98 17:24 <glozow> lightlike: good question. i guess that could make sense to the user, but then we're cluttering the UTXO set.
99 17:24 <achow101> stickies-v: alternatively, why should we clutter all of the coin selection code with subtractFeeFromOutputs handling when a sweep function would be fairly simple to create.
100 17:24 <willcl_ark> Sometimes you just wanna use up all UTXOs
101 17:25 <rahim> lightlike: couldn't a negative effective value input become a "positive" effective value one eventually, assuming price increases?
102 17:25 <larryruane> basic question: with this sweeping use-case, the user *wants* to spend negative-effective-value UTXOs, even though doing that is uneconomical? If so, I don't see why that's desired
103 17:25 <lightlike> brunoerg: wouldn't the new solution be actually worse to privacy than just forgetting about the utxos, because it allows third partieds to connects more utxos?
104 17:25 <willcl_ark> You're also altruistically reducing the UTXO set size
105 17:25 <glozow> achow101: +1, an extra RPC method is better than complicating the coin selection logic imo
106 17:25 <stickies-v> achow101 excellent point, I don't have any counterpoints haha
107 17:26 <larryruane> (lightlike +1 you beat me to it on that question)
108 17:26 <kalpa> if btc goes to a million dollars it could make sense to sweep all dust outputs into one now
109 17:26 <glozow> rahim: good point, if you're able to create very low feerate transactions in the future, they might be positive effective value
110 17:26 <glozow> but this has nothing to do with the exchange rate with USD
111 17:27 <fqxjkdtbwnm5> @rahim, I do not think so, because everything written above is in the "unit" sats and correct independent of USD price
112 17:27 <brunoerg> lightlike: hmmmm, good point.
113 17:27 <achow101> there is some philosophical questions around whether sweeping should ignore negative ev utxos. imo users would expect that sweeping results in their wallet balance becoming 0, but not spending negative ev utxos means that there will be a non-zero remaining balance
114 17:27 <larryruane> kalpa: I'm not sure the btc price matters, because wouldnt everything (fees, etc.) scale linearly?
115 17:28 <rahim> gotcha; fees are calculated from the tx size and independent of price, right?
116 17:28 <erik> rahim: feerate is calculated in sats
117 17:28 <callebtc> Is there a technical reason why there isn't a `sweepwallet` RPC (excluding complexity, bloat, ugliness, etc)?
118 17:28 <glozow> larryruane: if i'm sweeping a wallet, i'll probably throw away my keys/backups, so i'd prefer not to leave anything behind. i'd also feel like i'm dumping plastic bags into the ocean, just not very eco-friendly to keep those in the UTXO set
119 17:29 <larryruane> glozow: +1
120 17:29 <constantin21> achow101: I think it comes down to donating dust to miners or "deflating" bitcoin supply?
121 17:29 <achow101> callebtc: not in particular
122 17:30 <glozow> okay we're halfway through the hour so i'll throw out the final conceptual question: Since this is a wallet PR, could this change cause users to pay extreme fees, end up with dust outputs, or leak privacy? Would this be confusing to you as a user if you are using the default settings?
123 17:30 <svav> Can someone give a simple explanation as to why this PR was felt necessary? If fees make it uneconomical to spend a particular UTXO, why would you ever bother spending it? What is the reason for this existence of this PR? X)
124 17:30 <glozow> (I included these in the notes as examples of what you might want to ask yourself when reviewing the PR)
125 17:31 <callebtc> I think it would be helpful to try to find an answer to the "why does this PR even exist?" questions
126 17:31 <achow101> svav: it is a bug fix for a reported bug where someone was sweeping their entire wallet but got an insufficient funds error. the root cause was found to be that their wallet contained negative ev utxos.
128 17:32 <achow101> however they were able to sweep with an older version of the software, so this is technically a regression
129 17:32 <kalpa> consolidating your UTXOs could be a privacy leak but that also happens with other coin selection algorithms
130 17:32 <constantin21> svav : It enables donating unobtainable dust to miners
131 17:33 <janoside> glozow: Pay extreme fees: no (changes seem independent of fee rate). End up with dust outputs: no (this will destroy dust outputs). Leak privacy: yes, possibly (history of the dust outputs can now be linked to the spend).
132 17:33 <glozow> kalpa: i don't think that's caused by this PR, but a side effect of consolidation.
133 17:33 <kalpa> glozow, of course
134 17:34 <glozow> janoside: same thing, this does not worsen the privacy of our coin selection implementation, it's always been the case that consolidating UTXOs links them to the same wallet
135 17:36 <glozow> any further questions about the concept? this discussion has been very lively \o/
136 17:36 <svav> If a user was sweeping their wallet, would they currently be adding each input manually?
137 17:36 <Murch> achow101: that sounds more like an issue of the wallet not distinguishing the balance and spendable balance
138 17:37 <kalpa> glozow, so this PR does not affect the gui wallet functionality correct?
139 17:37 <achow101> Murch: the spendable balance depends on the feerate though, and having a balance that constantly changes doesn't sound like good UX
140 17:37 <lightlike> but I think it does worsen the privacy of that particular transaction - using less utxos (which are not necessary/economical) should improve privacy.
141 17:37 <m011> Correct. This PR does not affect the GUI
142 17:38 <svav> Apart from sweeping a wallet, are there any other production circumstances where a coin being able to have a negative effective value would be useful/needed?
143 17:38 <glozow> kalpa: depends on what you mean. the gui wallet is the same as the wallet, so no. but nothing should change in the gui itself, so yes.
144 17:38 <glozow> lightlike: ah that is a good point
145 17:39 <kalpa> glozow, but the gui wallet does not use the fundrawtransaction RPC or does it? or is it supposed to do so in the future?
146 17:39 <Murch> achow101: Sure, but in the context of building a transaction with a specific feerate, the spendable balance would be stable.
147 17:40 <achow101> kalpa: the gui, fundrwatransaction, and the send* functions, etc. all end up using the same CreateTransaction function which contains all of the coin selection logic
148 17:42 <callebtc> does anyone know how many utxo's out there have negative EV?
149 17:42 <sipa> that depends on the feerate
150 17:42 <callebtc> And how many sats they effectively lock away?
151 17:42 <larryruane> just so everyone's aware, a sophisticated user can also choose the inputs (createrawtransaction) to have complete control (not use the built-in coin selection algorithms)
152 17:43 <glozow> svav: i can't think of any other use cases, so I'm a proponent of adding a `sweepwallet` RPC that sweeps all UTXOs. i think if this is the expected behavior, users should be able to say they specifically want to sweep
153 17:43 <callebtc> sipa: isn't a lower bound 1sat/vbyte anyway?
154 17:43 <sipa> in practice yes
155 17:44 <sipa> But the answer to the question "how many utxos have negative EV" isn't well defined - it depends on the feerate the spender wants to use.
156 17:44 <glozow> maybe one day we can lower the min relay feerate
158 17:44 <callebtc> I see. I'll rephrase: how many UTXO's have negative EV at 1sat/vB fee rate?
159 17:44 <sipa> Probably very little, because the dust rule prevents such UTXOs from being created.
160 17:45 <m011> I think adding something like "max"parameter to fundrwatransaction or send RPC can be a better approach.
161 17:45 <callebtc> I guess I'd have to ask the on chain metrics people...
162 17:45 <callebtc> sipa: I see
163 17:45 <stickies-v> sipa but the dust rule is policy right, so non-core clients may not adhere to that? and I don't think the dust policy has always been in place?
164 17:46 <sipa> Sure, but the minrelayfee of 1 sat/vbyte is also policy.
165 17:47 <stickies-v> ah, right
166 17:47 <glozow> stickies-v: non-core clients would be silly not to adhere to policy, since it would make their mempool very unreliable
167 17:47 <erik> Maybe a few very old utxos from minrelayfee times
168 17:48 <erik> But may be minor
169 17:48 <stickies-v> yeah good point - and to be clear i'm not advocating for any of this, just checking boundaries
170 17:48 <rahim> could the wallet construct the transaction, but not broadcast it until the EV is positive? Simply "waiting" doesn't seem very elegant...
171 17:48 <sipa> rahim: That's equivalent to just using a lower feerate.
172 17:48 <glozow> rahim: the feerate of the transaction wouldn't change after you've created it
173 17:48 <sipa> The feerate is choice by the user.
174 17:49 <Murch> glozow: Wouldn't they'd just have a few extra transactions below minRelayTxFeeRate? I.e. expect a few more txes to be in a block when it would otherwise not be full?
175 17:49 <rahim> gotcha, makes sense
176 17:49 <glozow> but yeah you could just wait until the feerates are low to sweep your wallet
177 17:49 <glozow> Murch: sorry who's they?
178 17:50 <rahim> glozow: Andreas Antonop has talked about that before, right
179 17:50 <Murch> People with a lower minRelayTxFeerate: how would their Mempool be "unreliable"?
180 17:50 <glozow> oh. the 1sat/vB floor doesn't change even if your mempool is empty, so if your mempool accepts something under 1sat/vB, that doesn't mean it'll propagate
181 17:50 <glozow> ideally your mempool is a good indicator of what miners have in their mempools
182 17:51 <Murch> Ah yes, in that regard they would be unreliable.
183 17:52 <glozow> alright i'll just throw out a few of the implementation questions now, since we're running out of time
185 17:53 <erik> It's std::optional
186 17:53 <larryruane> it's an optional, which means it can have a value or not, sort of replaces using a null pointer to mean no value
187 17:53 <glozow> erik: almost there, it's an optional what?
188 17:53 <stickies-v> SelectionResult
190 17:54 <glozow> larryruane: yep, true if there's a `SelectionResult`, false if it's a nullopt
191 17:54 <glozow> but on that line, why use `auto`?
192 17:54 <erik> Optional can be used in expressions, if some data is returned it return true, false otherwise
193 17:54 <larryruane> you can test an optional value as if it's a boolean, but a more explicit way is to call `has_value()` (i think)
194 17:54 <erik> Like Some() in Rust
195 17:55 <sipa> Or Just in Haskell.
196 17:55 <glozow> erik: yeah exactly
197 17:55 <erik> glozow: Auto instructs the compiler to infer the type
198 17:55 <erik> With is std::optional
199 17:55 <erik> *infer based on the assigment
200 17:55 <glozow> erik: yup
201 17:56 <glozow> Can you find other examples of this calling pattern in the codebase? We use it quite often
202 17:56 <kalpa> what does bnb and srd stand for?
203 17:56 <glozow> kalpa: Branch and Bound, Single Random Draw. coin selection algos
204 17:57 <larryruane> you can use the indirect (*) to fetch the value or you can be more explicit and call the `value()` method
205 17:57 <sipa> Super pedantic nit: auto isn't "the compiler inferring the type"; it is specified by the language what the type is; just implied by the expression rather than being explicit (I say this because it isn't like compilers that are smarter can be better at this or so).
206 17:57 <brunoerg> interesting
207 17:58 <glozow> sipa: right
208 17:58 <glozow> and you won't accidentally cast something
210 17:59 <glozow> alrighty the rest of the questions are left as an exercise to the PR reviewer :) thank you all for coming!
211 17:59 <glozow> #endmeeting