Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(verification): externalize verification dependencies [part 1/2] #859

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion hathor/builder/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -531,8 +531,13 @@ def _get_or_create_bit_signaling_service(self) -> BitSignalingService:
def _get_or_create_verification_service(self) -> VerificationService:
if self._verification_service is None:
verifiers = self._get_or_create_vertex_verifiers()
daa = self._get_or_create_daa()
feature_service = self._get_or_create_feature_service()
self._verification_service = VerificationService(verifiers=verifiers, feature_service=feature_service)
self._verification_service = VerificationService(
verifiers=verifiers,
daa=daa,
feature_service=feature_service
)

return self._verification_service

Expand Down
6 changes: 5 additions & 1 deletion hathor/builder/cli_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,11 @@ def create_manager(self, reactor: Reactor) -> HathorManager:
daa = DifficultyAdjustmentAlgorithm(settings=settings, test_mode=test_mode)

vertex_verifiers = VertexVerifiers.create_defaults(settings=settings, daa=daa)
verification_service = VerificationService(verifiers=vertex_verifiers, feature_service=self.feature_service)
verification_service = VerificationService(
verifiers=vertex_verifiers,
daa=daa,
feature_service=self.feature_service
)

cpu_mining_service = CpuMiningService()

Expand Down
2 changes: 1 addition & 1 deletion hathor/cli/mining.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ def execute(args: Namespace) -> None:
settings = get_global_settings()
daa = DifficultyAdjustmentAlgorithm(settings=settings)
verifiers = VertexVerifiers.create_defaults(settings=settings, daa=daa)
verification_service = VerificationService(verifiers=verifiers)
verification_service = VerificationService(verifiers=verifiers, daa=daa)
verification_service.verify_without_storage(block)
except HathorError:
print('[{}] ERROR: Block has not been pushed because it is not valid.'.format(datetime.datetime.now()))
Expand Down
8 changes: 5 additions & 3 deletions hathor/transaction/resources/create_tx.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
from hathor.transaction import Transaction, TxInput, TxOutput
from hathor.transaction.scripts import create_output_script
from hathor.util import api_catch_exceptions, json_dumpb, json_loadb
from hathor.verification.verification_dependencies import TransactionDependencies


def from_raw_output(raw_output: dict, tokens: list[bytes]) -> TxOutput:
Expand Down Expand Up @@ -109,15 +110,16 @@ def _verify_unsigned_skip_pow(self, tx: Transaction) -> None:
""" Same as .verify but skipping pow and signature verification."""
assert type(tx) is Transaction
verifiers = self.manager.verification_service.verifiers
deps = TransactionDependencies.create(tx)
verifiers.tx.verify_number_of_inputs(tx)
verifiers.vertex.verify_number_of_outputs(tx)
verifiers.vertex.verify_outputs(tx)
verifiers.tx.verify_output_token_indexes(tx)
verifiers.vertex.verify_sigops_output(tx)
verifiers.tx.verify_sigops_input(tx)
verifiers.tx.verify_sigops_input(tx, deps)
# need to run verify_inputs first to check if all inputs exist
verifiers.tx.verify_inputs(tx, skip_script=True)
verifiers.vertex.verify_parents(tx)
verifiers.tx.verify_inputs(tx, deps, skip_script=True)
verifiers.vertex.verify_parents(tx, deps)
verifiers.tx.verify_sum(tx.get_complete_token_info())


Expand Down
21 changes: 14 additions & 7 deletions hathor/transaction/storage/simple_memory_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from hathor.transaction.storage import TransactionStorage
from hathor.transaction.storage.exceptions import TransactionDoesNotExist
from hathor.types import VertexId
from hathor.util import not_none


class SimpleMemoryStorage:
Expand Down Expand Up @@ -47,6 +48,10 @@ def get_transaction(self, tx_id: VertexId) -> Transaction:
assert isinstance(tx, Transaction)
return tx

def get_vertex(self, vertex_id: VertexId) -> BaseTransaction:
"""Return a vertex from the storage, raise if it's not found."""
return self._get_vertex(self._vertices, vertex_id)

@staticmethod
def _get_vertex(storage: dict[VertexId, BaseTransaction], vertex_id: VertexId) -> BaseTransaction:
"""Return a vertex from a storage, throw if it's not found."""
Expand All @@ -71,13 +76,19 @@ def add_vertices_from_storage(self, storage: TransactionStorage, ids: list[Verte

def add_vertex_from_storage(self, storage: TransactionStorage, vertex_id: VertexId) -> None:
"""
Add a vertex to this storage. It automatically fetches data from the provided TransactionStorage and a list
of ids.
Add a vertex to this storage. It automatically fetches data from the provided TransactionStorage and vertex_id.
"""
vertex = storage.get_transaction(vertex_id)

self.add_vertex(vertex)

def add_vertex(self, vertex: BaseTransaction) -> None:
"""Add a vertex to this storage."""
vertex_id = not_none(vertex.hash)

if vertex_id in self._vertices:
return

vertex = storage.get_transaction(vertex_id)
clone = vertex.clone(include_metadata=True, include_storage=False)

if isinstance(vertex, Block):
Expand All @@ -90,10 +101,6 @@ def add_vertex_from_storage(self, storage: TransactionStorage, vertex_id: Vertex

raise NotImplementedError

def get_vertex(self, vertex_id: VertexId) -> BaseTransaction:
# TODO: Currently unused, will be implemented in a next PR.
raise NotImplementedError

def get_best_block_tips(self) -> list[VertexId]:
# TODO: Currently unused, will be implemented in a next PR.
raise NotImplementedError
23 changes: 9 additions & 14 deletions hathor/verification/block_verifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

from hathor.conf.settings import HathorSettings
from hathor.daa import DifficultyAdjustmentAlgorithm
from hathor.feature_activation.feature_service import BlockIsMissingSignal, BlockIsSignaling, BlockSignalingState
from hathor.feature_activation.feature_service import BlockIsMissingSignal, BlockIsSignaling
from hathor.transaction import Block
from hathor.transaction.exceptions import (
BlockMustSignalError,
Expand All @@ -27,8 +27,7 @@
TransactionDataError,
WeightError,
)
from hathor.transaction.storage.simple_memory_storage import SimpleMemoryStorage
from hathor.util import not_none
from hathor.verification.verification_dependencies import BasicBlockDependencies, BlockDependencies


class BlockVerifier:
Expand All @@ -51,20 +50,16 @@ def verify_height(self, block: Block) -> None:
if meta.height < meta.min_height:
raise RewardLocked(f'Block needs {meta.min_height} height but has {meta.height}')

def verify_weight(self, block: Block) -> None:
def verify_weight(self, block: Block, block_deps: BasicBlockDependencies) -> None:
"""Validate minimum block difficulty."""
memory_storage = SimpleMemoryStorage()
dependencies = self._daa.get_block_dependencies(block)
memory_storage.add_vertices_from_storage(not_none(block.storage), dependencies)

min_block_weight = self._daa.calculate_block_difficulty(block, memory_storage)
min_block_weight = self._daa.calculate_block_difficulty(block, block_deps.storage)
if block.weight < min_block_weight - self._settings.WEIGHT_TOL:
raise WeightError(f'Invalid new block {block.hash_hex}: weight ({block.weight}) is '
f'smaller than the minimum weight ({min_block_weight})')

def verify_reward(self, block: Block) -> None:
def verify_reward(self, block: Block, block_deps: BasicBlockDependencies) -> None:
"""Validate reward amount."""
parent_block = block.get_block_parent()
parent_block = block_deps.storage.get_parent_block(block)
tokens_issued_per_block = self._daa.get_tokens_issued_per_block(parent_block.get_height() + 1)
if block.sum_outputs != tokens_issued_per_block:
raise InvalidBlockReward(
Expand All @@ -86,14 +81,14 @@ def verify_data(self, block: Block) -> None:
if len(block.data) > self._settings.BLOCK_DATA_MAX_SIZE:
raise TransactionDataError('block data has {} bytes'.format(len(block.data)))

def verify_mandatory_signaling(self, signaling_state: BlockSignalingState) -> None:
def verify_mandatory_signaling(self, block_deps: BlockDependencies) -> None:
"""Verify whether this block is missing mandatory signaling for any feature."""
match signaling_state:
match block_deps.signaling_state:
case BlockIsSignaling():
return
case BlockIsMissingSignal(feature):
raise BlockMustSignalError(
f"Block must signal support for feature '{feature.value}' during MUST_SIGNAL phase."
)
case _:
assert_never(signaling_state)
assert_never(block_deps.signaling_state)
6 changes: 3 additions & 3 deletions hathor/verification/merge_mined_block_verifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@

from hathor.conf.settings import HathorSettings
from hathor.feature_activation.feature import Feature
from hathor.feature_activation.model.feature_description import FeatureInfo
from hathor.transaction import MergeMinedBlock
from hathor.verification.verification_dependencies import BlockDependencies


class MergeMinedBlockVerifier:
Expand All @@ -24,13 +24,13 @@ class MergeMinedBlockVerifier:
def __init__(self, *, settings: HathorSettings) -> None:
self._settings = settings

def verify_aux_pow(self, block: MergeMinedBlock, feature_info: dict[Feature, FeatureInfo]) -> None:
def verify_aux_pow(self, block: MergeMinedBlock, block_deps: BlockDependencies) -> None:
""" Verify auxiliary proof-of-work (for merged mining).
"""
assert block.aux_pow is not None

max_merkle_path_length = self._settings.OLD_MAX_MERKLE_PATH_LENGTH
merkle_path_info = feature_info.get(Feature.INCREASE_MAX_MERKLE_PATH_LENGTH)
merkle_path_info = block_deps.feature_info.get(Feature.INCREASE_MAX_MERKLE_PATH_LENGTH)

if merkle_path_info and merkle_path_info.state.is_active():
max_merkle_path_length = self._settings.NEW_MAX_MERKLE_PATH_LENGTH
Expand Down
12 changes: 6 additions & 6 deletions hathor/verification/transaction_verifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
from hathor.transaction.transaction import TokenInfo
from hathor.transaction.util import get_deposit_amount, get_withdraw_amount
from hathor.types import TokenUid, VertexId
from hathor.verification.verification_dependencies import TransactionDependencies

cpu = get_cpu_profiler()

Expand All @@ -51,8 +52,6 @@ def __init__(self, *, settings: HathorSettings, daa: DifficultyAdjustmentAlgorit

def verify_parents_basic(self, tx: Transaction) -> None:
"""Verify number and non-duplicity of parents."""
assert tx.storage is not None

# check if parents are duplicated
parents_set = set(tx.parents)
if len(tx.parents) > len(parents_set):
Expand All @@ -72,15 +71,15 @@ def verify_weight(self, tx: Transaction) -> None:
raise WeightError(f'Invalid new tx {tx.hash_hex}: weight ({tx.weight}) is '
f'greater than the maximum allowed ({max_tx_weight})')

def verify_sigops_input(self, tx: Transaction) -> None:
def verify_sigops_input(self, tx: Transaction, tx_deps: TransactionDependencies) -> None:
""" Count sig operations on all inputs and verify that the total sum is below the limit
"""
from hathor.transaction.scripts import get_sigops_count
from hathor.transaction.storage.exceptions import TransactionDoesNotExist
n_txops = 0
for tx_input in tx.inputs:
try:
spent_tx = tx.get_spent_tx(tx_input)
spent_tx = tx_deps.storage.get_vertex(tx_input.tx_id)
except TransactionDoesNotExist:
raise InexistentInput('Input tx does not exist: {}'.format(tx_input.tx_id.hex()))
if tx_input.index >= len(spent_tx.outputs):
Expand All @@ -92,7 +91,7 @@ def verify_sigops_input(self, tx: Transaction) -> None:
raise TooManySigOps(
'TX[{}]: Max number of sigops for inputs exceeded ({})'.format(tx.hash_hex, n_txops))

def verify_inputs(self, tx: Transaction, *, skip_script: bool = False) -> None:
def verify_inputs(self, tx: Transaction, tx_deps: TransactionDependencies, *, skip_script: bool = False) -> None:
"""Verify inputs signatures and ownership and all inputs actually exist"""
from hathor.transaction.storage.exceptions import TransactionDoesNotExist

Expand All @@ -104,7 +103,8 @@ def verify_inputs(self, tx: Transaction, *, skip_script: bool = False) -> None:
))

try:
spent_tx = tx.get_spent_tx(input_tx)
spent_tx = tx_deps.storage.get_vertex(input_tx.tx_id)
assert spent_tx.hash is not None
if input_tx.index >= len(spent_tx.outputs):
raise InexistentInput('Output spent by this input does not exist: {} index {}'.format(
input_tx.tx_id.hex(), input_tx.index))
Expand Down
88 changes: 88 additions & 0 deletions hathor/verification/verification_dependencies.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
# Copyright 2023 Hathor Labs
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

from dataclasses import dataclass

from typing_extensions import Self

from hathor.daa import DifficultyAdjustmentAlgorithm
from hathor.feature_activation.feature import Feature
from hathor.feature_activation.feature_service import BlockSignalingState, FeatureService
from hathor.feature_activation.model.feature_description import FeatureInfo
from hathor.transaction import Block
from hathor.transaction.storage.simple_memory_storage import SimpleMemoryStorage
from hathor.transaction.transaction import Transaction


@dataclass(frozen=True, slots=True)
class VertexDependencies:
"""A dataclass of dependencies necessary for vertex verification."""
storage: SimpleMemoryStorage


@dataclass(frozen=True, slots=True)
class BasicBlockDependencies(VertexDependencies):
"""A dataclass of dependencies necessary for basic block verification."""

@classmethod
def create(cls, block: Block, daa: DifficultyAdjustmentAlgorithm, *, skip_weight_verification: bool) -> Self:
"""Create a basic block dependencies instance."""
assert block.storage is not None
simple_storage = SimpleMemoryStorage()
daa_deps = [] if skip_weight_verification else daa.get_block_dependencies(block)
deps = block.parents + daa_deps

simple_storage.add_vertices_from_storage(block.storage, deps)

return cls(simple_storage)


@dataclass(frozen=True, slots=True)
class BlockDependencies(VertexDependencies):
"""A dataclass of dependencies necessary for block verification."""
signaling_state: BlockSignalingState
feature_info: dict[Feature, FeatureInfo]

@classmethod
def create(cls, block: Block, feature_service: FeatureService) -> Self:
"""Create a block dependencies instance."""
assert block.storage is not None
signaling_state = feature_service.is_signaling_mandatory_features(block)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it part of the dependencies?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because since the Feature Activation process is dynamic, it is not straightforward to determine which blocks are necessary to calculate this state in advance (differently from the DAA, for example, where it's easy to determine the number of necessary parent blocks).

In order to determine the necessary blocks, we would have to run the algorithm anyway, so there's almost no difference in cost between determining all block dependencies in advance, or determining the state itself in advance.

I suggest we implement it like this for now, and then after the verification is moved to another process, we measure the cost of doing this calculation in main process, to decide if it's worth the refactor (we would need to change Feature Activation code in order to get just the required blocks).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed in our call, we'll keep this for now. After completing the Multiprocess Verification, we'll measure its performance and determine if it should be improved or not.

feature_info = feature_service.get_feature_info(block=block)
simple_storage = SimpleMemoryStorage()

simple_storage.add_vertices_from_storage(block.storage, block.parents)
simple_storage.add_vertex(block) # we add the block itself so its metadata can be used as a dependency.

return cls(
storage=simple_storage,
signaling_state=signaling_state,
feature_info=feature_info,
)


class TransactionDependencies(VertexDependencies):
"""A dataclass of dependencies necessary for transaction verification."""

@classmethod
def create(cls, tx: Transaction) -> Self:
"""Create a transaction dependencies instance."""
assert tx.storage is not None
simple_storage = SimpleMemoryStorage()
spent_txs = [tx_input.tx_id for tx_input in tx.inputs]
deps = tx.parents + spent_txs

simple_storage.add_vertices_from_storage(tx.storage, deps)

return cls(storage=simple_storage)
Loading
Loading