diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index 150295e5b7b..50a7922a8d5 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -54,9 +54,22 @@ public: virtual std::vector getCoinbaseMerklePath() = 0; /** - * Construct and broadcast the block. + * Construct and broadcast the block. Modifies the template in place, + * updating the fields listed below as well as the merkle root. * - * @returns if the block was processed, independent of block validity + * @param[in] version version block header field + * @param[in] timestamp time block header field (unix timestamp) + * @param[in] nonce nonce block header field + * @param[in] coinbase complete coinbase transaction (including witness) + * + * @note unlike the submitblock RPC, this method does NOT add the + * coinbase witness automatically. + * + * @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). */ virtual bool submitSolution(uint32_t version, uint32_t timestamp, uint32_t nonce, CTransactionRef coinbase) = 0; diff --git a/src/node/miner.cpp b/src/node/miner.cpp index 28e9048a4dd..b988e28a3ff 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -452,6 +452,11 @@ void AddMerkleRootAndCoinbase(CBlock& block, CTransactionRef coinbase, uint32_t block.nTime = timestamp; block.nNonce = nonce; block.hashMerkleRoot = BlockMerkleRoot(block); + + // Reset cached checks + block.m_checked_witness_commitment = false; + block.m_checked_merkle_root = false; + block.fChecked = false; } std::unique_ptr WaitAndCreateNewBlock(ChainstateManager& chainman, diff --git a/test/functional/interface_ipc.py b/test/functional/interface_ipc.py index abcc4d6b5d1..e905c7753be 100755 --- a/test/functional/interface_ipc.py +++ b/test/functional/interface_ipc.py @@ -8,7 +8,11 @@ from io import BytesIO from pathlib import Path import shutil from test_framework.messages import (CBlock, CTransaction, ser_uint256, COIN) -from test_framework.test_framework import (BitcoinTestFramework, assert_equal) +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, + assert_not_equal +) from test_framework.wallet import MiniWallet # Test may be skipped and not have capnp installed @@ -49,10 +53,10 @@ class IPCInterfaceTest(BitcoinTestFramework): } def set_test_params(self): - self.num_nodes = 1 + self.num_nodes = 2 def setup_nodes(self): - self.extra_init = [{"ipcbind": True}] + self.extra_init = [{"ipcbind": True}, {}] super().setup_nodes() # Use this function to also load the capnp modules (we cannot use set_test_params for this, # as it is being called before knowing whether capnp is available). @@ -206,11 +210,26 @@ class IPCInterfaceTest(BitcoinTestFramework): self.log.debug("Submit a valid block") block.nVersion = original_version block.solve() + + self.log.debug("First call checkBlock()") res = await mining.result.checkBlock(block.serialize(), check_opts) assert_equal(res.result, True) + + self.log.debug("Submitted coinbase must include witness") + assert_not_equal(coinbase.serialize_without_witness().hex(), coinbase.serialize().hex()) + res = await template.result.submitSolution(ctx, block.nVersion, block.nTime, block.nNonce, coinbase.serialize_without_witness()) + assert_equal(res.result, False) + + self.log.debug("Submit again, with the witness") res = await template.result.submitSolution(ctx, block.nVersion, block.nTime, block.nNonce, coinbase.serialize()) assert_equal(res.result, True) - assert_equal(self.nodes[0].getchaintips()[0]["height"], current_block_height + 1) + + self.log.debug("Block should propagate") + assert_equal(self.nodes[1].getchaintips()[0]["height"], current_block_height + 1) + # Stalls if a regression causes submitBlock() to accept an invalid block: + self.sync_all() + assert_equal(self.nodes[0].getchaintips()[0], self.nodes[1].getchaintips()[0]) + miniwallet.rescan_utxos() assert_equal(miniwallet.get_balance(), balance + 1) self.log.debug("Check block should fail now, since it is a duplicate")