Expose block filters over REST (rpc/rest/zmq)

https://github.com/bitcoin/bitcoin/pull/17631

Host: dergoegge  -  PR author: TheBlueMatt

The PR branch HEAD was 3684fb7ae at the time of this review club meeting.

Notes

  • The REST interface is a lightweight interface that serves public data over HTTP on the same port as the JSON-RPC interface. It can be enabled using the -rest config option.

  • Endpoint examples:

    • Query transactions by their ID: /rest/tx/<TX-HASH>.<bin|hex|json>
    • Query blocks: /rest/block/<BLOCK-HASH>.<bin|hex|json>
    • Query the contents of a node’s mempool: /rest/mempool/contents.json
  • Most of the endpoints support responding in three different formats: binary, hex string or json.

  • Just like the JSON-RPC interface it is not recommended to expose the REST interface to the public.

Questions

  1. Did you review the PR? Concept ACK, approach ACK, tested ACK, or NACK?

  2. What are blockfilters and what are they used for (hint: see BIP157 and BIP158)

  3. Can you explain what REST is?

  4. What are the main differences between the JSON-RPC and REST interface?

  5. The JSON-RPC interface is already capable of serving blockfilters, why do we want this ability for the REST interface?

  6. There is a NACK (#23259) on the PR suggesting that the REST interface should be removed entirely in favour of external proxy servers. Do you agree? Why/why not?

Meeting Log

  117:00 <dergoegge> #startmeeting
  217:00 <dergoegge> Hi everyone! Welcome to this week's PR Review Club!
  317:00 <jnewbery> hi!
  417:00 <glozow> hi!
  517:00 <esraa> hello
  617:00 <dergoegge> feel free to say hi to let people know you are here
  717:01 <svav> Hi!
  817:01 <ajayparmar904> Hi
  917:01 <dergoegge> is anyone here for the first time? :)
 1017:01 <tr3xx> hi!
 1117:01 <larryruane> hi
 1217:01 <ajayparmar904> Yes.. i am here for first time
 1317:01 <esraa> first time here (:
 1417:01 <Kaizen_K_> 2nd time, just trying to build a habit
 1517:01 <Kaizen_K_> hi all
 1617:01 <dergoegge> ajayparmar904: welcome!
 1717:01 <dergoegge> esraa: hi, welcome!
 1817:01 <svav> New all time high today!
 1917:02 <tr3xx> dergoegge: third time for me :)
 2017:02 <dergoegge> today we are looking at #17631 - Expose block filters over REST
 2117:02 <jnewbery> ajayparmar904 esraa: welcome!
 2217:02 <David[m]12345> 2nd time and no clue, just lurking, thx :)
 2317:02 <dergoegge> notes and questions are in the usual place: https://bitcoincore.reviews/17631
 2417:02 <dergoegge> lurkers are also welcome!
 2517:02 <stickies-v> hi - sorry I'm late!
 2617:02 <dergoegge> ok lets get started: Did you review the PR? Concept ACK, approach ACK, tested ACK, or NACK?
 2717:03 <stickies-v> approach NACK
 2817:03 <svav> I read the notes ...
 2917:03 <jnewbery> concept & approach ACK. I'd like Matt to fix his commit log before I ACK it though :)
 3017:04 <sipa> concept ACK
 3117:04 <dergoegge> jnewbery: oh i pointed that out for the PR description, did not see it for the commit
 3217:05 <Kaizen_K_> what does ACK mean?
 3317:05 <larryruane> utACK
 3417:05 <glozow> Kaizen_K_: https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#peer-review
 3517:06 <jnewbery> dergoegge: ah. If you open a PR with just one commit, then github will use the commit log as the PR description by default. I guess that's where it came from.
 3617:06 <Kaizen_K_> glozow: ty
 3717:06 <dergoegge> ok next question: What are blockfilters and what are they used for? (hint: see BIP157 and BIP158)
 3817:06 <jnewbery> I think it's a shame that the github review process de-emphasizes commit logs. It'd be nice to be able to comment on them inline just like for the code. They're important!
 3917:07 <michaelfolkson> hhi
 4017:07 <svav> Does ACK stand for acknowledge or is it an acronym?
 4117:07 <dergoegge> jnewbery: thats true
 4217:07 <Kaizen_K_> dergoegge: I understand it as something related to the bloom filter
 4317:08 <stickies-v> Blockfilters are a replacement to bloom filters that allow light nodes to significantly reduce bandwidth, storage and verification without sacrificing privacy like bloom filters did. A block filter is a compressed list of prevouts and UTXOs in a block
 4417:08 <Kaizen_K_> where it allows a smaller amout of data to be sent around the network and reconstructed on the other end?
 4517:08 <sipa> there is no reconstruction
 4617:08 <Kaizen_K_> ah, thx stickies-v
 4717:09 <dergoegge> Kaizen_K_ : they enable a similar use case as the bloom filters but they are actually a replacement for the bloom filters
 4817:09 <glozow> the light client requests the entire block if the filter indicates there's something they're interested in
 4917:09 <sipa> it's just a way to quickly test whether a block may contain transactions that are interesting or not
 5017:09 <svav> A blockfilter is a filter on the data in a block, allowing a compact representation of the data.
 5117:09 <dergoegge> stickies-v: correct!
 5217:09 <larryruane> I looked up bips 157, 158, and they are both in "Draft" status .... what does that mean? It seems they've been implemented
 5317:09 <sipa> larryruane: BIP statuses are neglected
 5417:10 <Kaizen_K_> got it.
 5517:10 <dergoegge> glozow, sipa: yes!
 5617:10 <sipa> svav: i wouldn't call it "representation"; they don't encode the full block - more like a fancy checksum, which allows you to quickly check whether the block may be interesting to you
 5717:10 <sipa> but the check may be wrong (you may think a block is interesting while it isn't)
 5817:10 <larryruane> "..may contain transactions that are interesting .." Also an important goal is to not leak information to the server (bitcoind node) about WHAT we (light client) are interested in, right?
 5917:11 <sipa> larryruane: indeed, that's the primary difference with BIP37
 6017:11 <jnewbery> they're similar to BIP37 bloom filters in as much as they're a probabilistic filter of set inclusion. However, they're different in that everyone uses the same filter for each block, so they don't need to be recalculated for every client
 6117:11 <dergoegge> one thing to note is that the filters come with false positives (not sure what the exact % is on those)
 6217:11 <stickies-v> sipa but the filter can only be wrong for false positives, but never false negatives right?
 6317:11 <dergoegge> larryruane: yes!
 6417:11 <larryruane> jnewbery: thanks, that helps a lot!
 6517:11 <sipa> with BIP37, the filtering was done on the server side (client gives filter of what they're interested in to server); with BIP157, it's done on the client side (the server gives filter about the block's contents to the client)
 6617:11 <sipa> stickies-v: correct
 6717:12 <stickies-v> ty
 6817:12 <sipa> there is also a technical difference, in that BIP37 uses a bloom filter, while BIP157 uses a golomb-coded filter; the difference between those is mostly an implementation detail (bloom is bigger but faster to update)
 6917:12 <Kaizen_K_> this seems like a privacy enhancement to the bloom filter. The server doesn't really know what the client is up to
 7017:13 <dergoegge> Kaizen_K_: yes, that was one design goal of the BIPs
 7117:13 <jnewbery> For anyone interested in learning about the Bitcoin Core implementation of block filters, we did a whole series of review clubs on them. Just look in https://bitcoincore.reviews/meetings/ for anything with "BIP 157" in the title
 7217:14 <dergoegge> There is a great short explanation of the BIPs on the optech website: https://bitcoinops.org/en/newsletters/2019/04/23/#footnotes
 7317:14 <Kaizen_K_> jnewbery:ty
 7417:14 <stickies-v> it's also great for scaling, because now each full node only has to calculate one filter per block, whereas previously it would have to spend additional resources for each bloom filter (which was unique per light client)
 7517:14 <Kaizen_K_> ty dergoegge
 7617:14 <larryruane> malicious servers can't make things appear to be present in the block that aren't (since client should always download block and check), but server could *withhold* items (from the filter), I think?
 7717:14 <sipa> stickies-v: indeed, BIp37 was effectively a hard-to-avoid DoS risk
 7817:14 <Kaizen_K_> damn, that is cool
 7917:15 <sipa> larryruane: withholding is indeed a problem; the real solution to that is having PoW commit to the filters...
 8017:15 <stickies-v> larryruane yes, but that's where block filter headers come into play. You should connect to multiple nodes, check if they all provide the same block filter headers (they commit to block filters), and if you see different filters amongst nodes investigate. You then also verify that the filter header matches the filter. So quite easy to catch attackers as long as you're onnected to one hoenst node
 8117:15 <jnewbery> there's also a summary here: https://bitcoin.stackexchange.com/questions/86231/whats-the-distinction-between-bip-157-and-bip-158-are-they-supported-by-bitcoi , but that was just me copy-pasting from the same optech description
 8217:16 <sipa> stickies-v: unfortunately, it does mean that one attacker peer is enough to force you into worst-case bandwidth usage (download all blocks)
 8317:16 <glozow> do we have multiple FilterTypes or is that just there for future extensibility?
 8417:16 <sipa> glozow: BIP158 initially defined two filter types; following review, one was dropped
 8517:17 <dergoegge> glozow: i was also wondering about that
 8617:17 <stickies-v> sipa true, but once established that he's an attacker you disconnect from that peer so it's only temporary?
 8717:17 <sipa> so it's just for future extensibility
 8817:17 <glozow> sipa: ah, thanks
 8917:17 <dergoegge> sipa: thanks
 9017:17 <larryruane> " ... PoW commit to the filters ..." That's cool, maybe that could be done in a future softfork?
 9117:17 <sipa> larryruane: yes, that's exactly what i mean
 9217:18 <dergoegge> Ok i think we covered blockfilters
 9317:18 <dergoegge> Next question: Can you explain what REST is?
 9417:18 <larryruane> very much an industry standard!
 9517:18 <dergoegge> My understanding of REST is very basic and i hope one of you has a great answer :D
 9617:18 <Kaizen_K_> I just understand it as an api access thtough the internet
 9717:19 <larryruane> tons and tons of "devices" (the kind I'm familiar with is storage nodes) can expose a REST interface to control and/or query the device
 9817:19 <jnewbery> There was lots of discussion on the mailing list about what to include in the block filters as the proposal was being developed (eg here https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-June/016057.html)
 9917:19 <glozow> afaiu, a guideline for web APIs
10017:19 <larryruane> (not standard in the specific messages and their meaning, but the mechanism)
10117:19 <stickies-v> A stateless representation of resources, typically (but I think not always?) communicated over HTTP with HTTP status codes. The stateless part basically means that the webserver has no state, i.e. each request is fully self contained and does not depend on e.g. previous requests. This makes for much more easily scalable services, since it doesn't matter which of many servers handles your request - there is noo state
10217:19 <stickies-v> anyway.
10317:20 <esraa> a request to the server over http/s
10417:21 <tr3xx> REST is short for Representational State Transfer and is usually used to send data over HTTP/S
10517:22 <dergoegge> from what i gathered it is a very common client-server architectural style for designing APIs that are scalable and simple.
10617:22 <dergoegge> pretty much what y'all just said
10717:23 <dergoegge> stickies-v: the "stateless" part is always mentioned when talking about REST so that seems like a core concept here.
10817:24 <stickies-v> yeah, unfortunately that's both the most important and the most difficult to grasp part :(
10917:24 <larryruane> ".. stateless.." this means the server doesn't need to maintain any per-client state (and time out this state, etc.)
11017:25 <Kaizen_K_> ah yea, you dont need an individual session per client request
11117:25 <Kaizen_K_> everyone making a REST call is equal
11217:26 <stickies-v> larryruane the "per-client" qualification is very helpful to make it more understandable indeed, good point
11317:26 <dergoegge> larryruane: +1
11417:27 <dergoegge> next question: What are the main differences between the JSON-RPC and REST interface?
11517:27 <Kaizen_K_> I understand the rest interface as being something public facing, just generally open to the publci.
11617:27 <Kaizen_K_> *public.
11717:28 <Kaizen_K_> The json-rpc would be something that the node operator would use when building a service ect
11817:28 <Kaizen_K_> like having your lightning network node access your btc node would be done through rpc
11917:28 <stickies-v> The REST api only offers a subset of functionality of the full fledged JSON-RPC. It is unauthenticated (but still weirdly somewhat trusted, so don't go ahead and open it up to the internet) and meant to expose public data in an easy and fast way.
12017:28 <larryruane> One big difference probably is that REST (the way bitcoind uses it) is read-only (query various stuff), while RPC can change things (like submit transactions)
12117:29 <glozow> I have a dumb question, I've never built a web app. I thought REST was just a concept, not itself a communication protocol or tool. if someone says "REST API" is that shorthand for "our web API which is RESTful" or?
12217:29 <Kaizen_K_> larryruane: that is insightfull, rest should be for read only, json-rpc is full command control
12317:29 <larryruane> glozow: that's a great question!
12417:29 <stickies-v> glozow indeed, REST is just a concept. It says nothing about the comms protocol, but HTTP(S) is by far the most used
12517:29 <dergoegge> Kaizen_K_: it is not, neither the REST nor the JSON-RPC interface are recommended to be open to the public
12617:30 <Kaizen_K_> dergoegge: good to know
12717:30 <glozow> stickies-v: okie thank u i was a lil confused
12817:30 <larryruane> I haven't checked but I assume the REST interface can't be used to extract secret material (such as keys)?
12917:30 <michaelfolkson> I think the distinction between REST vs JSON-RPC generally and Core's REST vs Core's JSON-RPC could avoid confusion
13017:31 <michaelfolkson> Some things are specific to Core
13117:31 <dergoegge> the REST interface can serve the contents of your mempool for example, so don't open it to the public!
13217:31 <sipa> stickies-v: it's "trusted" in the sense that if you expose it to the internet, you risk being DoS'ed
13317:31 <sipa> it has no privileged access
13417:31 <larryruane> dergoegge: can you elaborate why exposing the content of your mempool is bad?
13517:32 <stickies-v> sipa ah that's good to know - and relevant to the last question of this PR review I suppose, thanks!
13617:32 <sipa> well, and privacy, i guess
13717:32 <dergoegge> larryruane: exposing the content of your mempool is v bad for privacy, an attacker could figure out which transactions are broadcast by you
13817:32 <larryruane> dergoegge: +1
13917:33 <dergoegge> so one big difference is that JSON-RPC is authenticated while the REST interface is not
14017:33 <Kaizen_K_> dergoegge: ty, that makes sense, a hostile party could filter peoples transactions to deny them entry into a block.
14117:34 <jnewbery> I also think that it's just generally not designed to be exposed on a public network. If you wanted to serve REST clients on a public network, you should put the bitcoind server behind a proxy.
14217:34 <dergoegge> jnewbery: +1
14317:35 <Kaizen_K_> So why are there these two interfaces, that seem like they shouldn't even be exposed publicly, are available to use instead of just one? Is it a performance issue?
14417:35 <sipa> michaelfolkson: obviously; you can't compare REST and JSON-RPC as concepts on their own, they're not comparable (one is a principle for client/server communication, the other is a specific protocol)
14517:35 <dergoegge> Kaizen_K_: as long as one miner is honest this won't happen
14617:35 <Kaizen_K_> dergoegg: ty
14717:35 <sipa> Kaizen_K_: REST is way more convenient to use
14817:35 <jnewbery> Kaiken_K: that's question 5 :)
14917:35 <dergoegge> Kaizen_K_: good question, this is also what the last question today will be about
15017:36 <sipa> < larryruane> I haven't checked but I assume the REST interface can't be used to extract secret material (such as keys)? <-- absolutely; only public data is available through REST
15117:36 <Kaizen_K_> So from what I gather, it must be a qol issue for developers.
15217:37 <Kaizen_K_> somethings are just too much a paid in the ass to authenticate for
15317:37 <dergoegge> just in short my understing of the two interfaces: The JSON-RPC interface is used to control your node through the “bitcoin-cli” binary while the REST interface is there to serve public data (blocks, txs, etc) to a trusted caller.
15417:37 <jnewbery> sipa: public-ish. The contents of your mempool are semi-public
15517:37 <sipa> jnewbery: well, it's not *secret* data; it may be private (in the sense of privacy)
15617:38 <sipa> but fair, that's a distinction
15717:38 <jnewbery> sipa: agree
15817:38 <jnewbery> dergoegge: bitcoin-cli is by far the most common client for the json-rpc interface, but it's perfectly possible to use other clients
15917:39 <Kaizen_K_> :/s/paid/pain/g/
16017:39 <dergoegge> jnewbery: true, i forgot :D
16117:39 <sipa> i'd be surprised if the number of RPC calls made with bitcoin-cli to bitcoind is even a measurable fraction of the total
16217:39 <sipa> it's the most visible for sure, as it's half-way intended for human/developer interaction
16317:40 <jnewbery> or maybe i'd restate as "it's perfectly possible for other applications to access that interface as a client"
16417:40 <sipa> but anything automated will just use a JSON-RPC library
16517:40 <dergoegge> lightning nodes also make significant use of the json-rpc interface
16617:40 <larryruane> sipa: like our python tests!
16717:40 <sipa> (except c-lightning, i think, which i don't comprehend...)
16817:41 <dergoegge> next question: The JSON-RPC interface is already capable of serving blockfilters, why do we want this ability for the REST interface?
16917:41 <Kaizen_K_> privacy?
17017:41 <larryruane> dergoegge: my impression is that every lightning full node needs (or typically has) its own dedicated local bitcoind
17117:41 <jnewbery> sipa: right. I retract my statement about bitcoin-cli being the most common client. It's only the most commonly used client amongst Bitcoin Core developers.
17217:41 <sipa> jnewbery: agree
17317:41 <dergoegge> larryruane: LND also an option to use their "neutrino" BIP158 light client
17417:41 <michaelfolkson> c-lightning doesn't use a JSON-RPC library, it uses bitcoin-cli and this surprises you? Have I understood that right sipa?
17517:42 <sipa> michaelfolkson: yes
17617:42 <sipa> (or this used to be the case, at least; my information may be outdated)
17717:42 <dergoegge> larryruane: although i think it is discouraged (not sure though)
17817:42 <michaelfolkson> Why shouldn't it use bitcoin-cli? More restrictive?
17917:42 <sipa> michaelfolkson: starting an entire new process for every RPC call?
18017:42 <sipa> that's some ridiculous overhead
18117:42 <stickies-v> dergoegge REST apis are super easy to consume, especially with all the tooling built around it (e.g. OpenAPI, although I've not seen an OpenAPI spec for bitcoin yet). I think developer ease of use is the main reason?
18217:43 <Kaizen_K_> Um, I think we are veering off topic, I don't really mind, I'm still learning anyway.
18317:43 <michaelfolkson> Sorry, just found that very interesting :)
18417:43 <Kaizen_K_> stickies-v: thats my intuition as well, quality of life and privacy
18517:43 <Kaizen_K_> michaelfolkson: np :)
18617:44 <dergoegge> stickies-v: yes ease of use and lack of authentication make the REST interface attractive in certain use cases
18717:44 <larryruane> dergoegge: "why do we want this ability for REST" -- it would have been nice if the PR answered this, I'm wondering too
18817:45 <dergoegge> i also think it just makes sense to also serve the blockfilters as they are also public data like blocks or transactions
18917:45 <larryruane> (unless it's just sort of a desire for more completeness)
19017:45 <stickies-v> does anyone know if there has been any discussion around making an OpenAPI spec for the REST API? If not, I could look into contributing that
19117:45 <jnewbery> The REST interface should also be more performant (at least in theory), since it can return binary data. The json-rpc interface always serializes its returned data into json text. If your application is going to immediately deserialize that, then it's unnecessary overhead.
19217:45 <dergoegge> larryruane: yeah matt did not mention any use cases
19317:45 <Kaizen_K_> larryruane: thats what I'm wondering too, I wonder if this is a scalability issue, RPC creates more network traffic through authentication? Rest maybe reduces that?
19417:46 <sipa> REST interface is binary (or at least can be)
19517:46 <dergoegge> jnewbery: good point the REST interface supports different formats
19617:46 <sipa> JSON-RPC needs hex + JSON encoding/decoding on both sides
19717:46 <sipa> oh, i'm repeating what jnewbery said
19817:46 <Kaizen_K_> Ah interesting
19917:46 <jnewbery> sipa: that's ok. The validation is nice :)
20017:46 <larryruane> just to complete the picture, we have these two, plus there's ZMQ
20117:47 <sipa> Kaizen_K_: it's not the authentication that's the issue; it's just that pumping large binary blobs through JSON is kind of dumb
20217:47 <Kaizen_K_> I see I see
20317:47 <Kaizen_K_> Rest gives a nice performance optimization on encoding/decoding and maybe less network traffic, so its all around more performant
20417:48 <sipa> and just... easier
20517:48 <Kaizen_K_> + the qol
20617:48 <Kaizen_K_> cool cool
20717:48 <dergoegge> sipa: +1
20817:48 <Kaizen_K_> shit man, these pr-clubs are super informative
20917:48 <jnewbery> larryruane: that's right. ZMQ is the third interface, but it's a completely different animal
21017:49 <larryruane> my impression of ZMQ is that it allows notifications, so the client doesn't have to continuously poll
21117:49 <Kaizen_K_> I've always wondered what amq was, I know its important with lightning-d
21217:49 <Kaizen_K_> lnd sry
21317:50 <Kaizen_K_> TIL: ZeroMQ (also known as ØMQ, 0MQ, or zmq) looks like an embeddable networking library but acts like a concurrency framework. It gives you sockets that carry atomic messages across various transports like in-process, inter-process, TCP, and multicast. You can connect sockets N-to-N with patterns like fan-out, pub-sub, task distribution, and request-reply. It's fast enough to be the
21417:50 <Kaizen_K_> fabric for clustered products. Its asynchronous I/O model gives you scalable multicore applications, built as asynchronous message-processing tasks. It has a score of language APIs and runs on most operating systems.
21517:50 <larryruane> Kaizen_K_: "are super informative" Yes, and be aware that you can go back and read all the old ones! (which I haven't done enough myself!)
21617:50 <Kaizen_K_> larryruane: thats a good suggestion, I'm going to do that
21717:51 <dergoegge> Kaizen_K_ all meeting logs can be found here: https://bitcoincore.reviews/meetings/
21817:51 <Kaizen_K_> This zeromq sounds similar to google-protobuffers or am I mistaken?
21917:51 <michaelfolkson> https://github.com/bitcoin/bitcoin/blob/master/doc/zmq.md
22017:51 <dergoegge> ok last question: There is a NACK (#23259) on the PR suggesting that the REST interface should be removed entirely in favour of external proxy servers. Do you agree? Why/why not?
22117:51 <dergoegge> https://github.com/bitcoin/bitcoin/issues/23259
22217:52 <Kaizen_K_> After this meeting, I disagree, a fair amount of data can be optimized
22317:52 <Kaizen_K_> Rest is good
22417:52 <dergoegge> Jeremy also made a PR to exemplify an external proxy server on top of the JSON-RPC interface: https://github.com/bitcoin/bitcoin/pull/23309
22517:54 <jnewbery> dergoegge: thanks for pointing that out. I wasn't aware of that PR
22617:54 <stickies-v> dergoegge I mostly agree. If we're trying to keep core reviewable and maintainable, I see this as an easy to carve out some code - even though it is tiny. Like Jeremy proposes, I think it would make sense to have a separate project to run a full fledged REST server (ideally with optional auth to access the full feature set that JSON RPC represents). However, I don't think the REST functionality is a particular
22717:54 <stickies-v> attack vector since it's so simple, so that's why I only mostly agree.
22817:54 <stickies-v> *mostly agree with Jeremy, that is
22917:55 <jnewbery> I disagree that we should remove the REST interface. I expect that a lot of people are using it.
23017:56 <michaelfolkson> Other than additional maintenance why not just add an external proxy server as an additional option rather than as a replacement for REST?
23117:56 <sipa> what does "just add an additional proxy server" mean?
23217:56 <dergoegge> stickies-v should that stop this PR from getting merged though? removing the REST interface is difficult if it has actual users
23317:56 <jnewbery> michaelfolkson: because it's not the responsibility of the bitcoin core developers/maintainers to write/test/maintain a proxy server
23417:57 <dergoegge> jnewbery: +1
23517:57 <stickies-v> stickies-v no I don't think it should stop this PR, this is a useful addition. I think they're separate. This PR is about getting the functionality up to date, Jeremy's PR is about carving out that functionality into a different project I think?
23617:57 <jnewbery> the responsibility of the Bitcoin Core project ends at the interface. How those interfaces are consumed/used is the responsibility of the client user/application
23717:57 <sipa> i think there is little point in removing the REST server; it's simple, straightforward code with barely a maintenance burden
23817:58 <sipa> and removing it makes the functionality it provides harder to use (separate server etc)
23917:58 <dergoegge> stickies-v he is proposing the proxy to still be in the core repo just not part of the binary
24017:59 <michaelfolkson> I think Jeremy's argument is that people shouldn't be using REST. e.g. the discussion on sanitizing
24117:59 <michaelfolkson> "exposing this rest endpoint over NGINX is precisely how our rest endpoint should not be used"
24217:59 <dergoegge> sipa: i agree
24317:59 <michaelfolkson> Kinda protecting the user (don't necessarily agree but I think that is the argument for removal)
24418:00 <sipa> i think the REST interface just shouldn't be exposed to the internet
24518:00 <dergoegge> #endmeeting
24618:00 <stickies-v> michaelfolkson I'm not sure that's what he's after, in https://github.com/bitcoin/bitcoin/issues/23259#issuecomment-940648658 he mentions that he's open to replacing the json rpc with rest entirely?
24718:00 <sipa> with or without proxy
24818:00 <dergoegge> thanks for coming everyone!
24918:00 <dergoegge> feel free to stay and discuss
25018:00 <glozow> thanks dergoegge!
25118:00 <Kaizen_K_> dergoegge: thanks for hosting, I learned a lot
25218:00 <sipa> if you really want to expose it, yes, use a wrapper to sanitize it
25318:01 <sipa> but that's not its intended use case
25418:01 <stickies-v> thanks for hosting dergoegge , very vibrant meeting! (and thanks Matt for the PR)
25518:01 <larryruane> PR 23309 says `[WIP]` (work in progress) instead of making the PR a draft -- is the GitHub draft PR feature not used much in Core?
25618:01 <larryruane> thanks, dergoegge this was great!!
25718:01 <sipa> #23309
25818:01 <Kaizen_K_> when does the next PR go up?
25918:01 <Kaizen_K_> I wanna be more prepared for next week
26018:01 <sipa> what "next PR" ?
26118:01 <Kaizen_K_> I guess the next PR review
26218:01 <sipa> ah, next review club?
26318:02 <jnewbery> dergoegge: great job. Thank you!
26418:02 <glozow> #22674 next week :)
26518:02 <larryruane> Kaizen_K_: around Friday usually
26618:02 <Kaizen_K_> cool cool
26718:02 <glozow> https://github.com/bitcoin/bitcoin/pull/22674
26818:02 <michaelfolkson> Yeah really interesting, thank you dergoegge
26918:02 <sipa> larryruane: the author may be unfamiliar with the feature
27018:02 <dergoegge> Kaizen_K_ if you lurk on this repo: https://github.com/bitcoin-core-review-club/website you will always be up to date on the next meeting
27118:03 <michaelfolkson> Yeah you can watch it and get email notifications
27218:03 <michaelfolkson> Beware you get a lot of emails if you watch a few repos :)
27318:03 <larryruane> after these meetings, I always have a ton of new broswer tabs open that I need to read, haha!
27418:03 <larryruane> michaelfolkson: thanks, TIL! (notifications)
27518:04 <Kaizen_K_> yea me too, it's really opening me up to my knowledge gaps
27618:04 <jnewbery> Also follow https://twitter.com/BitcoinCorePRs for all updates
27718:04 <stickies-v> I'm not sure if anyone has any thoughts on this, but my Approach NACK is because I don't think <COUNT> should be a path parameter in `GET /rest/blockfilterheaders/<FILTERTYPE>/<COUNT>/<BLOCK-HASH>.<bin|hex|json>` - it's not restful.
27818:04 <stickies-v> Instead, I think this should be a query parameter, for example `GET /rest/blockfilterheaders/<FILTERTYPE>/<BLOCK-HASH>.<bin|hex|json>?count=<COUNT>`. Thoughts?
27918:04 <jnewbery> and there's also a feed on the website: https://bitcoincore.reviews/feed.xml
28018:04 <larryruane> jnewbery: I do follow but darn twitter doesn't show me its tweets!
28118:04 <stickies-v> (and also - does a reservation like that warrant an Approach NACK?)
28218:06 <tr3xx> This was great, I have multiple tabs open as well! It was fun lurking, watching the discussions :)
28318:06 <larryruane> BTW in case anyone hasn't tried fetching a block filter yet, I ran `bitcoin-cli getblockfilter 00000000000000000006f9a460e2f86f4262d8970902f7f38b0f86bf08bfc898` and got the error `Index is not enabled for filtertype basic`
28418:06 <michaelfolkson> stickies-v: I think suggest it as a change and then if the author doesn't want to make the change it is up to you whether you would rather the PR wasn't merged because of it (whether it is worthy of an Approach NACK)
28518:06 <sipa> stickies-v: voicing the actual objection would certainly be far more productive than a blanket nack
28618:06 <michaelfolkson> You can always open an alternative PR if you feel that strongly about it
28718:07 <dergoegge> larryruane: did you run your node with the `-blockfilterindex` option?
28818:07 <larryruane> so i added `blockfilterindex=1` to my config file, restarted, and now it's building these filters, starting from block 0, in the background (still adding new blocks)
28918:07 <jnewbery> stickies-v: you should definitely raise specific objections in review. I find that the word NACK tends to antagonize people, so I use it sparingly and only when I think the PR is harmful to the project.
29018:08 <stickies-v> michaelfolkson sipa jnewbery got it, thanks for the advice on how to approach this- that makes sense! Will start with the suggestion first.
29118:08 <sipa> stickies-v: i'd reserve an approach nack for "you're doing this completely the wrong, the whole thing needs to be done differently"