📝 fanquake locked a pull request: "bitcoin"
(https://github.com/bitcoin/bitcoin/pull/29106)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 fanquake commented on pull request "refactor: C++20: Use std::rotl":
(https://github.com/bitcoin/bitcoin/pull/29085#issuecomment-1860456382)
If we are going to change it here, should also switch in `hash.cpp`:

https://github.com/bitcoin/bitcoin/blob/c840dea27edfcfc67beb756ca6eda27b319f93d5/src/hash.cpp#L12-L15

Also cc @theuni.

📝 kristapsk opened a pull request: "Fix spelling errors"
(https://github.com/bitcoin/bitcoin/pull/29107)
Found these when running lint tests locally.

```
src/rpc/util.h:405: falsy ==> falsely, false
src/rpc/util.h:408: falsy ==> falsely, false
src/test/fuzz/package_eval.cpp:214: non-existant ==> non-existent
src/test/span_tests.cpp:56: memeber ==> member
^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/spelling.ignore-words.txt
```

Guess it's because I'm having different version of codespell?

In any case,
...

💬 stickies-v commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1430148600)
> No, `m_offsets` is not uninitialized

The array is initialized, but the items are not, and afaik `int64_t` doesn't have a default constructor?

💬 stickies-v commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1430149707)
Care to elaborate why? Since this is newly introduced code, seems sensible to do it right away?

🤔 ismaelsadeeq reviewed a pull request: "fuzz: make FuzzedDataProvider usage deterministic"
(https://github.com/bitcoin/bitcoin/pull/29043#pullrequestreview-1786902962)
>Good point, I guess since the spec is unspecified they have the right to change it but I would still be surprised if they do. I'm not sure if there is a trivial way to check if they ever have.

The order of evaluation issue is a characteristic of the C++ language standard. While the LLVM compiler might have a consistent order of evaluation, but generally compiler behavior could change, maybe we should not use the current LLVM behavior as validation for this but rather refer to the overall lan
...

💬 ismaelsadeeq commented on pull request "fuzz: make FuzzedDataProvider usage deterministic":
(https://github.com/bitcoin/bitcoin/pull/29043#discussion_r1430148489)
Nit: use descriptive variable names for more clarity instead of single letters
<details>
<summary>see diff</summary>

```diff
diff --git a/src/test/fuzz/crypto.cpp b/src/test/fuzz/crypto.cpp
index aa478277e35..2fa2b109e55 100644
--- a/src/test/fuzz/crypto.cpp
+++ b/src/test/fuzz/crypto.cpp
@@ -23,8 +23,8 @@ FUZZ_TARGET(crypto)
std::vector<uint8_t> data = ConsumeRandomLengthByteVector(fuzzed_data_provider);
if (data.empty()) {
auto new_size = fuzzed_data_prov
...

💬 dergoegge commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1430151965)
https://godbolt.org/z/5PKET7E39

💬 ismaelsadeeq commented on pull request "fuzz: make FuzzedDataProvider usage deterministic":
(https://github.com/bitcoin/bitcoin/pull/29043#discussion_r1430154424)
This is okay since the standard is left-to-right evaluation order for initializer-clauses within an initializer-list?

💬 stickies-v commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1430166505)
Ah cool, didn't know, thanks. Still, having default (0-)values in sorted_copy seems like it would lead to an incorrect median calculation?

🚀 fanquake merged a pull request: "fuzz: Improve fuzzing stability for minisketch harness"
(https://github.com/bitcoin/bitcoin/pull/29064)

💬 sipa commented on issue "Performance decrease after tapscript miniscript":
(https://github.com/bitcoin/bitcoin/issues/29098#issuecomment-1860568169)
That sounds very plausible, @darosior.

If so, I think there is an approach that doesn't lose optimality: keep track of the optimal satisfaction size, and once reached, don't bother evaluation further keys?

📝 maflcko opened a pull request: "refactor: Replace ALWAYS_FALSE with false"
(https://github.com/bitcoin/bitcoin/pull/29108)
Now that `static_assert(false)` when evaluated in a non-instantiated template is treated as a no-op, use it over the workaround, and remove the workaround.

💬 darosior commented on issue "Performance decrease after tapscript miniscript":
(https://github.com/bitcoin/bitcoin/issues/29098#issuecomment-1860578924)
Neat. This would give us the same performance in the large majority of cases while still being optimal in the rare non-SIGHASH_DEFAULT cases. In any case this would fix the slowdown you noticed @eriknylund.

✅ maflcko closed a pull request: "refactor: Replace ALWAYS_FALSE with false"
(https://github.com/bitcoin/bitcoin/pull/29108)

💬 maflcko commented on pull request "refactor: Replace ALWAYS_FALSE with false":
(https://github.com/bitcoin/bitcoin/pull/29108#issuecomment-1860587423)
Oh nvm, this is C++26

⚠️ c0deright opened an issue: "Old wallet.dat: Error reading wallet database: keymeta found with unexpected path / All keys read correctly, but transaction data or address metadata may be missing or incorrect"
(https://github.com/bitcoin/bitcoin/issues/29109)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

When starting bitcoind with an old wallet (created July 2019, probably `v0.17.x` or `v0.18.x`) the log records 2 errors and one warning that are not self-explanatory.

Might be related to #19051

### Expected behaviour

No warning/errors are shown or the errors/warnings are more detailed as to what's going on.

### Steps to reproduce

Run v26.0 with a very old `wallet.dat` file. I wasn't
...

💬 vasild commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1430198623)
I was just thinking on ways to simplify this. `CheckForStaleTipAndEvictPeers()` is executed every 45 seconds. I think it is ok if for a while (~ 10 mins) we think we are close to the tip but we are not in practice. Given that we realize this in a few minutes and don't get stuck with the wrong assumption forever.

👍 instagibbs approved a pull request: "wallet, mempool: propagete `checkChainLimits` error message to wallet"
(https://github.com/bitcoin/bitcoin/pull/28863#pullrequestreview-1787005073)
LGTM

💬 instagibbs commented on pull request "wallet, mempool: propagete `checkChainLimits` error message to wallet":
(https://github.com/bitcoin/bitcoin/pull/28863#discussion_r1430210897)
this would be good