Better nonce handling (#1609)

There's an issue in nonce handling in that calling a trade that e.g.,
previews the call before submitting can fail itself (e.g., slippage),
which ends up updating the internal nonce count before the transaction
gets submitted. We solve this by passing in a nonce function instead of
the actual nonce, and call this nonce function at the last possible
moment (i.e., when sending the built transaction) to ensure nonce
bookkeeping isn't incrementing prematurely.

scripts/checkpoint_bots.py

@@ -230,16 +230,13 @@ def run_checkpoint_bot(
                 # 0 is the max iterations for distribute excess idle, where it will default to
                 # the default max iterations
                 fn_args = (checkpoint_time, 0)
-                # Call the thread safe get nonce function for this transaction's nonce
-                nonce = async_get_nonce(web3, sender)
 
                 # Try preview call
                 _ = smart_contract_preview_transaction(
                     hyperdrive_contract,
                     sender.address,
                     "checkpoint",
                     *fn_args,
-                    nonce=nonce,
                 )
 
                 receipt = smart_contract_transact(
@@ -248,7 +245,7 @@ def run_checkpoint_bot(
                     sender,
                     "checkpoint",
                     *fn_args,
-                    nonce=nonce,
+                    nonce_func=partial(async_get_nonce, web3, sender),
                 )
                 # Reset fail count on successful transaction
                 fail_count = 0

scripts/run_unit_fuzz.py

@@ -3,7 +3,6 @@
 from __future__ import annotations
 
 import argparse
-import logging
 import sys
 import time
 from typing import NamedTuple, Sequence
@@ -90,7 +89,7 @@ def main(argv: Sequence[str] | None = None):
             pass
         except Exception as e:  # pylint: disable=broad-except
             print("Unexpected error:\n", repr(e))
-            rollbar.report_exc_info(level=logging.CRITICAL)
+            rollbar.report_exc_info(level="critical")
             if parsed_args.pause_on_fail:
                 # We don't log info from logging, so we print to ensure this shows up
                 # TODO we don't have access to the hyperdrive pool here, ideally we would
@@ -130,7 +129,7 @@ def main(argv: Sequence[str] | None = None):
             pass
         except Exception as e:  # pylint: disable=broad-except
             print("Unexpected error:\n", repr(e))
-            rollbar.report_exc_info(level=logging.CRITICAL)
+            rollbar.report_exc_info(level="critical")
             if parsed_args.pause_on_fail:
                 # We don't log info from logging, so we print to ensure this shows up
                 # TODO we don't have access to the hyperdrive pool here, ideally we would
@@ -158,7 +157,7 @@ def main(argv: Sequence[str] | None = None):
             pass
         except Exception as e:  # pylint: disable=broad-except
             print("Unexpected error:\n", repr(e))
-            rollbar.report_exc_info(level=logging.CRITICAL)
+            rollbar.report_exc_info(level="critical")
             if parsed_args.pause_on_fail:
                 # We don't log info from logging, so we print to ensure this shows up
                 # TODO we don't have access to the hyperdrive pool here, ideally we would
@@ -192,7 +191,7 @@ def main(argv: Sequence[str] | None = None):
             pass
         except Exception as e:  # pylint: disable=broad-except
             print("Unexpected error:\n", repr(e))
-            rollbar.report_exc_info(level=logging.CRITICAL)
+            rollbar.report_exc_info(level="critical")
             if parsed_args.pause_on_fail:
                 # We don't log info from logging, so we print to ensure this shows up
                 # TODO we don't have access to the hyperdrive pool here, ideally we would

src/agent0/core/hyperdrive/interactive/exec/execute_agent_trades.py

@@ -28,10 +28,8 @@
 )
 from agent0.core.hyperdrive.policies import HyperdriveBasePolicy
 from agent0.core.test_utils import assert_never
-from agent0.ethpy.base.transactions import DEFAULT_READ_RETRY_COUNT
 from agent0.ethpy.hyperdrive import HyperdriveReadInterface, HyperdriveReadWriteInterface
 from agent0.ethpy.hyperdrive.event_types import BaseHyperdriveEvent
-from agent0.utils import retry_call
 
 
 def get_liquidation_trades(
@@ -129,7 +127,7 @@ async def async_execute_agent_trades(
     wallet_func: Callable[[], HyperdriveWallet],
     policy: HyperdriveBasePolicy | None,
     preview_before_trade: bool,
-    base_nonce: Nonce | None = None,
+    nonce_func: Callable[[], Nonce] | None = None,
 ) -> list[TradeResult]:
     """Executes a single agent's trade based on its policy.
     This function is async as `_match_contract_call_to_trade` waits for a transaction receipt.
@@ -157,8 +155,10 @@ async def async_execute_agent_trades(
         The policy being executed.
     preview_before_trade: bool
         Whether or not to preview the trade before it is executed.
-    base_nonce: Nonce, optional
-        The base nonce to use for this set of trades.
+    nonce_func: Callable[[], Nonce] | None
+        A callable function to use to get a nonce. This function is useful for e.g.,
+        passing in a safe nonce getter tied to an agent.
+        Defaults to setting it to the result of `get_transaction_count`.
 
     Returns
     -------
@@ -167,15 +167,6 @@ async def async_execute_agent_trades(
         TradeResult handles any information about the trade, as well as any trade errors.
     """
 
-    # Make trades async for this agent. This way, an agent can submit multiple trades for a single block
-    # To do this, we need to manually set the nonce, so we get the base transaction count here
-    # and pass in an incrementing nonce per call
-    # TODO figure out which exception here to retry on
-    if base_nonce is None:
-        base_nonce = retry_call(
-            DEFAULT_READ_RETRY_COUNT, None, interface.web3.eth.get_transaction_count, account.address, "pending"
-        )
-
     # Here, gather returns results based on original order of trades due to nonce getting explicitly set based
     # on iterating through the list
 
@@ -191,8 +182,8 @@ async def async_execute_agent_trades(
                 account,
                 interface,
                 trade_object,
-                nonce=Nonce(base_nonce + i),
                 preview_before_trade=preview_before_trade,
+                nonce_func=nonce_func,
             )
             for i, trade_object in enumerate(trades)
         ],
@@ -225,8 +216,8 @@ async def async_execute_single_trade(
     trade_object: Trade[HyperdriveMarketAction],
     execute_policy_post_action: bool,
     preview_before_trade: bool,
+    nonce_func: Callable[[], Nonce] | None = None,
     policy: HyperdriveBasePolicy | None = None,
-    nonce: Nonce | None = None,
 ) -> TradeResult:
     """Executes a single trade made by the agent.
 
@@ -252,32 +243,27 @@ async def async_execute_single_trade(
         Whether or not to execute the post_action of the policy after the trade.
     preview_before_trade: bool
         Whether or not to preview the trade before it is executed.
+    nonce_func: Callable[[], Nonce] | None
+        A callable function to use to get a nonce. This function is useful for e.g.,
+        passing in a safe nonce getter tied to an agent.
+        Defaults to setting it to the result of `get_transaction_count`.
     policy: HyperdriveBasePolicy | None, optional
         The policy attached to the agent. This is only used to potentially call `post_action`
         of the policy.
-    nonce: Nonce | None, optional
-        The nonce for the trade.
 
     Returns
     -------
     TradeResult
         The result of the trade.
     """
 
-    # TODO we likely need to bookkeep nonces here to avoid a race condition when this function
-    # is being called asynchronously. We use a lock for the time being as a stopgap.
-    if nonce is None:
-        nonce = retry_call(
-            DEFAULT_READ_RETRY_COUNT, None, interface.web3.eth.get_transaction_count, account.address, "pending"
-        )
-
     try:
         receipt_or_exception = await _async_match_contract_call_to_trade(
             account,
             interface,
             trade_object,
-            nonce,
             preview_before_trade,
+            nonce_func=nonce_func,
         )
     except Exception as e:  # pylint: disable=broad-except
         receipt_or_exception = e
@@ -372,8 +358,8 @@ async def _async_match_contract_call_to_trade(
     account: LocalAccount,
     interface: HyperdriveReadWriteInterface,
     trade_envelope: Trade[HyperdriveMarketAction],
-    nonce: Nonce,
     preview_before_trade: bool,
+    nonce_func: Callable[[], Nonce] | None = None,
 ) -> BaseHyperdriveEvent:
     """Match statement that executes the smart contract trade based on the provided type.
 
@@ -385,10 +371,12 @@ async def _async_match_contract_call_to_trade(
         The Hyperdrive API interface object.
     trade_envelope: Trade[HyperdriveMarketAction]
         A specific Hyperdrive trade requested by the given agent.
-    nonce: Nonce, optional
-        Override the transaction number assigned to the transaction call from the agent wallet.
     preview_before_trade: bool
         Whether or not to preview the trade before it is executed.
+    nonce_func: Callable[[], Nonce] | None
+        A callable function to use to get a nonce. This function is useful for e.g.,
+        passing in a safe nonce getter tied to an agent.
+        Defaults to setting it to the result of `get_transaction_count`.
 
     Returns
     -------
@@ -406,7 +394,7 @@ async def _async_match_contract_call_to_trade(
                 trade.trade_amount,
                 slippage_tolerance=trade.slippage_tolerance,
                 gas_limit=trade.gas_limit,
-                nonce=nonce,
+                nonce_func=nonce_func,
                 preview_before_trade=preview_before_trade,
             )
 
@@ -419,7 +407,7 @@ async def _async_match_contract_call_to_trade(
                 trade.maturity_time,
                 slippage_tolerance=trade.slippage_tolerance,
                 gas_limit=trade.gas_limit,
-                nonce=nonce,
+                nonce_func=nonce_func,
                 preview_before_trade=preview_before_trade,
             )
 
@@ -429,7 +417,7 @@ async def _async_match_contract_call_to_trade(
                 trade.trade_amount,
                 slippage_tolerance=trade.slippage_tolerance,
                 gas_limit=trade.gas_limit,
-                nonce=nonce,
+                nonce_func=nonce_func,
                 preview_before_trade=preview_before_trade,
             )
 
@@ -442,7 +430,7 @@ async def _async_match_contract_call_to_trade(
                 trade.maturity_time,
                 slippage_tolerance=trade.slippage_tolerance,
                 gas_limit=trade.gas_limit,
-                nonce=nonce,
+                nonce_func=nonce_func,
                 preview_before_trade=preview_before_trade,
             )
 
@@ -458,7 +446,7 @@ async def _async_match_contract_call_to_trade(
                 trade.max_apr,
                 slippage_tolerance=None,
                 gas_limit=trade.gas_limit,
-                nonce=nonce,
+                nonce_func=nonce_func,
                 preview_before_trade=preview_before_trade,
             )
 
@@ -467,7 +455,7 @@ async def _async_match_contract_call_to_trade(
                 account,
                 trade.trade_amount,
                 gas_limit=trade.gas_limit,
-                nonce=nonce,
+                nonce_func=nonce_func,
                 preview_before_trade=preview_before_trade,
             )
 
@@ -476,7 +464,7 @@ async def _async_match_contract_call_to_trade(
                 account,
                 trade.trade_amount,
                 gas_limit=trade.gas_limit,
-                nonce=nonce,
+                nonce_func=nonce_func,
                 preview_before_trade=preview_before_trade,
             )
 

src/agent0/core/hyperdrive/interactive/hyperdrive_agent.py

@@ -153,6 +153,16 @@ def __init__(
         self.current_nonce = 0
 
     def _get_nonce_safe(self) -> Nonce:
+        """Get agent nonces in a thread-safe manner.
+
+        We pass in the callable function to underlying ethpy calls so that
+        we get nonce when we sign the transaction.
+
+        Returns
+        -------
+        int
+            The agent's current nonce value.
+        """
         with self.nonce_lock:
             # Since we're handling nonces here, we assume this wallet isn't making other trades
             # so we always use the latest block
@@ -163,7 +173,8 @@ def _get_nonce_safe(self) -> Nonce:
             else:
                 out_nonce = self.current_nonce
                 self.current_nonce += 1
-            return Nonce(out_nonce)
+
+        return Nonce(out_nonce)
 
     def _reset_nonce(self) -> None:
         with self.nonce_lock:
@@ -324,8 +335,8 @@ def open_long(self, base: FixedPoint, pool: Hyperdrive | None = None) -> OpenLon
                 trade_object,
                 self.chain.config.always_execute_policy_post_action,
                 self.chain.config.preview_before_trade,
-                self._active_policy,
-                nonce=self._get_nonce_safe(),
+                nonce_func=self._get_nonce_safe,
+                policy=self._active_policy,
             )
         )
         try:
@@ -372,8 +383,8 @@ def close_long(self, maturity_time: int, bonds: FixedPoint, pool: Hyperdrive | N
                 trade_object,
                 self.chain.config.always_execute_policy_post_action,
                 self.chain.config.preview_before_trade,
-                self._active_policy,
-                nonce=self._get_nonce_safe(),
+                nonce_func=self._get_nonce_safe,
+                policy=self._active_policy,
             )
         )
         try:
@@ -417,8 +428,8 @@ def open_short(self, bonds: FixedPoint, pool: Hyperdrive | None = None) -> OpenS
                 trade_object,
                 self.chain.config.always_execute_policy_post_action,
                 self.chain.config.preview_before_trade,
-                self._active_policy,
-                nonce=self._get_nonce_safe(),
+                nonce_func=self._get_nonce_safe,
+                policy=self._active_policy,
             )
         )
         try:
@@ -463,8 +474,8 @@ def close_short(self, maturity_time: int, bonds: FixedPoint, pool: Hyperdrive |
                 trade_object,
                 self.chain.config.always_execute_policy_post_action,
                 self.chain.config.preview_before_trade,
-                self._active_policy,
-                nonce=self._get_nonce_safe(),
+                nonce_func=self._get_nonce_safe,
+                policy=self._active_policy,
             )
         )
         try:
@@ -507,8 +518,8 @@ def add_liquidity(self, base: FixedPoint, pool: Hyperdrive | None = None) -> Add
                 trade_object,
                 self.chain.config.always_execute_policy_post_action,
                 self.chain.config.preview_before_trade,
-                self._active_policy,
-                nonce=self._get_nonce_safe(),
+                nonce_func=self._get_nonce_safe,
+                policy=self._active_policy,
             )
         )
         try:
@@ -551,8 +562,8 @@ def remove_liquidity(self, shares: FixedPoint, pool: Hyperdrive | None = None) -
                 trade_object,
                 self.chain.config.always_execute_policy_post_action,
                 self.chain.config.preview_before_trade,
-                self._active_policy,
-                nonce=self._get_nonce_safe(),
+                nonce_func=self._get_nonce_safe,
+                policy=self._active_policy,
             )
         )
         try:
@@ -595,8 +606,8 @@ def redeem_withdrawal_share(self, shares: FixedPoint, pool: Hyperdrive | None =
                 trade_object,
                 self.chain.config.always_execute_policy_post_action,
                 self.chain.config.preview_before_trade,
-                self._active_policy,
-                nonce=self._get_nonce_safe(),
+                nonce_func=self._get_nonce_safe,
+                policy=self._active_policy,
             )
         )
         try:
@@ -708,7 +719,7 @@ def execute_action(
                 # We pass in policy here for `post_action`. Post action is ignored if policy not set.
                 policy=self._active_policy,
                 preview_before_trade=self.chain.config.preview_before_trade,
-                base_nonce=self._get_nonce_safe(),
+                nonce_func=self._get_nonce_safe,
             )
         )
         out_events = []
@@ -717,9 +728,6 @@ def execute_action(
             hyperdrive_event = self._handle_trade_result(trade_result, pool, always_throw_exception=False)
             if hyperdrive_event is not None:
                 out_events.append(hyperdrive_event)
-            else:
-                # We always reset nonce on failure to avoid skipped nonces
-                self._reset_nonce()
         return out_events
 
     def execute_policy_action(self, pool: Hyperdrive | None = None) -> list[BaseHyperdriveEvent]:

src/agent0/core/hyperdrive/interactive/local_hyperdrive_agent.py

@@ -175,7 +175,8 @@ def set_active(
     def account(self) -> LocalAccount:
         """Returns the `LocalAccount` associated with the agent."""
         # Account should always be set in local agents
-        assert self._account is not None
+        if self._account is None:
+            raise ValueError("Must initialize agent with private key to access agent's LocalAccount.")
         return self._account
 
     ################