💬 brunoerg commented on pull request "test: wallet: cover wallet passphrase with a null char":
(https://github.com/bitcoin/bitcoin/pull/32675#discussion_r2132345210)
Same. And maybe out of scope, will see.

💬 brunoerg commented on pull request "test: wallet: cover wallet passphrase with a null char":
(https://github.com/bitcoin/bitcoin/pull/32675#discussion_r2132345397)
Same

📝 josibake opened a pull request: "depends: fix cmake compatibility error for freetype"
(https://github.com/bitcoin/bitcoin/pull/32693)
## Problem

While doing a depends build with CMake 4.0.1, I got the following error:

```
Extracting freetype...
/root/bitcoin/depends/sources/freetype-2.11.0.tar.xz: OK
Preprocessing freetype...
Configuring freetype...
CMake Error at CMakeLists.txt:100 (cmake_minimum_required):
Compatibility with CMake < 3.5 has been removed from CMake.

Update the VERSION argument <min> value. Or, use the <min>...<max> syntax
to tell CMake that the project requires at least <min> but has be
...

💬 hebasto commented on pull request "depends: fix multiprocess build on OpenBSD (apply capnp patch, correct SHA256SUM command)":
(https://github.com/bitcoin/bitcoin/pull/32690#discussion_r2132380254)
> `build_freebsd_SHA256SUM = sha256sum` already outputs the format we need.

```sh
$ freebsd-version
14.2-RELEASE
$ sha256 ~/.cshrc
SHA256 (/home/hebasto/.cshrc) = 523d96d41067f380469d4f3d68b540edb63b127e91a9955fb7f6a01c53545258
```

💬 PeterWrighten commented on pull request "wallet: Allow read-only database access for info and dump commands":
(https://github.com/bitcoin/bitcoin/pull/32685#issuecomment-2949576459)
Review request: @achow101 @furszy @ryanofsky @Sjors

💬 hebasto commented on issue "depends: OpenBSD (aarch64) needs gcc (instruction) for libevent":
(https://github.com/bitcoin/bitcoin/issues/32691#issuecomment-2949584140)
> I spun up an equivalent aarch64 VM. I installed `bash gmake gtar` and `cmake` (and `git` to clone the repo). Ran into the same error as with UTM.
>
> So it seems to be an arm thing?

Seems like a bug in CMake's `CMakeDetermineCCompiler` module.

💬 mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2132394370)
done

💬 mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2132394658)
done

💬 mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2132395636)
changed to "because" - it was meant to imply a causality between the sentences.

💬 mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#issuecomment-2949593753)
[fdcdc2c](https://github.com/bitcoin/bitcoin/commit/fdcdc2cdeadb5d36076f1b94b54261e22e031354) to [76c835f](https://github.com/bitcoin/bitcoin/commit/76c835f77205d04c4206acad1a8a636562c22f79): addressed nits

💬 hebasto commented on issue "depends: OpenBSD (aarch64) needs gcc (instruction) for libevent":
(https://github.com/bitcoin/bitcoin/issues/32691#issuecomment-2949596188)
@Sjors

What are outputs of the `printenv` and `which cc` commands on your OpenBSD aarch64?

💬 Sjors commented on issue "depends: OpenBSD (aarch64) needs gcc (instruction) for libevent":
(https://github.com/bitcoin/bitcoin/issues/32691#issuecomment-2949608234)
```
_=/usr/bin/printenv
LOGNAME=root
PWD=/root
HOME=/root
SSH_TTY=/dev/ttyp0
MAIL=/var/mail/root
SSH_CLIENT=******** 10013 22
PATH=/sbin:/usr/sbin:/bin:/usr/bin:/usr/X11R6/bin:/usr/local/sbin:/usr/local/bin
TERM=xterm-256color
SHELL=/bin/ksh
SSH_CONNECTION=*************
USER=root
```

```
which cc
/usr/bin/cc
```

💬 kevkevinpal commented on pull request "test: added fuzz coverage for consensus/merkle.cpp":
(https://github.com/bitcoin/bitcoin/pull/32243#discussion_r2132408562)
Thanks! That makes sense, updated in [95969bc](https://github.com/bitcoin/bitcoin/pull/32243/commits/95969bc58ae0cd928e536d7cb8541de93e8c7205)

💬 Sjors commented on pull request "depends: fix multiprocess build on OpenBSD (apply capnp patch, correct SHA256SUM command)":
(https://github.com/bitcoin/bitcoin/pull/32690#discussion_r2132409537)
`sha256sum` != `sha256` != `shasum -a 256`

👍 ryanofsky approved a pull request: "validation: stricter internal handling of invalid blocks"
(https://github.com/bitcoin/bitcoin/pull/31405#pullrequestreview-2904963797)
Code review ACK fdcdc2cdeadb5d36076f1b94b54261e22e031354. Looks good, thanks for many new clarifications! Only changes since last review were switching from a CBlockIndexWorkComparator to a nChainWork comparison, switching from InvalidChainFound to InvalidBlockFound without changing behavior, and improving many comment. I left more suggestions below which are not important and all only about comments.

💬 ryanofsky commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2132355311)
In commit "validation: in invalidateblock, calculate m_best_header right away" (809413fde6893e60078e02b401a9d98a3e341fc1)

Commit message says "Before, m_best_header would be calculated only after disconnecting multiple blocks" but I'm actually not seeing where this was happening, though I think it must have been happening?

Could help if commit message was clarified (or not if I'm just missing something obvious).

💬 ryanofsky commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2132281266)
In commit "validation: in invalidateblock, mark children as invalid right away" (328345d571d9b86af54e915b30183b91d28cab2f)

Would seem more accurate to say this code is marking out-of-chain descendants of the "disconnected block" rather than the "invalidated block" since the invalidated block sounds like the block passed to invalidateblock

💬 ryanofsky commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2132302141)
In commit "validation: in invalidateblock, calculate m_best_header right away" (809413fde6893e60078e02b401a9d98a3e341fc1)

Maybe replace "but there may be better ones" with "but there could be a better header, which will be found in the loop below", because current comment seems to imply that the code will not actually find the best header.

💬 ryanofsky commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2132334798)
In commit "validation: in invalidateblock, calculate m_best_header right away" (809413fde6893e60078e02b401a9d98a3e341fc1)

It seems like it could be better to use CBlockIndexWorkComparator here instead of relying highpow_outofchain_headers multimap ordering when choosing m_best_header and two headers have the same mount of work because CBlockIndexWorkComparator should be more deterministic and consistent with surrounding code, while map order will depend on iteration order of the `m_block_inde
...

💬 ryanofsky commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2132373845)
In commit "doc: Improve m_best_header documentation" (fdcdc2cdeadb5d36076f1b94b54261e22e031354)

It seems like it would be nice to use CBlockIndexWorkComparator consistently for comparisons, but since it's probably beyond scope of this PR, it's good to have this documentation comment mentioning the lack of consistency.

Just in case its useful for future reference, the post describing the current situation in the most detail is https://github.com/bitcoin/bitcoin/pull/31405#discussion_r212978
...