From 650d4a0af511075fd610985f090ce0a3df3f6b7c Mon Sep 17 00:00:00 2001 From: madlabman <10616301+madlabman@users.noreply.github.com> Date: Thu, 6 Apr 2023 21:05:57 +0300 Subject: [PATCH 01/14] tests(accounting): unit tests --- src/modules/accounting/accounting.py | 2 +- .../accounting/test_accounting_module.py | 485 ++++++++++++++++-- 2 files changed, 436 insertions(+), 51 deletions(-) diff --git a/src/modules/accounting/accounting.py b/src/modules/accounting/accounting.py index 95b1d4068..ce838c6b1 100644 --- a/src/modules/accounting/accounting.py +++ b/src/modules/accounting/accounting.py @@ -237,7 +237,7 @@ def simulate_cl_rebase(self, blockstamp: ReferenceBlockStamp) -> LidoReportRebas Simulate rebase excluding any execution rewards. This used to check worst scenarios in bunker service. """ - return self.simulate_rebase_after_report(blockstamp, Wei(0)) + return self.simulate_rebase_after_report(blockstamp, el_rewards=Wei(0)) def simulate_full_rebase(self, blockstamp: ReferenceBlockStamp) -> LidoReportRebase: el_rewards = self.w3.lido_contracts.get_el_vault_balance(blockstamp) diff --git a/tests/modules/accounting/test_accounting_module.py b/tests/modules/accounting/test_accounting_module.py index f6a99f6c7..b032c34de 100644 --- a/tests/modules/accounting/test_accounting_module.py +++ b/tests/modules/accounting/test_accounting_module.py @@ -1,37 +1,90 @@ +from dataclasses import asdict +from typing import Any, Iterable, cast from unittest.mock import Mock, patch import pytest +from web3.types import Wei from src import variables -from src.modules.accounting import accounting +from src.modules.accounting import accounting as accounting_module from src.modules.accounting.accounting import Accounting +from src.modules.accounting.accounting import logger as accounting_logger +from src.modules.accounting.typings import LidoReportRebase +from src.modules.submodules.oracle_module import ModuleExecuteDelay +from src.modules.submodules.typings import ChainConfig, FrameConfig from src.services.withdrawal import Withdrawal -from tests.factory.blockstamp import ReferenceBlockStampFactory +from src.typings import BlockStamp, ReferenceBlockStamp +from src.web3py.extensions.lido_validators import NodeOperatorId, StakingModule +from tests.factory.blockstamp import BlockStampFactory, ReferenceBlockStampFactory from tests.factory.configs import ChainConfigFactory, FrameConfigFactory from tests.factory.contract_responses import LidoReportRebaseFactory -from tests.factory.no_registry import StakingModuleFactory, LidoValidatorFactory +from tests.factory.no_registry import LidoValidatorFactory, StakingModuleFactory + + +@pytest.fixture(autouse=True) +def silence_logger() -> None: + accounting_logger.disabled = True @pytest.fixture -def accounting_module(web3, contracts): +def accounting(web3, contracts): yield Accounting(web3) +@pytest.fixture +def bs() -> BlockStamp: + return cast(BlockStamp, BlockStampFactory.build()) + + +@pytest.fixture +def ref_bs() -> ReferenceBlockStamp: + return cast(ReferenceBlockStamp, ReferenceBlockStampFactory.build()) + + +@pytest.fixture +def chain_config() -> ChainConfig: + return cast(ChainConfig, ChainConfigFactory.build()) + + +@pytest.fixture +def frame_config() -> FrameConfig: + return cast(FrameConfig, FrameConfigFactory.build()) + + @pytest.mark.unit -def test_get_updated_modules_stats(accounting_module): - staking_modules = [ +def test_accounting_execute_module(accounting: Accounting, bs: BlockStamp): + accounting.get_blockstamp_for_report = Mock(return_value=None) + assert ( + accounting.execute_module(last_finalized_blockstamp=bs) is ModuleExecuteDelay.NEXT_FINALIZED_EPOCH + ), "execute_module should wait for the next finalized epoch" + accounting.get_blockstamp_for_report.assert_called_once_with(bs) + + accounting.get_blockstamp_for_report = Mock(return_value=bs) + accounting.process_report = Mock(return_value=None) + accounting.process_extra_data = Mock(return_value=None) + assert ( + accounting.execute_module(last_finalized_blockstamp=bs) is ModuleExecuteDelay.NEXT_SLOT + ), "execute_module should wait for the next slot" + accounting.get_blockstamp_for_report.assert_called_once_with(bs) + accounting.process_report.assert_called_once_with(bs) + accounting.process_extra_data.assert_called_once_with(bs) + + +@pytest.mark.unit +def test_get_updated_modules_stats(accounting: Accounting): + staking_modules: list[StakingModule] = [ StakingModuleFactory.build(exited_validators_count=10), StakingModuleFactory.build(exited_validators_count=20), StakingModuleFactory.build(exited_validators_count=30), ] node_operators_stats = { - (staking_modules[0].id, 0): 10, - (staking_modules[1].id, 0): 25, - (staking_modules[2].id, 0): 30, + (staking_modules[0].id, NodeOperatorId(0)): 10, + (staking_modules[1].id, NodeOperatorId(0)): 25, + (staking_modules[2].id, NodeOperatorId(0)): 30, } - module_ids, exited_validators_count_list = accounting_module.get_updated_modules_stats( + module_ids, exited_validators_count_list = accounting.get_updated_modules_stats( staking_modules, node_operators_stats, ) @@ -42,12 +95,13 @@ def test_get_updated_modules_stats(accounting_module): @pytest.mark.unit -def test_get_consensus_lido_state(accounting_module, lido_validators): +@pytest.mark.usefixtures("lido_validators") +def test_get_consensus_lido_state(accounting: Accounting): bs = ReferenceBlockStampFactory.build() validators = LidoValidatorFactory.batch(10) - accounting_module.w3.lido_validators.get_lido_validators = Mock(return_value=validators) + accounting.w3.lido_validators.get_lido_validators = Mock(return_value=validators) - count, balance = accounting_module._get_consensus_lido_state(bs) + count, balance = accounting._get_consensus_lido_state(bs) assert count == 10 assert balance == sum((int(val.balance) for val in validators)) @@ -62,24 +116,23 @@ def test_get_consensus_lido_state(accounting_module, lido_validators): (18 * 10**18, 14 * 10**18, 1285714285714285714285714285), ], ) -def test_get_finalization_data(accounting_module, post_total_pooled_ether, post_total_shares, expected_share_rate): +def test_get_finalization_data(accounting: Accounting, post_total_pooled_ether, post_total_shares, expected_share_rate): lido_rebase = LidoReportRebaseFactory.build( post_total_pooled_ether=post_total_pooled_ether, post_total_shares=post_total_shares, ) - accounting_module.get_chain_config = Mock(return_value=ChainConfigFactory.build()) - accounting_module.get_frame_config = Mock( - return_value=FrameConfigFactory.build(initial_epoch=2, epochs_per_frame=1) - ) - accounting_module.simulate_full_rebase = Mock(return_value=lido_rebase) - accounting_module._is_bunker = Mock(return_value=False) + + accounting.get_chain_config = Mock(return_value=ChainConfigFactory.build()) + accounting.get_frame_config = Mock(return_value=FrameConfigFactory.build(initial_epoch=2, epochs_per_frame=1)) + accounting.simulate_full_rebase = Mock(return_value=lido_rebase) + accounting._is_bunker = Mock(return_value=False) bs = ReferenceBlockStampFactory.build() with patch.object(Withdrawal, '__init__', return_value=None), patch.object( Withdrawal, 'get_finalization_batches', return_value=[] ): - share_rate, batches = accounting_module._get_finalization_data(bs) + share_rate, batches = accounting._get_finalization_data(bs) assert batches == [] assert share_rate == expected_share_rate @@ -91,51 +144,383 @@ def test_get_finalization_data(accounting_module, post_total_pooled_ether, post_ @pytest.mark.unit -def test_get_slots_elapsed_from_initialize(accounting_module, contracts): - accounting_module.get_chain_config = Mock(return_value=ChainConfigFactory.build()) - accounting_module.get_frame_config = Mock( - return_value=FrameConfigFactory.build(initial_epoch=2, epochs_per_frame=1) - ) +# @pytest.mark.usefixtures("contracts") +def test_get_slots_elapsed_from_initialize(accounting: Accounting): + accounting.get_chain_config = Mock(return_value=ChainConfigFactory.build()) + accounting.get_frame_config = Mock(return_value=FrameConfigFactory.build(initial_epoch=2, epochs_per_frame=1)) - accounting_module.w3.lido_contracts.get_accounting_last_processing_ref_slot = Mock(return_value=None) + accounting.w3.lido_contracts.get_accounting_last_processing_ref_slot = Mock(return_value=None) bs = ReferenceBlockStampFactory.build(ref_slot=100) - slots_elapsed = accounting_module._get_slots_elapsed_from_last_report(bs) + slots_elapsed = accounting._get_slots_elapsed_from_last_report(bs) assert slots_elapsed == 100 - 32 * 2 @pytest.mark.unit -def test_get_slots_elapsed_from_last_report(accounting_module, contracts): - accounting_module.get_chain_config = Mock(return_value=ChainConfigFactory.build()) - accounting_module.get_frame_config = Mock( - return_value=FrameConfigFactory.build(initial_epoch=2, epochs_per_frame=1) - ) +# @pytest.mark.usefixtures("contracts") +def test_get_slots_elapsed_from_last_report(accounting: Accounting): + accounting.get_chain_config = Mock(return_value=ChainConfigFactory.build()) + accounting.get_frame_config = Mock(return_value=FrameConfigFactory.build(initial_epoch=2, epochs_per_frame=1)) - accounting_module.w3.lido_contracts.get_accounting_last_processing_ref_slot = Mock(return_value=70) + accounting.w3.lido_contracts.get_accounting_last_processing_ref_slot = Mock(return_value=70) bs = ReferenceBlockStampFactory.build(ref_slot=100) - slots_elapsed = accounting_module._get_slots_elapsed_from_last_report(bs) + slots_elapsed = accounting._get_slots_elapsed_from_last_report(bs) assert slots_elapsed == 100 - 70 -class TestAccountingSanityCheck: +class TestAccountingReportingAllowed: + def test_env_toggle(self, accounting: Accounting, monkeypatch: pytest.MonkeyPatch, ref_bs: ReferenceBlockStamp): + accounting._is_bunker = Mock(return_value=True) + with monkeypatch.context() as ctx: + ctx.setattr(accounting_module, 'ALLOW_REPORTING_IN_BUNKER_MODE', True) + assert accounting.is_reporting_allowed(ref_bs) + + def test_no_bunker_mode(self, accounting: Accounting, ref_bs): + accounting._is_bunker = Mock(return_value=False) + assert accounting.is_reporting_allowed(ref_bs) + + def test_bunker_mode_active(self, accounting: Accounting, ref_bs: ReferenceBlockStamp): + accounting._is_bunker = Mock(return_value=True) + assert accounting.is_reporting_allowed(ref_bs) is variables.ALLOW_REPORTING_IN_BUNKER_MODE + + +class TestAccountingProcessExtraData: @pytest.fixture - def bs(self): - yield ReferenceBlockStampFactory.build() + def submit_extra_data_mock(self, accounting: Accounting, monkeypatch: pytest.MonkeyPatch) -> Iterable[Mock]: + with monkeypatch.context() as m: + mock = Mock() + m.setattr(accounting, '_submit_extra_data', mock) + yield mock - def test_env_toggle(self, accounting_module, monkeypatch, bs, caplog): - accounting_module._is_bunker = Mock(return_value=True) - with monkeypatch.context() as ctx: - ctx.setattr(accounting, 'ALLOW_REPORTING_IN_BUNKER_MODE', True) - assert accounting_module.is_reporting_allowed(bs) - assert "Bunker mode is active" in caplog.text + @pytest.fixture + def _no_sleep_before_report(self, accounting: Accounting): + accounting.get_chain_config = Mock(return_value=Mock(seconds_per_slot=0)) + accounting._get_slot_delay_before_data_submit = Mock(return_value=0) + + @pytest.mark.unit + @pytest.mark.usefixtures('_no_sleep_before_report') + def test_no_sumbit_if_can_submit_is_false( + self, + accounting: Accounting, + submit_extra_data_mock: Mock, + ref_bs: ReferenceBlockStamp, + bs: BlockStamp, + ): + accounting._get_latest_blockstamp = Mock(return_value=bs) + accounting.can_submit_extra_data = Mock(return_value=False) + + accounting.process_extra_data(ref_bs) + + accounting.can_submit_extra_data.assert_called_once_with(bs) + submit_extra_data_mock.assert_not_called() + + @pytest.mark.unit + @pytest.mark.usefixtures('_no_sleep_before_report') + def test_sumbit_if_can_submit_is_true( + self, + accounting: Accounting, + submit_extra_data_mock: Mock, + ref_bs: ReferenceBlockStamp, + bs: BlockStamp, + ): + accounting._get_latest_blockstamp = Mock(return_value=bs) + accounting.can_submit_extra_data = Mock(return_value=True) + + accounting.process_extra_data(ref_bs) + + accounting.can_submit_extra_data.assert_called_once_with(bs) + submit_extra_data_mock.assert_called_once_with(ref_bs) + + +class TestAccountingSubmitExtraData: + def test_submit_extra_data_non_empty( + self, + accounting: Accounting, + ref_bs: ReferenceBlockStamp, + chain_config: ChainConfig, + ): + extra_data = bytes(32) + + accounting.get_chain_config = Mock(return_value=chain_config) + accounting.lido_validator_state_service.get_extra_data = Mock(return_value=Mock(extra_data=extra_data)) + accounting.report_contract.functions.submitReportExtraDataList = Mock() # type: ignore + accounting.w3.transaction = Mock() + + accounting._submit_extra_data(ref_bs) + + accounting.report_contract.functions.submitReportExtraDataList.assert_called_once_with(extra_data) + accounting.lido_validator_state_service.get_extra_data.assert_called_once_with(ref_bs, chain_config) + accounting.get_chain_config.assert_called_once_with(ref_bs) + + @pytest.mark.unit + @pytest.mark.parametrize( + ("extra_data",), + [ + (None,), + (bytes(0),), + ([],), + (b'',), + ('',), + (False,), + ], + ) + def test_submit_extra_data_empty( + self, + accounting: Accounting, + ref_bs: ReferenceBlockStamp, + chain_config: ChainConfig, + extra_data: Any, + ): + accounting.get_chain_config = Mock(return_value=chain_config) + accounting.lido_validator_state_service.get_extra_data = Mock(return_value=Mock(extra_data=extra_data)) + accounting.report_contract.functions.submitReportExtraDataList = Mock() # type: ignore + accounting.report_contract.functions.submitReportExtraDataEmpty = Mock() # type: ignore + accounting.w3.transaction = Mock() + + accounting._submit_extra_data(ref_bs) - def test_no_bunker_mode(self, accounting_module, bs): - accounting_module._is_bunker = Mock(return_value=False) - assert accounting_module.is_reporting_allowed(bs) + accounting.report_contract.functions.submitReportExtraDataEmpty.assert_called_once() + accounting.report_contract.functions.submitReportExtraDataList.assert_not_called() + accounting.lido_validator_state_service.get_extra_data.assert_called_once_with(ref_bs, chain_config) + accounting.get_chain_config.assert_called_once_with(ref_bs) - def test_bunker_mode_active(self, accounting_module, bs): - accounting_module._is_bunker = Mock(return_value=True) - assert accounting_module.is_reporting_allowed(bs) is variables.ALLOW_REPORTING_IN_BUNKER_MODE + +@pytest.mark.unit +@pytest.mark.parametrize( + ("main_data_submitted", "extra_data_submitted", "expected"), + [ + (False, False, False), + (False, True, False), + (True, False, True), + (True, True, False), + ], +) +def test_can_sumbit_extra_data( + accounting: Accounting, + extra_data_submitted: bool, + main_data_submitted: bool, + expected: bool, + bs: BlockStamp, +): + accounting._get_processing_state = Mock( + return_value=Mock( + extra_data_submitted=extra_data_submitted, + main_data_submitted=main_data_submitted, + ) + ) + + out = accounting.can_submit_extra_data(bs) + + assert out == expected, "can_submit_extra_data returned unexpected value" + accounting._get_processing_state.assert_called_once_with(bs) + + +@pytest.mark.unit +@pytest.mark.parametrize( + ("main_data_submitted", "can_submit_extra_data", "expected"), + [ + (False, False, True), + (False, True, True), + (True, False, False), + (True, True, True), + ], +) +def test_is_contract_reportable( + accounting: Accounting, + main_data_submitted: bool, + can_submit_extra_data: bool, + expected: bool, + bs: BlockStamp, +): + accounting.is_main_data_submitted = Mock(return_value=main_data_submitted) + accounting.can_submit_extra_data = Mock(return_value=can_submit_extra_data) + + out = accounting.is_contract_reportable(bs) + + assert out == expected, "is_contract_reportable returned unexpected value" + + +@pytest.mark.unit +def test_is_main_data_submitted( + accounting: Accounting, + bs: BlockStamp, +): + accounting._get_processing_state = Mock(return_value=Mock(main_data_submitted=False)) + assert accounting.is_main_data_submitted(bs) is False, "is_main_data_submitted returned unexpected value" + accounting._get_processing_state.assert_called_once_with(bs) + + accounting._get_processing_state.reset_mock() + + accounting._get_processing_state = Mock(return_value=Mock(main_data_submitted=True)) + assert accounting.is_main_data_submitted(bs) is True, "is_main_data_submitted returned unexpected value" + accounting._get_processing_state.assert_called_once_with(bs) + + +@pytest.mark.unit +def test_build_report( + accounting: Accounting, + ref_bs: ReferenceBlockStamp, +): + REPORT = object() + + accounting._calculate_report = Mock(return_value=Mock(as_tuple=Mock(return_value=REPORT))) + + report = accounting.build_report(ref_bs) + + assert report is REPORT, "build_report returned unexpected value" + accounting._calculate_report.assert_called_once_with(ref_bs) + + # @lru_cache + accounting._calculate_report.reset_mock() + accounting.build_report(ref_bs) + accounting._calculate_report.assert_not_called() + + +@pytest.mark.unit +def test_get_shares_to_burn( + accounting: Accounting, + bs: BlockStamp, + monkeypatch: pytest.MonkeyPatch, +): + call_mock = accounting.w3.lido_contracts.burner.functions.getSharesRequestedToBurn = Mock() # type: ignore + + with monkeypatch.context() as m: + shares_data = Mock(cover_shares=42, non_cover_shares=17) + m.setattr(accounting_module, 'named_tuple_to_dataclass', Mock(return_value=shares_data)) + + out = accounting.get_shares_to_burn(bs) + + assert ( + out == shares_data.cover_shares + shares_data.non_cover_shares + ), "get_shares_to_burn returned unexpected value" + call_mock.assert_called_once() + + # @lru_cache + call_mock.reset_mock() + accounting.get_shares_to_burn(bs) + call_mock.assert_not_called() + + +@pytest.mark.unit +def test_simulate_cl_rebase(accounting: Accounting, ref_bs: ReferenceBlockStamp): + RESULT = object() + accounting.simulate_rebase_after_report = Mock(return_value=RESULT) + + out = accounting.simulate_cl_rebase(ref_bs) + + assert out is RESULT, "simulate_cl_rebase returned unexpected value" + accounting.simulate_rebase_after_report.assert_called_once_with(ref_bs, el_rewards=0) + + +@pytest.mark.unit +def test_simulate_full_rebase(accounting: Accounting, ref_bs: ReferenceBlockStamp): + RESULT = object() + accounting.simulate_rebase_after_report = Mock(return_value=RESULT) + accounting.w3.lido_contracts.get_el_vault_balance = Mock(return_value=42) + + out = accounting.simulate_full_rebase(ref_bs) + + assert out is RESULT, "simulate_full_rebase returned unexpected value" + accounting.simulate_rebase_after_report.assert_called_once_with(ref_bs, el_rewards=42) + + +@pytest.mark.unit +def test_simulate_rebase_after_report( + accounting: Accounting, + ref_bs: ReferenceBlockStamp, + chain_config: ChainConfig, +): + # NOTE: we don't test the actual rebase calculation here, just the logic of the method + + accounting.get_chain_config = Mock(return_value=chain_config) + accounting.w3.lido_contracts.get_withdrawal_balance = Mock(return_value=17) + accounting.get_shares_to_burn = Mock(return_value=13) + + accounting._get_consensus_lido_state = Mock(return_value=(0, 0)) + accounting._get_slots_elapsed_from_last_report = Mock(return_value=42) + + simulation_tx = Mock( + call=Mock( + return_value=asdict( + LidoReportRebaseFactory.build(), + ).values(), + ) + ) + accounting.w3.lido_contracts.lido.functions.handleOracleReport = Mock(return_value=simulation_tx) # type: ignore + + out = accounting.simulate_rebase_after_report(ref_bs, Wei(0)) + assert isinstance(out, LidoReportRebase), "simulate_rebase_after_report returned unexpected value" + + +@pytest.mark.unit +@pytest.mark.usefixtures('lido_validators') +def test_get_newly_exited_validators_by_modules(accounting: Accounting, ref_bs: ReferenceBlockStamp): + accounting.w3.lido_validators.get_staking_modules = Mock(return_value=[Mock(), Mock()]) + accounting.lido_validator_state_service.get_exited_lido_validators = Mock(return_value=[]) + + RESULT = object() + accounting.get_updated_modules_stats = Mock(return_value=RESULT) + + out = accounting._get_newly_exited_validators_by_modules(ref_bs) + + assert out is RESULT + accounting.w3.lido_validators.get_staking_modules.assert_called_once_with(ref_bs) + accounting.lido_validator_state_service.get_exited_lido_validators.assert_called_once_with(ref_bs) + + +@pytest.mark.unit +def test_get_processing_state( + accounting: Accounting, + bs: BlockStamp, + monkeypatch: pytest.MonkeyPatch, +): + processing_state = Mock() + RESULT = object() + + call_mock = accounting.report_contract.functions.getProcessingState = Mock(return_value=processing_state) # type: ignore + + with monkeypatch.context() as m: + m.setattr(accounting_module, 'named_tuple_to_dataclass', Mock(return_value=RESULT)) + + out = accounting._get_processing_state(bs) + + assert out is RESULT, "_get_processing_state returned unexpected value" + call_mock.assert_called_once() + + # @lru_cache + call_mock.reset_mock() + accounting._get_processing_state(bs) + call_mock.assert_not_called() + + +@pytest.mark.unit +def test_is_bunker( + accounting: Accounting, + ref_bs: ReferenceBlockStamp, + chain_config: ChainConfig, + frame_config: FrameConfig, +): + CL_REBASE = object() + BUNKER = object() + + accounting.get_frame_config = Mock(return_value=frame_config) + accounting.get_chain_config = Mock(return_value=chain_config) + accounting.simulate_cl_rebase = Mock(return_value=CL_REBASE) + accounting.bunker_service.is_bunker_mode = Mock(return_value=BUNKER) + + out = accounting._is_bunker(ref_bs) + assert out is BUNKER, "_is_bunker returned unexpected value" + + args = accounting.bunker_service.is_bunker_mode.call_args[0] + assert ref_bs in args, "is_bunker_mode called with unexpected blockstamp" + assert frame_config in args, "is_bunker_mode called with unexpected frame_config" + assert chain_config in args, "is_bunker_mode called with unexpected chain_config" + assert CL_REBASE in args, "is_bunker_mode called with unexpected cl_rebase_report" + + # @lru_cache + accounting.bunker_service.is_bunker_mode.reset_mock() + accounting._is_bunker(ref_bs) + accounting.bunker_service.is_bunker_mode.assert_not_called() From df57731ae05607a3216acbeb80327fe2ce2bc77b Mon Sep 17 00:00:00 2001 From: rkolpakov Date: Fri, 7 Apr 2023 17:47:24 +0300 Subject: [PATCH 02/14] fix: split consensus layer and keys api http timeouts --- README.md | 5 +++-- src/providers/http_provider.py | 12 +++++++++--- src/variables.py | 3 ++- src/web3py/extensions/consensus.py | 3 ++- src/web3py/extensions/keys_api.py | 3 ++- tests/providers.py | 4 ++-- tests/providers_clients/test_consensus_client.py | 2 +- tests/providers_clients/test_http_provider.py | 8 ++++---- tests/providers_clients/test_keys_api_client.py | 2 +- 9 files changed, 26 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 6d4c57e44..dc4e6330a 100644 --- a/README.md +++ b/README.md @@ -65,7 +65,7 @@ docker build -t lidofinance/oracle . ## Env variables | Name | Description | Required | Example value | -|----------------------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------|-------------------------| +| -------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -------- | ----------------------- | | `EXECUTION_CLIENT_URI` | URI of the Execution Layer client | True | `http://localhost:8545` | | `CONSENSUS_CLIENT_URI` | URI of the Consensus Layer client | True | `http://localhost:5052` | | `KEYS_API_URI` | URI of the Keys API | True | `http://localhost:8080` | @@ -78,7 +78,8 @@ docker build -t lidofinance/oracle . | `SUBMIT_DATA_DELAY_IN_SLOTS` | The difference in slots between submit data transactions from Oracles. It is used to prevent simultaneous sending of transactions and, as a result, transactions revert. | False | `6` | | `HTTP_REQUEST_RETRY_COUNT` | Total number of retries to fetch data from endpoint | False | `5` | | `HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS` | The delay http provider sleeps if API is stuck | False | `12` | -| `HTTP_REQUEST_TIMEOUT` | Timeout for HTTP requests | False | `300` | +| `HTTP_REQUEST_TIMEOUT_CONSENSUS` | Timeout for HTTP consensus layer requests | False | `300` | +| `HTTP_REQUEST_TIMEOUT_KEYS_API` | Timeout for HTTP keys api requests | False | `300` | | `PRIORITY_FEE_PERCENTILE` | Priority fee percentile from prev block that would be used to send tx | False | `3` | | `MIN_PRIORITY_FEE` | Min priority fee that would be used to send tx | False | `50000000` | | `MAX_PRIORITY_FEE` | Max priority fee that would be used to send tx | False | `100000000000` | diff --git a/src/providers/http_provider.py b/src/providers/http_provider.py index 786fd5849..34e7ad567 100644 --- a/src/providers/http_provider.py +++ b/src/providers/http_provider.py @@ -10,7 +10,7 @@ from requests.exceptions import ConnectionError as RequestsConnectionError from urllib3 import Retry -from src.variables import HTTP_REQUEST_RETRY_COUNT, HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS, HTTP_REQUEST_TIMEOUT +from src.variables import HTTP_REQUEST_RETRY_COUNT, HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS logger = logging.getLogger(__name__) @@ -34,10 +34,14 @@ class HTTPProvider(ABC): Base HTTP Provider with metrics and retry strategy integrated inside. """ PROMETHEUS_HISTOGRAM: Histogram + HTTP_REQUEST_TIMEOUT: int - def __init__(self, hosts: list[str]): + def __init__(self, hosts: list[str], http_request_timeout: int): if not hosts: raise NoHostsProvided(f"No hosts provided for {self.__class__.__name__}") + + if not http_request_timeout: + raise NoHostsProvided(f"No timeout provided for {self.__class__.__name__}") self.hosts = hosts @@ -52,6 +56,8 @@ def __init__(self, hosts: list[str]): self.session.mount("https://", adapter) self.session.mount("http://", adapter) + self.HTTP_REQUEST_TIMEOUT = http_request_timeout + @staticmethod def _urljoin(host, url): if not host.endswith('/'): @@ -113,7 +119,7 @@ def _get_without_fallbacks( response = self.session.get( self._urljoin(host, complete_endpoint if path_params else endpoint), params=query_params, - timeout=HTTP_REQUEST_TIMEOUT, + timeout=self.HTTP_REQUEST_TIMEOUT, ) except RequestsConnectionError as error: logger.debug({'msg': str(error)}) diff --git a/src/variables.py b/src/variables.py index a9690d62a..1b2da44bc 100644 --- a/src/variables.py +++ b/src/variables.py @@ -33,7 +33,8 @@ CYCLE_SLEEP_IN_SECONDS = int(os.getenv('CYCLE_SLEEP_IN_SECONDS', 12)) HTTP_REQUEST_RETRY_COUNT = int(os.getenv('HTTP_REQUEST_RETRY_COUNT', 5)) HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS = int(os.getenv('HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS', 5)) -HTTP_REQUEST_TIMEOUT = int(os.getenv('HTTP_REQUEST_TIMEOUT', 5 * 60)) +HTTP_REQUEST_TIMEOUT_KEYS_API = int(os.getenv('HTTP_REQUEST_TIMEOUT', 5 * 60)) +HTTP_REQUEST_TIMEOUT_CONSENSUS = int(os.getenv('HTTP_REQUEST_TIMEOUT', 5 * 60)) # - Metrics - PROMETHEUS_PORT = int(os.getenv('PROMETHEUS_PORT', 9000)) diff --git a/src/web3py/extensions/consensus.py b/src/web3py/extensions/consensus.py index d53ea2aa8..16291aa40 100644 --- a/src/web3py/extensions/consensus.py +++ b/src/web3py/extensions/consensus.py @@ -2,11 +2,12 @@ from web3.module import Module from src.providers.consensus.client import ConsensusClient +from src.variables import HTTP_REQUEST_TIMEOUT_CONSENSUS class ConsensusClientModule(ConsensusClient, Module): def __init__(self, hosts: list[str], w3: Web3): self.w3 = w3 - super(ConsensusClient, self).__init__(hosts) + super(ConsensusClient, self).__init__(hosts, HTTP_REQUEST_TIMEOUT_CONSENSUS) super(Module, self).__init__() diff --git a/src/web3py/extensions/keys_api.py b/src/web3py/extensions/keys_api.py index 1cc22ba63..467ef8f50 100644 --- a/src/web3py/extensions/keys_api.py +++ b/src/web3py/extensions/keys_api.py @@ -2,11 +2,12 @@ from web3.module import Module from src.providers.keys.client import KeysAPIClient +from src.variables import HTTP_REQUEST_TIMEOUT_KEYS_API class KeysAPIClientModule(KeysAPIClient, Module): def __init__(self, hosts: list[str], w3: Web3): self.w3 = w3 - super(KeysAPIClient, self).__init__(hosts) + super(KeysAPIClient, self).__init__(hosts, HTTP_REQUEST_TIMEOUT_KEYS_API) super(Module, self).__init__() diff --git a/tests/providers.py b/tests/providers.py index a7bcb9411..ffa8fc65b 100644 --- a/tests/providers.py +++ b/tests/providers.py @@ -106,7 +106,7 @@ def use_mock(self, mock_path: Path): class ResponseFromFileHTTPProvider(HTTPProvider, Module, FromFile): def __init__(self, mock_path: Path, w3: Web3): self.w3 = w3 - HTTPProvider.__init__(self, hosts=[""]) + HTTPProvider.__init__(self, hosts=[""], http_request_timeout=5 * 60) Module.__init__(self, w3) FromFile.__init__(self, mock_path) @@ -128,7 +128,7 @@ class UpdateResponsesHTTPProvider(HTTPProvider, Module, UpdateResponses): def __init__(self, mock_path: Path, host: str, w3: Web3): self.w3 = w3 - super().__init__([host]) + super().__init__([host], http_request_timeout=5 * 60) super(Module, self).__init__() self.responses = [] self.from_file = ResponseFromFileHTTPProvider(mock_path, w3) diff --git a/tests/providers_clients/test_consensus_client.py b/tests/providers_clients/test_consensus_client.py index da01dc824..4f2320a4b 100644 --- a/tests/providers_clients/test_consensus_client.py +++ b/tests/providers_clients/test_consensus_client.py @@ -9,7 +9,7 @@ @pytest.fixture def consensus_client(): - return ConsensusClient(CONSENSUS_CLIENT_URI) + return ConsensusClient(CONSENSUS_CLIENT_URI, 5 * 60) @pytest.mark.integration diff --git a/tests/providers_clients/test_http_provider.py b/tests/providers_clients/test_http_provider.py index 6624458e0..878271406 100644 --- a/tests/providers_clients/test_http_provider.py +++ b/tests/providers_clients/test_http_provider.py @@ -19,13 +19,13 @@ def test_urljoin(): def test_all_fallbacks_ok(): - provider = HTTPProvider(['http://localhost:1', 'http://localhost:2']) + provider = HTTPProvider(['http://localhost:1', 'http://localhost:2'], 5 * 60) provider._get_without_fallbacks = lambda host, endpoint, path_params, query_params: (host, endpoint) assert provider._get('test') == ('http://localhost:1', 'test') def test_all_fallbacks_bad(): - provider = HTTPProvider(['http://localhost:1', 'http://localhost:2']) + provider = HTTPProvider(['http://localhost:1', 'http://localhost:2'], 5 * 60) with pytest.raises(Exception): provider._get('test') @@ -36,7 +36,7 @@ def _simple_get(host, endpoint, *_): raise Exception('Bad host') # pylint: disable=broad-exception-raised return host, endpoint - provider = HTTPProvider(['http://localhost:1', 'http://localhost:2']) + provider = HTTPProvider(['http://localhost:1', 'http://localhost:2'], 5 * 60) provider._get_without_fallbacks = _simple_get assert provider._get('test') == ('http://localhost:2', 'test') @@ -50,7 +50,7 @@ def _simple_get(host, endpoint, *_): raise Exception('Bad host') # pylint: disable=broad-exception-raised return host, endpoint - provider = HTTPProvider(['http://localhost:1', 'http://localhost:2']) + provider = HTTPProvider(['http://localhost:1', 'http://localhost:2'], 5 * 60) provider._get_without_fallbacks = Mock(side_effect=_simple_get) with pytest.raises(CustomError): provider._get('test', force_raise=lambda errors: CustomError) diff --git a/tests/providers_clients/test_keys_api_client.py b/tests/providers_clients/test_keys_api_client.py index ed5ddd657..d966a1fd6 100644 --- a/tests/providers_clients/test_keys_api_client.py +++ b/tests/providers_clients/test_keys_api_client.py @@ -12,7 +12,7 @@ @pytest.fixture() def keys_api_client(): - return KeysAPIClient(KEYS_API_URI) + return KeysAPIClient(KEYS_API_URI, 5 * 60) empty_blockstamp = ReferenceBlockStampFactory.build(block_number=0) From c42d06dcba34a31406af8ae8fee2eddd231c06d4 Mon Sep 17 00:00:00 2001 From: Sergey Khomutinin <31664571+skhomuti@users.noreply.github.com> Date: Tue, 11 Apr 2023 17:19:54 +0500 Subject: [PATCH 03/14] Update TX_GAS_ADDITION example value --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 2b2af3ce0..42919ab94 100644 --- a/README.md +++ b/README.md @@ -152,7 +152,7 @@ Full variables list could be found [here](https://github.com/lidofinance/lido-or | `MEMBER_PRIV_KEY_FILE` | A path to the file contained the private key of the Oracle member account. It takes precedence over `MEMBER_PRIV_KEY` | False | `/app/private_key` | | `FINALIZATION_BATCH_MAX_REQUEST_COUNT` | The size of the batch to be finalized per request (The larger the batch size, the more memory of the contract is used but the fewer requests are needed) | False | `1000` | | `ALLOW_REPORTING_IN_BUNKER_MODE` | Allow the Oracle to do report if bunker mode is active | False | `True` | -| `TX_GAS_ADDITION` | Used to modify gas parameter that used in transaction. (gas = estimated_gas + TX_GAS_ADDITION) | False | `1.75` | +| `TX_GAS_ADDITION` | Used to modify gas parameter that used in transaction. (gas = estimated_gas + TX_GAS_ADDITION) | False | `100000` | | `CYCLE_SLEEP_IN_SECONDS` | The time between cycles of the oracle's activity | False | `12` | | `SUBMIT_DATA_DELAY_IN_SLOTS` | The difference in slots between submit data transactions from Oracles. It is used to prevent simultaneous sending of transactions and, as a result, transactions revert. | False | `6` | | `HTTP_REQUEST_RETRY_COUNT` | Total number of retries to fetch data from endpoint | False | `5` | From 505c94cd175c439a3e30fb8e1057b7274bcb6eb7 Mon Sep 17 00:00:00 2001 From: F4ever <1590415904a@gmail.com> Date: Wed, 12 Apr 2023 15:42:15 +0200 Subject: [PATCH 04/14] tx no funds fix --- README.md | 4 ++-- src/web3py/extensions/tx_utils.py | 8 +++++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 2b2af3ce0..df692b565 100644 --- a/README.md +++ b/README.md @@ -109,7 +109,7 @@ Full variables list could be found [here](https://github.com/lidofinance/lido-or Set required values. It will be enough to run the oracle in _check mode_. 2. Check that your environment is ready to run the oracle using the following command: ```bash - docker run --env-file .env --rm lidofinance/oracle:{tag} check + docker run -ti --env-file .env --rm lidofinance/oracle:{tag} check ``` If everything is ok, you will see that all required checks are passed and your environment is ready to run the oracle. @@ -178,7 +178,7 @@ groups: severity: critical annotations: summary: "Dangerously low account balance" - description: "Account balance is less than 3 ETH. Address: {.labels.address}: {.value} ETH" + description: "Account balance is less than 1 ETH. Address: {.labels.address}: {.value} ETH" - alert: OutdatedData expr: (lido_oracle_genesis_time + ignoring (state) lido_oracle_slot_number{state="head"} * 12) < time() - 300 for: 1h diff --git a/src/web3py/extensions/tx_utils.py b/src/web3py/extensions/tx_utils.py index e22f18e23..9b41a569f 100644 --- a/src/web3py/extensions/tx_utils.py +++ b/src/web3py/extensions/tx_utils.py @@ -42,6 +42,9 @@ def _check_transaction(transaction, params: TxParams) -> bool: except ContractLogicError as error: logger.warning({"msg": "Transaction reverted.", "error": str(error)}) return False + except ValueError as error: + logger.error({"msg": "Not enough funds.", "error": str(error)}) + return False logger.info({"msg": "Transaction executed successfully.", "value": result}) return True @@ -79,7 +82,10 @@ def _estimate_gas(transaction: ContractFunction, account: LocalAccount) -> Optio try: gas = transaction.estimate_gas({'from': account.address}) except ContractLogicError as error: - logger.warning({'msg': 'Contract logic error', 'error': str(error)}) + logger.warning({'msg': 'Contract logic error.', 'error': str(error)}) + return None + except ValueError as error: + logger.warning({'msg': 'Execution reverted.', 'error': str(error)}) return None return min( From f97cf82a2783071b2cc0feb96a6afb0416e94cf4 Mon Sep 17 00:00:00 2001 From: F4ever <1590415904a@gmail.com> Date: Wed, 12 Apr 2023 15:44:25 +0200 Subject: [PATCH 05/14] Do not sleep if extra data wasn't submitted --- src/modules/accounting/accounting.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/modules/accounting/accounting.py b/src/modules/accounting/accounting.py index ce838c6b1..308ce4b3e 100644 --- a/src/modules/accounting/accounting.py +++ b/src/modules/accounting/accounting.py @@ -71,10 +71,15 @@ def execute_module(self, last_finalized_blockstamp: BlockStamp) -> ModuleExecute return ModuleExecuteDelay.NEXT_FINALIZED_EPOCH def process_extra_data(self, blockstamp: ReferenceBlockStamp): + latest_blockstamp = self._get_latest_blockstamp() + if not self.can_submit_extra_data(latest_blockstamp): + logger.info({'msg': 'Extra data can not be submitted.'}) + return + chain_config = self.get_chain_config(blockstamp) slots_to_sleep = self._get_slot_delay_before_data_submit(blockstamp) seconds_to_sleep = slots_to_sleep * chain_config.seconds_per_slot - logger.info({'msg': f'Sleep for {seconds_to_sleep} before sending extra data.'}) + logger.info({'msg': f'Sleep for {seconds_to_sleep} seconds before sending extra data.'}) sleep(seconds_to_sleep) latest_blockstamp = self._get_latest_blockstamp() From 32b0df8117a2d068484aa73b46f3f752a3605921 Mon Sep 17 00:00:00 2001 From: F4ever <1590415904a@gmail.com> Date: Wed, 12 Apr 2023 16:01:19 +0200 Subject: [PATCH 06/14] fix test --- tests/modules/accounting/test_accounting_module.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/modules/accounting/test_accounting_module.py b/tests/modules/accounting/test_accounting_module.py index b032c34de..572c81b2f 100644 --- a/tests/modules/accounting/test_accounting_module.py +++ b/tests/modules/accounting/test_accounting_module.py @@ -231,7 +231,8 @@ def test_sumbit_if_can_submit_is_true( accounting.process_extra_data(ref_bs) - accounting.can_submit_extra_data.assert_called_once_with(bs) + assert accounting.can_submit_extra_data.call_count == 2 + assert accounting.can_submit_extra_data.call_args[0][0] is bs submit_extra_data_mock.assert_called_once_with(ref_bs) From 3cb79984a1519616a40231cf02b8db07d51c21da Mon Sep 17 00:00:00 2001 From: rkolpakov Date: Wed, 12 Apr 2023 19:16:03 +0300 Subject: [PATCH 07/14] fix: review fixes --- README.md | 40 ++++++++++--------- src/providers/http_provider.py | 18 ++++++--- src/providers/keys/client.py | 9 ++--- src/variables.py | 17 ++++++-- src/web3py/extensions/consensus.py | 13 +++++- src/web3py/extensions/keys_api.py | 13 +++++- tests/providers.py | 8 +++- .../test_consensus_client.py | 2 +- tests/providers_clients/test_http_provider.py | 8 ++-- .../providers_clients/test_keys_api_client.py | 6 +-- 10 files changed, 86 insertions(+), 48 deletions(-) diff --git a/README.md b/README.md index dc4e6330a..d99b4350c 100644 --- a/README.md +++ b/README.md @@ -64,25 +64,27 @@ docker build -t lidofinance/oracle . ## Env variables -| Name | Description | Required | Example value | -| -------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -------- | ----------------------- | -| `EXECUTION_CLIENT_URI` | URI of the Execution Layer client | True | `http://localhost:8545` | -| `CONSENSUS_CLIENT_URI` | URI of the Consensus Layer client | True | `http://localhost:5052` | -| `KEYS_API_URI` | URI of the Keys API | True | `http://localhost:8080` | -| `LIDO_LOCATOR_ADDRESS` | Address of the Lido contract | True | `0x1...` | -| `MEMBER_PRIV_KEY` | Private key of the Oracle member account | False | `0x1...` | -| `FINALIZATION_BATCH_MAX_REQUEST_COUNT` | The size of the batch to be finalized per request (The larger the batch size, the more memory of the contract is used but the fewer requests are needed) | False | `1000` | -| `ALLOW_REPORTING_IN_BUNKER_MODE` | Allow the Oracle to do report if bunker mode is active | False | `True` | -| `TX_GAS_ADDITION` | Used to modify gas parameter that used in transaction. (gas = estimated_gas + TX_GAS_ADDITION) | False | `1.75` | -| `CYCLE_SLEEP_IN_SECONDS` | The time between cycles of the oracle's activity | False | `12` | -| `SUBMIT_DATA_DELAY_IN_SLOTS` | The difference in slots between submit data transactions from Oracles. It is used to prevent simultaneous sending of transactions and, as a result, transactions revert. | False | `6` | -| `HTTP_REQUEST_RETRY_COUNT` | Total number of retries to fetch data from endpoint | False | `5` | -| `HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS` | The delay http provider sleeps if API is stuck | False | `12` | -| `HTTP_REQUEST_TIMEOUT_CONSENSUS` | Timeout for HTTP consensus layer requests | False | `300` | -| `HTTP_REQUEST_TIMEOUT_KEYS_API` | Timeout for HTTP keys api requests | False | `300` | -| `PRIORITY_FEE_PERCENTILE` | Priority fee percentile from prev block that would be used to send tx | False | `3` | -| `MIN_PRIORITY_FEE` | Min priority fee that would be used to send tx | False | `50000000` | -| `MAX_PRIORITY_FEE` | Max priority fee that would be used to send tx | False | `100000000000` | +| Name | Description | Required | Example value | +|--------------------------------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -------- | ----------------------- | +| `EXECUTION_CLIENT_URI` | URI of the Execution Layer client | True | `http://localhost:8545` | +| `CONSENSUS_CLIENT_URI` | URI of the Consensus Layer client | True | `http://localhost:5052` | +| `KEYS_API_URI` | URI of the Keys API | True | `http://localhost:8080` | +| `LIDO_LOCATOR_ADDRESS` | Address of the Lido contract | True | `0x1...` | +| `MEMBER_PRIV_KEY` | Private key of the Oracle member account | False | `0x1...` | +| `FINALIZATION_BATCH_MAX_REQUEST_COUNT` | The size of the batch to be finalized per request (The larger the batch size, the more memory of the contract is used but the fewer requests are needed) | False | `1000` | +| `ALLOW_REPORTING_IN_BUNKER_MODE` | Allow the Oracle to do report if bunker mode is active | False | `True` | +| `TX_GAS_ADDITION` | Used to modify gas parameter that used in transaction. (gas = estimated_gas + TX_GAS_ADDITION) | False | `1.75` | +| `CYCLE_SLEEP_IN_SECONDS` | The time between cycles of the oracle's activity | False | `12` | +| `SUBMIT_DATA_DELAY_IN_SLOTS` | The difference in slots between submit data transactions from Oracles. It is used to prevent simultaneous sending of transactions and, as a result, transactions revert. | False | `6` | +| `HTTP_REQUEST_TIMEOUT_CONSENSUS` | Timeout for HTTP consensus layer requests | False | `300` | +| `HTTP_REQUEST_RETRY_COUNT_CONSENSUS` | Total number of retries to fetch data from endpoint for consensus layer requests | False | `5` | +| `HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS_CONSENSUS` | The delay http provider sleeps if API is stuck for consensus layer | False | `12` | +| `HTTP_REQUEST_TIMEOUT_KEYS_API` | Timeout for HTTP keys api requests | False | `300` | +| `HTTP_REQUEST_RETRY_COUNT_KEYS_API` | Total number of retries to fetch data from endpoint for keys api requests | False | `300` | +| `HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS_KEYS_API` | The delay http provider sleeps if API is stuck for keys api | False | `300` | +| `PRIORITY_FEE_PERCENTILE` | Priority fee percentile from prev block that would be used to send tx | False | `3` | +| `MIN_PRIORITY_FEE` | Min priority fee that would be used to send tx | False | `50000000` | +| `MAX_PRIORITY_FEE` | Max priority fee that would be used to send tx | False | `100000000000` | ## Monitoring TBD diff --git a/src/providers/http_provider.py b/src/providers/http_provider.py index 34e7ad567..1e4c17604 100644 --- a/src/providers/http_provider.py +++ b/src/providers/http_provider.py @@ -10,8 +10,6 @@ from requests.exceptions import ConnectionError as RequestsConnectionError from urllib3 import Retry -from src.variables import HTTP_REQUEST_RETRY_COUNT, HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS - logger = logging.getLogger(__name__) @@ -36,19 +34,25 @@ class HTTPProvider(ABC): PROMETHEUS_HISTOGRAM: Histogram HTTP_REQUEST_TIMEOUT: int - def __init__(self, hosts: list[str], http_request_timeout: int): + def __init__( + self, + hosts: list[str], + http_request_timeout: int, + http_request_retry_count: int, + http_request_sleep_in_seconds: int + ): if not hosts: raise NoHostsProvided(f"No hosts provided for {self.__class__.__name__}") - + if not http_request_timeout: raise NoHostsProvided(f"No timeout provided for {self.__class__.__name__}") self.hosts = hosts retry_strategy = Retry( - total=HTTP_REQUEST_RETRY_COUNT, + total=http_request_retry_count, status_forcelist=[418, 429, 500, 502, 503, 504], - backoff_factor=HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS, + backoff_factor=http_request_sleep_in_seconds, ) adapter = HTTPAdapter(max_retries=retry_strategy) @@ -57,6 +61,8 @@ def __init__(self, hosts: list[str], http_request_timeout: int): self.session.mount("http://", adapter) self.HTTP_REQUEST_TIMEOUT = http_request_timeout + self.HTTP_REQUEST_RETRY_COUNT = http_request_retry_count + self.HTTP_REQUEST_SLEEP_IN_SECONDS = http_request_sleep_in_seconds @staticmethod def _urljoin(host, url): diff --git a/src/providers/keys/client.py b/src/providers/keys/client.py index 5b34be04f..08d1604e6 100644 --- a/src/providers/keys/client.py +++ b/src/providers/keys/client.py @@ -7,7 +7,6 @@ from src.typings import BlockStamp from src.utils.dataclass import list_of_dataclasses from src.utils.cache import global_lru_cache as lru_cache -from src import variables class KeysOutdatedException(Exception): @@ -33,17 +32,17 @@ def _get_with_blockstamp(self, url: str, blockstamp: BlockStamp, params: Optiona """ Returns response if blockstamp < blockNumber from response """ - for i in range(variables.HTTP_REQUEST_RETRY_COUNT): + for i in range(self.HTTP_REQUEST_RETRY_COUNT): data, meta = self._get(url, query_params=params) blocknumber_meta = meta['meta']['elBlockSnapshot']['blockNumber'] KEYS_API_LATEST_BLOCKNUMBER.set(blocknumber_meta) if blocknumber_meta >= blockstamp.block_number: return data - if i != variables.HTTP_REQUEST_RETRY_COUNT - 1: - sleep(variables.HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS) + if i != self.HTTP_REQUEST_RETRY_COUNT - 1: + sleep(self.HTTP_REQUEST_SLEEP_IN_SECONDS) - raise KeysOutdatedException(f'Keys API Service stuck, no updates for {variables.HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS * variables.HTTP_REQUEST_RETRY_COUNT} seconds.') + raise KeysOutdatedException(f'Keys API Service stuck, no updates for {self.HTTP_REQUEST_SLEEP_IN_SECONDS * self.HTTP_REQUEST_RETRY_COUNT} seconds.') @lru_cache(maxsize=1) @list_of_dataclasses(LidoKey.from_response) diff --git a/src/variables.py b/src/variables.py index 1b2da44bc..dc54d9d6a 100644 --- a/src/variables.py +++ b/src/variables.py @@ -31,10 +31,19 @@ # If contract is reportable each member in order will submit data with difference with this amount of slots SUBMIT_DATA_DELAY_IN_SLOTS = int(os.getenv('SUBMIT_DATA_DELAY_IN_SLOTS', 6)) CYCLE_SLEEP_IN_SECONDS = int(os.getenv('CYCLE_SLEEP_IN_SECONDS', 12)) -HTTP_REQUEST_RETRY_COUNT = int(os.getenv('HTTP_REQUEST_RETRY_COUNT', 5)) -HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS = int(os.getenv('HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS', 5)) -HTTP_REQUEST_TIMEOUT_KEYS_API = int(os.getenv('HTTP_REQUEST_TIMEOUT', 5 * 60)) -HTTP_REQUEST_TIMEOUT_CONSENSUS = int(os.getenv('HTTP_REQUEST_TIMEOUT', 5 * 60)) + +HTTP_REQUEST_TIMEOUT_CONSENSUS = int(os.getenv('HTTP_REQUEST_TIMEOUT_CONSENSUS', 5 * 60)) +HTTP_REQUEST_RETRY_COUNT_CONSENSUS = int(os.getenv('HTTP_REQUEST_RETRY_COUNT_CONSENSUS', 5)) +HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS_CONSENSUS = int( + os.getenv('HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS_CONSENSUS', 5) +) + + +HTTP_REQUEST_TIMEOUT_KEYS_API = int(os.getenv('HTTP_REQUEST_TIMEOUT_KEYS_API', 5 * 60)) +HTTP_REQUEST_RETRY_COUNT_KEYS_API = int(os.getenv('HTTP_REQUEST_RETRY_COUNT_KEYS_API', 5)) +HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS_KEYS_API = int( + os.getenv('HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS_KEYS_API', 5) +) # - Metrics - PROMETHEUS_PORT = int(os.getenv('PROMETHEUS_PORT', 9000)) diff --git a/src/web3py/extensions/consensus.py b/src/web3py/extensions/consensus.py index 16291aa40..341c670bc 100644 --- a/src/web3py/extensions/consensus.py +++ b/src/web3py/extensions/consensus.py @@ -2,12 +2,21 @@ from web3.module import Module from src.providers.consensus.client import ConsensusClient -from src.variables import HTTP_REQUEST_TIMEOUT_CONSENSUS +from src.variables import ( + HTTP_REQUEST_TIMEOUT_CONSENSUS, + HTTP_REQUEST_RETRY_COUNT_CONSENSUS, + HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS_CONSENSUS +) class ConsensusClientModule(ConsensusClient, Module): def __init__(self, hosts: list[str], w3: Web3): self.w3 = w3 - super(ConsensusClient, self).__init__(hosts, HTTP_REQUEST_TIMEOUT_CONSENSUS) + super(ConsensusClient, self).__init__( + hosts, + HTTP_REQUEST_TIMEOUT_CONSENSUS, + HTTP_REQUEST_RETRY_COUNT_CONSENSUS, + HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS_CONSENSUS + ) super(Module, self).__init__() diff --git a/src/web3py/extensions/keys_api.py b/src/web3py/extensions/keys_api.py index 467ef8f50..917e01adf 100644 --- a/src/web3py/extensions/keys_api.py +++ b/src/web3py/extensions/keys_api.py @@ -2,12 +2,21 @@ from web3.module import Module from src.providers.keys.client import KeysAPIClient -from src.variables import HTTP_REQUEST_TIMEOUT_KEYS_API +from src.variables import ( + HTTP_REQUEST_TIMEOUT_KEYS_API, + HTTP_REQUEST_RETRY_COUNT_KEYS_API, + HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS_KEYS_API +) class KeysAPIClientModule(KeysAPIClient, Module): def __init__(self, hosts: list[str], w3: Web3): self.w3 = w3 - super(KeysAPIClient, self).__init__(hosts, HTTP_REQUEST_TIMEOUT_KEYS_API) + super(KeysAPIClient, self).__init__( + hosts, + HTTP_REQUEST_TIMEOUT_KEYS_API, + HTTP_REQUEST_RETRY_COUNT_KEYS_API, + HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS_KEYS_API + ) super(Module, self).__init__() diff --git a/tests/providers.py b/tests/providers.py index ffa8fc65b..5e8765b85 100644 --- a/tests/providers.py +++ b/tests/providers.py @@ -106,7 +106,9 @@ def use_mock(self, mock_path: Path): class ResponseFromFileHTTPProvider(HTTPProvider, Module, FromFile): def __init__(self, mock_path: Path, w3: Web3): self.w3 = w3 - HTTPProvider.__init__(self, hosts=[""], http_request_timeout=5 * 60) + HTTPProvider.__init__( + self, hosts=[""], http_request_timeout=5 * 60, http_request_retry_count=5, http_request_sleep_in_seconds=5 + ) Module.__init__(self, w3) FromFile.__init__(self, mock_path) @@ -128,7 +130,9 @@ class UpdateResponsesHTTPProvider(HTTPProvider, Module, UpdateResponses): def __init__(self, mock_path: Path, host: str, w3: Web3): self.w3 = w3 - super().__init__([host], http_request_timeout=5 * 60) + super().__init__( + [host], http_request_timeout=5 * 60, http_request_retry_count=5, http_request_sleep_in_seconds=5 + ) super(Module, self).__init__() self.responses = [] self.from_file = ResponseFromFileHTTPProvider(mock_path, w3) diff --git a/tests/providers_clients/test_consensus_client.py b/tests/providers_clients/test_consensus_client.py index 4f2320a4b..56349092c 100644 --- a/tests/providers_clients/test_consensus_client.py +++ b/tests/providers_clients/test_consensus_client.py @@ -9,7 +9,7 @@ @pytest.fixture def consensus_client(): - return ConsensusClient(CONSENSUS_CLIENT_URI, 5 * 60) + return ConsensusClient(CONSENSUS_CLIENT_URI, 5 * 60, 5, 5) @pytest.mark.integration diff --git a/tests/providers_clients/test_http_provider.py b/tests/providers_clients/test_http_provider.py index 878271406..9f51b4d0d 100644 --- a/tests/providers_clients/test_http_provider.py +++ b/tests/providers_clients/test_http_provider.py @@ -19,13 +19,13 @@ def test_urljoin(): def test_all_fallbacks_ok(): - provider = HTTPProvider(['http://localhost:1', 'http://localhost:2'], 5 * 60) + provider = HTTPProvider(['http://localhost:1', 'http://localhost:2'], 5 * 60, 1, 1) provider._get_without_fallbacks = lambda host, endpoint, path_params, query_params: (host, endpoint) assert provider._get('test') == ('http://localhost:1', 'test') def test_all_fallbacks_bad(): - provider = HTTPProvider(['http://localhost:1', 'http://localhost:2'], 5 * 60) + provider = HTTPProvider(['http://localhost:1', 'http://localhost:2'], 5 * 60, 1, 1) with pytest.raises(Exception): provider._get('test') @@ -36,7 +36,7 @@ def _simple_get(host, endpoint, *_): raise Exception('Bad host') # pylint: disable=broad-exception-raised return host, endpoint - provider = HTTPProvider(['http://localhost:1', 'http://localhost:2'], 5 * 60) + provider = HTTPProvider(['http://localhost:1', 'http://localhost:2'], 5 * 60, 1, 1) provider._get_without_fallbacks = _simple_get assert provider._get('test') == ('http://localhost:2', 'test') @@ -50,7 +50,7 @@ def _simple_get(host, endpoint, *_): raise Exception('Bad host') # pylint: disable=broad-exception-raised return host, endpoint - provider = HTTPProvider(['http://localhost:1', 'http://localhost:2'], 5 * 60) + provider = HTTPProvider(['http://localhost:1', 'http://localhost:2'], 5 * 60, 1, 1) provider._get_without_fallbacks = Mock(side_effect=_simple_get) with pytest.raises(CustomError): provider._get('test', force_raise=lambda errors: CustomError) diff --git a/tests/providers_clients/test_keys_api_client.py b/tests/providers_clients/test_keys_api_client.py index d966a1fd6..e6f326479 100644 --- a/tests/providers_clients/test_keys_api_client.py +++ b/tests/providers_clients/test_keys_api_client.py @@ -12,7 +12,7 @@ @pytest.fixture() def keys_api_client(): - return KeysAPIClient(KEYS_API_URI, 5 * 60) + return KeysAPIClient(KEYS_API_URI, 5 * 60, 5, 5) empty_blockstamp = ReferenceBlockStampFactory.build(block_number=0) @@ -47,5 +47,5 @@ def test_get_with_blockstamp_retries_exhausted(keys_api_client, monkeypatch): m.setattr(keys_api_client_module, "sleep", sleep_mock) keys_api_client.get_used_lido_keys(empty_blockstamp) - assert sleep_mock.call_count == variables.HTTP_REQUEST_RETRY_COUNT - 1 - sleep_mock.assert_called_with(variables.HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS) + assert sleep_mock.call_count == variables.HTTP_REQUEST_RETRY_COUNT_KEYS_API - 1 + sleep_mock.assert_called_with(variables.HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS_KEYS_API) From 9413d0d4bec5513442fbe79ae985a130dc14b6f9 Mon Sep 17 00:00:00 2001 From: rkolpakov Date: Wed, 12 Apr 2023 17:34:03 +0300 Subject: [PATCH 08/14] feat: add health checker --- src/main.py | 32 ++++++++++----------- src/modules/checks/suites/common.py | 2 +- src/providers/consensus/client.py | 6 ++++ src/providers/http_provider.py | 11 +++++-- src/providers/keys/client.py | 4 +++ src/web3py/extensions/__init__.py | 2 ++ src/web3py/extensions/consistency.py | 43 ++++++++++++++++++++++++++++ src/web3py/extensions/fallback.py | 14 +++++++++ tests/providers.py | 6 ++++ 9 files changed, 101 insertions(+), 19 deletions(-) create mode 100644 src/web3py/extensions/consistency.py create mode 100644 src/web3py/extensions/fallback.py diff --git a/src/main.py b/src/main.py index 760b9f378..738d57736 100644 --- a/src/main.py +++ b/src/main.py @@ -2,7 +2,6 @@ from typing import cast from prometheus_client import start_http_server -from web3_multi_provider import FallbackProvider from web3.middleware import simple_cache_middleware from src import variables @@ -20,6 +19,7 @@ ConsensusClientModule, KeysAPIClientModule, LidoValidatorsProvider, + FallbackProviderModule ) from src.web3py.middleware import metrics_collector from src.web3py.typings import Web3 @@ -57,7 +57,7 @@ def main(module: OracleModule): start_http_server(variables.PROMETHEUS_PORT) logger.info({'msg': 'Initialize multi web3 provider.'}) - web3 = Web3(FallbackProvider(variables.EXECUTION_CLIENT_URI)) + web3 = Web3(FallbackProviderModule(variables.EXECUTION_CLIENT_URI)) logger.info({'msg': 'Modify web3 with custom contract function call.'}) tweak_w3_contracts(web3) @@ -68,6 +68,8 @@ def main(module: OracleModule): logger.info({'msg': 'Initialize keys api client.'}) kac = KeysAPIClientModule(variables.KEYS_API_URI, web3) + check_providers_chain_ids(web3, cc, kac) + web3.attach_modules({ 'lido_contracts': LidoContracts, 'lido_validators': LidoValidatorsProvider, @@ -81,7 +83,6 @@ def main(module: OracleModule): web3.middleware_onion.add(simple_cache_middleware) logger.info({'msg': 'Sanity checks.'}) - check_providers_chain_ids(web3) if module == OracleModule.ACCOUNTING: logger.info({'msg': 'Initialize Accounting module.'}) @@ -101,19 +102,18 @@ def check(): return ChecksModule().execute_module() -def check_providers_chain_ids(web3: Web3): - execution_chain_id = web3.eth.chain_id - consensus_chain_id = int(web3.cc.get_config_spec().DEPOSIT_CHAIN_ID) - chain_ids = [ - Web3.to_int(hexstr=provider.make_request("eth_chainId", []).get('result')) - for provider in cast(FallbackProvider, web3.provider)._providers # type: ignore[attr-defined] # pylint: disable=protected-access - ] - keys_api_chain_id = web3.kac.get_status().chainId - if any(execution_chain_id != chain_id for chain_id in [*chain_ids, consensus_chain_id, keys_api_chain_id]): - raise ValueError('Different chain ids detected:\n' - f'Execution chain ids: {", ".join(map(str, chain_ids))}\n' - f'Consensus chain id: {consensus_chain_id}\n' - f'Keys API chain id: {keys_api_chain_id}\n') +def check_providers_chain_ids(web3: Web3, cc: ConsensusClientModule, kac: KeysAPIClientModule): + keys_api_chain_id = kac.check_providers_consistency() + consensus_chain_id = cc.check_providers_consistency() + execution_chain_id = cast(FallbackProviderModule, web3.provider).check_providers_consistency() + + if execution_chain_id == consensus_chain_id == keys_api_chain_id: + return + + raise ValueError('Different chain ids detected:\n' + f'Execution chain id: {execution_chain_id}\n' + f'Consensus chain id: {consensus_chain_id}\n' + f'Keys API chain id: {keys_api_chain_id}\n') if __name__ == '__main__': diff --git a/src/modules/checks/suites/common.py b/src/modules/checks/suites/common.py index f9cf0e442..2bd9310ed 100644 --- a/src/modules/checks/suites/common.py +++ b/src/modules/checks/suites/common.py @@ -24,7 +24,7 @@ def ejector(web3, skip_locator): def check_providers_chain_ids(web3): """Make sure all providers are on the same chain""" - chain_ids_check(web3) + chain_ids_check(web3, web3.cc, web3.kac) def check_accounting_contract_configs(accounting): diff --git a/src/providers/consensus/client.py b/src/providers/consensus/client.py index f567c4305..6cff15d68 100644 --- a/src/providers/consensus/client.py +++ b/src/providers/consensus/client.py @@ -156,3 +156,9 @@ def __raise_last_missed_slot_error(self, errors: list[Exception]) -> Exception | return error return None + + def get_chain_id(self, host: str) -> int: + data, _ = self._get_without_fallbacks(host, self.API_GET_SPEC) + if not isinstance(data, dict): + raise ValueError("Expected mapping response from getSpec") + return int(BeaconSpecResponse.from_response(**data).DEPOSIT_CHAIN_ID) diff --git a/src/providers/http_provider.py b/src/providers/http_provider.py index 786fd5849..894398eeb 100644 --- a/src/providers/http_provider.py +++ b/src/providers/http_provider.py @@ -1,7 +1,7 @@ import logging from abc import ABC from http import HTTPStatus -from typing import Optional, Tuple, Sequence, Callable +from typing import Optional, Tuple, Sequence, Callable, List from urllib.parse import urljoin, urlparse from prometheus_client import Histogram @@ -11,6 +11,7 @@ from urllib3 import Retry from src.variables import HTTP_REQUEST_RETRY_COUNT, HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS, HTTP_REQUEST_TIMEOUT +from src.web3py.extensions.consistency import ProviderConsistencyModule logger = logging.getLogger(__name__) @@ -29,7 +30,7 @@ def __init__(self, *args, status: int, text: str): super().__init__(*args) -class HTTPProvider(ABC): +class HTTPProvider(ProviderConsistencyModule, ABC): """ Base HTTP Provider with metrics and retry strategy integrated inside. """ @@ -151,3 +152,9 @@ def _get_without_fallbacks( meta = {} return data, meta + + def get_all_hosts(self) -> List[Tuple[str, str]]: + return list(map(lambda host: (host, host), self.hosts)) + + def get_chain_id(self, host) -> int: + raise NotImplementedError("_chain_id should be implemented") diff --git a/src/providers/keys/client.py b/src/providers/keys/client.py index 5b34be04f..132958cb8 100644 --- a/src/providers/keys/client.py +++ b/src/providers/keys/client.py @@ -55,3 +55,7 @@ def get_status(self) -> KeysApiStatus: """Docs: https://keys-api.lido.fi/api/static/index.html#/status/StatusController_get""" data, _ = self._get(self.STATUS) return KeysApiStatus.from_response(**cast(dict, data)) + + def get_chain_id(self, host: str) -> int: + data, _ = self._get_without_fallbacks(host, self.STATUS) + return KeysApiStatus.from_response(**cast(dict, data)).chainId diff --git a/src/web3py/extensions/__init__.py b/src/web3py/extensions/__init__.py index b7b8ac898..2edd2242c 100644 --- a/src/web3py/extensions/__init__.py +++ b/src/web3py/extensions/__init__.py @@ -3,3 +3,5 @@ from src.web3py.extensions.consensus import ConsensusClientModule from src.web3py.extensions.contracts import LidoContracts from src.web3py.extensions.lido_validators import LidoValidatorsProvider +from src.web3py.extensions.fallback import FallbackProviderModule +from src.web3py.extensions.consistency import ProviderConsistencyModule diff --git a/src/web3py/extensions/consistency.py b/src/web3py/extensions/consistency.py new file mode 100644 index 000000000..0c680e5a3 --- /dev/null +++ b/src/web3py/extensions/consistency.py @@ -0,0 +1,43 @@ +from typing import Any, Tuple, List, Optional +from abc import abstractmethod, ABC +from requests.exceptions import ConnectionError as RequestsConnectionError + + +class ProviderConsistencyModule(ABC): + """ + A class that provides HTTP provider with the ability to check that + provided hosts are alive and chain ids are same. + + Methods must be implemented: + def get_all_hosts(self) -> [any, str]: + def get_chain_id(self, host) -> int: + """ + def check_providers_consistency(self) -> Optional[int]: + chain_id = None + + for (host, endpoint) in self.get_all_hosts(): + try: + curr_chain_id = self.get_chain_id(host) + if chain_id is None: + chain_id = curr_chain_id + elif chain_id != curr_chain_id: + raise ValueError(f'Different chain ids detected: {endpoint}') + except Exception as exc: + raise RequestsConnectionError(f"Provider doesn't respond: {endpoint}") from exc + + return chain_id + + @abstractmethod + def get_all_hosts(self) -> List[Tuple[Any, str]]: + """ + Returns a list of hosts and URIs to be health checked. + + HTTP provider returns URI string. + Web3 provider returns Provider instance. + """ + raise NotImplementedError("get_all_hosts should be implemented") + + @abstractmethod + def get_chain_id(self, host) -> int: + """Does a health check call and returns chain_id for current host""" + raise NotImplementedError("_chain_id should be implemented") diff --git a/src/web3py/extensions/fallback.py b/src/web3py/extensions/fallback.py new file mode 100644 index 000000000..356f3b0f3 --- /dev/null +++ b/src/web3py/extensions/fallback.py @@ -0,0 +1,14 @@ +from typing import Any, Tuple, List + +from web3_multi_provider import FallbackProvider +from src.web3py.extensions.consistency import ProviderConsistencyModule +from web3 import Web3 + + +class FallbackProviderModule(ProviderConsistencyModule, FallbackProvider): + + def get_all_hosts(self) -> List[Tuple[Any, str]]: + return list(map(lambda provider: (provider, provider.endpoint_uri), self._providers)) + + def get_chain_id(self, host) -> int: + return Web3.to_int(hexstr=host.make_request("eth_chainId", []).get('result')) diff --git a/tests/providers.py b/tests/providers.py index a7bcb9411..5e5c7be84 100644 --- a/tests/providers.py +++ b/tests/providers.py @@ -123,6 +123,12 @@ def _get( return response["response"] raise NoMockException('There is no mock for response') + def get_all_hosts(self) -> list: + return [] + + def get_chain_id(self, host) -> int: + return 0 + class UpdateResponsesHTTPProvider(HTTPProvider, Module, UpdateResponses): def __init__(self, mock_path: Path, host: str, w3: Web3): From 04397e987e8371809d6e5d6de0e6817e34b5ef1d Mon Sep 17 00:00:00 2001 From: F4ever <1590415904a@gmail.com> Date: Thu, 13 Apr 2023 09:09:56 +0200 Subject: [PATCH 09/14] From uppercase to lowercase + timeout for keys api to 10 seconds --- README.md | 4 ++-- src/providers/http_provider.py | 24 ++++++++++-------------- src/providers/keys/client.py | 8 ++++---- src/services/validator_state.py | 2 +- src/variables.py | 3 +-- src/web3py/extensions/consensus.py | 4 ++-- 6 files changed, 20 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index cb38cdfaf..16a5120a4 100644 --- a/README.md +++ b/README.md @@ -152,13 +152,13 @@ Full variables list could be found [here](https://github.com/lidofinance/lido-or | `MEMBER_PRIV_KEY_FILE` | A path to the file contained the private key of the Oracle member account. It takes precedence over `MEMBER_PRIV_KEY` | False | `/app/private_key` | | `FINALIZATION_BATCH_MAX_REQUEST_COUNT` | The size of the batch to be finalized per request (The larger the batch size, the more memory of the contract is used but the fewer requests are needed) | False | `1000` | | `ALLOW_REPORTING_IN_BUNKER_MODE` | Allow the Oracle to do report if bunker mode is active | False | `True` | -| `TX_GAS_ADDITION` | Used to modify gas parameter that used in transaction. (gas = estimated_gas + TX_GAS_ADDITION) | False | `100000` | +| `TX_GAS_ADDITION` | Used to modify gas parameter that used in transaction. (gas = estimated_gas + TX_GAS_ADDITION) | False | `100000` | | `CYCLE_SLEEP_IN_SECONDS` | The time between cycles of the oracle's activity | False | `12` | | `SUBMIT_DATA_DELAY_IN_SLOTS` | The difference in slots between submit data transactions from Oracles. It is used to prevent simultaneous sending of transactions and, as a result, transactions revert. | False | `6` | | `HTTP_REQUEST_TIMEOUT_CONSENSUS` | Timeout for HTTP consensus layer requests | False | `300` | | `HTTP_REQUEST_RETRY_COUNT_CONSENSUS` | Total number of retries to fetch data from endpoint for consensus layer requests | False | `5` | | `HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS_CONSENSUS` | The delay http provider sleeps if API is stuck for consensus layer | False | `12` | -| `HTTP_REQUEST_TIMEOUT_KEYS_API` | Timeout for HTTP keys api requests | False | `300` | +| `HTTP_REQUEST_TIMEOUT_KEYS_API` | Timeout for HTTP keys api requests | False | `10` | | `HTTP_REQUEST_RETRY_COUNT_KEYS_API` | Total number of retries to fetch data from endpoint for keys api requests | False | `300` | | `HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS_KEYS_API` | The delay http provider sleeps if API is stuck for keys api | False | `300` | | `PRIORITY_FEE_PERCENTILE` | Priority fee percentile from prev block that would be used to send tx | False | `3` | diff --git a/src/providers/http_provider.py b/src/providers/http_provider.py index 1e4c17604..c400b57a2 100644 --- a/src/providers/http_provider.py +++ b/src/providers/http_provider.py @@ -32,27 +32,27 @@ class HTTPProvider(ABC): Base HTTP Provider with metrics and retry strategy integrated inside. """ PROMETHEUS_HISTOGRAM: Histogram - HTTP_REQUEST_TIMEOUT: int + request_timeout: int def __init__( self, hosts: list[str], - http_request_timeout: int, - http_request_retry_count: int, - http_request_sleep_in_seconds: int + request_timeout: int, + retry_total: int, + retry_backoff_factor: int, ): if not hosts: raise NoHostsProvided(f"No hosts provided for {self.__class__.__name__}") - if not http_request_timeout: - raise NoHostsProvided(f"No timeout provided for {self.__class__.__name__}") - self.hosts = hosts + self.request_timeout = request_timeout + self.retry_count = retry_total + self.backoff_factor = retry_backoff_factor retry_strategy = Retry( - total=http_request_retry_count, + total=self.retry_count, status_forcelist=[418, 429, 500, 502, 503, 504], - backoff_factor=http_request_sleep_in_seconds, + backoff_factor=self.backoff_factor, ) adapter = HTTPAdapter(max_retries=retry_strategy) @@ -60,10 +60,6 @@ def __init__( self.session.mount("https://", adapter) self.session.mount("http://", adapter) - self.HTTP_REQUEST_TIMEOUT = http_request_timeout - self.HTTP_REQUEST_RETRY_COUNT = http_request_retry_count - self.HTTP_REQUEST_SLEEP_IN_SECONDS = http_request_sleep_in_seconds - @staticmethod def _urljoin(host, url): if not host.endswith('/'): @@ -125,7 +121,7 @@ def _get_without_fallbacks( response = self.session.get( self._urljoin(host, complete_endpoint if path_params else endpoint), params=query_params, - timeout=self.HTTP_REQUEST_TIMEOUT, + timeout=self.request_timeout, ) except RequestsConnectionError as error: logger.debug({'msg': str(error)}) diff --git a/src/providers/keys/client.py b/src/providers/keys/client.py index 08d1604e6..1d7726bce 100644 --- a/src/providers/keys/client.py +++ b/src/providers/keys/client.py @@ -32,17 +32,17 @@ def _get_with_blockstamp(self, url: str, blockstamp: BlockStamp, params: Optiona """ Returns response if blockstamp < blockNumber from response """ - for i in range(self.HTTP_REQUEST_RETRY_COUNT): + for i in range(self.retry_count): data, meta = self._get(url, query_params=params) blocknumber_meta = meta['meta']['elBlockSnapshot']['blockNumber'] KEYS_API_LATEST_BLOCKNUMBER.set(blocknumber_meta) if blocknumber_meta >= blockstamp.block_number: return data - if i != self.HTTP_REQUEST_RETRY_COUNT - 1: - sleep(self.HTTP_REQUEST_SLEEP_IN_SECONDS) + if i != self.retry_count - 1: + sleep(self.backoff_factor) - raise KeysOutdatedException(f'Keys API Service stuck, no updates for {self.HTTP_REQUEST_SLEEP_IN_SECONDS * self.HTTP_REQUEST_RETRY_COUNT} seconds.') + raise KeysOutdatedException(f'Keys API Service stuck, no updates for {self.backoff_factor * self.retry_count} seconds.') @lru_cache(maxsize=1) @list_of_dataclasses(LidoKey.from_response) diff --git a/src/services/validator_state.py b/src/services/validator_state.py index 52b38537a..da94c07d5 100644 --- a/src/services/validator_state.py +++ b/src/services/validator_state.py @@ -9,7 +9,7 @@ from src.metrics.prometheus.accounting import ( ACCOUNTING_STUCK_VALIDATORS, ACCOUNTING_EXITED_VALIDATORS, - ACCOUNTING_DELAYED_VALIDATORS + ACCOUNTING_DELAYED_VALIDATORS, ) from src.modules.accounting.extra_data import ExtraDataService, ExtraData from src.modules.accounting.typings import OracleReportLimits diff --git a/src/variables.py b/src/variables.py index bfdcb9c78..57b684ea6 100644 --- a/src/variables.py +++ b/src/variables.py @@ -48,8 +48,7 @@ os.getenv('HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS_CONSENSUS', 5) ) - -HTTP_REQUEST_TIMEOUT_KEYS_API = int(os.getenv('HTTP_REQUEST_TIMEOUT_KEYS_API', 5 * 60)) +HTTP_REQUEST_TIMEOUT_KEYS_API = int(os.getenv('HTTP_REQUEST_TIMEOUT_KEYS_API', 10)) HTTP_REQUEST_RETRY_COUNT_KEYS_API = int(os.getenv('HTTP_REQUEST_RETRY_COUNT_KEYS_API', 5)) HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS_KEYS_API = int( os.getenv('HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS_KEYS_API', 5) diff --git a/src/web3py/extensions/consensus.py b/src/web3py/extensions/consensus.py index 341c670bc..0ece08538 100644 --- a/src/web3py/extensions/consensus.py +++ b/src/web3py/extensions/consensus.py @@ -5,7 +5,7 @@ from src.variables import ( HTTP_REQUEST_TIMEOUT_CONSENSUS, HTTP_REQUEST_RETRY_COUNT_CONSENSUS, - HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS_CONSENSUS + HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS_CONSENSUS, ) @@ -17,6 +17,6 @@ def __init__(self, hosts: list[str], w3: Web3): hosts, HTTP_REQUEST_TIMEOUT_CONSENSUS, HTTP_REQUEST_RETRY_COUNT_CONSENSUS, - HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS_CONSENSUS + HTTP_REQUEST_SLEEP_BEFORE_RETRY_IN_SECONDS_CONSENSUS, ) super(Module, self).__init__() From eaf15bb1798a680e6012817c31968eed6a8b8a58 Mon Sep 17 00:00:00 2001 From: F4ever <1590415904a@gmail.com> Date: Thu, 13 Apr 2023 09:38:15 +0200 Subject: [PATCH 10/14] fix tests --- tests/providers.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/providers.py b/tests/providers.py index 5e8765b85..c9aac21ac 100644 --- a/tests/providers.py +++ b/tests/providers.py @@ -107,7 +107,11 @@ class ResponseFromFileHTTPProvider(HTTPProvider, Module, FromFile): def __init__(self, mock_path: Path, w3: Web3): self.w3 = w3 HTTPProvider.__init__( - self, hosts=[""], http_request_timeout=5 * 60, http_request_retry_count=5, http_request_sleep_in_seconds=5 + self, + hosts=[""], + request_timeout=5 * 60, + retry_total=5, + retry_backoff_factor=5, ) Module.__init__(self, w3) FromFile.__init__(self, mock_path) @@ -131,7 +135,10 @@ def __init__(self, mock_path: Path, host: str, w3: Web3): self.w3 = w3 super().__init__( - [host], http_request_timeout=5 * 60, http_request_retry_count=5, http_request_sleep_in_seconds=5 + [host], + request_timeout=5 * 60, + retry_total=5, + retry_backoff_factor=5, ) super(Module, self).__init__() self.responses = [] From 27ddfb264133d73dcf13c6a13e2ed2c0eedace4f Mon Sep 17 00:00:00 2001 From: F4ever <1590415904a@gmail.com> Date: Thu, 13 Apr 2023 12:00:24 +0200 Subject: [PATCH 11/14] update healthcheck provider --- src/providers/consensus/client.py | 4 +-- src/providers/http_provider.py | 6 ++-- src/providers/keys/client.py | 4 +-- src/web3py/extensions/consistency.py | 49 +++++++++++++++------------- src/web3py/extensions/fallback.py | 9 +++-- 5 files changed, 38 insertions(+), 34 deletions(-) diff --git a/src/providers/consensus/client.py b/src/providers/consensus/client.py index 6cff15d68..adcb8a456 100644 --- a/src/providers/consensus/client.py +++ b/src/providers/consensus/client.py @@ -157,8 +157,8 @@ def __raise_last_missed_slot_error(self, errors: list[Exception]) -> Exception | return None - def get_chain_id(self, host: str) -> int: - data, _ = self._get_without_fallbacks(host, self.API_GET_SPEC) + def _get_chain_id_with_provider(self, provider_index: int) -> int: + data, _ = self._get_without_fallbacks(self.hosts[provider_index], self.API_GET_SPEC) if not isinstance(data, dict): raise ValueError("Expected mapping response from getSpec") return int(BeaconSpecResponse.from_response(**data).DEPOSIT_CHAIN_ID) diff --git a/src/providers/http_provider.py b/src/providers/http_provider.py index 894398eeb..ea34a43a1 100644 --- a/src/providers/http_provider.py +++ b/src/providers/http_provider.py @@ -153,8 +153,8 @@ def _get_without_fallbacks( return data, meta - def get_all_hosts(self) -> List[Tuple[str, str]]: - return list(map(lambda host: (host, host), self.hosts)) + def get_all_providers(self) -> list[str]: + return self.hosts - def get_chain_id(self, host) -> int: + def _get_chain_id_with_provider(self, provider_index: int) -> int: raise NotImplementedError("_chain_id should be implemented") diff --git a/src/providers/keys/client.py b/src/providers/keys/client.py index 132958cb8..49e26fe9d 100644 --- a/src/providers/keys/client.py +++ b/src/providers/keys/client.py @@ -56,6 +56,6 @@ def get_status(self) -> KeysApiStatus: data, _ = self._get(self.STATUS) return KeysApiStatus.from_response(**cast(dict, data)) - def get_chain_id(self, host: str) -> int: - data, _ = self._get_without_fallbacks(host, self.STATUS) + def _get_chain_id_with_provider(self, provider_index: int) -> int: + data, _ = self._get_without_fallbacks(self.hosts[provider_index], self.STATUS) return KeysApiStatus.from_response(**cast(dict, data)).chainId diff --git a/src/web3py/extensions/consistency.py b/src/web3py/extensions/consistency.py index 0c680e5a3..c861125d6 100644 --- a/src/web3py/extensions/consistency.py +++ b/src/web3py/extensions/consistency.py @@ -3,41 +3,46 @@ from requests.exceptions import ConnectionError as RequestsConnectionError +class InconsistentProviders(Exception): + pass + + +class NotHealthyProvider(Exception): + pass + + class ProviderConsistencyModule(ABC): """ - A class that provides HTTP provider with the ability to check that + A class that provides HTTP provider ability to check that provided hosts are alive and chain ids are same. Methods must be implemented: - def get_all_hosts(self) -> [any, str]: - def get_chain_id(self, host) -> int: + def get_all_providers(self) -> [any]: + def _get_chain_id_with_provider(self, int) -> int: """ - def check_providers_consistency(self) -> Optional[int]: + def check_providers_consistency(self) -> int: chain_id = None - for (host, endpoint) in self.get_all_hosts(): + for provider_index in range(len(self.get_all_providers())): try: - curr_chain_id = self.get_chain_id(host) - if chain_id is None: - chain_id = curr_chain_id - elif chain_id != curr_chain_id: - raise ValueError(f'Different chain ids detected: {endpoint}') - except Exception as exc: - raise RequestsConnectionError(f"Provider doesn't respond: {endpoint}") from exc + curr_chain_id = self._get_chain_id_with_provider(provider_index) + except Exception as error: + raise NotHealthyProvider(f'Provider [{provider_index}] does not responding.') from error + + if chain_id is None: + chain_id = curr_chain_id + elif chain_id != curr_chain_id: + raise InconsistentProviders(f'Different chain ids detected for {provider_index=}. ' + f'Expected {curr_chain_id=}, got {chain_id=}.') return chain_id @abstractmethod - def get_all_hosts(self) -> List[Tuple[Any, str]]: - """ - Returns a list of hosts and URIs to be health checked. - - HTTP provider returns URI string. - Web3 provider returns Provider instance. - """ - raise NotImplementedError("get_all_hosts should be implemented") + def get_all_providers(self) -> list[Any]: + """Returns list of hosts or providers.""" + raise NotImplementedError("get_all_providers should be implemented") @abstractmethod - def get_chain_id(self, host) -> int: + def _get_chain_id_with_provider(self, provider_index: int) -> int: """Does a health check call and returns chain_id for current host""" - raise NotImplementedError("_chain_id should be implemented") + raise NotImplementedError("get_chain_id should be implemented") diff --git a/src/web3py/extensions/fallback.py b/src/web3py/extensions/fallback.py index 356f3b0f3..081cdac22 100644 --- a/src/web3py/extensions/fallback.py +++ b/src/web3py/extensions/fallback.py @@ -6,9 +6,8 @@ class FallbackProviderModule(ProviderConsistencyModule, FallbackProvider): + def get_all_providers(self) -> list[Any]: + return self._providers - def get_all_hosts(self) -> List[Tuple[Any, str]]: - return list(map(lambda provider: (provider, provider.endpoint_uri), self._providers)) - - def get_chain_id(self, host) -> int: - return Web3.to_int(hexstr=host.make_request("eth_chainId", []).get('result')) + def _get_chain_id_with_provider(self, provider_index: int) -> int: + return Web3.to_int(hexstr=self._providers[provider_index].make_request("eth_chainId", []).get('result')) From a6895735d477dba2600adc222bb3ccd0822edc7f Mon Sep 17 00:00:00 2001 From: F4ever <1590415904a@gmail.com> Date: Thu, 13 Apr 2023 12:10:56 +0200 Subject: [PATCH 12/14] fix linters --- src/providers/http_provider.py | 2 +- src/web3py/extensions/consistency.py | 5 ++--- src/web3py/extensions/fallback.py | 6 +++--- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/providers/http_provider.py b/src/providers/http_provider.py index ea34a43a1..3404f654f 100644 --- a/src/providers/http_provider.py +++ b/src/providers/http_provider.py @@ -1,7 +1,7 @@ import logging from abc import ABC from http import HTTPStatus -from typing import Optional, Tuple, Sequence, Callable, List +from typing import Optional, Tuple, Sequence, Callable from urllib.parse import urljoin, urlparse from prometheus_client import Histogram diff --git a/src/web3py/extensions/consistency.py b/src/web3py/extensions/consistency.py index c861125d6..274b834b8 100644 --- a/src/web3py/extensions/consistency.py +++ b/src/web3py/extensions/consistency.py @@ -1,6 +1,5 @@ -from typing import Any, Tuple, List, Optional +from typing import Any, Optional from abc import abstractmethod, ABC -from requests.exceptions import ConnectionError as RequestsConnectionError class InconsistentProviders(Exception): @@ -20,7 +19,7 @@ class ProviderConsistencyModule(ABC): def get_all_providers(self) -> [any]: def _get_chain_id_with_provider(self, int) -> int: """ - def check_providers_consistency(self) -> int: + def check_providers_consistency(self) -> Optional[int]: chain_id = None for provider_index in range(len(self.get_all_providers())): diff --git a/src/web3py/extensions/fallback.py b/src/web3py/extensions/fallback.py index 081cdac22..11a12cb67 100644 --- a/src/web3py/extensions/fallback.py +++ b/src/web3py/extensions/fallback.py @@ -1,4 +1,4 @@ -from typing import Any, Tuple, List +from typing import Any, Union from web3_multi_provider import FallbackProvider from src.web3py.extensions.consistency import ProviderConsistencyModule @@ -7,7 +7,7 @@ class FallbackProviderModule(ProviderConsistencyModule, FallbackProvider): def get_all_providers(self) -> list[Any]: - return self._providers + return self._providers # type: ignore[attr-defined] def _get_chain_id_with_provider(self, provider_index: int) -> int: - return Web3.to_int(hexstr=self._providers[provider_index].make_request("eth_chainId", []).get('result')) + return Web3.to_int(hexstr=self._providers[provider_index].make_request("eth_chainId", []).get('result')) # type: ignore[attr-defined] From 84e0cb39869d30a9e1a05dbcd6b2e0a9ff9c48ec Mon Sep 17 00:00:00 2001 From: F4ever <1590415904a@gmail.com> Date: Thu, 13 Apr 2023 12:20:58 +0200 Subject: [PATCH 13/14] fix linters --- src/web3py/extensions/fallback.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/web3py/extensions/fallback.py b/src/web3py/extensions/fallback.py index 11a12cb67..3a8959d29 100644 --- a/src/web3py/extensions/fallback.py +++ b/src/web3py/extensions/fallback.py @@ -1,4 +1,4 @@ -from typing import Any, Union +from typing import Any from web3_multi_provider import FallbackProvider from src.web3py.extensions.consistency import ProviderConsistencyModule From 444c29d5a06fe3484dfd4f38ac17d77bdae394bf Mon Sep 17 00:00:00 2001 From: F4ever <1590415904a@gmail.com> Date: Thu, 13 Apr 2023 13:21:44 +0200 Subject: [PATCH 14/14] fix variable parsing --- src/variables.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/variables.py b/src/variables.py index 57b684ea6..2d91ed0ce 100644 --- a/src/variables.py +++ b/src/variables.py @@ -25,7 +25,7 @@ # - App specific - LIDO_LOCATOR_ADDRESS = os.getenv('LIDO_LOCATOR_ADDRESS') -FINALIZATION_BATCH_MAX_REQUEST_COUNT = os.getenv('FINALIZATION_BATCH_MAX_REQUEST_COUNT', 1000) +FINALIZATION_BATCH_MAX_REQUEST_COUNT = int(os.getenv('FINALIZATION_BATCH_MAX_REQUEST_COUNT', 1000)) ALLOW_REPORTING_IN_BUNKER_MODE = os.getenv('ALLOW_REPORTING_IN_BUNKER_MODE', 'False').lower() == 'true' # We add some gas to the transaction to be sure that we have enough gas to execute corner cases # eg when we tried to submit a few reports in a single block