refactor(verification): organization and typing improvements

Author: glevco

hathor/manager.py

@@ -929,9 +929,10 @@ def push_tx(self, tx: Transaction, allow_non_standard_script: bool = False,
             raise NonStandardTxError('Transaction is non standard.')
 
         # Validate tx.
-        success, message = self.verification_service.validate_vertex_error(tx)
-        if not success:
-            raise InvalidNewTransaction(message)
+        try:
+            self.verification_service.verify(tx)
+        except TxValidationError as e:
+            raise InvalidNewTransaction(str(e))
 
         self.propagate_tx(tx, fails_silently=False)
 

hathor/transaction/base_transaction.py

@@ -139,7 +139,7 @@ def __init__(self,
                  nonce: int = 0,
                  timestamp: Optional[int] = None,
                  signal_bits: int = 0,
-                 version: int = TxVersion.REGULAR_BLOCK,
+                 version: TxVersion = TxVersion.REGULAR_BLOCK,
                  weight: float = 0,
                  inputs: Optional[list['TxInput']] = None,
                  outputs: Optional[list['TxOutput']] = None,

hathor/transaction/block.py

@@ -47,7 +47,7 @@ def __init__(self,
                  nonce: int = 0,
                  timestamp: Optional[int] = None,
                  signal_bits: int = 0,
-                 version: int = TxVersion.REGULAR_BLOCK,
+                 version: TxVersion = TxVersion.REGULAR_BLOCK,
                  weight: float = 0,
                  outputs: Optional[list[TxOutput]] = None,
                  parents: Optional[list[bytes]] = None,

hathor/transaction/merge_mined_block.py

@@ -28,7 +28,7 @@ def __init__(self,
                  nonce: int = 0,
                  timestamp: Optional[int] = None,
                  signal_bits: int = 0,
-                 version: int = TxVersion.MERGE_MINED_BLOCK,
+                 version: TxVersion = TxVersion.MERGE_MINED_BLOCK,
                  weight: float = 0,
                  outputs: Optional[list[TxOutput]] = None,
                  parents: Optional[list[bytes]] = None,

hathor/transaction/resources/create_tx.py

@@ -88,7 +88,7 @@ def render_POST(self, request):
             # conservative estimate of the input data size to estimate a valid weight
             tx_input.data = b'\0' * 107
         tx.weight = self.manager.daa.minimum_tx_weight(fake_signed_tx)
-        self.manager.verification_service.verifiers.tx.verify_unsigned_skip_pow(tx)
+        self._verify_unsigned_skip_pow(tx)
 
         if tx.is_double_spending():
             raise InvalidNewTransaction('At least one of your inputs has already been spent.')
@@ -104,6 +104,19 @@ def render_POST(self, request):
             'data': data,
         })
 
+    def _verify_unsigned_skip_pow(self, tx: Transaction) -> None:
+        """ Same as .verify but skipping pow and signature verification."""
+        assert type(tx) is Transaction
+        verifier = self.manager.verification_service.verifiers.tx
+        verifier.verify_number_of_inputs(tx)
+        verifier.verify_number_of_outputs(tx)
+        verifier.verify_outputs(tx)
+        verifier.verify_sigops_output(tx)
+        verifier.verify_sigops_input(tx)
+        verifier.verify_inputs(tx, skip_script=True)  # need to run verify_inputs first to check if all inputs exist
+        verifier.verify_parents(tx)
+        verifier.verify_sum(tx)
+
 
 CreateTxResource.openapi = {
     '/create_tx': {

hathor/transaction/token_creation_tx.py

@@ -36,7 +36,7 @@ def __init__(self,
                  nonce: int = 0,
                  timestamp: Optional[int] = None,
                  signal_bits: int = 0,
-                 version: int = TxVersion.TOKEN_CREATION_TRANSACTION,
+                 version: TxVersion = TxVersion.TOKEN_CREATION_TRANSACTION,
                  weight: float = 0,
                  inputs: Optional[list[TxInput]] = None,
                  outputs: Optional[list[TxOutput]] = None,

hathor/transaction/transaction.py

@@ -57,7 +57,7 @@ def __init__(self,
                  nonce: int = 0,
                  timestamp: Optional[int] = None,
                  signal_bits: int = 0,
-                 version: int = TxVersion.REGULAR_TRANSACTION,
+                 version: TxVersion = TxVersion.REGULAR_TRANSACTION,
                  weight: float = 0,
                  inputs: Optional[list[TxInput]] = None,
                  outputs: Optional[list[TxOutput]] = None,

hathor/verification/transaction_verifier.py

@@ -75,17 +75,6 @@ def verify(self, tx: Transaction, *, reject_locked_reward: bool = True) -> None:
         if reject_locked_reward:
             self.verify_reward_locked(tx)
 
-    def verify_unsigned_skip_pow(self, tx: Transaction) -> None:
-        """ Same as .verify but skipping pow and signature verification."""
-        self.verify_number_of_inputs(tx)
-        self.verify_number_of_outputs(tx)
-        self.verify_outputs(tx)
-        self.verify_sigops_output(tx)
-        self.verify_sigops_input(tx)
-        self.verify_inputs(tx, skip_script=True)  # need to run verify_inputs first to check if all inputs exist
-        self.verify_parents(tx)
-        self.verify_sum(tx)
-
     def verify_parents_basic(self, tx: Transaction) -> None:
         """Verify number and non-duplicity of parents."""
         assert tx.storage is not None

hathor/verification/verification_service.py

@@ -14,11 +14,12 @@
 
 from typing import NamedTuple
 
+from typing_extensions import assert_never
+
 from hathor.conf.settings import HathorSettings
 from hathor.daa import DifficultyAdjustmentAlgorithm
 from hathor.feature_activation.feature_service import FeatureService
 from hathor.transaction import BaseTransaction, Block, MergeMinedBlock, Transaction, TxVersion
-from hathor.transaction.exceptions import TxValidationError
 from hathor.transaction.token_creation_tx import TokenCreationTransaction
 from hathor.transaction.validation_state import ValidationState
 from hathor.verification.block_verifier import BlockVerifier
@@ -107,76 +108,64 @@ def verify_basic(self, vertex: BaseTransaction, *, skip_block_weight_verificatio
         """Basic verifications (the ones without access to dependencies: parents+inputs). Raises on error.
 
         Used by `self.validate_basic`. Should not modify the validation state."""
+        # We assert with type() instead of isinstance() because each subclass has a specific branch.
         match vertex.version:
             case TxVersion.REGULAR_BLOCK:
-                assert isinstance(vertex, Block)
+                assert type(vertex) is Block
                 self.verifiers.block.verify_basic(
                     vertex,
                     skip_block_weight_verification=skip_block_weight_verification
                 )
             case TxVersion.MERGE_MINED_BLOCK:
-                assert isinstance(vertex, MergeMinedBlock)
+                assert type(vertex) is MergeMinedBlock
                 self.verifiers.merge_mined_block.verify_basic(
                     vertex,
                     skip_block_weight_verification=skip_block_weight_verification
                 )
             case TxVersion.REGULAR_TRANSACTION:
-                assert isinstance(vertex, Transaction)
+                assert type(vertex) is Transaction
                 self.verifiers.tx.verify_basic(vertex)
             case TxVersion.TOKEN_CREATION_TRANSACTION:
-                assert isinstance(vertex, TokenCreationTransaction)
+                assert type(vertex) is TokenCreationTransaction
                 self.verifiers.token_creation_tx.verify_basic(vertex)
             case _:
-                raise NotImplementedError
+                assert_never(vertex.version)
 
     def verify(self, vertex: BaseTransaction, *, reject_locked_reward: bool = True) -> None:
         """Run all verifications. Raises on error.
 
         Used by `self.validate_full`. Should not modify the validation state."""
+        # We assert with type() instead of isinstance() because each subclass has a specific branch.
         match vertex.version:
             case TxVersion.REGULAR_BLOCK:
-                assert isinstance(vertex, Block)
+                assert type(vertex) is Block
                 self.verifiers.block.verify(vertex)
             case TxVersion.MERGE_MINED_BLOCK:
-                assert isinstance(vertex, MergeMinedBlock)
+                assert type(vertex) is MergeMinedBlock
                 self.verifiers.merge_mined_block.verify(vertex)
             case TxVersion.REGULAR_TRANSACTION:
-                assert isinstance(vertex, Transaction)
+                assert type(vertex) is Transaction
                 self.verifiers.tx.verify(vertex, reject_locked_reward=reject_locked_reward)
             case TxVersion.TOKEN_CREATION_TRANSACTION:
-                assert isinstance(vertex, TokenCreationTransaction)
+                assert type(vertex) is TokenCreationTransaction
                 self.verifiers.token_creation_tx.verify(vertex, reject_locked_reward=reject_locked_reward)
             case _:
-                raise NotImplementedError
+                assert_never(vertex.version)
 
     def verify_without_storage(self, vertex: BaseTransaction) -> None:
+        # We assert with type() instead of isinstance() because each subclass has a specific branch.
         match vertex.version:
             case TxVersion.REGULAR_BLOCK:
-                assert isinstance(vertex, Block)
+                assert type(vertex) is Block
                 self.verifiers.block.verify_without_storage(vertex)
             case TxVersion.MERGE_MINED_BLOCK:
-                assert isinstance(vertex, MergeMinedBlock)
+                assert type(vertex) is MergeMinedBlock
                 self.verifiers.merge_mined_block.verify_without_storage(vertex)
             case TxVersion.REGULAR_TRANSACTION:
-                assert isinstance(vertex, Transaction)
+                assert type(vertex) is Transaction
                 self.verifiers.tx.verify_without_storage(vertex)
             case TxVersion.TOKEN_CREATION_TRANSACTION:
-                assert isinstance(vertex, TokenCreationTransaction)
+                assert type(vertex) is TokenCreationTransaction
                 self.verifiers.token_creation_tx.verify_without_storage(vertex)
             case _:
-                raise NotImplementedError
-
-    def validate_vertex_error(self, vertex: BaseTransaction) -> tuple[bool, str]:
-        """ Verify if tx is valid and return success and possible error message
-
-            :return: Success if tx is valid and possible error message, if not
-            :rtype: tuple[bool, str]
-        """
-        success = True
-        message = ''
-        try:
-            self.verify(vertex)
-        except TxValidationError as e:
-            success = False
-            message = str(e)
-        return success, message
+                assert_never(vertex.version)

poetry.lock

@@ -78,6 +78,7 @@ structlog-sentry = {version = "^1.4.0", optional = true}
 hathorlib = "0.3.0"
 pydantic = "~1.10.13"
 pyyaml = "^6.0.1"
+typing-extensions = "~4.8.0"
 
 [tool.poetry.extras]
 sentry = ["sentry-sdk", "structlog-sentry"]