From 45dcc317fec7c3e7dc0d6f81383f68661308377e Mon Sep 17 00:00:00 2001 From: derekpierre Date: Tue, 27 Aug 2024 18:35:02 -0400 Subject: [PATCH 01/16] Add redundancy POA error middleware that adds/replaces geth poa middleware if ExtraDataLengthError is encountered at any time. --- nucypher/blockchain/eth/clients.py | 14 ++++++-- nucypher/blockchain/middleware/poa.py | 49 +++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 nucypher/blockchain/middleware/poa.py diff --git a/nucypher/blockchain/eth/clients.py b/nucypher/blockchain/eth/clients.py index 30fb72683..ebc512f7e 100644 --- a/nucypher/blockchain/eth/clients.py +++ b/nucypher/blockchain/eth/clients.py @@ -16,6 +16,7 @@ from nucypher.blockchain.eth.constants import ( POA_CHAINS, PUBLIC_CHAINS, ) +from nucypher.blockchain.middleware.poa import POAErrorRedundancyMiddleware from nucypher.blockchain.middleware.retry import ( AlchemyRetryRequestMiddleware, InfuraRetryRequestMiddleware, @@ -97,13 +98,22 @@ class EthereumClient: chain_id = self.chain_id is_poa = chain_id in POA_CHAINS - self.log.debug( + self.log.info( f"Blockchain: {self.chain_name} (chain_id={chain_id}, poa={is_poa})" ) if is_poa: # proof-of-authority blockchain self.log.info("Injecting POA middleware at layer 0") - self.inject_middleware(geth_poa_middleware, layer=0, name="poa") + self.inject_middleware( + # use naming from redundancy middleware + geth_poa_middleware, + layer=0, + name=POAErrorRedundancyMiddleware.POA_MIDDLEWARE_NAME, + ) + + # POA error redundancy middleware, just in case + self.log.debug("Adding POA redundancy middleware") + self.add_middleware(POAErrorRedundancyMiddleware) # simple cache middleware self.log.debug("Adding simple_cache_middleware") diff --git a/nucypher/blockchain/middleware/poa.py b/nucypher/blockchain/middleware/poa.py new file mode 100644 index 000000000..72c90b20b --- /dev/null +++ b/nucypher/blockchain/middleware/poa.py @@ -0,0 +1,49 @@ +from typing import Any, Callable + +from web3 import Web3 +from web3.exceptions import ExtraDataLengthError +from web3.middleware import geth_poa_middleware +from web3.types import RPCEndpoint, RPCResponse + +from nucypher.utilities.logging import Logger + + +class POAErrorRedundancyMiddleware: + """ + Redundant middleware for replacing already added named poa middleware if ExtraDataLengthError + still encountered. Extra layer of defense in case random POA error is observed + """ + + POA_MIDDLEWARE_NAME = "poa" + + def __init__( + self, + make_request: Callable[[RPCEndpoint, Any], RPCResponse], + w3: Web3, + existing_poa_middleware_name: str = POA_MIDDLEWARE_NAME, + ): + self.w3 = w3 + self.make_request = make_request + self.existing_poa_middleware_name = existing_poa_middleware_name + self.logger = Logger(self.__class__.__name__) + + def __call__(self, method, params) -> RPCResponse: + try: + response = self.make_request(method, params) + except ExtraDataLengthError: + self.logger.warn( + "RPC request failed due to extraData error; re-injecting poa middleware and retrying" + ) + # add / replace existing poa middleware; replacement seems unlikely but just in case + if self.w3.middleware_onion.get(self.existing_poa_middleware_name): + # we can't have > 1 geth_poa_middleware added (event with different names) so the + # removal-then-add is just for safety. + self.w3.middleware_onion.remove(self.existing_poa_middleware_name) + self.w3.middleware_onion.inject( + geth_poa_middleware, layer=0, name=self.existing_poa_middleware_name + ) + + # try again + response = self.make_request(method, params) + + return response From a0c043196906d060c482d876f6ca39c540dd6dc8 Mon Sep 17 00:00:00 2001 From: derekpierre Date: Wed, 28 Aug 2024 09:00:22 -0400 Subject: [PATCH 02/16] Modify how poa error redundancy middleware is created for clarity and better cohesion with providing the name of the original poa middleware used. --- nucypher/blockchain/eth/clients.py | 4 +- nucypher/blockchain/middleware/poa.py | 60 +++++++++++++-------------- 2 files changed, 30 insertions(+), 34 deletions(-) diff --git a/nucypher/blockchain/eth/clients.py b/nucypher/blockchain/eth/clients.py index ebc512f7e..02d2d2fb3 100644 --- a/nucypher/blockchain/eth/clients.py +++ b/nucypher/blockchain/eth/clients.py @@ -16,7 +16,7 @@ from nucypher.blockchain.eth.constants import ( POA_CHAINS, PUBLIC_CHAINS, ) -from nucypher.blockchain.middleware.poa import POAErrorRedundancyMiddleware +from nucypher.blockchain.middleware.poa import create_poa_error_redundancy_middleware from nucypher.blockchain.middleware.retry import ( AlchemyRetryRequestMiddleware, InfuraRetryRequestMiddleware, @@ -113,7 +113,7 @@ class EthereumClient: # POA error redundancy middleware, just in case self.log.debug("Adding POA redundancy middleware") - self.add_middleware(POAErrorRedundancyMiddleware) + self.add_middleware(create_poa_error_redundancy_middleware(existing_poa_middleware_name=poa_middleware_name)) # simple cache middleware self.log.debug("Adding simple_cache_middleware") diff --git a/nucypher/blockchain/middleware/poa.py b/nucypher/blockchain/middleware/poa.py index 72c90b20b..0c4d6401c 100644 --- a/nucypher/blockchain/middleware/poa.py +++ b/nucypher/blockchain/middleware/poa.py @@ -3,47 +3,43 @@ from typing import Any, Callable from web3 import Web3 from web3.exceptions import ExtraDataLengthError from web3.middleware import geth_poa_middleware -from web3.types import RPCEndpoint, RPCResponse +from web3.types import Middleware, RPCEndpoint, RPCResponse from nucypher.utilities.logging import Logger -class POAErrorRedundancyMiddleware: +def create_poa_error_redundancy_middleware(existing_poa_middleware_name: str = "poa") -> Middleware: """ Redundant middleware for replacing already added named poa middleware if ExtraDataLengthError - still encountered. Extra layer of defense in case random POA error is observed + still encountered. Extra layer of defense in case random POA error is observed. """ - POA_MIDDLEWARE_NAME = "poa" + def poa_error_redundancy_middleware( + make_request: Callable[[RPCEndpoint, Any], RPCResponse], _w3: "Web3" + ) -> Callable[[RPCEndpoint, Any], RPCResponse]: + logger = Logger("POAErrorRedundancyMiddleware") - def __init__( - self, - make_request: Callable[[RPCEndpoint, Any], RPCResponse], - w3: Web3, - existing_poa_middleware_name: str = POA_MIDDLEWARE_NAME, - ): - self.w3 = w3 - self.make_request = make_request - self.existing_poa_middleware_name = existing_poa_middleware_name - self.logger = Logger(self.__class__.__name__) + def middleware(method: RPCEndpoint, params: Any) -> RPCResponse: + try: + response = make_request(method, params) + except ExtraDataLengthError: + logger.warn( + "RPC request failed due to extraData error; re-injecting poa middleware and retrying" + ) + # add / replace existing poa middleware; replacement seems unlikely but just in case + if _w3.middleware_onion.get(existing_poa_middleware_name): + # we can't have > 1 geth_poa_middleware added (event with different names) so the + # removal-then-add is just for safety. + _w3.middleware_onion.remove(existing_poa_middleware_name) + _w3.middleware_onion.inject( + geth_poa_middleware, layer=0, name=existing_poa_middleware_name + ) - def __call__(self, method, params) -> RPCResponse: - try: - response = self.make_request(method, params) - except ExtraDataLengthError: - self.logger.warn( - "RPC request failed due to extraData error; re-injecting poa middleware and retrying" - ) - # add / replace existing poa middleware; replacement seems unlikely but just in case - if self.w3.middleware_onion.get(self.existing_poa_middleware_name): - # we can't have > 1 geth_poa_middleware added (event with different names) so the - # removal-then-add is just for safety. - self.w3.middleware_onion.remove(self.existing_poa_middleware_name) - self.w3.middleware_onion.inject( - geth_poa_middleware, layer=0, name=self.existing_poa_middleware_name - ) + # try again + response = make_request(method, params) - # try again - response = self.make_request(method, params) + return response - return response + return middleware + + return poa_error_redundancy_middleware From 8be1f2590ad936f6b1efc16aef5af53121ed1951 Mon Sep 17 00:00:00 2001 From: derekpierre Date: Wed, 28 Aug 2024 09:01:16 -0400 Subject: [PATCH 03/16] Always inject the poa middleware irrespective of chain. --- nucypher/blockchain/eth/clients.py | 18 +++++++++--------- tests/mock/interfaces.py | 3 +++ 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/nucypher/blockchain/eth/clients.py b/nucypher/blockchain/eth/clients.py index 02d2d2fb3..db64fea6d 100644 --- a/nucypher/blockchain/eth/clients.py +++ b/nucypher/blockchain/eth/clients.py @@ -101,16 +101,16 @@ class EthereumClient: self.log.info( f"Blockchain: {self.chain_name} (chain_id={chain_id}, poa={is_poa})" ) - if is_poa: - # proof-of-authority blockchain - self.log.info("Injecting POA middleware at layer 0") - self.inject_middleware( - # use naming from redundancy middleware - geth_poa_middleware, - layer=0, - name=POAErrorRedundancyMiddleware.POA_MIDDLEWARE_NAME, - ) + # proof-of-authority blockchain + poa_middleware_name = "poa" + self.log.info("Injecting POA middleware at layer 0") + self.inject_middleware( + # use naming from redundancy middleware + geth_poa_middleware, + layer=0, + name=poa_middleware_name, + ) # POA error redundancy middleware, just in case self.log.debug("Adding POA redundancy middleware") self.add_middleware(create_poa_error_redundancy_middleware(existing_poa_middleware_name=poa_middleware_name)) diff --git a/tests/mock/interfaces.py b/tests/mock/interfaces.py index 4ee1c4a2c..7bd00fa27 100644 --- a/tests/mock/interfaces.py +++ b/tests/mock/interfaces.py @@ -71,6 +71,9 @@ class MockEthereumClient(EthereumClient): def add_middleware(self, middleware): pass + def inject_middleware(self, middleware, **kwargs): + pass + @property def chain_id(self) -> int: return TESTERCHAIN_CHAIN_ID From 930eaf9b23f2cecd40558d100cba324ed14c71f6 Mon Sep 17 00:00:00 2001 From: derekpierre Date: Wed, 28 Aug 2024 09:03:41 -0400 Subject: [PATCH 04/16] Fix linter errors. --- nucypher/blockchain/eth/clients.py | 6 +++++- nucypher/blockchain/middleware/poa.py | 4 +++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/nucypher/blockchain/eth/clients.py b/nucypher/blockchain/eth/clients.py index db64fea6d..694646061 100644 --- a/nucypher/blockchain/eth/clients.py +++ b/nucypher/blockchain/eth/clients.py @@ -113,7 +113,11 @@ class EthereumClient: ) # POA error redundancy middleware, just in case self.log.debug("Adding POA redundancy middleware") - self.add_middleware(create_poa_error_redundancy_middleware(existing_poa_middleware_name=poa_middleware_name)) + self.add_middleware( + create_poa_error_redundancy_middleware( + existing_poa_middleware_name=poa_middleware_name + ) + ) # simple cache middleware self.log.debug("Adding simple_cache_middleware") diff --git a/nucypher/blockchain/middleware/poa.py b/nucypher/blockchain/middleware/poa.py index 0c4d6401c..4e3df6bba 100644 --- a/nucypher/blockchain/middleware/poa.py +++ b/nucypher/blockchain/middleware/poa.py @@ -8,7 +8,9 @@ from web3.types import Middleware, RPCEndpoint, RPCResponse from nucypher.utilities.logging import Logger -def create_poa_error_redundancy_middleware(existing_poa_middleware_name: str = "poa") -> Middleware: +def create_poa_error_redundancy_middleware( + existing_poa_middleware_name: str = "poa", +) -> Middleware: """ Redundant middleware for replacing already added named poa middleware if ExtraDataLengthError still encountered. Extra layer of defense in case random POA error is observed. From f692b0650f41b422d6673e54b5d730753edb71e6 Mon Sep 17 00:00:00 2001 From: derekpierre Date: Wed, 28 Aug 2024 11:51:52 -0400 Subject: [PATCH 05/16] Always inject poa middleware for Web3 instances used for evaluating RPCConditions (and sub-classes). --- nucypher/blockchain/eth/clients.py | 3 +-- nucypher/policy/conditions/evm.py | 7 +++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/nucypher/blockchain/eth/clients.py b/nucypher/blockchain/eth/clients.py index 694646061..516a989b4 100644 --- a/nucypher/blockchain/eth/clients.py +++ b/nucypher/blockchain/eth/clients.py @@ -102,11 +102,10 @@ class EthereumClient: f"Blockchain: {self.chain_name} (chain_id={chain_id}, poa={is_poa})" ) - # proof-of-authority blockchain + # add POA middleware irrespective of chain poa_middleware_name = "poa" self.log.info("Injecting POA middleware at layer 0") self.inject_middleware( - # use naming from redundancy middleware geth_poa_middleware, layer=0, name=poa_middleware_name, diff --git a/nucypher/policy/conditions/evm.py b/nucypher/policy/conditions/evm.py index f9505c03a..bd395ea96 100644 --- a/nucypher/policy/conditions/evm.py +++ b/nucypher/policy/conditions/evm.py @@ -18,7 +18,6 @@ from web3.middleware import geth_poa_middleware from web3.providers import BaseProvider from web3.types import ABIFunction -from nucypher.blockchain.eth.constants import POA_CHAINS from nucypher.policy.conditions import STANDARD_ABI_CONTRACT_TYPES, STANDARD_ABIS from nucypher.policy.conditions.base import AccessControlCondition from nucypher.policy.conditions.context import ( @@ -211,9 +210,9 @@ class RPCCondition(AccessControlCondition): # Instantiate a local web3 instance self.provider = provider w3 = Web3(provider) - if self.chain in POA_CHAINS: - # inject web3 middleware to handle POA chain extra_data field. - w3.middleware_onion.inject(geth_poa_middleware, layer=0) + # inject web3 middleware to handle POA chain extra_data field. + w3.middleware_onion.inject(geth_poa_middleware, layer=0, name="poa") + return w3 def _check_chain_id(self) -> None: From 2ec5680a1f66391ce99b57e8ba36b857bb89e668 Mon Sep 17 00:00:00 2001 From: derekpierre Date: Wed, 28 Aug 2024 11:52:37 -0400 Subject: [PATCH 06/16] Always name the middlewares we add to web3 middleware onion. --- nucypher/blockchain/eth/clients.py | 12 ++++++------ tests/mock/interfaces.py | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/nucypher/blockchain/eth/clients.py b/nucypher/blockchain/eth/clients.py index 516a989b4..f0f2b864d 100644 --- a/nucypher/blockchain/eth/clients.py +++ b/nucypher/blockchain/eth/clients.py @@ -86,13 +86,13 @@ class EthereumClient: endpoint_uri = getattr(self.w3.provider, "endpoint_uri", "") if "infura" in endpoint_uri: self.log.debug("Adding Infura RPC retry middleware to client") - self.add_middleware(InfuraRetryRequestMiddleware) + self.add_middleware(InfuraRetryRequestMiddleware, name="infura_retry") elif "alchemyapi.io" in endpoint_uri: self.log.debug("Adding Alchemy RPC retry middleware to client") - self.add_middleware(AlchemyRetryRequestMiddleware) + self.add_middleware(AlchemyRetryRequestMiddleware, name="alchemy_retry") else: self.log.debug("Adding RPC retry middleware to client") - self.add_middleware(RetryRequestMiddleware) + self.add_middleware(RetryRequestMiddleware, name="retry") # poa middleware chain_id = self.chain_id @@ -120,7 +120,7 @@ class EthereumClient: # simple cache middleware self.log.debug("Adding simple_cache_middleware") - self.add_middleware(simple_cache_middleware) + self.add_middleware(simple_cache_middleware, name="simple_cache") @property def chain_name(self) -> str: @@ -141,8 +141,8 @@ class EthereumClient: def inject_middleware(self, middleware, **kwargs): self.w3.middleware_onion.inject(middleware, **kwargs) - def add_middleware(self, middleware): - self.w3.middleware_onion.add(middleware) + def add_middleware(self, middleware, **kwargs): + self.w3.middleware_onion.add(middleware, **kwargs) def set_gas_strategy(self, gas_strategy): self.w3.eth.set_gas_price_strategy(gas_strategy) diff --git a/tests/mock/interfaces.py b/tests/mock/interfaces.py index 7bd00fa27..10a98eff7 100644 --- a/tests/mock/interfaces.py +++ b/tests/mock/interfaces.py @@ -68,7 +68,7 @@ class MockEthereumClient(EthereumClient): def __init__(self, w3): super().__init__(w3=w3) - def add_middleware(self, middleware): + def add_middleware(self, middleware, **kwargs): pass def inject_middleware(self, middleware, **kwargs): From eb7ecc844ddba008c19dcabda2f230c918808352 Mon Sep 17 00:00:00 2001 From: derekpierre Date: Wed, 28 Aug 2024 18:30:05 -0400 Subject: [PATCH 07/16] Add test for POA error redundancy middleware. --- .../network/test_poa_redundancy_middleware.py | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 tests/integration/network/test_poa_redundancy_middleware.py diff --git a/tests/integration/network/test_poa_redundancy_middleware.py b/tests/integration/network/test_poa_redundancy_middleware.py new file mode 100644 index 000000000..fd590ed77 --- /dev/null +++ b/tests/integration/network/test_poa_redundancy_middleware.py @@ -0,0 +1,58 @@ +from unittest.mock import ANY, Mock + +from web3.exceptions import ExtraDataLengthError +from web3.middleware import geth_poa_middleware +from web3.types import RPCEndpoint, RPCResponse + +from nucypher.blockchain.middleware.poa import create_poa_error_redundancy_middleware + + +def test_request_with_poa_issues(): + make_request = Mock() + w3 = Mock() + middleware_onion = Mock() + w3.middleware_onion = middleware_onion + + poa_name = "poa_test" + + poa_redundancy_middleware = create_poa_error_redundancy_middleware( + existing_poa_middleware_name=poa_name + ) + + valid_response = RPCResponse( + jsonrpc="2.0", id=1, result="Geth/v1.14.8-stable-a9523b64/linux-amd64/go1.22.6" + ) + + redundant_middleware = poa_redundancy_middleware(make_request, w3) + + # 1. no poa error, simply return response + make_request.side_effect = [valid_response] + response = redundant_middleware(RPCEndpoint("web3_clientVersion"), None) + + assert response == valid_response + middleware_onion.get.assert_not_called() + middleware_onion.remove.assert_not_called() + middleware_onion.inject.assert_not_called() + + # 2. poa error; no prior poa middleware + make_request.side_effect = [ExtraDataLengthError(), valid_response] + middleware_onion.get.return_value = None + + response = redundant_middleware(RPCEndpoint("web3_clientVersion"), None) + + assert response == valid_response + middleware_onion.get.assert_called_once_with(poa_name) + middleware_onion.remove.assert_not_called() + middleware_onion.inject.assert_called_once_with(ANY, layer=0, name=poa_name) + + # 3. poa error; prior poa middleware + make_request.side_effect = [ExtraDataLengthError(), valid_response] + middleware_onion.get.return_value = geth_poa_middleware + response = redundant_middleware(RPCEndpoint("web3_clientVersion"), None) + + assert response == valid_response + assert middleware_onion.get.call_count == 2 + middleware_onion.get.assert_called_with(poa_name) + middleware_onion.remove.assert_called_once_with(poa_name) + assert middleware_onion.inject.call_count == 2 + middleware_onion.inject.assert_called_with(ANY, layer=0, name=poa_name) From 43e0dcb941d53afb5ce2a8f69027c5475caa9e21 Mon Sep 17 00:00:00 2001 From: derekpierre Date: Wed, 28 Aug 2024 20:00:53 -0400 Subject: [PATCH 08/16] Make a call to eth.get_block after establishing an EthereumClient with middlewares to ensure that poa middleware, if needed, works correctly. --- nucypher/blockchain/eth/clients.py | 9 --------- nucypher/blockchain/eth/interfaces.py | 7 +++++++ tests/unit/test_web3_clients.py | 7 +++++-- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/nucypher/blockchain/eth/clients.py b/nucypher/blockchain/eth/clients.py index f0f2b864d..0a2fae3bf 100644 --- a/nucypher/blockchain/eth/clients.py +++ b/nucypher/blockchain/eth/clients.py @@ -13,7 +13,6 @@ from web3.types import TxReceipt, Wei from nucypher.blockchain.eth.constants import ( AVERAGE_BLOCK_TIME_IN_SECONDS, - POA_CHAINS, PUBLIC_CHAINS, ) from nucypher.blockchain.middleware.poa import create_poa_error_redundancy_middleware @@ -94,14 +93,6 @@ class EthereumClient: self.log.debug("Adding RPC retry middleware to client") self.add_middleware(RetryRequestMiddleware, name="retry") - # poa middleware - chain_id = self.chain_id - is_poa = chain_id in POA_CHAINS - - self.log.info( - f"Blockchain: {self.chain_name} (chain_id={chain_id}, poa={is_poa})" - ) - # add POA middleware irrespective of chain poa_middleware_name = "poa" self.log.info("Injecting POA middleware at layer 0") diff --git a/nucypher/blockchain/eth/interfaces.py b/nucypher/blockchain/eth/interfaces.py index b8ff3449e..fa1830a2d 100644 --- a/nucypher/blockchain/eth/interfaces.py +++ b/nucypher/blockchain/eth/interfaces.py @@ -331,6 +331,13 @@ class BlockchainInterface: # client mutates w3 instance (configures middleware etc.) self.client = EthereumClient(w3=self.w3) + # log info + latest_block_number = self.client.get_block("latest")["number"] + chain_id = self.client.chain_id + self.log.info( + f"Blockchain: {self.client.chain_name} (chain_id={chain_id}, block_num={latest_block_number})" + ) + # web3 instance fully configured; share instance with ATxM and respective strategies speedup_strategy = ExponentialSpeedupStrategy( w3=self.w3, diff --git a/tests/unit/test_web3_clients.py b/tests/unit/test_web3_clients.py index 548a16661..e1845308e 100644 --- a/tests/unit/test_web3_clients.py +++ b/tests/unit/test_web3_clients.py @@ -51,9 +51,12 @@ class SyncedMockW3Eth: chain_id = hex(CHAIN_ID) block_number = 5 - def getBlock(self, blockNumber): + def get_block(self, blockNumber): return { - 'timestamp': datetime.datetime.timestamp(datetime.datetime.now() - datetime.timedelta(seconds=25)) + "timestamp": datetime.datetime.timestamp( + datetime.datetime.now() - datetime.timedelta(seconds=25) + ), + "number": 123456789, } From 2158136c6f10fcd8c469bcb3ed8b0c2546bcbb2b Mon Sep 17 00:00:00 2001 From: derekpierre Date: Tue, 3 Sep 2024 15:20:42 -0400 Subject: [PATCH 09/16] BlockchainInterface.connect() should only ever be called once if successful; blips in connectivity should not cause it to be called again. Blips in connectivity can cause resetting of underlying w3, client instances, and missing poa middleware if chain_id cannot be determined. Co-authored-by: KPrasch --- nucypher/blockchain/eth/clients.py | 4 --- nucypher/blockchain/eth/interfaces.py | 40 +++++++++++++++------------ 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/nucypher/blockchain/eth/clients.py b/nucypher/blockchain/eth/clients.py index 0a2fae3bf..076549515 100644 --- a/nucypher/blockchain/eth/clients.py +++ b/nucypher/blockchain/eth/clients.py @@ -118,10 +118,6 @@ class EthereumClient: name = PUBLIC_CHAINS.get(self.chain_id, UNKNOWN_DEVELOPMENT_CHAIN_ID) return name - @property - def is_connected(self): - return self.w3.is_connected() - @property def accounts(self): return self.w3.eth.accounts diff --git a/nucypher/blockchain/eth/interfaces.py b/nucypher/blockchain/eth/interfaces.py index fa1830a2d..8fb1cff63 100644 --- a/nucypher/blockchain/eth/interfaces.py +++ b/nucypher/blockchain/eth/interfaces.py @@ -253,6 +253,8 @@ class BlockchainInterface: self.gas_strategy = gas_strategy or self.DEFAULT_GAS_STRATEGY self.max_gas_price = max_gas_price + self.__is_connected = False + def __repr__(self): r = "{name}({uri})".format(name=self.__class__.__name__, uri=self.endpoint) return r @@ -262,12 +264,7 @@ class BlockchainInterface: @property def is_connected(self) -> bool: - """ - https://web3py.readthedocs.io/en/stable/__provider.html#examples-using-automated-detection - """ - if self.client is NO_BLOCKCHAIN_CONNECTION: - return False - return self.client.is_connected + return self.__is_connected @classmethod def get_gas_strategy(cls, gas_strategy: Union[str, Callable] = None) -> Callable: @@ -310,7 +307,7 @@ class BlockchainInterface: # self.log.debug(f"Gas strategy currently reports a gas price of {gwei_gas_price} gwei.") def connect(self): - if self.is_connected: + if self.__is_connected: # safety check - connect was already previously called return @@ -327,24 +324,21 @@ class BlockchainInterface: raise self.NoProvider("There are no configured blockchain providers") try: - self.w3 = self.Web3(provider=self._provider) + w3 = self.Web3(provider=self._provider) # client mutates w3 instance (configures middleware etc.) - self.client = EthereumClient(w3=self.w3) + client = EthereumClient(w3=w3) # log info - latest_block_number = self.client.get_block("latest")["number"] - chain_id = self.client.chain_id - self.log.info( - f"Blockchain: {self.client.chain_name} (chain_id={chain_id}, block_num={latest_block_number})" - ) + latest_block_number = client.get_block("latest")["number"] + chain_id = client.chain_id # web3 instance fully configured; share instance with ATxM and respective strategies speedup_strategy = ExponentialSpeedupStrategy( - w3=self.w3, + w3=w3, min_time_between_speedups=120, ) # speedup txs if not mined after 2 mins. - self.tx_machine = AutomaticTxMachine( - w3=self.w3, tx_exec_timeout=self.TIMEOUT, strategies=[speedup_strategy] + tx_machine = AutomaticTxMachine( + w3=w3, tx_exec_timeout=self.TIMEOUT, strategies=[speedup_strategy] ) except requests.ConnectionError: # RPC raise self.ConnectionFailed( @@ -355,7 +349,17 @@ class BlockchainInterface: f"Connection Failed - {str(self.endpoint)} - is IPC enabled?" ) - return self.is_connected + # Only set member variables once early set up is successful + # - prevents incomplete instantiations + self.w3 = w3 + self.client = client + self.tx_machine = tx_machine + self.log.info( + f"Blockchain: {client.chain_name} (chain_id={chain_id}, block_num={latest_block_number})" + ) + + self.__is_connected = True + return self.__is_connected @property def provider(self) -> BaseProvider: From 258ebef8efdc72991113e11e07a3653be9ba0a39 Mon Sep 17 00:00:00 2001 From: derekpierre Date: Tue, 3 Sep 2024 17:16:56 -0400 Subject: [PATCH 10/16] Remove poa error redundancy middleware now that we understand the underlying cause of the problem. It was always a stop gap measure until we could determine the exact cause of the poa error; Now that we know the error it is no longer needed. --- nucypher/blockchain/eth/clients.py | 8 --- nucypher/blockchain/middleware/poa.py | 47 --------------- .../network/test_poa_redundancy_middleware.py | 58 ------------------- 3 files changed, 113 deletions(-) delete mode 100644 nucypher/blockchain/middleware/poa.py delete mode 100644 tests/integration/network/test_poa_redundancy_middleware.py diff --git a/nucypher/blockchain/eth/clients.py b/nucypher/blockchain/eth/clients.py index 076549515..a7a3e27e3 100644 --- a/nucypher/blockchain/eth/clients.py +++ b/nucypher/blockchain/eth/clients.py @@ -15,7 +15,6 @@ from nucypher.blockchain.eth.constants import ( AVERAGE_BLOCK_TIME_IN_SECONDS, PUBLIC_CHAINS, ) -from nucypher.blockchain.middleware.poa import create_poa_error_redundancy_middleware from nucypher.blockchain.middleware.retry import ( AlchemyRetryRequestMiddleware, InfuraRetryRequestMiddleware, @@ -101,13 +100,6 @@ class EthereumClient: layer=0, name=poa_middleware_name, ) - # POA error redundancy middleware, just in case - self.log.debug("Adding POA redundancy middleware") - self.add_middleware( - create_poa_error_redundancy_middleware( - existing_poa_middleware_name=poa_middleware_name - ) - ) # simple cache middleware self.log.debug("Adding simple_cache_middleware") diff --git a/nucypher/blockchain/middleware/poa.py b/nucypher/blockchain/middleware/poa.py deleted file mode 100644 index 4e3df6bba..000000000 --- a/nucypher/blockchain/middleware/poa.py +++ /dev/null @@ -1,47 +0,0 @@ -from typing import Any, Callable - -from web3 import Web3 -from web3.exceptions import ExtraDataLengthError -from web3.middleware import geth_poa_middleware -from web3.types import Middleware, RPCEndpoint, RPCResponse - -from nucypher.utilities.logging import Logger - - -def create_poa_error_redundancy_middleware( - existing_poa_middleware_name: str = "poa", -) -> Middleware: - """ - Redundant middleware for replacing already added named poa middleware if ExtraDataLengthError - still encountered. Extra layer of defense in case random POA error is observed. - """ - - def poa_error_redundancy_middleware( - make_request: Callable[[RPCEndpoint, Any], RPCResponse], _w3: "Web3" - ) -> Callable[[RPCEndpoint, Any], RPCResponse]: - logger = Logger("POAErrorRedundancyMiddleware") - - def middleware(method: RPCEndpoint, params: Any) -> RPCResponse: - try: - response = make_request(method, params) - except ExtraDataLengthError: - logger.warn( - "RPC request failed due to extraData error; re-injecting poa middleware and retrying" - ) - # add / replace existing poa middleware; replacement seems unlikely but just in case - if _w3.middleware_onion.get(existing_poa_middleware_name): - # we can't have > 1 geth_poa_middleware added (event with different names) so the - # removal-then-add is just for safety. - _w3.middleware_onion.remove(existing_poa_middleware_name) - _w3.middleware_onion.inject( - geth_poa_middleware, layer=0, name=existing_poa_middleware_name - ) - - # try again - response = make_request(method, params) - - return response - - return middleware - - return poa_error_redundancy_middleware diff --git a/tests/integration/network/test_poa_redundancy_middleware.py b/tests/integration/network/test_poa_redundancy_middleware.py deleted file mode 100644 index fd590ed77..000000000 --- a/tests/integration/network/test_poa_redundancy_middleware.py +++ /dev/null @@ -1,58 +0,0 @@ -from unittest.mock import ANY, Mock - -from web3.exceptions import ExtraDataLengthError -from web3.middleware import geth_poa_middleware -from web3.types import RPCEndpoint, RPCResponse - -from nucypher.blockchain.middleware.poa import create_poa_error_redundancy_middleware - - -def test_request_with_poa_issues(): - make_request = Mock() - w3 = Mock() - middleware_onion = Mock() - w3.middleware_onion = middleware_onion - - poa_name = "poa_test" - - poa_redundancy_middleware = create_poa_error_redundancy_middleware( - existing_poa_middleware_name=poa_name - ) - - valid_response = RPCResponse( - jsonrpc="2.0", id=1, result="Geth/v1.14.8-stable-a9523b64/linux-amd64/go1.22.6" - ) - - redundant_middleware = poa_redundancy_middleware(make_request, w3) - - # 1. no poa error, simply return response - make_request.side_effect = [valid_response] - response = redundant_middleware(RPCEndpoint("web3_clientVersion"), None) - - assert response == valid_response - middleware_onion.get.assert_not_called() - middleware_onion.remove.assert_not_called() - middleware_onion.inject.assert_not_called() - - # 2. poa error; no prior poa middleware - make_request.side_effect = [ExtraDataLengthError(), valid_response] - middleware_onion.get.return_value = None - - response = redundant_middleware(RPCEndpoint("web3_clientVersion"), None) - - assert response == valid_response - middleware_onion.get.assert_called_once_with(poa_name) - middleware_onion.remove.assert_not_called() - middleware_onion.inject.assert_called_once_with(ANY, layer=0, name=poa_name) - - # 3. poa error; prior poa middleware - make_request.side_effect = [ExtraDataLengthError(), valid_response] - middleware_onion.get.return_value = geth_poa_middleware - response = redundant_middleware(RPCEndpoint("web3_clientVersion"), None) - - assert response == valid_response - assert middleware_onion.get.call_count == 2 - middleware_onion.get.assert_called_with(poa_name) - middleware_onion.remove.assert_called_once_with(poa_name) - assert middleware_onion.inject.call_count == 2 - middleware_onion.inject.assert_called_with(ANY, layer=0, name=poa_name) From bff74b7a25bb4998ba865175dfd1ac16e64e1d07 Mon Sep 17 00:00:00 2001 From: derekpierre Date: Tue, 3 Sep 2024 20:10:32 -0400 Subject: [PATCH 11/16] Add regression test that ensures that BlockchainInterface.connect() only sets its underling instances once after successful execution. --- tests/unit/test_blockchain_interface.py | 71 ++++++++++++++++++++++++- 1 file changed, 70 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_blockchain_interface.py b/tests/unit/test_blockchain_interface.py index 9c2f9f5ca..af724569c 100644 --- a/tests/unit/test_blockchain_interface.py +++ b/tests/unit/test_blockchain_interface.py @@ -1,6 +1,9 @@ - +from typing import Optional +from unittest.mock import PropertyMock from constant_sorrow.constants import ALL_OF_THEM +from requests import HTTPError +from web3 import BaseProvider from web3.gas_strategies import time_based from nucypher.blockchain.eth.interfaces import BlockchainInterface @@ -99,3 +102,69 @@ def test_use_pending_nonce_when_building_payload(mock_testerchain, mocker, rando assert payload['nonce'] == 6 payload = mock_testerchain.build_payload(sender_address=sender, payload=None, use_pending_nonce=False) assert payload['nonce'] == 6 + + +def test_connect_handle_connectivity_issues(mocker): + + mock_eth = mocker.MagicMock() + type(mock_eth).chain_id = PropertyMock(return_value=137) + + mock_middleware_onion = mocker.Mock() + + class MockWeb3: + def __init__(self, provider: Optional[BaseProvider] = None, *args, **kwargs): + self.provider = provider + self.eth = mock_eth + self.middleware_onion = mock_middleware_onion + + middlewares = [] + self.middleware_onion.middlewares = middlewares + + def add_middleware(middleware, name=None): + middlewares.append(middleware) + + def inject_middleware(middleware, layer=0, name=None): + middlewares.insert(layer, middleware) + + mock_middleware_onion.add.side_effect = add_middleware + mock_middleware_onion.inject.side_effect = inject_middleware + + class TestBlockchainInterface(BlockchainInterface): + Web3 = MockWeb3 + + blockchain_interface = TestBlockchainInterface( + endpoint="https://public-node.io:8445" + ) + + assert not blockchain_interface.is_connected + + # connect() is called with no connectivity issues and executes successfully + blockchain_interface.connect() + assert blockchain_interface.is_connected + + # poa, retry, simplecache + current_middlewares = blockchain_interface.w3.middleware_onion.middlewares + assert len(current_middlewares) == 3 + + w3 = blockchain_interface.w3 + client = blockchain_interface.client + tx_machine = blockchain_interface.tx_machine + + # mimic connectivity issues + type(mock_eth).chain_id = PropertyMock(side_effect=HTTPError("connectivity issue")) + + # Mimic scanner task that connectivity experienced exception and ran connect() + # again on blockchain interface. + # However, connect() does nothing the 2nd time around because it already completed + # successfully the first time + blockchain_interface.connect() + + # no change; + # same underlying instances + assert w3 == blockchain_interface.w3 + assert client == blockchain_interface.client + assert tx_machine == blockchain_interface.tx_machine + + # same middlewares remain - poa, retry, simplecache + assert len(blockchain_interface.w3.middleware_onion.middlewares) == 3 + assert blockchain_interface.w3.middleware_onion.middlewares == current_middlewares From 85aaa2916301dbcab6af6ce08681c28d89975a8f Mon Sep 17 00:00:00 2001 From: derekpierre Date: Tue, 3 Sep 2024 20:40:06 -0400 Subject: [PATCH 12/16] Add newsfragment from #3549. --- newsfragments/3549.bugfix.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 newsfragments/3549.bugfix.rst diff --git a/newsfragments/3549.bugfix.rst b/newsfragments/3549.bugfix.rst new file mode 100644 index 000000000..412eb3491 --- /dev/null +++ b/newsfragments/3549.bugfix.rst @@ -0,0 +1,2 @@ +Prevent connectivity issues from improperly re-initializing underlying instances of ``EthereumClient`` +and ``Web3 within a ``BlockchainInterface`` instance. From 3b0710d6115ee062c3a313cd61d8fdc9ee1cae9f Mon Sep 17 00:00:00 2001 From: KPrasch Date: Wed, 4 Sep 2024 12:51:25 +0200 Subject: [PATCH 13/16] Update newsfragments/3549.bugfix.rst MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: David Núñez --- newsfragments/3549.bugfix.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/newsfragments/3549.bugfix.rst b/newsfragments/3549.bugfix.rst index 412eb3491..4f177a991 100644 --- a/newsfragments/3549.bugfix.rst +++ b/newsfragments/3549.bugfix.rst @@ -1,2 +1,2 @@ Prevent connectivity issues from improperly re-initializing underlying instances of ``EthereumClient`` -and ``Web3 within a ``BlockchainInterface`` instance. +and ``Web3`` within a ``BlockchainInterface`` instance. From 646e2f5b11fc646297731d427201aa5dcbb8e4e5 Mon Sep 17 00:00:00 2001 From: derekpierre Date: Wed, 4 Sep 2024 07:55:35 -0400 Subject: [PATCH 14/16] Remove is_connected in favour of is_initialized for clarity. --- nucypher/blockchain/eth/interfaces.py | 14 +++++++------- tests/unit/test_blockchain_interface.py | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/nucypher/blockchain/eth/interfaces.py b/nucypher/blockchain/eth/interfaces.py index 8fb1cff63..d6dbf60d5 100644 --- a/nucypher/blockchain/eth/interfaces.py +++ b/nucypher/blockchain/eth/interfaces.py @@ -253,7 +253,7 @@ class BlockchainInterface: self.gas_strategy = gas_strategy or self.DEFAULT_GAS_STRATEGY self.max_gas_price = max_gas_price - self.__is_connected = False + self.__is_initialized = False def __repr__(self): r = "{name}({uri})".format(name=self.__class__.__name__, uri=self.endpoint) @@ -263,8 +263,8 @@ class BlockchainInterface: return self.client.get_blocktime() @property - def is_connected(self) -> bool: - return self.__is_connected + def is_initialized(self) -> bool: + return self.__is_initialized @classmethod def get_gas_strategy(cls, gas_strategy: Union[str, Callable] = None) -> Callable: @@ -307,7 +307,7 @@ class BlockchainInterface: # self.log.debug(f"Gas strategy currently reports a gas price of {gwei_gas_price} gwei.") def connect(self): - if self.__is_connected: + if self.__is_initialized: # safety check - connect was already previously called return @@ -358,8 +358,8 @@ class BlockchainInterface: f"Blockchain: {client.chain_name} (chain_id={chain_id}, block_num={latest_block_number})" ) - self.__is_connected = True - return self.__is_connected + self.__is_initialized = True + return self.__is_initialized @property def provider(self) -> BaseProvider: @@ -844,7 +844,7 @@ class BlockchainInterfaceFactory: # Connect and Sync interface, emitter = cached_interface - if not interface.is_connected: + if not interface.is_initialized: interface.connect() return interface diff --git a/tests/unit/test_blockchain_interface.py b/tests/unit/test_blockchain_interface.py index af724569c..ef0bc3829 100644 --- a/tests/unit/test_blockchain_interface.py +++ b/tests/unit/test_blockchain_interface.py @@ -136,11 +136,11 @@ def test_connect_handle_connectivity_issues(mocker): endpoint="https://public-node.io:8445" ) - assert not blockchain_interface.is_connected + assert not blockchain_interface.is_initialized # connect() is called with no connectivity issues and executes successfully blockchain_interface.connect() - assert blockchain_interface.is_connected + assert blockchain_interface.is_initialized # poa, retry, simplecache current_middlewares = blockchain_interface.w3.middleware_onion.middlewares From b22fd0fcb58f68d058a30879b8ebcd491b0831ba Mon Sep 17 00:00:00 2001 From: derekpierre Date: Wed, 4 Sep 2024 07:58:07 -0400 Subject: [PATCH 15/16] Messages for logging addition of middleware now logged at info level so that we can always observe them in the logs. --- nucypher/blockchain/eth/clients.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/nucypher/blockchain/eth/clients.py b/nucypher/blockchain/eth/clients.py index a7a3e27e3..85101e11e 100644 --- a/nucypher/blockchain/eth/clients.py +++ b/nucypher/blockchain/eth/clients.py @@ -83,13 +83,13 @@ class EthereumClient: # retry request middleware endpoint_uri = getattr(self.w3.provider, "endpoint_uri", "") if "infura" in endpoint_uri: - self.log.debug("Adding Infura RPC retry middleware to client") + self.log.info("Adding Infura RPC retry middleware to client") self.add_middleware(InfuraRetryRequestMiddleware, name="infura_retry") elif "alchemyapi.io" in endpoint_uri: - self.log.debug("Adding Alchemy RPC retry middleware to client") + self.log.info("Adding Alchemy RPC retry middleware to client") self.add_middleware(AlchemyRetryRequestMiddleware, name="alchemy_retry") else: - self.log.debug("Adding RPC retry middleware to client") + self.log.info("Adding RPC retry middleware to client") self.add_middleware(RetryRequestMiddleware, name="retry") # add POA middleware irrespective of chain @@ -102,7 +102,7 @@ class EthereumClient: ) # simple cache middleware - self.log.debug("Adding simple_cache_middleware") + self.log.info("Adding simple_cache_middleware") self.add_middleware(simple_cache_middleware, name="simple_cache") @property From 5892b67d235b278f1b47a6e5e4858069c16f6aba Mon Sep 17 00:00:00 2001 From: derekpierre Date: Wed, 4 Sep 2024 09:15:19 -0400 Subject: [PATCH 16/16] Modify order of adding middlewares, poa first (layer 0), then retry, then simple cache. --- nucypher/blockchain/eth/clients.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/nucypher/blockchain/eth/clients.py b/nucypher/blockchain/eth/clients.py index 85101e11e..10c14ba57 100644 --- a/nucypher/blockchain/eth/clients.py +++ b/nucypher/blockchain/eth/clients.py @@ -80,6 +80,15 @@ class EthereumClient: self._add_default_middleware() def _add_default_middleware(self): + # add POA middleware irrespective of chain + poa_middleware_name = "poa" + self.log.info("Injecting POA middleware at layer 0") + self.inject_middleware( + geth_poa_middleware, + layer=0, + name=poa_middleware_name, + ) + # retry request middleware endpoint_uri = getattr(self.w3.provider, "endpoint_uri", "") if "infura" in endpoint_uri: @@ -92,15 +101,6 @@ class EthereumClient: self.log.info("Adding RPC retry middleware to client") self.add_middleware(RetryRequestMiddleware, name="retry") - # add POA middleware irrespective of chain - poa_middleware_name = "poa" - self.log.info("Injecting POA middleware at layer 0") - self.inject_middleware( - geth_poa_middleware, - layer=0, - name=poa_middleware_name, - ) - # simple cache middleware self.log.info("Adding simple_cache_middleware") self.add_middleware(simple_cache_middleware, name="simple_cache")