💬 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)

💬 sipa commented on pull request "randomenv: drop self define of 'environ'":
(https://github.com/bitcoin/bitcoin/pull/33675#issuecomment-3431269051)
It appears that mac OS actually requires defining it manually.

💬 comassky commented on issue "Hidden service created by `--torcontrol` does not accept connections (General SOCKS server failure)":
(https://github.com/bitcoin/bitcoin/issues/25094#issuecomment-3431281507)
Hi, same issue here with 29.2

```
listen=1
discover=0

onion=tor:9050
onlynet=onion

torcontrol=tor:9051
torpassword=<PASSWORD<

debug=tor
bind=0.0.0.0:8334=onion
```

```
2025-10-22T09:09:19Z [tor] Reading cached private key from /home/bitcoin/.bitcoin/onion_v3_private_key
2025-10-22T09:09:19Z [tor] Successfully connected!
2025-10-22T09:09:19Z msghand thread start
2025-10-22T09:09:19Z [tor] Connected to Tor version 0.4.8.19
2025-10-22T09:09:19Z [tor] Supported authentication method: HASHEDPASS
...

💬 laanwj commented on pull request "randomenv: Fix MinGW dllimport warning for `environ`":
(https://github.com/bitcoin/bitcoin/pull/33570#issuecomment-3431298433)
After discussion i'm convinced that this is the right solution. According to man pages, on POSIX (non-windows) platforms, including MacOS, you are supposed to define this variable yourself. So it can't be removed entirely.

On Windows (irrespective of compiler or toolchain) it's provided by the headers, and defined in a different way, so if we want to use environ therel, we shouldn't define it ourselves.

Note that it's [deprecated](https://learn.microsoft.com/en-us/cpp/c-runtime-library/env
...

💬 achow101 commented on pull request "wallettool, test: Remove BDB/ legacy wallets dump feature":
(https://github.com/bitcoin/bitcoin/pull/33161#issuecomment-3431305009)
> Should we document this?

No, bdb-descriptor was never a officially supported configuration.

💬 maflcko commented on pull request "ci: add Valgrind fuzz":
(https://github.com/bitcoin/bitcoin/pull/33461#discussion_r2451180232)
Hmm, looks like it was faster, which seems odd

💬 glozow commented on pull request "doc: add AGENTS.md":
(https://github.com/bitcoin/bitcoin/pull/33662#issuecomment-3431424348)
If this is/becomes an established convention, I think it's fine to have an agents.md. I'm interested to see what other ways this can help - my cursory reading suggests this can aid agents in understanding dos and donts of the repo, codebase organization, documentation hints, etc. But I'm not really interested in encouraging more contributions that are authored this way.

My current reaction is that this will "catch" very few people who don't want to disclose. In my experience, you'd really nee
...

👍 hebasto approved a pull request: "randomenv: Fix MinGW dllimport warning for `environ`"
(https://github.com/bitcoin/bitcoin/pull/33570#pullrequestreview-3364800231)
My Guix build:
```
917a1df9f8f548b0c6d99bce8bb4774e7095480f7a5a14d4a0d724db78204e98 guix-build-9610b0d1e28a/output/dist-archive/bitcoin-9610b0d1e28a.tar.gz
3372aedd6ed25924fc6e22596114ad8d9a994fb4c1342e1308b4fdbb7a38bdf7 guix-build-9610b0d1e28a/output/x86_64-w64-mingw32/SHA256SUMS.part
8e2bd977f7b6ef6993361d3b49fec11e5d92e6ed72142d9b25896cc4412e2655 guix-build-9610b0d1e28a/output/x86_64-w64-mingw32/bitcoin-9610b0d1e28a-win64-codesigning.tar.gz
fd160165f6e496f0f498b56a467dea67af2c4f341326ea56e
...