<jeremyrubin>
well it's always kind of a negotiation of if the nits need to be addressed here or later. part of the joy of having a guideline on how to handle is that there's a default answer.
<jeremyrubin>
I think we may even get better quality code, because right now i don't nit things i don't want to bother the author to fix given the costs of re-review
<sipa>
occasionally i don't bother leaving nits on the PRs directly, but make a follow-up PR myself after it's merged.
<jeremyrubin>
i think that's reasonable -- no nits, but just fix it yourself as a follow up. they who nit-it fix-it?
<luke-jr>
I planned to do that with NAT-PMP but never got around to it :x
ozawa has quit [Quit: Connection closed]
b10c has quit [Quit: Connection closed for inactivity]
rex4539 has joined #bitcoin-core-dev
rex4539 has quit [Ping timeout: 276 seconds]
luke-jr has quit [Read error: Connection reset by peer]
luke-jr has joined #bitcoin-core-dev
andrewtoth_ has quit [Remote host closed the connection]
andrewtoth_ has joined #bitcoin-core-dev
jtrag has quit [Ping timeout: 250 seconds]
ozawa has joined #bitcoin-core-dev
zonemix has quit [Read error: Connection reset by peer]
zonemix_ has joined #bitcoin-core-dev
<ariard>
laanwj: yeah sad with the premature merge of 22674, commit e12fafd was added on the last push and not previously reviewed, you have a good chunk of context with implications for the rest of package relay support imho :(
<ariard>
nice if past reviewers can re-look on that
<laanwj>
okay, sorry
<laanwj>
yes i screwed up well it's the last PR i'll merge for a while
jtrag has joined #bitcoin-core-dev
<laanwj>
and reverting e12fafd would make sense then (better to do it as a new PR)
bitcoin-git has joined #bitcoin-core-dev
<bitcoin-git>
[bitcoin] laanwj opened pull request #23793: validation: Revert "de-duplicate package transactions already in mempool" (master...2021-12-revert-package-dedup) https://github.com/bitcoin/bitcoin/pull/23793
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
<laanwj>
there
earnestly has quit [Ping timeout: 250 seconds]
<laanwj>
oh, it isn't possible to revert it by itself, revert the entire PR then?
<laanwj>
ok: it introduces MempoolAcceptResult::ResultType::MEMPOOL_ENTRY which is used in the commit after it, but that's only tests, could remove or comment out the test for now
rex4539 has joined #bitcoin-core-dev
<laanwj>
done that
zonemix_ has quit [Remote host closed the connection]
zonemix has joined #bitcoin-core-dev
bitdex has joined #bitcoin-core-dev
rex4539 has quit [Ping timeout: 276 seconds]
zonemix has quit [Remote host closed the connection]
zonemix has joined #bitcoin-core-dev
zonemix has quit [Remote host closed the connection]
zonemix has joined #bitcoin-core-dev
tla2k21 has quit [Remote host closed the connection]
zonemix has quit [Ping timeout: 240 seconds]
tla2k21 has joined #bitcoin-core-dev
zonemix has joined #bitcoin-core-dev
jarthur has joined #bitcoin-core-dev
ozawa has quit [Quit: Connection closed]
cmirror has quit [Remote host closed the connection]
<bitcoin-git>
bitcoin/master 618f4d2 seaona: test: re-organized array according to order of logs and included 2 more in...
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
bitcoin-git has joined #bitcoin-core-dev
<bitcoin-git>
[bitcoin] MarcoFalke merged pull request #23782: test: include two more interruptions points (master...2021-12-feature-init-test-interruptions) https://github.com/bitcoin/bitcoin/pull/23782
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
bitdex has quit [Remote host closed the connection]
bitdex has joined #bitcoin-core-dev
freesprung has quit [Ping timeout: 260 seconds]
freesprung has joined #bitcoin-core-dev
bitcoin-git has joined #bitcoin-core-dev
<bitcoin-git>
bitcoin/master 4807f73 w0xlt: refactor: Implement restorewallet() logic in the wallet section
<bitcoin-git>
bitcoin/master abbb7ec w0xlt: refactor: Move restorewallet() RPC logic to the wallet section
<MarcoFalke>
PSA: Due to "activity" on archived issues and pull requests, I'll be locking everything up to Dec 31st 2019.
<MarcoFalke>
Please open a new issue or leave a question on IRC if something relevant to a locked issue or pull request comes p
<MarcoFalke>
*up
dr-orlovsky has quit [Ping timeout: 240 seconds]
dr-orlovsky has joined #bitcoin-core-dev
<MarcoFalke>
Preemptively locking helps against spam attacks on archived threds. Also, it reduces harrassment of people's inbox that left a comment on a thread more than two years ago.
<MarcoFalke>
*threads
<jnewbery>
MarcoFalke: ACK. Seems like a good policy. Please thank drahtbot for beings so active on issue/pr management :)
<MarcoFalke>
Drahty will get some nice carrots for new years
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
sdfgsdfg has quit [Quit: ZzzZ]
jtrag has quit [Ping timeout: 240 seconds]
bomb-on has quit [Quit: aллилѹіа!]
jtrag has joined #bitcoin-core-dev
bitdex has quit [Remote host closed the connection]
bitdex has joined #bitcoin-core-dev
zonemix has quit [Ping timeout: 256 seconds]
jtrag has quit [Quit: 💙]
zonemix has joined #bitcoin-core-dev
jtrag has joined #bitcoin-core-dev
jtrag has quit [Client Quit]
jtrag has joined #bitcoin-core-dev
jespada has quit [Quit: My MacBook has gone to sleep. ZZZzzz…]
jespada has joined #bitcoin-core-dev
jtrag is now known as Guest7615
jtrag_ has joined #bitcoin-core-dev
jtrag_ is now known as jtrag
Guest7615 has quit [Ping timeout: 240 seconds]
mikehu44 has quit [Ping timeout: 240 seconds]
<laanwj>
MarcoFalke: ACK, assuming you mean closed issues and PRs
<MarcoFalke>
jup, closed ones
<laanwj>
great!
b10c has joined #bitcoin-core-dev
bitcoin-git has joined #bitcoin-core-dev
<bitcoin-git>
[bitcoin] MarcoFalke opened pull request #23795: refactor: Remove implicit-integer-sign-change suppressions in validation (master...2112-refValSup) https://github.com/bitcoin/bitcoin/pull/23795
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
b10c has quit [Changing host]
b10c has joined #bitcoin-core-dev
saranshsharma has joined #bitcoin-core-dev
saranshsharma has quit [Remote host closed the connection]
<glozow>
MarcoFalke: ACK, will bring carrots for Drahty
jtrag_ has joined #bitcoin-core-dev
jtrag is now known as Guest2357
jtrag_ is now known as jtrag
Guest2357 has quit [Ping timeout: 256 seconds]
evbo has joined #bitcoin-core-dev
bitcoin-git has joined #bitcoin-core-dev
<bitcoin-git>
[bitcoin] theStack opened pull request #23796: test: check that pruneblockchain RPC fails for future block or timestamp (master...202112-test-add_pruneblockchain_test_coverage) https://github.com/bitcoin/bitcoin/pull/23796
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
joshuajbouw has quit [Ping timeout: 250 seconds]
andrewtoth_ has quit [Remote host closed the connection]
jtrag_ has joined #bitcoin-core-dev
jtrag is now known as Guest1840
andrewtoth has joined #bitcoin-core-dev
Guest1840 has quit [Ping timeout: 240 seconds]
zonemix has quit [Remote host closed the connection]
zonemix has joined #bitcoin-core-dev
zonemix has quit [Ping timeout: 240 seconds]
masta`` has joined #bitcoin-core-dev
b10c_ has joined #bitcoin-core-dev
zonemix has joined #bitcoin-core-dev
b10c has quit []
b10c_ is now known as b10c
bomb-on has joined #bitcoin-core-dev
b10c has quit [Changing host]
b10c has joined #bitcoin-core-dev
jtrag__ has joined #bitcoin-core-dev
jtrag_ has quit [Ping timeout: 240 seconds]
zonemix has quit [Remote host closed the connection]
zonemix has joined #bitcoin-core-dev
Talkless has joined #bitcoin-core-dev
andrewtoth_ has joined #bitcoin-core-dev
andrewtoth has quit [Ping timeout: 276 seconds]
jb55 has joined #bitcoin-core-dev
Trinity96 has joined #bitcoin-core-dev
Trinity96 has quit [Client Quit]
zonemix has quit [Remote host closed the connection]
zonemix has joined #bitcoin-core-dev
zonemix has quit [Ping timeout: 268 seconds]
bomb-on has quit [Read error: Connection reset by peer]
bomb-on_ has joined #bitcoin-core-dev
zonemix has joined #bitcoin-core-dev
zonemix has quit [Ping timeout: 240 seconds]
arythmetic has quit [Remote host closed the connection]
<laanwj>
welcome to what i guess is the last general IRC meeting of 2021
<michaelfolkson>
hi
<luke-jr>
hi
<ariard>
hi
<sipa>
hi
<fjahr>
hi
<b10c>
hi
<lightlike>
hi
<laanwj>
there haven't been any pre-proposed meeting topics (use #proposedmeetingtopic during any time of the week)
<laanwj>
any last minute one?
<achow101>
suggested topic: wallet maintainer
<jonatack>
hi
<luke-jr>
achow101: are you volunteering? :P
<achow101>
yes
<laanwj>
jamesob: yeah time flies when you're having-- well, time flies anyway
<cfields_>
hi
<jamesob>
laanwj: haha
<jb55>
achow101 it is then, that was easy
<luke-jr>
laanwj: lol
arythmetic has quit [Ping timeout: 240 seconds]
<michaelfolkson>
That was too easy. NACK to achow101
<laanwj>
#topic Wallet maintainer
<core-meetingbot>
topic: Wallet maintainer
<MarcoFalke>
jamesob is having a rebase party (fun)
<michaelfolkson>
jokez
<luke-jr>
x.x
<jamesob>
MarcoFalke: how did you know?? ;)
arythmetic has joined #bitcoin-core-dev
zonemix has joined #bitcoin-core-dev
<luke-jr>
I need to have a rebase party too soon :x
<MarcoFalke>
jamesob: I can hear your keyboard
<jamesob>
lol
<laanwj>
hahaha
<jamesob>
nope, that's just PTSD from _actually_ hearing my keyboard two years ago
<luke-jr>
._.
<jb55>
rebase party sounds fun but I might have too many conflicts
<cfields_>
lol
<MarcoFalke>
achow101: Looks like there are no objections. Just promise to merge no bugs, ok?
<laanwj>
ok, on topic, ack to achow101 wallet maintainer
<cfields_>
ack
<jb55>
ack
<michaelfolkson>
ACK
<ariard>
ack
<fjahr>
ack
<luke-jr>
should probably post to ML first, but SGTM
<jamesob>
ack achow
<laanwj>
no, this is a project decision, not a ML decision
<luke-jr>
(or not since it's just a Core internal thing idk)
<b10c>
ach ackow101
<laanwj>
luke-jr: right
<sipa>
ack
<jonatack>
ACK modulo no bugs :p
<luke-jr>
laanwj: right, but some people might not be here
<jamesob>
b10c: god bless you
<achow101>
MarcoFalke: those aren't bugs, they're features :p
<MarcoFalke>
jup bitcoin-core-dev is only for releases
<laanwj>
luke-jr: we'll not immediately assign it so other people have time to comment
<kvaciral[m]>
ack
<MarcoFalke>
There will also be a pull request, which people can comment on
<laanwj>
it's not an actual decision in the meeting just an oppertunity to bring up objections
<laanwj>
right
<luke-jr>
MarcoFalke: good point
<MarcoFalke>
I expect that pull to be open for a week at least, maybe longer because holidays?
<laanwj>
yeah
<achow101>
i'll open it after the meeting then
<sipa>
sgtm
<laanwj>
oh github has something new, if you click projects you get "projects (beta)" which doesn't actually have the projects... was afraid for a moment they were all gone
<laanwj>
#topic High priority for review
<core-meetingbot>
topic: High priority for review
<MarcoFalke>
can I haz ACK/NACK on #23411 ?
<gribble>
https://github.com/bitcoin/bitcoin/issues/23411 | refactor: Avoid integer overflow in ApplyStats when activating snapshot by MarcoFalke · Pull Request #23411 · bitcoin/bitcoin · GitHub
<laanwj>
michaelfolkson: but according to achow101 it's unreachable anyway
<luke-jr>
XD
<laanwj>
in any case reverting the entire seems unnecessary, most of the commits did have extensive review, it was only that one that didn't
jtrag__ is now known as jtrag
<ariard>
yeah it's not used currently, submitpackage rpc hasn't been yet introduced afaik, though critical path for future package relay support
<achow101>
if it were reachable, i would recommend revert because there's a crashing bug, but it's not reachable, so meh
<michaelfolkson>
laanwj: Gotcha, thanks. Have you ever thought of giving advance warning that you're intending to merge something? I've always wondered that
<michaelfolkson>
Something like "I think this is ready for merge. Intending to merge in say 3 days"
<laanwj>
michaelfolkson: i do that sometimes
<michaelfolkson>
Ah ok
<luke-jr>
these issues are rare enough that reverting when there's a problem is probably a better approach IMO
<laanwj>
i made mistake here
<michaelfolkson>
It has secondary effect of being like "last orders" or "last chance to review before merging"
<MarcoFalke>
laanwj: I think mistakes are impossible to avoid in general
<laanwj>
luke-jr: it only makes sense for extensive or somewhat controversial PRs, not for run of the mill ones
<MarcoFalke>
I've merged silent merge conflicts twice in the last week or so
<michaelfolkson>
But maybe it has downsides, I don't know. I guess the author would generally prefer immediate merging to avoid need for more rebases
<luke-jr>
laanwj: right
<MarcoFalke>
They are just too hard to catch while merging
<jamesob>
merge-freezes and plenty of testing before releases are the only saving graces afaict
<laanwj>
MarcoFalke: yes that happens often enough to me too, usually my local build catches them but it's impossible to run for every platform combo
<MarcoFalke>
I'll experiment with DrahtBot re-running the CI on weekends
<MarcoFalke>
There is less "real" traffic on weekends
<laanwj>
that would be useful
<luke-jr>
btw, any reason not to set `greedy` in Cirrus CI stuff?
<sipa>
greedy?
<luke-jr>
and bump up the -j
<MarcoFalke>
luke-jr: Sure, go for it
<luke-jr>
sipa: lets the VM use more CPU if it's idle
<sipa>
oh, cool
<luke-jr>
MarcoFalke: k
<MarcoFalke>
sipa: If there is idle CPU on the host it will consume it, even if less CPU was requested
<laanwj>
thanks to ariard for doing another review round and catching it anyhow
<luke-jr>
laanwj: eh, in theory it sounds easy (in practice, tho..)
<laanwj>
think of the dependency graph :-)
<sipa>
Yes, and in theory, there is no difference between theory and practice; in practice however...
<luke-jr>
XD
bitcoin-git has joined #bitcoin-core-dev
<bitcoin-git>
[bitcoin] luke-jr opened pull request #23797: ci: Use Cirrus "greedy" flag to use idle CPU time when available (master...cirrus_greedy) https://github.com/bitcoin/bitcoin/pull/23797
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
<luke-jr>
michaelfolkson: eh, Cirrus allows up to 32 GB RAM O.o
<MarcoFalke>
sipa: I think there is no downside using a higher -j (if you have enough memory)
<jonatack>
laanwj: yes, those purisms don't have many
<jonatack>
sipa: (with 32go RAM)
<sipa>
jonattack: well, benchmark on your own system which amount works best. I imagine it's a complex function of cpu / diskspeed etc.
<luke-jr>
laanwj: mine is only 16 cores, but they're SMT4
<laanwj>
jonatack: right, neither do ARM and RV boards for that matter
<b10c>
MarcoFalke: thanks, so hetzner cloud (based on cpx21)
<MarcoFalke>
b10c: heh, right
<_aj_>
jonatack: test_runner.py -j 24 # works pretty well for me with 16GB; usually get one spurious failure though
<laanwj>
luke-jr: ye that helps
<jonatack>
i do have to use timeout-factor tho, otherwise a few tests time out frequently (rpc_misc mostly, then feature_blockfilterindex_prune and feature_assumevalid)
<sipa>
I/O speed matters a lot too.
<laanwj>
the way around that is to mount /tmp as tmpfs
<jonatack>
maybe my disk encryption doesn't help matters
<jonatack>
aj: nice
<laanwj>
or at least, whatever temp directory you use for the functional tests
<_aj_>
jonatack: (/tmp is on ssd)
<luke-jr>
laanwj: I have a zram :p
<luke-jr>
not sure I can recommend it though, as it seems Linux is buggy
<luke-jr>
(the zram's ext4 fs gets corrupted sometimes)
<MarcoFalke>
I also use tmpfs for /tmp
<laanwj>
TIL about zram
<sipa>
same
<laanwj>
i guess that concludes the topic? let's see what #23797 does
<luke-jr>
(haven't noticed any swap-zram issues, so I'm guessing the mentioned bugs are probably ext4)
<laanwj>
i don't think it makes sense to do a meeting next week (dec 23th) or after (dec 30th), so next one will be jan 6
<sipa>
yeah, agree
<michaelfolkson>
Cool. Next week's Core PR review club is on multiprocess if anyone wants to use that as an excuse to play around with the multiprocess stuff :)
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
zonemix has joined #bitcoin-core-dev
luke-jr has quit [Ping timeout: 256 seconds]
lukedashjr has quit [Ping timeout: 256 seconds]
luke-jr has joined #bitcoin-core-dev
zonemix has quit [Ping timeout: 256 seconds]
zonemix has joined #bitcoin-core-dev
zonemix has quit [Remote host closed the connection]
zonemix has joined #bitcoin-core-dev
zonemix has quit [Ping timeout: 268 seconds]
Talkless has quit [Quit: Konversation terminated!]
bitcoin-git has joined #bitcoin-core-dev
<bitcoin-git>
[bitcoin] sipa opened pull request #23799: Let test_runner.py start multiple jobs per timeslot (master...202112_multitesturn) https://github.com/bitcoin/bitcoin/pull/23799
bitcoin-git has left #bitcoin-core-dev [#bitcoin-core-dev]
zonemix has joined #bitcoin-core-dev
zonemix has quit [Remote host closed the connection]
zonemix has joined #bitcoin-core-dev
zonemix has quit [Ping timeout: 268 seconds]
zonemix has joined #bitcoin-core-dev
zonemix has quit [Remote host closed the connection]
zonemix has joined #bitcoin-core-dev
zonemix has quit [Ping timeout: 240 seconds]
<jb55>
I ran the functional test suite for the first time today, all is good except feature_dbcrash.py seems to hang, is this a known issue?
<sipa>
On what kind of system? I don't see any problems.
<jb55>
linux (nixos). hmm ok now I need to figure out how to debug this...
sdfgsdfg has joined #bitcoin-core-dev
zonemix has joined #bitcoin-core-dev
jtrag_ has joined #bitcoin-core-dev
jtrag is now known as Guest6729
zonemix has quit [Ping timeout: 256 seconds]
jtrag_ is now known as jtrag
Guest6729 has quit [Ping timeout: 256 seconds]
rex4539 has quit []
zonemix has joined #bitcoin-core-dev
zonemix has quit [Ping timeout: 268 seconds]
<jb55>
sipa: ah it wasn't hanging, it looks like the test goes through 40 iterations and each iteration takes over a minute.
<sipa>
Slow disk I/O?
<jb55>
it's a new SSD, every other test ran quickly
<jb55>
Iteration 16, generating 2500 transactions - looks like this is taking about a minute each iteration...