mining: pass missing context to BlockTemplate methods

This is breaking IPC interface change.

Adding a context parameter ensures that these methods are run in
their own thread and don't block other calls. They were missing
for:

- createNewBlock()
- checkBlock()

A later commit makes createNewBlock() potentially long running. This
makes the lack of context parameter a problem, justifying a breaking
change.

Since this is a breaking API change, we also fix the same issue for
checkBlock. This ensures that when checkBlock() receives a slow to
validate block from an untrusted origin (the IPC connection itself
is assumed to trusted), it will not block the IPC server (though
validation itself still blocks cs_main).

To make this change explicit, bump the version of makeMining from 2
to 3. Old clients calling the original ordinal will receive an error
message suggesting they update. A test covers this behavior.

Author: Sjors

src/interfaces/init.h

@@ -39,6 +39,9 @@ class Init
     virtual Ipc* ipc() { return nullptr; }
     virtual bool canListenIpc() { return false; }
     virtual const char* exeName() { return nullptr; }
+    virtual void makeMiningDeprecatedV2() {
+        throw std::runtime_error("Old mining interface (@2) not supported. Please update your client!");
+    }
 };
 
 //! Return implementation of Init interface for the node process. If the argv

src/ipc/capnp/init.capnp

@@ -19,5 +19,8 @@ using Mining = import "mining.capnp";
 interface Init $Proxy.wrap("interfaces::Init") {
     construct @0 (threadMap: Proxy.ThreadMap) -> (threadMap :Proxy.ThreadMap);
     makeEcho @1 (context :Proxy.Context) -> (result :Echo.Echo);
-    makeMining @2 (context :Proxy.Context) -> (result :Mining.Mining);
+    makeMining @3 (context :Proxy.Context) -> (result :Mining.Mining);
+
+    # DEPRECATED: no longer supported; server returns an error.
+    makeMiningDeprecatedV2 @2 () -> ();
 }

src/ipc/capnp/mining.capnp

@@ -17,8 +17,8 @@ interface Mining $Proxy.wrap("interfaces::Mining") {
     isInitialBlockDownload @1 (context :Proxy.Context) -> (result: Bool);
     getTip @2 (context :Proxy.Context) -> (result: Common.BlockRef, hasResult: Bool);
     waitTipChanged @3 (context :Proxy.Context, currentTip: Data, timeout: Float64) -> (result: Common.BlockRef);
-    createNewBlock @4 (options: BlockCreateOptions) -> (result: BlockTemplate);
-    checkBlock @5 (block: Data, options: BlockCheckOptions) -> (reason: Text, debug: Text, result: Bool);
+    createNewBlock @4 (context :Proxy.Context, options: BlockCreateOptions) -> (result: BlockTemplate);
+    checkBlock @5 (context :Proxy.Context, block: Data, options: BlockCheckOptions) -> (reason: Text, debug: Text, result: Bool);
 }
 
 interface BlockTemplate $Proxy.wrap("interfaces::BlockTemplate") {

test/functional/interface_ipc.py

@@ -68,9 +68,27 @@ async def async_routine():
 
         asyncio.run(capnp.run(async_routine()))
 
+    def run_deprecated_mining_test(self):
+        self.log.info("Running deprecated mining interface test")
+        async def async_routine():
+            ctx, init = await make_capnp_init_ctx(self)
+            self.log.debug("Calling deprecated makeMiningDeprecatedV2 should raise an error")
+            try:
+                await init.makeMiningDeprecatedV2()
+                assert False, "Expected an exception"
+            except capnp.KjException as e:
+                assert "Old mining interface" in str(e)
+        asyncio.run(capnp.run(async_routine()))
+
     def run_test(self):
         self.run_echo_test()
         self.run_mining_test()
 
+        # Run at the end: the exception triggers a disconnect which causes a race
+        # in libmultiprocess thread cleanup. See:
+        # https://github.com/bitcoin-core/libmultiprocess/issues/189
+        # https://github.com/bitcoin/bitcoin/pull/34284
+        self.run_deprecated_mining_test()
+
 if __name__ == '__main__':
     IPCInterfaceTest(__file__).main()

test/functional/interface_ipc_mining.py

@@ -261,7 +261,7 @@ async def async_routine():
             self.log.debug("Enforce minimum reserved weight for IPC clients too")
             opts.blockReservedWeight = 0
             try:
-                await mining.createNewBlock(opts)
+                await mining.createNewBlock(ctx, opts)
                 raise AssertionError("createNewBlock unexpectedly succeeded")
             except capnp.lib.capnp.KjException as e:
                 if e.type == "DISCONNECTED":
@@ -288,7 +288,7 @@ async def async_routine():
             current_block_height = self.nodes[0].getchaintips()[0]["height"]
             check_opts = self.capnp_modules['mining'].BlockCheckOptions()
 
-            async with destroying((await mining.createNewBlock(self.default_block_create_options)).result, ctx) as template:
+            async with destroying((await mining.createNewBlock(ctx, self.default_block_create_options)).result, ctx) as template:
                 block = await mining_get_block(template, ctx)
                 balance = self.miniwallet.get_balance()
                 coinbase = await self.build_coinbase_test(template, ctx, self.miniwallet)
@@ -301,7 +301,7 @@ async def async_routine():
                 self.log.debug("Submit a block with a bad version")
                 block.nVersion = 0
                 block.solve()
-                check = await mining.checkBlock(block.serialize(), check_opts)
+                check = await mining.checkBlock(ctx, block.serialize(), check_opts)
                 assert_equal(check.result, False)
                 assert_equal(check.reason, "bad-version(0x00000000)")
                 submitted = (await template.submitSolution(ctx, block.nVersion, block.nTime, block.nNonce, coinbase.serialize())).result
@@ -311,7 +311,7 @@ async def async_routine():
                 block.solve()
 
                 self.log.debug("First call checkBlock()")
-                block_valid = (await mining.checkBlock(block.serialize(), check_opts)).result
+                block_valid = (await mining.checkBlock(ctx, block.serialize(), check_opts)).result
                 assert_equal(block_valid, True)
 
                 # The remote template block will be mutated, capture the original:
@@ -343,7 +343,7 @@ async def async_routine():
             self.miniwallet.rescan_utxos()
             assert_equal(self.miniwallet.get_balance(), balance + 1)
             self.log.debug("Check block should fail now, since it is a duplicate")
-            check = await mining.checkBlock(block.serialize(), check_opts)
+            check = await mining.checkBlock(ctx, block.serialize(), check_opts)
             assert_equal(check.result, False)
             assert_equal(check.reason, "inconclusive-not-best-prevblk")
 

test/functional/test_framework/ipc_util.py

@@ -108,7 +108,7 @@ async def make_capnp_init_ctx(self):
 
 async def mining_create_block_template(mining, stack, ctx, opts):
     """Call mining.createNewBlock() and return template, then call template.destroy() when stack exits."""
-    response = await mining.createNewBlock(opts)
+    response = await mining.createNewBlock(ctx, opts)
     if not response._has("result"):
         return None
     return await stack.enter_async_context(destroying(response.result, ctx))