diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index ff4f87109f9..1ed224e3051 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -16,6 +16,7 @@ #include #include #include +#include #include #include @@ -59,6 +60,8 @@ public: * @param[in] timestamp time block header field (unix timestamp) * @param[in] nonce nonce block header field * @param[in] coinbase complete coinbase transaction (including witness) + * @param[out] reason failure reason (BIP22) + * @param[out] debug more detailed rejection reason * * @note Unlike the submitblock RPC, this method does not call * UpdateUncommittedBlockStructures to add a missing coinbase witness @@ -69,13 +72,16 @@ public: * is only one byte long, so the coinbase scriptSig needs at least * one additional byte of data to avoid bad-cb-length. * - * @returns if the block was processed, does not necessarily indicate validity. - * - * @note Returns true if the block is already known, which can happen if - * the solved block is constructed and broadcast by multiple nodes - * (e.g. both the miner who constructed the template and the pool). + * @returns true if the block was accepted as a new block */ - virtual bool submitSolution(uint32_t version, uint32_t timestamp, uint32_t nonce, CTransactionRef coinbase) = 0; + virtual bool submitSolution(uint32_t version, uint32_t timestamp, uint32_t nonce, CTransactionRef coinbase, std::string& reason, std::string& debug) = 0; + + //! Deprecated older method preserved to return an explicit error for IPC + //! clients using mining.capnp @7. + virtual bool submitSolutionOld7(uint32_t, uint32_t, uint32_t, CTransactionRef) + { + throw std::runtime_error("Old submitSolution (@7) not supported. Please update your client!"); + } /** * Waits for fees in the next block to rise, a new tip or the timeout. diff --git a/src/ipc/capnp/mining.capnp b/src/ipc/capnp/mining.capnp index a6dd8d71f31..f2aee662e03 100644 --- a/src/ipc/capnp/mining.capnp +++ b/src/ipc/capnp/mining.capnp @@ -36,9 +36,12 @@ interface BlockTemplate $Proxy.wrap("interfaces::BlockTemplate") { getTxSigops @4 (context: Proxy.Context) -> (result: List(Int64)); getCoinbaseTx @5 (context: Proxy.Context) -> (result: CoinbaseTx); getCoinbaseMerklePath @6 (context: Proxy.Context) -> (result: List(Data)); - submitSolution @7 (context: Proxy.Context, version: UInt32, timestamp: UInt32, nonce: UInt32, coinbase :Data) -> (result: Bool); + submitSolution @10 (context: Proxy.Context, version: UInt32, timestamp: UInt32, nonce: UInt32, coinbase :Data) -> (reason: Text, debug: Text, result: Bool); waitNext @8 (context: Proxy.Context, options: BlockWaitOptions) -> (result: BlockTemplate); interruptWait @9() -> (); + + # DEPRECATED: older version of submitSolution which returns an error. + submitSolutionOld7 @7 (context: Proxy.Context, version: UInt32, timestamp: UInt32, nonce: UInt32, coinbase :Data) -> (result: Bool); } struct BlockCreateOptions $Proxy.wrap("node::BlockCreateOptions") { diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 6747dd60aaa..0699ff8b2f1 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -917,12 +917,12 @@ public: return TransactionMerklePath(m_block_template->block, 0); } - bool submitSolution(uint32_t version, uint32_t timestamp, uint32_t nonce, CTransactionRef coinbase) override + bool submitSolution(uint32_t version, uint32_t timestamp, uint32_t nonce, CTransactionRef coinbase, std::string& reason, std::string& debug) override { AddMerkleRootAndCoinbase(m_block_template->block, std::move(coinbase), version, timestamp, nonce); - std::string reason; - std::string debug; - return SubmitBlock(chainman(), std::make_shared(m_block_template->block), /*new_block=*/nullptr, reason, debug); + bool new_block; + const bool accepted = SubmitBlock(chainman(), std::make_shared(m_block_template->block), &new_block, reason, debug); + return accepted && new_block && reason.empty(); } std::unique_ptr waitNext(BlockWaitOptions options) override diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index fd9559b543c..51fe5699403 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -860,9 +860,9 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) } // Alternate calls between submitBlock and submitSolution via the // Mining interface. + std::string reason{"stale reason"}; + std::string debug{"stale debug"}; if (current_height % 2 == 0) { - std::string reason{"stale reason"}; - std::string debug{"stale debug"}; BOOST_REQUIRE(mining->submitBlock(block, reason, debug)); BOOST_REQUIRE_EQUAL(reason, ""); BOOST_REQUIRE_EQUAL(debug, ""); @@ -873,7 +873,14 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) BOOST_REQUIRE_EQUAL(reason, "duplicate"); BOOST_REQUIRE_EQUAL(debug, ""); } else { - BOOST_REQUIRE(block_template->submitSolution(block.nVersion, block.nTime, block.nNonce, MakeTransactionRef(txCoinbase))); + reason = "stale reason"; + debug = "stale debug"; + BOOST_REQUIRE(block_template->submitSolution(block.nVersion, block.nTime, block.nNonce, MakeTransactionRef(txCoinbase), reason, debug)); + BOOST_REQUIRE_EQUAL(reason, ""); + BOOST_REQUIRE_EQUAL(debug, ""); + BOOST_CHECK_THROW(block_template->submitSolutionOld7(block.nVersion, block.nTime, block.nNonce, + MakeTransactionRef(txCoinbase)), + std::runtime_error); } { LOCK(cs_main); diff --git a/test/functional/interface_ipc_mining.py b/test/functional/interface_ipc_mining.py index 8d2ee85964c..89d91c96ef5 100755 --- a/test/functional/interface_ipc_mining.py +++ b/test/functional/interface_ipc_mining.py @@ -531,8 +531,10 @@ class IPCMiningTest(BitcoinTestFramework): assert_equal(check.reason, "bad-version(0x00000000)") assert_equal(check.debug, "rejected nVersion=0x00000000 block") self.log.debug("submitSolution should reject a bad-version block") - submitted = (await template.submitSolution(ctx, block.nVersion, block.nTime, block.nNonce, coinbase.serialize())).result - assert_equal(submitted, False) + result = await template.submitSolution(ctx, block.nVersion, block.nTime, block.nNonce, coinbase.serialize()) + assert_equal(result.result, False) + assert_equal(result.reason, "bad-version(0x00000000)") + assert_equal(result.debug, "rejected nVersion=0x00000000 block") self.log.debug("submitBlock should reject a bad-version block") await self.assert_submit_block( mining2, @@ -565,8 +567,10 @@ class IPCMiningTest(BitcoinTestFramework): missing_witness_block.hashMerkleRoot = missing_witness_block.calc_merkle_root() missing_witness_block.solve() self.log.debug("submitSolution should reject a coinbase missing witness") - submitted = (await template.submitSolution(ctx, block.nVersion, block.nTime, block.nNonce, coinbase.serialize_without_witness())).result - assert_equal(submitted, False) + result = await template.submitSolution(ctx, block.nVersion, block.nTime, block.nNonce, coinbase.serialize_without_witness()) + assert_equal(result.result, False) + assert_equal(result.reason, "bad-witness-nonce-size") + assert_equal(result.debug, "CheckWitnessMalleation : invalid witness reserved value size") self.log.debug("Even a rejected submitSolution() mutates the template's block") # Can be used by clients to download and inspect the (rejected) @@ -585,8 +589,10 @@ class IPCMiningTest(BitcoinTestFramework): ) self.log.debug("Submit again, with the witness") - submitted = (await template.submitSolution(ctx, block.nVersion, block.nTime, block.nNonce, coinbase.serialize())).result - assert_equal(submitted, True) + result = await template.submitSolution(ctx, block.nVersion, block.nTime, block.nNonce, coinbase.serialize()) + assert_equal(result.result, True) + assert_equal(result.reason, "") + assert_equal(result.debug, "") self.log.debug("Submit a valid complete block through the disconnected node") await self.assert_submit_block(mining2, ctx2, block, result=True) @@ -615,7 +621,7 @@ class IPCMiningTest(BitcoinTestFramework): self.log.debug("submitBlock on the same node should fail with duplicate after submitSolution succeeds") await self.assert_submit_block(mining, ctx, block, result=False, reason="duplicate") - self.log.debug("submitSolution should still return True for a duplicate after submitBlock succeeds") + self.log.debug("submitSolution should return duplicate after submitBlock succeeds") async with destroying((await mining2.createNewBlock(ctx2, self.default_block_create_options)).result, ctx2) as template2: duplicate_block = await self.build_candidate_block(template2, ctx2) duplicate_coinbase = duplicate_block.vtx[0] @@ -623,32 +629,53 @@ class IPCMiningTest(BitcoinTestFramework): self.log.debug("Submit a valid complete block before duplicate submitSolution") await self.assert_submit_block(mining2, ctx2, duplicate_block, result=True) self.nodes[2].waitforblockheight(current_block_height + 2) - self.log.debug("submitSolution should accept the duplicate block") - submitted = (await template2.submitSolution(ctx2, duplicate_block.nVersion, duplicate_block.nTime, duplicate_block.nNonce, duplicate_coinbase.serialize())).result - assert_equal(submitted, True) + self.log.debug("submitSolution should reject the duplicate block") + result = await template2.submitSolution(ctx2, duplicate_block.nVersion, duplicate_block.nTime, duplicate_block.nNonce, duplicate_coinbase.serialize()) + assert_equal(result.result, False) + assert_equal(result.reason, "duplicate") + assert_equal(result.debug, "") self.sync_all() - self.log.debug("submitBlock should report inconclusive for a valid stale block") + self.log.debug( + "submitSolution and submitBlock should report inconclusive for valid stale blocks") async with AsyncExitStack() as stack: active_template = await mining_create_block_template( mining2, stack, ctx2, self.default_block_create_options) + solution_template = await mining_create_block_template( + mining2, stack, ctx2, self.default_block_create_options) submit_block_template = await mining_create_block_template( mining2, stack, ctx2, self.default_block_create_options) assert active_template is not None + assert solution_template is not None assert submit_block_template is not None active_block = await self.build_candidate_block( active_template, ctx2, extra_nonce=b"\x01") + solution_block = await self.build_candidate_block( + solution_template, ctx2, extra_nonce=b"\x02") submit_block = await self.build_candidate_block( - submit_block_template, ctx2, extra_nonce=b"\x02") + submit_block_template, ctx2, extra_nonce=b"\x03") active_block.solve() + solution_block.solve() submit_block.solve() - # Both templates share a parent. The first block becomes active, - # so the remaining valid block is accepted as stale. + # All three templates share a parent. The first block becomes + # active, so the remaining valid blocks are accepted as stale. await self.assert_submit_block( mining2, ctx2, active_block, result=True) + solution_coinbase = solution_block.vtx[0] + result = await solution_template.submitSolution( + ctx2, + solution_block.nVersion, + solution_block.nTime, + solution_block.nNonce, + solution_coinbase.serialize(), + ) + assert_equal(result.result, False) + assert_equal(result.reason, "inconclusive") + assert_equal(result.debug, "") + await self.assert_submit_block( mining2, ctx2, submit_block, result=False, reason="inconclusive") self.sync_all() @@ -724,8 +751,10 @@ class IPCMiningTest(BitcoinTestFramework): block.vtx[0] = coinbase block.hashMerkleRoot = block.calc_merkle_root() block.solve() - submitted = (await template.submitSolution(ctx, block.nVersion, block.nTime, block.nNonce, coinbase.serialize())).result - assert_equal(submitted, True) + result = await template.submitSolution(ctx, block.nVersion, block.nTime, block.nNonce, coinbase.serialize()) + assert_equal(result.result, True) + assert_equal(result.reason, "") + assert_equal(result.debug, "") assert_equal(node.getblockcount(), height) asyncio.run(capnp.run(async_routine()))