💬 stickies-v commented on pull request "mempool: Don't sort in entryAll":
(https://github.com/bitcoin/bitcoin/pull/29019#issuecomment-1845497136)
> Current call sites of entryAll do not require the entries to be sorted

Will this PR not cause issues [here](https://github.com/TheCharlatan/bitcoin/blob/87fa444a04854a5d3a9ff283a8b6c34587e8430f/src/wallet/wallet.cpp#L1938)? In [`CWallet::transactionAddedToMempool`](https://github.com/TheCharlatan/bitcoin/blob/87fa444a04854a5d3a9ff283a8b6c34587e8430f/src/wallet/wallet.cpp#L1406), we eventually call `CWallet::AddToWalletIfInvolvingMe` where we have a [dependency](https://github.com/TheCharlat
...

📝 maflcko opened a pull request: "doc: Add link to needs-release-notes label"
(https://github.com/bitcoin/bitcoin/pull/29025)
This makes it easier to spot and not forget. C.f. https://github.com/bitcoin/bitcoin/pull/28597#issuecomment-1845299642

💬 ajtowns commented on pull request "refactor: Remove unused and fragile string interface from arith_uint256":
(https://github.com/bitcoin/bitcoin/pull/28924#issuecomment-1845501182)
ACK fa63f16018d9468e1751d2107b5102184ac2d5ae

👍 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.