✅ fanquake closed an issue: "Intermittent issue in p2p_i2p_ports.py AssertionError: [node 0] Expected messages "['Error connecting to [...].b32.i2p:0: Cannot connect to 127.0.0.1:60000']" does not partially match log:"
(https://github.com/bitcoin/bitcoin/issues/30030)

🚀 fanquake merged a pull request: "test: Use unassigned p2p_port instead of hardcoded 60000 in p2p_i2p_ports.py"
(https://github.com/bitcoin/bitcoin/pull/33670)

💬 ajtowns commented on pull request "p2p: Drop unsolicited CMPCTBLOCK from non-HB peer":
(https://github.com/bitcoin/bitcoin/pull/32606#issuecomment-3431050727)
Concept ACK here too; haven't looked at the code yet

🤔 stickies-v reviewed a pull request: "Add libbitcoinkernel example files"
(https://github.com/bitcoin/bitcoin/pull/33669#pullrequestreview-3364281398)
Concept ACK, but I agree with @TheCharlatan that this repo is not be the most productive place to put them.

💬 m3dwards commented on pull request "ci: Only write docker build images to Cirrus cache":
(https://github.com/bitcoin/bitcoin/pull/33639#issuecomment-3431070343)
ACK fabe0e07de1ad2f26da62f3ebe0e9be3f939b1f8

This will save a lot of space and help to prevent the other caches from getting evicted which are much more important than the docker build layer caches. Should only slow down the jobs by a few minutes at most and hopefully be a net positive as the other more important caches (like ccache) will not have been evicted.

Ran on master on my own fork.

💬 Raimo33 commented on pull request "wallet: remove redundant sighash calculation in Musig2 signing flow":
(https://github.com/bitcoin/bitcoin/pull/33665#issuecomment-3431076499)
Concept ACK

💬 hebasto commented on pull request "randomenv: Fix MinGW dllimport warning for `environ`":
(https://github.com/bitcoin/bitcoin/pull/33570#issuecomment-3431122616)
Friendly ping people who reviewed the previous [change](https://github.com/bitcoin/bitcoin/pull/30491):

@sipa @sipsorcery @theuni

💬 laanwj commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2450954020)
nit: please capitalize P2P (another one below in `btck_block_to_bytes`)

💬 sipa commented on pull request "randomenv: Fix MinGW dllimport warning for `environ`":
(https://github.com/bitcoin/bitcoin/pull/33570#issuecomment-3431135242)
utACK 9610b0d1e28aeda02a2ddcf1f0591ae577c3e88e if this makes the compilers happy

👍 laanwj approved a pull request: "refactor: optimize block index comparisons (1.4-6.8x faster)"
(https://github.com/bitcoin/bitcoin/pull/33637#pullrequestreview-3364368958)
Code review ACK c15d839aefba017e64f14cd9cd2779655352f4b6
i did not run the benchmarks but the code changes look good, using `<=>` makes sense.

💬 ajtowns commented on pull request "p2p: Drop unsolicited CMPCTBLOCK from non-HB peer":
(https://github.com/bitcoin/bitcoin/pull/32606#discussion_r2450961798)
> I think this check is still permissive enough such that a `NetMsgType::CMPCTBLOCK` message can be sent to a node in -blocksonly mode and still get processed. T

Should we only send `SENDCMPCT hb=false` after `VERACK` if we're not blocksonly? Making all the compact block stuff conditional on not being `-blocksonly=1` would make sense to me, but probably isn't needed for this PR.

💬 ajtowns commented on pull request "p2p: Drop unsolicited CMPCTBLOCK from non-HB peer":
(https://github.com/bitcoin/bitcoin/pull/32606#discussion_r2450941415)
This doesn't seem to have been taken (or has been lost in a rebase?) Should this be a NET log message or a CMPCTBLOCK log message?

📝 sipa opened a pull request: "randomenv: drop self define of 'environ'"
(https://github.com/bitcoin/bitcoin/pull/33675)
This variable/symbol/macro ought to be defined by the system includes, not us.

Alternative to #33570.

🤔 hebasto reviewed a pull request: "randomenv: drop self define of 'environ'"
(https://github.com/bitcoin/bitcoin/pull/33675#pullrequestreview-3364479135)
Concept ACK.

💬 achow101 commented on pull request "wallettool, test: Remove BDB/ legacy wallets dump feature":
(https://github.com/bitcoin/bitcoin/pull/33161#issuecomment-3431219751)
Removing this functionality does not meaningfully reduce the amount of code that we have since it is relying on things that we must keep for migration anyways. I think it is also useful to continue to be able to dump legacy wallets with current tooling. Also being able to dump a BDB wallet is the only way to migrate a bdb-descriptor wallet (these could only be created for a few months in 2020).

✅ achow101 closed a pull request: "wallettool, test: Remove BDB/ legacy wallets dump feature"
(https://github.com/bitcoin/bitcoin/pull/33161)

💬 laanwj commented on pull request "randomenv: drop self define of 'environ'":
(https://github.com/bitcoin/bitcoin/pull/33675#issuecomment-3431220853)
Concept ACK

💬 maflcko commented on pull request "randomenv: drop self define of 'environ'":
(https://github.com/bitcoin/bitcoin/pull/33675#issuecomment-3431240789)
(mimicking drahtbot, bleeb blop)

https://github.com/bitcoin/bitcoin/actions/runs/18710961104/job/53359113198?pr=33675#step:9:1582:

```
[ 13%] Building CXX object src/util/CMakeFiles/bitcoin_util.dir/__/randomenv.cpp.o
cd /home/admin/actions-runner/_work/_temp/build/src/util && /bin/ccache /usr/bin/clang++ --target=arm64-apple-darwin -isysroot/home/admin/actions-runner/_work/_temp/depends/SDKs/Xcode-15.0-15A240d-extracted-SDK-with-libcxx-headers -nostdlibinc -iwithsysroot/usr/include/c++
...

💬 pablomartin4btc commented on pull request "wallettool, test: Remove BDB/ legacy wallets dump feature":
(https://github.com/bitcoin/bitcoin/pull/33161#issuecomment-3431249637)
> being able to dump a BDB wallet is the only way to migrate a bdb-descriptor wallet (these could only be created for a few months in 2020).

Should we document this? Perhaps in the "migrating" [section](https://github.com/bitcoin/bitcoin/blob/master/doc/managing-wallets.md#migrating-legacy-wallets-to-descriptor-wallets) on `managing-wallets.md`...

✅ sipa closed a pull request: "randomenv: drop self define of 'environ'"
(https://github.com/bitcoin/bitcoin/pull/33675)