👍 brunoerg approved a pull request: "addrman: Enable selecting addresses by network"
(https://github.com/bitcoin/bitcoin/pull/27214)
crACK 25a64a20749f10ce84060f3570ad76d1a4776948

📝 MarcoFalke opened a pull request: "test: Use self.wait_until over wait_until_helper"
(https://github.com/bitcoin/bitcoin/pull/27226)
`wait_until_helper` is a "private" helper, not intended to be used directly, because it doesn't scale the timeout with the timeout factor. Fix this by replacing it with a call to `self.wait_until`, which does the scaling.

💬 MarcoFalke commented on issue "Issue in `p2p_ibd_stalling.py` under Valgrind":
(https://github.com/bitcoin/bitcoin/issues/27208#issuecomment-1459956694)
Ok, see https://github.com/bitcoin/bitcoin/pull/27226

💬 TheCharlatan commented on pull request "refactor / kernel: Move non-gArgs chainparams functionality to kernel":
(https://github.com/bitcoin/bitcoin/pull/26177#issuecomment-1459958764)
Updated f9e3c91f4d482b0c74ef899056d5a2c63e67e1fb -> c44983faa02b4845cd7605abe99c332980a07af1 ([tc/2022-09-libbitcoinkernel-chainparams-args_3](https://github.com/TheCharlatan/bitcoin/commits/tc/2022-09-libbitcoinkernel-chainparams-args_3) -> [tc/2022-09-libbitcoinkernel-chainparams-args_4](https://github.com/TheCharlatan/bitcoin/commits/tc/2022-09-libbitcoinkernel-chainparams-args_4), [compare](https://github.com/TheCharlatan/bitcoin/compare/tc/2022-09-libbitcoinkernel-chainparams-args_3..tc/202
...

💬 MarcoFalke commented on pull request "test: Default timeout factor to 4 under --valgrind":
(https://github.com/bitcoin/bitcoin/pull/27221#issuecomment-1459961347)
Yeah, this doesn't affect the valgrind CI system. Only test runs with `--valgrind` passed (and no timeout factor).

💬 MarcoFalke commented on issue "Rescanning use only 1 core":
(https://github.com/bitcoin/bitcoin/issues/19992#issuecomment-1459970293)
I think rescan is still single threaded, but there shouldn't be any reason to call it often, so I am not sure if there is a use case.

💬 MarcoFalke commented on issue "Rescanning use only 1 core":
(https://github.com/bitcoin/bitcoin/issues/19992#issuecomment-1459971651)
The feature request didn't seem to attract much attention in the past. Also, the issue seems not important enough right now to keep it sitting around idle in the list of open issues.

Closing due to lack of interest. Pull requests with improvements are always welcome.

✅ MarcoFalke closed an issue: "Rescanning use only 1 core"
(https://github.com/bitcoin/bitcoin/issues/19992)

💬 MarcoFalke commented on pull request "refactor: Remove CAddressBookData::destdata":
(https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1129250725)
```

src/wallet/walletdb.cpp:616:58: error: passing result of std::move() as a const reference argument; no move will actually happen [performance-move-const-arg,-warnings-as-errors]
data.SetReceiveRequest(strKey.substr(2), std::move(strValue));

💬 fanquake commented on pull request "test: Default timeout factor to 4 under --valgrind":
(https://github.com/bitcoin/bitcoin/pull/27221#issuecomment-1459979548)
> Yeah, this doesn't affect the valgrind CI system.

Ok. Can we consolidate this at all? The disparity is somewhat confusing because you would assume that the Valgrind CI job would use the dedicated `--valgrind` option provided by the test framework.

💬 fanquake commented on pull request "doc: docment json rpc endpoints":
(https://github.com/bitcoin/bitcoin/pull/27225#issuecomment-1459981209)
Concept ACK

💬 fanquake commented on pull request "test: Use self.wait_until over wait_until_helper":
(https://github.com/bitcoin/bitcoin/pull/27226#issuecomment-1459982082)
Concept ACK. Will test under the CI

💬 willcl-ark commented on issue "Option to ignore small inputs when internal wallet is building TXes?":
(https://github.com/bitcoin/bitcoin/issues/20870#issuecomment-1459986236)
@Crypto2 can you not just do the inverse and use coin selection/manual construction when you want to exclude small outputs?

💬 MarcoFalke commented on pull request "test: Default timeout factor to 4 under --valgrind":
(https://github.com/bitcoin/bitcoin/pull/27221#issuecomment-1459987786)
Not sure if that makes sense. This would require special casing the non-test binaries `bitcoind/-cli/-tx/-util` etc in the valgrind wrapper. Also, if someone wants to interactively re-run a single test inside the CI pod, they'd have to pass `--valgrind`. Maybe open a new discussion thread or pull request?

💬 fanquake commented on pull request "build: use _FORTIFY_SOURCE=3":
(https://github.com/bitcoin/bitcoin/pull/27027#issuecomment-1459992955)
> This triggered https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=56529 ?

It will have been #27118 that triggered it. I can't look at this in mroe detail right now, but will by next week.

> On my Gentoo system with GCC 11.3.1 and glibc 2.36 this now gives me these warnings for each .cpp file when building Bitcoin Core

Looks like that's because you're using GCC 11.x, which doesn't support >=3 (it is supported with Clang >= 9). Somewhat annoying that glibc is going to warn like that,
...

💬 fanquake commented on pull request "bench: update logging benchmarks":
(https://github.com/bitcoin/bitcoin/pull/26957#discussion_r1129269455)
In 102b2033493f0d61e9763d094cb8a0017f7e3a10 bench: order the logging benchmark code by output:

Given that the code-movement, and re-ordering the `BENCHMARK(` calls doesn't change the output, this seems like a no-op?

In general, is (re-)ordering code in a `.cpp` file based on the order functions are called something we do?

💬 kristapsk commented on pull request "build: use _FORTIFY_SOURCE=3":
(https://github.com/bitcoin/bitcoin/pull/27027#issuecomment-1460007675)
> Looks like that's because you're using GCC 11.x, which doesn't support >=3 (it is supported with Clang >= 9). Somewhat annoying that glibc is going to warn like that, but they are also harmless, and fortification (level 2) is still being applied.

Could we somehow detect this at configure stage and apply fortification 3 only if it is supported?

💬 fanquake commented on pull request "rpc, test: remove newline escape sequence from wallet warning fields":
(https://github.com/bitcoin/bitcoin/pull/27138#issuecomment-1460011191)
> Saw those for (maybe, maybe not) another pull; prefer to keep this small and focused.

If they are the same issue, what's the reasoning for leaving them for another PR? Not sure how the PR becomes less focused by fixing more of the same problem.

💬 fanquake commented on pull request "refactor: replace all implicit C-style const/const+reinterpret with explicit casts":
(https://github.com/bitcoin/bitcoin/pull/27126#discussion_r1129284068)
> That might make sense. If I get more time sometime soon, I'll see if I can easily span-ify it.

There doesn't seem to be much buy-in to making this change, and there's no real rush in any case, so feel free to look at using the Spans. Could probably mark this as a draft for now, if that is what you're going to look at.

💬 real-or-random commented on pull request "Reduce wasted pseudorandom bytes in ChaCha20 + various improvements":
(https://github.com/bitcoin/bitcoin/pull/26153#discussion_r1129293131)
```suggestion
static_assert(Num3072::BYTE_SIZE % 64 == 0);
ChaCha20Aligned(hashed_in.data(), hashed_in.size()).Keystream64(tmp, Num3072::BYTE_SIZE / 64);

```