💬 MarcoFalke commented on pull request "log: Check that the timestamp string is non-empty to avoid undefined behavior":
(https://github.com/bitcoin/bitcoin/pull/27317#discussion_r1146718782)
This should never happen, so instead of adding dead code, what about simply combining the two if conditions?

💬 martinus commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#issuecomment-1481782854)
Updated d87cb99 -> 9f947fc3d4b779f017332135323b34e8f216f613 ([pr25325.1](https://github.com/martinus/bitcoin/commits/pr25325.1) -> [pr25325.2](https://github.com/martinus/bitcoin/commits/pr25325.2))

There is a single behavior change in pool.h, now `NumElemAlignBytes` adds `+ (bytes == 0)` so that allocations of 0 bytes work with the PoolAllocator.

Other than that, updated tests to include allocation of 0 bytes, and fixed all the nits.

💬 stratospher commented on pull request "p2p: Improve diversification of new connections":
(https://github.com/bitcoin/bitcoin/pull/27264#issuecomment-1481786231)
yes! thanks @mzumsande, will address this in a follow-up PR.

💬 john-moffett commented on pull request "log: Check that the timestamp string is non-empty to avoid undefined behavior":
(https://github.com/bitcoin/bitcoin/pull/27317#discussion_r1146738163)
My thinking was that if it never happens, then this PR is unnecessary. If it could happen, then I'd rather have an explicit message rather than output that entirely lacks timestamps, which someone may not notice is bizarre.

Happy to change it, though, if people prefer your approach.

📝 furszy opened a pull request: "test: wallet_create_tx.py fix race"
(https://github.com/bitcoin/bitcoin/pull/27318)
Fixes #27316

As wallets are internally synced by the
validation interface, and the interface
dispatches events on a worker thread,
it can happen that the tx sent from the
first wallet doesn't arrive to the second
wallet before the second wallet tries to
create a new transaction using one of
the outputs of the first tx as input.

💬 jonatack commented on pull request "log: Check that the timestamp string is non-empty to avoid undefined behavior":
(https://github.com/bitcoin/bitcoin/pull/27317#discussion_r1146755842)
Assuming it can happen, maybe wrap the message in, say, square brackets ("[...]") to be clearer in the log.

💬 pinheadmz commented on issue "pruneblockchain should be able to increase the size of pruned blockchain":
(https://github.com/bitcoin/bitcoin/issues/24286#issuecomment-1481813132)
Ok, closing this for now as duplicate of https://github.com/bitcoin/bitcoin/issues/19807 which has a bit more active discussion. Will try to close that one as well in favor of #21267 if OP agrees. @prusnak please feel free to comment if you think we need to keep this one open as well.

✅ pinheadmz closed an issue: "pruneblockchain should be able to increase the size of pruned blockchain"
(https://github.com/bitcoin/bitcoin/issues/24286)

💬 john-moffett commented on pull request "log: Check that the timestamp string is non-empty to avoid undefined behavior":
(https://github.com/bitcoin/bitcoin/pull/27317#discussion_r1146762216)
I think I've already come around to the view that it's so unlikely as to be not worth the additional LoC, so I'll switch to the simpler version.

Thanks!

💬 prusnak commented on issue "pruneblockchain should be able to increase the size of pruned blockchain":
(https://github.com/bitcoin/bitcoin/issues/24286#issuecomment-1481826196)
Ack

💬 MarcoFalke commented on issue "tidy: enable `cppcoreguidelines-pro-type-member-init`":
(https://github.com/bitcoin/bitcoin/issues/27315#issuecomment-1481831715)
This indeed looks more involved than I initially thought. Report on the cpp files only:

```diff
diff --git a/src/.clang-tidy b/src/.clang-tidy
index b2c1b49..c5379f1 100644
--- a/src/.clang-tidy
+++ b/src/.clang-tidy
@@ -1,5 +1,6 @@
Checks: '
-*,
+cppcoreguidelines-pro-type-member-init,
bugprone-argument-comment,
bugprone-use-after-move,
misc-unused-using-decls,
@@ -17,4 +18,3 @@ WarningsAsErrors: '*'
CheckOptions:
- key: performance-move-const-arg.CheckTriviallyCopyable
...

📝 brunoerg opened a pull request: "addrman, refactor: improve stochastic test in `AddSingle`"
(https://github.com/bitcoin/bitcoin/pull/27319)
In the current implementation, if `pinfo->nRefCount` is 0, we created an unnecessary variable (`nFactor`). The new approach changes it to avoid it.

💬 brunoerg commented on pull request "wallet: add config to prioritize a solution that doesn't create change in coin selection ":
(https://github.com/bitcoin/bitcoin/pull/23475#issuecomment-1481851030)
Closing for now as I haven't been working on it for a while

✅ brunoerg closed a pull request: "wallet: add config to prioritize a solution that doesn't create change in coin selection "
(https://github.com/bitcoin/bitcoin/pull/23475)

💬 TheCharlatan commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#issuecomment-1481881156)
Not sure why the functional tests are failing. The test failing on the Win64 job is passing on the 32-bit job and vice-versa. Both are passing locally.

💬 achow101 commented on pull request "bench: update logging benchmarks":
(https://github.com/bitcoin/bitcoin/pull/26957#issuecomment-1481883224)
ACK 8c47d599b87d6b2d43e7d37ce0aaf4f541535bb9

🚀 achow101 merged a pull request: "bench: update logging benchmarks"
(https://github.com/bitcoin/bitcoin/pull/26957)

💬 ryanofsky commented on pull request "init: Error if ignored bitcoin.conf file is found":
(https://github.com/bitcoin/bitcoin/pull/27302#issuecomment-1481906438)
Updated 972335762aeaab557dbb2e44b149b18005987f8b -> d00762c75a1b9ad16e0dadf25886a7baefa84a12 ([`pr/ignoredconf.6`](https://github.com/ryanofsky/bitcoin/commits/pr/ignoredconf.6) -> [`pr/ignoredconf.7`](https://github.com/ryanofsky/bitcoin/commits/pr/ignoredconf.7), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ignoredconf.6..pr/ignoredconf.7)) to try to fix a new win64 CI error in test_ignored_default_conf:

```
Traceback (most recent call last):
File "C:\Users\ContainerAdminis
...

💬 ryanofsky commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#issuecomment-1481912454)
Win64 wallet_create_tx.py failure is probably fixed by #27318

💬 jonatack commented on pull request "Add "warnings", deprecate "warning" in {create,load,unload,restore}wallet":
(https://github.com/bitcoin/bitcoin/pull/27279#discussion_r1146864188)
Thanks @vasild. It could maybe even be split further into two methods, one for the `warning` deprecations in 4 wallet RPCs, and one called by all the wallet RPCs having a `warnings` field. Will explore.