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

📝 fanquake locked 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
...

💬 ben-arnao commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1863993219)
Just my two cents.. if you look at this from a perspective of a bad actor paying to hurt BTC network, as long as there is a cost required to keep up the attack what is the issue? Trying to filter txs that we think are not legitimate seems like a risky road to go down.

💬 S3RK commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1863995929)
Code review ACK db6b61e9e7635c9cb97d73fd44b9e7266b8eef51

💬 kristapsk commented on pull request "include verbose debug messages in testmempoolaccept reject-reason":
(https://github.com/bitcoin/bitcoin/pull/28121#issuecomment-1864000603)
Concept ACK