refactor(verification): remove BlockVerifier inheritance (#833)

Author: glevco

hathor/cli/mining.py

@@ -138,11 +138,12 @@ def execute(args: Namespace) -> None:
 
         try:
             from hathor.daa import DifficultyAdjustmentAlgorithm
-            from hathor.verification.block_verifier import BlockVerifier
+            from hathor.verification.verification_service import VerificationService, VertexVerifiers
             settings = get_settings()
             daa = DifficultyAdjustmentAlgorithm(settings=settings)
-            verifier = BlockVerifier(settings=settings, daa=daa)
-            verifier.verify_without_storage(block)
+            verifiers = VertexVerifiers.create_defaults(settings=settings, daa=daa)
+            verification_service = VerificationService(verifiers=verifiers)
+            verification_service.verify_without_storage(block)
         except HathorError:
             print('[{}] ERROR: Block has not been pushed because it is not valid.'.format(datetime.datetime.now()))
         else:

hathor/simulator/simulator.py

@@ -259,11 +259,7 @@ def _build_vertex_verifiers(
     """
     return VertexVerifiers(
         block=SimulatorBlockVerifier(settings=settings, daa=daa, feature_service=feature_service),
-        merge_mined_block=SimulatorMergeMinedBlockVerifier(
-            settings=settings,
-            daa=daa,
-            feature_service=feature_service
-        ),
+        merge_mined_block=SimulatorMergeMinedBlockVerifier(),
         tx=SimulatorTransactionVerifier(settings=settings, daa=daa),
         token_creation_tx=SimulatorTokenCreationTransactionVerifier(settings=settings, daa=daa),
     )

hathor/verification/block_verifier.py

@@ -15,7 +15,6 @@
 from hathor.conf.settings import HathorSettings
 from hathor.daa import DifficultyAdjustmentAlgorithm
 from hathor.feature_activation.feature_service import BlockIsMissingSignal, BlockIsSignaling, FeatureService
-from hathor.profiler import get_cpu_profiler
 from hathor.transaction import BaseTransaction, Block
 from hathor.transaction.exceptions import (
     BlockMustSignalError,
@@ -28,8 +27,6 @@
 )
 from hathor.verification.vertex_verifier import VertexVerifier
 
-cpu = get_cpu_profiler()
-
 
 class BlockVerifier(VertexVerifier):
     __slots__ = ('_feature_service', )
@@ -44,45 +41,6 @@ def __init__(
         super().__init__(settings=settings, daa=daa)
         self._feature_service = feature_service
 
-    def verify_basic(self, block: Block, *, skip_block_weight_verification: bool = False) -> None:
-        """Partially run validations, the ones that need parents/inputs are skipped."""
-        if not skip_block_weight_verification:
-            self.verify_weight(block)
-        self.verify_reward(block)
-
-    @cpu.profiler(key=lambda _, block: 'block-verify!{}'.format(block.hash.hex()))
-    def verify(self, block: Block) -> None:
-        """
-            (1) confirms at least two pending transactions and references last block
-            (2) solves the pow with the correct weight (done in HathorManager)
-            (3) creates the correct amount of tokens in the output (done in HathorManager)
-            (4) all parents must exist and have timestamp smaller than ours
-            (5) data field must contain at most BLOCK_DATA_MAX_SIZE bytes
-            (6) whether this block must signal feature support
-        """
-        # TODO Should we validate a limit of outputs?
-        if block.is_genesis:
-            # TODO do genesis validation
-            return
-
-        self.verify_without_storage(block)
-
-        # (1) and (4)
-        self.verify_parents(block)
-
-        self.verify_height(block)
-
-        self.verify_mandatory_signaling(block)
-
-    def verify_without_storage(self, block: Block) -> None:
-        """ Run all verifications that do not need a storage.
-        """
-        self.verify_pow(block)
-        self.verify_no_inputs(block)
-        self.verify_outputs(block)
-        self.verify_data(block)
-        self.verify_sigops_output(block)
-
     def verify_height(self, block: Block) -> None:
         """Validate that the block height is enough to confirm all transactions being confirmed."""
         meta = block.get_metadata()

hathor/verification/merge_mined_block_verifier.py

@@ -12,18 +12,12 @@
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
 
-from hathor.transaction import Block, MergeMinedBlock
-from hathor.verification.block_verifier import BlockVerifier
+from hathor.transaction import MergeMinedBlock
 
 
-class MergeMinedBlockVerifier(BlockVerifier):
+class MergeMinedBlockVerifier:
     __slots__ = ()
 
-    def verify_without_storage(self, block: Block) -> None:
-        assert isinstance(block, MergeMinedBlock)
-        self.verify_aux_pow(block)
-        super().verify_without_storage(block)
-
     def verify_aux_pow(self, block: MergeMinedBlock) -> None:
         """ Verify auxiliary proof-of-work (for merged mining).
         """

hathor/verification/verification_service.py

@@ -19,6 +19,7 @@
 from hathor.conf.settings import HathorSettings
 from hathor.daa import DifficultyAdjustmentAlgorithm
 from hathor.feature_activation.feature_service import FeatureService
+from hathor.profiler import get_cpu_profiler
 from hathor.transaction import BaseTransaction, Block, MergeMinedBlock, Transaction, TxVersion
 from hathor.transaction.token_creation_tx import TokenCreationTransaction
 from hathor.transaction.validation_state import ValidationState
@@ -27,6 +28,8 @@
 from hathor.verification.token_creation_transaction_verifier import TokenCreationTransactionVerifier
 from hathor.verification.transaction_verifier import TransactionVerifier
 
+cpu = get_cpu_profiler()
+
 
 class VertexVerifiers(NamedTuple):
     """A group of verifier instances, one for each vertex type."""
@@ -49,7 +52,7 @@ def create_defaults(
         """
         return VertexVerifiers(
             block=BlockVerifier(settings=settings, daa=daa, feature_service=feature_service),
-            merge_mined_block=MergeMinedBlockVerifier(settings=settings, daa=daa, feature_service=feature_service),
+            merge_mined_block=MergeMinedBlockVerifier(),
             tx=TransactionVerifier(settings=settings, daa=daa),
             token_creation_tx=TokenCreationTransactionVerifier(settings=settings, daa=daa),
         )
@@ -116,16 +119,10 @@ def verify_basic(self, vertex: BaseTransaction, *, skip_block_weight_verificatio
         match vertex.version:
             case TxVersion.REGULAR_BLOCK:
                 assert type(vertex) is Block
-                self.verifiers.block.verify_basic(
-                    vertex,
-                    skip_block_weight_verification=skip_block_weight_verification
-                )
+                self._verify_basic_block(vertex, skip_weight_verification=skip_block_weight_verification)
             case TxVersion.MERGE_MINED_BLOCK:
                 assert type(vertex) is MergeMinedBlock
-                self.verifiers.merge_mined_block.verify_basic(
-                    vertex,
-                    skip_block_weight_verification=skip_block_weight_verification
-                )
+                self._verify_basic_merge_mined_block(vertex, skip_weight_verification=skip_block_weight_verification)
             case TxVersion.REGULAR_TRANSACTION:
                 assert type(vertex) is Transaction
                 self.verifiers.tx.verify_basic(vertex)
@@ -135,6 +132,15 @@ def verify_basic(self, vertex: BaseTransaction, *, skip_block_weight_verificatio
             case _:
                 assert_never(vertex.version)
 
+    def _verify_basic_block(self, block: Block, *, skip_weight_verification: bool) -> None:
+        """Partially run validations, the ones that need parents/inputs are skipped."""
+        if not skip_weight_verification:
+            self.verifiers.block.verify_weight(block)
+        self.verifiers.block.verify_reward(block)
+
+    def _verify_basic_merge_mined_block(self, block: MergeMinedBlock, *, skip_weight_verification: bool) -> None:
+        self._verify_basic_block(block, skip_weight_verification=skip_weight_verification)
+
     def verify(self, vertex: BaseTransaction, *, reject_locked_reward: bool = True) -> None:
         """Run all verifications. Raises on error.
 
@@ -143,10 +149,10 @@ def verify(self, vertex: BaseTransaction, *, reject_locked_reward: bool = True)
         match vertex.version:
             case TxVersion.REGULAR_BLOCK:
                 assert type(vertex) is Block
-                self.verifiers.block.verify(vertex)
+                self._verify_block(vertex)
             case TxVersion.MERGE_MINED_BLOCK:
                 assert type(vertex) is MergeMinedBlock
-                self.verifiers.merge_mined_block.verify(vertex)
+                self._verify_merge_mined_block(vertex)
             case TxVersion.REGULAR_TRANSACTION:
                 assert type(vertex) is Transaction
                 self.verifiers.tx.verify(vertex, reject_locked_reward=reject_locked_reward)
@@ -156,15 +162,42 @@ def verify(self, vertex: BaseTransaction, *, reject_locked_reward: bool = True)
             case _:
                 assert_never(vertex.version)
 
+    @cpu.profiler(key=lambda _, block: 'block-verify!{}'.format(block.hash.hex()))
+    def _verify_block(self, block: Block) -> None:
+        """
+            (1) confirms at least two pending transactions and references last block
+            (2) solves the pow with the correct weight (done in HathorManager)
+            (3) creates the correct amount of tokens in the output (done in HathorManager)
+            (4) all parents must exist and have timestamp smaller than ours
+            (5) data field must contain at most BLOCK_DATA_MAX_SIZE bytes
+            (6) whether this block must signal feature support
+        """
+        # TODO Should we validate a limit of outputs?
+        if block.is_genesis:
+            # TODO do genesis validation
+            return
+
+        self.verify_without_storage(block)
+
+        # (1) and (4)
+        self.verifiers.block.verify_parents(block)
+
+        self.verifiers.block.verify_height(block)
+
+        self.verifiers.block.verify_mandatory_signaling(block)
+
+    def _verify_merge_mined_block(self, block: MergeMinedBlock) -> None:
+        self._verify_block(block)
+
     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 type(vertex) is Block
-                self.verifiers.block.verify_without_storage(vertex)
+                self._verify_without_storage_block(vertex)
             case TxVersion.MERGE_MINED_BLOCK:
                 assert type(vertex) is MergeMinedBlock
-                self.verifiers.merge_mined_block.verify_without_storage(vertex)
+                self._verify_without_storage_merge_mined_block(vertex)
             case TxVersion.REGULAR_TRANSACTION:
                 assert type(vertex) is Transaction
                 self.verifiers.tx.verify_without_storage(vertex)
@@ -173,3 +206,16 @@ def verify_without_storage(self, vertex: BaseTransaction) -> None:
                 self.verifiers.token_creation_tx.verify_without_storage(vertex)
             case _:
                 assert_never(vertex.version)
+
+    def _verify_without_storage_block(self, block: Block) -> None:
+        """ Run all verifications that do not need a storage.
+        """
+        self.verifiers.block.verify_pow(block)
+        self.verifiers.block.verify_no_inputs(block)
+        self.verifiers.block.verify_outputs(block)
+        self.verifiers.block.verify_data(block)
+        self.verifiers.block.verify_sigops_output(block)
+
+    def _verify_without_storage_merge_mined_block(self, block: MergeMinedBlock) -> None:
+        self.verifiers.merge_mined_block.verify_aux_pow(block)
+        self._verify_without_storage_block(block)