💬 ajtowns commented on pull request "net: Provide block templates to peers on request":
(https://github.com/bitcoin/bitcoin/pull/33191#discussion_r2521976766)
No, but only in the same sense that it's not possible to split an 8MWU template into two <= 4MWU blocks, so the fact that with that change you couldn't include 3x 2.6 MWU txns seems more of a feature than a bug.

👍 rkrux approved a pull request: "scripted-diff: fix leftover references to `policy/fees.h`"
(https://github.com/bitcoin/bitcoin/pull/33864#pullrequestreview-3457992495)
lgtm ACK b0a38871546dfcdd3a578c1ae4c28a88b6ee32d5

💬 maflcko commented on pull request "kernel: allow null data_directory":
(https://github.com/bitcoin/bitcoin/pull/33867#discussion_r2522131622)
I don't think this change is right. The path can not be null or empty, because that will fail later on with:

```
unknown location(0): fatal error: in "fs_tests/fsbridge_stem": std::filesystem::__cxx11::filesystem_error: filesystem error: cannot make absolute path: Invalid argument []

💬 alexanderwiederin commented on pull request "kernel: add btck_block_tree_entry_equals":
(https://github.com/bitcoin/bitcoin/pull/33855#issuecomment-3526237850)
@stickies-v, what is the _real_ benefit over a hash-based equality check?

I wonder if we can avoid committing to a pointer-based check here given it's a non-trivial decision that is hard to reverse. The performance gain is negligible from what I understand.

What do you think?

💬 maflcko commented on pull request "kernel: Allow null arguments for serialized data":
(https://github.com/bitcoin/bitcoin/pull/33853#discussion_r2522177400)
> How about keeping the checking, but testing it without going through the c++ wrapper?

Sure, that'll be my suggestion above: "Side-step the C++ wrapper and directly call the C code with a raw C-span from this test."

Anything should be fine, and the only problem I see with that would be that we won't be able to enable hardened C in the future. What can be achieved with `std::span` and library hardening in C++, can be achieved in C via `__counted_by` and `-fbounds-safety`.

📝 maflcko opened a pull request: "refactor: Avoid -W*-whitespace in git archive"
(https://github.com/bitcoin/bitcoin/pull/33869)
Otherwise, compilation with GCC-15+ will warn about it:

```
src/clientversion.cpp:33:79: error: trailing whitespace [-Werror=trailing-whitespace=]
33 | //! git will put "#define GIT_COMMIT_ID ..." on the next line inside archives.
```

Follow-up to https://github.com/bitcoin/bitcoin/pull/32482#issuecomment-3522280482

Can be tested via `git archive --output=/tmp/a.tar HEAD`

💬 maflcko commented on pull request "miner: drop dummy extraNonce in coinbase scriptSig for templates requested via IPC":
(https://github.com/bitcoin/bitcoin/pull/32420#discussion_r2522490088)

/**\n+ * Whether to include and OP_0 as a dummy extraNonce in the template's coinbase\n+ */\n -> "and" -> "an" [Correct article: "an OP_0" (singular noun) makes the sentence grammatical and clear.]

💬 maflcko commented on pull request "refactor: Improve assumeutxo state representation":
(https://github.com/bitcoin/bitcoin/pull/30214#discussion_r2435927466)
llm nits:


assumetxo -> assumeutxo [spelling mistake in comments; inconsistent with the rest of the code/documentation]
assumetxo -> assumeutxo [same misspelling repeated elsewhere in the added enum comments]
contains database -> contains a database [missing article makes the sentence ungrammatical / harder to parse]

💬 maflcko commented on pull request "tests: Add witness commitment if we have a witness transaction in `FullBlockTest.update_block()`":
(https://github.com/bitcoin/bitcoin/pull/31823#discussion_r2522534594)

doesnt -> doesn’t [missing apostrophe in contraction “doesn't”]

✅ maflcko closed a pull request: "tests: Add witness commitment if we have a witness transaction in `FullBlockTest.update_block()`"
(https://github.com/bitcoin/bitcoin/pull/31823)

📝 maflcko reopened a pull request: "tests: Add witness commitment if we have a witness transaction in `FullBlockTest.update_block()`"
(https://github.com/bitcoin/bitcoin/pull/31823)
This is useful for test cases where we want to test logic invalid blocks that contain witness transactions. If we don't add the witness commitment as per [BIP141](https://github.com/bitcoin/bips/blob/master/bip-0141.mediawiki#user-content-Commitment_structure), blocks will be rejected with the error [`Block mutated`](https://github.com/bitcoin/bitcoin/blob/fb0ada982a73687520c43b8fde480fa5d456f3e1/src/validation.cpp#L4180).

This change was needed in https://github.com/ajtowns/bitcoin/pull/13 w
...

💬 fanquake commented on pull request "refactor: Avoid -W*-whitespace in git archive":
(https://github.com/bitcoin/bitcoin/pull/33869#issuecomment-3526740723)
Should we modify one job to use the archive? We should probably be catching, and minimising any differences, like this, between building from source, and the tarball.

💬 maflcko commented on pull request "refactor: Avoid -W*-whitespace in git archive":
(https://github.com/bitcoin/bitcoin/pull/33869#issuecomment-3526775978)
> Should we modify one job to use the archive?

Possibly, but I don't think any job is running with gcc-15, so the benefits here would be limited. Though, I can modify my nightly matrix to use archives explicitly, instead of by accident of git not being present.



> We should probably be catching, and minimising any differences, like this, between building from source, and the tarball.

I want to keep the changes here minimal. Reworking this is already tracked in https://github.com/bitc
...

💬 m3dwards commented on pull request "ci: Lint follow-ups":
(https://github.com/bitcoin/bitcoin/pull/33776#discussion_r2522600040)
Missing import time?

💬 m3dwards commented on pull request "ci: Lint follow-ups":
(https://github.com/bitcoin/bitcoin/pull/33776#discussion_r2522613366)
Should be `build_cmd`

💬 vasild commented on pull request "net_processing: rename RelayTransaction to better describe what it does":
(https://github.com/bitcoin/bitcoin/pull/33565#issuecomment-3526831112)
`9989853447...78595ae0e7`: take suggestions

✅ fanquake closed an issue: "fees: leftover references to `policy/fees.cpp`"
(https://github.com/bitcoin/bitcoin/issues/33863)

🚀 fanquake merged a pull request: "scripted-diff: fix leftover references to `policy/fees.h`"
(https://github.com/bitcoin/bitcoin/pull/33864)

💬 vasild commented on pull request "net_processing: rename RelayTransaction to better describe what it does":
(https://github.com/bitcoin/bitcoin/pull/33565#discussion_r2522652086)
This slipped through, thanks for the reminder. Done now and also took the changes into https://github.com/bitcoin/bitcoin/pull/29415

💬 vasild commented on pull request "net_processing: rename RelayTransaction to better describe what it does":
(https://github.com/bitcoin/bitcoin/pull/33565#discussion_r2522653126)
Done

💬 vasild commented on pull request "net_processing: rename RelayTransaction to better describe what it does":
(https://github.com/bitcoin/bitcoin/pull/33565#discussion_r2522654868)
Done