✅ 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

🤔 S3RK reviewed a pull request: "wallet, rpc: `FundTransaction` refactor"
(https://github.com/bitcoin/bitcoin/pull/28560#pullrequestreview-1790386436)
I'm still trying to fully grasp the existing code (which is a big mess), so bear with me if I don't make sense. Left some comment/questions based on my current understanding.

💬 S3RK commented on pull request "wallet, rpc: `FundTransaction` refactor":
(https://github.com/bitcoin/bitcoin/pull/28560#discussion_r1432414337)
Same comment as for other `FundTransaction`. I think we should get rid of ambiguity in input parameters and don't pass `tx`. In this case it's even more confusing, because `tx.vout` is actually still checked.

💬 S3RK commented on pull request "wallet, rpc: `FundTransaction` refactor":
(https://github.com/bitcoin/bitcoin/pull/28560#discussion_r1432396784)
`has_data` never set to `true`

Also no tests fail, so it seems there is no test coverage for that

💬 S3RK commented on pull request "wallet, rpc: `FundTransaction` refactor":
(https://github.com/bitcoin/bitcoin/pull/28560#discussion_r1432411126)
Not a huge fan of this change. Now if we pass tx.vout they'll be ignored.

I'd prefer if we pass `txin` and `recipients` and don't pass `tx` at all. That would be a clearer interface in my opinion.

💬 S3RK commented on pull request "wallet, rpc: `FundTransaction` refactor":
(https://github.com/bitcoin/bitcoin/pull/28560#discussion_r1432392999)
Let's not pass all the options as we need only one of them.

💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#issuecomment-1864100266)
Finally submitted the optimized code.
I also [tried caching](https://github.com/naumenkogs/bitcoin/commit/ae19891dfb588daefcda475a860464587f0480dc), but the speed went _down_ 10x (either map or unordered_map). Perhaps @sipa @mzumsande have interest in looking what could be the issue.

💬 fanquake commented on pull request "refactor: Remove Span operator==, Use std::ranges::equal":
(https://github.com/bitcoin/bitcoin/pull/29071#issuecomment-1864156198)
> Looks like this also fails on macOS on GHA.

Pretty sure that job should be rerun, because it's failing in the brew install stage.