💬 ryanofsky commented on pull request "Fix crash when closing wallet":
(https://github.com/bitcoin-core/gui/pull/835#issuecomment-2350959839)
One way to test difference between the current fix and suggested fix is to delay the notification:

```diff
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -174,7 +174,10 @@ bool RemoveWallet(WalletContext& context, const std::shared_ptr<CWallet>& wallet
context.wallets.erase(i);
}
// Notify unload so that upper layers release the shared pointer.
- wallet->NotifyUnload();
+ std::thread([wallet]() {
+ std::this_thread::sleep_for(std::chrono::s
...

📝 hebasto opened a pull request: "cmake: Revamp handling of data files for `{test,bench}_bitcoin` targets"
(https://github.com/bitcoin/bitcoin/pull/30901)
This PR leverages the approach from the https://github.com/chaincodelabs/libmultiprocess project and introduces a new `target_data_sources()` function, which minimizes code needed to assign a `*.json` or `*.raw` data file to the `test_bitcoin` or `bench_bitcoin` target.

💬 hebasto commented on pull request "cmake: Revamp handling of data files for `{test,bench}_bitcoin` targets":
(https://github.com/bitcoin/bitcoin/pull/30901#issuecomment-2350962600)
Friendly ping @ryanofsky :)

💬 hebasto commented on pull request "contrib: fix check-deps.sh to check for weak symbols":
(https://github.com/bitcoin/bitcoin/pull/30415#discussion_r1759725111)
The comment above no longer seems correct.

📝 hebasto opened a pull request: "Remove Autotools packages from depends and CI"
(https://github.com/bitcoin/bitcoin/pull/30902)
This PR is a follow-up to https://github.com/bitcoin/bitcoin/pull/30752 and addresses https://github.com/bitcoin/bitcoin/pull/30752#discussion_r1758594864.

💬 hebasto commented on pull request "guix: Drop unused autotools packages":
(https://github.com/bitcoin/bitcoin/pull/30752#discussion_r1759727460)
Thanks! Addressed in https://github.com/bitcoin/bitcoin/pull/30902.

💬 fanquake commented on pull request "Remove Autotools packages from depends and CI":
(https://github.com/bitcoin/bitcoin/pull/30902#discussion_r1759727845)
I don't think this is correct. Given we still have depends packages that can output libtool archives (which we currently remove).

💬 Sjors commented on pull request "ci: honor ci bypass prefix in test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/30898#issuecomment-2350973678)
> when opening or updating a PR that only touches a markdown file, I'd use this

We might still want to run spell check and such things for documentation changes. Skipping unnecessary tasks could be done by CI itself. So I don't think that's an argument for this PR.

💬 hebasto commented on pull request "contrib: fix check-deps.sh to check for weak symbols":
(https://github.com/bitcoin/bitcoin/pull/30415#discussion_r1759729804)
Adjusted in https://github.com/bitcoin/bitcoin/pull/30901.

💬 hebasto commented on pull request "Remove Autotools packages from depends and CI":
(https://github.com/bitcoin/bitcoin/pull/30902#discussion_r1759730052)
Thanks! Reverted back.

💬 jonatack commented on pull request "ci: honor ci bypass prefix in test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/30898#issuecomment-2350987707)
> > when opening or updating a PR that only touches a markdown file, I'd use this
>
> We might still want to run spell check and such things for documentation changes.

Insofar as the spelling linter in the CI doesn’t raise, running it in the CI isn't very important -- it's a facultative aid that can be run locally by authors and reviewers. Thus, I'd use it on my own pulls to avoid wasting CI cycles where they aren't needed.

👍 pablomartin4btc approved a pull request: "qt: Translations update"
(https://github.com/bitcoin/bitcoin/pull/30899#pullrequestreview-2304483433)
ACK ae0529576147a1a5bee992574e2cefc8a1fa37d0

<details>
<summary>I've run the translation update tool on <code>master</code> and compared the changes with this branch.</summary>

```

./../bitcoin-maintainer-tools/update-translations.py

git diff --cached --name-only -- src/qt/locale
src/qt/locale/bitcoin_am.ts
src/qt/locale/bitcoin_bn.ts
src/qt/locale/bitcoin_de.ts
src/qt/locale/bitcoin_de_CH.ts
src/qt/locale/bitcoin_gl_ES.ts
src/qt/locale/bitcoin_ru.ts
src/qt/locale/bitcoin_sw.
...

💬 naiyoma commented on pull request "p2p: Increase inbound capacity for block-relay only connections":
(https://github.com/bitcoin/bitcoin/pull/28463#discussion_r1759742652)
nit: I’m trying to understand something. According to the PR description, 50% of inbound slots are allocated for tx-relaying peers, but the commit message seems to imply that 50% are for block-relay-only peers. Does this mean that the inbound slots are equally split between tx-relaying and block-relay-only peers? If so, could the description be expanded for clarity?

💬 pablomartin4btc commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#issuecomment-2351018530)
Since you are here, you could update the translate instructions as well (I've tested them), if @hebasto agrees with the changes:

- `doc/translation_process.md`

```diff
@@ -18,8 +18,8 @@ We use automated scripts to help extract translations in both Qt, and non-Qt sou

To automatically regenerate the `bitcoin_en.ts` file, run the following commands:
```sh
-cd src/
-make translate
+cmake -B build -DWITH_BDB=ON -DBUILD_GUI=ON
+cmake --build build --target
...

📝 hebasto opened a pull request: "cmake: Add `FindZeroMQ` module"
(https://github.com/bitcoin/bitcoin/pull/30903)
This PR introduces the `FindZeroMQ` module, which first attempts to find the `libzmq` library using CMake's `find_package()` and falls back to `pkg_check_modules()` if unsuccessful.

Resolves https://github.com/bitcoin/bitcoin/issues/30876.

💬 ismaelsadeeq commented on issue "Intermittent failure in feature_fee_estimation.py in sanity_check_rbf_estimates: est_feerate = node.estimatesmartfee(2)["feerate"] (KeyError: 'feerate')":
(https://github.com/bitcoin/bitcoin/issues/30640#issuecomment-2351038000)
I read the log from [https://api.cirrus-ci.com/v1/task/5165357323780096/logs/ci.log](https://api.cirrus-ci.com/v1/task/5165357323780096/logs/ci.log) and tried to recreate this intermittent failure.
```
cmake -B build -DWITH_ZMQ=ON -DREDUCE_EXPORTS=ON -DCMAKE_C_FLAGS="-g0 -O2 -funsigned-char" -DCMAKE_CXX_FLAGS="-g0 -O2 -funsigned-char -DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE"
```

I used the seed where the failure occurred was but it passes locally:
<details>
<summary>logs</summary>

```
bu
...

💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1759630712)
Removed this generate() call.

🤔 jamesob reviewed a pull request: "rpc: add getdescriptoractivity"
(https://github.com/bitcoin/bitcoin/pull/30708#pullrequestreview-2304336437)
I've pushed an update addressing feedback and adding release notes. Thanks for all review so far.

💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1759630564)
I'm in favor of leaving this in as a convenience to the end user, although I think the suggestion to include a hex-encoded sPK is a good one and I'll add that. Many wallets will ultimately want to show which address is being spent from, e.g.


If we can inexpensively determine that here, I think it's a nice option for the end user.

In terms of a blank string vs. omitted key, I think it's better to h
...

💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1759640764)
This value is used as an invalid hash elsewhere (https://github.com/bitcoin/bitcoin/blob/master/test/functional/wallet_listsinceblock.py#L95) so I'm going to leave this as is.