Calculate fees in getblock using BlockUndo data (
rpc/rest/zmq) Sep 30, 2020
The PR branch HEAD was 1298f0fca7 at the time of this review club meeting.
A transaction’s fee is not set explicitly. The fee can be implicitly
calculated from the difference between total input value and the total output
This PR updates the
getblock RPC to include an extra “fee” field for transactions whenever:
verbosity >= 2
Undo data (which can be used for efficient fee calculation) exists for that block
We discussed block undo data and how it’s stored on disk in
a previous review club meeting.
Some extra changes were added to prevent fuzz test failures; we discussed
fuzz testing and how it’s used in Bitcoin Core in
a previous review club
How are we using it for fee calculation?
Where else is it used?
Why doesn’t a coinbase transaction have Undo data?
What’s the IO cost to read Undo data for a block?
How could we calculate the fee if Undo data didn’t exist for a block, and
what’s the IO cost?
If there’s a txindex?
If there’s no txindex?
What other options are there (e.g. asserts), and when should we use each
Knowing the transaction fee is useful, but does only returning it in some
circumstances actually provide a better client experience, or is it
preferable for clients to always get exactly what they expect?
Should we also calculate the fee if Undo data is missing but a
At the moment, the fact that we might return “fee” is undocumented. Is it
worth increasing the complexity of the documentation to say “we SOMETIMES
return the fee; here’s when we do so”?
We add some extra checks to prevent integer overflow in fuzz tests. But does
it make sense for fuzz tests to generate inputs that violate function
1 17:00 <robot-dreams> #startmeeting
4 17:00 <robot-dreams> Hi everyone, welcome!
6 17:00 <stacie> hi! first timer here
11 17:00 <robot-dreams> Welcome Stacie!
12 17:00 <gloriazhao> ooo hello stacie so excited to see you here!!!!
13 17:01 <stacie> Thanks everyone! :)
14 17:01 <robot-dreams> Friendly reminder that all questions are welcome! Feel free to jump in at any point
16 17:01 <troygiorshev> hi!
17 17:01 <robot-dreams> Anyone else here for the first time?
18 17:01 <elle> yeah, my first time :)
19 17:01 <robot-dreams> Welcome Elle :)
20 17:01 <jrawsthorne> Second time, first time not being late
22 17:02 <robot-dreams> Congrats jrawsthorne and great to have you back!
23 17:02 <robot-dreams> And last quick poll before we get into the questions, who's had a chance to review the PR? y/n
33 17:03 <amiti> n! I've taken a look but haven't properly reviewed
36 17:04 <robot-dreams> Awesome!! Hopefully it was a relatively short / readable change, but I think there are some interesting underlying ideas.
37 17:04 <robot-dreams> All good Amiti, thanks for taking a look!
38 17:04 <robot-dreams> All right, let's jump in.
39 17:04 <robot-dreams> First off, what is `CTxUndo`?
41 17:05 <jrawsthorne> utxos spent by a transaction
42 17:05 <sunon> Class containing undo information for a CTransaction and its txins
43 17:05 <dhruvm> It's a convenience data structure used to unlink a block during IBD
45 17:06 <robot-dreams> Excellent! I would agree with all of that
46 17:06 <robot-dreams> How are using `CTxUndo` for fee calculation in the PR?
47 17:06 <felixweis> its a representation of the rev* data in your blocks/ directory. it allows to restore an UTXO in the set if we need to "rewind" the blockchain because a longer branch was found. remember, all spends from the shorter block chain removed the info (amount, bitcoin script)
48 17:08 <dhruvm> If CTxUndo is available at the call site for TxToUniv, we pass it in and use it to compute sum(outputs)-sum(inputs)
49 17:08 <felixweis> CTxUndo got introduced with UltraPrune invented by sipa and shipped in Bitcoin-Qt 0.8
50 17:08 <robot-dreams> dhruvm exactly! And question to anyone, which part of this computation involves the undo data?
51 17:09 <elle> the sum of the value of the inputs
52 17:09 <robot-dreams> elle: yes, exactly
53 17:09 <Paul_R> hi, excuse my tardiness.
54 17:09 <robot-dreams> felixweis thanks for the extra context!
55 17:09 <Paul_R> is there anyway I can read the backlog of this session?
56 17:10 <robot-dreams> hi Paul_R, welcome!
57 17:10 <Paul_R> thx robot-dreams
58 17:10 <robot-dreams> It'll be posted on the website after the session, so you'll be able to see the full log
60 17:10 <robot-dreams> felixweis: btw thanks for writing the commit that added this!
61 17:11 <felixweis> thanks *you* for picking it back up!
62 17:11 <robot-dreams> Quick question to make sure we're all on the same page, why doesn't a coinbase transaction have Undo data?
63 17:12 <sunon> There are no txins right?
64 17:12 <dhruvm> Because there's no UTXO
65 17:12 <sipa> no effective txins
66 17:12 <elle> It doesnt have an input that needs to be undone
67 17:12 <robot-dreams> sunon dhruvm sipa elle exactly! sounds like we're all on the same page here
68 17:12 <sipa> there is a CTxIn in a coinbase, but it's not actually spending any preexisting UTXO
69 17:12 <sunon> Ah yeah so not a relevant txin
70 17:13 <robot-dreams> An interesting follow-up question (outside the scope of this session) is: what are all the uses of the coinbase CTxIn?
71 17:13 <robot-dreams> And finally, where else is CTxUndo used? dhruvm already mentioned unlinking a block during IBD; is there anything else?
72 17:14 <sunon> Extra nonce space?
74 17:15 <sunon> I found it being used in UpdateCoins() in validation but have no idea why to be honest.
75 17:15 <sipa> that's just where it's constructed
76 17:16 <jrawsthorne> For the filter index
78 17:17 <robot-dreams> sunon: Yup, I agree there's not a lot of context in UpdateCoins itself, but I found looking at where UpdateCoins itself got called to be helpful
79 17:17 <felixweis> BIP34 encodes the block height in the coinbase scriptSig field. so coinbase txids are still unique. previously they sometimes did override old coins.
80 17:18 <jrawsthorne> For previous question, although not related to consensus pools use it to mark their blocks
81 17:19 <robot-dreams> jrawsthorne Nice, that seems like another interesting use of the undo data! Something else we can follow up on if we have time later, "what exactly is the filter index and how does that work"
82 17:19 <robot-dreams> But let's proceed for now.
83 17:19 <robot-dreams> This might be my favorite question for today's session:
84 17:19 <robot-dreams> What's the IO cost to read Undo data for a block?
85 17:19 <Paul_R> by override, could you equally say the over-wrote the old coins? ie. they ceased to exist anymore? seems like i would have heard of that bug by now, so i'm expecting override to not mean overwrite
87 17:20 <Paul_R> felixweis
88 17:21 <sipa> Paul_R: read BIP30
89 17:21 <dhruvm> `UndoReadFromDisk` would imply it's not expected to be in memory and is being read from disk when/if the block has not been pruned.
90 17:21 <Paul_R> @sipa ok
91 17:21 <dhruvm> But I haven't actually looked inside that function yet
92 17:22 <robot-dreams> dhruvm: Yeah, I agree with that! We are gonna be hitting disk.
93 17:22 <felixweis> I believe its fairly low thanks to the excellent design of UtraPrune™. we only need to read a single additional file which has around ~100 kB of data per block to get all the amount information of all the txins spend in that particular block.
94 17:23 <dhruvm> And this is only in response to a privileged rpc call, so there's probably no abuse concerns.
95 17:24 <robot-dreams> felixweis exactly!
96 17:24 <dhruvm> We don't need to worry about cascading issues even if the read is slow.
97 17:24 <robot-dreams> dhruvm great point, we probably don't want a P2P message that says "hey peer, read from disk"
98 17:24 <sipa> i mean: that exists
99 17:24 <Paul_R> where can i read more about priveleged rpc calls (ie. examples of such)
100 17:24 <sipa> it's called getdata
101 17:25 <robot-dreams> sipa: I realized that shortly after I said my previous statement
102 17:25 <jrawsthorne> I can get your node to give me the entire blockchain over and over again right
103 17:25 <robot-dreams> Yes jrawsthorne! I stand corrected
104 17:25 <sipa> Paul_R: he means RPC is only ever exposed to privileged software, so we don't need to worry about DoS potential, i assume
105 17:26 <dhruvm> sipa: yes, that's what i meant
106 17:26 <robot-dreams> We'll edit that in post :)
107 17:26 <sipa> there is no distinction between privileged and unprivileged RPC - it's just that RPC is only intended to be exposed to trusted software
108 17:26 <sipa> (it can send a shutdown RPC too...)
110 17:27 <robot-dreams> One follow-up question here is, "what are the DoS mitigation mechanisms getdata has that this RPC doesn't"
111 17:27 <dhruvm> we mitigate the DoS potential of getdata disk access using peer reputation though, i imagine?
112 17:27 <sipa> dhruvm: nope, the only protection that currently exists is just that an attacker would need to waste N bytes bandwidth to make us read N bytes from disk
113 17:28 <dhruvm> huh, interesting!
114 17:28 <robot-dreams> So it's a matter of symmetry between cost to the attacker and cost to the node?
115 17:28 <sipa> (which is one of the reasons why BIP37 was disabled by default - it enabled an attacker to ask for a block to be read from disk without pretty much any bandwidth at all)
116 17:29 <robot-dreams> felixweis: going back to IO cost, yup! We read a contiguous section of a `rev?????.dat` file, which contains the serialized `CTxUndo` data (a vector of UTXO info)
117 17:30 <robot-dreams> I got an estimate of 64kb by adding up the size of all the rev* files and dividing by the number of blocks. Same order of magnitude as your 100kb estimate.
118 17:30 <@jnewbery> there is some (limited) protection from -maxuploadtarget. It's more to protect bandwidth, but I think probably also protects disk reads indirectly
119 17:32 <jrawsthorne> Didn't know about that flag
120 17:32 <robot-dreams> dhruvm: great point to consider DoS potential and sipa: jnewbery: thanks for the extra context on this!
121 17:33 <robot-dreams> Any final thoughts on DoS before we talk about a world without Undo data?
122 17:33 <lightlike> sipa: don't we only process a single GETDATA block request each cycle in ProcessMessages - this should mitigate DOS concerns a bit.
123 17:33 <robot-dreams> Thanks lightlike!
124 17:34 <emzy> How to test this PR manually. Like with bitcoin-cli? I tested "getblock" and will get an error.
125 17:34 <sipa> lightlike: somewhat, indeed
126 17:34 <robot-dreams> OK moving onto the next question, how could we calculate the fee if Undo data didn’t exist for a block, and what’s the IO cost?
127 17:35 <jrawsthorne> If we had txindex we could find each input by txid and output index
128 17:35 <emzy> ' bitcoin-cli getblock "$(bitcoin-cli getblockhash 650683)" 2 ' I will get a 'core_write.cpp:251 (TxToUniv) Internal bug detected: 'MoneyRange(fee)''
129 17:36 <robot-dreams> emzy: Great catch, thanks for finding that! I definitely want to look into that afterwards.
130 17:36 <robot-dreams> jrawsthorne: Great! How does that cost compare to reading `rev?????.dat`?
131 17:37 <robot-dreams> And jrawsthorne described the case where we already have a txindex. How could we get the fee if we didn't turn on txindex?
132 17:39 <@jnewbery> without an undo file, you'd need to somehow find the txouts in the block files, and they'd be scattered across many blocks
133 17:39 <jrawsthorne> A lot more expensive, with undo we can read it all at once but with this we'd have to get each tx individually and then find the correct output
134 17:39 <jrawsthorne> Is there a function for just getting a specific output or is it just the whole tx?
135 17:39 <robot-dreams> jnewbery: jrawsthorne: right! Even with a txindex that doesn't sound too fun
136 17:39 <sipa> jrawsthorne: not even
137 17:40 <sipa> you'd need to read through the entire blockchain
138 17:40 <nehan> i think you'd have to scan the whole blockchain
139 17:40 <dhruvm> So `TxToUniv` seems to have access to `CTransaction`. `CTransaction` has `vins` and `vouts`. `vouts` have amount, but `cTxIn` does not - it points to prevout.
140 17:40 <felixweis> i dont think you can find the the prev_out txs without scanning trough every block
141 17:40 <robot-dreams> With a txindex: for each spent UTXO we'd need one read from LevelDB, then one read from disk
142 17:40 <sipa> indeed, that's exactly what the txindex is for
143 17:40 <robot-dreams> sipa: felixweis: Yeah, I'd agree that without a txindex you'd just have to scan
144 17:40 <nehan> you could do one scan looking for all the outputs
145 17:40 <sipa> without it, you need to reconstruct it from scratch
146 17:41 <sipa> nehan: basically building the txindex :)
147 17:41 <robot-dreams> All right, so it sounds like using Undo data is a better strategy than "scan the entire blockchain" :)
148 17:41 <dhruvm> little bit
150 17:41 <felixweis> nehan: yes building a list first of all inputs and then scaning through the chain is a lot mor efficient than the other way around :-)
151 17:41 <robot-dreams> OK, going onto some more open-ended and possibly stylistic questions here.
152 17:42 <robot-dreams> What does CHECK_NONFATAL do, and how does it compare to other options we might have like asserts?
153 17:42 <nehan> question: why not put amounts and/or scriptpks in inputs, besides space? (or is it just space)
154 17:42 <robot-dreams> When should we consider using each option?
155 17:42 <jrawsthorne> throws a specific exception that is caught by the rpc code and sent as an error in the response
156 17:43 <jrawsthorne> if a condition is not met
157 17:43 <nehan> given that all nodes store undo data anyway, why not just... put it in the blocks?
158 17:43 <sipa> nehan: define "put it in the blocks"?
159 17:43 <dhruvm> nehan: when the same data is stored in two places, could it increase the possibility of bugs where the two are inconsistent?
160 17:43 <sipa> on disk? in p2p? in commitment structure?
161 17:44 <nehan> sipa: good question! hmm. i guess in this case, in the blocks
162 17:44 <nehan> that would help with reorgs
163 17:44 <sipa> nehan: you haven't answered my question :p
164 17:44 <robot-dreams> nehan: Interesting design question! Yeah, "if we were redesigning from scratch would we just put rev????? data right next to block data"
165 17:44 <nehan> oh, i meant on disk
166 17:45 <felixweis> you mean the blk????? files
167 17:45 <sipa> nehan: i'd say arguably we do
168 17:45 <sipa> it's just split between blk and rev files
169 17:45 <sipa> so that the rev files can be deleted sooner
170 17:45 <nehan> what about in p2p?
171 17:45 <nehan> i guess the answer to that is bw
172 17:45 <stacie> when talking about txindex, is it defined as `block_height:index_of_tx_in_block` or `txid:index_of_output` (in terms of finding more details on a vin). I'm guessing it's the former?
173 17:46 <robot-dreams> jrawsthorne: Yup! Returning an error instead of crashing the node.
174 17:46 <sipa> nehan: well, that's what utreexo (and cory's predecessor idea to it) do ;)
175 17:46 <nehan> sipa: yes, was just thinking of that
176 17:46 <Paul_R> @nehan 'bw'?
177 17:46 <sipa> bandwidth
179 17:47 <robot-dreams> OK, next is a "UX" question: Knowing the transaction fee is useful, but does only returning it in some circumstances actually provide a better client experience, or is it preferable for clients to always get exactly what they expect?
180 17:48 <felixweis> stacie: index is I believe txid -> block_height
181 17:49 <robot-dreams> stacie: Yeah, great question! I agree with felixweis, I think specifically the txindex tells you the file where you can get the block data, as well as some relevant offsets into the file to find the data
184 17:50 <lightlike> another question: while this PR shows the fees for each tx: would it maybe be useful to also display the total amount of fees spent in a block in the getblock RPC?
185 17:50 <sipa> it's txid -> (block_file_num, offset_of_block_in_file, offset_of_tx_from_block)
186 17:50 <@jnewbery> internally, the index actually stores txid->diskpos (ie where in a block file the tx can be found)
188 17:51 <robot-dreams> sipa: your recollection agrees with what you previously wrote in the linked StackExchange answer above!
189 17:51 <felixweis> so pretty much the location of the first byte of the transaction on disk?
190 17:51 <elle> lightlike: i think that that can already be implied from the coinbase transaction? coinbaseAmt minus blockreward
191 17:52 <nehan> sipa: i don't know what you mean by the third option, "commitment structure". input data is already committed to in a transaction. somewhere else?
192 17:52 <@jnewbery> oops actually the name of the object is CDiskTxPos
193 17:52 <robot-dreams> lightlike: elle: great points! I don't have enough context to know whether we want to provide the extra convenience of such a calculation, or assume RPC clients can do that on their own
194 17:53 <sipa> nehan: this may be a bit of a tangent, but i think it's easiest to think of blocks and transactions as abstract data structures that are defined by their commitment structure (so a transaction is defined by its txid, witness transaction by its wtxid, block by its block hash, ...), and how/what is exactly stored/sent is implementation detail
195 17:54 <lightlike> elle: right, that makes more sense :-)
196 17:54 <sipa> nehan: so in that sense, sennding input data along with blocks isn't making it part of "the block" - it's just auxiliary data that happens to be sent/stored along with it
197 17:55 <robot-dreams> All right, we have 5 minutes left.
198 17:55 <dhruvm> robot-dreams: in response to the UX question. when representing the same data in two ways(in this case as fee, and sum(op) - sum(ip)), i worry what happens if a bug makes the two inconsistent. which representation is truth for the client? should a low-level protocol only represent the bare minimum and leave the rest to clients? should the protocol minimize computation needed from clients?
199 17:55 <dhruvm> tough question.
200 17:55 <robot-dreams> At this point does anyone have any general questions?
201 17:55 <@jnewbery> 5 minutes left. Don't be shy!
202 17:56 <sipa> dhruvm: what do you mean by representing in that context? the fee is never stored explicitly afaik
203 17:57 <jonatack> if anyone's interested, there was a related review club session in April, "Flush undo files after last block write (validation)" https://bitcoincore.reviews/17994
204 17:57 <dhruvm> sipa: I mean from the client's perspective. If the fee and sum(op)-sum(ip) ever disagree, the clients should prefer the latter. But explicitly sending them the fee makes it possible that they might not?
205 17:57 <felixweis> I'm curious about how people feel about Question 6. Should we just return the fee always and not introduce another "verbosity" level? even if there are speed penatlys for the rpc?
206 17:58 <robot-dreams> felixweis: Yeah, very interesting question. On a similar note I'm also curious if people think we should calculate the fee when a txindex (but not block undo data) is available.
207 17:59 <@jnewbery> robot-dreams, I don't think that's a realistic scenario. The txindex requires the entire blockchain (i.e. no pruning)
208 17:59 <felixweis> It would of course not be possible if you have your node pruned beyond the requested block but then wold you request getblock anyway?
209 17:59 <sipa> dhruvm: what is "the fee"
210 17:59 <sipa> dhruvm: the fee is defined as sum(out)-sum(in), so i don't know how they could be different
211 17:59 <@jnewbery> so if you have txindex, you should also have all the undo files. We only prune undo files when we prune the equivalent block files
212 17:59 <robot-dreams> jnewbery: felixweis: fair points!
213 18:00 <robot-dreams> All right, we're out of time for today's session.
214 18:00 <robot-dreams> Thanks so much everyone for joining!
215 18:00 <dhruvm> sipa: yeah i guess it's just my ocd when representing the same data in two ways. I've experienced bugs from that in the past. Perhaps not something to worry about.
216 18:00 <robot-dreams> Especially those of you who were able to join us for the first time, welcome again, and I hope to see you around at future sessions!
217 18:00 <@jnewbery> Thanks robot-dreams!
218 18:00 <felixweis> thanks robot-dreams! and everynone else participating and lurking.
219 18:00 <nehan> thanks robot-dreams!
220 18:00 <sipa> dhruvm: but i still don't understand what you mean by representing
221 18:01 <stacie> thanks robot-dreams:! Learned a lot
222 18:01 <robot-dreams> #endmeeting
223 18:01 <sipa> the fee isn't stored
224 18:01 <emzy> thanks robot-dreams
225 18:01 <troygiorshev> thanks robot-dreams!
226 18:01 <lightlike> thanks!
227 18:01 <sipa> it's computed as sum(out)-sum(in) whenever needed
228 18:01 <kristie14> thanks! this was great as a lurker
229 18:01 <Paul_R> thanks robot-dreams
230 18:01 <elle> thanks robot-dreams!
231 18:01 <felixweis> the fee is implicit
233 18:01 <dhruvm> sipa: right. if there's a future bug in that logic is what I'm concerned about.
234 18:01 <jonatack> thanks robot-dreams, great session!
235 18:01 <sipa> dhruvm: in what logic?
237 18:02 <robot-dreams> dhruvm: Am I understanding that you're talking about "fee for the entire block vs. fee for each individual transaction"?
238 18:02 <dhruvm> robot-dreams: no, not that.
239 18:04 <robot-dreams> Got it, thanks
240 18:05 <emzy> robot-dreams: You have time for my problem?
241 18:05 <dhruvm> sipa: I am thinking of a future scenario where the server computes fee=sum(op)-sum(ip) incorrectly due to a bug. Now the client receives fee that is different than the formula.
242 18:05 <robot-dreams> emzy: I will in a bit!
243 18:05 <felixweis> emzy: i got the same problem
244 18:06 <emzy> felixweis: good to know :)
245 18:06 <dhruvm> sipa: But I can also see how that's just paranoia. If there's a bug, we'll fix it. It's not going to be at the storage level since like you said we are not storing anything differently.
246 18:08 <sipa> dhruvm: who is the server and the client? fees are never sent explicitly
247 18:08 <dhruvm> sipa: rpc server, rpc client
249 18:09 <sipa> the rpc client never computes anything
250 18:09 <sipa> so there can be discrepancy
252 18:10 <dhruvm> i see. because it merely prints it out.
253 18:10 <jonatack> dhruvm: the only client-side computation occurs in src/bitcoin-cli.cpp: -getinfo, -netinfo, -generate
254 18:11 <dhruvm> what about when the rpc client is third party software?
255 18:11 <sipa> dhruvm: if they implement fee calculation incorrectly, then they have a bug and should fix it
256 18:11 <sipa> i don't see how that's any worse (or better) than doing whatever else they're doing incorrectly
257 18:11 <dhruvm> right, that's if we let them do the computation. in this case we are choosing to do the computation for them.
258 18:12 <sipa> dhruvm: i think you're focussing too much on a consistency problem here; the goal isn't consistency - it's correctness
259 18:12 <sipa> ask yourself: would it be better or worse if somehow both the server and the client simultaneously introduce the same "bug"
260 18:12 <sipa> if it's better, then it's a consistency problem
261 18:12 <sipa> if it's worse, then it's a correctness proble
262 18:13 <sipa> the goal isn't that two computations end up with the same result per se - the goal is that the fee is correctly calculated, everywhere, and that implies that all implementations will come to the same conclusion
263 18:13 <sipa> but just ending up with the same conclusion isn't the actual goal
264 18:14 <dhruvm> oh wow, that's a mindset shift. that makes a TON of sense.
265 18:15 <sipa> something like bitcoin block validation is in many ways not a correctness problem - there are plenty of weird deviations from the consensus rules that could be accidentally made, as long as everyone makes the same deviations
266 18:15 <sipa> but here "the fee" is a well defined thing, and if you implement something that doesn't match the definition, you're wrong, period
267 18:15 <dhruvm> because there's a mutual understanding of the protocol, and that's what matters more than any implementation detail.
268 18:15 <emzy> sipa: but someone needs to be the reference.
269 18:16 <sipa> emzy: i hereby decree that The Fee is defined as the sum of transaction outputs' values minus the sum of the values of the outputs spent by the transaction's input.
272 18:16 <dhruvm> emzy: i think what sipa is saying is that it's well understood sum(op)-sum(ip) is the definition of fee. now, if there's a bug on rpc server, or rpc client, that doesn't matter as much.
273 18:16 <emzy> Yes this is a easy one.
274 18:17 <dhruvm> Thank you for teaching us, sipa!