diff --git a/hathor/feature_activation/feature_service.py b/hathor/feature_activation/feature_service.py index 6566d56bb..e07cc7d2d 100644 --- a/hathor/feature_activation/feature_service.py +++ b/hathor/feature_activation/feature_service.py @@ -35,30 +35,38 @@ def is_feature_active(self, *, block: Block, feature: Feature) -> bool: return state == FeatureState.ACTIVE def get_state(self, *, block: Block, feature: Feature) -> FeatureState: - """Returns the state of a feature at a certain block.""" + """Returns the state of a feature at a certain block. Uses block metadata to cache states.""" # per definition, the genesis block is in the DEFINED state for all features if block.is_genesis: return FeatureState.DEFINED + if state := block.get_feature_state(feature=feature): + return state + # All blocks within the same evaluation interval have the same state, that is, the state is only defined for - # the block in each interval boundary. Therefore, we get the state of the previous boundary. + # the block in each interval boundary. Therefore, we get the state of the previous boundary block or calculate + # a new state if this block is a boundary block. height = block.get_height() offset_to_boundary = height % self._feature_settings.evaluation_interval offset_to_previous_boundary = offset_to_boundary or self._feature_settings.evaluation_interval previous_boundary_height = height - offset_to_previous_boundary previous_boundary_block = self._get_ancestor_at_height(block=block, height=previous_boundary_height) - previous_state = self.get_state(block=previous_boundary_block, feature=feature) + previous_boundary_state = self.get_state(block=previous_boundary_block, feature=feature) if offset_to_boundary != 0: - return previous_state + return previous_boundary_state - return self._calculate_new_state( + new_state = self._calculate_new_state( boundary_block=block, feature=feature, - previous_state=previous_state + previous_state=previous_boundary_state ) + block.update_feature_state(feature=feature, state=new_state) + + return new_state + def _calculate_new_state( self, *, diff --git a/hathor/transaction/block.py b/hathor/transaction/block.py index 1ccae6818..51ffacc55 100644 --- a/hathor/transaction/block.py +++ b/hathor/transaction/block.py @@ -21,6 +21,8 @@ from hathor import daa from hathor.checkpoint import Checkpoint from hathor.conf import HathorSettings +from hathor.feature_activation.feature import Feature +from hathor.feature_activation.model.feature_state import FeatureState from hathor.profiler import get_cpu_profiler from hathor.transaction import BaseTransaction, TxOutput, TxVersion from hathor.transaction.exceptions import ( @@ -420,3 +422,20 @@ def _get_feature_activation_bitmask(cls) -> int: bitmask = (1 << settings.FEATURE_ACTIVATION.max_signal_bits) - 1 return bitmask + + def get_feature_state(self, *, feature: Feature) -> Optional[FeatureState]: + """Returns the state of a feature from metadata.""" + metadata = self.get_metadata() + feature_states = metadata.feature_states or {} + + return feature_states.get(feature) + + def update_feature_state(self, *, feature: Feature, state: FeatureState) -> None: + """Updates the state of a feature in metadata and persists it.""" + assert self.storage is not None + metadata = self.get_metadata() + feature_states = metadata.feature_states or {} + feature_states[feature] = state + metadata.feature_states = feature_states + + self.storage.save_transaction(self, only_metadata=True) diff --git a/hathor/transaction/transaction_metadata.py b/hathor/transaction/transaction_metadata.py index 8176a7734..cecf1c8f5 100644 --- a/hathor/transaction/transaction_metadata.py +++ b/hathor/transaction/transaction_metadata.py @@ -15,6 +15,8 @@ from collections import defaultdict from typing import TYPE_CHECKING, Any, Optional +from hathor.feature_activation.feature import Feature +from hathor.feature_activation.model.feature_state import FeatureState from hathor.transaction.validation_state import ValidationState from hathor.util import practically_equal @@ -49,6 +51,9 @@ class TransactionMetadata: # the previous boundary block up to this block, including it. LSB is on the left. feature_activation_bit_counts: Optional[list[int]] + # A dict of features in the feature activation process and their respective state. Must only be used by Blocks, + # is None otherwise. + feature_states: Optional[dict[Feature, FeatureState]] = None # It must be a weakref. _tx_ref: Optional['ReferenceType[BaseTransaction]'] @@ -181,9 +186,9 @@ def __eq__(self, other: Any) -> bool: """Override the default Equals behavior""" if not isinstance(other, TransactionMetadata): return False - for field in ['hash', 'conflict_with', 'voided_by', 'received_by', - 'children', 'accumulated_weight', 'twins', 'score', - 'first_block', 'validation', 'min_height', 'feature_activation_bit_counts']: + for field in ['hash', 'conflict_with', 'voided_by', 'received_by', 'children', + 'accumulated_weight', 'twins', 'score', 'first_block', 'validation', + 'min_height', 'feature_activation_bit_counts', 'feature_states']: if (getattr(self, field) or None) != (getattr(other, field) or None): return False @@ -219,6 +224,10 @@ def to_json(self) -> dict[str, Any]: data['height'] = self.height data['min_height'] = self.min_height data['feature_activation_bit_counts'] = self.feature_activation_bit_counts + + if self.feature_states is not None: + data['feature_states'] = {feature.value: state.value for feature, state in self.feature_states.items()} + if self.first_block is not None: data['first_block'] = self.first_block.hex() else: @@ -270,6 +279,13 @@ def create_from_json(cls, data: dict[str, Any]) -> 'TransactionMetadata': meta.min_height = data.get('min_height', 0) meta.feature_activation_bit_counts = data.get('feature_activation_bit_counts', []) + feature_states_raw = data.get('feature_states') + if feature_states_raw: + meta.feature_states = { + Feature(feature): FeatureState(feature_state) + for feature, feature_state in feature_states_raw.items() + } + first_block_raw = data.get('first_block', None) if first_block_raw: meta.first_block = bytes.fromhex(first_block_raw) diff --git a/tests/feature_activation/test_feature_service.py b/tests/feature_activation/test_feature_service.py index 976e35c6e..ec3d6a1cf 100644 --- a/tests/feature_activation/test_feature_service.py +++ b/tests/feature_activation/test_feature_service.py @@ -364,6 +364,36 @@ def test_get_state_from_active(block_mocks: list[Block], tx_storage: Transaction assert result == FeatureState.ACTIVE +@pytest.mark.parametrize('block_height', [12, 13, 14, 15]) +def test_caching_mechanism(block_mocks: list[Block], tx_storage: TransactionStorage, block_height: int) -> None: + feature_settings = FeatureSettings.construct( + evaluation_interval=4, + features={ + Feature.NOP_FEATURE_1: Criteria.construct( + bit=Mock(), + start_height=0, + timeout_height=4, + activate_on_timeout=True, + version=Mock() + ) + } + ) + service = FeatureService(feature_settings=feature_settings, tx_storage=tx_storage) + block = block_mocks[block_height] + calculate_new_state_mock = Mock(wraps=service._calculate_new_state) + + with patch.object(FeatureService, '_calculate_new_state', calculate_new_state_mock): + result1 = service.get_state(block=block, feature=Feature.NOP_FEATURE_1) + + assert result1 == FeatureState.ACTIVE + assert calculate_new_state_mock.call_count == 3 + + result2 = service.get_state(block=block, feature=Feature.NOP_FEATURE_1) + + assert result2 == FeatureState.ACTIVE + assert calculate_new_state_mock.call_count == 3 + + @pytest.mark.parametrize('block_height', [12, 13, 14, 15]) def test_is_feature_active(block_mocks: list[Block], tx_storage: TransactionStorage, block_height: int) -> None: feature_settings = FeatureSettings.construct( diff --git a/tests/feature_activation/test_feature_simulation.py b/tests/feature_activation/test_feature_simulation.py index b51a5c744..ce34e8463 100644 --- a/tests/feature_activation/test_feature_simulation.py +++ b/tests/feature_activation/test_feature_simulation.py @@ -15,15 +15,21 @@ from typing import Any from unittest.mock import Mock, patch -from hathor.feature_activation import feature_service +import pytest + +from hathor.builder import Builder +from hathor.feature_activation import feature_service as feature_service_module from hathor.feature_activation.feature import Feature +from hathor.feature_activation.feature_service import FeatureService from hathor.feature_activation.model.criteria import Criteria from hathor.feature_activation.resources.feature import FeatureResource from hathor.feature_activation.settings import Settings as FeatureSettings +from hathor.simulator.miner import AbstractMiner from hathor.simulator.trigger import StopAfterNMinedBlocks from tests import unittest from tests.resources.base_resource import StubSite from tests.simulation.base import SimulatorTestCase +from tests.utils import HAS_ROCKSDB _FEATURE_SETTINGS = FeatureSettings( evaluation_interval=4, @@ -41,46 +47,57 @@ ) -class BaseTestFeatureSimulation(SimulatorTestCase): - def setUp(self): - super().setUp() - artifacts = self.simulator.create_artifacts() - manager = artifacts.manager - manager.allow_mining_without_peers() - - self.feature_service = artifacts.feature_service - self.feature_service._feature_settings = _FEATURE_SETTINGS - feature_resource = FeatureResource( - feature_settings=_FEATURE_SETTINGS, - feature_service=self.feature_service, - tx_storage=manager.tx_storage - ) - self.web_client = StubSite(feature_resource) - - self.miner = self.simulator.create_miner(manager, hashpower=1e6) - self.miner.start() +class BaseFeatureSimulationTest(SimulatorTestCase): + builder: Builder - def _get_result_after(self, *, n_blocks: int) -> dict[str, Any]: - trigger = StopAfterNMinedBlocks(self.miner, quantity=n_blocks) - self.simulator.run(7200, trigger=trigger) + def _get_result_after(self, *, n_blocks: int, miner: AbstractMiner, web_client: StubSite) -> dict[str, Any]: + """Returns the feature activation api response after N blocks.""" + trigger = StopAfterNMinedBlocks(miner, quantity=n_blocks) + self.simulator.run(36000, trigger=trigger) - response = self.web_client.get('feature') + response = web_client.get('feature') result = response.result.json_value() del result['block_hash'] # we don't assert the block hash because it's not always the same return result - def test_feature(self): + @staticmethod + def _get_state_mock_block_height_calls(get_state_mock: Mock) -> list[int]: + """Returns the heights of blocks that get_state_mock was called with.""" + return [call.kwargs['block'].get_height() for call in get_state_mock.call_args_list] + + def test_feature(self) -> None: """ - Test that a feature goes through all possible states in the correct block heights, and also assert internal - method call counts to make sure we're executing it in the most performatic way. + Tests that a feature goes through all possible states in the correct block heights, and also assert internal + method call counts and args to make sure we're executing it in the most performatic way. """ - get_ancestor_iteratively_mock = Mock(wraps=feature_service._get_ancestor_iteratively) + artifacts = self.simulator.create_artifacts(self.builder) + manager = artifacts.manager + manager.allow_mining_without_peers() + + feature_service = artifacts.feature_service + feature_service._feature_settings = _FEATURE_SETTINGS + feature_resource = FeatureResource( + feature_settings=_FEATURE_SETTINGS, + feature_service=feature_service, + tx_storage=artifacts.tx_storage + ) + web_client = StubSite(feature_resource) + + miner = self.simulator.create_miner(manager, hashpower=1e6) + miner.start() - with patch.object(feature_service, '_get_ancestor_iteratively', get_ancestor_iteratively_mock): - # at the beginning, the feature is DEFINED - assert self._get_result_after(n_blocks=10) == dict( + get_state_mock = Mock(wraps=feature_service.get_state) + get_ancestor_iteratively_mock = Mock(wraps=feature_service_module._get_ancestor_iteratively) + + with ( + patch.object(FeatureService, 'get_state', get_state_mock), + patch.object(feature_service_module, '_get_ancestor_iteratively', get_ancestor_iteratively_mock) + ): + # at the beginning, the feature is DEFINED: + result = self._get_result_after(n_blocks=10, miner=miner, web_client=web_client) + assert result == dict( block_height=10, features=[ dict( @@ -96,11 +113,15 @@ def test_feature(self): ) ] ) - # no blocks are voided, so we only use the height index: + # so we query states all the way down to genesis: + assert self._get_state_mock_block_height_calls(get_state_mock) == [10, 8, 4, 0] + # no blocks are voided, so we only use the height index, and not get_ancestor_iteratively: assert get_ancestor_iteratively_mock.call_count == 0 + get_state_mock.reset_mock() - # at block 19, the feature is DEFINED, just before becoming STARTED - assert self._get_result_after(n_blocks=9) == dict( + # at block 19, the feature is DEFINED, just before becoming STARTED: + result = self._get_result_after(n_blocks=9, miner=miner, web_client=web_client) + assert result == dict( block_height=19, features=[ dict( @@ -116,10 +137,14 @@ def test_feature(self): ) ] ) + # so we query states from block 19 to 8, as it's cached: + assert self._get_state_mock_block_height_calls(get_state_mock) == [19, 16, 12, 8] assert get_ancestor_iteratively_mock.call_count == 0 + get_state_mock.reset_mock() - # at block 20, the feature becomes STARTED - assert self._get_result_after(n_blocks=1) == dict( + # at block 20, the feature becomes STARTED: + result = self._get_result_after(n_blocks=1, miner=miner, web_client=web_client) + assert result == dict( block_height=20, features=[ dict( @@ -135,10 +160,13 @@ def test_feature(self): ) ] ) + assert self._get_state_mock_block_height_calls(get_state_mock) == [20, 16] assert get_ancestor_iteratively_mock.call_count == 0 + get_state_mock.reset_mock() - # at block 39, the feature is STARTED, just before becoming ACTIVE - assert self._get_result_after(n_blocks=39) == dict( + # at block 39, the feature is STARTED, just before becoming ACTIVE: + result = self._get_result_after(n_blocks=39, miner=miner, web_client=web_client) + assert result == dict( block_height=59, features=[ dict( @@ -154,10 +182,144 @@ def test_feature(self): ) ] ) + assert ( + self._get_state_mock_block_height_calls(get_state_mock) == [59, 56, 52, 48, 44, 40, 36, 32, 28, 24, 20] + ) + assert get_ancestor_iteratively_mock.call_count == 0 + get_state_mock.reset_mock() + + # at block 60, the feature becomes ACTIVE, forever: + result = self._get_result_after(n_blocks=1, miner=miner, web_client=web_client) + assert result == dict( + block_height=60, + features=[ + dict( + name='NOP_FEATURE_1', + state='ACTIVE', + acceptance=None, + threshold=0.75, + start_height=20, + timeout_height=60, + minimum_activation_height=0, + activate_on_timeout=True, + version='0.0.0' + ) + ] + ) + assert self._get_state_mock_block_height_calls(get_state_mock) == [60, 56] + assert get_ancestor_iteratively_mock.call_count == 0 + get_state_mock.reset_mock() + + +class BaseMemoryStorageFeatureSimulationTest(BaseFeatureSimulationTest): + def setUp(self): + super().setUp() + self.builder = self.simulator.get_default_builder() + + +@pytest.mark.skipif(not HAS_ROCKSDB, reason='requires python-rocksdb') +class BaseRocksDBStorageFeatureSimulationTest(BaseFeatureSimulationTest): + def setUp(self): + super().setUp() + import tempfile + + self.rocksdb_directory = tempfile.mkdtemp() + self.tmpdirs.append(self.rocksdb_directory) + + self.builder = self.simulator.get_default_builder() \ + .use_rocksdb(path=self.rocksdb_directory) \ + .disable_full_verification() + + def test_feature_from_existing_storage(self) -> None: + """ + Tests that feature states are correctly retrieved from an existing storage, so no recalculation is required. + """ + artifacts1 = self.simulator.create_artifacts(self.builder) + manager1 = artifacts1.manager + manager1.allow_mining_without_peers() + + feature_service = artifacts1.feature_service + feature_service._feature_settings = _FEATURE_SETTINGS + feature_resource = FeatureResource( + feature_settings=_FEATURE_SETTINGS, + feature_service=feature_service, + tx_storage=artifacts1.tx_storage + ) + web_client = StubSite(feature_resource) + + miner = self.simulator.create_miner(manager1, hashpower=1e6) + miner.start() + + get_state_mock = Mock(wraps=feature_service.get_state) + get_ancestor_iteratively_mock = Mock(wraps=feature_service_module._get_ancestor_iteratively) + + with ( + patch.object(FeatureService, 'get_state', get_state_mock), + patch.object(feature_service_module, '_get_ancestor_iteratively', get_ancestor_iteratively_mock) + ): + assert artifacts1.tx_storage.get_vertices_count() == 3 # genesis vertices in the storage + + result = self._get_result_after(n_blocks=60, miner=miner, web_client=web_client) + assert result == dict( + block_height=60, + features=[ + dict( + name='NOP_FEATURE_1', + state='ACTIVE', + acceptance=None, + threshold=0.75, + start_height=20, + timeout_height=60, + minimum_activation_height=0, + activate_on_timeout=True, + version='0.0.0' + ) + ] + ) + # feature states have to be calculated for all blocks in evaluation interval boundaries, as this is the + # first run: + assert self._get_state_mock_block_height_calls(get_state_mock) == list(range(60, -4, -4)) + # no blocks are voided, so we only use the height index: assert get_ancestor_iteratively_mock.call_count == 0 + assert artifacts1.tx_storage.get_vertices_count() == 63 + get_state_mock.reset_mock() + + miner.stop() + manager1.stop() + artifacts1.rocksdb_storage.close() + + builder = self.simulator.get_default_builder() \ + .use_rocksdb(path=self.rocksdb_directory) \ + .disable_full_verification() + artifacts2 = self.simulator.create_artifacts(builder) + + # new feature_service is created with the same storage generated above + feature_service = artifacts2.feature_service + feature_service._feature_settings = _FEATURE_SETTINGS + feature_resource = FeatureResource( + feature_settings=_FEATURE_SETTINGS, + feature_service=feature_service, + tx_storage=artifacts2.tx_storage + ) + web_client = StubSite(feature_resource) + + get_state_mock = Mock(wraps=feature_service.get_state) + get_ancestor_iteratively_mock = Mock(wraps=feature_service_module._get_ancestor_iteratively) + + with ( + patch.object(FeatureService, 'get_state', get_state_mock), + patch.object(feature_service_module, '_get_ancestor_iteratively', get_ancestor_iteratively_mock) + ): + # the new storage starts populated + assert artifacts2.tx_storage.get_vertices_count() == 63 + self.simulator.run(3600) - # at block 60, the feature becomes ACTIVE, forever - assert self._get_result_after(n_blocks=1) == dict( + response = web_client.get('feature') + result = response.result.json_value() + + del result['block_hash'] # we don't assert the block hash because it's not always the same + + assert result == dict( block_height=60, features=[ dict( @@ -173,17 +335,37 @@ def test_feature(self): ) ] ) + # features states are not queried for previous blocks, as they have it cached: + assert self._get_state_mock_block_height_calls(get_state_mock) == [60] assert get_ancestor_iteratively_mock.call_count == 0 + assert artifacts2.tx_storage.get_vertices_count() == 63 + get_state_mock.reset_mock() + + +class SyncV1MemoryStorageFeatureSimulationTest(unittest.SyncV1Params, BaseMemoryStorageFeatureSimulationTest): + __test__ = True + + +class SyncV2MemoryStorageFeatureSimulationTest(unittest.SyncV2Params, BaseMemoryStorageFeatureSimulationTest): + __test__ = True + + +# sync-bridge should behave like sync-v2 +class SyncBridgeMemoryStorageFeatureSimulationTest(unittest.SyncBridgeParams, BaseMemoryStorageFeatureSimulationTest): + __test__ = True -class SyncV1BaseTestFeatureSimulation(unittest.SyncV1Params, BaseTestFeatureSimulation): +class SyncV1RocksDBStorageFeatureSimulationTest(unittest.SyncV1Params, BaseRocksDBStorageFeatureSimulationTest): __test__ = True -class SyncV2BaseTestFeatureSimulation(unittest.SyncV2Params, BaseTestFeatureSimulation): +class SyncV2RocksDBStorageFeatureSimulationTest(unittest.SyncV2Params, BaseRocksDBStorageFeatureSimulationTest): __test__ = True # sync-bridge should behave like sync-v2 -class SyncBridgeBaseTestFeatureSimulation(unittest.SyncBridgeParams, SyncV2BaseTestFeatureSimulation): +class SyncBridgeRocksDBStorageFeatureSimulationTest( + unittest.SyncBridgeParams, + BaseRocksDBStorageFeatureSimulationTest +): __test__ = True