From a5e61b1917aff7567599beb59cc7093978b7e336 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Mon, 8 Dec 2025 21:23:40 -0500 Subject: [PATCH] test: interface_ipc.py minor fixes and cleanup There are a few things that are incorrect or messy in the interface_ipc.py test. This commit tries to clean them up: - isTestChain and isInitialBlockDownload asserts were not checking the results of those calls, only that calls were, made because they were not checking the responses' .result member. - A lot of result accesses like `template.result` `mining.result` were repeated unnecessarily because variables like `template` and `mining` were assigned response objects instead of result objects. These variables are now changed to point directly to results. - Some coroutine calls were assigned to temporary `wait` before being awaited. This was unnecessarily confusing and would make code not run in top-down order. - `to_dict` calls were being made to check if result variables were unset. This was inefficient and indirect because it iterates over all fields in response structs instead of just checking whether the result field is present. The to_dict calls are now replaced with more direct `_has('result')` calls. - The `res` variables used to hold various responses did not have descriptive names. These are replaced with clearer names. Co-authored-by: rkrux --- test/functional/interface_ipc.py | 77 +++++++++++++++++--------------- 1 file changed, 41 insertions(+), 36 deletions(-) diff --git a/test/functional/interface_ipc.py b/test/functional/interface_ipc.py index d16145107c7..c04a05762b0 100755 --- a/test/functional/interface_ipc.py +++ b/test/functional/interface_ipc.py @@ -128,27 +128,25 @@ class IPCInterfaceTest(BitcoinTestFramework): async def async_routine(): ctx, init = await self.make_capnp_init_ctx() self.log.debug("Create Mining proxy object") - mining = init.makeMining(ctx) + mining = init.makeMining(ctx).result self.log.debug("Test simple inspectors") - assert (await mining.result.isTestChain(ctx)) - assert (await mining.result.isInitialBlockDownload(ctx)) - blockref = await mining.result.getTip(ctx) + assert (await mining.isTestChain(ctx)).result + assert not (await mining.isInitialBlockDownload(ctx)).result + blockref = await mining.getTip(ctx) assert blockref.hasResult assert_equal(len(blockref.result.hash), block_hash_size) current_block_height = self.nodes[0].getchaintips()[0]["height"] assert blockref.result.height == current_block_height self.log.debug("Mine a block") - wait = mining.result.waitTipChanged(ctx, blockref.result.hash, ) self.generate(self.nodes[0], 1) - newblockref = await wait - assert_equal(len(newblockref.result.hash), block_hash_size) - assert_equal(newblockref.result.height, current_block_height + 1) + newblockref = (await mining.waitTipChanged(ctx, blockref.result.hash)).result + assert_equal(len(newblockref.hash), block_hash_size) + assert_equal(newblockref.height, current_block_height + 1) self.log.debug("Wait for timeout") - wait = mining.result.waitTipChanged(ctx, newblockref.result.hash, timeout) - oldblockref = await wait - assert_equal(len(newblockref.result.hash), block_hash_size) - assert_equal(oldblockref.result.hash, newblockref.result.hash) - assert_equal(oldblockref.result.height, newblockref.result.height) + oldblockref = (await mining.waitTipChanged(ctx, newblockref.hash, timeout)).result + assert_equal(len(newblockref.hash), block_hash_size) + assert_equal(oldblockref.hash, newblockref.hash) + assert_equal(oldblockref.height, newblockref.height) async with AsyncExitStack() as stack: self.log.debug("Create a template") @@ -156,12 +154,13 @@ class IPCInterfaceTest(BitcoinTestFramework): opts.useMempool = True opts.blockReservedWeight = 4000 opts.coinbaseOutputMaxAdditionalSigops = 0 - template = await create_block_template(mining.result, stack, ctx, opts) + template = await create_block_template(mining, stack, ctx, opts) + self.log.debug("Test some inspectors of Template") - header = await template.getBlockHeader(ctx) - assert_equal(len(header.result), block_header_size) + header = (await template.getBlockHeader(ctx)).result + assert_equal(len(header), block_header_size) block = await self.parse_and_deserialize_block(template, ctx) - assert_equal(ser_uint256(block.hashPrevBlock), newblockref.result.hash) + assert_equal(ser_uint256(block.hashPrevBlock), newblockref.hash) assert len(block.vtx) >= 1 txfees = await template.getTxFees(ctx) assert_equal(len(txfees.result), 0) @@ -171,6 +170,7 @@ class IPCInterfaceTest(BitcoinTestFramework): coinbase = CTransaction() coinbase.deserialize(coinbase_data) assert_equal(coinbase.vin[0].prevout.hash, 0) + self.log.debug("Wait for a new template") waitoptions = self.capnp_modules['mining'].BlockWaitOptions() waitoptions.timeout = timeout @@ -179,28 +179,33 @@ class IPCInterfaceTest(BitcoinTestFramework): template2 = await wait_next_template(template, stack, ctx, waitoptions) block2 = await self.parse_and_deserialize_block(template2, ctx) assert_equal(len(block2.vtx), 1) + self.log.debug("Wait for another, but time out") template3 = await template2.waitNext(ctx, waitoptions) - assert_equal(template3.to_dict(), {}) + assert_equal(template3._has("result"), False) + self.log.debug("Wait for another, get one after increase in fees in the mempool") miniwallet.send_self_transfer(fee_rate=10, from_node=self.nodes[0]) template4 = await wait_next_template(template2, stack, ctx, waitoptions) block3 = await self.parse_and_deserialize_block(template4, ctx) assert_equal(len(block3.vtx), 2) + self.log.debug("Wait again, this should return the same template, since the fee threshold is zero") waitoptions.feeThreshold = 0 template5 = await wait_next_template(template4, stack, ctx, waitoptions) block4 = await self.parse_and_deserialize_block(template5, ctx) assert_equal(len(block4.vtx), 2) waitoptions.feeThreshold = 1 + self.log.debug("Wait for another, get one after increase in fees in the mempool") miniwallet.send_self_transfer(fee_rate=10, from_node=self.nodes[0]) template6 = await wait_next_template(template5, stack, ctx, waitoptions) block4 = await self.parse_and_deserialize_block(template6, ctx) assert_equal(len(block4.vtx), 3) + self.log.debug("Wait for another, but time out, since the fee threshold is set now") template7 = await template6.waitNext(ctx, waitoptions) - assert_equal(template7.to_dict(), {}) + assert_equal(template7._has("result"), False) self.log.debug("interruptWait should abort the current wait") wait_started = asyncio.Event() @@ -209,7 +214,8 @@ class IPCInterfaceTest(BitcoinTestFramework): new_waitoptions.timeout = waitoptions.timeout * 60 # 1 minute wait new_waitoptions.feeThreshold = 1 wait_started.set() - return await template6.waitNext(ctx, new_waitoptions) + template7 = await template6.waitNext(ctx, new_waitoptions) + assert_equal(template7._has("result"), False) async def interrupt_wait(): await wait_started.wait() # Wait for confirmation wait started @@ -222,11 +228,10 @@ class IPCInterfaceTest(BitcoinTestFramework): result = await wait_task await interrupt_task - assert_equal(result.to_dict(), {}) current_block_height = self.nodes[0].getchaintips()[0]["height"] check_opts = self.capnp_modules['mining'].BlockCheckOptions() - async with destroying((await mining.result.createNewBlock(opts)).result, ctx) as template: + async with destroying((await mining.createNewBlock(opts)).result, ctx) as template: block = await self.parse_and_deserialize_block(template, ctx) coinbase = await self.parse_and_deserialize_coinbase_tx(template, ctx) balance = miniwallet.get_balance() @@ -238,26 +243,26 @@ class IPCInterfaceTest(BitcoinTestFramework): self.log.debug("Submit a block with a bad version") block.nVersion = 0 block.solve() - res = await mining.result.checkBlock(block.serialize(), check_opts) - assert_equal(res.result, False) - assert_equal(res.reason, "bad-version(0x00000000)") - res = await template.submitSolution(ctx, block.nVersion, block.nTime, block.nNonce, coinbase.serialize()) - assert_equal(res.result, False) + check = await mining.checkBlock(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 + assert_equal(submitted, 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) + block_valid = (await mining.checkBlock(block.serialize(), check_opts)).result + assert_equal(block_valid, 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.submitSolution(ctx, block.nVersion, block.nTime, block.nNonce, coinbase.serialize_without_witness()) - assert_equal(res.result, False) + submitted = (await template.submitSolution(ctx, block.nVersion, block.nTime, block.nNonce, coinbase.serialize_without_witness())).result + assert_equal(submitted, False) self.log.debug("Even a rejected submitBlock() mutates the template's block") # Can be used by clients to download and inspect the (rejected) @@ -266,8 +271,8 @@ class IPCInterfaceTest(BitcoinTestFramework): assert_not_equal(remote_block_before.serialize().hex(), remote_block_after.serialize().hex()) self.log.debug("Submit again, with the witness") - res = await template.submitSolution(ctx, block.nVersion, block.nTime, block.nNonce, coinbase.serialize()) - assert_equal(res.result, True) + submitted = (await template.submitSolution(ctx, block.nVersion, block.nTime, block.nNonce, coinbase.serialize())).result + assert_equal(submitted, True) self.log.debug("Block should propagate") # Check that the IPC node actually updates its own chain @@ -280,9 +285,9 @@ class IPCInterfaceTest(BitcoinTestFramework): 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") + check = await mining.checkBlock(block.serialize(), check_opts) + assert_equal(check.result, False) + assert_equal(check.reason, "inconclusive-not-best-prevblk") asyncio.run(capnp.run(async_routine()))