19:00:52 <wumpus> #startmeeting
19:00:52 <lightningbot> Meeting started Thu Jul  9 19:00:52 2020 UTC.  The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot.
19:00:52 <lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
19:00:52 <MarcoFalke> ahoi
19:00:57 <hebasto> hi
19:00:58 <achow101> hi
19:00:59 <troygiorshev> hi
19:01:03 <jnewbery> hi
19:01:07 <wumpus> #bitcoin-core-dev Meeting: wumpus sipa gmaxwell jonasschnelli morcos luke-jr sdaftuar jtimon cfields petertodd kanzure bluematt instagibbs phantomcircuit codeshark michagogo marcofalke paveljanik NicolasDorier jl2012 achow101 meshcollider jnewbery maaku fanquake promag provoostenator aj Chris_Stewart_5 dongcarl gwillen jamesob ken281221 ryanofsky gleb moneyball kvaciral ariard digi_james amiti fjahr
19:01:09 <wumpus> jeremyrubin lightlike emilengler jonatack hebasto jb55 elichai2
19:01:19 <jb55> hi
19:01:20 <fjahr> hi
19:01:22 <amiti> hi
19:01:26 <luke-jr> ih
19:01:37 <promag> hi
19:01:38 <kanzure> hi
19:01:50 <jonasschnelli> hi
19:01:57 <jeremyrubin> hola
19:02:12 <gwillen> hi
19:02:21 <cfields> hi
19:02:28 <sipa> hi
19:02:30 <wumpus> one proposed meeting topic: small clarification on the goals of the mempool project (jeremyrubin)
19:02:36 <wumpus> any last minute topics?
19:02:50 <phantomcircuit> hi
19:02:56 <sipa> another short one: can we drop gcc 4.8?
19:03:06 <wumpus> ok
19:03:25 <instagibbs> proposed topic: new zmq notifications, or something else https://github.com/bitcoin/bitcoin/pull/19462#issuecomment-656140421 and https://github.com/bitcoin/bitcoin/pull/19476
19:03:41 <luke-jr> sipa: not sure that's a good meeting topic; it just needs someone to investigate distros?
19:03:44 <ariard> hi
19:03:54 <elichai2> hu
19:04:04 <wumpus> #topic High priority for review
19:04:16 <MarcoFalke> can i haz #19386
19:04:17 <achow101> #19325 pls
19:04:19 <gribble> https://github.com/bitcoin/bitcoin/issues/19386 | rpc: Assert that RPCArg names are equal to CRPCCommand ones (server) by MarcoFalke · Pull Request #19386 · bitcoin/bitcoin · GitHub
19:04:20 <gribble> https://github.com/bitcoin/bitcoin/issues/19325 | wallet: Refactor BerkeleyDatabase to introduce DatabaseBatch abstract class by achow101 · Pull Request #19325 · bitcoin/bitcoin · GitHub
19:04:27 <wumpus> https://github.com/bitcoin/bitcoin/projects/8  13 blockers, 1 bugfix, 3 chasing concept ACK
19:04:36 <wumpus> please don't add any more before removing any xD
19:04:38 <jamesob> hi
19:04:45 <ariard> you can drop #18787, after talking with few people, a libstandardness would be more accurate
19:04:50 <gribble> https://github.com/bitcoin/bitcoin/issues/18787 | wallet: descriptor wallet release notes and cleanups by achow101 · Pull Request #18787 · bitcoin/bitcoin · GitHub
19:05:01 <ariard> #18797 sorry
19:05:04 <gribble> https://github.com/bitcoin/bitcoin/issues/18797 | Export standard Script flags in bitcoinconsensus by ariard · Pull Request #18797 · bitcoin/bitcoin · GitHub
19:05:10 <jeremyrubin> removing #18191
19:05:13 <gribble> https://github.com/bitcoin/bitcoin/issues/18191 | Change UpdateForDescendants to use Epochs by JeremyRubin · Pull Request #18191 · bitcoin/bitcoin · GitHub
19:06:17 <jonatack> hi
19:06:22 <promag> please see #19033, its for 0.20.1
19:06:24 <gribble> https://github.com/bitcoin/bitcoin/issues/19033 | http: Release work queue after event base finish by promag · Pull Request #19033 · bitcoin/bitcoin · GitHub
19:06:25 <wumpus> ariard, achow101  done
19:07:11 <wumpus> jeremyrubin: ok
19:07:13 <luke-jr> would be nice to get the build tarballs fixed finally too <.<
19:07:19 <meshcollider> hi
19:08:10 <pinheadmz1> hi
19:09:20 <wumpus> #topic clarification on the goals of the mempool project (jeremyrubin)
19:09:49 <jeremyrubin> Yeah just quick;
19:10:10 <jeremyrubin> So I think that there's some confusion or at least co-mingling of concerns based on how I've presented things
19:10:35 <jeremyrubin> A lot of the motivation is to reduce the memory usage in the mempool
19:10:49 <jeremyrubin> and to better bound the algorithmic complexity on functions
19:11:09 <jeremyrubin> So that one day, maybe, we could lift some restrictions (e.g., descendents)
19:11:42 <jeremyrubin> It's less so "make mempool faster now" performance motivation.
19:12:09 <wumpus> ok good to know
19:12:13 <jeremyrubin> In a lot of cases I think it would be fine to accept a *slower* score on some benchmark (or at least the same) when the goal of the PR is reducing memory
19:12:44 <jeremyrubin> especially in cases where there may be a distant invariant which is currently protecting us from a "crash the network" style DoS
19:12:56 <luke-jr> if those are the goals, I wonder if it might make sense to move complexity into the miner
19:13:00 <wumpus> we could have benchmarks that measure memory usage I suppose
19:13:08 <luke-jr> but that could interfere with compact blocks
19:13:27 <jeremyrubin> luke-jr: this does make sense for certain things.
19:13:49 <luke-jr> I wonder if it'd be possible to support a build without mining support that performs better
19:14:12 <jeremyrubin> I think one of the things that is difficult in particular is that we don't have a great set of adversarial benches for the mempool
19:14:23 <jeremyrubin> And you need both whole-system and unit tests for the functions
19:14:30 <wumpus> I think there's something of a conflict there, miners want the block selection to be as fast as possible, while other node users would want to reduce memory usage for the mempool as much as possible
19:14:34 <jeremyrubin> And both asymptotic and bounded size
19:14:37 <cfields> wumpus: +1
19:15:19 <luke-jr> wumpus: by making mining build-time-optional, we could possibly get both cheaply?
19:15:46 <ariard> I'm not sure about reducing the memory usage, what the relation between size of your mempool and perf of fee estimation ?
19:15:50 <luke-jr> I wonder if there's a way to change class sizes at runtime in C++
19:16:03 <jeremyrubin> Reducing memory usage is related to DoS primarily
19:16:06 <wumpus> that's kind of annoying for testing but yes
19:16:24 <jeremyrubin> So the goal is to eliminate the class  of vuln
19:16:42 <luke-jr> jeremyrubin: eh? users can always adjust their mempool size
19:16:46 <jeremyrubin> I don't care about performance here that much, even a 2x slower mempool with less DoS surface is probably fine
19:16:53 <wumpus> it doesn't help if only the miners would have the vulnerability though
19:16:55 <sipa> mempool size _predictability_ is a DoS concern
19:16:55 <jeremyrubin> luke-jr: no for the algorithms themselves
19:16:57 <luke-jr> reducing mem usage would just mean more txs per MB
19:17:03 <jeremyrubin> not for the mempool size itself
19:17:20 <ariard> I think there is a confusion there with memory usage of caching data structure for update and overall mempool size
19:17:23 <sipa> e.g. improving average memory usage on average, but worsening it under a particular edge case might constitute a vulnerability
19:17:23 <jeremyrubin> Mostly fixing short-lived allocations
19:17:31 <nehan> jeremyrubin: what is your expected memory usage improvement?
19:17:50 <luke-jr> sipa: but there's no vuln with zero mempool…
19:18:32 <jeremyrubin> nehan: it's a project with a bunch of algorithm improvements based on epochs for traversal instead of sets
19:18:37 <aj> (5min cap exceeded fwiw)
19:18:51 <jeremyrubin> ah yeah didn't mean to turn this into an ordeal, just trying to clarify
19:19:05 <luke-jr> not like we have any other topics?
19:19:10 <sipa> perhaps just document this as a summary on the relevant PRs?
19:19:16 <jeremyrubin> happy to move on if there's other things to discuss
19:19:17 <sipa> unless it already is
19:19:17 <wumpus> luke-jr: we do, sipa had a topic
19:19:21 <luke-jr> oh
19:19:30 <sipa> nothing important
19:19:30 <luke-jr> right
19:19:48 <wumpus> #topic can we drop gcc 4.8 (sipa)
19:19:51 <MarcoFalke> why?
19:19:54 <wumpus> just something to annoy fanquake
19:19:58 <wumpus> :)
19:19:59 <luke-jr> lol
19:19:59 <cfields> lol
19:20:02 <sipa> so, i was looking at #18086 again
19:20:05 <gribble> https://github.com/bitcoin/bitcoin/issues/18086 | Accurately account for mempool index memory by sipa · Pull Request #18086 · bitcoin/bitcoin · GitHub
19:20:32 <sipa> and was trying to make the accounting allocator not track copies of containers, which is a feature of the C++11 allocator infrastructure
19:20:39 <wumpus> I *really* think we should wait with bumping gcc versions until c++17 requirement
19:20:45 <sipa> but it seems gcc 4.8 ignores it or otherwise fails to implement it correctly
19:20:46 <wumpus> which isn't too far away, just one major version
19:20:52 <sipa> yeah
19:21:17 <luke-jr> sipa: not the end of the world if it's wrong?
19:21:18 <sipa> but i've just noticed at appveyor also fails at it :s
19:21:25 <sipa> luke-jr: could cause UB
19:21:32 <luke-jr> hmm
19:21:33 <sipa> if used in totally reasonable ways
19:21:40 <sipa> (but probably not in my actual PR)
19:21:46 <wumpus> not that I'm opposed to bumping it sooner if it's eally required for something, but it seems a waste of time to discuss minor version bumps if a big one is around the cornr :)
19:21:59 <luke-jr> when C++20? :p
19:22:07 <wumpus> in 2025
19:22:11 * luke-jr found C++20 to be required for totally obvious/expected features the other day :/
19:22:14 <sipa> luke-jr: hard to talk about things that don't exist
19:22:42 <cfields> luke-jr: get sipa to backport for you like Span :p
19:22:50 <luke-jr> cfields: can't backport syntax
19:22:59 <MarcoFalke> Only 3.6 months till branch off: https://github.com/bitcoin/bitcoin/issues/18947
19:23:07 <luke-jr> struct type var = { .a = 1, .b = 2 }
19:23:16 <jnewbery> sipa: can you #ifdef support for accounting allocators for only certain versions of gcc until we move to c++ 17?
19:23:30 <wumpus> jnewbery: +1!
19:23:34 <sipa> jnewbery: ugh
19:23:34 <aj> luke-jr: omg, don't tease things like that
19:23:37 <sipa> that's horrendous
19:23:51 <luke-jr> aj: it's common C99, no clue why C++ forbids it :/
19:24:09 <cfields> luke-jr: juse use unnamed initializers ?
19:24:17 <luke-jr> cfields: but the whole point is the names!
19:24:18 <wumpus> I mean, accurate memory accounting is nice but not critical I suppose, not enough reason to really forbid building on a platform
19:24:59 <luke-jr> cfields: I ended up putting defaults for every member, and just setting the ones I care about after init
19:25:19 <sipa> wumpus: it would mean ifdefs all over to maintain to old heuristics for every data structure, plus a accounting based one
19:25:29 <luke-jr> https://github.com/bitcoin/bitcoin/pull/19463/files#diff-b2bb174788c7409b671c46ccc86034bdR291
19:25:33 <sipa> so i guess i'd just wait instead
19:25:35 <wumpus> anyhow according to #16684 the minimum gcc version will go to 8.3
19:25:38 <gribble> https://github.com/bitcoin/bitcoin/issues/16684 | Discussion: upgrading to C++17 · Issue #16684 · bitcoin/bitcoin · GitHub
19:25:47 <sipa> or try to minimize the risk of copying
19:25:59 <cfields> sipa: can you point to exactly what old gcc gets wrong? I'm just curious to see.
19:26:07 <jnewbery> sipa: ah ok. If it's super invasive to do, then not worth it
19:26:28 <sipa> cfields: have a container with an accounting allocator, encapsulated in some class
19:26:36 <sipa> return a copy of it for public consumption
19:26:37 <wumpus> is changing this really urgent or can it wait until after 0.21?
19:26:56 <sipa> now any changes to that copy need to lock the origin datastructure's accounting variable
19:27:11 <luke-jr> 8.3 sounds pretty recent; is it already a sure thing major distros will have it in their stable releases?
19:27:11 <sipa> because they're shared
19:27:22 <MarcoFalke> gcc 7 is enough: https://github.com/bitcoin/bitcoin/pull/19183/files#diff-0c8311709d11060c5156268e58f5f209R14
19:27:57 <wumpus> MarcoFalke: okay maybe I'm misreading the issue then
19:27:59 <aj> 8.3 is in debian stable as the default gcc, only gcc 6 in oldstable
19:28:15 <luke-jr> aj: RHEL tends to be the bottleneck
19:28:18 <wumpus> in any case there is going to be a large bump
19:28:38 <MarcoFalke> sipa: Maybe rebase to see if msvc can compile it with C++17. If not, there is something else to look into first anyway. Pretty sure the 3 months will pass quickly ;)
19:28:39 <aj> luke-jr: it has software collections now so you get new gcc/clang on old rhel pretty easy
19:28:54 <sipa> RHEL8 has gcc 8.2
19:28:57 <luke-jr> aj: oh!
19:29:37 <sipa> RHEL7 uses gcc 4.8 by default
19:29:53 <luke-jr> do we care about old stables now?
19:29:53 <sipa> (we've strayed a bit from the topic, but that's ok unless someone has something else)
19:30:01 <aj> https://www.softwarecollections.org/en/scls/rhscl/devtoolset-8/  -- gcc 8 for rhel 6 and 7
19:30:08 <instagibbs> mempool delta notifications topic
19:30:13 <wumpus> can always cross-compile anyway
19:30:21 <luke-jr> wumpus: that's a bit much for most users
19:30:34 <wumpus> #topic mempool delta notifications (instagibbs)
19:30:56 <cfields> sipa: I was under the impression you weren't supposed to inherit from std::allocator in c++11.
19:31:04 <cfields> ok, will look more after the meeting.
19:31:10 <instagibbs> Ok, so recently I wrote a one-off zmq notification for mempool evictions, which are currently not covered. Other people have more exntensive ideas: https://github.com/bitcoin/bitcoin/pull/19476
19:31:18 <instagibbs> shuckc, can you speak to motivation/usage?
19:31:19 <wumpus> luke-jr: maybe, it's not that much more difficult, especially nowadays with the extreme availability of VMs etc
19:31:32 <sipa> cfields: ah, i can try avoiding that
19:31:43 <instagibbs> promag also made a WIP mempool delta RPC as another possible option https://github.com/bitcoin/bitcoin/pull/19476
19:32:00 <cfields> sipa: no idea if that's useful, need to spend a whole lot more time understanding what you're doing :)
19:32:53 <shuckc> We track a huge number of wallets, keeping mempool contents synchronised is tricky given incomplete notifications, and difficult to sychronise between api and zmq notifications
19:33:35 <shuckc> seems like an opportunity to cover off a lot of the edge cases in one go
19:33:48 <instagibbs> so ideally you could getrawmempool like once, then use zmq notifications to track delta, then maybe call getrawmempool when something drops for whatever reason, and be able to figure out "where" in that notification stream the snapshot is from
19:33:54 <luke-jr> instagibbs: you linked the same PR twice
19:33:58 <instagibbs> oh woops
19:34:09 <instagibbs> https://github.com/bitcoin/bitcoin/pull/19462#issueacomment-656140421
19:34:11 <luke-jr> instagibbs: ZMQ is very unreliable..
19:34:38 <wumpus> instagibbs: that makes sense
19:34:45 <wumpus> no, ZMQ is not very unreliable
19:34:52 <instagibbs> sure luke-jr so alternative would likely look like promag pr i linked
19:34:56 <instagibbs> maybe long polling
19:34:59 <wumpus> only in pretty extreme circumstances it sometimes drops a packet
19:35:02 <sipa> ZMQ has no reliability _guarantees_
19:35:17 <wumpus> and in that case there needs to be a way to resynchronize, anyway, as instagibbs  says
19:35:27 <luke-jr> wumpus: I used it for low traffic on a reliable network, and it still lost stuff regularly
19:35:28 <sipa> but absent overflow conditiins, it is very reliable
19:35:32 <wumpus> notifications have sequence numbers to be able to detect that
19:35:38 <sipa> luke-jr: hmm, ok
19:35:39 <instagibbs> and avoiding "lots" of getrawmempools I guess is hte biggest goal
19:35:44 <wumpus> sipa: yes, in the general case it is very reliable
19:35:47 <promag> the biggest problem if you have to take the client offline for a bit
19:35:54 <shuckc> ZMQ generally work as well as any other pubsub system so long as you have the high watermark set sufficiently high, and you are not trying to consume slower than the publisher is producing. I don't see that as a particular obstancle
19:36:11 <wumpus> unless your client is not consuming the notifications reliably: there can't be an infinite buffer
19:36:24 <wumpus> (unless you'd spool to disk or something like that)
19:36:47 <promag> shuckc: zmq pub doesn't hold msg if client is offline.
19:36:51 <wumpus> but I don't think adding yet another notifications system with mail-like reliablity is really what we want
19:37:10 <shuckc> if your client goes away, you are going to have to hit getrawmempools for sure. but would like to avoid those calls in the general case as big result set (even when brief)
19:37:39 <wumpus> I mean there's mq systems like rabbitmq that guarantee 100% reliability
19:37:51 <promag> the approach in #19476 avoids periodic getrawmempools
19:37:53 <gribble> https://github.com/bitcoin/bitcoin/issues/19476 | wip, rpc: Add mempoolchanges by promag · Pull Request #19476 · bitcoin/bitcoin · GitHub
19:38:00 <jnewbery> why not have ZMQ log every txid it sends a notification for along with seq number. If your client detects a drop it can consult the log and query the mempool for that txid?
19:38:04 <wumpus> but I mean, how many of these things do you want to integrate with
19:38:07 <instagibbs> promag, your rpc could be maybe generalized into block hash announcements too, connect/disconnect
19:38:24 <instagibbs> jnewbery, it has as seq number
19:38:36 <instagibbs> but right now it's a hodgepodge of endpoints
19:38:41 <wumpus> yes, it has a seq number
19:38:43 <instagibbs> and missing eviction entirely
19:39:07 <jnewbery> right, we're talking about two things here. Let's assume that your eviction PR is merged
19:39:11 <wumpus> all the zmq endpoints have seq numbers
19:39:31 <shuckc> if eviction PR is merged, the remaining issues are:
19:39:31 <wumpus> I'mnot sure these are actually useful because people keep complaining about this
19:39:53 <promag> wumpus: you don't know at what sequence corresponds a getrawmempool response
19:40:02 <wumpus> no, indeed you don't
19:40:15 <shuckc> I cannot know for sure if the txn hash broadcasts are adds or block removals, as they don't specify, and I can't for sure know how it lines up with the results of getrawmempool
19:40:16 <jnewbery> for reliability, ZMQ can log seq_num:txid to file every time a notification is sent. If a client detects a missing seq_num, you consult the log and query the mempool rpc for that file
19:40:35 <wumpus> zmq logging to file? taht sounds really at odd with low-latency
19:40:35 <jnewbery> *for that txid
19:40:45 <luke-jr> wumpus: mkfifo ☺
19:40:55 <wumpus> luke-jr: fifo is one to one, not one to many
19:40:56 <luke-jr> not sure why ZMQ is involved at that point lol
19:40:58 <luke-jr> true
19:41:05 <wumpus> luke-jr: if only UNIX had a one to many notification mechanism
19:41:13 <wumpus> except for mail
19:41:14 <luke-jr> dbus?
19:41:27 <sipa> wumpus: oh you can have many writers and many readers for one fifo just fine ;)    and no guarantee which write goes to which read
19:41:29 <aj> wumpus: wall(1) :)
19:41:32 * luke-jr is not sure dbus actually has this
19:41:35 <luke-jr> aj: lol
19:41:38 <wumpus> no, dbus doesn't have it either
19:41:48 <wumpus> dbus is one to one, it has no realible multi consumer broadcase
19:42:13 <luke-jr> I suppose you could just use a tmpfs file
19:42:13 <wumpus> it's a difficult issue in general
19:42:20 <wumpus> because some consumer might always be lagging
19:42:21 <luke-jr> and punch holes at the start as desired
19:42:29 <wumpus> this can potentially result in infinitely much storage needed
19:42:36 <luke-jr> or store to disk and let Linux's buffers handle it
19:42:47 <wumpus> rabbitmq is pretty good if you really need this
19:45:10 <instagibbs> Well aside from fixing infinite buffer problems, I think it'd be good to improve where we can. When there's a failure there's always the fallback of getrawmempool for example
19:45:11 <jnewbery> wumpus: zmq already logs (if -logging=zmq is enabled). It just doesn't log the seq num, so it's not easy for a client to tell which messages were dropped
19:45:32 <instagibbs> I was joking that you could also do minisketch for set reconciliation of mempool views
19:45:47 <sipa> haha
19:45:54 <luke-jr> jnewbery: I'm not sure we want to encourage software to parse debug.log …
19:45:58 <instagibbs> zmq to keep difference small ;)
19:46:00 <wumpus> jnewbery: I don't think clients can ever know what message is dropped; usually, missing a sequnence number means having to resyncronize in some way (e.g.e query the entire mempool)
19:46:11 <wumpus> luke-jr: exactly
19:46:30 <wumpus> I don't think 'parse the log' is a good option, though it serves one-to-many notification perfectly
19:46:37 <wumpus> mq is essentially a log
19:46:43 <wumpus> until your disk is full
19:46:45 <luke-jr> a dedicated, well-defined-format log might be okay
19:47:07 <wumpus> it's also a high latency option but that might not matter
19:47:07 <luke-jr> but something needs to do hole-punching to clean it up before disk fills
19:47:15 <luke-jr> wumpus: why is it high latency?
19:47:23 <wumpus> yes, but if you do tha, some clients might miss messages
19:47:32 <sipa> luke-jr: bitcoind will already shut down when disk is full
19:47:33 <sipa> :)
19:47:35 <luke-jr> depends on who does it
19:47:35 <wumpus> unless they tell you they read up until that far
19:47:43 <luke-jr> sipa: yes, but you don't want that
19:47:55 <wumpus> luke-jr: because disk/block devices are slow, compared to networking, latency wise
19:48:02 <luke-jr> wumpus: Linux at least has buffers
19:48:08 <wumpus> even with that
19:48:11 <luke-jr> :/
19:48:20 <luke-jr> the write/read won't even need to hit disk
19:48:32 * luke-jr wonders if you can tell Linux to never flush to disk unless it has to
19:48:37 <luke-jr> per-device*
19:48:45 <luke-jr> per-file would also be nice :P
19:49:04 <wumpus> yes, there's an option for that afaik, but it also means in case of power loss...
19:49:47 <jnewbery> shuckc: it sounded like you were going to mention more issues. Was there anything else?
19:49:50 <wumpus> reliable delivery of messages to multiple consumers is a difficult topic
19:50:08 <sipa> we need a blockchain
19:50:13 <wumpus> sipa: :-)
19:50:31 <instagibbs> so i think the biggest oustanding issue(if evictions are announced and we're ok with drops once i na while) is being able to line up getrawmempool results with the notifications
19:50:32 <wumpus> yes, at least the blockchain always allows going back i time... well unless pruning
19:50:44 <wumpus> pruning is kind of the 'what if not all consumers have seen this yet' problem
19:50:45 <shuckc> the sequence number on the response to getrawmempool
19:51:09 <shuckc> obviously has backward compatibility concerns, and other suggestions?
19:51:20 <sipa> shuckc: hmm?
19:51:29 <luke-jr> wumpus: there is an option for that? what? :o
19:51:32 <instagibbs> sipa, he wants to know where the mempool "snapshot" came from
19:51:37 <sipa> does getrawmempool report a sequence number now?
19:51:41 <instagibbs> no
19:51:45 <wumpus> it doesn't
19:51:49 <shuckc> no, I'm suggeting it should
19:51:49 <sipa> and adding one would help?
19:51:52 <shuckc> yes
19:51:54 <sipa> oh, ok
19:52:03 <instagibbs> so you don't know if the getrawmempool result is stale or from "future" wrt zmq reports
19:52:05 <sipa> what would be the reason not to?
19:52:05 <shuckc> because you can't tell which delta(s) have already been added
19:52:09 <wumpus> I guess it could prove that there were no updates in between
19:52:12 <sipa> adding new fields has no backward compatibility concerns
19:52:19 <instagibbs> sipa, I think you'd have to add *all* the zmq notification seq numbers
19:52:24 <sipa> ah
19:52:27 <instagibbs> well it's an array result ;
19:52:28 <instagibbs> ;)
19:52:34 <promag> can bnly be added if verbose=true in getrawmempool
19:52:39 <instagibbs> but like I said I think optional arg -> json object
19:52:45 <wumpus> I wonder why every zmq message has its own sequence number
19:52:54 <wumpus> couldn't it be just one increasing atomic?
19:52:55 <shuckc> unless you have one single notifier that you use for all the messages you need to sync with
19:52:56 <instagibbs> wumpus, it's a local member of hte notification, for whatever reason
19:53:09 <promag> wumpus: a client knows if he missed anything
19:53:18 <wumpus> promag: oh, true
19:53:18 <shuckc> it's only a mess if need to track lots of notifiers
19:53:32 <wumpus> yes, makes sense, a client is generally interested in only a subset of message kinds
19:53:35 <instagibbs> shuckc, shouldn't be too bad, you just grab the one you care about
19:53:45 <wumpus> so a global sequence number would be useless
19:54:11 <promag> wumpus: not really, that number should be exposed in both rpc response and zmq message
19:54:18 <wumpus> in any case getmempool would only need the mempool related numbers...
19:54:26 <promag> but I'm not fond of that..
19:54:42 <wumpus> it seems like some kind of layer violation
19:54:46 <wumpus> having RPC query ZMQ
19:54:54 <promag> because as a client, you need to use rpc AND zmq
19:54:59 <wumpus> yes
19:55:02 <instagibbs> otherwise you need to somehow ask for unique snapshot
19:55:11 <promag> hence my draft PR
19:55:11 <instagibbs> of the mempool
19:55:34 <instagibbs> but yes, it's a light violation at least
19:55:40 <phantomcircuit> shuckc, zmq can and will silently drop messages, unless you have sequence numbers in the application layer you cannot detect that
19:55:44 <shuckc> My suggestion includes connectblock and disconnect block notifications on the same new channel, because they allow you to keep your local mempool up to date and equally you need to know where in the stream of deltas they arrived
19:56:00 <wumpus> of course, that could be worked around by having both RPC and ZMQ query another source for sequence numbers
19:56:03 <jnewbery> how about if the mempool itself had a seq number that is incremented on every add/remove?
19:56:09 <wumpus> right
19:56:19 <instagibbs> jnewbery, that's promag's pr pretty much-ish
19:56:20 <wumpus> I think that would make sense
19:57:08 <instagibbs> 3 minutes
19:57:25 <wumpus> so +1 on jnewbery/promag's idea then
19:57:44 <shuckc> with promags suggestion, bitcoind has to keep state/buffer for each consumer, the zmq model makes it state-less, and promag you also need to poll for new messages which is something of a step backwatfs
19:58:05 <wumpus> I didn't understand it like that
19:58:09 <promag> note that in my PR, the "stream" will be upper bounded in size, so no OOM concerns
19:58:10 <luke-jr> shuckc: not if he adds longpolling
19:58:15 <wumpus> the mempool itself would keep the seq number
19:58:24 <wumpus> not per consumer
19:58:29 <promag> shuckc: ill add long poll
19:58:36 <wumpus> it's kind of strange if different consumers have differnt seq ids
19:58:41 <shuckc> do any other commands use long polling?
19:58:45 <wumpus> because this is global state we're exposing
19:58:46 <luke-jr> GBT
19:58:46 <phantomcircuit> wumpus, if zmq is using a global sequence number for all messages, i'd suggest just adding that to rpc as a header or something
19:58:46 <promag> shuckc: yes
19:58:51 <instagibbs> getblocktemplate <--- GBT
19:59:12 <promag> I don't like longpoll that much tbh, at least in json rpc
19:59:33 <promag> especially because of libevent, timeouts etcetc
19:59:51 <promag> but "poll and wait up to n secs" if fine imo
19:59:52 <wumpus> I don't think adding a different notification mechanism will really solve the 'clients could stop consuming and keep behind'  problem
20:00:02 <luke-jr> promag: same thing? :P
20:00:03 <wumpus> it would mean accumulating in memory in that case?
20:00:20 <promag> luke-jr: n secs < timeoud (:
20:00:26 <promag> wumpus: yes
20:00:31 <fjahr> I guess I am the hammer that sees nails everywhere, but how about a hash (muhash) for the mempool states instead of a sequence number? but not sure i grasp the problem completely yet...
20:00:33 <promag> but thats's fine
20:00:50 <luke-jr> fjahr: then you need to log the hash..
20:00:55 <promag> if the client doensn't pull the stream, the stream will be terminated
20:00:56 <wumpus> no, I don't think that's fine, if there's no limit a client could forget to connect and fill your memory entirely
20:01:00 <instagibbs> fjahr, even more ridiculous than my minisketch idea ;P
20:01:10 <wumpus> promag: that's just another "lose notifications" then
20:01:17 <fjahr> hehe
20:01:41 <luke-jr> wumpus: it's one thing to begin dropping stuff after N minutes of downtime; another to lose them randomly as a normal event
20:01:47 <wumpus> reliable notification is really a non-trivial issue :)
20:01:49 <promag> wumpus: no, the stream will be terminated, the client starts over and gets a fresh stream
20:01:57 <wumpus> in any case, it's time
20:02:09 <aj> instagibbs: (minisketch is a great idea!)
20:02:11 <wumpus> #endmeeting