💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2510927772)
Looks good to me

📝 vasild opened a pull request: "test: add tests for private broadcast"
(https://github.com/bitcoin/bitcoin/pull/33843)
This PR contains https://github.com/bitcoin/bitcoin/pull/29415 + two more commits that add functional and fuzz tests.

Putting those in a separate PR not to burden the main one, since the tests are extensive:
code: `1036 insertions(+), 91 deletions(-)`
tests: `548 insertions(+), 5 deletions(-)`

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3512327977)
`29ef3c62de...d419fbc42b`: drop the last commit which contained the functional tests. That functional test + a fuzz test now live in a separate PR: https://github.com/bitcoin/bitcoin/pull/33843. This reduces the main PR from
`1451 insertions(+), 91 deletions(-)` to
`1036 insertions(+), 91 deletions(-)`.

💬 dergoegge commented on pull request "test: add tests for private broadcast":
(https://github.com/bitcoin/bitcoin/pull/33843#issuecomment-3512335697)
Can you elaborate on the rational here? Why would the tests burden the main change? In my opinion, the purpose of tests should be to make it easier to review a code change and gain confidence in it's correctness etc. Splitting them from the code change seems counter productive.

💬 hebasto commented on pull request "build: add `-W*-whitespace`":
(https://github.com/bitcoin/bitcoin/pull/32482#issuecomment-3512348200)
> > Even if it’s been fixed upstream,
>
> It's not clear to me if it has been fixed or not, can you link to the relevant change / issue?

See https://bugreports.qt.io/browse/QTBUG-141858.

> I can cherry-pick the change above in, if you create a commit, and add a comment inline, for when the change can be dropped.

Feel free to pick top two commits from https://github.com/hebasto/bitcoin/commits/pr32482/1110/.

💬 fanquake commented on pull request "test: add tests for private broadcast":
(https://github.com/bitcoin/bitcoin/pull/33843#issuecomment-3512354999)
Concept NACK to deferring adding tests until after adding a new feature.

🤔 hebasto reviewed a pull request: "build: Bump g++ minimum supported version to 12"
(https://github.com/bitcoin/bitcoin/pull/33842#pullrequestreview-3443882743)
Such a change might also require updating the build guides. For instance, the following commands on Ubuntu 22.04 would use GCC 11:
```
sudo apt-get install build-essential ...
cmake --build build
```

✅ vasild closed a pull request: "test: add tests for private broadcast"
(https://github.com/bitcoin/bitcoin/pull/33843)

💬 vasild commented on pull request "test: add tests for private broadcast":
(https://github.com/bitcoin/bitcoin/pull/33843#issuecomment-3512512550)
Closing after discussion at https://www.erisian.com.au/bitcoin-core-dev/log-2025-11-10.html#l-236

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3512531635)
`d419fbc42b..29ef3c62de`: restore back the test here and close https://github.com/bitcoin/bitcoin/pull/33843 after discussion at https://www.erisian.com.au/bitcoin-core-dev/log-2025-11-10.html#l-236

If people review up to but not including the test, they can ACK the last but one commit.

💬 fanquake commented on pull request "build: add `-W*-whitespace`":
(https://github.com/bitcoin/bitcoin/pull/32482#issuecomment-3512546980)
> See https://bugreports.qt.io/browse/QTBUG-141858.
> Feel free to pick

Thanks. I've pulled those two in here.

💬 achow101 commented on pull request "wallet: Always rewrite tx records during migration":
(https://github.com/bitcoin/bitcoin/pull/32985#issuecomment-3512556430)
> these changes are persisted during `ReorderTransactions()`

Not for migration since legacy wallets are only opened with a `BerkeleyRODatabase` which does not write anything.

💬 maflcko commented on pull request "build: Bump g++ minimum supported version to 12":
(https://github.com/bitcoin/bitcoin/pull/33842#issuecomment-3512575639)
> Such a change might also require updating the build guides. For instance, the following commands on Ubuntu 22.04 would use GCC 11:

I am happy to add a general note that the latest stable or LTS is recommended and that earlier releases may need the user to install the required minimum dependencies.

📝 nichechristie opened a pull request: "Claude/create coin 011 c uz rux6 kwdft yev8 a5 zsg"
(https://github.com/bitcoin/bitcoin/pull/33844)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

✅ hebasto closed a pull request: "Claude/create coin 011 c uz rux6 kwdft yev8 a5 zsg"
(https://github.com/bitcoin/bitcoin/pull/33844)

🤔 janb84 reviewed a pull request: "kernel: trim Chain interface"
(https://github.com/bitcoin/bitcoin/pull/33820#pullrequestreview-3443942718)
ACK 66978a1a95379a2fe5d41032682dedfaddc99db9

Was already wondering why these 2 functions where present in the code. With the iteration ability I agree that there is no need for these "speciality" functions. The functionality can be implemented downstream if needed.

steps taken:
code review,
build,
ran all tests,

(small contemplation; chain.tip is 'often' used in the rest of the bitcoin code, is removing the `tip` function from the header not a performance burden downstream to ca
...

💬 janb84 commented on pull request "kernel: trim Chain interface":
(https://github.com/bitcoin/bitcoin/pull/33820#discussion_r2511078966)
```C
auto genesis_block_raw{chainman->ReadBlock(genesis_index).value().ToBytes()};
auto first_index{chain.GetByHeight(0)};
auto first_block_raw{chainman->ReadBlock(genesis_index).value().ToBytes()};
```

Not sure if this part of the test still makes sense, the double retrieval of the data of block 0 `auto first_block_raw{chainman->ReadBlock(genesis_index).value().ToBytes()};`
we are now testing if `chainman->ReadBlock` will retrieve the same block twice not if it will get the
...

💬 achow101 commented on pull request "log: reduce excessive "rolling back/forward" messages during block replay":
(https://github.com/bitcoin/bitcoin/pull/33443#issuecomment-3512621236)
ACK 1fc7a81f1f5f30ba3ebe305ac2a520c7b4afb596

📝 maflcko opened a pull request: "ci: Enable experimental kernel stuff in ASan task"
(https://github.com/bitcoin/bitcoin/pull/33845)
Base the task on --preset=dev-mode to ensure maximal coverage and add the following:

```
bitcoin-chainstate (experimental) ... ON
libbitcoinkernel (experimental) ..... ON
kernel-test (experimental) .......... ON
```

Currently a draft to figure out https://github.com/bitcoin/bitcoin/pull/33824#issuecomment-3511082065

🚀 achow101 merged a pull request: "log: reduce excessive "rolling back/forward" messages during block replay"
(https://github.com/bitcoin/bitcoin/pull/33443)