test: remove txid caching in CTransaction class

Rather than txids (represented by the fields `.sha256` and `.hash`)
being stateful, simply compute them on-the-fly. This ensures that
the correct values are always returned and takes the burden of
rehashing from test writers, making the code shorter overall.
In a first step, the fields are kept at the same name with @Property
functions as drop-in replacements, for a minimal diff. In later commits,
the names are changed to be more descriptive and indicating the return
type of the txid.

Author: theStack

test/functional/feature_block.py

@@ -1053,10 +1053,9 @@ def run_test(self):
         self.log.info("Reject a block containing a transaction spending from a non-existent input")
         self.move_tip(69)
         self.next_block(70, spend=out[21])
-        bogus_tx = CTransaction()
-        bogus_tx.sha256 = uint256_from_str(b"23c70ed7c0506e9178fc1a987f40a33946d4ad4c962b5ae3a52546da53af0c5c")
+        bogus_txid_int = uint256_from_str(b"23c70ed7c0506e9178fc1a987f40a33946d4ad4c962b5ae3a52546da53af0c5c")
         tx = CTransaction()
-        tx.vin.append(CTxIn(COutPoint(bogus_tx.sha256, 0), b"", SEQUENCE_FINAL))
+        tx.vin.append(CTxIn(COutPoint(bogus_txid_int, 0), b"", SEQUENCE_FINAL))
         tx.vout.append(CTxOut(1, b""))
         b70 = self.update_block(70, [tx])
         self.send_blocks([b70], success=False, reject_reason='bad-txns-inputs-missingorspent', reconnect=True)

test/functional/test_framework/messages.py

@@ -585,8 +585,7 @@ def is_null(self):
 
 
 class CTransaction:
-    __slots__ = ("hash", "nLockTime", "version", "sha256", "vin", "vout",
-                 "wit")
+    __slots__ = ("nLockTime", "version", "vin", "vout", "wit")
 
     def __init__(self, tx=None):
         if tx is None:
@@ -595,15 +594,11 @@ def __init__(self, tx=None):
             self.vout = []
             self.wit = CTxWitness()
             self.nLockTime = 0
-            self.sha256 = None
-            self.hash = None
         else:
             self.version = tx.version
             self.vin = copy.deepcopy(tx.vin)
             self.vout = copy.deepcopy(tx.vout)
             self.nLockTime = tx.nLockTime
-            self.sha256 = tx.sha256
-            self.hash = tx.hash
             self.wit = copy.deepcopy(tx.wit)
 
     def deserialize(self, f):
@@ -625,8 +620,6 @@ def deserialize(self, f):
         else:
             self.wit = CTxWitness()
         self.nLockTime = int.from_bytes(f.read(4), "little")
-        self.sha256 = None
-        self.hash = None
 
     def serialize_without_witness(self):
         r = b""
@@ -667,23 +660,27 @@ def serialize(self):
     def getwtxid(self):
         return hash256(self.serialize())[::-1].hex()
 
+    @property
+    def hash(self):
+        """Return txid (transaction hash without witness) as hex string."""
+        return hash256(self.serialize_without_witness())[::-1].hex()
+
+    @property
+    def sha256(self):
+        """Return txid (transaction hash without witness) as integer."""
+        return uint256_from_str(hash256(self.serialize_without_witness()))
+
     # Recalculate the txid (transaction hash without witness)
+    # TODO: get rid of this method, replace call-sites by .hash access
     def rehash(self):
-        self.sha256 = None
-        self.calc_sha256()
         return self.hash
 
-    # We will only cache the serialization without witness in
-    # self.sha256 and self.hash -- those are expected to be the txid.
+    # TODO: get rid of this method, replace call-sites by .wtxid_int access (not introduced yet)
     def calc_sha256(self, with_witness=False):
         if with_witness:
             # Don't cache the result, just return it
             return uint256_from_str(hash256(self.serialize_with_witness()))
 
-        if self.sha256 is None:
-            self.sha256 = uint256_from_str(hash256(self.serialize_without_witness()))
-        self.hash = hash256(self.serialize_without_witness())[::-1].hex()
-
     def is_valid(self):
         self.calc_sha256()
         for tout in self.vout: