← Watch feed
test: interface_ipc.py minor fixes and cleanup
Public commit record
100/100 · Strong
What the developer wrote
Authored by Ryan Ofsky
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 <rkrux.connect@gmail.com>
✓ Descriptive subject✓ Names a concrete action or component✓ Uses a recognizable type or scope✓ Provides detailed explanatory context✓ Explains rationale or failure mode✓ Mentions testing or verification
What changed, and why it matters
This commit only cleans up a single test file. It fixes how the test checks return values from internal Bitcoin IPC calls and renames some variables for clarity. It does not change any production code that handles real Bitcoin transactions, blocks, or network messages, so it has no direct security impact on running Bitcoin nodes.
Recommended action
No security action needed. Treat as ordinary test maintenance.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch refactors test/functional/interface_ipc.py. It changes Cap’n Proto response handling so that variables like mining and template point to .result objects rather than response wrappers, and assertions now check the actual boolean/struct results of calls such as isTestChain, isInitialBlockDownload, checkBlock, and submitSolution. It also replaces to_dict() checks with _has('result'), removes intermediate wait assignments, and renames res to descriptive names. No production code is modified.
Changed components
test/functional/interface_ipc.pyInspect captured patch +41 / −36
diff --git a/test/functional/interface_ipc.py b/test/functional/interface_ipc.py
index d1614510..c04a0576 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()))
Risk score
Our methodology →Why this scored 15/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.