📝 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

💬 m3dwards commented on pull request "ci: Lint follow-ups":
(https://github.com/bitcoin/bitcoin/pull/33776#issuecomment-3526866633)
The GHA behaviour of removing annotations from previous runs is very annoying! I experimented with having an upstream job that printed the annotation and having all the other jobs depend on it but again when you rerun a failed downstream job you lose the annotation from the run upstream job.

It's a shame that in this PR we have to have the annotation printed many times, one per job, as I think it might drown out other notices. Without knowing exactly how this machine readable annotation is be
...

💬 TheCharlatan commented on pull request "refactor: Let CCoinsViewCache::BatchWrite return void":
(https://github.com/bitcoin/bitcoin/pull/33866#issuecomment-3526892334)
Updated 6d1818888b1e56dec6c57fadb3a99bfacbab742f -> d1823ebdd32d106668a3a8085da31e274d5c0ac1 ([batch_write_void_0](https://github.com/TheCharlatan/bitcoin/tree/batch_write_void_0) -> [batch_write_void_1](https://github.com/TheCharlatan/bitcoin/tree/batch_write_void_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/batch_write_void_0..batch_write_void_1))

* Addressed @andrewtoth's [comment](https://github.com/bitcoin/bitcoin/pull/33866#discussion_r2520746299), use `NextAndMaybeEras
...

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3526942592)
`875795ef0f...7547f915bc`: take suggestions from https://github.com/bitcoin/bitcoin/pull/33565

💬 fanquake commented on pull request "refactor: Avoid -W*-whitespace in git archive":
(https://github.com/bitcoin/bitcoin/pull/33869#issuecomment-3527012975)
> Possibly, but I don't think any job is running with gcc-15, so the benefits here would be limited.

I'm just thinking generally, not about this single warning, or gcc-15. Happy for it to best changed in a followup, or in another related change.

💬 fanquake commented on pull request "guix: build `bitcoin-qt` with static libxcb & utils":
(https://github.com/bitcoin/bitcoin/pull/33537#discussion_r2522793220)
Fixed.

💬 stickies-v commented on pull request "kernel: add btck_block_tree_entry_equals":
(https://github.com/bitcoin/bitcoin/pull/33855#discussion_r2522796090)
_response to https://github.com/bitcoin/bitcoin/pull/33855#issuecomment-3526237850_

@alexanderwiederin opening this thread for easier focus. Are you suggesting we 1) implement `btck_block_tree_entry_equals` with a hash equality check, or 2) to not expose `btck_block_tree_entry_equals` at all and let the user define it however they want (such as using hash equality), or 3) something else?

If 1), I don't see the benefit? Pointer comparisons are faster, and are conceptually correct, and what
...

💬 maflcko commented on pull request "ci: Lint follow-ups":
(https://github.com/bitcoin/bitcoin/pull/33776#issuecomment-3527196104)
> It's a shame that in this PR we have to have the annotation printed many times, one per job, as I think it might drown out other notices.

Agree, but I think we can use warning notices to bubble up more important ones.



> Without knowing exactly how this machine readable annotation is being consumed or experimenting more, I'm not aware of an alternative?

It can be used via the API, e.g. https://github.com/maflcko/DrahtBot/blob/6c91378116516ca5974d78a4e0c5c495b38acf95/webhook_feature
...

💬 vasild commented on pull request "net_processing: reorder the code that handles the VERSION message":
(https://github.com/bitcoin/bitcoin/pull/33792#issuecomment-3527310703)
Good question, I guess it is a bit obscure. Let me elaborate. So, when we receive the `VERSION` message, its handling (inside the `if (msg_type == NetMsgType::VERSION) {`) currently goes like this:

```
if (...)
A
if (...)
MakeAndPushMessage()
if (...)
B
if (...)
MakeAndPushMessage()
if (...)
some stuff that better be avoided for private broadcast connections
```

For private broadcast we don't want to push any extra messages. To do that I took the approach to
...

💬 vasild commented on pull request "net_processing: reorder the code that handles the VERSION message":
(https://github.com/bitcoin/bitcoin/pull/33792#issuecomment-3527377529)
> What’s the rationale for moving `m_addrman.Good(pfrom.addr)` earlier ?

Hmm, wait! `m_addrman.Good()` need not be earlier, it is one of the "some stuff that better be avoided". Then this can be simplified? :bulb:

💬 ajtowns commented on pull request "txgraph: drop move assignment operator":
(https://github.com/bitcoin/bitcoin/pull/33862#issuecomment-3527384125)
ACK aef40b93cf057d2a27d61881b0858d491206bcd3 -- matches what I was thinking

Adding some assertions in `Ref::Ref(Ref&&)` triggers when I run the fuzz binary over some txgraph corpus data I generated previously, so https://github.com/bitcoin/bitcoin/pull/33862#discussion_r2519205585 doesn't seem like a real problem.