Repository: bitcoin/bitcoin
File: test/functional/interface_ipc.py (Python)

Code before:
            res = await mining.result.checkBlock(block.serialize(), check_opts)
            assert_equal(res.result, True)
            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)
            miniwallet.rescan_utxos()
            assert_equal(miniwallet.get_balance(), balance + 1)
            self.log.debug("Check block should fail now, since it is a duplicate")
            res = await mining.result.checkBlock(block.serialize(), check_opts)
            assert_equal(res.result, False)
            assert_equal(res.reason, "inconclusive-not-best-prevblk")

            self.log.debug("Destroy template objects")
            template.result.destroy(ctx)
            template2.result.destroy(ctx)
            template3.result.destroy(ctx)
            template4.result.destroy(ctx)
            template5.result.destroy(ctx)
            template6.result.destroy(ctx)
            template7.result.destroy(ctx)
        asyncio.run(capnp.run(async_routine()))

    def run_test(self):
        self.run_echo_test()
        self.run_mining_test()

if __name__ == '__main__':
    IPCInterfaceTest(__file__).main()


Review comment (bug):
I want to suggest adding a check here:

```suggestion
            assert_equal(res.result, True)
            # self.nodes[1]'s chaintip did not advance.
            assert_equal(self.nodes[1].getchaintips()[0]["height"], current_block_height)
```

But I'm a bit confused, because testing this without the changes to `src/node/miner.cpp`, self.nodes[1]'s chaintip advances after submitSolution is called with the witnessless coinbase, and this check I'm suggesting fails. 

To reproduce, I'm using the following diff on this branch: 

```diff
diff --git a/src/node/miner.cpp b/src/node/miner.cpp
index b988e28a3f..485984e437 100644
--- a/src/node/miner.cpp
+++ b/src/node/miner.cpp
@@ -454,9 +454,9 @@ void AddMerkleRootAndCoinbase(CBlock& block, CTransactionRef coinbase, uint32_t
     block.hashMerkleRoot = BlockMerkleRoot(block);

     // Reset cached checks
-    block.m_checked_witness_commitment = false;
-    block.m_checked_merkle_root = false;
-    block.fChecked = false;
+    // block.m_checked_witness_commitment = false;
+    // block.m_checked_merkle_root = false;
+    // block.fChecked = false;
 }

 std::unique_ptr<CBlockTemplate> WaitAndCreateNewBlock(ChainstateManager& chainman,
diff --git a/test/functional/interface_ipc.py b/test/functional/interface_ipc.py
index a6010c2fb3..0d99a6b2c3 100755
--- a/test/functional/interface_ipc.py
+++ b/test/functional/interface_ipc.py
@@ -219,7 +219,9 @@ class IPCInterfaceTest(BitcoinTestFramework):
             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)
+            #assert_equal(res.result, False)
+            # self.nodes[1]'s chaintip did not advance.
+            assert_equal(self.nodes[1].getchaintips()[0]["height"], current_block_height)

             self.log.debug("Submit again, with the witness")
             res = await template.result.submitSolution(ctx, block.nVersion, block.nTime, block.nNonce, coinbase.serialize())
```

And the check fails, because self.nodes[1]'s chaintip was advanced!

Code after:
            assert_equal(res.result, False)
            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)

            # The remote template block will be mutated, capture the original:
            remote_block_before = await self.parse_and_deserialize_block(template, ctx)

            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("Even a rejected submitBlock() mutates the template's block")
            # Can be used by clients to download and inspect the (rejected)
            # reconstructed block.
            remote_block_after = await self.parse_and_deserialize_block(template, ctx)
            assert_not_equal(remote_block_before.serialize().hex(), remote_block_after.serialize().hex())

            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)

            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")
            res = await mining.result.checkBlock(block.serialize(), check_opts)
            assert_equal(res.result, False)
            assert_equal(res.reason, "inconclusive-not-best-prevblk")

            self.log.debug("Destroy template objects")
            template.result.destroy(ctx)
            template2.result.destroy(ctx)
            template3.result.destroy(ctx)
            template4.result.destroy(ctx)
            template5.result.destroy(ctx)
            template6.result.destroy(ctx)
            template7.result.destroy(ctx)
        asyncio.run(capnp.run(async_routine()))

    def run_test(self):