< bitcoin-git> [bitcoin] achow101 opened pull request #18971: wallet: Refactor the classes in wallet/db.{cpp/h} (master...refactor-storage) https://github.com/bitcoin/bitcoin/pull/18971
< bitcoin-git> [bitcoin] luke-jr opened pull request #18972: WIP: net: Add blockfilters white{bind,list} permission flag (master...neutrino_whitelist) https://github.com/bitcoin/bitcoin/pull/18972
< bitcoin-git> [bitcoin] fanquake opened pull request #18973: [0.20] Final backports for rc2 (0.20...0_20_0rc2_final_backports) https://github.com/bitcoin/bitcoin/pull/18973
< bitcoin-git> [bitcoin] elichai closed pull request #17953: refactor: Abstract boost::variant out (master...2020-01-variant) https://github.com/bitcoin/bitcoin/pull/17953
< goatpig> hi
< goatpig> would a transaction with no outputs be valid?
< goatpig> thanks
< theStack> has anyone ever used the functional test framework to interact with an already running full node? for some (performance) tests it would make sense to use the mainnet i think
< fjahr> theStack: have you seen the TestShell class from James Chiang? It is used in the jupyter notebooks for the optech Taproot course, that might be helpful for you.
< theStack> fjahr: thanks for the hint, will take a look at it!
< fjahr> I am not sure about the already running part though. Should probably work if you just change the pid? But I have not tried it.
< bitcoin-git> [bitcoin] MarcoFalke opened pull request #18974: test: Check that invalid witness destinations can not be imported (master...2005-testInvalidWitnessDestination) https://github.com/bitcoin/bitcoin/pull/18974
< fanquake> promag: can you check the coin control backports in #18973
< gribble> https://github.com/bitcoin/bitcoin/issues/18973 | [0.20] Final backports for rc2 by fanquake · Pull Request #18973 · bitcoin/bitcoin · GitHub
< bitcoin-git> [bitcoin] fanquake pushed 5 commits to master: https://github.com/bitcoin/bitcoin/compare/04c09553d898...b9c504cbc4ba
< bitcoin-git> bitcoin/master fab6d06 MarcoFalke: test: Add unregister_validation_interface_race test
< bitcoin-git> bitcoin/master fa770ce MarcoFalke: validationinterface: Rework documentation, Rename pwalletIn to callbacks
< bitcoin-git> bitcoin/master fa5ceb2 MarcoFalke: test: Remove UninterruptibleSleep from test and replace it by SyncWithVali...
< bitcoin-git> [bitcoin] fanquake merged pull request #18742: miner: Avoid stack-use-after-return in validationinterface (master...2004-minerNoCrash) https://github.com/bitcoin/bitcoin/pull/18742
< theStack> fjahr: the interesting part for me would be that it doesn't start a new chain from scratch; if i need 100000s of blocks for performance tests starting a new regtest chain is not an option
< theStack> creating a TestNode() instance with right parameters could maybe work
< jnewbery> theStack: I made some modifications to test_shell to connect to a running full node: https://github.com/jnewbery/bitcoin/tree/node-shell YMMV
< jnewbery> were you planning to do some perf testing for 18960?
< fanquake> MarcoFalke: is there anything obvious to backport into the 0.20 branch to fix the test failures in 18973. Looks like it's mostly in the valgrind builds
< theStack> jnewbery: sounds like exactly what i need, will take a look. thanks!
< jnewbery> fanquake: #18899 removes the valgrind build from travis because it was timing out jobs
< gribble> https://github.com/bitcoin/bitcoin/issues/18899 | travis: Remove valgrind by MarcoFalke · Pull Request #18899 · bitcoin/bitcoin · GitHub
< theStack> jnewbery: yes indeed i was planning to repeatedly fetch the bip 157 checkpoint headers, comparing how long it takes in master and 18960 branches
< theStack> (maybe also compare it with the original proposed solutions by jimpo)
< fanquake> jnewbery: guess that might be worth backporting as well then
< jnewbery> theStack: great. Thanks! The first time you send a getcfcheckpt, it's reading the values from disk, so it should be slower than the second and subsequent requests
< jnewbery> fanquake: as long as it's not hiding and actual bugs that need to be backported. https://travis-ci.org/github/bitcoin/bitcoin/builds/686857580 do actually look like actual test failures rather than timeouts.
< fanquake> jnewbery: if you've got test backport suggestions I'm all ears. I see some process_message related in the fuzzer, which I assume we've fixed.
< fanquake> I'm going to untag #18287 for 0.20.0. Only myself and sipsorcery have tested it, and neither of us could verify that the change is fixing the actual problem. The patch does fix libevents getaddrinfo detection, but Core still doesn't behave as expected; so I think this needs further testing, and shouldn't be bundled in last minute.
< gribble> https://github.com/bitcoin/bitcoin/issues/18287 | depends: Patch libevent build to fix IPv6 -rpcbind on Windows by luke-jr · Pull Request #18287 · bitcoin/bitcoin · GitHub
< theStack> jnewbery: do you think that is also true for the version without the cache implementation (i.e. master branch)? i don't know if BDB does caching internally (or maybe even the OS?), but there must be something as some people have expressed their doubt if the cache implementation is really improving anything
< theStack> but anyways, i will hopefully find out soon through the benchmarks :)
< jnewbery> theStack: perhaps. It's worth testing.
< jnewbery> (the block filter index uses leveldb, not bdb. bdb is only used in the wallet)
< jnewbery> fanquake: looks like you'll need #18757 to avoid those false fuzzing failures
< gribble> https://github.com/bitcoin/bitcoin/issues/18757 | test: Remove enumeration of expected deserialization exceptions in ProcessMessage(...) fuzzer by practicalswift · Pull Request #18757 · bitcoin/bitcoin · GitHub
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #18968: doc: noban precludes maxuploadtarget disconnects (master...2005-docMaxuploadtarget) https://github.com/bitcoin/bitcoin/pull/18968
< bitcoin-git> [bitcoin] MarcoFalke reopened pull request #18968: doc: noban precludes maxuploadtarget disconnects (master...2005-docMaxuploadtarget) https://github.com/bitcoin/bitcoin/pull/18968
< vasild> ResolveSubNet("::/0").Match(ResolveIP("")) -- is true
< vasild> ResolveSubNet("::AAAA:0:0/80").Match(ResolveIP("")) -- is true
< vasild> but
< vasild> ResolveSubNet("::FFFF:0:0/96").Match(ResolveIP("")) -- is false
< vasild> is this a bug?
< vasild> I think it is strange to match IPv4 addresses as belonging to some IPv6 network, so IMO all should be false. But if I accept that we should, for some reason, match IPv4 addresses as belonging to some IPv6 networks, then also the last one should be true.
< vasild> It is not true because "::FFFF:0:0" is interpreted as an IPv4 address ( and the netmask /96 bricks it. It is the same as "".
< wumpus> it's somewhat strange, but it's a by-effect of IPv4 addresses have always been encoded as a kind of IPv6 address in bitcoin
< wumpus> not sure this should affect anything in practice
< vasild> What about stopping to accept IPv4 as IPv6 addresses? That is - all of the above return false?
< vasild> Sorry, "stopping to accept IPv4 as belonging to some IPv6 networks"
< vasild> I am modifying this code wrt ADDRv2 and the new code would return false for all of the above 3 tests. So I wonder if this change in behavior would be ok. In order to mimic the current (strange) behavor a few twists and tweaks would have to be made to the new code, making it less straight forward and a bit more difficult to review.
< vasild> I will go for "no change in behavior" (+some twists in the patch). If it turns out that we don't need to accept IPv4 as part of IPv6 networks (e.g. `::/0`), then the patch can be simplified.
< wumpus> I think that's fine, I don't think it'll affect anything in practice
< wumpus> there's no reason to do mixed IPv6 IPv4 subnet matching
< wumpus> so having it always return false if someone tries seems ok to me
< vasild> ah, excellent!
< Pavlenex> Hello. I've asked before but got no reply. Serbian translation for 0.20 is ready, it was translated from scratch basically as the language never got much translators. However the language does not have a coordinator or reviewer so it can't be pushed as complete. Who is in charge of Transifex that can provide me with an access so we can proceed? Thanks.
< wumpus> Pavlenex: I should be able to set someone as coordinator
< Pavlenex> wumpus: Hey! https://www.transifex.com/user/profile/nedved/ is my transifex username. However if you want to set somebody else from that team instead, it's fine as long as we can proceed with a review.
< Pavlenex> If I can suggest alternative member that would be casperBGD, he continued on my initial efforts and completed the translations. Thanks in advance.
< bitcoin-git> [bitcoin] laanwj pushed 1 commit to 0.20: https://github.com/bitcoin/bitcoin/compare/5747c4ca1bf8...7d87ba0e0227
< bitcoin-git> bitcoin/0.20 7d87ba0 MarcoFalke: travis: Remove valgrind
< bitcoin-git> [bitcoin] laanwj pushed 1 commit to 0.20: https://github.com/bitcoin/bitcoin/compare/7d87ba0e0227...aa7c6858e6e4
< bitcoin-git> bitcoin/0.20 aa7c685 MarcoFalke: travis: Remove s390x
< wumpus> Pavlenex: okay! let me see
< wumpus> Pavlenex: I added you as coordinator for all the Serbian languages, I think you should be able to add other reviewers now
< Pavlenex> wumpus: Yup, just got the notification. Thanks. I'll try to begin the review process asap.
< wumpus> thank you!
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #18973: [0.20] Final backports for rc2 (0.20...0_20_0rc2_final_backports) https://github.com/bitcoin/bitcoin/pull/18973
< bitcoin-git> [bitcoin] MarcoFalke reopened pull request #18973: [0.20] Final backports for rc2 (0.20...0_20_0rc2_final_backports) https://github.com/bitcoin/bitcoin/pull/18973
< bitcoin-git> [bitcoin] laanwj opened pull request #18975: test: Remove const to work around compiler error on xenial (master...2020_05_xenial_compile_issue) https://github.com/bitcoin/bitcoin/pull/18975
< bitcoin-git> [bitcoin] MarcoFalke opened pull request #18976: test: Add {} to force default initialization (master...2005-testDefaultInit) https://github.com/bitcoin/bitcoin/pull/18976
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #18976: test: Add {} to force default initialization (master...2005-testDefaultInit) https://github.com/bitcoin/bitcoin/pull/18976
< theStack> jnewbery: i posted the results of my getcfcheckpt cache (#18960) research in the PR thread, with a link to the script; there doesn't seem to be any difference, any request up to block 630000 takes 50ms on my machine
< gribble> https://github.com/bitcoin/bitcoin/issues/18960 | [indexes] Add compact block filter headers cache by jnewbery · Pull Request #18960 · bitcoin/bitcoin · GitHub
< MarcoFalke> hi, I won't make it to today's meeting, but I'd like to put something up for high prio
< MarcoFalke> #proposedmeetingtopic Add #18638 to high prio (MarcoFalke, not actually a topic)
< gribble> https://github.com/bitcoin/bitcoin/issues/18638 | net: Use mockable time for ping/pong, add tests by MarcoFalke · Pull Request #18638 · bitcoin/bitcoin · GitHub
< bitcoin-git> [bitcoin] laanwj pushed 3 commits to master: https://github.com/bitcoin/bitcoin/compare/b9c504cbc4ba...4dd2e5255a7f
< bitcoin-git> bitcoin/master fa182a8 MarcoFalke: rpcwallet: Replace boost::optional<T>::emplace with simple assignment of T...
< bitcoin-git> bitcoin/master fa1f840 MarcoFalke: rpcwallet: Replace pwallet-> with wallet.
< bitcoin-git> bitcoin/master 4dd2e52 Wladimir J. van der Laan: Merge #18946: rpcwallet: Replace boost::optional<T>::emplace with simple a...
< bitcoin-git> [bitcoin] laanwj merged pull request #18946: rpcwallet: Replace boost::optional<T>::emplace with simple assignment of T{} (master...2005-rpcWalletOptional) https://github.com/bitcoin/bitcoin/pull/18946
< bitcoin-git> [bitcoin] laanwj pushed 5 commits to master: https://github.com/bitcoin/bitcoin/compare/4dd2e5255a7f...2d7489be8f77
< bitcoin-git> bitcoin/master 061acf6 fanquake: scripts: no-longer check for 32 bit windows in security-check.py
< bitcoin-git> bitcoin/master 13f606b fanquake: scripts: remove NONFATAL from security-check.py
< bitcoin-git> bitcoin/master 83d063e fanquake: scripts: add run_command to security-check.py
< bitcoin-git> [bitcoin] laanwj merged pull request #18796: scripts: security-check.py refactors (master...security_check_no_more_32bit) https://github.com/bitcoin/bitcoin/pull/18796
< bitcoin-git> [bitcoin] laanwj pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/2d7489be8f77...553bb3fc3d95
< bitcoin-git> bitcoin/master 7467366 John Newbery: [net processing] Only send a getheaders for one block in an INV
< bitcoin-git> bitcoin/master 553bb3f Wladimir J. van der Laan: Merge #18962: net processing: Only send a getheaders for one block in an I...
< bitcoin-git> [bitcoin] laanwj merged pull request #18962: net processing: Only send a getheaders for one block in an INV (master...2020-05-limit-block-inv) https://github.com/bitcoin/bitcoin/pull/18962
< wumpus> #startmeeting
< lightningbot> Meeting started Thu May 14 19:00:31 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.
< kanzure> hi
< sipsorcery> hi
< elichai2> 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
< jonasschnelli> Hi
< jnewbery> hi
< hebasto> hi
< fjahr> hi
< jkczyz> hi
< meshcollider> hi
< aj> hi
< jeremyrubin> hola
< wumpus> no proposed meeting topics in http://gnusha.org/bitcoin-core-dev/proposedmeetingtopics.txt (only a suggestion for high prio for review)
< achow101> hi
< jeremyrubin> w.r.t. hi prio, i'd like to get the mempool project back on track it's review blocked
< sipa> i have a topic: ok to update to boost 1.59? and if so, when?
< wumpus> any any last minute proposals?
< wumpus> sipa: sgtm (though I'm not sure everyone is here wrt that discussion)
< wumpus> #topic High priority for review
< wumpus> Add #18638 to high prio (MarcoFalke, not actually a topic)
< gribble> https://github.com/bitcoin/bitcoin/issues/18638 | net: Use mockable time for ping/pong, add tests by MarcoFalke · Pull Request #18638 · bitcoin/bitcoin · GitHub
< jnewbery> Could we add #18960 as a blocker?
< gribble> https://github.com/bitcoin/bitcoin/issues/18960 | [indexes] Add compact block filter headers cache by jnewbery · Pull Request #18960 · bitcoin/bitcoin · GitHub
< wumpus> sure
< wumpus> added
< jnewbery> thanks!
< wumpus> https://github.com/bitcoin/bitcoin/projects/8 6 blockers, 1 bugfix, 4 chasing concept ACK now
< wumpus> jeremyrubin: do you have any specific suggestions wrt prs?
< wumpus> ah thanks
< wumpus> added
< wumpus> #topic required boost to 1.59 (sipa)
< sipa> hi
< wumpus> see also #8875
< sipa> i'm considering various approaches to improving to some of the p2p tx download logic
< gribble> https://github.com/bitcoin/bitcoin/issues/8875 | Bump minimum required Boost version · Issue #8875 · bitcoin/bitcoin · GitHub
< sipa> and one of them would rely on boost multi_index's ranked indexes
< sipa> which were added in boost 1.59
< sipa> so i was wondering if that's a possibility for 0.21
< sipa> or if i should consider other options (which i may do anyway)
< wumpus> that seems a fair enough reason
< * luke-jr> grumbles about RHEL/CentOS packages being hard to search
< wumpus> I think the most important thing to check is the versions in currently supported linux distros for building
< wumpus> "Version 1.59.0 August 13th, 2015 15:23 GMT" that sounds old enough, but knowing what old crap some distros ship with, it doesn't say everything
< sipa> well if we're going to be building with c++17, it seems we'd be relying on compilers newer than that anyway
< wumpus> yes
< jeremyrubin> sipa: ranked indexes look pretty cool
< sipa> they're very cool
< luke-jr> sounds like RHEL/CentOS 8 has boost 1.66
< wumpus> sipa: the dumbest way to find out would be to do a PR that uses that feature and see if it passes travis :)
< wumpus> luke-jr: oh! that's surprisingly new
< luke-jr> Debian has 1.67
< sipa> ok, will try opening a dummy PR and see what happens
< sipa> good enough for me
< jeremyrubin> does it make sense to do a minimum bump for feature or to bump to the highest minimum target we support?
< luke-jr> wumpus: not 100% sure
< wumpus> well sipa has a good reason now
< jeremyrubin> sipa: maybe check there's no bug-fixes to that new functionality in newer versions?
< wumpus> the reason #16381 was closed is that it wasn't important/urgent at the time
< gribble> https://github.com/bitcoin/bitcoin/issues/16381 | Set minimum required Boost to 1.53.0 by hebasto · Pull Request #16381 · bitcoin/bitcoin · GitHub
< sipa> 1.64 has a bug fix related to ranked indices
< jeremyrubin> sipa: how severe?
< sipa> but the bug is just something that doesn't compile, which should
< sipa> so not severe at all
< wumpus> "it's eight years old" isn't enough to bump a minimum version, "I need this new data structure" well might be
< wumpus> we already use boost 1.70 in depends so that's definitely new enough
< sipa> oh!
< sipa> ok
< wumpus> so yeah the current version used for gitian builds doesn't need to be bumped, only the minimum version
< sipa> that's enough for my topic
< jeremyrubin> sipa: are you adding this to mempool or to a diff data structure?
< wumpus> (supported at all)
< luke-jr> Ubuntu LTS is at 1.71
< luke-jr> Arch has 1.72
< jeremyrubin> Just worried that if it goes to mempool multiindex it looks like there's a non-trivial perfromance overhead on all erases
< luke-jr> Gentoo stablre is 1.72
< wumpus> if luke-jr is right it's complely uncontroversial to bump the minimum to 1.59
< jeremyrubin> but this is less build systemy and more about how you're using it
< wumpus> (it likely depends on CenOS which is alwasys slowest)
< luke-jr> SuSE is 1.66
< sipa> jeremyrubin: no, this is unrelated to mempool
< luke-jr> any other popular distros these days?
< sipa> it's in net processing
< jeremyrubin> cool looking forward to seeing it.
< jeremyrubin> use might actually have some speedups for mempool come to think of it for the eviction logic & mining logic. will noodle on that.
< wumpus> any other topics for this week?
< wumpus> PSA: we're wrapping up 0.20.0rc2 at the moment
< sipa> they add 1 pointer memory usage per entry, fwiw (compared to ordered_index)
< luke-jr> wumpus: do you want me to try to reduce the size of my 0.20 release tarball fixes PR?
< wumpus> #18973
< gribble> https://github.com/bitcoin/bitcoin/issues/18973 | [0.20] Final backports for rc2 by fanquake · Pull Request #18973 · bitcoin/bitcoin · GitHub
< wumpus> (finally)
< jeremyrubin> sipa: it's the log n erase that's more worrisome
< wumpus> luke-jr: sure, but we're not going to hold up the rc on it
< luke-jr> wumpus: it's a security violation for users who build from source
< luke-jr> to answer how relevant it is
< wumpus> how so?
< luke-jr> and can result in leaking user private info to the network
< luke-jr> wumpus: it's looking at every parent directory to the source code for a git repo
< wumpus> I mean, it's been absurd how long it's taking to roll this release
< luke-jr> and taking whatever HEAD hash it finds into the version number
< luke-jr> pulling a hash out of an unrelated git repo the user might have, and publishing it, isn't very nice
< wumpus> I agree
< sipa> that sounds like something we should fix
< luke-jr> (Gentoo will kill the build instead)
< wumpus> but honestly we should cut a release at some point, there's aalways some edge case
< luke-jr> this is a regression, too
< wumpus> also #18902 rewrites a large part of the gitian descriptor and changes 6 files, can't this be fixed with some small patch?
< gribble> https://github.com/bitcoin/bitcoin/issues/18902 | Bugfix: Only use git for build info if the repository is actually the right one by luke-jr · Pull Request #18902 · bitcoin/bitcoin · GitHub
< luke-jr> probably
< luke-jr> I was trying to avoid conflicts with the autogen fix, but I guess we're missing that?
< wumpus> in any case: I agree it should be fixed, but I don't think it's terribly urgent
< hebasto> as a workaround a user could use BITCOIN_GENBUILD_NO_GIT variable
< luke-jr> maybe a release notes mention of the known issues
< wumpus> yes
< wumpus> I think that's fine
< luke-jr> current draft is in wiki still? or git?
< luke-jr> k, I'll see about writing something up for that
< wumpus> thanks!
< luke-jr> should I reduce the footprint of #18902 and/or #18909 also?
< gribble> https://github.com/bitcoin/bitcoin/issues/18902 | Bugfix: Only use git for build info if the repository is actually the right one by luke-jr · Pull Request #18902 · bitcoin/bitcoin · GitHub
< gribble> https://github.com/bitcoin/bitcoin/issues/18909 | [0.20] Fix release tarball by luke-jr · Pull Request #18909 · bitcoin/bitcoin · GitHub
< luke-jr> would be nice to just get 18902 merged to master as-is to fix all the issues at once :x but maybe reducing 18909 makes more sense
< wumpus> fwiw the git_check_in_repo() check looks straightforward enough
< wumpus> why isn't that enough?
< wumpus> (or alternatively, check if a .git is in the top level of the source directory)
< wumpus> besides that change in genbuild.sh, are any other changes needed to fix this?
< hebasto> it is all about "version hack"
< hebasto> in gitian builds
< luke-jr> wumpus: we'd need to restore the "version hack"
< wumpus> luke-jr talks about a security vulnerability importing data from a git repository above the bitcoin one, this can be avoided by checking in genbuild that we're really in a git repo right?
< luke-jr> the genbuild changes in 18902 are a proper fix for that
< wumpus> (I mean, in the right git repo)
< wumpus> yes my question is why isn't that the only change needed?
< wumpus> hebasto: so we can fix this by reverting a change too?
< hebasto> #18349 probably has simpler approach
< luke-jr> those are the only commits specific to 18902 - the other 3 are inherited from 18818
< gribble> https://github.com/bitcoin/bitcoin/issues/18349 | build: Fix quick hack for version string in releases by hebasto · Pull Request #18349 · bitcoin/bitcoin · GitHub
< wumpus> (at least in 0.20)
< hebasto> wumpus: not sure
< luke-jr> reverting a change would be the first commit of 18902 + restoring the "version hack"
< wumpus> ok
< luke-jr> (to literally revert, would be undoing all of the git-archive stuff)
< wumpus> so it's much more complex than I thought
< luke-jr> wumpus: it can be simplified with a few hours I suspect
< wumpus> let's just add a note to the release notes then
< wumpus> any other topics?
< wumpus> #endmeeting
< lightningbot> Meeting ended Thu May 14 19:44:05 2020 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
< wumpus> it really looks like the only build configuration used and supported in practice is building from git, everything else is fragile at most
< wumpus> FWIW for releases *and RCs all the version information in in configure.ac
< wumpus> I'm not sure what it gets from git at all
< sipa> random brainstorming (and highly unsure it's worth spending time on): what if we create a file that contains a list of (sha512 tree hash, version string) pairs, and at build time we compare the tree with that file, and use its name if found, tree hash otherwise
< sipa> that would disentangle version naming entirely from git
< wumpus> that's pretty much what git tags are
< wumpus> I mean all the version information is in configure.ac. My point is I don't see why any external information is needed.
< wumpus> neither from git tags nor any external file
< sipa> fair
< wumpus> this used to be differnt, to be clear, at some point the build process derives the rcX from the git tag
< wumpus> but we've already made everything explicit now
< sipa> oh, ok
< wumpus> derived*
< sipa> i was not aware of that
< wumpus> no worries, I don't have a full grasp in the version logic actually used either :)
< luke-jr> wumpus: but then we can't build non-releases with correct version info?
< wumpus> luke-jr: what is 'correct version info' in that case anyway?
< luke-jr> [19:50:46] <wumpus> it really looks like the only build configuration used and supported in practice is building from git, everything else is fragile at most <-- I suppose we could just not publish the tarball
< luke-jr> wumpus: a commit hash abbreviation seems fine
< wumpus> well the tarballs are fine, they are for releases and thus contain correct version info
< luke-jr> something like git-describe
< wumpus> they don't need *any* reference to git
< luke-jr> wumpus: currently, they don't..
< wumpus> luke-jr: they contain configure.ac, which has all the version info
< wumpus> including rc level
< wumpus> I know this because I have to set this every time before a rc
< luke-jr> hmm
< bitcoin-git> [bitcoin] sipa opened pull request #18977: Test ranked_index (master...202005_try_ranked) https://github.com/bitcoin/bitcoin/pull/18977
< sipa> it seems xenial only have boost 1.58
< jeremyrubin> so.... not till 2021 then?
< jeremyrubin> (Xenial is 16.04 LTS, which goes out of mantenance and to security only in 2021
< jeremyrubin> Personally I think it's probably OK given that it's 4 LTS's old, but it is still in maintenance so conceivable someone is relying on it still...
< jeremyrubin> err 3 LTS's old, not 4 (or 2 depending on how you count the current one)
< MarcoFalke> copy the hpp to our repo? I wanted to remove boost for 0.22.0
< sipa> remove boost? how are you going to do that?
< jeremyrubin> MarcoFalke: IDK if it's feasible givent that multiindex uses like all of boost internally.
< MarcoFalke> Switch to c++17 ;)
< MarcoFalke> If we need a single boost file we could subtree that single file?
< sipa> we're not going to start maintaining our own copy of boost to get around depending on boost
< sipa> it's a zillion .hpp files
< sipa> across various boost packages
< jeremyrubin> MarcoFalke: I don't think that's feasible, you can try bcp'ing multiindex out and it's big
< MarcoFalke> Ok, didn't know how big it is
< sipa> $ find /usr/include/boost/multi_index* -type f | sort | uniq | wc -l
< sipa> we're not going to start maintaining our own copy of boost to get around depending on boost
< sipa> heh
< sipa> $ find /usr/include/boost/multi_index* -type f | sort | uniq | wc -l
< sipa> 78
< MarcoFalke> ouch
< jeremyrubin> Oh 78? I thought surely it was more than 80, so let's do it!
< sipa> which depends on boost mpl, boost move, a bunch of shared boost utility stuff, ...
< MarcoFalke> fanquake: Looks like you can do your bump ^
< jeremyrubin> MarcoFalke: OTOH rewriting multiindex for our specific use cases seems doable.
< sipa> boost mpl is 1045 files...
< fanquake> 👀
< MarcoFalke> jeremyrubin: Can you write one in less than 100 LOC (clang-formatted)?
< sipa> lol
< sipa> i do not want to write and maintain a copy of multi_index
< jeremyrubin> You can't even define a concrete type for the one we have in less than 100 LOC
< sipa> wahaha
< jeremyrubin> sipa: I think it wouldn't be writing a custom one, but writing a non template concrete impl for the one that we currently use.
< sipa> jeremyrubin: i spent 2 weeks trying to design a custom data structure for the thing i want :)
< sipa> and gave up
< jeremyrubin> Yeah. I'm just prodding MarcoFalke if he wants to ever get rid of boost
< sipa> i mean, it obviously could be done
< sipa> but it'd be far harder to review, and a properly abstracted version would like tend to have some minimal multi_index like reimplementation anyway
< luke-jr> sipa: jeremyrubin: since when do we support anything more than the latest LTS?
< sipa> MarcoFalke: i think as an intermediate goal maybe it's worth trying to only have header-only boost dependencies
< luke-jr> MarcoFalke: copying the hpp is strictly worse than depending on boost
< luke-jr> removing boost doesn't seem like a goal
< luke-jr> just using C++11 (or C++17 eventually) instead
< sipa> i think replacing boost with standard replacements when they're appropriate is great
< sipa> removing boost at all costs seems counter-productive
< luke-jr> exactly
< luke-jr> and we want to add boost::process soon for several things
< luke-jr> which AFAIK there is no C++ std alternative to
< sipa> what for?
< sipa> (just curous)
< jeremyrubin> smooth brain: replacing boost multiindex; galaxy brain: getting multiindex into c++next std
< luke-jr> sipa: HWI, Tor, IIRC there were a few others
< sipa> jeremyrubin: great, let's switch to C++29
< MarcoFalke> ACK
< jeremyrubin> TBH capnproto sounds like a better step to do than boost::process
< jeremyrubin> Should be able to handle similar use cases?
< sipa> ?
< sipa> i don't see how they're related
< luke-jr> jeremyrubin: totally different things..
< jeremyrubin> Isn't boost::process just some inter-process glue stuff? IIRC capnproto allows you to setup similar constructs but I could be off.
< luke-jr> boost::process lets us manage running other processes
< sipa> jeremyrubin: that's like saying JSON can replace TCP-IP
< jeremyrubin> gotcha, I just glanced at boost::process and saw that it was talking about IPC stuff
< sipa> i'm not convinced we need boost::process btw, but i haven't thought hard about it
< MarcoFalke> If we require boost::process, we also need to ditch bionic
< fanquake> Just require Debian sid from here on out
< MarcoFalke> At least the switch to C++20 will be included for free /s
< luke-jr> latest stable Ubuntu is focal
< luke-jr> anything older is not guaranteed supported
< luke-jr> also, boost::process can/will probably be optional at first?
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #18973: [0.20] Final backports for rc2 (0.20...0_20_0rc2_final_backports) https://github.com/bitcoin/bitcoin/pull/18973
< bitcoin-git> [bitcoin] MarcoFalke reopened pull request #18973: [0.20] Final backports for rc2 (0.20...0_20_0rc2_final_backports) https://github.com/bitcoin/bitcoin/pull/18973
< MarcoFalke> And in reply to "removing boost should not be a goal". I think I disagree. Most of boost seems unmaintained at this point. For example the bug that crashed Bitcoin Core was reported to upstream with steps to reproduce and even a fix. I am still waiting for a reply...
< sipa> which bug?
< sipa> ha
< sipa> i think the quality differs a lot between boost libraries
< sipa> and for every goal, we should find the right solution
< sipa> whether that's boost or something else
< sipa> like... boost filesystem and boost shared_ptr effectively turned into c++ stl
< jeremyrubin> Well I think to MarcoFalke's point, if something like multiindex is thousands of files/78 modules
< sipa> yes, and it's great
< sipa> i know nothing that compares to it
< MarcoFalke> Just saying we should be cautious about pulling in dependencies
< sipa> MarcoFalke: absolutely
< jeremyrubin> I think MarcoFalke point is more around security than on features
< sipa> but "it's boost" on itself is not a point against a particular solution
< sipa> "it's unmaintained" is
< sipa> or "it's a logistical pain for our build system" is
< fanquake> MarcoFalke: did you save the log that you wanted. I'm about to push more commits
< MarcoFalke> fanquake: I reset the build, which archives the log. Go ahead!
< MarcoFalke> Let's pray the issue is just a race in the test
< MarcoFalke> At least it is not a regression
< MarcoFalke> sipa: I guess your feature wouldn't make sense if it was enabled conditionally on boost multi_index being available?
< MarcoFalke> So xenial users can still compile, but wouldn't get your feature
< sipa> MarcoFalke: it can be worked around with O(n) complexity instead of O(log n)
< sipa> which is probably not terrible in practice, but harder to guarantee no remotely-triggerable extreme conditions that become very slow
< jeremyrubin> sipa: Out of curiosity for the design you're making, wouldn't you have to take an action for everything in that rank group anyways?
< jeremyrubin> e.g., isn't this fundamentally O(N)?
< jeremyrubin> *ie
< sipa> jeremyrubin: no