📝 kevincatty opened a pull request: "chore: remove redundant words in comment"
(https://github.com/bitcoin/bitcoin/pull/32037)
remove redundant words in comment

✅ fanquake closed a pull request: "chore: remove redundant words in comment"
(https://github.com/bitcoin/bitcoin/pull/32037)

💬 maflcko commented on pull request "Require sqlite when building the wallet":
(https://github.com/bitcoin/bitcoin/pull/31961#issuecomment-2716994263)
No strong opinion about depends (follow-up or here), otherwise:

review ACK ac6cde731314d981391bc313c98d671c68211d33 🐥

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krx
...

💬 maflcko commented on pull request "Require sqlite when building the wallet":
(https://github.com/bitcoin/bitcoin/pull/31961#discussion_r1990912432)
In any case, this is unrelated to the changes here. If this is done, it would have to be done for all python packages like py-zmq, not only py-sqlite, in a separate change.

💬 fanquake commented on pull request "depends: patch around PlacementNew issue in capnp":
(https://github.com/bitcoin/bitcoin/pull/31998#discussion_r1990914356)
Thanks, I've adjusted this to use `-p2` (should have just done this in the first place rather than adjusting the paths).

💬 maflcko commented on pull request "tests: Improve stderr validation in test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/31966#issuecomment-2717005621)
Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

💬 fanquake commented on pull request "cmake: Check for `makensis` and `zip` tools before using them for optional `deploy` targets":
(https://github.com/bitcoin/bitcoin/pull/32019#discussion_r1990943885)
We also don't do this for other similar targets, like `docs`. Where we just print `"Error: Doxygen not found"`.

💬 maflcko commented on pull request "qt: 29.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/32004#issuecomment-2717053653)
> I'd like to reiterate my point that Transifex is poorly suited for Bitcoin Core translations, and I'm keen to use GitHub to track translation changes (as was done many years ago). This seems doable in the context of the recently discussed separation of the GUI repository.

The gui repository is already a separate monotree in https://github.com/bitcoin-core/gui, so it should be possible to drop transifex already today, if translators and translation reviewers prefer to use GitHub. Maybe I am
...

👍 dergoegge approved a pull request: "validation, fix: Use wtxid instead of txid in `CheckEphemeralSpends`"
(https://github.com/bitcoin/bitcoin/pull/32025#pullrequestreview-2677593378)
Code review ACK e637dc2c01c3b566e6c51c911c5881a8d206c924

💬 dergoegge commented on pull request "cmake: Avoid fuzzer "multiple definition of `main'" errors":
(https://github.com/bitcoin/bitcoin/pull/31992#issuecomment-2717130966)
Why do we want IPC enabled for the fuzzing builds? It's not used/tested by our fuzzing harnesses.

💬 laanwj commented on issue "guix: re-enable exported symbol checking for RISC-V":
(https://github.com/bitcoin/bitcoin/issues/28095#issuecomment-2717186516)
Filed an upstream issue for binutils: https://sourceware.org/bugzilla/show_bug.cgi?id=32783

💬 Sjors commented on pull request "Require sqlite when building the wallet":
(https://github.com/bitcoin/bitcoin/pull/31961#issuecomment-2717203847)
I initially considered dropping the `NO_WALLET` option from depends. Most other depends packages refer to the library name rather than the feature, so this seemed better than dropping `NO_SQLITE`.

However it could break automations (and habits) of anyone who tries to build the node without wallet support. So I ended up dropping `NO_SQLITE` and keeping `NO_WALLET`.

Also rebased after #31161 just in case.

💬 Sjors commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#issuecomment-2717384152)
This seems like a nice simplification. It would be good to do before nobody remembers what the old code was trying to do (and prevent). Since @theUni tried something similar before in #10738 it would be great if he can look at this again.

In particular the 2017 PR claimed "special care must be taken" as to which thread destroys the object, but @vasild believes this doesn't matter (here): https://github.com/bitcoin/bitcoin/pull/32015#discussion_r1984875189

The end result of that PR seems a
...

💬 Sjors commented on pull request "Update `secp256k1` subtree to latest master":
(https://github.com/bitcoin/bitcoin/pull/32028#issuecomment-2717395528)
Concept ACK on (temporarily) using master instead of a release tag. However we should probably wait with merging this until #31507 has at least one code review ACK from a build system expert?

ACK 7803eb4a51f3a7a7e76b4f45fb211ffa87a7e9bd

Ran the `git-subtree-check.sh` check. I didn't review the upstream changes.

On (ARM) macOS 15.3.1 I ran a testnet4 IBD with a debug build.

🤔 rkrux reviewed a pull request: "Require sqlite when building the wallet"
(https://github.com/bitcoin/bitcoin/pull/31961#pullrequestreview-2677975455)
Concept ACK 47e5ed3a5ba21049a9a8f3c4c44023e82e6ab88e

I've checked that there are no more instances of the following make options present and `WITH_SQLITE` doesn't appear anymore in the output of `ccmake build`.

```
➜ bitcoin git:(2025/02/sqlite) ✗ git grep "USE_SQLITE"
➜ bitcoin git:(2025/02/sqlite) ✗ git grep "WITH_SQLITE"
➜ bitcoin git:(2025/02/sqlite) ✗ git grep "NO_SQLITE"
```

Since the PR is rebased after the #31161 changes, I rm-rf'ed my build directory before configuring
...

💬 ismaelsadeeq commented on issue "Fee Estimation via Fee rate Forecasters tracking issue":
(https://github.com/bitcoin/bitcoin/issues/30392#issuecomment-2717491587)
Following some positive feedback from the recent in-person Core Dev meeting, a notable one is from @ajtowns who suggest exposing the data used for the mempool health check

I’ve made several improvements:

Updating the design document to reflect recent improvements and is now shared on
github gist for more review and feedback https://gist.github.com/ismaelsadeeq/bcfa6b1216eef10076665551d9885722)

#### Updated the Complete Feature Implementation

**Mempool Health Check**:
The node’s mem
...

💬 hebasto commented on pull request "Require sqlite when building the wallet":
(https://github.com/bitcoin/bitcoin/pull/31961#issuecomment-2717498680)
> However, the `NO_WALLET=1` option (used like below) seemed to have no effect and I was able to use the wallet still. PLMK if I used this option incorrectly. It was only after I used `ENABLE_WALLET=OFF` that I was not able to use the wallet RPCs.
>
> ```
> cmake -B build -DNO_WALLET=1 -LH
> ```

`NO_WALLET=1` is dedicated to be used when building depends: `make -C depends NO_WALLET=1`.

💬 laanwj commented on pull request "cmake: Check for `makensis` and `zip` tools before using them for optional `deploy` targets":
(https://github.com/bitcoin/bitcoin/pull/32019#discussion_r1991235225)
i kind of liked the mention of `MAKENSIS_EXECUTABLE`, it is my experience that it can be frustrating to find out what exact environment variable to set to point the build system at the right locations if you don't have everything installed in standard locations.

But yes maybe error messages aren't the best place for documentation.

🤔 hebasto reviewed a pull request: "Require sqlite when building the wallet"
(https://github.com/bitcoin/bitcoin/pull/31961#pullrequestreview-2678010665)
Approach ACK 47e5ed3a5ba21049a9a8f3c4c44023e82e6ab88e.

Here are two suggestions:
1. Simplify CMake code:
```diff
--- a/src/wallet/CMakeLists.txt
+++ b/src/wallet/CMakeLists.txt
@@ -28,6 +28,7 @@ add_library(bitcoin_wallet STATIC EXCLUDE_FROM_ALL
rpc/wallet.cpp
scriptpubkeyman.cpp
spend.cpp
+ sqlite.cpp
transaction.cpp
wallet.cpp
walletdb.cpp
@@ -37,17 +38,13 @@ target_link_libraries(bitcoin_wallet
PRIVATE
core_interface
bitcoin_common
+ $<TA
...

👍 hebasto approved a pull request: "qa: Enable feature_init.py on Windows"
(https://github.com/bitcoin/bitcoin/pull/32021#pullrequestreview-2678071942)
ACK 59c4930394cafc939eb396224b3d60d01ba0ce37, I have reviewed the code and it looks OK.