🚀 fanquake merged a pull request: "mapport: require miniupnpc API version 17 or later"
(https://github.com/bitcoin/bitcoin/pull/27016)

💬 MarcoFalke commented on pull request "test: Fix intermittent sync issue in wallet_pruning":
(https://github.com/bitcoin/bitcoin/pull/27066#issuecomment-1428249997)
> https://cirrus-ci.com/task/5441089956478976 cry

Ah, good catch. This is the still exactly the same error as in https://github.com/bitcoin/bitcoin/issues/27065. While the fix here is unrelated to fixing that bug, I think it is still needed to fix the same bug happening in a different line.

📝 MarcoFalke opened a pull request: "test: Fix intermittent sync issue in wallet_pruning"
(https://github.com/bitcoin/bitcoin/pull/27093)
The `sync_fun=self.no_op` has no motivation or rationale, and seems to be causing issues.

Fix that by removing it.

Actually fixes https://github.com/bitcoin/bitcoin/issues/27065, see https://github.com/bitcoin/bitcoin/pull/27066#issuecomment-1428249997

💬 MarcoFalke commented on pull request "test: Fix intermittent sync issue in wallet_pruning":
(https://github.com/bitcoin/bitcoin/pull/27066#issuecomment-1428252597)
See https://github.com/bitcoin/bitcoin/pull/27093

💬 furszy commented on pull request "refactor: wallet, remove global 'ArgsManager' dependency":
(https://github.com/bitcoin/bitcoin/pull/26889#discussion_r1104733597)
ah, you are suggesting to change it to a signed type. Sure.
Pushed.

💬 TheCharlatan commented on pull request "refactor: wallet, remove global 'ArgsManager' dependency":
(https://github.com/bitcoin/bitcoin/pull/26889#discussion_r1104753896)
I might have missed something, but why not use the `m_keypool_size` from the available wallet here?

```
diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp
index 64253789ec..d9c6755bcb 100644
--- a/src/wallet/rpc/backup.cpp
+++ b/src/wallet/rpc/backup.cpp
@@ -15,7 +15,6 @@
#include <script/standard.h>
#include <sync.h>
#include <util/bip32.h>
-#include <util/system.h>
#include <util/time.h>
#include <util/translation.h>
#include <wallet/rpc/util.h>
@@ -1480,7
...

💬 furszy commented on pull request "refactor: wallet, remove global 'ArgsManager' dependency":
(https://github.com/bitcoin/bitcoin/pull/26889#issuecomment-1428276879)
Updated per feedback. Thanks hebasto.

Plus added 1ff1b9c to protect `importdescriptors` from negative '-keypool' value.
Applied the same approach used in the wallet code to handle a negative keypool size. Currently, the keypool size is set to 1 in such cases, although it could also be handled by throwing an initialization error but.. the objective was to implement the smallest change in this PR to avoid extending its scope.

💬 furszy commented on pull request "refactor: wallet, remove global 'ArgsManager' dependency":
(https://github.com/bitcoin/bitcoin/pull/26889#discussion_r1104756635)
ha, because I'm still sleepy, good catch. Pushing it..

💬 instagibbs commented on issue "24.0.1 crash on restart":
(https://github.com/bitcoin/bitcoin/issues/27088#issuecomment-1428279748)
Happened againt today: https://gist.github.com/instagibbs/17d6513f26d8999ffebc25cf745d84fb

I'll try and see if I can interleaved bitcoind logs in CI as well, but it looks like at this line https://gist.github.com/instagibbs/17d6513f26d8999ffebc25cf745d84fb#file-bitcoind-crash-cln-ci-L2725 CLN thinks it talked to bitcoind fine, but immediately after it has an issue due to the crash.

💬 MarcoFalke commented on pull request "ci: A few fixes of `ccache` issues":
(https://github.com/bitcoin/bitcoin/pull/27084#discussion_r1104757809)
I am still not sure what this means and which paths are preferred or discouraged

💬 john-moffett commented on pull request "wallet: SecureString to allow null characters":
(https://github.com/bitcoin/bitcoin/pull/27068#issuecomment-1428311028)
> Concept ACK, great catch indeed.

Thank you (and @furszy)!

>> Check to see if the wallet would decrypt using the substring up to the first null and only then emit errors (or even just warnings) like in 1 or 2.

> I think this is the best approach. ... Additionally, I think we could also encourage the user to re-encrypt the wallet with a different passphrase (without null char) for maximum backward compatibility, and perhaps even offer a one-click solution to do so in qt?

Sounds good,
...

💬 vasild commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#discussion_r1104785968)
It has inside, `ReachableNets::m_mutex`.

💬 brunoerg commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#discussion_r1104803546)
In d9b95cb1eedb9f165df93dcae0aba96ede40767a, I could check that all CJDNS address starts with [OxFC](https://github.com/cjdelisle/cjdns/blob/master/doc/Whitepaper.md#pulling-it-all-together), so, it switches the first byte to `0x55` to avoid generating it that look like CJDNS. But why specifically `0x55`?

💬 brunoerg commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#discussion_r1104805263)
nit:
```suggestion
const bool reachable{g_reachable_nets.Contains(addr)};
```

💬 ryanofsky commented on pull request "Multiprocess bitcoin":
(https://github.com/bitcoin/bitcoin/pull/10102#issuecomment-1428374812)
re: https://github.com/bitcoin/bitcoin/pull/10102#issuecomment-1399216846

> Not sure if related or not:

Yes, the IPC serialization code wasn't setting the `interfaces::FoundBlock.found` field, which caused wallets not to sync correctly after #26679. (The serialization bug present before #26679, but didn't have an effect until new code tried to use the missing field.)

---

Rebased 05e44e7fd41b32d57379bb6a87c4bebb4a166f9c -> 5e90c75de0d13aedb7e7c06ebe5aa55b11bb398d ([`pr/ipc.183`](https
...

💬 theuni commented on pull request "build: Add CMake-based build system (1 of N)":
(https://github.com/bitcoin/bitcoin/pull/27060#discussion_r1104819743)
Note that this ends up setting:
```
C compiler ............................ /usr/bin/cc
...
C++ compiler .......................... /usr/bin/c++
```
Which is different from what autotools would find. cc/c++ are usually symlinks to a valid compiler, but they don't always exist.

For ex, this would actually break my current workflow because my current llvm+clang toolchain did not ship with these links. So I'd see surprising results with a build like:
```
export PATH=/opt/clang+llvm-14.0.
...

💬 theuni commented on pull request "build: Add CMake-based build system (1 of N)":
(https://github.com/bitcoin/bitcoin/pull/27060#discussion_r1104782068)
This introduces the possibility of a departure from Autotools behavior, and is THE reason I was so opposed to switching to CMake for so long. After the "Autotools Hell" years, it mostly froze in time and (i'm oversimplifying) modules/features became locked-in. That means, in practice, you don't have to worry about whether autoconf's m4's are new enough for feature X.

CMake, however, takes us back in time in this regard. We now have to worry about its version and the feature-set that it provid
...

💬 theuni commented on pull request "build: Add CMake-based build system (1 of N)":
(https://github.com/bitcoin/bitcoin/pull/27060#discussion_r1104752652)
Why is c++20 turned on automatically for MSVC? Afaics autotools doesn't do that.

💬 theuni commented on pull request "build: Add CMake-based build system (1 of N)":
(https://github.com/bitcoin/bitcoin/pull/27060#discussion_r1104821340)
Does this need CMP0083 ?

💬 vasild commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#issuecomment-1428383348)
> I think of `netbase` as a module for mostly context-free basic low-level methods - this PR moves `IsReachable` to it and saves its state in a global variable,

I was thinking about the same, but `netbase` is already stateful - `proxyInfo`, `nameProxy`, `nConnectTimeout`, `fNameLookup`, `g_socks5_recv_timeout`, `interruptSocks5Recv` are global variables that keep state, which influence the output of the functions defined in `netbase`, making them context-dependent.

`SetProxy()`, `GetProxy(
...