💬 rkrux commented on pull request "wallet: Automatically repair corrupted metadata with doubled derivation path":
(https://github.com/bitcoin/bitcoin/pull/29124#discussion_r1939245322)
Reference for line format: https://github.com/bitcoin/bitcoin/blob/28.x/src/wallet/rpc/backup.cpp#L778-L802

💬 rkrux commented on pull request "wallet: Automatically repair corrupted metadata with doubled derivation path":
(https://github.com/bitcoin/bitcoin/pull/29124#discussion_r1939186315)
For others, reference: https://github.com/bitcoin/bitcoin/blob/28.x/src/wallet/rpc/wallet.cpp#L521

💬 rkrux commented on pull request "wallet: Automatically repair corrupted metadata with doubled derivation path":
(https://github.com/bitcoin/bitcoin/pull/29124#discussion_r1939237181)
Similar tying to a constant here if possible.

💬 vijayabhaskar78 commented on issue "cmake: incorrectly reporting MSVC as using ccache":
(https://github.com/bitcoin/bitcoin/issues/31771#issuecomment-2630755334)
@fanquake @hebasto @purpleKarrot

### **Problem Summary**

- **False Reporting:** CMake incorrectly reports `ccache` as "ON" for MSVC builds despite excluding it in `cmake/ccache.cmake`.
- **Launcher Conflicts:** Forced `CMAKE_*_COMPILER_LAUNCHER` assignments override user/CI settings (e.g., `sccache`).

### **Proposed Fix**

#### **1. Delete `cmake/ccache.cmake`**

```bash
rm cmake/ccache.cmake
```

- **Why?** Removes forced `ccache` logic conflicting with external launchers.
- **
...

💬 fanquake commented on pull request "cmake: Improve safety and robustness during building `crc32c` subtree":
(https://github.com/bitcoin/bitcoin/pull/31779#issuecomment-2630755945)
~0.

> However, the perspective has since https://github.com/bitcoin/bitcoin/pull/30905#issuecomment-2426263301.

#30905 wasn't merged because it just took upstreams compilation flags, which didn't make sense for us, and because switching came with pretty much no benefit (and still couldn't be done without other modifications to the subtree).

I think time would be better spent modifying our subtree, so we could use the CMake file (if that's what we really want to do), rather than trying t
...

💬 fanquake commented on pull request "Pass custom DEP_OPTS and CONFIG_FLAGS to guix-build":
(https://github.com/bitcoin/bitcoin/pull/31763#issuecomment-2630760306)
~0. The benefit here seems limited, as the project has no need for these options in its release process. Outside of that, anyone wanting to do a "custom" release build, is likely already modifying the source, so can change the script for their own needs. As mentioned in the PR description, arbitrary changes to the CMake config, also don't really make sense, because they will cause failures for various reasons (i.e disabling the GUI would then cause failures at the packaging steps), and if you ne
...

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1939287479)
I'd rather remove the line, it might have had a use before that no longer applies.

If we want to get fees via the CBLockTemplate then it should just get a field for that.

But I don't think it's useful to do any of that, because this (duplicate) code will likely go away soon: https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1937451223

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1939291428)
Step (3) doesn't happen. A shutdown doesn't null out `m_tip_block`.

💬 hebasto commented on pull request "build: simplify by flattening the dependency graph":
(https://github.com/bitcoin/bitcoin/pull/30911#issuecomment-2630847614)
It seems I lack a deep understanding of CMake's underlying behaviour.

The comment in c4e498300c7e6b23dc464eca75a2bc9f86270dab states:
> Specifically, this prevents generated test headers (such as
test/data/base58_encode_decode.json.h) from depending on the
dependencies of test_bitcoin (libcrc32c.a libcrc32c_sse42.a libleveldb.a)
>
> Note that this is currently only relevant for Ninja.

And this aligns perfectly with the CMake [docs](https://cmake.org/cmake/help/latest/command/add_custo
...

💬 laanwj commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1939354206)
i guess it is to make it link qt6 statically

💬 Sjors commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1939369971)
`MaybeFlipIPv6toCJDNS` only does something if `addr_(bind_)` `IsIPv6()`, which checks `m_net == NET_IPV6`.

This is the default for a `CNetAddr`, however deeply buried in `-ThreadI2PAccept` - > `i2p::sam::Session::Accept` -> `DestB64ToAddr` -> `DestBinToAddr` -> `CNetAddr::SetSpecial` -> `SetI2P` do we find `m_net = NET_I2P`. So the flip won't interfere with an I2P connection (in the case its prefix coincidentally matches the IPv6 CJDNS prefix).

💬 laanwj commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1939378508)
Wait, X11 is still the only backend we support after this change, right?

Do you have X installed at all, or is the system wayland-only?

`libwayland-cursor0` is for native wayland usage, not for using xserver-wayland. XCB is still required if you want to use emulated X.

💬 laanwj commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1939393293)
Oh, that list has all of the other libxcb components, just not -cursor
```
libxcb-damage0:amd64 1.15-1ubuntu2
libxcb-dri2-0:amd64 1.15-1ubuntu2
libxcb-dri3-0:amd64 1.15-1ubuntu2
libxcb-glx0:amd64 1.15-1ubuntu2
libxcb-icccm4:amd64 0.4.1-1.1build3
libxcb-image0:amd64 0.4.0-2build1
libxcb-keysyms1:amd64 0.4.0-1build4
libxcb-present0:amd64 1.15-1ubuntu2
libxcb-randr0:amd64 1.15-1ubuntu2
libxcb-render-util0:amd64 0.3.9-1build4
libxcb-render0:amd64 1.15-1ubuntu2
libxcb-res0:amd64 1.15-1ub
...

💬 hebasto commented on pull request "build: set build type and per-build-type flags as early as possible":
(https://github.com/bitcoin/bitcoin/pull/31711#discussion_r1939413700)
Is the default build configuration really important for detecting the compiler and its capabilities?

🤔 hebasto reviewed a pull request: "build: set build type and per-build-type flags as early as possible"
(https://github.com/bitcoin/bitcoin/pull/31711#pullrequestreview-2589973088)
Tested 240ff782ad5a8974aff35e78356520917926e525 on Ubuntu 24.10 with CMake 3.30.3.

The `CMAKE_BUILD_TYPE` cache variable lost their help string:
- on the master @ d7f56cc5d9e12ad31dd1ce8b34c3ff4ec5c1b70c:
```
$ cmake -B build -G "Ninja" -LH 2>&1 | grep -B 1 CMAKE_BUILD_TYPE:
// Choose the type of build, options are: None Debug Release RelWithDebInfo MinSizeRel ...
CMAKE_BUILD_TYPE:STRING=RelWithDebInfo
```
- with this PR:
```
$ cmake -B build -G "Ninja" -LH 2>&1 | grep -B 1 CMAKE_BUI
...

💬 Sjors commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1939408935)
In 91d97a198fb57d47870ce094454c3776bc737ba0 "net: split CConnman::ConnectNode()": looking at the body of `ConnectAndMakeNodeId` the code paths with and without proxy are vastly different. They also have separate call sites.

So instead of using an `std::variant`, it seems more clear to just have both `ConnectAndMakeNodeId` and `ProxyConnectAndMakeNodeId`.

💬 Sjors commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1939396960)
In 91d97a198fb57d47870ce094454c3776bc737ba0 "net: split CConnman::ConnectNode()": why this new `assert`? If you need it all, maybe make it an `Assume`?

💬 vasild commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1939436862)
But what about this comment:

https://github.com/bitcoin/bitcoin/blob/1172bc4157eefe80d1aaf0b56459857ec651e535/src/node/kernel_notifications.h#L61-L62

related:

https://github.com/bitcoin/bitcoin/blob/1172bc4157eefe80d1aaf0b56459857ec651e535/src/init.cpp#L1817-L1827

📝 kevkevinpal opened a pull request: "test: added additional coverage to waitforblock and waitforblockheight rpc's"
(https://github.com/bitcoin/bitcoin/pull/31784)
Similar to https://github.com/bitcoin/bitcoin/pull/31746

This adds test coverage to the `waitforblock` and `waitforblockheight` rpc's by adding a test to assert we get an rpc error if we include a negative timeout

💬 laanwj commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1939445814)
```
# collection of types that map to MIME and file types
```