💬 maflcko commented on pull request "log: Use more severe log level (warn/err) where appropriate":
(https://github.com/bitcoin/bitcoin/pull/33960#discussion_r2568467420)
Sure, but the node does not shut down (fatal error)

💬 maflcko commented on pull request "log: Use more severe log level (warn/err) where appropriate":
(https://github.com/bitcoin/bitcoin/pull/33960#discussion_r2568474903)
thx, fixed (they are all errors and shut down the node)

💬 maflcko commented on pull request "log: Use more severe log level (warn/err) where appropriate":
(https://github.com/bitcoin/bitcoin/pull/33960#issuecomment-3585650768)
thx, fixed all, except for those that are dead code anyway (left a reply there in the review thread)

💬 rkrux commented on pull request "log: Use more severe log level (warn/err) where appropriate":
(https://github.com/bitcoin/bitcoin/pull/33960#issuecomment-3585706616)
Maybe can also mention this benefit in the PR description that `LogPrintf` was unconditional logging that was deprecated while `LogError` and `LogWarning` use basic rate limiting to mitigate disk filling attacks.

https://github.com/bitcoin/bitcoin/blob/38c8474d0d774b1ed5d6139a9fec9933a6cfc099/src/logging.h#L364-L373

💬 maflcko commented on pull request "log: Use more severe log level (warn/err) where appropriate":
(https://github.com/bitcoin/bitcoin/pull/33960#issuecomment-3585776972)
> Maybe can also mention this benefit in the PR description that `LogPrintf` was unconditional logging that was deprecated while `LogError` and `LogWarning` use basic rate limiting to mitigate disk filling attacks.

I don't think this is correct. You can see from the code you quoted that the legacy one is just an alias for the one "modern" one. Also, all three have `/*should_ratelimit=*/true`.

👍 sedited approved a pull request: "Policy: Report debug message why inputs are non standard"
(https://github.com/bitcoin/bitcoin/pull/29060#pullrequestreview-3515295631)
ACK 9eea72d3f3647197c24329b412c7fc71895e3ea2

💬 rkrux commented on pull request "log: Use more severe log level (warn/err) where appropriate":
(https://github.com/bitcoin/bitcoin/pull/33960#issuecomment-3585822978)
Ah, you are correct. I got carried away by the comment and assumed the code is like below (`LogPrintLevel` instead of `LogInfo`):

```cpp
// Deprecated unconditional logging.
#define LogPrintf(...) LogPrintLevel_(__VA_ARGS__)
```

💬 l0rinc commented on pull request "merkle: pre‑reserve leaves to prevent reallocs with odd vtx count":
(https://github.com/bitcoin/bitcoin/pull/32497#discussion_r2568873272)
You're right, it needs more explanation:
I could change this to use a more modern style, but it would be different from the code that it's trying to measure, namely:
https://github.com/bitcoin/bitcoin/blob/9c24cda72edb2085edfa75296d6b42fab34433d9/src/consensus/merkle.cpp#L81
It's a very tiny difference, but I usually prefer the benchmark being representative of the code so that it's grep-friendly (in case we modify one and search for similar code)

💬 l0rinc commented on pull request "log: Use more severe log level (warn/err) where appropriate":
(https://github.com/bitcoin/bitcoin/pull/33960#issuecomment-3586103150)
Code review ACK fa45a1503eee603059166071857215ea9bd7242a

💬 littledeb77-wq commented on pull request "merkle: pre‑reserve leaves to prevent reallocs with odd vtx count":
(https://github.com/bitcoin/bitcoin/pull/32497#issuecomment-3586131853)
t

On Thcga!6;-u, Nov 27, 2025 at 7:31 AM optout ***@***.***>
wrote:

> ***@***.**** commented on this 45pull request.
> ------------------------------
>
> In src/xxxxxx cbench/merkle_root.cpp
> <https://github.com/bitcoin/bitcoin/pull/32497#discussion_r2568419746>:
>
> > item = rng.rand256();
> }
> - bench.batch(leaves.size()).unit("leaf").run([&] {
> - bool mutation = false;
> - uint256 hash = ComputeMerkleRoot(std::vector<uint256>(leaves), &mutati
...

💬 m3dwards commented on pull request "depends, doc: Learn `x86_64-w64-mingw32ucrt` host and document it":
(https://github.com/bitcoin/bitcoin/pull/33857#discussion_r2568947262)
I don't think this check is strictly necessary?

```shell
root:/bitcoin# host=x86_64-w64-mingw32
root:/bitcoin# command -v $host-gcc-posix
/usr/bin/x86_64-w64-mingw32-gcc-posix
root:/bitcoin# host=x86_64-w64-mingw32ucrt
root:/bitcoin# command -v $host-gcc-posix
root:/bitcoin#
```

🚀 fanquake merged a pull request: "ci: clear out space on CentOS, depends, gui GHA job"
(https://github.com/bitcoin/bitcoin/pull/33514)

💬 maflcko commented on pull request "Policy: Report debug message why inputs are non standard":
(https://github.com/bitcoin/bitcoin/pull/29060#discussion_r2568995848)
i don't think this is correct. the sigops loop breaks as soon as the limit is overshot. However, the real sigops number can be anything larger than that. I don't think there is any value in reporting a wrong value.

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2569042469)
Done

💬 maflcko commented on pull request "transaction: Adding script witness to ToString for CTxIn":
(https://github.com/bitcoin/bitcoin/pull/33711#discussion_r2568775620)
pls use named args and snake_case:


```suggestion
str += " " + tx_in.ToString(/*include_witness=*/false) + "\n";
```

💬 fanquake commented on pull request "contrib: turn off compression of macOS SDK to fix determinism (across distros)":
(https://github.com/bitcoin/bitcoin/pull/32009#issuecomment-3586302030)
Guix Build (x86_64):
```bash
cc72adc0e567f33605050a98cd7cd08db2b0c7da1aa11dd5f73b9077653b5e00 guix-build-3e01b5d0e7be/output/aarch64-linux-gnu/SHA256SUMS.part
15d23cda22ba4afceddb733820893e4ef3a5307717002403cdc5e66d3561bafb guix-build-3e01b5d0e7be/output/aarch64-linux-gnu/bitcoin-3e01b5d0e7be-aarch64-linux-gnu-debug.tar.gz
2d0e852259c802ea3c2411a38b93148dcaa2415b5d5d34d1f6362df39108e718 guix-build-3e01b5d0e7be/output/aarch64-linux-gnu/bitcoin-3e01b5d0e7be-aarch64-linux-gnu.tar.gz
b9c4572
...

🤔 hodlinator reviewed a pull request: "merkle: pre‑reserve leaves to prevent reallocs with odd vtx count"
(https://github.com/bitcoin/bitcoin/pull/32497#pullrequestreview-3515742822)
Reviewed e3b02f1539e59a6a20a00b4161219b5aba6fefed

Thanks for reverting to a simpler version of this change!

2 inline questions.

💬 hodlinator commented on pull request "merkle: pre‑reserve leaves to prevent reallocs with odd vtx count":
(https://github.com/bitcoin/bitcoin/pull/32497#discussion_r2568982833)
in 45dc62f0edf71d52ae6b024dec6b11746e3c4076 "refactor: make `MerkleRoot` benchmark more representative": Sending in the `&mutation` parameter makes the function perform more work (somewhat quadratically?). Are we sure we want to drop that?

💬 hodlinator commented on pull request "merkle: pre‑reserve leaves to prevent reallocs with odd vtx count":
(https://github.com/bitcoin/bitcoin/pull/32497#discussion_r2569096555)
`(size + 1) & -2` eh? :)

I'm happy with `(size + 1) & ~1ULL`. It should narrow down to 4-byte int on 32-bit.

Diffed `ull_literal` vs `decltype_trick` for x86-64 and ARM32 and they are the same. :+1:

💬 hodlinator commented on pull request "merkle: pre‑reserve leaves to prevent reallocs with odd vtx count":
(https://github.com/bitcoin/bitcoin/pull/32497#discussion_r2569010822)
nit in 6c2d181bdc09024c5de5e29d9e913fcc2dba5071 "test: adjust `ComputeMerkleRoot` tests": Why extract `vtx_count` when it's only used once?