Bitcoin Core Github
44 subscribers
121K links
Download Telegram
πŸ’¬ luke-jr commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2087743666)
This PR shouldn't be merged at all, but obviously to even be considered bugs like this would have to be fixed. Don't forget the scriptPubKey lengths.
πŸ€” 1440000bytes reviewed a pull request: "policy: uncap datacarrier by default"
(https://github.com/bitcoin/bitcoin/pull/32406#pullrequestreview-2838300949)
ACK https://github.com/bitcoin/bitcoin/pull/32406/commits/35bcd8eed38130445aef5ebe217ab42248fa6f18

I have tested this branch with transactions having multiple OP_RETURN. Steps are shared in this comment marked as off-topic: https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2875133307

I am interested to see the usage of multiple OP_RETURN after v30 if this pull request gets merged. Written a [post](https://uncensoredtech.substack.com/p/op_return-and-coinjoin) about their usage in c
...
πŸ“ achow101 opened a pull request: "gui: Menu action to export a watchonly wallet"
(https://github.com/bitcoin-core/gui/pull/872)
Allows a user to export a watchonly version of their wallet to be used in an airgapped setup.

Built on https://github.com/bitcoin/bitcoin/pull/32489
πŸ’¬ stickies-v commented on pull request "kernel: Separate UTXO set access from validation functions":
(https://github.com/bitcoin/bitcoin/pull/32317#issuecomment-2878217156)
PSA: Bitcoin Core PR Review Club will cover this PR in its next meeting at 2025-05-14 at 17:00 UTC. See https://bitcoincore.reviews/32317 for notes, questions, and instructions on [how to join](https://bitcoincore.reviews/).
πŸ’¬ jmatcho commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2878273731)
Concept NACK for changing an existing default and deprecating an existing feature that take control away from the people's nodes.
πŸ’¬ luke-jr commented on pull request "docs: clarify RPC credentials security boundary":
(https://github.com/bitcoin/bitcoin/pull/32424#issuecomment-2878375720)
Seems to contradict the existence of `-rpcwhitelist`
πŸ’¬ bigshiny90 commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2878447923)
Concept NACK

there is no clear need for this change. Use cases expressed are minor, with only hope of future usage. Expressed use cases can be accommodated in a simpler way with a minor size change. If Core would like this op return change for the future, do the work and convince node runners to change their defaults. Arguments for block propagation speed and fee estimates, though technically sound, are potentially minor compared to forcing the default size to max. Slow this down and move to
...
πŸ’¬ BitcoinMechanic commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2878452352)
Did the NACK/ACK bot break? Doesn't seem to be updating? @DrahtBot
πŸ‘‹ ajtowns's pull request is ready for review: "ArgsManager: support subcommand-specific options"
(https://github.com/bitcoin/bitcoin/pull/28802)
πŸ’¬ ajtowns commented on pull request "ArgsManager: support subcommand-specific options":
(https://github.com/bitcoin/bitcoin/pull/28802#issuecomment-2878651168)
Rebased and undrafted
πŸ’¬ maflcko commented on pull request "fuzz: Properly setup wallet in wallet_fees target":
(https://github.com/bitcoin/bitcoin/pull/32488#issuecomment-2878682301)
(side note: I would have expected that the fuzz stability was less, but https://oss-fuzz.com/fuzzer-stats?group_by=by-fuzzer&date_start=2025-04-14&date_end=2025-05-13&fuzzer=afl_bitcoin-core_wallet_fees&job=afl_asan_bitcoin-core&project=bitcoin-core claims it is 100%)
πŸ’¬ maflcko commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2878722476)
> Did the NACK/ACK bot break? Doesn't seem to be updating? @DrahtBot

This is a known issue. See https://github.com/maflcko/DrahtBot/issues/51. However, I can't spot the error, as explained in that thread.
πŸ’¬ maflcko commented on pull request "Fix ASM ambiguity":
(https://github.com/bitcoin/bitcoin/pull/28824#issuecomment-2878740702)
ping @willcl-ark. There hasn't been any activity here since the revived discussion more than a year ago.
πŸ’¬ chrisguida commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2878752046)
Concept NACK

I definitely already did this, but I don't see my comment anymore and it was never registered by drahtbot, so I'm redoing it.

Same rationale as https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2833932824
πŸ’¬ Eunovo commented on pull request "Fix listdescriptors true fails with 'Can't get descriptor string' in non-watch-only descriptor wallet":
(https://github.com/bitcoin/bitcoin/pull/32471#issuecomment-2878821724)
> Concept NACK
>
> The purpose of the `private` argument is to show the private keys. If there are no private keys to show, I think it is a reasonable error to say that, rather than just showing the public descriptors. I think changing the behavior could result in a footgun where people use it thinking that they are being shown private keys when they actually are not.

I think there has been a misunderstanding. **There are private keys to show.** This PR doesn't change the behaviour of `li
...
πŸ’¬ Eunovo commented on pull request "Fix listdescriptors true fails with 'Can't get descriptor string' in non-watch-only descriptor wallet":
(https://github.com/bitcoin/bitcoin/pull/32471#issuecomment-2878828270)
> I think that's fine to show a result, but I don't think we should show any result if there are no private keys at all.

No result is shown if there are no private keys
πŸ’¬ Eunovo commented on pull request "Fix listdescriptors true fails with 'Can't get descriptor string' in non-watch-only descriptor wallet":
(https://github.com/bitcoin/bitcoin/pull/32471#issuecomment-2878844076)
> It's unclear why someone should receive public keys with `private=true`. Why not just use the default command, `bitcoin-cli listdescriptors`, which already shows public keys? I assume the user is certain about the nature of the descriptors they have in their wallet. Maybe if I do not fully understand this PR.

It is possible to have descriptors with missing private keys in a non-watchonly wallet. `importdescriptors` only rejects descriptors without any private keys. Trying `listdescriptors p
...
πŸ’¬ Sjors commented on pull request "Remove Taproot activation height":
(https://github.com/bitcoin/bitcoin/pull/26201#issuecomment-2878943691)
Trivial rebase after #32386.
πŸ’¬ Sjors commented on pull request "doc: warn that CheckBlock() underestimates sigops":
(https://github.com/bitcoin/bitcoin/pull/31624#discussion_r2088206037)
> a clear explanation of whether the check is redundant or necessary

The problem is that I don't know for sure. I _think_ it's redundant, but I don't want to mislead some future dev. They should do their own research.

> Even without this PR ... someone could dig into the code and eventually realize that it’s an incomplete check

The comment is there to prevent someone who _doesn't_ check from accidentally introducing a consensus bug.