💬 achow101 commented on pull request "wallet: Ensure best block matches wallet scan state":
(https://github.com/bitcoin/bitcoin/pull/30221#discussion_r2080598493)
I could not figure out the CI failure, so I've reverted this change.

💬 achow101 commented on pull request "wallet: Ensure best block matches wallet scan state":
(https://github.com/bitcoin/bitcoin/pull/30221#issuecomment-2864638615)
> Simplest fix would probably be to make the CWallet object a static variable in the function like the TestingSetup variable is, instead of a global variable.

It can't be static because the `FUZZ_TARGET` below needs to access it. However, the wallet could be created in there instead of in the setup, as the other wallet fuzz tests do.

But, with moving `WriteBestBlock` back to `RemoveWallet`, this is not necessary either.

👍 theStack approved a pull request: "policy: uncap datacarrier by default"
(https://github.com/bitcoin/bitcoin/pull/32406#pullrequestreview-2826561409)
Concept and code-review ACK 8c360d78b13fa7136db8d6e1e0af5d05f5141a64

💬 theStack commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2080597980)
Seeing this constant also tricked me into thinking that the script size limit might play a role here. Agree that a comment would be nice, or maybe just bump it up to something much higher like e.g. 50000.

💬 achow101 commented on pull request "wallet: init, don't error out when loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/32449#issuecomment-2864640606)
I think we could have a test for this in `wallet_backwards_compatibility.py`.

🤔 w0xlt reviewed a pull request: "fuzz: doc: add info about `afl-system-config` for macOS"
(https://github.com/bitcoin/bitcoin/pull/32175#pullrequestreview-2826580160)
ACK https://github.com/bitcoin/bitcoin/pull/32175/commits/61ea5f348da71b886807c0492587835dd7e57499

Good clarification.

💬 rot13maxi commented on pull request "[Policy] Discourage Unsigned Annexes":
(https://github.com/bitcoin/bitcoin/pull/32453#issuecomment-2864661542)
Im surprised we haven’t seen any weird annex-use yet. This seems like a good safety policy to have in here until the annex’s use gets consensus. Could someone remove this? Sure, but at least theres sort of a warning sign that they might not want to.

🤔 w0xlt reviewed a pull request: "build: let CMake determine the year"
(https://github.com/bitcoin/bitcoin/pull/32445#pullrequestreview-2826582868)
ACK https://github.com/bitcoin/bitcoin/pull/32445/files

💬 davidgumberg commented on pull request "deps: Bump lief to 0.16.5":
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2864794113)
I definitely opened this PR prematurely, thinking I could get away with just bumping the version and fixing the python scripts 😂.

Working on getting guix builds working right, I referenced prior art by @willcl-ark here (https://github.com/willcl-ark/bitcoin/commit/ede9aa93fe6339fe6285c954dc8fbd9ae8623916).

The python build is convinced that a `build` option is set in `config-settings`, even when I override `configure-flags`, which is where `config-settings` is supposed to get it's value f
...

💬 davidgumberg commented on pull request "deps: Bump lief to 0.16.5":
(https://github.com/bitcoin/bitcoin/pull/32431#discussion_r2080687125)
Yes, but it was to appease the CI linter, I understand now that this script is not meant for general use, so I changed this to just cast to `lief.Binary` without checking

💬 0xBEEFCAF3 commented on pull request "CAT in Tapscript (BIP-347)":
(https://github.com/bitcoin/bitcoin/pull/29247#issuecomment-2864936156)
>Let me see if there is anything that can be done to ensure it is never the case that OP_CAT = False && DISCOURAGE_OP_CAT = False since that should never happen outside of tests.

By default, the `SCRIPT_VERIFY_DISCOURAGE_OP_CAT` bit [is set](https://github.com/bitcoin/bitcoin/pull/29247/files#diff-1fc0f6b5081e8ed5dfa8bf230744ad08cc6f4c1147e98552f1f424b0492fe9bdR121), and we do not expose this as a configurable mempool policy. The only practical way for node operators to unset this bit (`SCRIP
...

📝 jb55 opened a pull request: "tracing: fix invalid argument in mempool_monitor"
(https://github.com/bitcoin/bitcoin/pull/32454)
The mempool_monitor tracing tool is incorrectly reading the reason as the first argument, which is incorrect. Fix this!

Noticed this during the bitcoin++ mempool hackathon 😅

cc @0xB10C

💬 Eunovo commented on pull request "Wallet: Fix Non-Ranged Descriptors with Range [0,0] Trigger Unexpected Wallet Errors in AddWalletDescriptor":
(https://github.com/bitcoin/bitcoin/pull/32344#issuecomment-2865239247)
> Including a range in a non-ranged descriptor import should cause the process to fail before reaching the wallet descriptor update function.

AFAICT this will cause `importdescriptor` to throw an error with the message "Range should not be specified for an un-ranged descriptor"

⚠️ hMsats opened an issue: "Self-compiled bitcoind 29.0 much slower than self-compiled 28.0 on my system"
(https://github.com/bitcoin/bitcoin/issues/32455)
I always compile the bitcoin software myself (`Ubuntu 24.04.2 LTS`). For `bitcoin-29.0` I compiled it (of course) for the first time with `cmake` (with and without `berkeley-db`).

For both (with and without `berkeley-db`), `bitcoind-29.0` is much slower than `bitcoin-28.0` which gives me problems on my server.

To investigate I wrote a shell script that determines the time difference in seconds between "Saw new header" and "UpdateTip":

```
2025-05-09T05:24:11Z Saw new header hash=000000000000
...

💬 fanquake commented on issue "Self-compiled bitcoind 29.0 much slower than self-compiled 28.0 on my system":
(https://github.com/bitcoin/bitcoin/issues/32455#issuecomment-2865331250)
> So my first question before I investigate any further is: how come my bitcoind is so big compared to the pre-compiled one?

This will be the debug symbols. If you run 'strip' on the binaries. i.e 'strip bitcoind' this debug information will be removed, and the size of your binaries should shrink 80-90%.

💬 0xB10C commented on pull request "tracing: fix invalid argument in mempool_monitor":
(https://github.com/bitcoin/bitcoin/pull/32454#issuecomment-2865366915)
Code Review ACK 31c5ebc4007884b655f2f90ca09e36e0b9ada4da

Seems like I introduced that bug in https://github.com/bitcoin/bitcoin/pull/31848 (ec47ba349d0b3cb2d274593ca7b828ae70584e10).

💬 polespinasa commented on pull request "tests: Expand HTTP coverage to assert libevent behavior":
(https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2081079062)
I'm not sure about this but shouldn't this be `socket.timeout`? As we're using `sock.recv` and not `getresponse()`?

🤔 polespinasa reviewed a pull request: "tests: Expand HTTP coverage to assert libevent behavior"
(https://github.com/bitcoin/bitcoin/pull/32408#pullrequestreview-2827278739)
tACK 840dd5b6eb0b2c1f35a91c36acac5d97933172dc

Maybe the three commits could be squashed?

💬 polespinasa commented on pull request "tests: Expand HTTP coverage to assert libevent behavior":
(https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2081097699)
If the timeout is set to 2 wouldn't make more sense to test between 2 and 4 seconds? Can it be less than 2 seconds?

💬 hMsats commented on issue "Self-compiled bitcoind 29.0 much slower than self-compiled 28.0 on my system":
(https://github.com/bitcoin/bitcoin/issues/32455#issuecomment-2865489117)
@fanquake Thanks a lot! I expected something like this (debug info) but thought it was some compiler option. The `strip` worked and now my bitcoind is only 12 MB 👍. As I never used `strip` on `bitcoin-28.0` I don't expect this to make a difference but will try it first on my server anyway or could it?

This is very confusing. I'm a long time bitcoin node runner and linux user but would never have thought of this possibility. **Shouldn't this be mentioned in the `doc/build-unix.md`?**