💬 fanquake commented on pull request "depends: remove xinerama extension from libxcb":
(https://github.com/bitcoin/bitcoin/pull/33217#issuecomment-3214089111)
Backported to 29.x in #33238.
(https://github.com/bitcoin/bitcoin/pull/33217#issuecomment-3214089111)
Backported to 29.x in #33238.
💬 luke-jr commented on pull request "cli: Handle arguments that can be either JSON or string":
(https://github.com/bitcoin/bitcoin/pull/33230#discussion_r2293514007)
Might be a good idea to actually test this is correct?
(https://github.com/bitcoin/bitcoin/pull/33230#discussion_r2293514007)
Might be a good idea to actually test this is correct?
🤔 luke-jr reviewed a pull request: "cli: Handle arguments that can be either JSON or string"
(https://github.com/bitcoin/bitcoin/pull/33230#pullrequestreview-3144205566)
I guess this basically works for the existing params, since a block hash is never valid JSON (which forbids leading zeros), but could be dangerous to use for new parameters that are ambiguous.
With that in mind, it might be worth instead having a new `RPCResult::Type::BLOCK_HEIGHT_OR_HASH` type to sanity check this. Or at least a warning comment (but I don't know where you'd put it to ensure it's seen).
(https://github.com/bitcoin/bitcoin/pull/33230#pullrequestreview-3144205566)
I guess this basically works for the existing params, since a block hash is never valid JSON (which forbids leading zeros), but could be dangerous to use for new parameters that are ambiguous.
With that in mind, it might be worth instead having a new `RPCResult::Type::BLOCK_HEIGHT_OR_HASH` type to sanity check this. Or at least a warning comment (but I don't know where you'd put it to ensure it's seen).
💬 luke-jr commented on pull request "rpc: add optional peer_ids param to filter getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/32741#discussion_r2293533369)
```suggestion
{"peer_id|peer_ids", RPCArg::Type::ARR, RPCArg::Optional::OMITTED,
```
(https://github.com/bitcoin/bitcoin/pull/32741#discussion_r2293533369)
```suggestion
{"peer_id|peer_ids", RPCArg::Type::ARR, RPCArg::Optional::OMITTED,
```
💬 luke-jr commented on pull request "rpc: add optional peer_ids param to filter getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/32741#discussion_r2293532733)
```suggestion
{RPCArgOptions{.skip_type_check = true, .type_str = {"", "numeric or array"}}}
```
(https://github.com/bitcoin/bitcoin/pull/32741#discussion_r2293532733)
```suggestion
{RPCArgOptions{.skip_type_check = true, .type_str = {"", "numeric or array"}}}
```
💬 fanquake commented on pull request "[29.x] depends: remove xinerama extension from libxcb":
(https://github.com/bitcoin/bitcoin/pull/33238#issuecomment-3214250090)
Guess we can't backport this unless we also backport the removal in Qt. Will take a look.
(https://github.com/bitcoin/bitcoin/pull/33238#issuecomment-3214250090)
Guess we can't backport this unless we also backport the removal in Qt. Will take a look.
🚀 fanquake merged a pull request: "doc: use new block_to_connect parameter name"
(https://github.com/bitcoin/bitcoin/pull/33237)
(https://github.com/bitcoin/bitcoin/pull/33237)
💬 maflcko commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2293722588)
nit: any reason to exclude negative values? They are documented to behave as-if 0 was passed, so it seems fine to keep the fuzz coverage for it? (https://maflcko.github.io/b-c-cov/fuzz.coverage/src/policy/feerate.cpp.gcov.html)
(There is a unit test covering it, so just a nit)
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2293722588)
nit: any reason to exclude negative values? They are documented to behave as-if 0 was passed, so it seems fine to keep the fuzz coverage for it? (https://maflcko.github.io/b-c-cov/fuzz.coverage/src/policy/feerate.cpp.gcov.html)
(There is a unit test covering it, so just a nit)
💬 maflcko commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2293731923)
nano nit: The c-style cast isn't needed and the narrowing check can be enabled:
```cpp
CAmount nFee{m_feerate.EvaluateFeeUp(virtual_bytes)};
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2293731923)
nano nit: The c-style cast isn't needed and the narrowing check can be enabled:
```cpp
CAmount nFee{m_feerate.EvaluateFeeUp(virtual_bytes)};
📝 gianlucamazza opened a pull request: "test: Add comprehensive tests for 999-of-999 Taproot multisig"
(https://github.com/bitcoin/bitcoin/pull/33239)
## Summary
This PR implements comprehensive test coverage for Bitcoin Core issue #28179, proving that 999-of-999 Taproot multisig descriptors work correctly while maintaining performance requirements.
## Background
Issue #28179 requested tests to verify that 999-of-999 Taproot multisig works, addressing the theoretical maximum for `multi_a` descriptors. Previous attempts in PR #28212 and #31719 failed due to performance issues when trying to generate actual spending transactions with 999 sign
...
(https://github.com/bitcoin/bitcoin/pull/33239)
## Summary
This PR implements comprehensive test coverage for Bitcoin Core issue #28179, proving that 999-of-999 Taproot multisig descriptors work correctly while maintaining performance requirements.
## Background
Issue #28179 requested tests to verify that 999-of-999 Taproot multisig works, addressing the theoretical maximum for `multi_a` descriptors. Previous attempts in PR #28212 and #31719 failed due to performance issues when trying to generate actual spending transactions with 999 sign
...
🤔 naiyoma reviewed a pull request: "test: rpc: add last block announcement time to getpeerinfo result"
(https://github.com/bitcoin/bitcoin/pull/27052#pullrequestreview-3144554944)
Tested ACK cbe4603a90220f0ef0b21c4da68ee16791ad9034
Changes since last review: rebased and addressed nit fixes.
Also tested with `msg_cmpctblock` - test passed as expected.
(https://github.com/bitcoin/bitcoin/pull/27052#pullrequestreview-3144554944)
Tested ACK cbe4603a90220f0ef0b21c4da68ee16791ad9034
Changes since last review: rebased and addressed nit fixes.
Also tested with `msg_cmpctblock` - test passed as expected.
📝 gianlucamazza converted_to_draft a pull request: "test: Add comprehensive tests for 999-of-999 Taproot multisig"
(https://github.com/bitcoin/bitcoin/pull/33239)
## Summary
This PR implements comprehensive test coverage for Bitcoin Core issue #28179, proving that 999-of-999 Taproot multisig descriptors work correctly while maintaining performance requirements.
## Background
Issue #28179 requested tests to verify that 999-of-999 Taproot multisig works, addressing the theoretical maximum for `multi_a` descriptors. Previous attempts in PR #28212 and #31719 failed due to performance issues when trying to generate actual spending transactions with 999 sign
...
(https://github.com/bitcoin/bitcoin/pull/33239)
## Summary
This PR implements comprehensive test coverage for Bitcoin Core issue #28179, proving that 999-of-999 Taproot multisig descriptors work correctly while maintaining performance requirements.
## Background
Issue #28179 requested tests to verify that 999-of-999 Taproot multisig works, addressing the theoretical maximum for `multi_a` descriptors. Previous attempts in PR #28212 and #31719 failed due to performance issues when trying to generate actual spending transactions with 999 sign
...
💬 fanquake commented on pull request "test: Add comprehensive tests for 999-of-999 Taproot multisig":
(https://github.com/bitcoin/bitcoin/pull/33239#issuecomment-3214498369)
> Previous attempts in PR ... https://github.com/bitcoin/bitcoin/pull/31719
This PR is still open?
(https://github.com/bitcoin/bitcoin/pull/33239#issuecomment-3214498369)
> Previous attempts in PR ... https://github.com/bitcoin/bitcoin/pull/31719
This PR is still open?
✅ maflcko closed a pull request: "test: Add comprehensive tests for 999-of-999 Taproot multisig"
(https://github.com/bitcoin/bitcoin/pull/33239)
(https://github.com/bitcoin/bitcoin/pull/33239)
💬 maflcko commented on pull request "test: Add comprehensive tests for 999-of-999 Taproot multisig":
(https://github.com/bitcoin/bitcoin/pull/33239#issuecomment-3214539631)
Thanks, but the value of purely LLM generated "vibe" pull requests is limited, because:
* The "author" does not understand the changes and can not reply to code review feedback
* There are more than 300 pull requests (most written by real humans) waiting for review
Also, the tests don't even pass, so I'll close this for now.
Generally, I think it is fine to use LLMs, if the author would have written the same code almost exactly the same way without the LLM, and also fully understands i
...
(https://github.com/bitcoin/bitcoin/pull/33239#issuecomment-3214539631)
Thanks, but the value of purely LLM generated "vibe" pull requests is limited, because:
* The "author" does not understand the changes and can not reply to code review feedback
* There are more than 300 pull requests (most written by real humans) waiting for review
Also, the tests don't even pass, so I'll close this for now.
Generally, I think it is fine to use LLMs, if the author would have written the same code almost exactly the same way without the LLM, and also fully understands i
...
⚠️ JustaNewer opened an issue: "Please Cancel the OP RETURN function"
(https://github.com/bitcoin/bitcoin/issues/33240)
### Please describe the feature you'd like to see added.
From a protocol design perspective, OP_RETURN is unnecessary within the Bitcoin network. Bitcoin’s fundamental and sole purpose should remain peer-to-peer value transfer. Introducing OP_RETURN increases the computational and storage burden on full nodes, resulting in additional costs without providing benefits aligned with Bitcoin’s core mission. In essence, embedding arbitrary data through OP_RETURN is redundant—analogous to attempting t
...
(https://github.com/bitcoin/bitcoin/issues/33240)
### Please describe the feature you'd like to see added.
From a protocol design perspective, OP_RETURN is unnecessary within the Bitcoin network. Bitcoin’s fundamental and sole purpose should remain peer-to-peer value transfer. Introducing OP_RETURN increases the computational and storage burden on full nodes, resulting in additional costs without providing benefits aligned with Bitcoin’s core mission. In essence, embedding arbitrary data through OP_RETURN is redundant—analogous to attempting t
...
✅ maflcko closed an issue: "Please Cancel the OP RETURN function"
(https://github.com/bitcoin/bitcoin/issues/33240)
(https://github.com/bitcoin/bitcoin/issues/33240)
💬 maflcko commented on issue "Please Cancel the OP RETURN function":
(https://github.com/bitcoin/bitcoin/issues/33240#issuecomment-3214559159)
Usually the issue tracker is used to track technical issues related to the Bitcoin Core code base.
General bitcoin questions and/or support requests are best directed to the [Bitcoin StackExchange](https://bitcoin.stackexchange.com) or the `#bitcoin` IRC channel on Libera Chat, or one of the Bitcoin subreddits, or any other place that you feel is well suited.
Network-wide consensus and/or P2P changes first need to be discussed with the greater ecosystem, for example https://groups.google.com/g
...
(https://github.com/bitcoin/bitcoin/issues/33240#issuecomment-3214559159)
Usually the issue tracker is used to track technical issues related to the Bitcoin Core code base.
General bitcoin questions and/or support requests are best directed to the [Bitcoin StackExchange](https://bitcoin.stackexchange.com) or the `#bitcoin` IRC channel on Libera Chat, or one of the Bitcoin subreddits, or any other place that you feel is well suited.
Network-wide consensus and/or P2P changes first need to be discussed with the greater ecosystem, for example https://groups.google.com/g
...
✅ maflcko closed an issue: "oss-fuzz: build is broken"
(https://github.com/bitcoin/bitcoin/issues/33232)
(https://github.com/bitcoin/bitcoin/issues/33232)
💬 maflcko commented on issue "oss-fuzz: build is broken":
(https://github.com/bitcoin/bitcoin/issues/33232#issuecomment-3214581022)
should be fixed?
(https://github.com/bitcoin/bitcoin/issues/33232#issuecomment-3214581022)
should be fixed?