Serve BIP 157 compact filters (
p2p) Nov 20, 2019
Recall from the review club meeting for
that we discussed BIP 158
which defines compact block filters.
BIP 157 is the
proposed specification for requesting and sending compact filters between nodes
on the p2p network.
This PR is an implementation of BIP157, such that full nodes with filter indexing
enabled can serve those filters to peers that ask
nicely :-) Questions
Did you review the PR?
Concept ACK, approach ACK, tested ACK, or
(Don’t forget to put your PR review on GitHub.)
How does a node signal to the network that it can serve compact block filters?
What is the behavior of a node that has not finished generating and indexing
compact filters for every block in the chain?
Does this PR expose any new denial-of-service attack vectors? If so, how are
they being handled? Could this be improved?
What is the purpose of the
Can Bitcoin Core be used to
retrieve compact block filters from another peer?
How is the test coverage in this PR? Do you think it could be improved, and how?
Are there other ways to test this PR outside of the usual testing framework?
1 13:00 <jnewbery> #startmeeting
9 13:01 <pinheadmz> Hi all, is this anyone's first review club meeting?
10 13:01 <michaelfolkson> Hi
11 13:01 <jnewbery> Thanks to pinheadmz for writing notes and hosting. I'll hand straight over to him
13 13:01 <miketwenty1> This is my first meeting
15 13:02 <andrewtoth> hi my first meeting
16 13:02 <pinheadmz> oh thanks jimpo for attending - he's the author's of todays PR
19 13:02 <tuxcanfly> hello
20 13:02 <pinheadmz> Ok so for new friends: don't be afraid to ask questions! at any time, we'll get to you.
21 13:03 <pinheadmz> and thanks tuxcanfly for attending, he's working on bip157/158 in bcoin and knows a little something about cfilters
22 13:04 <pinheadmz> Cool, so does someone want to summarize the PR in a few lines?
23 13:04 <fjahr> It makes the filters from bip157/158 available on the P2P layer
24 13:05 <pinheadmz> fjahr: yes!
25 13:05 <pinheadmz> so bip158 filters are a compact way to represent the contents of a block
27 13:05 <jonatack> from the bitcoiner meetup in Paris :)
28 13:06 <pinheadmz> and bip157 is the specification on how those filters can be requested and delivered around the netwrk
29 13:06 <pinheadmz> jonatack: ooh lala!
30 13:06 <pinheadmz> Did everyone get a chance to review the PR? How about a quick y/n from everyone
31 13:06 <pinheadmz> maybe tell us how far you got in review?
34 13:06 <michaelfolkson> y
35 13:07 <jnewbery> y. high-level review. Didn't do detailed review
37 13:07 <jkczyz> y - most code but didn't get through the tests
38 13:07 <fjahr> y, had looked at it a while back, did it again this week, wish I had more time for testing
40 13:08 <amiti> ish- took a look but didn't dig in
41 13:08 <pinheadmz> Great! Lets go through the questions -- how does a node signal to the network that it is able to serve CFilters ?
43 13:08 <pinheadmz> michaelfolkson: do you wanna take this one? ^
45 13:08 <fjahr> there is a service bit signalling bip158 support
47 13:08 <andrewtoth> NODE_COMPACT_FILTERS service bit
48 13:09 <michaelfolkson> Signals NODE_COMPACT_FILTERS yup
49 13:09 <miketwenty1> cfilters = compact filters?
50 13:09 <pinheadmz> miketwenty1: yes
51 13:09 <jnewbery> (it's initially slightly confusing that that's defined in BIP 158)
52 13:10 <pinheadmz> miketwenty1: bip158 defines cfilters or compact filters or casually "neutrino filters"
53 13:10 <jimpo> jnewbery: Yeah... I agree :-/
54 13:10 <miketwenty1> thanks pinheadmz
55 13:11 <pinheadmz> does anyone NOT understand what cfilters are or why they are important? maybe we should check in on that
56 13:11 <nobody123> is there a class which handles all the p2p networking?
57 13:12 <pinheadmz> nobody123: the net_processing.cpp file is where most of the messaging logic is handled - is that what you mean?
58 13:12 <jnewbery> nobody123: not really a class. Look in net.cpp for the low-level peers and connections management, and then in net_processing.cpp for the application-level logic
59 13:13 <nobody123> jnewbery, pinheadmz thx
60 13:13 <pinheadmz> Ok, so on the topic of the service bit -- Did anyone notice the sort of open-ended question about this service bit as it relates to this PR?
61 13:13 <pinheadmz> (hint: question #3)
62 13:14 <sanoj> in terms of how to do this without blocking net threads and not signal to peer peers until the indexing is in sync?
63 13:14 <pinheadmz> sanoj: yes!
64 13:14 <pinheadmz> As far as I understand, the service bits are all set on launch and aren't really designed to ever change during run time
65 13:14 <pinheadmz> jimpo: is that correct? ^
68 13:15 <pinheadmz> sanoj: right - so what happens once a node has finished indexing? Can it start serving filters right away?
69 13:15 <jimpo> this would be the first case of a dynamically changing service flag (if that idea makes it through) review, hence the discussion
70 13:15 <andrewtoth> From jimpo in the PR: "And if I understand correctly, we re-advertise our local address with service bits to each peer every day on average, so the network should see the change eventually without the node operator having to do anything." Where does this happen?
71 13:15 <headway-translat> pinheadmz: purpose of compact filters is to create a space-efficient proof of txns non-inclusion in a block so that light clients have cheaper access to truth. e.g. lightning light clients can space / bandwidth efficiently certify that channel closure txns etc aren't in blocks
72 13:16 <jnewbery> my point was that service bits should advertise *capabilities*, not *current node state*. That's my understanding at least
73 13:16 <jimpo> andrewtoth: looking
74 13:16 <pinheadmz> headway-translat: great summary, yes
75 13:16 <pinheadmz> jnewbery: thats a good point
76 13:17 <jnewbery> the service bits are somewhat long-lasting. They're gossipped in VERSION messages, and served on DNS seed responses I believe
77 13:18 <jimpo> andrewtoth: From my reading AdvertiseLocal + nNextLocalAddrSend
78 13:18 <jnewbery> so if I start by telling my peers that I don't support a feature, that'll get gossipped around the network for some time after I change my service bit flags
79 13:18 <andrewtoth> jimpo: thanks
80 13:18 <pinheadmz> I suppose the question for light clients would be: if you request a cfilter and the node respnds with notfound or null -- is that reason to disconnect or ban? Because that node could still just be syncing
81 13:18 <jimpo> jnewbery: My argument would be that that's blurry. A node is incapable of serving filters it hasn't indexed yet.
82 13:20 <andrewtoth> How can a light client verify that the cfilters are correct?
83 13:20 <michaelfolkson> So jamesob makes the comparison with IBD, is how that is dealt with instructive here?
84 13:21 <fjahr> andrewtoth: the client can compare cfilters from different nodes
85 13:21 <pinheadmz> andrewtoth: good question - until the cfilters are committed in the block somehow (and therefore verified by all network nodes and ensured by proof of work) - there is no way to know without checking the block yourself
87 13:22 <pinheadmz> andrewtoth: fjahr: but yes, BIP157 specifies a process by which a light node can be "pretty sure" the filters they get are correct
88 13:22 <jimpo> pinheadz: Even checking the block yourself is insufficient. You'd need chainstate or undo data. :-/
89 13:22 <michaelfolkson> You could have two flags. One saying that you intend to provide them and one saying you're not ready to provide them? After a certain period of time the gossip would ignore the second flag?
90 13:22 <pinheadmz> jimpo: interesting - why would that be? aren't the cfilters noncontextual ?
91 13:22 <jimpo> jnewbery: Yes, that is my concern. That allowing a valid case where a node advertises that it can serve filters and is unable to would complicate client logic.
92 13:23 <jimpo> pinheadmz: There was a change to the BIP last year that made them contextual. Specifically, the filters include the prevout output scripts, which are not included in blocks, only referenced by inputs.
93 13:23 <Talkless> hi, are these filters are against txids, scripts (addresses), both?
94 13:23 <jimpo> just scripts. Block output script and input (prev output) scripts.
95 13:24 <Talkless> thanks
96 13:24 <pinheadmz> jimpo: wow ok! extra work for the node... tuxcanfly did you know that? lol
97 13:24 <jnewbery> I think you need that logic anyway. In an untrusted p2p network, you don't know who is advertising that they can serve the filters: malicious nodes, buggy implementations, stalling node, unreliable connections, ... A BIP 157 client has to be able to handle cases where it gets invalid or missng reponses
98 13:24 <andrewtoth> michaelfolkson: what would be the difference to just signaling the first bit and then disconnecting after the ignore time time if they don't start providing?
99 13:25 <jimpo> jnewbery: Sure, but you just ban. This is a case where you need to disconnect from a *properly behaving* node.
100 13:26 <pinheadmz> jnewbery: youre thinking the signal bit should always be active, and if a node can't reply with a filter, they get banned by the client?
101 13:26 <Talkless> jimpo: but that's only about tx'es inside actual block, transactions queued in mempool will not be delivered through this mechanism to the light wallets?
102 13:26 <sanoj> jimpo: what about a state that returns some sort of sync message. I wouldn't imagine it'd happen that often but it would avoid the disconnect for proper behavior and maintain the validity of the capabilities service bit
103 13:26 <jimpo> Probably what a client should do is fetch the cfcheckpts to see how in-sync the node is
104 13:26 <pinheadmz> Talkless: thats my understanding
105 13:26 <jimpo> if nodes advertise the service bit before they have an in-sync index
106 13:27 <jnewbery> A bit more on the contextual/non-contextual stuff: the new scripts created in the block can obviously be constructed from the block. The scripts spent in the block require a node that has the UTXO set or the 'undo data' for the block
107 13:27 <pinheadmz> Talkless: wasabi wallet, for example, uses neturino filters but ALSO connects to Tx relay to discover "0-conf"
108 13:27 <Talkless> pinheadmz: interesting.
109 13:27 <jnewbery> ('undo data' == outputs spent in a block)
110 13:28 <michaelfolkson> andrewtoth: I suppose it depends whether you are concerned about maintaining connections or not. If you're not then sure advertise that you offer something and get disconnected when you disappoint
111 13:28 <miketwenty1> undo data = data needed in case of a reorg?
112 13:29 <pinheadmz> miketwenty1: yep, the coins spent by a block - so they are "unspent" if a blockis disconnected
113 13:29 <michaelfolkson> I would think you would generally want to minimize disconnections when both parties are honest and responsive
114 13:29 <pinheadmz> I like jimpo idea that light clients should request cfheckpts first to determine how in sync a node is
115 13:30 <pinheadmz> then the service bit could stay on, like jnewbery said to "indicate capability"
116 13:30 <jimpo> just... but why?
117 13:30 <jimpo> like I don't understand why dynamically switching on a service flag is such a bad idea
118 13:31 <Talkless> will new light wallet have to download all filters from the beginning, or could start from it's own "birthday" if wanted?
119 13:31 <jimpo> if it could flap, sure that's not good. but it's just a one-way transition from off to on.
120 13:31 <miketwenty1> just a clarification q.
121 13:32 <miketwenty1> pinheadmz "checkpts"?
122 13:32 <pinheadmz> jimpo: I was wondering about the tiny amount of time it would take a fully synced node to produce the cfilter... there could be a moment where even a synced node can't provide the filter for the tip ?
123 13:32 <andrewtoth> Talkless: I believe this PR introduces a way to request a range of filters, so it can start at birthdate
124 13:32 <jnewbery> I personally don't like it for a couple of reasons: service bits are handled by our net layer, and I don't like coupling together net with the compact blocks index. second: I think there's a general assumption that service bits don't change and I'd prefer not to break that
125 13:32 <jimpo> nope, we block on the background thread in that case
126 13:32 <jimpo> until the filter is built
127 13:32 <pinheadmz> jimpo: 👍
128 13:32 <headway-translat> Talkless CMIIR but i believe how the encoding works, to have higher degree of soundness, client would need to request filters several blocks before their bday
129 13:32 <pinheadmz> miketwenty1: so we'll get to checkpts i none sec...
130 13:33 <headway-translat> CMIIW*
131 13:33 <pinheadmz> s/in one...
132 13:33 <Talkless> andrewtoth: thanks
133 13:33 <Talkless> headway-translat: interesting, any ideas why?
134 13:34 <pinheadmz> headway-translat: Talkless: There is also the filter-headers, which commit to the previous block
136 13:35 <pinheadmz> IIUC, both filter headers and checkpoints are needed just because we can't commit the filter the in the block yet
137 13:35 <headway-translat> yeah I suppose I am combining the two proofs
138 13:35 <headway-translat> erroneously
139 13:35 <andrewtoth> jnewbery: re service bits not changing, what is relying on this assumption?
140 13:36 <pinheadmz> lets talk about the checkpoints - does anyone want to explain the purpose of the cfcheckpt message ?
141 13:36 <michaelfolkson> andrewtoth: I think jnewbery answered this earlier. The incorrect service bit would percolate around the gossip network and be a pain to replace
142 13:37 <jimpo> well, let's be clear. A service bit can change on a node -- it just requires a restart currently.
143 13:37 <jimpo> and still does take a while for the new setting to propagate.
144 13:37 <andrewtoth> yes, that's what I was thinking, a restart would change and gossip would still be out of date
145 13:38 <jimpo> the difference here is that a service bit cannot (yet) change over the lifetime of a *connection*
146 13:38 <pinheadmz> tuxcanfly: are you still with us? do you want to explain the cfcheckpt message?
147 13:38 <jnewbery> andrewtoth: when peer addresses are advertised in ADDR messages, that peer's service bits are included, so you can think of other nodes caching the peer's service capabilities
148 13:39 <jnewbery> jimpo: right. If you consider switching nodes off/on then anything can change!
149 13:39 <miketwenty1> q. are checkpoints ephemeral or forever?
150 13:40 <miketwenty1> im not clear on the purpose of the checkpoint
151 13:40 <jkczyz> pinheadmz: the checkpoints allow clients to parallelize getcfilters from peers
152 13:40 <miketwenty1> in the context of this BIP
153 13:40 <pinheadmz> jkczyz: ty! yes
155 13:40 <pinheadmz> miketwenty1: so thats an interesting q, because the checkpoints are based on 1000-block intervals from the requested "stop" block
156 13:41 <pinheadmz> so its possible that every time you make that request, you get a different series back
157 13:41 <pinheadmz> (jimpo right?)
158 13:41 <jimpo> no, 1000-block intervals from the start block
159 13:41 <pinheadmz> sorry thanks
160 13:41 <jimpo> so they are the same on every request so they can be cached easily
162 13:41 <pinheadmz> just mentions the stophash ...?
163 13:42 <jimpo> the start block is 0
165 13:43 <pinheadmz> So if I'm a brand new light client, what order do I send the BIP157 messages in?
167 13:44 <tuxcanfly> pinheadmz: sorry afk a bit, but yeah I remember coming across getcfcheckpt as a way to sync cfilters in batches
168 13:44 <tuxcanfly> because earlier you had to make a request per block... for each cfilter!
169 13:44 <jimpo> eek, the client-operation in the BIP is still out of date and assumes the old outpoint filter spec
170 13:44 * jimpo covers eyes
171 13:44 <pinheadmz> tuxcanfly: ty! a nice optimization
172 13:44 <jimpo> Need to finish re-writing that
173 13:45 <tuxcanfly> yeah you even had two types of filters so that would have been double the number of p2p messages for each client
174 13:45 <pinheadmz> jimpo: 😆
175 13:46 <sanoj> do i understand correctly that if we are syncing to block 600,001, the last checkpoint we'd receive covers up to 600,000?
176 13:47 <pinheadmz> sanoj: yes that sounds right, but then presumably when we request the actual filters (in parallel) we would get that last 1 as well
177 13:47 <sanoj> if so, what happens if there is a reorg that spans that 1000 filter header mark?
178 13:47 <pinheadmz> because the light client knows the block headers on the best-work-valid chain
179 13:49 <jnewbery> sanoj: 'checkpoints' might be a bit confusing terminology here because we also have checkpointed blocks in Bitcoin. Here, cfcheckpoints are just an optimization for downloading the full set of compact block filters for the chain
180 13:49 <miketwenty1> i fell into this group initially^
181 13:49 <pinheadmz> Ok home stretch! lets start wrapping up --
182 13:49 <jimpo> as a clarification, they help parallelize the download of the filter headers
183 13:50 <jimpo> then the filter headers help parallelize the download of the actual filters
184 13:50 <pinheadmz> Q#6 - does this PR enable bitcoin core to RETRIEVE filters from peers ?
185 13:50 <jnewbery> once a client has downloaded all the compact block filters initially, then checkpoints have no further function
186 13:50 <sanoj> i see. Thanks all.
187 13:50 <jnewbery> jimpo: right - thanks for the clarification!
188 13:51 <miketwenty1> so, @jn
189 13:51 <michaelfolkson> What was implementing Neutrino on Bcoin like pinheadmz? Did you consider similar issues? Anything you will revisit now it is (almost) fully implemented on Core?
190 13:51 <miketwenty1> jnewbery to answer @sanoj's question .. you would just undo data.. and redownload compact filters?
191 13:52 <pinheadmz> michaelfolkson: tuxcanfly might wanna field this one - so far we have bip158 implemented but actually not bip157 yet
192 13:52 <jkczyz> pinheadmz: Nope, with this PR it only sends filters. It does not request or handle receiving them
193 13:52 <michaelfolkson> pinheadmz: Oh interesting. Delayed because of Core timetable or just lack of resources?
194 13:52 <pinheadmz> jkczyz: thats right!
195 13:53 <jnewbery> miketwenty1: light clients are not keeping track of the UTXO set, so they don't need undo data to roll back during a reorg
196 13:53 <pinheadmz> We have the spec and the bip157 process, but since bitcoind is not intended to be a light client, it doesn't need to request the filters
197 13:53 <pinheadmz> (although in a former pr review club, we DID discuss how bip158 filters are useful internally for the wallet)
198 13:53 <jkczyz> jnewbery, jimpo: Peers can send notfound in response to getdata. Would something analogous be suitable for when the filters are not ready? Is there some general error mechanism in Core's P2P layer?
199 13:53 <jnewbery> they do need to keep track of confirmations, so they'd need to work out whether any transactions that they're interested in had been removed from the block chain during the reorg. That doesn't require undo data
200 13:54 <jimpo> jkczyz: I remember some discussion of getting rid of NOTFOUND. Where did that end up?
201 13:54 <jnewbery> jkczyz: I agree that NOTFOUND seems appropriate here
202 13:54 <jnewbery> jimpo: not that I'm aware of. Perhaps you're thinking of REJECT messages?
204 13:54 <pinheadmz> jimpo: i thought that discussion was about mempool leaks?
205 13:55 <tuxcanfly> michaelfol: to answer your bcoin question - it was pretty straightforward... however there were some bip updates which needed to be synced... the test vectors were helpful in that regard, to make sure we're core compatible for every edge case
206 13:55 <pinheadmz> er, nm - i might be confused
207 13:55 <fjahr> could also implement something like NOTREADY?
209 13:55 <fjahr> to make sure it is clear why it was not found
211 13:56 <pinheadmz> OK five minutes left !!
212 13:56 <tuxcanfly> michaelfol: the p2p cfilter serve was part of the PR but didn't enough review so it isn't merged just yet... we will be getting that one soon
213 13:56 <pinheadmz> Did anyone play with the tests at all? Or try anything wacky with an actual lightning node?
214 13:56 <pinheadmz> I ran the test suite with wireshark open, sniffing loca host - and is really cool to see the dialog between the test nodes
215 13:57 <pinheadmz> *localhost
216 13:57 <tuxcanfly> I just have one question to jimpo or anyone interested
217 13:57 <jimpo> sorry, guys I have to go now
218 13:57 <jimpo> thanks everyone for you time reviewing!
219 13:57 <pinheadmz> jimpo: TY so much for attending! <3
220 13:57 <jonatack> thanks jimpo
221 13:57 <jnewbery> jimpo: thanks so much for dropping in and answering questions!
222 13:57 <miketwenty1> (y)
223 13:57 <fjahr> I just inspected some messages by logging, nothing much
224 13:58 <pinheadmz> tuxcanfly: whats your Q ?
225 13:58 <Talkless> thanks jimpo!
226 13:58 <jonatack> met laurent mt here and we've been discussing and following along
227 13:58 <tuxcanfly> ah well... I was just curious if more thought was given to committing cfilters to blocks via coinbase (something similar was mentioned in the btcd/bitcoind comments)
228 13:59 <fjahr> I thought for a moment the filters in the test might be all 0's because there are no transactions in the blocks but forgot about the coinbase of course...
229 13:59 <Talkless> <andrewtoth> Talkless: I believe this PR introduces a way to request a range of filters, so it can start at birthdate
230 13:59 <Talkless> I wonder this range is defined w.r.t block number, or UTC..?
231 13:59 <Talkless> if new light wallet is created, how does it define it's birthday?
232 14:00 <pinheadmz> Talkless: the day it generated the seed :-)
233 14:00 <Talkless> if it's block number, it has to check.. what..? Ask node whats latest block..? at that time..?
234 14:00 <Talkless> so the time?
235 14:00 <pinheadmz> yeah or block height would work
236 14:01 <jnewbery> Talkless: either is fine. If you generate a new private key randomly, you know for sure that there haven't been any sends to that key before that time
237 14:01 <pinheadmz> just to know at what point in the past history there will definitely not be any incoming transacations (bc the addresses didnt esit yet)
238 14:01 <Talkless> well if wallet knows time, and p2p services would need block indexes, some translation would be needed.
239 14:01 <jnewbery> whether you store that as a clock time or block number is an implementation detail
240 14:01 <pinheadmz> jnewbery: do you know how the discussion ended up on the mailing list re: commiting filters to the block? I know it was proposed...
241 14:01 <Talkless> jnewbery: my question is how would you ask a node a block filters since your birthday? Will node need time, or block index?
242 14:02 <jnewbery> pinheadmz: I haven't seen any discussion of that recently. I think we'd want non-committed BIP157 filters to be used for a long time before proposing a soft fork to commit to them
243 14:02 <pinheadmz> Talkless: you provide a start height
245 14:03 <pinheadmz> jnewbery: makes sense, might need updates to the protocol before making it consensus
246 14:03 <Talkless> pinheadmz: ok so if wallet knows UTC time of it's "birthday", how could it transalte it's UTC birthday to the height?
247 14:03 <pinheadmz> Talkless: ha, ok I guess block height is more relevant in this case :-)
248 14:03 <pinheadmz> although timestamps are commited in the block headers as well
249 14:03 <jnewbery> Talkless: sync all block headers and look at timestamps (and then add some buffer)
250 14:03 <pinheadmz> Ok that about does it!
251 14:03 <miketwenty1> is this the part of the meeting where you let the new guy merge the PR?
252 14:03 <pinheadmz> Anyone have any lingering questions or comments?
253 14:04 <pinheadmz> miketwenty1: 😂
254 14:04 <jnewbery> #action ship it
255 14:04 <Talkless> jnewbery: how much all block headers take space?
256 14:04 <Talkless> bandwth?
257 14:04 <pinheadmz> Talkless: headers are 8 bytes x ~600,000 blocks in the chain
258 14:04 <pinheadmz> *80 bytes sorry
259 14:04 <Talkless> 80 yeah
260 14:04 <Talkless> right, forgot that
261 14:04 <Talkless> simple math
262 14:05 <pinheadmz> ok im calling it!
263 14:05 <pinheadmz> #endmeeting
264 14:05 <pinheadmz> thanks everyone for hanging out!
265 14:05 <emzy> ca. 50 MB
266 14:05 <jnewbery> Thanks for hosting pinheadmz. Great job!
268 14:05 <tuxcanfly> thanks pinheadmz
269 14:05 <Talkless> thanks!
270 14:05 <michaelfolkson> Thanks pinheadmz jnewbery et al
271 14:05 * pinheadmz headbanging \m/
272 14:05 <andrewtoth> thanks!