Bitcoin Core Github
43 subscribers
122K links
Download Telegram
💬 instagibbs commented on pull request "rpc: Remove submitblock pre-checks":
(https://github.com/bitcoin/bitcoin/pull/31175#issuecomment-2491873388)
minimal test case for pruning, feel free to take: https://gist.github.com/instagibbs/cb1f7cfbd0a70e8b5393e457c7a959e6
💬 jonatack commented on issue "Avoid internet traffic from tests":
(https://github.com/bitcoin/bitcoin/issues/31339#issuecomment-2491945225)
> Enforce this in the CI, having it to detect non-local traffic and fail accordingly.

Concept ACK
🤔 BrandonOdiwuor reviewed a pull request: "contrib: fix BUILDDIR in gen-bitcoin-conf script"
(https://github.com/bitcoin/bitcoin/pull/31332#pullrequestreview-2452391378)
tACK b3a96b35691f3fbf958958056cd905e119fb48fe
💬 instagibbs commented on pull request "test: Deduplicate assert_mempool_contents()":
(https://github.com/bitcoin/bitcoin/pull/31338#issuecomment-2492087451)
ACK a0eafc10f94362408f54195ffd5a9237dc1ef638

thanks for opening this
🤔 jonatack reviewed a pull request: "contrib: fix BUILDDIR in gen-bitcoin-conf script"
(https://github.com/bitcoin/bitcoin/pull/31332#pullrequestreview-2452455980)
Tested ACK b3a96b35691f3fbf958958056cd905e119fb48fe

With respect to #31161, the bug can be fixed now with this change and that pull easily updated if it moves forward.
💬 hebasto commented on pull request "Add multiprocess binaries to release build":
(https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1852773687)
> TODO:
>
> * [ ] fix `unknown target CPU 'armv8-a+crc+crypto'` [failure](https://cirrus-ci.com/task/5887765378760704) and re-enable macOS cross

Similar to https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2483394112, I do think that `-DWITH_MULTIPROCESS=OFF` can be dropped now as depends are [fixed](https://github.com/bitcoin/bitcoin/pull/30975#discussion_r1848171448).
💬 mzumsande commented on issue "Avoid internet traffic from tests":
(https://github.com/bitcoin/bitcoin/issues/31339#issuecomment-2492157450)
> Honestly, I forgot which test was that.

Maybe it was https://github.com/bitcoin/bitcoin/pull/29605#discussion_r1701616675 (I independently ran into that a few weeks ago and included a fix in #31142)
📝 Parkeragan opened a pull request: "Add files via upload"
(https://github.com/bitcoin/bitcoin/pull/31342)


Bitcoin Core has a thorough review process and even the most trivial change
needs to pass a lot of eyes and requires non-zero or even substantial time
effort to review. There is a huge lack of active reviewers on the project, so
patches often sit for a long time.
-->
willcl-ark closed a pull request: "Add files via upload"
(https://github.com/bitcoin/bitcoin/pull/31342)
📝 fanquake locked a pull request: "Add files via upload"
(https://github.com/bitcoin/bitcoin/pull/31342)


Bitcoin Core has a thorough review process and even the most trivial change
needs to pass a lot of eyes and requires non-zero or even substantial time
effort to review. There is a huge lack of active reviewers on the project, so
patches often sit for a long time.
-->
💬 TheCharlatan commented on pull request "rpc: Remove submitblock pre-checks":
(https://github.com/bitcoin/bitcoin/pull/31175#issuecomment-2492366799)
Thank you for the reviews and the test case @instagibbs,

Updated 563278b6a125f1a3d2111d4ebd74b9cc16d83ec5 -> 73db95c65c1d372822166045ca8b9f173d5fd883 ([submitblock_prechecks_3](https://github.com/TheCharlatan/bitcoin/tree/submitblock_prechecks_3) -> [submitblock_prechecks_4](https://github.com/TheCharlatan/bitcoin/tree/submitblock_prechecks_4), [compare](https://github.com/TheCharlatan/bitcoin/compare/submitblock_prechecks_3..submitblock_prechecks_4))

* Addressed @Sjors' [comment](https://
...
👍 TheCharlatan approved a pull request: "Drop script_pub_key arg from createNewBlock"
(https://github.com/bitcoin/bitcoin/pull/31318#pullrequestreview-2452805746)
ACK 027ce3e9634b2f47c508899942d05690572de516
💬 andremralves commented on pull request "contrib: skip missing binaries in gen-manpages":
(https://github.com/bitcoin/bitcoin/pull/30986#discussion_r1852949174)
> This could also be because the user might not have built the binaries yet even in the case of default build path.

Yes, I haven't added it to the message because I though it would be too verbose, but I can reconsider it.
👍 hodlinator approved a pull request: "policy: ephemeral dust followups"
(https://github.com/bitcoin/bitcoin/pull/31279#pullrequestreview-2452817327)
ACK 466e4df3fb83ef82b6add22e202e3a70dbf83a12

Mostly responded to my feedback since my last review.

Compiled with BDB and passed functional tests, unittests and ran `ephemeral_package_eval` fuzz target for 1 minute.

Still have [reservations about the prioritisetransaction RPC](https://github.com/bitcoin/bitcoin/pull/31279/files#r1848453460), but not blocking.
🤔 hebasto reviewed a pull request: "guix: swap `moreutils` for just `sponge`"
(https://github.com/bitcoin/bitcoin/pull/31323#pullrequestreview-2452825209)
My Guix build:
```
riscv64
74a02707614d7fc7ed460148b37e90c126d09c2e9b2962765de14d267a3f22d3 guix-build-c4ee9b8aa078/output/aarch64-linux-gnu/SHA256SUMS.part
263f079da730485eb583995c1fc6304b7ca957a668fd5aac8f879bc59746dc67 guix-build-c4ee9b8aa078/output/aarch64-linux-gnu/bitcoin-c4ee9b8aa078-aarch64-linux-gnu-debug.tar.gz
745a9bfdd9550cf9abf80415c4efe818446ea05e9c4f659f643669c3dcb49893 guix-build-c4ee9b8aa078/output/aarch64-linux-gnu/bitcoin-c4ee9b8aa078-aarch64-linux-gnu.tar.gz
5810b1ef
...
💬 andremralves commented on pull request "contrib: skip missing binaries in gen-manpages":
(https://github.com/bitcoin/bitcoin/pull/30986#discussion_r1852957327)
I've updated it, thank for the suggestion.
💬 hodlinator commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1852962780)
`-version=true` and `-version=false` are both interpreted as `-version=0` (there is no special parsing for booleans expressed as strings).
`-version=0` is equivalent to `-noversion` or `-noversion=1`.

`IsArgSet()` returns `true` if the user has set a given arg to *any value* (including negation), through command line or settings-file.
💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2492442146)
Updated 97fe2b25af31ca612c1f8d9f3de739fa3dee3902 -> 5910f9bb18da35101144304d385618d7b82420f5 ([kernelApi_2](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_2) -> [kernelApi_3](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_3), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_2..kernelApi_3))

* As discussed with @stickies-v out of band, make callbacks only return `const` pointers, which further ensures that the user does not de-allocate or take ownership of th
...
💬 hodlinator commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1852982119)
What would you use as category at the beginning of commit messages when implementations are merged with tests? - Feels logical to have for example `net: ...` commit + `test net: ...` commit, you would just have the first message and merge them together?

If the pre-existing tests were to be broken by changing the implementations, I can understand changing them in the same commit, but AFAIK that is not the case in this PR.

My impression is that this project encourages separate commits for te
...
📝 theStack opened a pull request: "test: avoid internet traffic in rpc_net.py"
(https://github.com/bitcoin/bitcoin/pull/31343)
Rather than opening a connection to a public IP, use one in the [link-local address range](https://en.wikipedia.org/wiki/Link-local_address#IPv4) in order to avoid the problems described in #31339. This is still not ideal and could be annoying to users if there is an actual machine reachable on this address, so I'm open for better suggestions. As far as I understand, we don't want to use localhost once again here, since for testing the filtering a different IP than the one already added before s
...