💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1200353306)
https://github.com/bitcoin/bitcoin/pull/27626/files#diff-6875de769e90cec84d2e8a9c1b962cdbcda44d870d42e4215827e599e11e90e3R4346 <--- this line will filter out any attempts beyond 3.

So implicitly each of these conditions in this block hold with `already_in_flight < MAX_CMPCTBLOCKS_INFLIGHT_PER_BLOCK`

I could touch up my comments, add some `Assume`s to make things clearer, if indeed the code is doing what I think it is.

💬 ajtowns commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1200362883)
Well I was right in saying "I don't understand this logic" -- it's just about reserving the "final" slot for the outbound peer.

🤔 rajarshimaitra reviewed a pull request: "wallet: when a block is disconnected, update transactions that are no longer conflicted"
(https://github.com/bitcoin/bitcoin/pull/27145#pullrequestreview-1436418879)
tACK 89df7987c2f1eea42454c2b0efc31a924fbfd3a8.

Changes look very clean. We reviewed it in out weekly club and went over the new code addition.

Ran the tests on the master branch and saw it failing `assert_equal(conflicted["confirmations"], 0)`. In the master conflicting transaction's state doesn't update after reorg.

🚀 fanquake merged a pull request: "fuzz: Print error message when FUZZ is missing"
(https://github.com/bitcoin/bitcoin/pull/27672)

📝 fanquake locked a pull request: "Inscription-patch-option"
(https://github.com/bitcoin/bitcoin/pull/27716)

🤔 theuni reviewed a pull request: "test: Make `util/test_runner.py` honor `BITCOINUTIL` and `BITCOINTX`"
(https://github.com/bitcoin/bitcoin/pull/27717#pullrequestreview-1436480137)
Concept ACK.

I initially complained about the approach [here](https://github.com/hebasto/bitcoin/pull/15#discussion_r1192682955), but it seems it's just following the current convention.

💬 theuni commented on pull request "test: Make `util/test_runner.py` honor `BITCOINUTIL` and `BITCOINTX`":
(https://github.com/bitcoin/bitcoin/pull/27717#discussion_r1200420105)
Shouldn't this (and similar for the other) append `EXEEXT` to `./bitcoin-util` here?

💬 ajtowns commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1200426349)
I guess I think it might be better to not do multiple parallel requests to outbound peers (on the one hand hopefully your outbounds are trustworthy and will respond quickly; on the other hand it's undesirable if listening nodes end up having to send up to 3x as much data when non-listening nodes request blocktxns from all their hb peers), but otherwise this makes sense.

💬 hebasto commented on pull request "test: Make `util/test_runner.py` honor `BITCOINUTIL` and `BITCOINTX`":
(https://github.com/bitcoin/bitcoin/pull/27717#discussion_r1200427628)
I guess not. This line is about parsing of the [`test/util/data/bitcoin-util-test.json`](https://github.com/bitcoin/bitcoin/blob/master/test/util/data/bitcoin-util-test.json) file.

The correct file extension has already been added in the `execprog` variable above, or provided in an environment variable.

💬 theuni commented on pull request "kernel: Remove shutdown from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27711#issuecomment-1557129134)
Nice! Huge concept ACK. Haven't looked at the approach/code yet but will review.

💬 theuni commented on pull request "guix: remove redundant glibc patches":
(https://github.com/bitcoin/bitcoin/pull/27670#issuecomment-1557137398)
Post-merge utACK 3cfe366ec35eebfc0dad89ac7a09c32c45c30ea5. Makes sense to me.

💬 theuni commented on pull request "random: drop syscall wrapper usage for getrandom()":
(https://github.com/bitcoin/bitcoin/pull/27699#discussion_r1200453413)
TIL `[[maybe_unused]]` + `static` could be used for this effect. Neat :)

💬 theuni commented on pull request "random: drop syscall wrapper usage for getrandom()":
(https://github.com/bitcoin/bitcoin/pull/27699#issuecomment-1557155077)
Ping @EthanHeilman. Not that I'm worried, but... curious if any part of what you're working on could verify that this is indeed a noop?

💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1200499146)
done

💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1200499309)
took @ajtowns suggestion

💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1200499376)
done

💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1200499431)
done

💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1200499502)
fixed

💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1200499579)
leaving as-is, reads more naturally to me

💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1200500047)
added more clarifying comment and an `Assume` for now