Better out of gas handlign when broadcasting (#181)

- Fix: `HotWallet.sign_bound_call_with_new_nonce` tries to avoid calling broken Web3 gas estimation machine if the gas parameters are already given as the arguments - Fix: Raise `OutOfGasFunds` in `_broadcast_multiple_nodes` and avoid transaction broadcast retry if we do not have gas money - Fix: Don't swallow nonce errors and chain id errors in `broadcast_multiple_nodes`
tradingstrategy-ai · Dec 12, 2023 · d8b6271 · d8b6271
1 parent 1ae789e
commit d8b6271
Show file tree

Hide file tree

Showing 18 changed files with 209 additions and 64 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,10 @@
 # Current
 
+- Fix: `HotWallet.sign_bound_call_with_new_nonce` tries to avoid calling broken Web3 gas estimation
+  machine if the gas parameters are already given as the arguments
+- Fix: Raise `OutOfGasFunds` in `_broadcast_multiple_nodes` and
+  avoid transaction broadcast retry if we do not have gas money
+- Fix: Don't swallow nonce errors and chain id errors in `broadcast_multiple_nodes`
 - Fix type normalisation of `tx_hash` in `fetch_transaction_revert_reason`
 
 # 0.24.4

diff --git a/eth_defi/confirmation.py b/eth_defi/confirmation.py
@@ -8,6 +8,7 @@
 import datetime
 import logging
 import time
+from _decimal import Decimal
 from typing import Dict, List, Set, Union, cast, Collection, TypeAlias
 
 from eth_account.datastructures import SignedTransaction
@@ -39,10 +40,26 @@ class ConfirmationTimedOut(Exception):
     """We exceeded the transaction confirmation timeout."""
 
 
+class NonRetryableBroadcastException(Exception):
+    """Don't try to rebroadcast these."""
+
+
 class NonceMismatch(Exception):
     """Chain has a different nonce than we expect."""
 
 
+class OutOfGasFunds(NonRetryableBroadcastException):
+    """Out of gas funds for an executor."""
+
+
+class NonceTooLow(NonRetryableBroadcastException):
+    """Out of gas funds for an executor."""
+
+
+class BadChainId(NonRetryableBroadcastException):
+    """Out of gas funds for an executor."""
+
+
 def wait_transactions_to_complete(
     web3: Web3,
     txs: List[Union[HexBytes, str]],
@@ -367,9 +384,34 @@ def _broadcast_multiple_nodes(providers: Collection[BaseProvider], signed_tx: Si
             logger.info("Source: %s", source)
 
             # When we rebroadcast we are getting nonce too low errors,
-            # both for too high and too low nonces
+            # both for too high and too low nonces.
+            # We also get nonce too low errors,
+            # when broadcasting through multiple nodes and those nodes sync nonce faster than we broadcast
             if resp_data["message"] == "nonce too low":
-                continue
+                if address:
+                    current_nonce = web3.eth.get_transaction_count(address)
+                else:
+                    current_nonce = None
+
+                logger.info("Nonce too low. Current:%s proposed:%s address:%s: tx:%s resp:%s", current_nonce, nonce, address, signed_tx, resp_data)
+                #raise NonceTooLow(f"Current on-chain nonce {current_nonce}, proposed {nonce}") from e
+
+            if "invalid chain" in resp_data["message"]:
+                # Invalid chain id / chain id missing.
+                # Cannot retry.
+                logger.warning("Invalid chain: %s %s", signed_tx, resp_data)
+                raise BadChainId() from e
+
+            if "insufficient funds for gas" in resp_data["message"]:
+                logger.warning("Out of balance error. Tx: %s, resp: %s", signed_tx, resp_data)
+                # Always raise when we are out of funds,
+                # because any retry is not help
+                if address:
+                    our_balance = web3.eth.get_balance(address)
+                    our_balance = Decimal(our_balance) / Decimal(10**18)
+                else:
+                    our_balance = None
+                raise OutOfGasFunds(f"Failed to broadcast {tx_hash}, out of gas, account {address} balance is {our_balance}.\n" f"TX details: {signed_tx}") from e
 
         except Exception as e:
             exceptions[p] = e
@@ -484,6 +526,10 @@ def wait_and_broadcast_multiple_nodes(
 
         In the case of multiple exceptions, the last one is raised.
         The exception is whatever lower stack is giving us.
+
+    :raise OutOfGasFunds:
+        The hot wallet account does not have enough native token to cover the tx fees.
+
     """
 
     assert isinstance(poll_delay, datetime.timedelta)
@@ -538,6 +584,9 @@ def wait_and_broadcast_multiple_nodes(
         try:
             _broadcast_multiple_nodes(providers, tx)
             last_exception = None
+        except NonRetryableBroadcastException:
+            # Don't try to handle
+            raise
         except Exception as e:
             last_exception = e
 
@@ -580,7 +629,6 @@ def wait_and_broadcast_multiple_nodes(
         unconfirmed_txs -= confirmation_received
 
         if unconfirmed_txs:
-
             # TODO: Clean this up after the root cause with Anvil is figured out
             if mine_blocks:
                 timestamp = get_latest_block_timestamp(web3)
@@ -611,9 +659,7 @@ def wait_and_broadcast_multiple_nodes(
                         logger.exception(e)
 
                 unconfirmed_tx_strs = ", ".join([tx_hash.hex() for tx_hash in unconfirmed_txs])
-                raise ConfirmationTimedOut(
-                    f"Transaction confirmation failed. Started: {started_at}, timed out after {max_timeout} ({max_timeout.total_seconds()}s). Poll delay: {poll_delay.total_seconds()}s. Still unconfirmed: {unconfirmed_tx_strs}"
-                )
+                raise ConfirmationTimedOut(f"Transaction confirmation failed. Started: {started_at}, timed out after {max_timeout} ({max_timeout.total_seconds()}s). Poll delay: {poll_delay.total_seconds()}s. Still unconfirmed: {unconfirmed_tx_strs}")
 
         if datetime.datetime.utcnow() >= next_node_switch:
             # Check if it time to try a better node provider

diff --git a/eth_defi/event_reader/lazy_timestamp_reader.py b/eth_defi/event_reader/lazy_timestamp_reader.py
@@ -192,4 +192,4 @@ def on_block_data(self, block_hash, block_number, timestamp):
         self.count += 1
 
     def get_count(self) -> int:
-        return self.count
+        return self.count
diff --git a/eth_defi/event_reader/multithread.py b/eth_defi/event_reader/multithread.py
@@ -150,15 +150,15 @@ class MultithreadEventReader(Web3EventReader):
     """
 
     def __init__(
-            self,
-            json_rpc_url: str,
-            max_threads=10,
-            reader_context: Any = None,
-            api_counter=True,
-            max_blocks_once=50_000,
-            reorg_mon: Optional[ReorganisationMonitor] = None,
-            notify: Optional[ProgressUpdate] = None,
-            auto_close_notify=True,
+        self,
+        json_rpc_url: str,
+        max_threads=10,
+        reader_context: Any = None,
+        api_counter=True,
+        max_blocks_once=50_000,
+        reorg_mon: Optional[ReorganisationMonitor] = None,
+        notify: Optional[ProgressUpdate] = None,
+        auto_close_notify=True,
     ):
         """Creates a multithreaded JSON-RPC reader pool.
 
@@ -227,13 +227,13 @@ def close(self):
                 self.notify.close()
 
     def __call__(
-            self,
-            web3: ReaderConnection,
-            start_block: int,
-            end_block: int,
-            events: Optional[List[ContractEvent]] = None,
-            filter: Optional[Filter] = None,
-            extract_timestamps: Optional[Callable] = None,
+        self,
+        web3: ReaderConnection,
+        start_block: int,
+        end_block: int,
+        events: Optional[List[ContractEvent]] = None,
+        filter: Optional[Filter] = None,
+        extract_timestamps: Optional[Callable] = None,
     ) -> Iterable[LogResult]:
         """Wrap the underlying low-level function.
 

diff --git a/eth_defi/event_reader/reader.py b/eth_defi/event_reader/reader.py
@@ -512,7 +512,6 @@ def read_events(
     last_timestamp = None
 
     for block_num in range(start_block, end_block + 1, chunk_size):
-
         last_of_chunk = min(end_block, block_num + chunk_size - 1)
 
         logger.debug("Extracting eth_getLogs from %d - %d", block_num, last_of_chunk)

diff --git a/eth_defi/gas.py b/eth_defi/gas.py
@@ -88,7 +88,7 @@ def estimate_gas_fees(web3: Web3) -> GasPriceSuggestion:
         return GasPriceSuggestion(method=GasPriceMethod.legacy, legacy_gas_price=web3.eth.generate_gas_price())
 
 
-def apply_gas(tx: dict, suggestion: GasPriceSuggestion):
+def apply_gas(tx: dict, suggestion: GasPriceSuggestion) -> dict:
     """Apply gas fees to a raw transaction dict.
 
     Example:
@@ -117,13 +117,18 @@ def apply_gas(tx: dict, suggestion: GasPriceSuggestion):
         tx_hash = web3.eth.send_raw_transaction(signed.rawTransaction)
         receipt = web3.eth.get_transaction_receipt(tx_hash)
 
+    :return:
+        Mutated dict
+
     """
     if suggestion.method == GasPriceMethod.london:
         tx["maxFeePerGas"] = suggestion.max_fee_per_gas
         tx["maxPriorityFeePerGas"] = suggestion.max_priority_fee_per_gas
     else:
         tx["gasPrice"] = suggestion.legacy_gas_price
 
+    return tx
+
 
 def node_default_gas_price_strategy(web3: Web3, transaction_params: dict) -> int:
     """Gas price strategy for blockchains not supporting dynamic gas fees.

diff --git a/eth_defi/hotwallet.py b/eth_defi/hotwallet.py
@@ -16,7 +16,9 @@
 from eth_account.signers.local import LocalAccount
 from hexbytes import HexBytes
 from web3 import Web3
+from web3._utils.contracts import prepare_transaction
 from web3.contract.contract import ContractFunction
+from web3.contract.utils import build_transaction_for_function
 
 from eth_defi.gas import estimate_gas_fees, apply_gas
 from eth_defi.tx import decode_signed_transaction
@@ -177,7 +179,11 @@ def sign_transaction_with_new_nonce(self, tx: dict) -> SignedTransactionWithNonc
         )
         return signed
 
-    def sign_bound_call_with_new_nonce(self, func: ContractFunction, tx_params: dict | None = None) -> SignedTransactionWithNonce:
+    def sign_bound_call_with_new_nonce(
+        self,
+        func: ContractFunction,
+        tx_params: dict | None = None,
+    ) -> SignedTransactionWithNonce:
         """Signs a bound Web3 Contract call.
 
         Example:
@@ -188,6 +194,15 @@ def sign_bound_call_with_new_nonce(self, func: ContractFunction, tx_params: dict
             signed_tx = hot_wallet.sign_bound_call_with_new_nonce(bound_func)
             web3.eth.send_raw_transaction(signed_tx.rawTransaction)
 
+        With manual gas estimation:
+
+        .. code-block:: python
+
+            approve_call = usdc.contract.functions.approve(quickswap.router.address, raw_amount)
+            gas_estimation = estimate_gas_fees(web3)
+            tx_gas_parameters = apply_gas({"gas": 100_000}, gas_estimation)  # approve should not take more than 100k gas
+            signed_tx = hot_wallet.sign_bound_call_with_new_nonce(approve_call, tx_gas_parameters)
+
         See also
 
         - :py:meth:`sign_transaction_with_new_nonce`
@@ -199,10 +214,31 @@ def sign_bound_call_with_new_nonce(self, func: ContractFunction, tx_params: dict
             Transaction parameters like `gas`
         """
         assert isinstance(func, ContractFunction)
+
         if tx_params is None:
             tx_params = {}
+
         tx_params["from"] = self.address
-        tx = func.build_transaction(tx_params)
+
+        if "chainId" not in tx_params:
+            tx_params["chainId"] = func.w3.eth.chain_id
+
+        if tx_params is None:
+            # Use the default gas filler
+            tx = func.build_transaction(tx_params)
+        else:
+            # Use given gas parameters
+            tx = prepare_transaction(
+                func.address,
+                func.w3,
+                fn_identifier=func.function_identifier,
+                contract_abi=func.contract_abi,
+                fn_abi=func.abi,
+                transaction=tx_params,
+                fn_args=func.args,
+                fn_kwargs=func.kwargs,
+            )
+
         return self.sign_transaction_with_new_nonce(tx)
 
     def get_native_currency_balance(self, web3: Web3) -> Decimal:

diff --git a/eth_defi/middleware.py b/eth_defi/middleware.py
@@ -98,14 +98,12 @@
     # cannot handle gracefully.
     # ValueError: {'message': 'Internal JSON-RPC error.', 'code': -32603}
     -32603,
-
     # ValueError: {'code': -32000, 'message': 'nonce too low'}.
     # Might happen when we are broadcasting multiple transactions through multiple RPC providers
     # using eth_sendRawTransaction
     # One provide has not yet seeing a transaction broadcast through the other provider.
     # CRAP! -32000 is also Execution reverted on Alchemy.
     # -32000,
-
     # ValueError: {'code': -32003, 'message': 'nonce too low'}.
     # Anvil variant for nonce too low, same as above
     -32003,
@@ -128,9 +126,7 @@
 #:
 #: See :py:data:`DEFAULT_RETRYABLE_RPC_ERROR_CODES`.
 #:
-DEFAULT_RETRYABLE_RPC_ERROR_MESSAGES = {
-    "nonce too low"
-}
+DEFAULT_RETRYABLE_RPC_ERROR_MESSAGES = {"nonce too low"}
 
 #: Ethereum JSON-RPC calls where the value never changes
 #:
@@ -197,7 +193,6 @@ def is_retryable_http_exception(
         if len(exc.args) > 0:
             arg = exc.args[0]
             if type(arg) == dict:
-
                 code = arg.get("code")
                 message = arg.get("message", "")
 
@@ -424,8 +419,8 @@ def static_call_cache_middleware(
     The cache is web3 instance itself, to allow sharing the cache
     between different JSON-RPC providers.
     """
-    def middleware(method: RPCEndpoint, params: Any) -> RPCResponse:
 
+    def middleware(method: RPCEndpoint, params: Any) -> RPCResponse:
         cache = getattr(web3, "static_call_cache", {})
         if method in STATIC_CALL_LIST:
             cached = cache.get(method)
@@ -438,4 +433,3 @@ def middleware(method: RPCEndpoint, params: Any) -> RPCResponse:
         return resp
 
     return middleware
-
diff --git a/eth_defi/provider/anvil.py b/eth_defi/provider/anvil.py
@@ -482,11 +482,12 @@ def mine(web3: Web3, timestamp: Optional[int] = None) -> None:
 
     if timestamp is None:
         make_anvil_custom_rpc_request(web3, "evm_mine")
-        #block = web3.eth.get_block(web3.eth.block_number)
-        #timestamp = block["timestamp"] + 1
+        # block = web3.eth.get_block(web3.eth.block_number)
+        # timestamp = block["timestamp"] + 1
     else:
         make_anvil_custom_rpc_request(web3, "evm_mine", [timestamp])
 
+
 def snapshot(web3: Web3) -> int:
     """Call evm_snapshot on Anvil"""
     return int(make_anvil_custom_rpc_request(web3, "evm_snapshot", []), 16)

diff --git a/eth_defi/provider/fallback.py b/eth_defi/provider/fallback.py
@@ -198,14 +198,7 @@ def make_request(self, method: RPCEndpoint, params: Any) -> RPCResponse:
                 return resp_data
 
             except Exception as e:
-                if is_retryable_http_exception(
-                    e,
-                    retryable_rpc_error_codes=self.retryable_rpc_error_codes,
-                    retryable_status_codes=self.retryable_status_codes,
-                    retryable_exceptions=self.retryable_exceptions,
-                    method=method,
-                    params=params
-                ):
+                if is_retryable_http_exception(e, retryable_rpc_error_codes=self.retryable_rpc_error_codes, retryable_status_codes=self.retryable_status_codes, retryable_exceptions=self.retryable_exceptions, method=method, params=params):
                     if self.has_multiple_providers():
                         self.switch_provider()
 

diff --git a/eth_defi/provider/multi_provider.py b/eth_defi/provider/multi_provider.py
@@ -204,8 +204,8 @@ def create_multi_provider_web3(
         session = requests.Session()
         retry = Retry(connect=3, backoff_factor=0.5)
         adapter = HTTPAdapter(max_retries=retry)
-        session.mount('http://', adapter)
-        session.mount('https://', adapter)
+        session.mount("http://", adapter)
+        session.mount("https://", adapter)
 
     call_providers = [HTTPProvider(url, request_kwargs=request_kwargs, session=session) for url in call_endpoints]