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

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

GIven our minimum supported macOS (11.x), you could run anything from Xcode 12.5+ onwards, which is roughly LLVM/Clang 12+. Although I'd expect most users compiling from source on macOS, would be using a much more recent Xcode, so likely Clang ~14+ at least.

Also, anyone should be able to install and run the latest LLVM/Clang via brew.

💬 VojtechMyslivec commented on issue "gettransaction details does not include send to myself balance changes for imported addresses":
(https://github.com/bitcoin/bitcoin/issues/28555#issuecomment-1864179337)
Is there anything more I can provide to investigate this problem?

I would label this with ~bug, ~descriptors and ~wallet

Thanks

💬 ismaelsadeeq commented on pull request "RPC: Add maxfeerate and maxburnamount args to submitpackage":
(https://github.com/bitcoin/bitcoin/pull/28950#issuecomment-1864179391)
Concept ACK

💬 maflcko commented on pull request "[WIP] C++20 std::views::reverse":
(https://github.com/bitcoin/bitcoin/pull/28687#issuecomment-1864192035)
Though, on GHA it says XCode 14.3, see https://github.com/bitcoin/bitcoin/actions/runs/7171999335/job/19528203033?pr=28687#step:3:14

```
clang --version
Apple clang version 14.0.3 (clang-1403.0.22.14.1)
Target: x86_64-apple-darwin22.6.0
Thread model: posix
InstalledDir: /Applications/Xcode_14.3.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin