< bitcoin-git> [bitcoin] jnewbery closed pull request #17601: Validation: Move CheckBlock() mutation guard to AcceptBlock() (master...2019-11-checkblock-in-acceptblock) https://github.com/bitcoin/bitcoin/pull/17601
< bitcoin-git> [bitcoin] hebasto opened pull request #19473: net: Add -networkactive option (master...200709-setnet) https://github.com/bitcoin/bitcoin/pull/19473
< fanquake> jonatack: are you using the (I assume self-built) Clang master branch for development/review?
< jonatack> fanquake: i changed from using debian stable to debian testing and added "deb http://apt.llvm.org/unstable/ llvm-toolchain main" to etc/apt/sources.list
< fanquake> Right. Why not use Clang 9/10 though?
< jonatack> No strong opinion yet, updated only last weekend. Which do you think is best for development/review?
< jonatack> My motivation was for testing the cfields' span PR with -Wdangling and have clang-format work again (it's currently broken for clang < 9)
< fanquake> Preferably a compiler that in not unreleased/WIP. 9 or 10 shouldn't make much difference. I was just curious after seeing your review comments.
< jonatack> Ok. FWIW #19454 fixes .clang-format for clang < 9, e.g. Debian stable
< gribble> https://github.com/bitcoin/bitcoin/issues/19454 | tools: `.clang-format` compat with clang versions < 9 by jonatack · Pull Request #19454 · bitcoin/bitcoin · GitHub
< jonatack> Will go for 10.
< someone235> Hi, does bitcoin core keep node ban score even if it reconnects (even if it's below the ban threshold)?
< sipa> no
< someone235> Thanks!
< someone235> What's the reasoning behind it?
< someone235> I would expect it to be kept so you reconnect each time your ban score is 90
< sipa> ban score logic doesn't prevent attacks
< sipa> it prevents wasting connection slots on broken software
< someone235> Sorry, I mean, so you *wont* reconnect each time your ban score is 90
< sipa> and i think ban scores as a concept should just go away
< someone235> Why?
< jonatack> someone235: good explanation why here: https://github.com/bitcoin/bitcoin/pull/19219#issuecomment-652684340
< sipa> if a client miabehaves they should be disconnected and/or banned immediately
< sipa> if they don't, we ahouldn't confuse things by maybe later doing that when some threshold is reached
< sipa> it just obscores things for no benefit
< someone235> Yeah, if we're talking about not talking with broken nodes I tend to agree
< sipa> yeah see jonatack's link
< someone235> But how will you deal with attackers? Like people who sends you full invalid blocks?
< someone235> *send
< sipa> ban them
< sipa> immediately
< someone235> oh right
< sipa> but also
< someone235> and bans are kept between reconnects
< someone235> and restarts
< sipa> there are plenty of ways to trivially DoS attack nodes by doing completely legitimate things
< sipa> that we can't ban for
< sipa> the eventual way to deal with that imho is tracking how muxh resources we spend on behalf of each peer
< sipa> and if we run out, deprioritize or disconnect the worst offenders (without banning)
< bitcoin-git> [bitcoin] kiminuo closed pull request #19466: Use operator/ in fs::absolute to prepare for C++17 (master...feature/2020-07-08-fs-paths) https://github.com/bitcoin/bitcoin/pull/19466
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/f7c19e829eca...c4a44186d816
< bitcoin-git> bitcoin/master e846a2a John Newbery: refactor: clean up PeriodicFlush()
< bitcoin-git> bitcoin/master c4a4418 MarcoFalke: Merge #19085: Refactor: clean up PeriodicFlush()
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #19085: Refactor: clean up PeriodicFlush() (master...2020-05-refactor-periodic-flush) https://github.com/bitcoin/bitcoin/pull/19085
< bitcoin-git> [bitcoin] fanquake pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/c4a44186d816...6b48c30541e4
< bitcoin-git> bitcoin/master b9253c7 Jon Atack: tools: clang-format 6 compatibility
< bitcoin-git> bitcoin/master 6b48c30 fanquake: Merge #19454: tools: `.clang-format` compat with clang versions < 9
< bitcoin-git> [bitcoin] fanquake merged pull request #19454: tools: `.clang-format` compat with clang versions < 9 (master...clang-6-compatibility) https://github.com/bitcoin/bitcoin/pull/19454
< bitcoin-git> [bitcoin] fanquake pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/6b48c30541e4...e3b31255c5ad
< bitcoin-git> bitcoin/master 0b8ba84 fanquake: banlist: log post-swept banlist size at startup
< bitcoin-git> bitcoin/master e3b3125 fanquake: Merge #19470: banlist: log post-swept banlist size at startup
< bitcoin-git> [bitcoin] fanquake merged pull request #19470: banlist: log post-swept banlist size at startup (master...log_post_sweep_banlist_size) https://github.com/bitcoin/bitcoin/pull/19470
< bitcoin-git> [bitcoin] fanquake closed pull request #18239: wip: gui: Refactor to drop client and wallet models setters (master...2020-03-drop-setmodel) https://github.com/bitcoin/bitcoin/pull/18239
< bitcoin-git> [bitcoin] fanquake closed pull request #17611: gui: Make send and receive widgets extend QWidget (master...2019-11-send-receive-widgets) https://github.com/bitcoin/bitcoin/pull/17611
< bitcoin-git> [bitcoin] fanquake pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/e3b31255c5ad...64bf5d64dfbf
< bitcoin-git> bitcoin/master 1e9bfd4 Hennadii Stepanov: qt: Reset toolbar after all wallets are closed
< bitcoin-git> bitcoin/master 64bf5d6 fanquake: Merge #18896: qt: Reset toolbar after all wallets are closed
< bitcoin-git> [bitcoin] fanquake merged pull request #18896: qt: Reset toolbar after all wallets are closed (master...200506-toolbar) https://github.com/bitcoin/bitcoin/pull/18896
< bitcoin-git> [bitcoin] fanquake closed pull request #17966: qt, refactor: Optimize signal-slot connections logic (master...20200119-gui-walletframe) https://github.com/bitcoin/bitcoin/pull/17966
< bitcoin-git> [bitcoin] fanquake closed pull request #18618: gui: Drop RecentRequestsTableModel dependency to WalletModel (master...2020-04-review-18608) https://github.com/bitcoin/bitcoin/pull/18618
< bitcoin-git> [bitcoin] laanwj pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/64bf5d64dfbf...21028ce9ffb8
< bitcoin-git> bitcoin/master 4b5ac25 Hennadii Stepanov: Drop unused CDBWrapper methods
< bitcoin-git> bitcoin/master 21028ce Wladimir J. van der Laan: Merge #19468: refactor: Drop unused CDBWrapper methods
< bitcoin-git> [bitcoin] laanwj merged pull request #19468: refactor: Drop unused CDBWrapper methods (master...200708-dbwrapper) https://github.com/bitcoin/bitcoin/pull/19468
< bitcoin-git> [bitcoin] fanquake closed pull request #17955: gui: Paste button in Open URI dialog (master...2020-01-paste-bitcoin-uri-button) https://github.com/bitcoin/bitcoin/pull/17955
< bitcoin-git> [bitcoin] laanwj pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/21028ce9ffb8...fc8da23fbe58
< bitcoin-git> bitcoin/master 2894e94 Aaron Clauson: Updates msvc build to use ISO standard C++17.
< bitcoin-git> bitcoin/master fc8da23 Wladimir J. van der Laan: Merge #19445: build: Update msvc build to use ISO standard C++17
< bitcoin-git> [bitcoin] laanwj merged pull request #19445: build: Update msvc build to use ISO standard C++17 (master...msvc_cpp17) https://github.com/bitcoin/bitcoin/pull/19445
< bitcoin-git> [bitcoin] fanquake pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/fc8da23fbe58...45f58dbc9b45
< bitcoin-git> bitcoin/master 2b78a11 nsa: doc: afl fuzzing comment about afl-gcc and afl-g++
< bitcoin-git> bitcoin/master 45f58db fanquake: Merge #19452: doc: afl fuzzing comment about afl-gcc and afl-g++
< bitcoin-git> [bitcoin] fanquake merged pull request #19452: doc: afl fuzzing comment about afl-gcc and afl-g++ (master...afl_gcc_0705) https://github.com/bitcoin/bitcoin/pull/19452
< bitcoin-git> [bitcoin] fanquake closed pull request #18865: gui: Refactor WalletFrame to extend QStackView (master...2020-05-walletframe) https://github.com/bitcoin/bitcoin/pull/18865
< bitcoin-git> [bitcoin] laanwj pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/45f58dbc9b45...9a3c7afe290f
< bitcoin-git> bitcoin/master c858302 jmorgan: Change format of log2_work for uniform output (zero-padded)
< bitcoin-git> bitcoin/master 9a3c7af Wladimir J. van der Laan: Merge #19317: Add a left-justified width field to log2_work component for ...
< willcl_ark> Is it possible to build the fuzzing harness (only) with gcc rather than clang?
< MarcoFalke> willcl_ark: Yes, e.g. with afl-gcc
< MarcoFalke> see doc/fuzzing.md
< MarcoFalke> #19452
< gribble> https://github.com/bitcoin/bitcoin/issues/19452 | doc: afl fuzzing comment about afl-gcc and afl-g++ by Crypt-iQ · Pull Request #19452 · bitcoin/bitcoin · GitHub
< willcl_ark> MarcoFalke: Right. I'm looking at #19388 a bit more, and it seems to me undesirable that we'd now require e.g. afl-gcc for all the gcc builds if fuzzing harnesses were built by default.
< gribble> https://github.com/bitcoin/bitcoin/issues/19388 | Build fuzz tests by default · Issue #19388 · bitcoin/bitcoin · GitHub
< MarcoFalke> Oh, if you don't need instrumentation, you should be able to use any cpp compiler
< MarcoFalke> The fuzz engine only provides the main function, but the individual fuzz tests don't know that. They can be compiled with any cpp compiler
< willcl_ark> OK, that's what I understood too (from Practicalswift's comments in https://github.com/bitcoin/bitcoin/pull/19366#issue-438840241), but i've been struggling with gcc. If it should be possible I'll keep grappling :)
< willcl_ark> My issues are mainly with the linking
< MarcoFalke> Yes, there might be issues linking the main function
< willcl_ark> Yeah that's the general complaint
< MarcoFalke> I'd have to look at the code or failure to say more, but generally it should be possible
< willcl_ark> OK thanks, perhaps I'll ping you in the issue when I have a clearer error to look at
< bitcoin-git> [bitcoin] fanquake pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/9a3c7afe290f...22ccc27046a8
< bitcoin-git> bitcoin/master fc6a637 10xcryptodev: qt: increase console command max length
< bitcoin-git> bitcoin/master 22ccc27 fanquake: Merge #18993: qt: increase console command max length
< bitcoin-git> [bitcoin] fanquake merged pull request #18993: qt: increase console command max length (master...202005-increase-console-command) https://github.com/bitcoin/bitcoin/pull/18993
< fanquake> I've swapped out my current high-prio PR for #19224.
< gribble> https://github.com/bitcoin/bitcoin/issues/19224 | [0.20] Backports by fanquake · Pull Request #19224 · bitcoin/bitcoin · GitHub
< fanquake> Review-beg if you have a PR that is being back-ported there.
< bitcoin-git> [bitcoin] dongcarl closed pull request #17099: depends: Eliminate hard dependency on Ubuntu-ABI specific clang (master...2019-09-no-clang-download) https://github.com/bitcoin/bitcoin/pull/17099
< bitcoin-git> [bitcoin] laanwj pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/22ccc27046a8...0d69fdb9a0e3
< bitcoin-git> bitcoin/master 1cabbdd Aaron Hook: refactor: Use uint16_t instead of unsigned short
< bitcoin-git> bitcoin/master 0d69fdb Wladimir J. van der Laan: Merge #19314: refactor: Use uint16_t instead of unsigned short
< bitcoin-git> [bitcoin] laanwj merged pull request #19314: refactor: Use uint16_t instead of unsigned short (master...akh_uint16_t) https://github.com/bitcoin/bitcoin/pull/19314
< instagibbs> suggested topic: unified zmq stream + mempool sequence number https://github.com/bitcoin/bitcoin/pull/19462#issuecomment-656140421
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/0d69fdb9a0e3...cc9d09e73de0
< bitcoin-git> bitcoin/master fa0540c MarcoFalke: net: Extract download permission from noban
< bitcoin-git> bitcoin/master cc9d09e MarcoFalke: Merge #19191: net: Extract download permission from noban
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #19191: net: Extract download permission from noban (master...2006-netPerDow) https://github.com/bitcoin/bitcoin/pull/19191
< bitcoin-git> [bitcoin] fanquake closed pull request #19001: qt: bugfix unsupported QLocale languages (master...202005-bugfix-qlocale) https://github.com/bitcoin/bitcoin/pull/19001
< bitcoin-git> [bitcoin] fanquake closed pull request #18604: Display command line usage to console without requiring X Windows (master...UsageToConsoleIfNotWin32) https://github.com/bitcoin/bitcoin/pull/18604
< bitcoin-git> [bitcoin] MarcoFalke opened pull request #19474: doc: Use precise permission flags where possible (master...2007-docPerm) https://github.com/bitcoin/bitcoin/pull/19474
< bitcoin-git> [bitcoin] laanwj pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/cc9d09e73de0...fd9db45c3ea1
< bitcoin-git> bitcoin/master a4a3fc4 Sjors Provoost: doc: improve subtree check instructions
< bitcoin-git> bitcoin/master fd9db45 Wladimir J. van der Laan: Merge #19258: doc: improve subtree check instructions
< bitcoin-git> [bitcoin] laanwj merged pull request #19258: doc: improve subtree check instructions (master...2020/06/subtree-docs) https://github.com/bitcoin/bitcoin/pull/19258
< bitcoin-git> [bitcoin] Mohamed-94 opened pull request #19475: script: Speed up default time-out and Container CPU usage (master...patch-1) https://github.com/bitcoin/bitcoin/pull/19475
< luke-jr> apparently there's a market for TNBTC
< luke-jr> kindof.
< instagibbs> it's hard to get your hands on large amounts unless you "know people"
< phantomcircuit> instagibbs, it's extremely easy to mine a huge amount of tnbtc if you modify a stratum proxy very slightly
< instagibbs> someone is warp mining this morning AFAICT
< cfields> jonatack: that warning has me completely stumped, and now I'm curious. Building clang from source now to play along.
< jonatack> cfields: idk if it's worth it. i didn't install from source. added "deb http://apt.llvm.org/unstable/ llvm-toolchain main" to my etc/apt/sources.list
< cfields> ack
< jonatack> fanquake suggested this am that i move back to clang stable 9 or 10 for reviewing purposes
< cfields> jonatack: do you get a ton of warnings? Or just that one?
< jonatack> just the one. i agree with you that it doesn't make sense.
< cfields> you actually checked out the commit, right? Didn't c/p from github?
< jonatack> yes, i always do. build several times with make distclean to be sure it was really doing that, and also compared with a clean gcc 9.3 build
< jonatack> built*
< cfields> So weird!
< cfields> We'll see if I can reproduce from a git build.
< sipa> jonatack: and you only get the warning on the new code introducsd to attrihutes.h?
< sipa> not anywhere else?
< jonatack> I'm afraid I've led you into a goosechase cfields. sipa: correct, but i've only been reviewing with this version of clang since sunday.
< cfields> jonatack: no worries, I needed to build llvm to test recent lld anyway. That's the only reason I jumped to compile so quickly.
< jonatack> i was on debian clang 6 which does not have -Wdangling and wanted to test cory's PR
< jonatack> cfields: yes. LMK
< jonatack> rebuilt just now again, saw it again. but also apt-update has new clang updates. installing and rebuilding.
< jonatack> cfields: upgraded from 11.0.0-++20200708111116+1f84ace3c72-1~exp1~20200708091733.3345 to 11.0.0-++20200709111134+dc4a6f5db4f-1~exp1~20200709091753.3347 and the warning is gone.
< sipa> chased goose is best goose
< jonatack> :D
< cfields> jonatack: heh, well that's rather unsatisfying
< cfields> jonatack: igoring all that, thanks for jumping through hoops to test :)
< jeremyrubin> #proposedmeetingtopic small clarification on the goals of the mempool project (cap at 5 mins meeting time)
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/fd9db45c3ea1...2aaff4813cc3
< bitcoin-git> bitcoin/master fa6d5ab MarcoFalke: refactor: Remove unused BlockAssembler::pblock member var
< bitcoin-git> bitcoin/master 2aaff48 MarcoFalke: Merge #19283: refactor: Remove unused BlockAssembler::pblock member var
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #19283: refactor: Remove unused BlockAssembler::pblock member var (master...2006-minerPblock) https://github.com/bitcoin/bitcoin/pull/19283
< bitcoin-git> [bitcoin] promag opened pull request #19476: wip, rpc: Add mempoolchanges (master...2020-07-rpc-mempoolchanges) https://github.com/bitcoin/bitcoin/pull/19476
< promag_> instagibbs: #19476
< gribble> https://github.com/bitcoin/bitcoin/issues/19476 | wip, rpc: Add mempoolchanges by promag · Pull Request #19476 · bitcoin/bitcoin · GitHub
< instagibbs> promag_, yeah saw that
< instagibbs> can someone give shuckc voice?
< sipa> shuckc voice?
< instagibbs> i am not irc wizard, he says he cannot say anything here
< achow101> pretty sure you need to be registered
< instagibbs> ah ok
< aj> yeah
< aj> register with nickserv
< wumpus> there should be no reason to deal out voice manually here, it's not a +m channel, but yes, registering with nickserv is required
< instagibbs> he's not registered, mystery solved
< MarcoFalke> Can someone edit the topic to say that register with nicksers is needed?
< wumpus> FWIW we've tried to remove that requirement many times and every time immediately some spam appeared
< MarcoFalke> I am not an IRC wizard either and I was muted at least twice in a meeting and didn't notice
< wumpus> I've been caught by that as well the feedback is pretty bad
< troygiorshev> we don't have +r though, so why is that happenin?
< shuckc> better?
< wumpus> shuckc: yes
< luke-jr> MarcoFalke: not sure topic change will help that..
< instagibbs> shuckc, see you
< luke-jr> MarcoFalke: especially if you don't notice the error that your msg didn't go
< promag> meeting?
< achow101> meeting?
< wumpus> #startmeeting
< MarcoFalke> ahoi
< lightningbot> Meeting started Thu Jul 9 19:00:52 2020 UTC. The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot.
< lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
< hebasto> hi
< achow101> hi
< troygiorshev> hi
< jnewbery> hi
< 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
< wumpus> jeremyrubin lightlike emilengler jonatack hebasto jb55 elichai2
< jb55> hi
< fjahr> hi
< amiti> hi
< luke-jr> ih
< promag> hi
< kanzure> hi
< jonasschnelli> hi
< jeremyrubin> hola
< gwillen> hi
< cfields> hi
< sipa> hi
< wumpus> one proposed meeting topic: small clarification on the goals of the mempool project (jeremyrubin)
< wumpus> any last minute topics?
< phantomcircuit> hi
< sipa> another short one: can we drop gcc 4.8?
< wumpus> ok
< 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
< luke-jr> sipa: not sure that's a good meeting topic; it just needs someone to investigate distros?
< ariard> hi
< elichai2> hu
< wumpus> #topic High priority for review
< MarcoFalke> can i haz #19386
< achow101> #19325 pls
< 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
< gribble> https://github.com/bitcoin/bitcoin/issues/19325 | wallet: Refactor BerkeleyDatabase to introduce DatabaseBatch abstract class by achow101 · Pull Request #19325 · bitcoin/bitcoin · GitHub
< wumpus> https://github.com/bitcoin/bitcoin/projects/8 13 blockers, 1 bugfix, 3 chasing concept ACK
< wumpus> please don't add any more before removing any xD
< jamesob> hi
< ariard> you can drop #18787, after talking with few people, a libstandardness would be more accurate
< gribble> https://github.com/bitcoin/bitcoin/issues/18787 | wallet: descriptor wallet release notes and cleanups by achow101 · Pull Request #18787 · bitcoin/bitcoin · GitHub
< ariard> #18797 sorry
< gribble> https://github.com/bitcoin/bitcoin/issues/18797 | Export standard Script flags in bitcoinconsensus by ariard · Pull Request #18797 · bitcoin/bitcoin · GitHub
< jeremyrubin> removing #18191
< gribble> https://github.com/bitcoin/bitcoin/issues/18191 | Change UpdateForDescendants to use Epochs by JeremyRubin · Pull Request #18191 · bitcoin/bitcoin · GitHub
< jonatack> hi
< promag> please see #19033, its for 0.20.1
< gribble> https://github.com/bitcoin/bitcoin/issues/19033 | http: Release work queue after event base finish by promag · Pull Request #19033 · bitcoin/bitcoin · GitHub
< wumpus> ariard, achow101 done
< wumpus> jeremyrubin: ok
< luke-jr> would be nice to get the build tarballs fixed finally too <.<
< meshcollider> hi
< pinheadmz1> hi
< wumpus> #topic clarification on the goals of the mempool project (jeremyrubin)
< jeremyrubin> Yeah just quick;
< jeremyrubin> So I think that there's some confusion or at least co-mingling of concerns based on how I've presented things
< jeremyrubin> A lot of the motivation is to reduce the memory usage in the mempool
< jeremyrubin> and to better bound the algorithmic complexity on functions
< jeremyrubin> So that one day, maybe, we could lift some restrictions (e.g., descendents)
< jeremyrubin> It's less so "make mempool faster now" performance motivation.
< wumpus> ok good to know
< 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
< jeremyrubin> especially in cases where there may be a distant invariant which is currently protecting us from a "crash the network" style DoS
< luke-jr> if those are the goals, I wonder if it might make sense to move complexity into the miner
< wumpus> we could have benchmarks that measure memory usage I suppose
< luke-jr> but that could interfere with compact blocks
< jeremyrubin> luke-jr: this does make sense for certain things.
< luke-jr> I wonder if it'd be possible to support a build without mining support that performs better
< 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
< jeremyrubin> And you need both whole-system and unit tests for the functions
< 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
< jeremyrubin> And both asymptotic and bounded size
< cfields> wumpus: +1
< luke-jr> wumpus: by making mining build-time-optional, we could possibly get both cheaply?
< ariard> I'm not sure about reducing the memory usage, what the relation between size of your mempool and perf of fee estimation ?
< luke-jr> I wonder if there's a way to change class sizes at runtime in C++
< jeremyrubin> Reducing memory usage is related to DoS primarily
< wumpus> that's kind of annoying for testing but yes
< jeremyrubin> So the goal is to eliminate the class of vuln
< luke-jr> jeremyrubin: eh? users can always adjust their mempool size
< jeremyrubin> I don't care about performance here that much, even a 2x slower mempool with less DoS surface is probably fine
< wumpus> it doesn't help if only the miners would have the vulnerability though
< sipa> mempool size _predictability_ is a DoS concern
< jeremyrubin> luke-jr: no for the algorithms themselves
< luke-jr> reducing mem usage would just mean more txs per MB
< jeremyrubin> not for the mempool size itself
< ariard> I think there is a confusion there with memory usage of caching data structure for update and overall mempool size
< sipa> e.g. improving average memory usage on average, but worsening it under a particular edge case might constitute a vulnerability
< jeremyrubin> Mostly fixing short-lived allocations
< nehan> jeremyrubin: what is your expected memory usage improvement?
< luke-jr> sipa: but there's no vuln with zero mempool…
< jeremyrubin> nehan: it's a project with a bunch of algorithm improvements based on epochs for traversal instead of sets
< aj> (5min cap exceeded fwiw)
< jeremyrubin> ah yeah didn't mean to turn this into an ordeal, just trying to clarify
< luke-jr> not like we have any other topics?
< sipa> perhaps just document this as a summary on the relevant PRs?
< jeremyrubin> happy to move on if there's other things to discuss
< sipa> unless it already is
< wumpus> luke-jr: we do, sipa had a topic
< luke-jr> oh
< sipa> nothing important
< luke-jr> right
< wumpus> #topic can we drop gcc 4.8 (sipa)
< MarcoFalke> why?
< wumpus> just something to annoy fanquake
< wumpus> :)
< luke-jr> lol
< cfields> lol
< sipa> so, i was looking at #18086 again
< gribble> https://github.com/bitcoin/bitcoin/issues/18086 | Accurately account for mempool index memory by sipa · Pull Request #18086 · bitcoin/bitcoin · GitHub
< sipa> and was trying to make the accounting allocator not track copies of containers, which is a feature of the C++11 allocator infrastructure
< wumpus> I *really* think we should wait with bumping gcc versions until c++17 requirement
< sipa> but it seems gcc 4.8 ignores it or otherwise fails to implement it correctly
< wumpus> which isn't too far away, just one major version
< sipa> yeah
< luke-jr> sipa: not the end of the world if it's wrong?
< sipa> but i've just noticed at appveyor also fails at it :s
< sipa> luke-jr: could cause UB
< luke-jr> hmm
< sipa> if used in totally reasonable ways
< sipa> (but probably not in my actual PR)
< 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 :)
< luke-jr> when C++20? :p
< wumpus> in 2025
< * luke-jr> found C++20 to be required for totally obvious/expected features the other day :/
< sipa> luke-jr: hard to talk about things that don't exist
< cfields> luke-jr: get sipa to backport for you like Span :p
< luke-jr> cfields: can't backport syntax
< MarcoFalke> Only 3.6 months till branch off: https://github.com/bitcoin/bitcoin/issues/18947
< luke-jr> struct type var = { .a = 1, .b = 2 }
< jnewbery> sipa: can you #ifdef support for accounting allocators for only certain versions of gcc until we move to c++ 17?
< wumpus> jnewbery: +1!
< sipa> jnewbery: ugh
< aj> luke-jr: omg, don't tease things like that
< sipa> that's horrendous
< luke-jr> aj: it's common C99, no clue why C++ forbids it :/
< cfields> luke-jr: juse use unnamed initializers ?
< luke-jr> cfields: but the whole point is the names!
< wumpus> I mean, accurate memory accounting is nice but not critical I suppose, not enough reason to really forbid building on a platform
< luke-jr> cfields: I ended up putting defaults for every member, and just setting the ones I care about after init
< sipa> wumpus: it would mean ifdefs all over to maintain to old heuristics for every data structure, plus a accounting based one
< sipa> so i guess i'd just wait instead
< wumpus> anyhow according to #16684 the minimum gcc version will go to 8.3
< gribble> https://github.com/bitcoin/bitcoin/issues/16684 | Discussion: upgrading to C++17 · Issue #16684 · bitcoin/bitcoin · GitHub
< sipa> or try to minimize the risk of copying
< cfields> sipa: can you point to exactly what old gcc gets wrong? I'm just curious to see.
< jnewbery> sipa: ah ok. If it's super invasive to do, then not worth it
< sipa> cfields: have a container with an accounting allocator, encapsulated in some class
< sipa> return a copy of it for public consumption
< wumpus> is changing this really urgent or can it wait until after 0.21?
< sipa> now any changes to that copy need to lock the origin datastructure's accounting variable
< luke-jr> 8.3 sounds pretty recent; is it already a sure thing major distros will have it in their stable releases?
< sipa> because they're shared
< wumpus> MarcoFalke: okay maybe I'm misreading the issue then
< aj> 8.3 is in debian stable as the default gcc, only gcc 6 in oldstable
< luke-jr> aj: RHEL tends to be the bottleneck
< wumpus> in any case there is going to be a large bump
< 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 ;)
< aj> luke-jr: it has software collections now so you get new gcc/clang on old rhel pretty easy
< sipa> RHEL8 has gcc 8.2
< luke-jr> aj: oh!
< sipa> RHEL7 uses gcc 4.8 by default
< luke-jr> do we care about old stables now?
< sipa> (we've strayed a bit from the topic, but that's ok unless someone has something else)
< instagibbs> mempool delta notifications topic
< wumpus> can always cross-compile anyway
< luke-jr> wumpus: that's a bit much for most users
< wumpus> #topic mempool delta notifications (instagibbs)
< cfields> sipa: I was under the impression you weren't supposed to inherit from std::allocator in c++11.
< cfields> ok, will look more after the meeting.
< 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
< instagibbs> shuckc, can you speak to motivation/usage?
< wumpus> luke-jr: maybe, it's not that much more difficult, especially nowadays with the extreme availability of VMs etc
< sipa> cfields: ah, i can try avoiding that
< instagibbs> promag also made a WIP mempool delta RPC as another possible option https://github.com/bitcoin/bitcoin/pull/19476
< cfields> sipa: no idea if that's useful, need to spend a whole lot more time understanding what you're doing :)
< 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
< shuckc> seems like an opportunity to cover off a lot of the edge cases in one go
< 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
< luke-jr> instagibbs: you linked the same PR twice
< instagibbs> oh woops
< luke-jr> instagibbs: ZMQ is very unreliable..
< wumpus> instagibbs: that makes sense
< wumpus> no, ZMQ is not very unreliable
< instagibbs> sure luke-jr so alternative would likely look like promag pr i linked
< instagibbs> maybe long polling
< wumpus> only in pretty extreme circumstances it sometimes drops a packet
< sipa> ZMQ has no reliability _guarantees_
< wumpus> and in that case there needs to be a way to resynchronize, anyway, as instagibbs says
< luke-jr> wumpus: I used it for low traffic on a reliable network, and it still lost stuff regularly
< sipa> but absent overflow conditiins, it is very reliable
< wumpus> notifications have sequence numbers to be able to detect that
< sipa> luke-jr: hmm, ok
< instagibbs> and avoiding "lots" of getrawmempools I guess is hte biggest goal
< wumpus> sipa: yes, in the general case it is very reliable
< promag> the biggest problem if you have to take the client offline for a bit
< 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
< wumpus> unless your client is not consuming the notifications reliably: there can't be an infinite buffer
< wumpus> (unless you'd spool to disk or something like that)
< promag> shuckc: zmq pub doesn't hold msg if client is offline.
< wumpus> but I don't think adding yet another notifications system with mail-like reliablity is really what we want
< 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)
< wumpus> I mean there's mq systems like rabbitmq that guarantee 100% reliability
< promag> the approach in #19476 avoids periodic getrawmempools
< gribble> https://github.com/bitcoin/bitcoin/issues/19476 | wip, rpc: Add mempoolchanges by promag · Pull Request #19476 · bitcoin/bitcoin · GitHub
< 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?
< wumpus> but I mean, how many of these things do you want to integrate with
< instagibbs> promag, your rpc could be maybe generalized into block hash announcements too, connect/disconnect
< instagibbs> jnewbery, it has as seq number
< instagibbs> but right now it's a hodgepodge of endpoints
< wumpus> yes, it has a seq number
< instagibbs> and missing eviction entirely
< jnewbery> right, we're talking about two things here. Let's assume that your eviction PR is merged
< wumpus> all the zmq endpoints have seq numbers
< shuckc> if eviction PR is merged, the remaining issues are:
< wumpus> I'mnot sure these are actually useful because people keep complaining about this
< promag> wumpus: you don't know at what sequence corresponds a getrawmempool response
< wumpus> no, indeed you don't
< 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
< 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
< wumpus> zmq logging to file? taht sounds really at odd with low-latency
< jnewbery> *for that txid
< luke-jr> wumpus: mkfifo ☺
< wumpus> luke-jr: fifo is one to one, not one to many
< luke-jr> not sure why ZMQ is involved at that point lol
< luke-jr> true
< wumpus> luke-jr: if only UNIX had a one to many notification mechanism
< wumpus> except for mail
< luke-jr> dbus?
< 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
< aj> wumpus: wall(1) :)
< * luke-jr> is not sure dbus actually has this
< luke-jr> aj: lol
< wumpus> no, dbus doesn't have it either
< wumpus> dbus is one to one, it has no realible multi consumer broadcase
< luke-jr> I suppose you could just use a tmpfs file
< wumpus> it's a difficult issue in general
< wumpus> because some consumer might always be lagging
< luke-jr> and punch holes at the start as desired
< wumpus> this can potentially result in infinitely much storage needed
< luke-jr> or store to disk and let Linux's buffers handle it
< wumpus> rabbitmq is pretty good if you really need this
< 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
< 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
< instagibbs> I was joking that you could also do minisketch for set reconciliation of mempool views
< sipa> haha
< luke-jr> jnewbery: I'm not sure we want to encourage software to parse debug.log …
< instagibbs> zmq to keep difference small ;)
< 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)
< wumpus> luke-jr: exactly
< wumpus> I don't think 'parse the log' is a good option, though it serves one-to-many notification perfectly
< wumpus> mq is essentially a log
< wumpus> until your disk is full
< luke-jr> a dedicated, well-defined-format log might be okay
< wumpus> it's also a high latency option but that might not matter
< luke-jr> but something needs to do hole-punching to clean it up before disk fills
< luke-jr> wumpus: why is it high latency?
< wumpus> yes, but if you do tha, some clients might miss messages
< sipa> luke-jr: bitcoind will already shut down when disk is full
< sipa> :)
< luke-jr> depends on who does it
< wumpus> unless they tell you they read up until that far
< luke-jr> sipa: yes, but you don't want that
< wumpus> luke-jr: because disk/block devices are slow, compared to networking, latency wise
< luke-jr> wumpus: Linux at least has buffers
< wumpus> even with that
< luke-jr> :/
< luke-jr> the write/read won't even need to hit disk
< * luke-jr> wonders if you can tell Linux to never flush to disk unless it has to
< luke-jr> per-device*
< luke-jr> per-file would also be nice :P
< wumpus> yes, there's an option for that afaik, but it also means in case of power loss...
< jnewbery> shuckc: it sounded like you were going to mention more issues. Was there anything else?
< wumpus> reliable delivery of messages to multiple consumers is a difficult topic
< sipa> we need a blockchain
< wumpus> sipa: :-)
< 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
< wumpus> yes, at least the blockchain always allows going back i time... well unless pruning
< wumpus> pruning is kind of the 'what if not all consumers have seen this yet' problem
< shuckc> the sequence number on the response to getrawmempool
< shuckc> obviously has backward compatibility concerns, and other suggestions?
< sipa> shuckc: hmm?
< luke-jr> wumpus: there is an option for that? what? :o
< instagibbs> sipa, he wants to know where the mempool "snapshot" came from
< sipa> does getrawmempool report a sequence number now?
< instagibbs> no
< wumpus> it doesn't
< shuckc> no, I'm suggeting it should
< sipa> and adding one would help?
< shuckc> yes
< sipa> oh, ok
< instagibbs> so you don't know if the getrawmempool result is stale or from "future" wrt zmq reports
< sipa> what would be the reason not to?
< shuckc> because you can't tell which delta(s) have already been added
< wumpus> I guess it could prove that there were no updates in between
< sipa> adding new fields has no backward compatibility concerns
< instagibbs> sipa, I think you'd have to add *all* the zmq notification seq numbers
< sipa> ah
< instagibbs> well it's an array result ;
< instagibbs> ;)
< promag> can bnly be added if verbose=true in getrawmempool
< instagibbs> but like I said I think optional arg -> json object
< wumpus> I wonder why every zmq message has its own sequence number
< wumpus> couldn't it be just one increasing atomic?
< shuckc> unless you have one single notifier that you use for all the messages you need to sync with
< instagibbs> wumpus, it's a local member of hte notification, for whatever reason
< promag> wumpus: a client knows if he missed anything
< wumpus> promag: oh, true
< shuckc> it's only a mess if need to track lots of notifiers
< wumpus> yes, makes sense, a client is generally interested in only a subset of message kinds
< instagibbs> shuckc, shouldn't be too bad, you just grab the one you care about
< wumpus> so a global sequence number would be useless
< promag> wumpus: not really, that number should be exposed in both rpc response and zmq message
< wumpus> in any case getmempool would only need the mempool related numbers...
< promag> but I'm not fond of that..
< wumpus> it seems like some kind of layer violation
< wumpus> having RPC query ZMQ
< promag> because as a client, you need to use rpc AND zmq
< wumpus> yes
< instagibbs> otherwise you need to somehow ask for unique snapshot
< promag> hence my draft PR
< instagibbs> of the mempool
< instagibbs> but yes, it's a light violation at least
< phantomcircuit> shuckc, zmq can and will silently drop messages, unless you have sequence numbers in the application layer you cannot detect that
< 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
< wumpus> of course, that could be worked around by having both RPC and ZMQ query another source for sequence numbers
< jnewbery> how about if the mempool itself had a seq number that is incremented on every add/remove?
< wumpus> right
< instagibbs> jnewbery, that's promag's pr pretty much-ish
< wumpus> I think that would make sense
< instagibbs> 3 minutes
< wumpus> so +1 on jnewbery/promag's idea then
< 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
< wumpus> I didn't understand it like that
< promag> note that in my PR, the "stream" will be upper bounded in size, so no OOM concerns
< luke-jr> shuckc: not if he adds longpolling
< wumpus> the mempool itself would keep the seq number
< wumpus> not per consumer
< promag> shuckc: ill add long poll
< wumpus> it's kind of strange if different consumers have differnt seq ids
< shuckc> do any other commands use long polling?
< wumpus> because this is global state we're exposing
< luke-jr> GBT
< 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
< promag> shuckc: yes
< instagibbs> getblocktemplate <--- GBT
< promag> I don't like longpoll that much tbh, at least in json rpc
< promag> especially because of libevent, timeouts etcetc
< promag> but "poll and wait up to n secs" if fine imo
< wumpus> I don't think adding a different notification mechanism will really solve the 'clients could stop consuming and keep behind' problem
< luke-jr> promag: same thing? :P
< wumpus> it would mean accumulating in memory in that case?
< promag> luke-jr: n secs < timeoud (:
< promag> wumpus: yes
< 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...
< promag> but thats's fine
< luke-jr> fjahr: then you need to log the hash..
< promag> if the client doensn't pull the stream, the stream will be terminated
< 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
< instagibbs> fjahr, even more ridiculous than my minisketch idea ;P
< wumpus> promag: that's just another "lose notifications" then
< fjahr> hehe
< 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
< wumpus> reliable notification is really a non-trivial issue :)
< promag> wumpus: no, the stream will be terminated, the client starts over and gets a fresh stream
< wumpus> in any case, it's time
< aj> instagibbs: (minisketch is a great idea!)
< wumpus> #endmeeting
< lightningbot> Meeting ended Thu Jul 9 20:02:11 2020 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
< luke-jr> promag: maybe this should be part of getrawmempool?
< instagibbs> aj, just need the tool in -cli, I think :D
< shuckc> if the longpoll machinery all works, then I don't have any reason why not to use it. So long as it can be sequenced with repect to the getrawmempool calls and covers all the removals/additions cases
< promag> luke-jr: meh, let's not overload, at least for the momemnt
< wumpus> it has the same problems I think
< luke-jr> promag: well, they need to interact..
< instagibbs> shuckc, I think the idea would be you'd replace getrawmempool entirely with this new call
< wumpus> it's another method to receive notifications but doesn't avoid any of the problems, the HTTP buffer can also be full
< promag> shuckc: you won't use getrawmempool anymore
< luke-jr> wumpus: forever buffer vs limited buffer vs no buffer
< shuckc> I kind of like that. getrawmempool could give the snapshot and then if you've got longpool flag then it continues to stream modifications
< wumpus> luke-jr: zmq has a limited buffer, like anything
< luke-jr> wumpus: not in my experience
< wumpus> you can even set it with some option
< promag> wumpus: you call "mempoolchanges <stream id> <max enntries> <timeout>"
< promag> wumpus: so you don't worry about http response size etc
< luke-jr> promag: s/stream id/last update id/
< wumpus> what if it exceeds the number of entries then?
< promag> the concept is pretty much like postgres write-ahead-log and replication slots
< luke-jr> promag: and for timeout, the client can just disconnect?
< luke-jr> wumpus: then you need to get the entire mempool and start over
< promag> luke-jr: no, stream id - you can have multiple clients, each with their own stream
< luke-jr> (which is another reason to overload getrawmempool?)
< wumpus> exactly as with zmq notifications if you miss a sequence number
< luke-jr> promag: they shouldn't have separate streams
< luke-jr> wumpus: except you're less likely to miss one
< wumpus> I really don't see how a different notificatoin mechanism solves the underlying problems, but ok, if you belive so, go ahead
< wumpus> 'less likely' is no guarantee!
< promag> need to afk, please view my PR code, it's very small/simple
< luke-jr> wumpus: it doesn't need to be a guarantee to work well
< wumpus> if losing a notification is a critical issue, a smaller chance of it is better but still a critical problem
< luke-jr> it's not critical, just something you'd prefer to avoid
< instagibbs> hopefully not critical anywhere
< shuckc> I'll review it promag and see if it covers all our uses
< wumpus> how do you mean 'work well' then, not lose money for *most* users?
< luke-jr> wumpus: it's an optimisation
< luke-jr> you *could* always just getrawmempool constantly, but it'd be slow
< wumpus> you need to setup your client code to handle missing notifications in any case
< luke-jr> yes, and hopefully it won't happen often
< wumpus> you can increase the zmq buffer size for that
< instagibbs> miss message -> patch with getrawmempool-like response
< instagibbs> keep that infrequent, it's a win
< wumpus> people have been discussing this since 2012 or so since zmq notifications were added
< instagibbs> and yet, we don't have eviction notifications at all :P
< wumpus> I wish we never did fwiw
< instagibbs> understood though im sure it's tiring
< luke-jr> wumpus: admittedly I haven't used ZMQ since these problems, but it didn't work for me
< wumpus> it's only been like this every time
< wumpus> ZMQ IS UNRELIABLE
< wumpus> ITS USELESS
< wumpus> yeshh
< wumpus> like let's just remove it
< aj> could convert mempool entry's nTime to a monotonic clock, and use that as a sequence? "getrawmempool -- but only entries added since time X" maybe
< instagibbs> well, it's being used in the wild, SFYL
< instagibbs> LND uses it too
< instagibbs> (don't know why exactly, just does)
< wumpus> instagibbs: I know
< promag> wumpus: too late, sorry for that honestly X)
< wumpus> instagibbs: I know it's actually used by peple in production, but the discussions are always the same
< instagibbs> understood
< promag> #6103 :shame:
< gribble> https://github.com/bitcoin/bitcoin/issues/6103 | Add ZeroMQ notifications by promag · Pull Request #6103 · bitcoin/bitcoin · GitHub
< wumpus> sorry :)
< promag> cheers
< wumpus> I think it was like 'zmq is even used by high frequency traders and such it's probably useful for us'
< cfields> sipa: I suppose the old gcc issue is that the propagate_on_container_* aren't respected ?
< sipa> cfields: possibly
< sipa> but it looks like that
< wumpus> every time anyone mentions zmq it's: but it's unreliable!!! which is true, it's optimized for low latency, not reliability, all other mq products that historically were optimized for reliability, zmq intentionally made this trade-off
< jnewbery> In general, I don't think we should add yet another notification system to work around a supposed shortcoming in ZMQ. Then we just have an additional notification system to maintain
< wumpus> one other common reliable mq notification system nowadays is Apache Kafka
< jnewbery> if ZMQ is really unfit for purpose it should be removed and replaced
< wumpus> I agree
< instagibbs> If you waved away the getrawmempool thing, I think it's probably fine enough
< instagibbs> block notifications are very cheap to "heal", 300MB mempool is not
< cfields> sipa: from libstdc++ 4.8 release docs "23.2.1General container requirementsPartialOnly vector and forward_list meet the requirements relating to allocator use and propagation."
< sipa> cfields: ah
< cfields> sipa: if it makes you feel any better, looks like support for all standard containers didn't exist until 6.x.
< instagibbs> jnewbery, so I think you'd still need to make a layer violation(or replace getrawmempool with set reconciliation protocol) to smoothly use zmq, even with logging backing. I think that's two complementary improvements.
< instagibbs> if you're going to have to use RPC, might as well use RPC?
< wumpus> instagibbs: I guess it could be cheaper by using some binary format for the mempool; but in practice, losing messages should be rae, when I still used zmq for some notificaiton system I don't remember ever losing a message, just make sure that's a thread in the client that keeps reading
< sipa> cfields: seems i may just need to give up on the no-propagate-on-copy, and add a big disclaimer
< cfields> sipa: that's really a shame. I remember spending a few days banging my head against the wall trying to make scoped_allocator_adaptor do something interesting. I now see that it was probably just quietly broken under the hood the whole time.
< wumpus> in any case, adding a sequence number to getrawmempool to that the full dump can be reconciled with updates makes sense
< wumpus> independent of the notification system
< instagibbs> wumpus, yeah, just saying aside from mempool sync protocol I think we need to tell the consumer where raw mempool responses work
< sipa> cfields: also now i remember the reason why i made it inherit from std::allocator
< sipa> old libstdc++ doesn't implement allocator_traits, so you need all dozen trivial functions and types on it
< cfields> oh, heh, yeah, that'd be a problem.
< sipa> it's just inconvenient
< wumpus> it's much more practical to have a system that can handle (but generally doesn't need to handle) going out of sync than something that can guarantee 100% delivery, so if adding a update sequence # to the mempool solves that that'd be preferable imo
< instagibbs> ok!
< cfields> sipa: when I said "you're not supposed to inherit from std::allocator in c++11", I just meant that IIRC you're supposed to lean on allocator_traits instead. So yeah, without that, I see why you just inherited.
< cfields> sipa: but given that support is missing from the containers themselves, I doubt that change will change anything for the better :(
< cfields> sipa: also, nit, isn't std::allocator guaranteed stateless? Can't you just call its static members?
< cfields> nm, obviously not.
< bitcoin-git> [bitcoin] JeremyRubin opened pull request #19478: Kill mapLinks data structure in Mempool (master...epoch-mempool-clean-split-3) https://github.com/bitcoin/bitcoin/pull/19478
< cfields> llvm's lld (in master, not released) is complete enough as of this week to link a working bitcoind for macos from linux, without having to resort to building apple's custom ld64 :)
< cfields> jonatack: thanks for the motivation to build llvm today, I really wanted to test ^^
< cfields> No idea how well at works, but "./bitcoind --help" works, at least.
< promag> cfields: nice, builds qt too?
< cfields> promag: heh, no clue. I didn't want to end up deep in qt link issues...
< cfields> I should though. This work is active on lld right now. Good time for us to be reporting issues/patches.
< cfields> promag: yeah, it doesn't know how to find .tbd frameworks yet, so no qt.
< cfields> I'll check to see if that _should_ be working yet and report it upstream if so.
< phantomcircuit> wumpus, you can just go to the zmq docs and see that it's not reliable, it's all about "implementing patterns" to achieve some kind of reliability
< phantomcircuit> i've used it before but only with the knowledge that it's basically udp