Add bitcoin node + IPC to test harness #602
No reviewers
Labels
No labels
bug
documentation
duplicate
enhancement
good first issue
help wanted
invalid
question
wontfix
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
parasitepool/para!602
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "bitcoin-node-ipc"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
CI never installs capnproto, this results in the IPC path being compiled out.
The test-linux job (.github/workflows/ci.yaml:40-61) runs only ./bin/install-bitcoin-core-linux (the "Install bitcoind" step) then
cargo test --all. Neitherjust installnor bin/install-capnproto-linux runs, and ubuntu-latest ships no libcapnp-dev.HAVE_CAPNP=falsemeans all code related to IPC is gutted from ckpool at compile time.@ -26,1 +25,4 @@# /usr/local/libexec is root-owned on GitHub Actions runners, so expose# bitcoin-node on PATH like bitcoind instead of installing into libexec.rm -f /usr/local/bin/bitcoin-nodeYou also should attempt (ignoring failure or gating on file existence/access before attempting)
rm -f /usr/local/libexec/bitcoin-nodeFailing to include this will leave the old copy on any machine that has previously executed this script and because of how it prioritizes libexec it will leave an outdated copy on future version bumps.
@ -382,0 +391,4 @@* single miner signet can sit in IBD forever by design and producing* templates there is the whole point. Mirrors the getblocktemplate* RPC, which rejects in IBD only when !isTestChain. */bool ready() const { return have_mining_.load() && (!ibd_.load() || is_test_chain_); }This will cause it to fail over from IPC to RPC with infinite retries, no backoff, and only stopping when IBD is complete.
See
stratifier.c:2063-2069for the offending loop. This is due to IPC dispatching tip updates during IBD and results in ipcnotify triggering a workbase update.Additionally this modification will cause Sv2 to report that template declarations are not allowed (persisting until IBD is complete or as long as Sv2 clients rely on the last reported configuration). This is lower priority than the above as it only impacts Sv2 which we do not use currently.
@ -480,2 +493,3 @@* unwinds to the reconnect path below. */* unwinds to the reconnect path below. The result is* the IBD state ready() gates on. */auto probe = mining_.isInitialBlockDownloadRequest();mining_ipc.cpp:495-497andmining_ipc.cpp:559-561are identical and can be abstracted to a privaterefresh_ibd(kj::WaitScope&)helper.@ -544,1 +561,3 @@probe.send().wait(ws);ibd_.store(probe.send().wait(ws).getResult());{What do these braces accomplish? It appears to be ineffectual in this context?
@ -598,0 +621,4 @@std::atomic<bool> ibd_{false};/* Written on the service thread before have_mining_ publishes the* connection, so plain bool is sufficient. */bool is_test_chain_{false};Comment is only true for initial publication which is latched. On a bitcoind restart, the keepalive loop catches the disconnect (clearing
have_mining_around line 501), then on its next pass callsobtain_mining(ws, init)atmining_ipc.cpp:488.obtain_mining (mining_ipc.cpp:549) rewritesis_test_chain_at line 566, then re-publishes withhave_mining_.store(true)at line 570.This creates a race condition that can misreport
is_test_chain_as false. Recommend usingstd::atomic<bool>as the above variables do.@ -623,0 +652,4 @@* regtest and single miner signets that wait never ends (upstream* says as much), and ckpool wants a template regardless, so opt* out. */req.setCooldown(false);This bypasses a reorg protection that is less important on regtest/signet, but very important to retain for mainnet.
I would recommend
setCooldown(!is_test_chain_)instead.@ -29,5 +29,6 @@"startdiff" : 1,"maxdiff" : 0,"zmqblock" : "tcp://127.0.0.1:28332","ipcmining" : "copr/signet/node.sock",The local testing setup should probably explicitly specify the same path for ipcbind/ipcmining. As this assumes some things about the current working directory.
@ -304,0 +307,4 @@let subdir = match self.network {Network::Signet => "signet",Network::Regtest => "regtest",_ => return None,The Option returned here is a bit ambigous and while it can be None for both a unsupported network OR a missing datadir, it is only ever reported in test_ckpool.rs as the latter.
We can dodge this function completely by just including a static path in our ipcbind for the harness.
ipcbind=unix:/absolute/path