🤔 rkrux requested changes to a pull request: "test: refactor mempool_accept_wtxid"
(https://github.com/bitcoin/bitcoin/pull/33067#pullrequestreview-3515035765)
Thanks for addressing the comments earlier.

> This PR improves mempool_accept_wtxid.py by using a pre-mined chain instead of generating new blocks, switching to MiniWallet to avoid RPC calls for signing transactions

I haven't timed the test yet but if some test latency improvement has been noticed (ref: https://github.com/bitcoin/bitcoin/pull/33067#pullrequestreview-3079794265), then it would be helpful to highlight it in the PR description imo.

💬 rkrux commented on pull request "test: refactor mempool_accept_wtxid":
(https://github.com/bitcoin/bitcoin/pull/33067#discussion_r2568435169)
Nit: It would be nice to calculate these amounts based on the wallet balance or the utxo being spent by the wallet instead of hardcoding that relies on some MiniWallet assumptions.

💬 rkrux commented on pull request "test: refactor mempool_accept_wtxid":
(https://github.com/bitcoin/bitcoin/pull/33067#discussion_r2568423936)
I feel there is scope for generalising this function further. Similar to how the previous class implementation used only the `CTransaction` class and the like.

Currently, this function is tied to the `MiniWallet` that I don't think is necessary. A more generic form of this function should be able to be used by the usual RPC wallets as well.

Consider the following diff that puts the responsibility of building the base parent transaction and submitting the final parent transaction to the caller
...

💬 fanquake commented on issue "guix: re-enable exported symbol checking for RISC-V":
(https://github.com/bitcoin/bitcoin/issues/28095#issuecomment-3585628172)
While there hasn't been a response to our upstream issue: https://sourceware.org/bugzilla/show_bug.cgi?id=32783, the test case seems to be improved as of binutils 2.45. I can recreate the issue using `2.44`. i.e:
```bash
# riscv64-linux-gnu-ld --version
GNU ld (GNU Binutils for Debian) 2.44
# riscv64-linux-gnu-g++ test.cpp -o test.rv -Wl,--exclude-libs,ALL -fvisibility=hidden -static-libstdc++
# riscv64-linux-gnu-objdump -T test.rv|grep "\.text"
0000000000016820 l d .text 0000000000000000
...

💬 maflcko commented on pull request "log: Use more severe log level (warn/err) where appropriate":
(https://github.com/bitcoin/bitcoin/pull/33960#discussion_r2568464699)
hmm, probably should be an `Assume(false)`, but I only want to change the logs here, not the other behavior. It is dead code either way.

💬 maflcko commented on pull request "log: Use more severe log level (warn/err) where appropriate":
(https://github.com/bitcoin/bitcoin/pull/33960#discussion_r2568466473)
hmm, probably should be an `Assume(false)`, but I only want to change the logs here, not the other behavior. it is dead code either way.

💬 maflcko commented on pull request "log: Use more severe log level (warn/err) where appropriate":
(https://github.com/bitcoin/bitcoin/pull/33960#discussion_r2568466984)
I don't think the node shuts down here (fatal error). Also, this is dead code, so I don't think it matters much.

💬 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.