Bitcoin Core Github
43 subscribers
122K links
Download Telegram
💬 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.
💬 brunoerg commented on pull request "fuzz: set the output argument of FuzzedSock::Accept()":
(https://github.com/bitcoin/bitcoin/pull/31316#issuecomment-2491407513)
> Perhaps this makes more sense in combination with a test that uses it? Currently FuzzedSock::Accept seems entirely unused: https://maflcko.github.io/b-c-cov/fuzz.coverage/src/test/fuzz/util/net.cpp.gcov.html

I agree. I was going to ask steps to reproduce then I noticed this is unused.
💬 Sjors commented on pull request "rpc: Remove submitblock pre-checks":
(https://github.com/bitcoin/bitcoin/pull/31175#discussion_r1852276089)
@mzumsande I don't recall such discussions. The only thing that matters for `getblockfrompeer` is that it _can_ store previously pruned blocks.

That said, I do think it's useful to allow `submitblock` to store a previously pruned block. It lets a user dump the block on one node and submit it on this node, without having go through the effort of establishing a p2p connection, figuring out the peer id and then calling `getblockfrompeer`.
💬 Sjors commented on pull request "rpc: Remove submitblock pre-checks":
(https://github.com/bitcoin/bitcoin/pull/31175#discussion_r1852312507)
@maflcko it's probably useful to briefly reopen #10146 and link to this thread for the likely real reason for that PR.

I'm speculating it was done in a stealthy way because there might be someone running a public service to submit blocks, and it would use that RPC under the hood. Or they were worried that since mining pools use this RPC internally (see e.g. #3658) they might have it exposed to the internet and therefore could be attacked.

cc @dergoegge @darosior in case you want to add it
...
📝 theStack opened a pull request: "test: add missing segwitv1 test cases to `script_standard_tests`"
(https://github.com/bitcoin/bitcoin/pull/31340)
Currently we have two segwitv1 output script types that are considered standard:
- `TxoutType::WITNESS_V1_TAPROOT` (P2TR): witness program has size 32 (introduced with taproot soft-fork)
- `TxoutType::ANCHOR` (P2A): witness program is {0x4e, 0x7e} (introduced with #30352)

This PR adds them to the script standardness unit tests where missing, i.e. for using them with the `ExtractDestination` and `GetScriptForDestination` functions.
💬 Sjors commented on pull request "rpc: Remove submitblock pre-checks":
(https://github.com/bitcoin/bitcoin/pull/31175#issuecomment-2491534171)
Note that #31196 drops `processNewBlock()` from the Mining IPC interface, which straight-forwardly uses `ProcessNewBlock`. The interface does still use it via `submitSolution()`.
💬 Sjors commented on pull request "Prune mining interface":
(https://github.com/bitcoin/bitcoin/pull/31196#issuecomment-2491565760)
I tested cherry-picking this PR on top of #31175 and the tests still pass.
📝 Parkeragan opened a pull request: "Add files via upload"
(https://github.com/bitcoin/bitcoin/pull/31341)

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improve coverage are always welcome.
* All other changes should have accompanying unit tests (see `src/test/`) or
functional tests (see `test/`). Contributors sh
...
💬 Sjors commented on pull request "rpc: Remove submitblock pre-checks":
(https://github.com/bitcoin/bitcoin/pull/31175#issuecomment-2491616239)
Something I noticed while looking at df7ad9223025b5d75e27813ce3b89ef6fc431805

The release notes of 0.18 "promise" that the RPC never returns `duplicate-invalid`:

```md
- The `submitblock` RPC previously returned the reason a rejected block
was invalid the first time it processed that block, but returned a
generic "duplicate" rejection message on subsequent occasions it
processed the same block. It now always returns the fundamental
reason for rejecting an invalid block and on
...
fanquake closed a pull request: "Add files via upload"
(https://github.com/bitcoin/bitcoin/pull/31341)