Bitcoin Core Github
43 subscribers
122K links
Download Telegram
💬 TheCharlatan commented on pull request "rpc: Remove submitblock pre-checks":
(https://github.com/bitcoin/bitcoin/pull/31175#issuecomment-2491006682)
309bd56d97f87f973f45897fc00b1bd2fc5cff1a -> 563278b6a125f1a3d2111d4ebd74b9cc16d83ec5 ([submitblock_prechecks_2](https://github.com/TheCharlatan/bitcoin/tree/submitblock_prechecks_2) -> [submitblock_prechecks_3](https://github.com/TheCharlatan/bitcoin/tree/submitblock_prechecks_3), [compare](https://github.com/TheCharlatan/bitcoin/compare/submitblock_prechecks_2..submitblock_prechecks_3))


* Added release notes documenting the submitblock change w.rt. duplicate block handling and pruning.
💬 vasild commented on issue "Avoid internet traffic from tests":
(https://github.com/bitcoin/bitcoin/issues/31339#issuecomment-2491017207)
@l0rinc the current end to end tests do communicate locally via the `lo` interface.

@fanquake Honestly, I forgot which test was that. During latest coredev I investigated that briefly with @maflcko or @0xB10C (?) and we observed the non local traffic with `tcpdump(1)`. Now, my point is to enforce this in the CI, even if such test does not exist currently - has been fixed since coredev or my investigation back then was wrong.
💬 TheCharlatan commented on pull request "rpc: Remove submitblock pre-checks":
(https://github.com/bitcoin/bitcoin/pull/31175#discussion_r1851973626)
I decided to add a release note for now. I think the changes in this PR are small enough that we can hash the changes out wholesale and it is also not a particularly pressing change. Still not wholly convinced, but thinking about it some more I don't think somebody was relying on this behaviour beforehand either.
💬 maflcko commented on pull request "refactor: Prepare compile-time check of bilingual format strings":
(https://github.com/bitcoin/bitcoin/pull/31295#discussion_r1851976250)
It shouldn't reproduce. This commit is just taken from the other pulls, so that clang-tidy is happy with them.
💬 Sjors commented on pull request "rpc: add optional blockhash to waitfornewblock":
(https://github.com/bitcoin/bitcoin/pull/30635#discussion_r1851979781)
Added
💬 Sjors commented on pull request "rpc: add optional blockhash to waitfornewblock":
(https://github.com/bitcoin/bitcoin/pull/30635#issuecomment-2491024996)
Rebased, added comment and expanded commit message to note the GUI improvement.
💬 maflcko commented on issue "Avoid internet traffic from tests":
(https://github.com/bitcoin/bitcoin/issues/31339#issuecomment-2491034618)
I think it makes sense to enforce this in CI (and have a tracking issue or pull for that). Otherwise, it is easy to regress, like in https://github.com/bitcoin/bitcoin/pull/30529#issuecomment-2252854223

However, if there is a violation of this, or a bug, it would be good to add exact steps to reproduce, or an explanation. Claiming that this is broken on "Ubuntu 28.04 LTS" doesn't seem helpful to uncover the bug, if there is one.

Related: https://github.com/bitcoin/bitcoin/issues/30030
💬 maflcko commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1851990186)
Actually, fixed. You can drop this now.
👍 theStack approved a pull request: "test: Deduplicate assert_mempool_contents()"
(https://github.com/bitcoin/bitcoin/pull/31338#pullrequestreview-2451281918)
ACK a0eafc10f94362408f54195ffd5a9237dc1ef638

Thanks for following up!
💬 Sjors commented on pull request "rpc: add optional blockhash to waitfornewblock":
(https://github.com/bitcoin/bitcoin/pull/30635#issuecomment-2491063362)
I added a commit to make `waitfornewblock` visible in `help`. It was introduced in 2016 by #8680 as a quick fix to deal with CI failures.

At this point is seem mature enough, no longer has weird quirks like requiring GUI users to enable the RPC server, it's probably actually useful and at least [one library uses it anyway](https://github.com/rust-bitcoin/rust-bitcoincore-rpc/blob/06a6c156b4d469c60c1abe8afdc002e7e11e5d3b/client/src/client.rs#L1100). So it seems fine to list it.
💬 m3dwards commented on pull request "test: Shut down framework cleanly on RPC connection failure":
(https://github.com/bitcoin/bitcoin/pull/30660#issuecomment-2491116799)
> Made some changes to this part of `TestNode.stop_node()` to handle an already dead process

Looks cleaner.

ACK ba28593147ccc880c3a4d40db0d4ef57f4766254
💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2491178957)
Rebased after #31288 and #31122.

This PR is now based on #31325 since that has enough support. Keeping this draft until that's merged (or dropped).

The test changes will probably cause a merge conflict with #31318, but it should be trivial.

I dropped the early return in `waitNext()` because `wait_until` should already take care of that. I also made the while loop use `<=` instead of `<` so this actually works: https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1843589729 Added a
...
fanquake closed an issue: "ci: brew link workaround step no-longer working"
(https://github.com/bitcoin/bitcoin/issues/31334)
🚀 fanquake merged a pull request: "macOS: swap docs & CI from pkg-config to pkgconf"
(https://github.com/bitcoin/bitcoin/pull/31335)
👍 brunoerg approved a pull request: "fuzz: Implement G_TEST_GET_FULL_NAME"
(https://github.com/bitcoin/bitcoin/pull/31333#pullrequestreview-2451460077)
code review ACK 92d3d691f097ead8e5ae571eb9bf691133a6aa49
🚀 fanquake merged a pull request: "fuzz: Implement G_TEST_GET_FULL_NAME"
(https://github.com/bitcoin/bitcoin/pull/31333)
💬 hebasto commented on issue "CMake `CheckPIESupported` doesn't always work":
(https://github.com/bitcoin/bitcoin/issues/30771#issuecomment-2491293124)
An upstream issue: https://gitlab.kitware.com/cmake/cmake/-/issues/26463.
💬 TheCharlatan commented on pull request "Drop script_pub_key arg from createNewBlock":
(https://github.com/bitcoin/bitcoin/pull/31318#discussion_r1852145969)
I would squash the last commit, then you can also drop the `using node::BlockAssembler` declaration entirely by just calling `MineBlock(g_setup->m_node, {})`.
💬 Sjors commented on pull request "Drop script_pub_key arg from createNewBlock":
(https://github.com/bitcoin/bitcoin/pull/31318#issuecomment-2491372423)
I dropped a few spurious `(...)coinbase(_output)_script_pub_key` renames from the test commit.

And dropped another unneeded `OP_TRUE` from tx_pool.
💬 Sjors commented on pull request "Add multiprocess binaries to release build":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2491393950)
Rebased after fe3457c.