💬 brunoerg commented on pull request "test: added fuzz coverage for consensus/merkle.cpp":
(https://github.com/bitcoin/bitcoin/pull/32243#discussion_r2083467901)
Why does it need `logging.h`?

💬 purpleKarrot commented on pull request "threading: drop CSemaphore in favor of c++20 std::counting_semaphore":
(https://github.com/bitcoin/bitcoin/pull/32466#issuecomment-2869668262)
When looking at the new file alone, I see several oportunities for improvement: Drop the `f` prefix, use default member initializers, use `std::exchange`. But when looking at the individual commits, I see that this is just moving existing code around, so it makes sense to do defer additional cleanup to a separate PR.

The code would have been easier to review if the last commit was not part of this PR, but it looks good.

ACK 6f7052a7b96f058568af9aed2f014ae7a25e0f68

💬 yope commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2869692955)
> Meta-protocols builders will find ways to store data on-chain regardless of the decision here. Currently it's in witness data. So this discussion does not solve any raised questions on storing data and its legality etc. However, enabling data in OP_RETURN does offer a number of benefits. Eg.:

I agree to this. But I don't think there are only benefits...

> * future meta-protocols may choose OP_RETURN as the storage option for their data (instead of witness)

And future undesirable meta
...

💬 lollerfirst commented on pull request "rpc:generatetomany":
(https://github.com/bitcoin/bitcoin/pull/32468#issuecomment-2869768454)
> > note the remainder in the last btc address
>
> what if you want to split the reminder between multiple addresses?

`
bitcoin-cli generatetomany "{\"bcrt1qal6p633hvwz2yp5mav0qy7u2az8gkn2xywnj6v\":1,\"bcrt1q3me8uxjzw2egq76hjw6zcgzmlhlzzztfwqzjfv\":1,\"bcrt1q57yu04dn4l0zxv6ul4prm7favr630d3lmwvjh0\":1,\"bcrt1qal6p633hvwz2yp5mav0qy7u2az8gkn2xywnj6v\":\"remainder\",
\"bcrt1qz07rr2j8r699c9r55rt44q9vm3faxgprlv62xx\":\"remainder\",
\"bcrt1q9vur8vqhfndr8r76eveuu2wr46f2rs8fgv2kvh\":\"remainder\
...

💬 maflcko commented on pull request "rpc:generatetomany":
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2083512984)
this is now dead code, according to https://corecheck.dev/bitcoin/bitcoin/pulls/32468

Generally, I am not sure about modifying real code to accommodate test-only code. It would be better to not modify `src/node` at all and just put the test-only code in the test-only code paths.

(Unless the code may be useful outside of tests, but then you'll have to explain how)

💬 kevkevinpal commented on pull request "test: added fuzz coverage for consensus/merkle.cpp":
(https://github.com/bitcoin/bitcoin/pull/32243#discussion_r2083516219)
thanks!

removed in [c5245d1](https://github.com/bitcoin/bitcoin/pull/32243/commits/c5245d1a76928c70cc25347bbf790246a98912a9)

💬 DuyzenZ commented on something "":
(https://github.com/bitcoin/bitcoin/commit/04a7a7a28cdca46d977e474436f7447157a52208#r156780678)
i dont running !

💬 ranathan14 commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2869852999)
**Reasons to Maintain Limitations on OP_RETURN in Bitcoin**

**1. Preserve Blockchain Pruning and Scalability**
* Bitcoin nodes need to process and store all transactions. If arbitrary data could be stored freely via OP_RETURN, it would bloat the UTXO set or blockchain size.
* Although OP_RETURN outputs are provably unspendable and don't enter the UTXO set, allowing large or unlimited data sizes could lead to excessive permanent data in the blockchain, making it harder for new nodes
...

💬 lollerfirst commented on pull request "rpc:generatetomany":
(https://github.com/bitcoin/bitcoin/pull/32468#issuecomment-2869943029)
@polespinasa Please check https://github.com/polespinasa/bitcoin/pull/1 and let me know what you think.

📝 hebasto opened a pull request: "cmake: Allow `WITH_DBUS` on all Unix-like systems"
(https://github.com/bitcoin/bitcoin/pull/32469)
This PR makes the `WITH_DBUS` option available on all Unix-like systems, not just Linux, thereby fixing a regression that was overlooked during the migration from Autotools.

Note: Enabling D-Bus support on macOS still makes no sense, since the `Notificator` class uses the User Notification Center regardless:https://github.com/bitcoin/bitcoin/blob/746ab19d5a13c98ae7492f9b6fb7bd6a2103c65d/src/qt/notificator.cpp#L43-L56

Fixes https://github.com/bitcoin/bitcoin/issues/32464.

💬 hebasto commented on issue "WITH_DBUS shouldn't be limited to Linux":
(https://github.com/bitcoin/bitcoin/issues/32464#issuecomment-2869961721)
@kev009

Thanks for reporting!

Fixed in https://github.com/bitcoin/bitcoin/pull/32469.

👍 hebasto approved a pull request: "Decouple WalletModel from RPCExecutor"
(https://github.com/bitcoin-core/gui/pull/841#pullrequestreview-2831443407)
ACK 002b792b9a85100d89e47706c29cf1fd355d9727, I have reviewed the code and it looks OK.

🚀 hebasto merged a pull request: "Decouple WalletModel from RPCExecutor"
(https://github.com/bitcoin-core/gui/pull/841)

🚀 hebasto merged a pull request: "qt, docs: Unify term "clipboard""
(https://github.com/bitcoin-core/gui/pull/871)

💬 k98kurz commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2870003040)
> > @BitcoinMechanic
> It does no harm to fee estimation or block propagation. Nodes can and do cache transactions they reject from their mempools making compact blocks just as quick to verify regardless of if some of their contents was filtered.

This is the first I have heard about this caching of non-standard transactions. If this is indeed the case, then the small-block-propagation-performance-degradation argument is largely invalidated, but this requires that the cache be sufficiently la
...

💬 polespinasa commented on pull request "rpc:generatetomany":
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2083578626)
> this is now dead code, according to https://corecheck.dev/bitcoin/bitcoin/pulls/32468

Will delete it, was just added as all the else statement code is not needed for the simeple `generatetoaddress`.

> Generally, I am not sure about modifying real code to accommodate test-only code. It would be better to not modify `src/node` at all and just put the test-only code in the test-only code paths.

I'm not understanding this part, what do you mean by test-only code? I don't see a way to add
...

✅ hMsats closed an issue: "Self-compiled bitcoind 29.0 much slower than self-compiled 28.0 on my system"
(https://github.com/bitcoin/bitcoin/issues/32455)

💬 hMsats commented on issue "Self-compiled bitcoind 29.0 much slower than self-compiled 28.0 on my system":
(https://github.com/bitcoin/bitcoin/issues/32455#issuecomment-2870019391)
Although the jury is still out, it might be related to what @sipa suggested: that the cache still hasn't warmed up enough and validation speed is temporarily slow. I'm testing different options like running with the pre-compiled bitcoind but every test takes quite a long time to come to a clear conclusion (days).

At least I got an answer to my questions of why my executables are so big. Closing for now, will reopen if this issue persists.

💬 polespinasa commented on pull request "rpc:generatetomany":
(https://github.com/bitcoin/bitcoin/pull/32468#issuecomment-2870019412)
> All fields with "remainder" to have the remainder split equally amongst them?

I don't see duplicating entries as a good option. Could lead to errors. I think is better to have two camps for each address entry, one for the amount (can be 0) and one for reminder (can be done with 0 or 1 or by setting "remainder" or an empty string)

💬 polespinasa commented on pull request "rpc:generatetomany":
(https://github.com/bitcoin/bitcoin/pull/32468#issuecomment-2870020026)
> @polespinasa @supertestnet Please check out [polespinasa#1](https://github.com/polespinasa/bitcoin/pull/1) with the discussed changes and let me know what you think.

left some comments on your proposal PR