👍 BrandonOdiwuor approved a pull request: "Update Node window title with the chain type"
(https://github.com/bitcoin-core/gui/pull/758#pullrequestreview-1739931365)
tested ACK 9d37886a3b6ce24f4a4a05193eb0d071655a8457


💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1399277318)
bf467f8286425b692a1736ab6d417d0ba6074658: history can be expanded:

```md
* Cluster mempool introduced, dropping rule 2 and introducing rule 7. As of **v27.0** ([PR 28676](https://github.com/bitcoin/bitcoin/pull/28676)).
```

💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1399277913)
Ok removed the 0 padding in 1b0fa12fa5

💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#issuecomment-1819164214)
Updated to address luke's comment and added a test to ensure rpccookieperms are being applied

💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1399281690)
bf467f8286425b692a1736ab6d417d0ba6074658: `; new chunk feerate %s <= old chunk feerate`

💬 hebasto commented on pull request "depends: fix libmultiprocess build on aarch64":
(https://github.com/bitcoin/bitcoin/pull/28846#discussion_r1399309912)
> Rather than adding lib64/ to the pkg-config and link flags, I opted for always installing into lib/.

Mind sharing your reasoning behind this decision? Asking because the alternative looks more generic and will work if any other dependency package will switch to CMake.

---

A side note, not directly related to this PR: the CMake-based build system has no such an issue at all.

💬 ryanofsky commented on pull request "depends: fix libmultiprocess build on aarch64":
(https://github.com/bitcoin/bitcoin/pull/28846#discussion_r1399329330)
> > Rather than adding lib64/ to the pkg-config and link flags, I opted for always installing into lib/.
>
> Mind sharing your reasoning behind this decision?

The quote you're responding is from fanquake, but I think it makes sense for the depends build to hardcode "lib" here because it is already hardcoding "lib" in `config.site.in`:

https://github.com/bitcoin/bitcoin/blob/d752349029ec7a76f1fd440db2ec2e458d0f3c99/depends/config.site.in#L91

So this change just keeps the build consist
...

💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#issuecomment-1819230080)
I was wondering if we can drop RBF rule 5 (in a followup), but I'm guessing not really. My initial thinking was the cluster limit could be used instead. But `CalculateMiningScoreOfReplacementTx` only checks that the _new_ cluster doesn't get too big.

But does the new cluster system make it less of a burden to have large numbers of transactions appear and disappear from the mempool?

💬 brunoerg commented on pull request "fuzz: add target for `DescriptorScriptPubKeyMan`":
(https://github.com/bitcoin/bitcoin/pull/28578#issuecomment-1819233630)
force-pushed to fix CI error

👋 brunoerg's pull request is ready for review: "fuzz: add target for `DescriptorScriptPubKeyMan`"
(https://github.com/bitcoin/bitcoin/pull/28578)

💬 hebasto commented on issue "Sync slow":
(https://github.com/bitcoin/bitcoin/issues/26063#issuecomment-1819241844)
> Please reopen the issue, I will try to give logs with lock conflicts later. Probably related #776

Windows and Fedora, which was mentioned by this issue author, are quite different systems.

The rule of thumb for Bitcoin Core users on Windows is to add the data and block directories into Windows Security exclusions.

✅ maflcko closed an issue: "Sync slow"
(https://github.com/bitcoin/bitcoin/issues/26063)

💬 maflcko commented on issue "Sync slow":
(https://github.com/bitcoin/bitcoin/issues/26063#issuecomment-1819250607)
> Windows and Fedora, which was mentioned by this issue author, are quite different systems.

Ok, closing again. Feel free to file a new issue if this happens again. Make sure to include details, as explained above. You may reference this issue, if you want.

💬 maflcko commented on pull request "fuzz: add target for `DescriptorScriptPubKeyMan`":
(https://github.com/bitcoin/bitcoin/pull/28578#discussion_r1399346490)
style nit: Please put `std::` includes in a new section

💬 maflcko commented on pull request "fuzz: add target for `DescriptorScriptPubKeyMan`":
(https://github.com/bitcoin/bitcoin/pull/28578#discussion_r1399351848)
```suggestion
WalletDescriptor w_desc{(std::move(parsed_desc), 0, 0, 1, 1)};
```

nit: no need to mention the type twice for a single symbol

💬 maflcko commented on pull request "fuzz: add target for `DescriptorScriptPubKeyMan`":
(https://github.com/bitcoin/bitcoin/pull/28578#discussion_r1399350026)
```suggestion
Chainstate& chainstate = node.chainman->ActiveChainstate();
```

any reason to turn a reference into a pointer when you can keep the reference?

💬 pinheadmz commented on pull request "coins: make sure PoolAllocator uses the correct alignment":
(https://github.com/bitcoin/bitcoin/pull/28913#issuecomment-1819261340)
confirmed unit passes on arm32 but also passes without the actual bugfix commit. so effectively this on top of d752349029:


```diff
commit 2c7f49e83a5d05a5ba8b29d52108c14bf4d65af7
Author: Martin Leitner-Ankerl <martin.ankerl@gmail.com>
Date: Sun Nov 19 18:43:15 2023 +0100

pool: change memusage_test to use int64_t

If alignment of the PoolAllocator would be insufficient, then the test would fail. This also catches the issue with ARM 32bit, where int64_t is aligned to 8 b
...

👍 TheCharlatan approved a pull request: "build: Windows SSP roundup"
(https://github.com/bitcoin/bitcoin/pull/28461#pullrequestreview-1740074663)
ACK f95af98128f17002bf137a48441167020f3ef9bb

I get the same hashes as @hebasto

💬 maflcko commented on pull request "fuzz: add target for `DescriptorScriptPubKeyMan`":
(https://github.com/bitcoin/bitcoin/pull/28578#discussion_r1399366365)
Using clang-tidy named args would be better than just listing `0,0,1,1`, which is hard to understand without meaning.

👍 stickies-v approved a pull request: "Drop CAutoFile"
(https://github.com/bitcoin/bitcoin/pull/28904#pullrequestreview-1740095172)
ACK 4eb2a9ea4b6262bec0bc7c20cb3e684ea75caf42

🤔 maflcko reviewed a pull request: "fuzz: add target for `DescriptorScriptPubKeyMan`"
(https://github.com/bitcoin/bitcoin/pull/28578#pullrequestreview-1740110220)
lgtm ACK 7774df96942ba10aeb5c818b100aedc9cb7d4b4f