💬 mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2075937910)
I think that this wouldn't be correct: We use `CBlockIndexWorkComparator` during normal adding to `setBlockIndexCandidates` ([here](https://github.com/bitcoin/bitcoin/blob/baa848b8d38187ce6b24a57cfadf1ea180209711/src/validation.cpp#L3840C41-L3840C79)) and also use it while asserting that blocks that sort worse are not in the set [here](https://github.com/bitcoin/bitcoin/blob/baa848b8d38187ce6b24a57cfadf1ea180209711/src/validation.cpp#L5412). If we'd insert blocks in the set that have the same `n
...

💬 maflcko commented on issue "Intermittent timeout in tsan feature_init.py":
(https://github.com/bitcoin/bitcoin/issues/30586#issuecomment-2855360892)
Not sure why CI runs into this so rarely. This reproduces easily locally on a fresh install of Ubuntu 25.04 Plucky:

```
1 apt update && apt install -y git ccache build-essential cmake pkg-config python3-zmq libevent-dev libboost-dev qt6-tools-dev qt6-l10n-tools qt6-base-dev systemtap-sdt-dev libqrencode-dev libzmq3-dev libsqlite3-dev
2 git clone https://github.com/bitcoin/bitcoin --depth=1 ./bitcoin-core
3 cd bitcoin-core/
4 apt install -y libc++abi-dev libc++-dev clang ll
...

✅ fanquake closed an issue: "gui: translation spam?"
(https://github.com/bitcoin/bitcoin/issues/32295)

💬 fanquake commented on issue "gui: translation spam?":
(https://github.com/bitcoin/bitcoin/issues/32295#issuecomment-2855376554)
Closing for now given #32352.

💬 mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2075959357)
> Actually, it looks like https://github.com/bitcoin/bitcoin/commit/79bef1bc52afab860782d6d9a6016b93f94c4503 is still here

oops, fixed

> releasing cs_main so another thread can change the BI

I wasn't suggesting this, the reason why we are releasing cs_main frequently during invalidateblock in the first place is so that the node/GUI doesn't become unresponsive in case many blocks in a row are disconnected.

In the past, there was a problem with the intermediate state in `InvalidateBloc
...

💬 mzumsande commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2855430075)
> These limits are easily bypassed by both direct submission to miner mempools (e.g. MARA Slipstream)

💬 ryanofsky commented on issue "Depends toolchain doesn't contain enough info to build from depends on a fresh NixOS install":
(https://github.com/bitcoin/bitcoin/issues/32428#issuecomment-2855454246)
I can confirm Will's commit in 437a4da93f437f9588ff32d19ac59fea3802c676 fixes the problem for me and seems like a good solution. But I think it can be simplified to only just `list(APPEND CMAKE_PREFIX_PATH "${CMAKE_CURRENT_LIST_DIR}")` and the if() guard should be unnecessary.

I also confirmed the other fix appending `-DCMAKE_PREFIX_PATH=/` to the command line from https://github.com/hebasto/bitcoin/issues/121#issuecomment-2015274066 works. I guess this works because cmake appends CMAKE_FIND_RO
...

💬 maflcko commented on pull request "test: Suppress Windows abort message in CI":
(https://github.com/bitcoin/bitcoin/pull/32409#issuecomment-2855482369)
> Not related to this PR, but the [error in the `wallet_fees` target](https://github.com/hebasto/bitcoin-core-nightly/actions/runs/14801894543/job/41562322239#step:8:484):
>
> ```
> wallet_fees: succeeded against 23 files in 0s.
> Run wallet_fees with args ['D:\\a\\bitcoin-core-nightly\\bitcoin-core-nightly\\build\\bin\\Debug\\fuzz.exe', WindowsPath('D:/a/_temp/qa-assets/fuzz_corpora/wallet_fees')]
> \u26a0\ufe0f Failure generated from target with exit code 3221225477: ['D:\\a\\bitcoin-cor
...

💬 w0xlt commented on pull request "(RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store":
(https://github.com/bitcoin/bitcoin/pull/32427#issuecomment-2855495136)
Approach ACK

The codebase changes seem surprisingly small for this proposal.
Changing the code to reduce the dependency on LevelDB sounds good to me.

💬 davidgumberg commented on issue "CI: Cmake warnings should be errors":
(https://github.com/bitcoin/bitcoin/issues/31476#issuecomment-2855503431)
Is it possible that instead it is the case that we have some warnings which should in fact be errors? For example, having a python version below minimum? I'm unclear where people stand conceptually on this after #31593 was merged, fixing the python warning.

🤔 janb84 reviewed a pull request: "docs: clarify RPC credentials security boundary"
(https://github.com/bitcoin/bitcoin/pull/32424#pullrequestreview-2819238684)
ACK [ed86201](https://github.com/bitcoin/bitcoin/pull/32424/commits/ed862012f747d4e5248f08ff25183dc666c3de6e)

The updated documentation provides greater clarity on the security consequences of sharing RPC credentials.

💬 janb84 commented on pull request "docs: clarify RPC credentials security boundary":
(https://github.com/bitcoin/bitcoin/pull/32424#discussion_r2076031813)
```suggestion
the node and the same system privileges as the operating system user account running bitcoind. When
```
Small NIT, but fine to ignore

💬 mzumsande commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2855558032)
I prefer #32406.
While it won't change the outcome if some nodes (or even a moderate majority of nodes) don't accept these transactions due to the high connectivity of the p2p network, it does matter how many miners do.

> These limits are easily bypassed by both direct submission to miner mempools (e.g. MARA Slipstream)

MARA has 5% of the hashrate, and it often makes a difference how long the expected time to mining is - for usability and time-sensitive applications.
If it turns out that
...

💬 maflcko commented on pull request "cmake: Introduce `WITH_PYTHON` build option":
(https://github.com/bitcoin/bitcoin/pull/31669#issuecomment-2855566064)
lgtm ACK 5155dbd5f8202b7c54b45c53d3cea5e57e8e53ad

My nit about the warning isn't a blocker.

💬 maflcko commented on issue "CI: Cmake warnings should be errors":
(https://github.com/bitcoin/bitcoin/issues/31476#issuecomment-2855566246)
> Is it possible that instead it is the case that we have some warnings which should in fact be errors? For example, having a python version below minimum?

I think this was implemented in https://github.com/bitcoin/bitcoin/pull/31669, but for some reason it was closed.

💬 davidgumberg commented on pull request "docs: clarify RPC credentials security boundary":
(https://github.com/bitcoin/bitcoin/pull/32424#issuecomment-2855566965)
lgtm ACK https://github.com/bitcoin/bitcoin/pull/32424/commits/ed862012f747d4e5248f08ff25183dc666c3de6e

I opened #32274 because I wasn't sure if this status quo is desirable or not, but I believe this documentation note reflects the present expectations for `bitcoind` RPC servers.

💬 davidgumberg commented on pull request "build: Make config warnings fatal if -DWCONFIGURE_ERROR=ON":
(https://github.com/bitcoin/bitcoin/pull/31665#issuecomment-2855591507)
Rebased to fix merge conflict, this needs conceptual review, it seems to me the main question is: Should it be possible to make config warnings fatal? Or is it the case that any time you want to make a warning fatal, you have a warning that should probably always be an error?

🤔 jamesob reviewed a pull request: "policy: uncap datacarrier by default"
(https://github.com/bitcoin/bitcoin/pull/32406#pullrequestreview-2819295707)
Concept ACK

Leaving the knob in but changing its default is a good approach.

💬 jamesob commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2076064182)
In a vacuum it might be nice as a `std::optional<unsigned int>`, but that probably causes too much downstream churn.

💬 maflcko commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2076086158)
However, if some people prefer the option to not be deprecated (for whatever reason), it seems pretty minimal maintenance to keep it around without deprecation for now. There is no rush for this pull request, as the 30.x release is months away and also no rush to remove or deprecate the setting. Even if it is only for the extremely unlikely case to avoid having to undeprecate it later on (see also https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2855558032). The option has existed for
...