re: #33819 (comment)
It turns out that using the getCoinbase() result directly in the Python test led to memory management related errors. Parts of the coinbase were dropped by the time of the second checkBlock().
This probably happened once I extracted the coinbase specific checks into build_coinbase_test, which returns the coinbase CTransaction but presumably let go of the getCoinbase() result.
Rather than debug (our use of) PyCapnp, I just introduced a @dataclass for CoinbaseTemplateData, so we fully own (everything in) it.
I looked into the diff and CI failure described here and found this change pretty confusing because I couldn’t see how the dataclass would make any difference. Introducing a dataclass would seem to just move around the invalid references without changing them.
The actual fix seems to be in the parse_and_deserialize_coinbase function which adds bytes casts. The bytes casts do nothing on my system which uses pycapnp-2.0.0, but in the CI job which uses pycapnp-2.2.1, it looks like casts have the effect of converting memoryview objects into bytes objects (https://github.com/capnproto/pycapnp/pull/380), fixing the crash that happens in CI.
I feel like the way the test works isn’t very clear from looking at the code, and that the dataclass adds extra complexity. So a change like following could make sense:
0--- a/test/functional/interface_ipc.py
1+++ b/test/functional/interface_ipc.py
2@@ -4,7 +4,6 @@
3 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
4 """Test the IPC (multiprocess) interface."""
5 import asyncio
6-from dataclasses import dataclass
7 from io import BytesIO
8 from pathlib import Path
9 import shutil
10@@ -23,16 +22,6 @@ from test_framework.test_framework import (BitcoinTestFramework, assert_equal)
11 from test_framework.wallet import MiniWallet
12
13
14-@dataclass
15-class CoinbaseTemplateData:
16- version: int
17- inputSequence: int
18- scriptSigPrefix: bytes
19- witness: Optional[bytes]
20- valueRemaining: int
21- outputs: list[bytes]
22- lockTime: int
23-
24 # Test may be skipped and not have capnp installed
25 try:
26 import capnp # type: ignore[import] # noqa: F401
27@@ -106,21 +95,6 @@ class IPCInterfaceTest(BitcoinTestFramework):
28 tx.deserialize(coinbase_data)
29 return tx
30
31- async def parse_and_deserialize_coinbase(self, block_template, ctx) -> CoinbaseTemplateData:
32- template_capnp = (await block_template.result.getCoinbase(ctx)).result
33- witness: Optional[bytes] = None
34- if template_capnp._has("witness"):
35- witness = bytes(template_capnp.witness)
36- return CoinbaseTemplateData(
37- version=int(template_capnp.version),
38- inputSequence=int(template_capnp.inputSequence),
39- scriptSigPrefix=bytes(template_capnp.scriptSigPrefix),
40- witness=witness,
41- valueRemaining=int(template_capnp.valueRemaining),
42- outputs=[bytes(output) for output in template_capnp.outputs],
43- lockTime=int(template_capnp.lockTime),
44- )
45-
46 def run_echo_test(self):
47 self.log.info("Running echo test")
48 async def async_routine():
49@@ -137,21 +111,25 @@ class IPCInterfaceTest(BitcoinTestFramework):
50
51 async def build_coinbase_test(self, template, ctx, miniwallet):
52 self.log.debug("Build coinbase transaction using getCoinbase()")
53- coinbase_template = await self.parse_and_deserialize_coinbase(template, ctx)
54+ # Note: the coinbase_template struct will be garbage-collected when this
55+ # method returns, so it is important to copy any Data fields from it
56+ # which need to be accessed later using the bytes() cast. Starting with
57+ # pycapnp v2.2.0, Data fields have type `memoryview` and are ephemeral.
58+ coinbase_template = (await template.result.getCoinbase(ctx)).result
59 coinbase = CTransaction()
60 coinbase.version = coinbase_template.version
61 coinbase.vin = [CTxIn()]
62 coinbase.vin[0].prevout = NULL_OUTPOINT
63 coinbase.vin[0].nSequence = coinbase_template.inputSequence
64 # Typically a mining pool appends its name and an extraNonce
65- coinbase.vin[0].scriptSig = coinbase_template.scriptSigPrefix
66+ coinbase.vin[0].scriptSig = bytes(coinbase_template.scriptSigPrefix) # copy data with bytes()
67
68 # We currently always provide a coinbase witness, even for empty
69 # blocks, but this may change, so always check:
70- has_witness = coinbase_template.witness is not None
71+ has_witness = coinbase_template._has("witness")
72 if has_witness:
73 coinbase.wit.vtxinwit = [CTxInWitness()]
74- coinbase.wit.vtxinwit[0].scriptWitness.stack = [coinbase_template.witness]
75+ coinbase.wit.vtxinwit[0].scriptWitness.stack = [bytes(coinbase_template.witness)] # copy data with bytes()
76
77 # First output is our payout
78 coinbase.vout = [CTxOut()]
Alternately, if the current approach is preferred, it would at least be good to add some comments explaining the bytes() calls.
