Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 l0rinc commented on pull request "coins: fix `cachedCoinsUsage` accounting in `CCoinsViewCache`":
(https://github.com/bitcoin/bitcoin/pull/32313#discussion_r2083296383)
> You can recreate the failures

Thanks a lot @fanquake, I managed to reproduce it this way (seeing that https://github.com/l0rinc/bitcoin/pull/10 was failing, I was trying to run them manually instead of relying on the ci scripts).

> why it isn't needed (or rather since when)

Thanks @maflcko for insisting, I have changed the order of commits to place the suppression removal at the very end since we still had a few failures for the first few commits - it's easier to remove them all at th
...
💬 supertestnet commented on pull request "rpc:generatetomany":
(https://github.com/bitcoin/bitcoin/pull/32468#issuecomment-2869219420)
Can a PSBT be constructed with only outputs, no inputs? If so, then maybe `generateblock` could accept a psbt or a string as an argument. If it is a psbt, it must contain outputs but no inputs.

But also, due to fees being variable, it may be useful to have some kind of "remainder" option so that you can say, for example, "give 1 btc apiece to the first 3 addresses, and give the remainder to the last address"
💬 pokrovskyy commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2869297376)
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.:
- future meta-protocols may choose OP_RETURN as the storage option for their data (instead of witness)
- this will decrease the data inflow into witness
- consequently, this will decrease the growth rate
...
💬 va7wv commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2869377798)
> @1ma As was discussed in the mailing list discussion, entities are using unspendable outputs in liu of OP_Return outputs. Precisely because of the size limit. This increases the UTXO set size unnecessarily, a harmful effect of having the arbitrary OP_Return output limitations. Your analysis does not take that kind of issue into account.
>
> Rather than do a stream of incremental increases, wasting dev bandwidth, this pull-req simply removes those arbitrary limits.
>
> Anyway, this kind o
...
🤔 shahsb reviewed a pull request: "threading: drop CSemaphore in favor of c++20 std::counting_semaphore"
(https://github.com/bitcoin/bitcoin/pull/32466#pullrequestreview-2831290143)
LGTM.. Thanks for making the changes!
💬 polespinasa commented on pull request "rpc:generatetomany":
(https://github.com/bitcoin/bitcoin/pull/32468#issuecomment-2869613736)
> Can a PSBT be constructed with only outputs, no inputs?

I'm not sure using a PSBT is the best approach. If we check the [normal workflow](https://github.com/bitcoin/bitcoin/blob/master/doc/psbt.md#overall-workflow) for a PSBT we see it is intended to:
1. Create the PSBT with inputs and outputs but no metadata
2. Updated with the data needed to spend the UTXOs in the inputs
3. Signed
4. Finalized

IMO using PSBTs here does not make sense, we don't need any of that, we don't need to sha
...
💬 supertestnet commented on pull request "rpc:generatetomany":
(https://github.com/bitcoin/bitcoin/pull/32468#issuecomment-2869615508)
> Do you have any idea on how that could look like?

Here is how the syntax looks for the `sendmany` command which I am using as inspiration:

`bitcoin-cli sendmany "" "{\"bc1q09vm5lfy0j5reeulh4x5752q25uqqvz34hufdl\":0.01,\"bc1q02ad21edsxd23d32dfgqqsz4vv4nmtfzuklhy3\":0.02}"`

So maybe we could do something similar:

`bitcoin-cli generatetomany "{\"bc1q09vm5lfy0j5reeulh4x5752q25uqqvz34hufdl\":0.01,\"bc1q02ad21edsxd23d32dfgqqsz4vv4nmtfzuklhy3\":0.01,\"bc1qmaf2fjnklfaffz385329cxcr267renn09
...
💬 polespinasa commented on pull request "rpc:generatetomany":
(https://github.com/bitcoin/bitcoin/pull/32468#issuecomment-2869620363)
> note the remainder in the last btc address

what if you want to split the reminder between multiple addresses?
💬 pithosian commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2083464617)
For additional context, in `mempool_args.cpp::apply_args_man_options`, we have this check:
```cpp
if (argsman.GetBoolArg("-datacarrier", DEFAULT_ACCEPT_DATACARRIER)) {
mempool_opts.max_datacarrier_bytes = argsman.GetIntArg("-datacarriersize", MAX_OP_RETURN_RELAY);
} else {
mempool_opts.max_datacarrier_bytes = std::nullopt;
}
```

`GetIntArg` can't take `nullopt` as a default (it wants an integer), so the true path has to set to `MAX_OP_RETURN_RELAY` as the default, rather than s
...
💬 pithosian commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2869652723)
Code ACK, conceptual NACK purely on option deprecation (comment [here](https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2868741871) and in the mailing list; the justification for deprecating/removing these options appears faulty to me).
💬 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)
💬 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.