19:00:42 <wumpus> #startmeeting 
19:00:42 <core-meetingbot> Meeting started Thu Nov 19 19:00:41 2020 UTC.  The chair is wumpus. Information about MeetBot at https://bitcoin.jonasschnelli.ch/ircmeetings.
19:00:42 <core-meetingbot> Available commands: action commands idea info link nick
19:00:44 <kanzure> hi
19:01:03 <jonasschnelli> hi
19:01:05 <wumpus> #bitcoin -core-dev Meeting: achow101 aj amiti ariard bluematt cfields Chris_Stewart_5 digi_james dongcarl elichai2 emilengler fanquake fjahr gleb gmaxwell gwillen hebasto instagibbs jamesob jb55 jeremyrubin jl2012 jnewbery jonasschnelli jonatack jtimon kallewoof kanzure kvaciral lightlike luke-jr maaku marcofalke meshcollider michagogo moneyball morcos nehan NicolasDorier paveljanik
19:01:07 <wumpus> petertodd phantomcircuit promag provoostenator ryanofsky sdaftuar sipa vasild wumpus
19:01:12 <jonatack> hi
19:01:27 <luke-jr> hi
19:01:35 <achow101> hi
19:01:36 <sipa> hi
19:01:44 <wumpus> there is one proposed meeting topic for this week: 0.20.2 (MarcoFalke)
19:01:49 <miketwenty1> hi
19:02:48 <murch> hi
19:02:57 <wumpus> I wanted to tag rc1 today but apparently some things have been tagge 0.21 since: https://github.com/bitcoin/bitcoin/pulls?q=is%3Aopen+is%3Apr+milestone%3A0.21.0
19:03:26 <wumpus> I guess if these are really urgent, please help reviewing them
19:03:43 <MarcoFalke> wumpus: Don't worry abou those. They are minor RPC inconsistencies
19:03:53 <hebasto> the tagged for backporting, no?
19:03:55 <wumpus> if not, let's move them to the 0.21.1 milestone instead
19:04:09 <luke-jr> wumpus: feature_segwit test is failing too
19:04:13 <MarcoFalke> I am sure we will find more issus for rc2
19:04:19 <sipa> inconsistencies in newly introduced stuff?
19:04:27 <jnewbery> hi
19:04:31 <MarcoFalke> sipa: The feerate things ...
19:04:41 <luke-jr> sipa: what we discussed the other night
19:04:42 <sipa> sounds like it - if so i think we'll want those in 0.21 anyway i guess
19:04:46 <murch> Sending feerates were restricted to >0 instead of >=0
19:05:00 <murch> mea culpa, I missed it in the review
19:05:01 <MarcoFalke> Also, "" == null 0==null
19:05:13 <MarcoFalke> and the third one a minor issue with upgradewallet
19:05:24 <sipa> no objection to doing rc1 without them in that case, but if it means there'll be an rc2 anyway.... up to wumpus i guess
19:05:31 <wumpus> anyhow right now they're holding up the release
19:05:44 <luke-jr> more of a beta1 than a rc1 in that case :P
19:05:50 <MarcoFalke> I'd be highly surprised if this was the first major release with just one rc
19:06:03 <sipa> ha, fair
19:06:08 <MarcoFalke> we'll find more bugs, don't worry
19:06:13 <wumpus> anyhow merging the feerate last minute was probaly ill advised so I understand if last minute issues came up
19:06:34 <sipa> luke-jr: i wonder why you're the only one hitting this
19:06:35 <MarcoFalke> wumpus: It had to be merged (or everything reverted)
19:06:45 <sipa> luke-jr: is signing or hashing particularly slow on your system?
19:06:47 <luke-jr> wumpus: well, it was really a bugfix for the explicit fee rate mess merged months ago
19:06:49 <wumpus> MarcoFalke: yes, but it should have been merged sooner in the release cycle
19:06:53 <luke-jr> sipa: I saw it on CI too?
19:06:57 <sipa> oh, ok
19:06:57 <MarcoFalke> wumpus: Agree
19:07:00 <jonatack> I think they've been addressed and those PRs are close to RFM. There is a lot of test coverage in place now, which helps.
19:07:04 <MarcoFalke> luke-jr: feature_taproot?
19:07:05 <luke-jr> sipa: no reason it would be - I have 16 SMT4 cores
19:07:26 <sipa> MarcoFalke: there is a simple fix; aj posted one the other day, i think an even simpler one is possible
19:07:29 <sipa> i'll PR soon
19:07:32 <MarcoFalke> wumpus: This release is the heavies ever in features merged per time, I think
19:07:44 <MarcoFalke> sipa: Thx, I'll ack
19:07:59 <luke-jr> MarcoFalke: yes, sorry
19:08:01 <MarcoFalke> so the delay was expected
19:08:05 <wumpus> MarcoFalke: yes, seems like it :)
19:08:13 <luke-jr> MarcoFalke: looks like it simply builds a huge wallet and then times out in coin selection
19:08:32 <sipa> yeah, or quadratic hashing of legacy spends
19:08:36 <MarcoFalke> luke-jr: No coin selection. It is signrawtxwithwallet
19:08:58 <MarcoFalke> (or a bug in the wallet)
19:09:16 <luke-jr> I thoguht it was the create
19:09:19 <wumpus> it fails on 0.21 branch but not on master?
19:09:21 <aj> sipa, luke-jr: want me to PR that patch?
19:09:27 <luke-jr> aj: sgtm
19:09:37 <MarcoFalke> wumpus: It fails on slow CPUs ;)
19:09:48 <luke-jr> MarcoFalke: my CPU is nothing close to slow..
19:10:05 <MarcoFalke> CI should be unaffected, because it has the timeouts cranked up to max
19:10:08 <wumpus> I haven't noticed any tests failing on master but haven't locally run the tests for the 0.21 branch yet
19:10:27 <wumpus> (because there is no divergence yet)
19:10:31 <achow101> I've had feature_taproot fail occasionally, but only with the test runner
19:10:33 <MarcoFalke> luke-jr: Its not intel, so it doesn't have the speedup-backdoors
19:10:58 <sipa> that could be it
19:11:03 <MarcoFalke> And running test_runner with high --jobs might also cause it
19:11:12 <sipa> SHA-NI hashing is a lot faster
19:11:17 <achow101> MarcoFalke: that's what I ran into
19:11:47 <jonatack> achow101: same here
19:11:54 <wumpus> it would definitely be good to not have the tests dependent on specific intel optimizations :)
19:12:00 <sipa> it's just silly
19:12:11 <luke-jr> aj's optimisation seems like a good idea even with longer timeouts
19:12:14 <sipa> we could also split the test in two, and run it once for post-activation and once for pre-activation
19:12:57 <michaelfolkson> sipa said he runs -j60. I'm assuming that is too high. Something like -j5 better?
19:13:07 <luke-jr> lol?
19:13:36 <wumpus> disable-all-timeouts mode would be useful in that case xD
19:13:42 <michaelfolkson> Or that's what I remembered. Am I remembering incorrectly...?
19:13:45 <sipa> -j60 works fine here
19:13:52 <MarcoFalke> wumpus: The ci runs it, but we can't enable that locally
19:13:55 <sipa> and seems around the fastest in overall clock time
19:14:00 <luke-jr> michaelfolkson: as long as -j is set sensibly, the number itself shouldn't make problems
19:14:03 <MarcoFalke> I guess we can scale the timeout with --jobs
19:14:32 <jonatack> michaelfolkson: i've adopted -j60 since sipa mentioned it, and added the suggestion as an option to try in the compile guide
19:14:39 <luke-jr> I was getting it 100% of the time, with just the one test
19:14:45 <wumpus> I'm not sure that needs to be done automatically, but instructions on how to do it for people running into timeouts would be useful
19:15:52 <sipa> luke-jr: i think it's fair to say the test is just broken, and it's masked by the fact that almost everyone runs it on hardware where SHA256 is sufficiently optimized to mask the issue
19:16:02 <jonatack> michaelfolkson: (not for make, just for running the functional suite)
19:16:03 <wumpus> hehe
19:16:14 <MarcoFalke> Jup, each raise of a timeout error could mention "You can use --timeout_factor to scale the timeouts"
19:16:27 <luke-jr> MarcoFalke: but it shouldn't be necessary
19:16:29 <wumpus> MarcoFalke: good idea!
19:16:42 <aj> sipa: seems like there's something wrong with signrawtx in that it's taking 20s (for me) or 30s+ (for luke) as well though
19:16:44 <sipa> luke-jr: https://0bin.net/paste/yldL+QX5#hXq0n10PZrK9flMwh2313ueXFAQjPBE2+VaWvKNO+bB
19:16:46 <luke-jr> if a test hits it normally, that means the timeout is too low
19:16:46 <michaelfolkson> jonatack: Let me try make with -j60... :)
19:17:02 <miketwenty1> does -j1 or no -j flag fail?
19:17:17 <MarcoFalke> with enough ram you can do -j 999
19:17:21 <sipa> miketwenty1: -j1 is just painfully slow
19:17:41 <luke-jr> miketwenty1: I was running the test by itself, nothing else
19:17:52 <sipa> aj: quadratic hashing of enormous (non-standard!) transactions is a known issue
19:17:53 <miketwenty1> i know it's slow i've compiled bitcoin like 5 times today with no -j flag
19:17:57 <wumpus> the thing with test timeouts (especially for CPU intensive ones) is that it's kind of a random barrel shoot, you can always have a slower machine where it fails
19:17:59 <MarcoFalke> sipa: That patch would still fail intermittently
19:18:03 <sipa> MarcoFalke: how so?
19:18:12 <luke-jr> miketwenty1: alias make='make -j60'
19:18:12 <MarcoFalke> sec ...
19:18:14 <jonatack> miketwenty1: use ccache? see doc/productivity.md
19:18:33 <jonatack> miketwenty1: also build only what you need
19:18:39 <miketwenty1> the hardware that's compiling is purposely small
19:18:39 <wumpus> alias invokeoomkiller='make -j60'
19:18:44 <luke-jr> wumpus: unless the timeout is set high enough it only fails when there's hanging
19:18:52 <luke-jr> wumpus: lol
19:18:59 <luke-jr> ok, I really do make -j32
19:19:37 <MarcoFalke> sipa: Maybe coin selection picking only the small coins?
19:19:38 <luke-jr> (but having it aliased really helps me use it)
19:20:27 <sipa> MarcoFalke: how would it fail?
19:20:41 <miketwenty1> what is the logical way to calculate how many threads or the -j<n> count you can use for a machine?
19:20:53 <MarcoFalke> sipa: It picks a lot of small coins, which takes long to sign?
19:21:09 <MarcoFalke> coin selection isn't deterministic and the amounts in the test aren't either
19:21:13 <sipa> MarcoFalke: with just 70% of the balance there should be no need for many tiny coins
19:21:42 <luke-jr> miketwenty1: start with number of cores + 1
19:21:55 <luke-jr> miketwenty1: possibly including SMT in core count
19:21:58 <sipa> miketwenty1: for building, not more than your number of threads + 1, and assume around 1 GB of memory per -j
19:22:01 <wumpus> miketwenty1: in my experience it's about 1.5GB memory per -j, for the C++ build, for running the functional tests it's different though
19:22:19 <sipa> miketwenty1: for functional tests, you can go way higher than what your thread count permits
19:22:59 <MarcoFalke> sipa: I guess we could try
19:23:12 <michaelfolkson> miketwenty1: https://jonatack.github.io/articles/how-to-compile-bitcoin-core-and-run-the-tests
19:23:15 <sipa> MarcoFalke: afaik the pessimal behavior of coin selections only occurs when you're really close to the balance
19:23:48 <sipa> luke-jr: does my patch fix things for you? (aj's may work too, i think in general mine may be even faster)
19:23:54 <MarcoFalke> Anyway back to the 0.21 topic, I suggest to ship rc1 soon, so that testing can start
19:24:01 <wumpus> heh that's like with place and route on FPGAs
19:24:06 <luke-jr> sipa: too many worktrees open to test right now, maybe after meeting
19:24:11 <sipa> luke-jr: sure
19:24:43 <MarcoFalke> If we are lucky and no issues are found we can ship 0.21 early-mid december
19:24:48 <wumpus> MarcoFalke: so maybe do a rc1 without merging anything more?
19:24:57 <MarcoFalke> I'd say so
19:25:12 <wumpus> okay, going to tag it after the meeting
19:25:16 <sipa> wumpus: with the plan of doing an rc2 anyway?
19:25:20 <luke-jr> I'd call it beta1, but dunno how easy that is
19:25:24 <MarcoFalke> sipa: I'd say so too
19:25:44 <MarcoFalke> The fixups are just for edge-cases in the rpc
19:25:50 <sipa> indeed
19:25:52 <MarcoFalke> (and they have tests)
19:26:01 <wumpus> right, major versions tend to have 3-5 rcs anyway
19:26:15 <sipa> wumpus: if doing rc1 now is fine by you, it's fine by me
19:26:23 <wumpus> sure
19:26:42 <luke-jr> might want to add a "Known issues" to the announce
19:26:46 <luke-jr> so people don't report them
19:26:57 <wumpus> I intended to do the rc1 tag right after the branch but was distracted by some msvc version stuff xD
19:26:58 <sipa> MarcoFalke: did 17 runs with my patch, no failures
19:27:18 <MarcoFalke> sipa: It passed for you before, too
19:27:45 <MarcoFalke> luke-jr should test maybe overnight in a loop or so
19:27:47 <jonatack> a propos editing the release notes in the wiki, istm that is still feasible until -final, right?
19:27:55 <MarcoFalke> jonatack: Jup
19:27:58 <wumpus> jonatack: yep
19:28:18 <MarcoFalke> About 0.20.2, we should ship that soon as well
19:28:20 <MarcoFalke> https://github.com/bitcoin/bitcoin/pulls?q=is%3Aopen+is%3Apr+milestone%3A0.21.0
19:28:24 <luke-jr> MarcoFalke: bonus: I have Chromium building now :P
19:28:26 <wumpus> #topic 0.20.2
19:28:26 <core-meetingbot> topic: 0.20.2
19:28:27 <MarcoFalke> ^ Though, those need review
19:28:57 <MarcoFalke> One is a trivial revert (empty diff can be reviewed by anyone)
19:29:20 <MarcoFalke> The other is a wallet fix, so needs the wallet people to take a look
19:29:46 <MarcoFalke> wrong milestone link (sry)
19:29:59 <wumpus> going back and forth on the wtxid relay is a bit ugly, but if there is no confidence in it anymore then it's good to revert it
19:30:04 <MarcoFalke> maybe this: https://github.com/bitcoin/bitcoin/milestone/49
19:31:12 <MarcoFalke> #link https://github.com/bitcoin/bitcoin/milestone/49
19:32:00 <wumpus> I remember it was quite a difficult merge in the first place, maybe we shouldn't have done it, but I remember some were saying it was really important to get it in at the time
19:32:24 <achow101> the wallet fix just does what we do in master. it's not quite a backport because we kind of got to this result in master by accident through 3 or so prs
19:32:25 <aj> wumpus: i think there's reasonable confidence, but there's no need for it anymore either
19:32:37 <wumpus> aj: what removed the need for it?
19:33:16 <sipa> wumpus: https://github.com/bitcoin/bitcoin/pull/20317#issuecomment-727624755
19:33:22 <MarcoFalke> achow101: Have the "3 or so prs" touched other parts than just this single function?
19:33:29 <achow101> MarcoFalke: yes
19:33:38 <aj> wumpus: #19620 which got backported to 0.19 and 0.20 after the wtxid backport PR was already open
19:33:41 <gribble> https://github.com/bitcoin/bitcoin/issues/19620 | Add txids with non-standard inputs to reject filter by sdaftuar · Pull Request #19620 · bitcoin/bitcoin · GitHub
19:33:54 <MarcoFalke> achow101: That might make it harder to review
19:34:03 <aj> wumpus: (19680 and 19681 were the backports)
19:34:13 <wumpus> sipa: thanks
19:34:36 <jnewbery> wumpus: should be trivial to back it out. It wasn't actually too difficult to backapply, and as Marco says, if the diff after the revert is empty then it's a trivial ACK
19:35:16 <wumpus> jnewbery: yes, it's easy to revert (not much happend on the 0.20 branch since), I was just confused about the change in priorities suddenly
19:35:58 <jnewbery> wumpus: me too :)
19:36:06 <MarcoFalke> wumpus: I think there was confusion generally, and probably an assumption that no 0.20 release was planned any time soon
19:36:52 <wumpus> MarcoFalke: yes, I do remember that we wanted to do the next 0.20 release after 0.21.0
19:36:58 <luke-jr> not so sure about the latter
19:37:23 <MarcoFalke> wumpus: Jup, that was under the assumption that wtxid relay was going to be backported
19:37:40 <wumpus> in any case please review the revert and other PRs for 0.20.2
19:38:39 <wumpus> when they're merged we can tag 0.20.2rc1 as well
19:39:17 <wumpus> any other topics?
19:39:57 <wumpus> I guess we can pick up high priority for review again
19:40:21 <wumpus> #topic High priority for review
19:40:21 <core-meetingbot> topic: High priority for review
19:40:23 <wumpus> https://github.com/bitcoin/bitcoin/projects/8
19:40:49 <wumpus> is the current list still relevant?
19:41:24 <MarcoFalke> Can I add #19893 ?
19:41:26 <gribble> https://github.com/bitcoin/bitcoin/issues/19893 | test: Remove or explain syncwithvalidationinterfacequeue by MarcoFalke · Pull Request #19893 · bitcoin/bitcoin · GitHub
19:41:48 <wumpus> sure
19:42:47 <jonasschnelli> I just removed #18242 (until the AEAD overhaul has been finalized)
19:42:53 <gribble> https://github.com/bitcoin/bitcoin/issues/18242 | Add BIP324 encrypted p2p transport de-/serializer (only used in tests) by jonasschnelli · Pull Request #18242 · bitcoin/bitcoin · GitHub
19:42:57 <achow101> #20040 pls
19:43:00 <gribble> https://github.com/bitcoin/bitcoin/issues/20040 | wallet: Refactor OutputGroups to handle fees and spending eligibility on grouping by achow101 · Pull Request #20040 · bitcoin/bitcoin · GitHub
19:45:03 <wumpus> achow101: jonasschnelli added
19:45:11 <jnewbery> I'd like to add #19910, which is next in the #19398 sequence
19:45:23 <gribble> https://github.com/bitcoin/bitcoin/issues/19910 | net processing: Move peer_map to PeerManager by jnewbery · Pull Request #19910 · bitcoin/bitcoin · GitHub
19:45:23 <gribble> https://github.com/bitcoin/bitcoin/issues/19398 | Move remaining application layer data to net processing · Issue #19398 · bitcoin/bitcoin · GitHub
19:46:12 <wumpus> jnewbery: added too
19:46:32 <jnewbery> thanks wumpus!
19:47:43 <wumpus> I think that concludes the meeting, thanks everyone
19:47:46 <wumpus> #endmeeting