💬 l0rinc commented on pull request "crypto: optimize SipHash Write() method with chunked processing":
(https://github.com/bitcoin/bitcoin/pull/33325#issuecomment-3305097747)
@luke-jr where is this general writer needed? We already have separate specialized siphash implementation for the critical usecases...

💬 l0rinc commented on pull request "RFC: blocks: add `-reobfuscate-blocks` arg to xor existing blk/rev on startup":
(https://github.com/bitcoin/bitcoin/pull/33324#issuecomment-3305103297)
> I agree a separate utility for this seems better

Can you quote what you're agreeing with specifically, not sure who suggested that.
Besides, @andrewtoth already has a tool for that, it was mentioned in the PR description.

> but in a new directory, and then atomically rename

I will think about it, could make sense, but in that case unrelated files should also be copied over (maybe duplicated to be safe) - and listing the directory content wouldn't make the progress obvious. What's wr
...

💬 HowHsu commented on pull request "index: remove unnecessary locator cleaning in BaseIndex::Init()":
(https://github.com/bitcoin/bitcoin/pull/32882#issuecomment-3305245694)
> ACK [facd01e](https://github.com/bitcoin/bitcoin/commit/facd01e6ffbbd019312f370a3807de0b95bbd745)
>
> Note that we don't usually add the `Signed off by` to the commit message, it just seems like noise to me. You can still sign your commits if you want, but I'm no sure what this line adds that isn't already obvious.
>

Added by my git config automatically, that is a kind of DCO for GPLv2, I'll remove it in later PRs since Bitcoin Core is MIT, so yes, it is not necessary to Sign.

> Als
...

💬 luke-jr commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2357526353)
Should we be logging which chainstate this is, in case there's multiple?

💬 l0rinc commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2357539686)
Currently we're only logging this if there's a single one, i.e. `GetRole() == ChainstateRole::NORMAL`

💬 luke-jr commented on pull request "help: enrich help text for `-loadblock`":
(https://github.com/bitcoin/bitcoin/pull/33343#issuecomment-3305484748)
I think it would make sense to support XOR for loadblock if we're assuming the chain has data that can trigger bad things

💬 kannapoix commented on pull request "doc: update multisig tutorial to use multipath descriptors":
(https://github.com/bitcoin/bitcoin/pull/33286#discussion_r2357586680)
This command may result in an error like:
```
error code: -8
error message:
Unknown named parameter cHNidP8BAH0CAAAAASMcBn5VJaIET6lL4b76V676ffyrvpxT6UI++gY14LwWAQAAAAD9////AoAaBgAAAAAAFgAUhv4Mbo6zEszAaF4zBry18DXCw+CuhQEAAAAAACIAIKUhPbGKzw2gMuSI/kFtVa5EYyTBZdlS7zeuyJg5kJ2kAAAAAAABAH0CAAAAARULOYOh2W+IbDULcj5n6NF4uzQx8ZcqXa+YrNgx+Y/XmwMAAAD9////AqQGAAAAAAAAFgAUvI6wMmXzdKiS7/nhen+pYfbaY8MgoQcAAAAAACIAILvTfOv022A80QUkWRjFA2e0RkHffMZf5J8TeDWo3ajRIB8EAAEBKyChBwAAAAAAIgAgu9N86/TbYDzRBSRZGMUDZ7RGQd98xl/k
...

💬 luke-jr commented on pull request "log: show reindex progress in `ImportBlocks`":
(https://github.com/bitcoin/bitcoin/pull/33353#discussion_r2357830141)
Files are not necessarily the same size/complexity...

💬 hebasto commented on pull request "cmake: Inherit WERROR setting for secp256k1 build":
(https://github.com/bitcoin/bitcoin/pull/33297#discussion_r2357851661)
Why not reusing the `working_compiler_werror_flag` variable?

💬 frankomosh commented on pull request "p2p: add `DifferenceFormatter` fuzz target and invariant check":
(https://github.com/bitcoin/bitcoin/pull/33252#discussion_r2357861031)
Thanks for the review @achow101

I agree it could be done directly in deserialization. The reason it’s in net_processing is that it acts as a boundary for untrusted peer input, so having the invariant explicit here serves as a defense-in-depth check (similar to other message level sanity checks here).
The extra loop is only executed after we already know the message is GETBLOCKTXN and after successful deserialization, so it’s a one-time O(n) pass per compact block request. Given the bounded
...

💬 dergoegge commented on pull request "p2p: add `DifferenceFormatter` fuzz target and invariant check":
(https://github.com/bitcoin/bitcoin/pull/33252#discussion_r2357909155)
Since this is using Assume, the loop shouldn't exist in the release binaries (i.e the compiler optimises it out), so the overhead doesn't exist.

📝 hodlinator opened a pull request: "build: Clean lingering Windows registry & shortcuts (#32132 follow-up)"
(https://github.com/bitcoin/bitcoin/pull/33422)
Prior to fb2b05b1259d3e69e6e675adfa30b429424c7625 / #32132 we installed using these paths. The lingering registry entries for the uninstaller would show up as "Bitcoin Core (64-bit)" in the list of installed programs and fail to work.

Disclaimer: not an NSIS expert - confirmed that added commands work both when items exist and when they don't.

💬 hodlinator commented on pull request "build: Remove bitness suffix from Windows installer":
(https://github.com/bitcoin/bitcoin/pull/32132#issuecomment-3305995368)
My testing shows #33422 does the job. I'd say we should have it in the v30 release or we should do the less risky thing and revert this PR (#32132) from v30.

💬 Sjors commented on pull request "wallet: warn against accidental unsafe older() import":
(https://github.com/bitcoin/bitcoin/pull/33135#discussion_r2357988037)
The Tidy CI job runs IWYU and doesn't complain: https://github.com/bitcoin/bitcoin/actions/runs/17605378583/job/50015040929?pr=33135#step:9:6218

💬 Sjors commented on pull request "wallet: warn against accidental unsafe older() import":
(https://github.com/bitcoin/bitcoin/pull/33135#discussion_r2357991533)
I went for a slight variant.

💬 Sjors commented on pull request "doc: update multisig tutorial to use multipath descriptors":
(https://github.com/bitcoin/bitcoin/pull/33286#discussion_r2358007567)
That might be a case of #32821. I would just wait until that's merged.

📝 hodlinator opened a pull request: "qa: Improvements to debug_assert_log + busy_wait_for_debug_log"
(https://github.com/bitcoin/bitcoin/pull/33423)
* Only compute the log string to be printed on failure *when we actually fail* instead of every 0.05s.
* As we find each needle (expected message) in the haystack (log output), stop searching for it. If we fail and time out, we will only complain about the needles (expected messages) we didn't find.

Found while developing a new test case in https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2351892330

💬 willcl-ark commented on pull request "ci: Skip Centos job on GHA runners":
(https://github.com/bitcoin/bitcoin/pull/33365#issuecomment-3306304341)
> Looks fine. I don't have strong feelings, and wouldn't NACK this, but my preference leans more towards having things still run on forks on GHA runners as it does for the main org albeit slower. Also has the benefit of having a bit less vendor lock in.

I think if I was to e.g. append `|| true` to the commands in https://github.com/willcl-ark/bitcoin/blob/f5885d05160fe0ac167976f2b36b6d5bec373e08/.github/actions/clear-files/action.yml to make it "fail-proof" (i.e. don't error on failures) then
...

💬 fanquake commented on pull request "[28.x] More backports":
(https://github.com/bitcoin/bitcoin/pull/33415#issuecomment-3306477254)
> Here is a branch to do this: https://github.com/glozow/bitcoin/tree/2025-09-28.3-backport

Thanks. Looks like there are some issues with this branch. i.e:
```bash
test/mempool_tests.cpp:434: Entering test case "MempoolSizeLimitTest"
<snip>
test/mempool_tests.cpp:562: error: in "mempool_tests/MempoolSizeLimitTest": check pool.GetMinFee(1).GetFeePerK() == maxFeeRateRemoved.GetFeePerK() + 1000 has failed [19941 != 20841]
2025-09-18T09:27:52.459923Z (mocktime: 1970-01-01T12:00:42Z) [test] [
...

👋 fanquake's pull request is ready for review: "ci: re-add Valgrind job to the CI"
(https://github.com/bitcoin/bitcoin/pull/33411)