💬 achow101 commented on 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#issuecomment-1863413296)
> Most are distinct but eleven key paths are found twice in the dump

Since there is an `inactivehdseed`, that means that the seed has been rotated, probably by encrypting the wallet. So these duplicated paths are expected. One of the keys is derived from the inactive seed, and the other from the current active seed.

> And then there are two with a very odd path: `m/0'/1'/299'/0'/1'/300'` and `m/0'/0'/299'/0'/0'/300'`.

This lines up with the errors in the debug.log.

These derivation p
...

💬 c0deright commented on 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#issuecomment-1863432854)
> Are there any keys with any of the derivation paths `m/0'/1'/299'`, `m/0'/1'/300'`, `m/0'/0'/299'`, or `m/0'/0'/300'`?

Only these two:

```
reserve=1 [..] hdkeypath=m/0'/0'/299'
reserve=1 [..] hdkeypath=m/0'/0'/300'
```

> If you are able to, try loading the wallet in 0.17 or earlier and then doing `dumpwallet`. If you lookup these two addresses (with the weird derivation paths) in the dump, what derivation paths do they have?

I did as you described and `dumpwallet` with 0.17.0 gi
...

💬 c0deright commented on 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#issuecomment-1863449058)
`bitcoin-wallet` doesn't create any file:

```
% bitcoin-wallet -datadir=. -dumpfile=026.dump -wallet=wallet.dat dump
Error reading wallet.dat! All keys read correctly, but transaction data or address book entries might be missing or incorrect.
```

🤔 furszy reviewed a pull request: "p2p: adaptive connections services flags"
(https://github.com/bitcoin/bitcoin/pull/28170#pullrequestreview-1789712050)
PR reworked based on the feedback. Thanks @vasild for the rounds of review.

The functionality is no longer tied to the last tip update time; is now linked to the actual tip time, which is now cached by ´PeerManager´ to prevent undesirable ´cs_main´ locks. This implies simpler code since we are no longer modifying ´TipMayBeStale()´ nor the ´m_initial_sync_finished´ field.

I still believe that we should implement some of the changes made earlier, such as the ´PeerManager´ IBD flag modificati
...

💬 achow101 commented on 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#issuecomment-1863491818)
> `bitcoin-wallet` doesn't create any file:

Oh, that's a bug, Should be simple to fix. In any case, I don't think that dump will be needed since the `dumpwallet` output should be enough to have an idea of what was stored.

***

The `dumpwallet` file has a xprv at the top. Can you try deriving the 4 keys at the derivation paths I mentioned above and see which match the lines with the weird derivation paths? I'm pretty sure the wallet just stored the wrong derivation path somehow.

💬 Sjors commented on pull request "[doc]: add doxygen comment describing what `CheckPackageLimits` returns":
(https://github.com/bitcoin/bitcoin/pull/29115#issuecomment-1863505558)
utACK 19bb65bf255df0f876e37de90fb8c4c6229cdf52

💬 Hamz0i commented on 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#issuecomment-1863514126)
reserve=1 [..] hdkeypath=m/0'/0'/299'
reserve=1 [..] hdkeypath=m/0'/0'/300'

💬 c0deright commented on 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#issuecomment-1863531177)
Will try tomorrow using https://github.com/dan-da/hd-wallet-derive

📝 achow101 opened a pull request: "wallettool: Always be able to dump a wallet's database"
(https://github.com/bitcoin/bitcoin/pull/29117)
https://github.com/bitcoin/bitcoin/issues/29109#issuecomment-1863449058 reports that a wallet with noncritical errors cannot be dumped with `bitcoin-wallet dump`. This was caused by an erroneous reset of the wallet pointer when the loading the wallet returns something other than `LOAD_OK`. Not all errors are errors that require aborting, so unilaterally resetting the pointer at that time is incorrect. The first commit resolves this issue.

Furthermore, if a wallet has loading errors, that shou
...

💬 achow101 commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1863562737)
> > [059d847](https://github.com/bitcoin/bitcoin/commit/059d847810051b52d33b5b16711a031c3fd199a9) looks good, except that you need to call `SetActiveHDKey()` in `CWallet::EncryptWallet` (to avoid needlessly running the upgrade code).
>
> Or maybe even add it to `SetupDescriptorScriptPubKeyMans(const CExtKey& master_key)`

It already does that in the `SetupDescriptorScriptPubKeyMans()` that is called by `EncryptWallet`.

💬 furszy commented on pull request "wallettool: Always be able to dump a wallet's database":
(https://github.com/bitcoin/bitcoin/pull/29117#discussion_r1431999422)
Not for this PR but could also provide the `-dumpfile` path arg instead of the entire `ArgsManager` and remove `<common/args.h>` dependency.

🤔 furszy reviewed a pull request: "wallettool: Always be able to dump a wallet's database"
(https://github.com/bitcoin/bitcoin/pull/29117#pullrequestreview-1789809065)
Code review ACK d83bea42d1

💬 maflcko commented on pull request "build: Bump guix time-machine to unlock riscv64 metal":
(https://github.com/bitcoin/bitcoin/pull/29078#issuecomment-1863598765)
Looks like this fails with:

<details><summary>macOS cross compile</summary>

```
make[4]: Leaving directory '/bitcoin/depends/work/build/x86_64-apple-darwin/native_libtapi/eb33a59f2e30ff9724dc1ea8bee8b5229b0557c9-dd025396ec5/build'
Built target install-tapi-headers
make[3]: Leaving directory '/bitcoin/depends/work/build/x86_64-apple-darwin/native_libtapi/eb33a59f2e30ff9724dc1ea8bee8b5229b0557c9-dd025396ec5/build'
/gnu/store/r5r4kqn2qmh1c4x0jq92z8zan5m0xwl4-cmake-minimal-3.24.2/bin/cmake
...

💬 maflcko commented on pull request "build: Bump guix time-machine to unlock riscv64 metal":
(https://github.com/bitcoin/bitcoin/pull/29078#issuecomment-1863602326)
Other than that:

```
# uname --machine && find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
riscv64
d137d8dc94b3d734965b7e50fac273f704e122f969430e1b43bfe21c9af31bbe guix-build-fa87a2072b91/output/aarch64-linux-gnu/SHA256SUMS.part
42d66d0daf6067a74c3ba4e55c012712ae5cca02c9123ed3ef7fd9a8f09c16d4 guix-build-fa87a2072b91/output/aarch64-linux-gnu/bitcoin-fa87a2072b91-aarch64-linux-gnu-debug.tar.gz
3ccda6af99c1dd4b54533d073d
...

💬 achow101 commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1863611791)
Intermittent failure should be fixed.

📝 rom213 opened a pull request: "Update compat.h"
(https://github.com/bitcoin/bitcoin/pull/29118)
<!--
*** 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
...

✅ maflcko closed a pull request: "Update compat.h"
(https://github.com/bitcoin/bitcoin/pull/29118)

📝 maflcko opened a pull request: "refactor: Use std::span over Span"
(https://github.com/bitcoin/bitcoin/pull/29119)
`std::span` allows static extents, which is a nice benefit over just `Span`.

However, the interface of the two isn't identical and requires some more changes than just defining an alias. This is my current draft to *compile* with `std::span`. This should be the minimal changes required to get a green CI, but the changes may not be ideal, so this remains a draft.

Also, this requires and is based on #29071, which is blocked on OSS-Fuzz.

In the meantime, changes that make sense on their ow
...

💬 maflcko commented on pull request "refactor: Remove Span operator==, Use std::ranges::equal":
(https://github.com/bitcoin/bitcoin/pull/29071#issuecomment-1863626492)
Looks like this also fails on macOS on GHA. Does anyone know what version(s) of clang can be expected to be available on macOS normally?

💬 achow101 commented on pull request "Add multiplication operator to CFeeRate":
(https://github.com/bitcoin/bitcoin/pull/29037#issuecomment-1863654003)
ACK 1757452cc55a6dacc62e4258043ee4d711fd281a