💬 Sjors commented on pull request "rpc: fix getblock(header) returns target for tip":
(https://github.com/bitcoin/bitcoin/pull/33446#discussion_r2375719841)
I could rename it if I have to retouch, but at the time didn't feel like adding another constant :-)
(https://github.com/bitcoin/bitcoin/pull/33446#discussion_r2375719841)
I could rename it if I have to retouch, but at the time didn't feel like adding another constant :-)
💬 Sjors commented on pull request "rpc: fix getblock(header) returns target for tip":
(https://github.com/bitcoin/bitcoin/pull/33446#discussion_r2375723320)
It just describes how to generate the data for this test, I'm not proposing actually removing these checks in the repository.
(https://github.com/bitcoin/bitcoin/pull/33446#discussion_r2375723320)
It just describes how to generate the data for this test, I'm not proposing actually removing these checks in the repository.
💬 Sjors commented on pull request "rpc: fix getblock(header) returns target for tip":
(https://github.com/bitcoin/bitcoin/pull/33446#discussion_r2375726568)
Will change if I need to retouch.
(https://github.com/bitcoin/bitcoin/pull/33446#discussion_r2375726568)
Will change if I need to retouch.
💬 willcl-ark commented on pull request "macdeploy: avoid use of `Bitcoin Core` in Linux cross build":
(https://github.com/bitcoin/bitcoin/pull/33158#issuecomment-3328361231)
Guix hashes:
```
e522b04c6af1759ea97e71c01b55713685ecb0b25557a864a9fa36af26042a8d guix-build-8e434a84999c/output/arm64-apple-darwin/SHA256SUMS.part
740e33ee5d6d9a9cbc25dc0b4775e108612650b8c644f75adbce00537316abd9 guix-build-8e434a84999c/output/arm64-apple-darwin/bitcoin-8e434a84999c-arm64-apple-darwin-codesigning.tar.gz
bca939a67f3ac36c4345d61ed498d815608b4889a0e2367a2f71273e8f03b605 guix-build-8e434a84999c/output/arm64-apple-darwin/bitcoin-8e434a84999c-arm64-apple-darwin-unsigned.tar.g
...
(https://github.com/bitcoin/bitcoin/pull/33158#issuecomment-3328361231)
Guix hashes:
```
e522b04c6af1759ea97e71c01b55713685ecb0b25557a864a9fa36af26042a8d guix-build-8e434a84999c/output/arm64-apple-darwin/SHA256SUMS.part
740e33ee5d6d9a9cbc25dc0b4775e108612650b8c644f75adbce00537316abd9 guix-build-8e434a84999c/output/arm64-apple-darwin/bitcoin-8e434a84999c-arm64-apple-darwin-codesigning.tar.gz
bca939a67f3ac36c4345d61ed498d815608b4889a0e2367a2f71273e8f03b605 guix-build-8e434a84999c/output/arm64-apple-darwin/bitcoin-8e434a84999c-arm64-apple-darwin-unsigned.tar.g
...
👍 willcl-ark approved a pull request: "macdeploy: avoid use of `Bitcoin Core` in Linux cross build"
(https://github.com/bitcoin/bitcoin/pull/33158#pullrequestreview-3262868167)
ACK 8e434a84999c473a7295772a346cbce27888d28e
Changes here seem good, I also prefer the new naming.
I guess no longer deriving from CLIENT_NAME _might_ affect forks (only if they use this same build/release process?), but the rename in the guix script is currently hardcoded (to `Bitcoin-Core.zip`), so I doubt this worked for them as-is anyway.
(https://github.com/bitcoin/bitcoin/pull/33158#pullrequestreview-3262868167)
ACK 8e434a84999c473a7295772a346cbce27888d28e
Changes here seem good, I also prefer the new naming.
I guess no longer deriving from CLIENT_NAME _might_ affect forks (only if they use this same build/release process?), but the rename in the guix script is currently hardcoded (to `Bitcoin-Core.zip`), so I doubt this worked for them as-is anyway.
💬 waketraindev commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3328498567)
This PR is pretty much a waste of time especially when considering all the other hundreds of open PRs
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3328498567)
This PR is pretty much a waste of time especially when considering all the other hundreds of open PRs
💬 waketraindev commented on pull request "Add reset button and console commands for clearing output/history":
(https://github.com/bitcoin-core/gui/pull/882#issuecomment-3328509707)
Obviously this project isn't interested in this PR I'll keep it for my own use and close this PR. Thanks everyone who participated in reviews.
(https://github.com/bitcoin-core/gui/pull/882#issuecomment-3328509707)
Obviously this project isn't interested in this PR I'll keep it for my own use and close this PR. Thanks everyone who participated in reviews.
🤔 theStack reviewed a pull request: "rpc: combinerawtransaction now rejects unmergeable transactions"
(https://github.com/bitcoin/bitcoin/pull/31298#pullrequestreview-3262922701)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/31298#pullrequestreview-3262922701)
Concept ACK
💬 theStack commented on pull request "rpc: combinerawtransaction now rejects unmergeable transactions":
(https://github.com/bitcoin/bitcoin/pull/31298#discussion_r2375806658)
nit: as per our coding guidelines, [variable names should be in `camel_case`](https://github.com/bitcoin/bitcoin/blob/ad4a49090da81a3683fc50694ed7b42a80fdf90b/doc/developer-notes.md?plain=1#L38-L39), and new style should be favored over mimicking the surrounding style
(https://github.com/bitcoin/bitcoin/pull/31298#discussion_r2375806658)
nit: as per our coding guidelines, [variable names should be in `camel_case`](https://github.com/bitcoin/bitcoin/blob/ad4a49090da81a3683fc50694ed7b42a80fdf90b/doc/developer-notes.md?plain=1#L38-L39), and new style should be favored over mimicking the surrounding style
💬 theStack commented on pull request "rpc: combinerawtransaction now rejects unmergeable transactions":
(https://github.com/bitcoin/bitcoin/pull/31298#discussion_r2375825517)
could test more thoroughly by repeatedly modifying other fields that commit to the txid (w/o scriptSig), but this can be done in a follow-up
(https://github.com/bitcoin/bitcoin/pull/31298#discussion_r2375825517)
could test more thoroughly by repeatedly modifying other fields that commit to the txid (w/o scriptSig), but this can be done in a follow-up
✅ waketraindev closed a pull request: "Add reset button and console commands for clearing output/history"
(https://github.com/bitcoin-core/gui/pull/882)
(https://github.com/bitcoin-core/gui/pull/882)
👍 rkrux approved a pull request: "cli: Handle arguments that can be either JSON or string"
(https://github.com/bitcoin/bitcoin/pull/33230#pullrequestreview-3263007651)
lgtm ACK df67bb6fd84c393eaf00f19074085ee080546bd3
Adds conveniences in CLI, cleans up tests too slightly.
(https://github.com/bitcoin/bitcoin/pull/33230#pullrequestreview-3263007651)
lgtm ACK df67bb6fd84c393eaf00f19074085ee080546bd3
Adds conveniences in CLI, cleans up tests too slightly.
✅ waketraindev closed a pull request: "rpc: add optional peer_ids param to filter getpeerinfo"
(https://github.com/bitcoin/bitcoin/pull/32741)
(https://github.com/bitcoin/bitcoin/pull/32741)
💬 zaidmstrr commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2375876150)
Thanks, I implemented the suggested changes.
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2375876150)
Thanks, I implemented the suggested changes.
💬 zaidmstrr commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2375886327)
Seems reasonable.
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2375886327)
Seems reasonable.
💬 theStack commented on pull request "Fix #25980: Validate transactions in combinerawtransaction":
(https://github.com/bitcoin/bitcoin/pull/33361#issuecomment-3328612816)
I'm currently favoring the fix in #31298, as it avoids comparing all the involved transaction fields (and input/output counts) by using the txid instead and is thus much simpler. One potential advantage of the approach of this PR though is that the error messages are more detailed, pointing out where exactly the txs differ -- personally I think just pointing out that the txs are not compatible is sufficient.
(https://github.com/bitcoin/bitcoin/pull/33361#issuecomment-3328612816)
I'm currently favoring the fix in #31298, as it avoids comparing all the involved transaction fields (and input/output counts) by using the txid instead and is thus much simpler. One potential advantage of the approach of this PR though is that the error messages are more detailed, pointing out where exactly the txs differ -- personally I think just pointing out that the txs are not compatible is sufficient.
💬 zaidmstrr commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2375898072)
I think using namespace is better here, as it provides clear intent of both the functions and how they are related. Also I find more readability using this approach.
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2375898072)
I think using namespace is better here, as it provides clear intent of both the functions and how they are related. Also I find more readability using this approach.
🚀 fanquake merged a pull request: "macdeploy: avoid use of `Bitcoin Core` in Linux cross build"
(https://github.com/bitcoin/bitcoin/pull/33158)
(https://github.com/bitcoin/bitcoin/pull/33158)
💬 ryanofsky commented on pull request "multiprocess: Add capnp wrapper for Chain interface":
(https://github.com/bitcoin/bitcoin/pull/29409#issuecomment-3328660664)
<!-- begin push-17 -->
Rebased b372063241fbd34d27d062f3e438a3e425a9dfc5 -> 7ba7ec9d08927f244da8d5c1a7a1d7ced3239465 ([`pr/ipc-chain.16`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-chain.16) -> [`pr/ipc-chain.17`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-chain.17), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ipc-chain.16-rebase..pr/ipc-chain.17))<!-- end --> due to conflicts with #33169 and #32345
(https://github.com/bitcoin/bitcoin/pull/29409#issuecomment-3328660664)
<!-- begin push-17 -->
Rebased b372063241fbd34d27d062f3e438a3e425a9dfc5 -> 7ba7ec9d08927f244da8d5c1a7a1d7ced3239465 ([`pr/ipc-chain.16`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-chain.16) -> [`pr/ipc-chain.17`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-chain.17), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ipc-chain.16-rebase..pr/ipc-chain.17))<!-- end --> due to conflicts with #33169 and #32345
✅ fanquake closed an issue: "RPC: getblock(header) returns the same target for every block"
(https://github.com/bitcoin/bitcoin/issues/33440)
(https://github.com/bitcoin/bitcoin/issues/33440)