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

💬 ajtowns commented on pull request "logging: Simplify API for level based logging":
(https://github.com/bitcoin/bitcoin/pull/28318#discussion_r1432281400)
nit notted

💬 ben-arnao commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1863965277)
> > The point of "fighting the spam TXs" is not to prevent all forms of data inscription at all costs, it's to fix Bitcoin's fee market. The crux of the problem as described [#28408 (comment)](https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1852569550) and [#28408 (comment)](https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1852673575) is that inscriptions dramatically overpay fees by design and thus price out large segments of Bitcoin transactors at very little absolute cost
...