💬 mzumsande commented on pull request "test: address self-announcement":
(https://github.com/bitcoin/bitcoin/pull/34039#discussion_r2628081222)
I would suggest to rework or remove this before merge, first person language and `print()` don't seem appropriate at that stage anymore, the mismatch print could be an assert if it's not expected to ever happen.

💬 mzumsande commented on pull request "test: address self-announcement":
(https://github.com/bitcoin/bitcoin/pull/34039#discussion_r2628279820)
nit: **an** outbound

💬 mzumsande commented on pull request "test: address self-announcement":
(https://github.com/bitcoin/bitcoin/pull/34039#discussion_r2628375476)
note: I first thought that there might be a race condition (node receives verack first, and getaddr with some delay, sends out self-advertisment in between) but this isn't actually true because in this case we won't self-advertise due to #21528 (`SetupAddressRelay` not called yet) . So this is fine for now (whether that's good is another issue)

🤔 furszy reviewed a pull request: "qa: Avoid knock-on exception in assert_start_raises_init_error"
(https://github.com/bitcoin/bitcoin/pull/32929#pullrequestreview-3589512416)
Code ACK 356883f0e48be59bcb154096cef82cbf3f0dd9d8

💬 hebasto commented on pull request "ci: Add IWYU job":
(https://github.com/bitcoin/bitcoin/pull/33810#issuecomment-3667066005)
@maflcko

Thank you for the review. The PR has been reworked per your feedback.

💬 hebasto commented on pull request "ci: Add IWYU job":
(https://github.com/bitcoin/bitcoin/pull/33810#discussion_r2628562127)
> edit: The full patch would be:

[Taken](https://github.com/bitcoin/bitcoin/pull/33810#issuecomment-3667066005).

💬 jb55 commented on issue "sqlite legacy descriptor wallet migration fails":
(https://github.com/bitcoin/bitcoin/issues/33468#issuecomment-3667104351)
oh nice I instinctually tried that and ran into the checksum error, didn't realize I could replace the checksum. will try

💬 jb55 commented on issue "sqlite legacy descriptor wallet migration fails":
(https://github.com/bitcoin/bitcoin/issues/33468#issuecomment-3667153801)
that works! thanks boss.

✅ jb55 closed an issue: "sqlite legacy descriptor wallet migration fails"
(https://github.com/bitcoin/bitcoin/issues/33468)

💬 stickies-v commented on issue "RFC: separate kernel logging infrastructure":
(https://github.com/bitcoin/bitcoin/issues/34062#issuecomment-3667169341)
I have implemented a new approach ("Layering") in https://github.com/bitcoin/bitcoin/compare/master...stickies-v:bitcoin:2025-12/kernel-logging-layering. The Layering approach turned out much less invasive than my initial attempts showed, and now has my preference.

In a nutshell:
- we distinguish between a (low-level) logger that produces log statements (`util::log::Logger`) and (higher-level) log sinks (`BCLog::Sink`, `btck_LoggingConnection`) that consume the logs by registering a callback on
...

👍 ryanofsky approved a pull request: "refactor: unify container presence checks"
(https://github.com/bitcoin/bitcoin/pull/33192#pullrequestreview-3589700908)
Code review ACK d9319b06cf82664d55f255387a348135fd7f91c7. I manually reviewed the full change, and it seems there are a lot of positive comments about this and no more very significant conflicts, so I will merge it shortly.

I am wondering if there are plans to turn on the clang-tidy check and follow up with other replacements?

🚀 ryanofsky merged a pull request: "refactor: unify container presence checks"
(https://github.com/bitcoin/bitcoin/pull/33192)

💬 l0rinc commented on pull request "refactor: unify container presence checks":
(https://github.com/bitcoin/bitcoin/pull/33192#issuecomment-3667210907)
> I am wondering if there are plans to turn on the clang-tidy check and follow up with other replacements?

I couldn't use tidy for many of these changes, but I'm looking forward to doing a follow-up.

Thanks for the persistent reviewers, I know it was a boring one :)

📝 Dahka2321 opened a pull request: "chore: bump checkout to v6"
(https://github.com/bitcoin/bitcoin/pull/34094)
Updated actions/checkout from v5 to v6 in ci.yml workflow

💬 waketraindev commented on pull request "[30.x] Finalise v30.1":
(https://github.com/bitcoin/bitcoin/pull/34092#issuecomment-3667331991)
ACK https://github.com/bitcoin/bitcoin/commit/2a21824b1190968ee8ba3120400c00e56be10591

💬 theuni commented on pull request "Add initial vectorized chacha20 implementation for 2-3x speedup":
(https://github.com/bitcoin/bitcoin/pull/34083#issuecomment-3667333373)
Ok, after some experimenting, I am very hesitant to add the helpers as suggested above. For example, consider @ajtowns's seemingly innocuous Repeat function:
```c++
template <size_t REPS, typename Fn>
ALWAYS_INLINE void Repeat(Fn&& fn)
{
if constexpr (REPS > 0) {
fn();
Repeat<REPS-1>(std::forward<Fn>(fn));
}
}
```

This causes a 10% slowdown on this branch, but with avx2 the performance is abysmal:

Baseline(master):
```
| 1.38 | 723,27
...

💬 sedited commented on issue "RFC: separate kernel logging infrastructure":
(https://github.com/bitcoin/bitcoin/issues/34062#issuecomment-3667358790)
> I'll open a PR if/when people are happy about the concept and initial approach, that's probably a better place to discuss code in more depth.

This newest approach looks good to me. I think some people might still have reservations about unconditionally logging all categories. I tend to agree with your opinion on this though. Once debug logging is activated, expectations on string serialization overhead should be the same, no matter the category selected.

📝 l0rinc opened a pull request: "refactor: enable `readability-container-contains` clang-tidy rule"
(https://github.com/bitcoin/bitcoin/pull/34095)
Replace the last few instances of `.count() != 0` and `.count() == 0` patterns with the more expressive C++20 `.contains()` method:

* `std::set<std::string>` in `getblocktemplate` RPC;
* `std::map<std::string, ...>` in `transaction_tests`;
* `boost::multi_index_container` (w)tx checks in `CTxMemPool::exists()`

With no remaining violations, enable the `readability-container-contains` clang-tidy check to prevent future regressions.

Follow-up to https://github.com/bitcoin/bitcoin/pull/33
...

💬 l0rinc commented on pull request "log: show reindex progress in `ImportBlocks`":
(https://github.com/bitcoin/bitcoin/pull/33353#issuecomment-3667507927)
rfm?

💬 arejula27 commented on pull request "build: Fix source paths for debugging in CMake":
(https://github.com/bitcoin/bitcoin/pull/34081#issuecomment-3667561373)
I have analysed how the different debug information is stored in the master branch, using my changes and removing the remaps (your proposal ). The observable difference is in how DWARF debug information records source file paths.

Here are the three scenarios I tested with `objdump -Wi` to extract the info:
```bash
readelf -wi ./build/bin/bitcoind | grep DW_AT_name | grep "\.cpp" >clean_readelf.txt
```

## First case (master branch – typical out-of-tree build)

```text
<3674904> DW_
...

👋 l0rinc's pull request is ready for review: "refactor: enable `readability-container-contains` clang-tidy rule"
(https://github.com/bitcoin/bitcoin/pull/34095)