👍 kristapsk approved a pull request: "doc: Add link to needs-release-notes label"
(https://github.com/bitcoin/bitcoin/pull/29025#pullrequestreview-1770365172)
ACK fa88953d6fb54fdb47485981279632c693534108

💬 maflcko commented on pull request "mempool: Don't sort in entryAll":
(https://github.com/bitcoin/bitcoin/pull/29019#issuecomment-1845504355)
Well, if it is needed, then 453b4813ebc74859864803e9972b58e4be76a4d6 should not have been tagged "refactor", but "bugfix" and should be backported, along with a regression test?

💬 willcl-ark commented on pull request "doc: add historical release notes for 26.0":
(https://github.com/bitcoin/bitcoin/pull/29023#issuecomment-1845508806)
#28597

```md
Wallet or RPCs?
-------

- The `createwallet` RPC will no longer create legacy (BDB) wallets when
setting `descriptors=false` without also providing the
`-deprecatedrpc=create_bdb` option. This is because the legacy wallet is
being deprecated in a future release.
```

💬 fanquake commented on issue "The `streams_tests/xor_file` test fails on Windows":
(https://github.com/bitcoin/bitcoin/issues/29014#issuecomment-1845509929)
We run these unit tests in the Windows CI: https://cirrus-ci.com/task/6292832290340864?logs=ci#L2787. So is this going to be some difference between the CI binaries & Guix Windows binaries?

💬 fanquake commented on pull request "build: Require C++20 compiler":
(https://github.com/bitcoin/bitcoin/pull/28349#issuecomment-1845516141)
Guix Build (x86_64):
```bash
e5353b158953861a821ed757f1e82ecb059b936c42fea7d0f218c5f5271f9e6e guix-build-fa6e50d6c796/output/aarch64-linux-gnu/SHA256SUMS.part
03d479c397af94a67e6430f9f9538a8b3cc540358eb1e94859efb863dcff06ab guix-build-fa6e50d6c796/output/aarch64-linux-gnu/bitcoin-fa6e50d6c796-aarch64-linux-gnu-debug.tar.gz
11dafce4484ae16a74721c88b8f4b1b3dc87637639cdb70116bc0b1f498ea851 guix-build-fa6e50d6c796/output/aarch64-linux-gnu/bitcoin-fa6e50d6c796-aarch64-linux-gnu.tar.gz
c740b34
...

💬 centaur1 commented on issue "Build error on Ubuntu 22.04.3 LTS":
(https://github.com/bitcoin/bitcoin/issues/29017#issuecomment-1845524210)
Duh. Thanks. It hadn't needed that previously. :-)

💬 furszy commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#discussion_r1419126483)
> > What you are suggesting is ok for a post-mortem comment.
>
> I think you're misunderstanding my point. I am saying in the context of this PR (which is a bug fix), I don't think the log line you are adding here adds value and that a better thing to add in this PR would be a check that BnB is never returning change, which either errors or logs a warning.

I'm following the discussion. The starting point was "the logging line is not useful". So, I presented the arguments for which this lin
...

💬 maflcko commented on issue "The `streams_tests/xor_file` test fails on Windows":
(https://github.com/bitcoin/bitcoin/issues/29014#issuecomment-1845538078)
Maybe your Windows doesn't understand the "wbx" flags? (C++17, https://en.cppreference.com/w/cpp/io/c/fopen)

💬 fanquake commented on issue "Build error on Ubuntu 22.04.3 LTS":
(https://github.com/bitcoin/bitcoin/issues/29017#issuecomment-1845540998)
@centaur1 does that mean this is solved and issue should be closed?

✅ centaur1 closed an issue: "Build error on Ubuntu 22.04.3 LTS"
(https://github.com/bitcoin/bitcoin/issues/29017)

💬 maflcko commented on pull request "build: Require C++20 compiler":
(https://github.com/bitcoin/bitcoin/pull/28349#issuecomment-1845542104)
The benchmarks on https://corecheck.dev/bitcoin/bitcoin/pulls/28349 are grey ("normal"/expected)

💬 centaur1 commented on issue "Build error on Ubuntu 22.04.3 LTS":
(https://github.com/bitcoin/bitcoin/issues/29017#issuecomment-1845542435)
Yes. I thought I hit "comment and close" but didn't.

💬 hebasto commented on issue "The `streams_tests/xor_file` test fails on Windows":
(https://github.com/bitcoin/bitcoin/issues/29014#issuecomment-1845544516)
> So is this going to be some difference between the CI binaries & Guix Windows binaries?

Cross-compiled binaries fail, not only Guix.

💬 fanquake commented on issue "The `streams_tests/xor_file` test fails on Windows":
(https://github.com/bitcoin/bitcoin/issues/29014#issuecomment-1845548049)
> Cross-compiled binaries fail,

Ok, but they don't fail in our CI. So I guess this is some difference between Wine and actual Windows? Needs more info if we are going to be able to debug anything.

💬 maflcko commented on issue "The `streams_tests/xor_file` test fails on Windows":
(https://github.com/bitcoin/bitcoin/issues/29014#issuecomment-1845550256)
> Maybe your Windows doesn't understand the "wbx" flags? (C++17, https://en.cppreference.com/w/cpp/io/c/fopen)

So to clarify, can you re-try by removing the `x`? (I don't have Windows, so I can't try)

💬 stickies-v commented on pull request "mempool: Don't sort in entryAll":
(https://github.com/bitcoin/bitcoin/pull/29019#issuecomment-1845551334)
That is a good point, I'd missed that this just reverts back to the behaviour pre-`entryAll()`. It seems that when iterating over a `boost::multi_index_container` without specifying an index, we default to the insertion order, so i think that would be sufficient guarantees that the `CWallet::transactionAddedToMempool` calls happen in a safe order too.

💬 fanquake commented on issue "The `streams_tests/xor_file` test fails on Windows":
(https://github.com/bitcoin/bitcoin/issues/29014#issuecomment-1845552718)
According to https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/fopen-wfopen?view=msvc-170 `x` should be supported, but maybe it is broken.

💬 sdaftuar commented on pull request "refactor: Replace sets of txiter with CTxMemPoolEntryRefs":
(https://github.com/bitcoin/bitcoin/pull/28886#issuecomment-1845558921)
> However, using the iterator type in the setEntries set provides little benefit in practice as applied currently. Its purpose, ownership, and safety semantics often remain ambiguous, and it is hardly used for actually iterating through the data structures. So replace it where possible with CTxMemPoolEntryRefs.

I was thinking about this issue of ownership and safety semantics some more -- it seems like that most of the issues around iterators exist with CTxMemPoolEntryRefs as well, which is t
...

💬 fanquake commented on pull request "wallet: No BDB creation, unless -deprecatedrpc=create_bdb":
(https://github.com/bitcoin/bitcoin/pull/28597#issuecomment-1845562237)
Release note being added in #29023.

💬 josibake commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#discussion_r1419154526)
Right, but this is my concern: a bug is found and fixed, and then a log is added that _might_ have helped detect the already fixed bug (in this case, I'm still not convinced this log as written would have helped much). This pattern tends toward log bloat over time.

What I have been suggesting from the beginning is that we keep this PR focused on the bug fix and address logging in a separate PR where we can take a more holistic look. I do think logging the input parameters for coin selection i
...