🤔 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

🚀 achow101 merged a pull request: "Add multiplication operator to CFeeRate"
(https://github.com/bitcoin/bitcoin/pull/29037)

💬 ajtowns commented on pull request "refactor: Print verbose serialize compiler error messages":
(https://github.com/bitcoin/bitcoin/pull/29056#issuecomment-1863893308)
reACK fae526345de539ab8f9b80100f6dfbe8e1d3284b

💬 ajtowns commented on pull request "logging: Simplify API for level based logging":
(https://github.com/bitcoin/bitcoin/pull/28318#discussion_r1432280734)
Added notes for these.