🚀 fanquake merged a pull request: "build: disable external-signer for Windows"
(https://github.com/bitcoin/bitcoin/pull/28967)

📝 dergoegge converted_to_draft a pull request: "fuzz: Improve fuzzing stability for minisketch harness"
(https://github.com/bitcoin/bitcoin/pull/29064)
The `minisketch` harness has low stability due to:
* Rng internal to minisketch
* Benchmarkning for the best minisketch impl

Fix this by seeding the rng and fixing the impl to 0.

Also see #29018.

📝 fanquake opened a pull request: "build: use `-no_exported_symbols` on macOS"
(https://github.com/bitcoin/bitcoin/pull/29072)
This reduces the size of the binary by 2-3% when building with `--enable-reduce-exports`.

We wont get this in release builds (yet), as that would require moving to a newer cctools/ld64 (at least ld 907). However, this flag is also supported in lld, so we would end up getting it once that migration happens.

> -no_exported_symbols
> Useful for main executable that don't have plugins and thus need no symbol exports.

Can be tested with `dyld_info -exports src/bitcoind`. The only exported s
...

💬 furszy commented on issue "fuzz, coinselection: Assertion 'result_bnb->GetChange(coin_params.m_cost_of_change, CAmount{0}) == 0' failed":
(https://github.com/bitcoin/bitcoin/issues/28918#issuecomment-1853803534)
> This is still broken, please reopen:
>
> ```
> $ echo "AQ0N0NfM0B7QEK1AL3m3AJWVlQAAgQAAAAAAlZWVAACBAAAAAACVlZUAAAAAAAAAAAAAAAAAXQAAgQAAAAAAAAAAAAAA" | base64 --decode > coinselection-90005214eeb91a419ca37def8d958bf6bb670edf.crash
> $ FUZZ=coinselection ./src/test/fuzz/fuzz coinselection-90005214eeb91a419ca37def8d958bf6bb670edf.crash
> fuzz_libfuzzer: wallet/test/fuzz/coinselection.cpp:122: void wallet::coinselection_fuzz_target(FuzzBufferType): Assertion `result_bnb->GetChange(coin_param
...

💬 furszy commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#issuecomment-1853803752)
> See [#28918 (comment)](https://github.com/bitcoin/bitcoin/issues/28918#issuecomment-1853532671). This doesn't seem to have fixed the fuzzer.

It fixed the initially reported one (https://github.com/bitcoin/bitcoin/issues/28918#issue-2002147628). The latest one is different.

📝 maflcko converted_to_draft a pull request: "refactor: Remove Span operator==, Use std::ranges::equal"
(https://github.com/bitcoin/bitcoin/pull/29071)
`std::span` removed the comparison operators, so it makes sense to remove them for the `Span` "backport" as well. Using `std::ranges::equal` also has the benefit that some `Span` temporary constructions can now be dropped.

💬 maflcko commented on pull request "refactor: Remove Span operator==, Use std::ranges::equal":
(https://github.com/bitcoin/bitcoin/pull/29071#discussion_r1425233553)
Review note: `Span{privkey}` can not be removed, for now. This is because the type of `Span{privkey}` and `Span{privkey.begin(), privkey.end()}` are different.

💬 stevenroose commented on pull request "Add OP_TXHASH and OP_CHECKTXHASHVERIFY opcodes":
(https://github.com/bitcoin/bitcoin/pull/29050#issuecomment-1853850878)
I added a commit that has a few question marks about which contexts should have their own TxHashCache. I'm not entirely understanding which contexts have the `PrecomputedTxData` either. The `MutableTransactionSignatureCreator` is the most troubling, because since the `TransactionSignatureChecker` takes a ptr to `TxHashCache`, some cache needs to live outside of the `MutableTransactionSignatureCreator` while actually that cache could be empty on every call. So it seems like we actually might have
...

💬 furszy commented on issue "fuzz, coinselection: Assertion 'result_bnb->GetChange(coin_params.m_cost_of_change, CAmount{0}) == 0' failed":
(https://github.com/bitcoin/bitcoin/issues/28918#issuecomment-1853853445)
First glance, the solution is to calculate the minimum viable change variable in the same way we do inside the wallet (not setting it randomly as we currently do) and provide it to `GetChange()` instead of providing `cost_of_change`.
There is more information about this inside the long convo Murch and I had on https://github.com/bitcoin/bitcoin/pull/28395#pullrequestreview-1620951490.

The issue arises when `cost_of_change < selected_value - target`. Because we provide the cost of creating ch
...

⚠️ fanquake opened an issue: "ci: failure in `wallet_multiwallet.py --legacy-wallet`"
(https://github.com/bitcoin/bitcoin/issues/29073)
https://cirrus-ci.com/task/5692425725804544?logs=ci#L3370:
```bash
node0 2023-12-13T12:44:27.733356Z [httpworker.1] [rpc/request.cpp:187] [parse] [rpc] ThreadRPCServer method=unloadwallet user=__cookie__
test 2023-12-13T12:44:27.782000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/ci_container_base/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_fr
...

⚠️ fanquake opened an issue: "ci: MSVC failures"
(https://github.com/bitcoin/bitcoin/issues/29074)
The Windows native CI is failing on unrelated changes, i.e in #29072.
https://github.com/bitcoin/bitcoin/actions/runs/7194911390/job/19596459746?pr=29072:
```bash
rpc_help.py | ✖ Failed | 1 s
rpc_signer.py | ✖ Failed | 4 s
wallet_signer.py --descriptors | ✖ Failed | 3 s
```

```bash
node0 2023-12-13T12:48:04.392953Z [D:\a\bitcoin\bitcoin\src\rpc\request.cpp:187] [parse] [rpc]
...

💬 fanquake commented on issue "ci: MSVC failures":
(https://github.com/bitcoin/bitcoin/issues/29074#issuecomment-1853871903)
This is happening on master too, and is related to disabling external signer on windows.

👍 stickies-v approved a pull request: "refactor: Remove pre-C++20 code, fs::path cleanup"
(https://github.com/bitcoin/bitcoin/pull/29040#pullrequestreview-1779595471)
ACK 856c88776f8486446602476a1c9e133ac0cff510

👍 fanquake approved a pull request: "Bump minimum required Boost version due to migration to C++20"
(https://github.com/bitcoin/bitcoin/pull/29066#pullrequestreview-1779598310)
ACK 49a90915aa3ee8e3a7e163f23a55de931faf8523

💬 maflcko commented on issue "ci: MSVC failures":
(https://github.com/bitcoin/bitcoin/issues/29074#issuecomment-1853892781)
I guess the functional tests fail to see that there is no signer for some reason?

💬 sdaftuar commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-1853897416)
I thought of a couple issues with the current version of the PR that I wanted to summarize (longer form [here](https://delvingbitcoin.org/t/cluster-mempool-rbf-thoughts/156/23?u=sdaftuar)):

1. The mempool might not be made "strictly better", in the sense that there are more fees available for every size of transactions selected, using the RBF criteria in this PR. (I'm trying to figure out if there are any simple heuristics we can apply to resolve this issue.)

2. It occurred to me that use
...

🤔 maflcko reviewed a pull request: "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly"
(https://github.com/bitcoin/bitcoin/pull/28051#pullrequestreview-1779599067)
ACK 6db04be102807ee0120981a9b8de62a55439dabb 👗

<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+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK 6db04be102807ee0120981a9b8
...

💬 maflcko commented on pull request "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly":
(https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1425329431)
in 73133c36aa: Just to note another thing: Now that the interrupt inside the NodeContext is unset, unit tests can not use it and must use the m_interrupt

💬 maflcko commented on pull request "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly":
(https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1425340275)
Nit in the last commit: Shouldn't new node code be in the node namespace? Otherwise it will have to be touched again when it is moved into the node namespace later?

💬 vasild commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#issuecomment-1853978983)
Would this resolve the problem?

<details>
<summary>[patch] update g_initial_block_download_completed periodically based on our tip age</summary>

```diff
diff --git i/src/net_processing.cpp w/src/net_processing.cpp
index df54a62f28..ec9bbff8f2 100644
--- i/src/net_processing.cpp
+++ w/src/net_processing.cpp
@@ -2045,13 +2045,12 @@ void PeerManagerImpl::NewPoWValidBlock(const CBlockIndex *pindex, const std::sha
* Update our best height and announce any block hashes which weren't pre
...