From 829378f39ee5999be83013b54143ad6d3e71ced6 Mon Sep 17 00:00:00 2001 From: Sheng Lundquist Date: Thu, 9 Nov 2023 10:54:10 -0800 Subject: [PATCH 1/9] Adding in custom wait for transaction receipt with exp backoff --- lib/ethpy/ethpy/base/transactions.py | 49 +++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/lib/ethpy/ethpy/base/transactions.py b/lib/ethpy/ethpy/base/transactions.py index 0283fa1a8..81a3d5365 100644 --- a/lib/ethpy/ethpy/base/transactions.py +++ b/lib/ethpy/ethpy/base/transactions.py @@ -219,6 +219,51 @@ def retry_preview_check(exc: Exception) -> bool: return {f"value{idx}": value for idx, value in enumerate(return_values)} +def wait_for_transaction_receipt( + web3: Web3, transaction_hash: HexBytes, timeout: float = 30, start_latency: float = 1, backoff: float = 2 +) -> TxReceipt: + """wait_for_transaction_receipt with exponential backoff + This function is copied from `web3.eth.wait_for_transaction_receipt`, but using exp backoff. + + Arguments + --------- + web3: Web3 + web3 provider object + transaction_hash: HexBytes + The hash of the transaction + timeout: float + The amount of time in seconds to time out the connection + poll_latency: float + The amount of time in seconds to wait between polls + + Returns + ------- + TxReceipt + The transaction receipt + """ + try: + with Timeout(timeout) as _timeout: + poll_latency = start_latency + random.uniform(0, 1) + while True: + try: + tx_receipt = web3.eth.get_transaction_receipt(transaction_hash) + except TransactionNotFound: + tx_receipt = None + if tx_receipt is not None: + break + _timeout.sleep(poll_latency) + # Exp backoff + poll_latency *= backoff + # Add random latency to avoid collisions + poll_latency += random.uniform(0, 1) + return tx_receipt + + except Timeout as exc: + raise TimeExhausted( + f"Transaction {HexBytes(transaction_hash) !r} is not in the chain " f"after {timeout} seconds" + ) from exc + + async def async_wait_for_transaction_receipt( web3: Web3, transaction_hash: HexBytes, timeout: float = 30, start_latency: float = 1, backoff: float = 2 ) -> TxReceipt: @@ -455,7 +500,7 @@ def _send_transaction_and_wait_for_receipt(unsent_txn: TxParams, signer: LocalAc """ signed_txn = signer.sign_transaction(unsent_txn) tx_hash = web3.eth.send_raw_transaction(signed_txn.rawTransaction) - tx_receipt = web3.eth.wait_for_transaction_receipt(tx_hash) + tx_receipt = wait_for_transaction_receipt(web3, tx_hash) # Error checking when transaction doesn't throw an error, but instead # has errors in the tx_receipt @@ -754,6 +799,8 @@ def _contract_function_abi_outputs(contract_abi: ABI, function_name: str) -> lis for output in function_outputs: return_names_and_types.append(_get_name_and_type_from_abi(output)) return return_names_and_types + if len(function_outputs) == 0: # No function arguments returned from preview + return None if ( function_outputs[0].get("type") == "tuple" and function_outputs[0].get("components") is not None ): # multiple named outputs were returned in a struct From 3a1b9dc7d1b7bac5ff5e4166e833f04b3389a3ef Mon Sep 17 00:00:00 2001 From: Sheng Lundquist Date: Thu, 9 Nov 2023 11:09:40 -0800 Subject: [PATCH 2/9] Transactions sometimes don't have logs --- lib/ethpy/ethpy/base/transactions.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/lib/ethpy/ethpy/base/transactions.py b/lib/ethpy/ethpy/base/transactions.py index 81a3d5365..3ba2dc980 100644 --- a/lib/ethpy/ethpy/base/transactions.py +++ b/lib/ethpy/ethpy/base/transactions.py @@ -381,11 +381,6 @@ async def _async_send_transaction_and_wait_for_receipt( raise UnknownBlockError("Receipt did not return status", f"{block_number=}") if status == 0: raise UnknownBlockError("Receipt has status of 0", f"{block_number=}", f"{tx_receipt=}") - logs = tx_receipt.get("logs", None) - if logs is None: - raise UnknownBlockError("Receipt did not return logs", f"{block_number=}") - if len(logs) == 0: - raise UnknownBlockError("Logs have a length of 0", f"{block_number=}", f"{tx_receipt=}") return tx_receipt @@ -513,11 +508,6 @@ def _send_transaction_and_wait_for_receipt(unsent_txn: TxParams, signer: LocalAc raise UnknownBlockError("Receipt did not return status", f"{block_number=}") if status == 0: raise UnknownBlockError("Receipt has status of 0", f"{block_number=}", f"{tx_receipt=}") - logs = tx_receipt.get("logs", None) - if logs is None: - raise UnknownBlockError("Receipt did not return logs", f"{block_number=}") - if len(logs) == 0: - raise UnknownBlockError("Logs have a length of 0", f"{block_number=}", f"{tx_receipt=}") return tx_receipt From 74a0914e082c8d763321d714ff7298a8333fd568 Mon Sep 17 00:00:00 2001 From: Sheng Lundquist Date: Thu, 9 Nov 2023 11:14:45 -0800 Subject: [PATCH 3/9] docstrings --- lib/ethpy/ethpy/base/transactions.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/ethpy/ethpy/base/transactions.py b/lib/ethpy/ethpy/base/transactions.py index 3ba2dc980..f1adc079d 100644 --- a/lib/ethpy/ethpy/base/transactions.py +++ b/lib/ethpy/ethpy/base/transactions.py @@ -233,8 +233,10 @@ def wait_for_transaction_receipt( The hash of the transaction timeout: float The amount of time in seconds to time out the connection - poll_latency: float - The amount of time in seconds to wait between polls + start_latency: float + The starting amount of time in seconds to wait between polls + backoff: float + The backoff factor for the exponential backoff Returns ------- @@ -279,8 +281,10 @@ async def async_wait_for_transaction_receipt( The hash of the transaction timeout: float The amount of time in seconds to time out the connection - poll_latency: float - The amount of time in seconds to wait between polls + start_latency: float + The starting amount of time in seconds to wait between polls + backoff: float + The backoff factor for the exponential backoff Returns ------- From cf81909850a21a108d0ed93f08c2739ab2a0893a Mon Sep 17 00:00:00 2001 From: Sheng Lundquist Date: Thu, 9 Nov 2023 11:16:07 -0800 Subject: [PATCH 4/9] lint --- lib/ethpy/ethpy/base/transactions.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/ethpy/ethpy/base/transactions.py b/lib/ethpy/ethpy/base/transactions.py index f1adc079d..288e891fe 100644 --- a/lib/ethpy/ethpy/base/transactions.py +++ b/lib/ethpy/ethpy/base/transactions.py @@ -771,6 +771,8 @@ def _get_name_and_type_from_abi(abi_outputs: ABIFunctionComponents | ABIFunction # TODO: add ability to parse function_signature as well def _contract_function_abi_outputs(contract_abi: ABI, function_name: str) -> list[tuple[str, str]] | None: + # TODO clean this function up + # pylint: disable=too-many-return-statements """Parse the function abi to get the name and type for each output""" function_abi = None # find the first function matching the function_name From 55b8bac536f5a130377c7c40ac8c40bbf526dae0 Mon Sep 17 00:00:00 2001 From: Sheng Lundquist Date: Thu, 9 Nov 2023 13:40:25 -0800 Subject: [PATCH 5/9] Build transactions in try/catch --- lib/ethpy/ethpy/base/transactions.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/ethpy/ethpy/base/transactions.py b/lib/ethpy/ethpy/base/transactions.py index 288e891fe..1882d577e 100644 --- a/lib/ethpy/ethpy/base/transactions.py +++ b/lib/ethpy/ethpy/base/transactions.py @@ -147,10 +147,6 @@ def smart_contract_preview_transaction( else: function = contract.get_function_by_name(function_name_or_signature)(*fn_args, **fn_kwargs) - # We build the raw transaction here in case of error. Note that we don't call `build_transaction` - # since it adds the nonce to the transaction, and we ignore nonce in preview - raw_txn = function.build_transaction({"from": signer_address}) - # We define the function to check the exception to retry on # This is the error we get when preview fails due to anvil def retry_preview_check(exc: Exception) -> bool: @@ -162,7 +158,13 @@ def retry_preview_check(exc: Exception) -> bool: # This is the additional transaction argument passed into function.call # that may contain additional call arguments such as max_gas, nonce, etc. transaction_kwargs = {"from": signer_address} + raw_txn = {} try: + # We build the raw transaction here in case of error. Note that we don't call `build_transaction` + # since it adds the nonce to the transaction, and we ignore nonce in preview + # Build transactions can fail, so we put this here in the try/catch + raw_txn = function.build_transaction({"from": signer_address}) + return_values = retry_call( READ_RETRY_COUNT, retry_preview_check, @@ -182,7 +184,7 @@ def retry_preview_check(exc: Exception) -> bool: function_name_or_signature=function_name_or_signature, fn_args=fn_args, fn_kwargs=fn_kwargs, - raw_txn=raw_txn, + raw_txn=dict(raw_txn), block_number=block_number, ) from err except Exception as err: @@ -193,7 +195,7 @@ def retry_preview_check(exc: Exception) -> bool: function_name_or_signature=function_name_or_signature, fn_args=fn_args, fn_kwargs=fn_kwargs, - raw_txn=raw_txn, + raw_txn=dict(raw_txn), block_number=block_number, ) from err From 1a2b3bd375fe0dc62bcce38180e0a0bf4fc9f84c Mon Sep 17 00:00:00 2001 From: Sheng Lundquist Date: Thu, 9 Nov 2023 14:11:22 -0800 Subject: [PATCH 6/9] Raising explicit error with redeem when invalid balance --- lib/ethpy/ethpy/hyperdrive/api/_contract_calls.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/ethpy/ethpy/hyperdrive/api/_contract_calls.py b/lib/ethpy/ethpy/hyperdrive/api/_contract_calls.py index 6eff8a8de..7e97a035f 100644 --- a/lib/ethpy/ethpy/hyperdrive/api/_contract_calls.py +++ b/lib/ethpy/ethpy/hyperdrive/api/_contract_calls.py @@ -443,13 +443,20 @@ async def _async_redeem_withdraw_shares( # before calling smart contract transact # Since current_pool_state.block_number is a property, we want to get the static block here current_block = cls.current_pool_state.block_number - _ = smart_contract_preview_transaction( + preview_result = smart_contract_preview_transaction( cls.hyperdrive_contract, agent_checksum_address, "redeemWithdrawalShares", *fn_args, block_number=current_block, ) + + # Here, a preview call of redeem withdrawal shares will still be successful without logs if + # the amount of shares to redeem is larger than what's in the wallet. We want to catch this error + # here with a useful error message, so we check that explicitly here + if preview_result["sharesRedeemed"] == 0 and trade_amount > 0: + raise ValueError("Preview call for redeem withdrawal shares returned 0 for non-zero input trade amount") + try: tx_receipt = await async_smart_contract_transact( cls.web3, From cf4bf9d04376c29602f4b31f4aeb2e1a78cfa6d1 Mon Sep 17 00:00:00 2001 From: Sheng Lundquist Date: Thu, 9 Nov 2023 14:12:14 -0800 Subject: [PATCH 7/9] Adding test for invalid balances --- tests/invalid_balance_trade_test.py | 836 ++++++++++++++++++++++++++++ 1 file changed, 836 insertions(+) create mode 100644 tests/invalid_balance_trade_test.py diff --git a/tests/invalid_balance_trade_test.py b/tests/invalid_balance_trade_test.py new file mode 100644 index 000000000..6d81c0f10 --- /dev/null +++ b/tests/invalid_balance_trade_test.py @@ -0,0 +1,836 @@ +"""Test for invalid trades due to balance""" +from __future__ import annotations + +import logging +import os +from typing import TYPE_CHECKING, Type, cast + +from agent0 import build_account_key_config_from_agent_config +from agent0.base.config import AgentConfig, EnvironmentConfig +from agent0.hyperdrive.exec import run_agents +from agent0.hyperdrive.policies import HyperdrivePolicy +from agent0.hyperdrive.state import HyperdriveActionType, HyperdriveMarketAction, HyperdriveWallet +from elfpy.types import MarketType, Trade +from eth_typing import URI +from ethpy import EthConfig +from ethpy.base.errors import ContractCallException +from fixedpointmath import FixedPoint +from numpy.random._generator import Generator as NumpyGenerator +from web3 import HTTPProvider +from web3.exceptions import ContractPanicError + +if TYPE_CHECKING: + from ethpy.hyperdrive import HyperdriveAddresses + from ethpy.hyperdrive.api import HyperdriveInterface + from ethpy.test_fixtures.local_chain import DeployedHyperdrivePool + + +# Start by defining policies for failed trades +# One policy per failed trade +# Starting with empty wallet, catching any closing trades. +class InvalidRemoveLiquidityFromZero(HyperdrivePolicy): + """A agent that submits a remove liquidity with a zero wallet""" + + def __init__( + self, + budget: FixedPoint, + rng: NumpyGenerator | None = None, + slippage_tolerance: FixedPoint | None = None, + # When this policy doesn't have a config and doesn't define a custom config object + # we still need it in the constructor since the object factory still calls with this arg + policy_config: HyperdrivePolicy.Config | None = None, # pylint: disable=unused-argument + ): + # TODO Base class doesn't take policy_config, but it's needed for the object factory, fix + super().__init__(budget, rng, slippage_tolerance) + + def action( + self, hyperdrive: HyperdriveInterface, wallet: HyperdriveWallet + ) -> tuple[list[Trade[HyperdriveMarketAction]], bool]: + # pylint: disable=unused-argument + action_list = [] + # Remove non-existing Liquidity + action_list.append( + Trade( + market_type=MarketType.HYPERDRIVE, + market_action=HyperdriveMarketAction( + action_type=HyperdriveActionType.REMOVE_LIQUIDITY, + trade_amount=FixedPoint(20000), + wallet=wallet, + ), + ) + ) + return action_list, True + + +class InvalidCloseLongFromZero(HyperdrivePolicy): + """A agent that submits a close long with a zero wallet""" + + def __init__( + self, + budget: FixedPoint, + rng: NumpyGenerator | None = None, + slippage_tolerance: FixedPoint | None = None, + # When this policy doesn't have a config and doesn't define a custom config object + # we still need it in the constructor since the object factory still calls with this arg + policy_config: HyperdrivePolicy.Config | None = None, # pylint: disable=unused-argument + ): + # TODO Base class doesn't take policy_config, but it's needed for the object factory, fix + super().__init__(budget, rng, slippage_tolerance) + + def action( + self, hyperdrive: HyperdriveInterface, wallet: HyperdriveWallet + ) -> tuple[list[Trade[HyperdriveMarketAction]], bool]: + # pylint: disable=unused-argument + action_list = [] + # Closing non-existent long + action_list.append( + Trade( + market_type=MarketType.HYPERDRIVE, + market_action=HyperdriveMarketAction( + action_type=HyperdriveActionType.CLOSE_LONG, + trade_amount=FixedPoint(20000), + slippage_tolerance=self.slippage_tolerance, + wallet=wallet, + maturity_time=1699561146, + ), + ) + ) + return action_list, True + + +class InvalidCloseShortFromZero(HyperdrivePolicy): + """A agent that submits a close short with a zero wallet""" + + def __init__( + self, + budget: FixedPoint, + rng: NumpyGenerator | None = None, + slippage_tolerance: FixedPoint | None = None, + # When this policy doesn't have a config and doesn't define a custom config object + # we still need it in the constructor since the object factory still calls with this arg + policy_config: HyperdrivePolicy.Config | None = None, # pylint: disable=unused-argument + ): + # TODO Base class doesn't take policy_config, but it's needed for the object factory, fix + super().__init__(budget, rng, slippage_tolerance) + + def action( + self, hyperdrive: HyperdriveInterface, wallet: HyperdriveWallet + ) -> tuple[list[Trade[HyperdriveMarketAction]], bool]: + # pylint: disable=unused-argument + action_list = [] + # Closing non-existent short + action_list.append( + Trade( + market_type=MarketType.HYPERDRIVE, + market_action=HyperdriveMarketAction( + action_type=HyperdriveActionType.CLOSE_SHORT, + trade_amount=FixedPoint(20000), + slippage_tolerance=self.slippage_tolerance, + wallet=wallet, + maturity_time=1699561146, + ), + ) + ) + return action_list, True + + +class InvalidRedeemWithdrawFromZero(HyperdrivePolicy): + """A agent that submits a redeem withdrawal share with a zero wallet""" + + def __init__( + self, + budget: FixedPoint, + rng: NumpyGenerator | None = None, + slippage_tolerance: FixedPoint | None = None, + # When this policy doesn't have a config and doesn't define a custom config object + # we still need it in the constructor since the object factory still calls with this arg + policy_config: HyperdrivePolicy.Config | None = None, # pylint: disable=unused-argument + ): + # TODO Base class doesn't take policy_config, but it's needed for the object factory, fix + super().__init__(budget, rng, slippage_tolerance) + + def action( + self, hyperdrive: HyperdriveInterface, wallet: HyperdriveWallet + ) -> tuple[list[Trade[HyperdriveMarketAction]], bool]: + # pylint: disable=unused-argument + action_list = [] + # Redeem non-existent withdrawal shares + action_list.append( + Trade( + market_type=MarketType.HYPERDRIVE, + market_action=HyperdriveMarketAction( + action_type=HyperdriveActionType.REDEEM_WITHDRAW_SHARE, + trade_amount=FixedPoint(20000), + wallet=wallet, + ), + ) + ) + return action_list, True + + +class InvalidRemoveLiquidityFromNonZero(HyperdrivePolicy): + """A agent that submits an invalid remove liquidity share with a non-zero wallet""" + + def __init__( + self, + budget: FixedPoint, + rng: NumpyGenerator | None = None, + slippage_tolerance: FixedPoint | None = None, + # When this policy doesn't have a config and doesn't define a custom config object + # we still need it in the constructor since the object factory still calls with this arg + policy_config: HyperdrivePolicy.Config | None = None, # pylint: disable=unused-argument + ): + # We want to do a sequence of trades one at a time, so we keep an internal counter based on + # how many times `action` has been called. + self.counter = 0 + super().__init__(budget, rng, slippage_tolerance) + + def action( + self, hyperdrive: HyperdriveInterface, wallet: HyperdriveWallet + ) -> tuple[list[Trade[HyperdriveMarketAction]], bool]: + # pylint: disable=unused-argument + action_list = [] + done_trading = False + if self.counter == 0: + # Add liquidity + action_list.append( + Trade( + market_type=MarketType.HYPERDRIVE, + market_action=HyperdriveMarketAction( + action_type=HyperdriveActionType.ADD_LIQUIDITY, + trade_amount=FixedPoint(10000), + wallet=wallet, + ), + ) + ) + elif self.counter == 1: + # Remove Liquidity for more than I have + action_list.append( + Trade( + market_type=MarketType.HYPERDRIVE, + market_action=HyperdriveMarketAction( + action_type=HyperdriveActionType.REMOVE_LIQUIDITY, + trade_amount=FixedPoint(20000), + wallet=wallet, + ), + ) + ) + done_trading = True + self.counter += 1 + return action_list, done_trading + + +class InvalidCloseLongFromNonZero(HyperdrivePolicy): + """A agent that submits an invalid close long with a non-zero wallet""" + + def __init__( + self, + budget: FixedPoint, + rng: NumpyGenerator | None = None, + slippage_tolerance: FixedPoint | None = None, + # When this policy doesn't have a config and doesn't define a custom config object + # we still need it in the constructor since the object factory still calls with this arg + policy_config: HyperdrivePolicy.Config | None = None, # pylint: disable=unused-argument + ): + # We want to do a sequence of trades one at a time, so we keep an internal counter based on + # how many times `action` has been called. + self.counter = 0 + super().__init__(budget, rng, slippage_tolerance) + + def action( + self, hyperdrive: HyperdriveInterface, wallet: HyperdriveWallet + ) -> tuple[list[Trade[HyperdriveMarketAction]], bool]: + # pylint: disable=unused-argument + action_list = [] + done_trading = False + if self.counter == 0: + # Open Long + action_list.append( + Trade( + market_type=MarketType.HYPERDRIVE, + market_action=HyperdriveMarketAction( + action_type=HyperdriveActionType.OPEN_LONG, + trade_amount=FixedPoint(10000), + slippage_tolerance=self.slippage_tolerance, + wallet=wallet, + ), + ), + ) + elif self.counter == 1: + # Closing existent long for more than I have + assert len(wallet.longs) == 1 + for long_time, _ in wallet.longs.items(): + action_list.append( + Trade( + market_type=MarketType.HYPERDRIVE, + market_action=HyperdriveMarketAction( + action_type=HyperdriveActionType.CLOSE_LONG, + trade_amount=FixedPoint(20000), + slippage_tolerance=self.slippage_tolerance, + wallet=wallet, + maturity_time=long_time, + ), + ) + ) + done_trading = True + self.counter += 1 + return action_list, done_trading + + +class InvalidCloseShortFromNonZero(HyperdrivePolicy): + """A agent that submits an invalid close short with a non-zero wallet""" + + def __init__( + self, + budget: FixedPoint, + rng: NumpyGenerator | None = None, + slippage_tolerance: FixedPoint | None = None, + # When this policy doesn't have a config and doesn't define a custom config object + # we still need it in the constructor since the object factory still calls with this arg + policy_config: HyperdrivePolicy.Config | None = None, # pylint: disable=unused-argument + ): + # We want to do a sequence of trades one at a time, so we keep an internal counter based on + # how many times `action` has been called. + self.counter = 0 + super().__init__(budget, rng, slippage_tolerance) + + def action( + self, hyperdrive: HyperdriveInterface, wallet: HyperdriveWallet + ) -> tuple[list[Trade[HyperdriveMarketAction]], bool]: + # pylint: disable=unused-argument + action_list = [] + done_trading = False + if self.counter == 0: + # Open Short + action_list.append( + Trade( + market_type=MarketType.HYPERDRIVE, + market_action=HyperdriveMarketAction( + action_type=HyperdriveActionType.OPEN_SHORT, + trade_amount=FixedPoint(10000), + slippage_tolerance=self.slippage_tolerance, + wallet=wallet, + ), + ) + ) + elif self.counter == 1: + # Closing existent short for more than I have + assert len(wallet.shorts) == 1 + for short_time, _ in wallet.shorts.items(): + action_list.append( + Trade( + market_type=MarketType.HYPERDRIVE, + market_action=HyperdriveMarketAction( + action_type=HyperdriveActionType.CLOSE_SHORT, + trade_amount=FixedPoint(20000), + slippage_tolerance=self.slippage_tolerance, + wallet=wallet, + maturity_time=short_time, + ), + ) + ) + done_trading = True + self.counter += 1 + return action_list, done_trading + + +class InvalidRedeemWithdrawInPool(HyperdrivePolicy): + """A agent that submits an invalid remove liquidity when not enough ready to withdrawal""" + + def __init__( + self, + budget: FixedPoint, + rng: NumpyGenerator | None = None, + slippage_tolerance: FixedPoint | None = None, + # When this policy doesn't have a config and doesn't define a custom config object + # we still need it in the constructor since the object factory still calls with this arg + policy_config: HyperdrivePolicy.Config | None = None, # pylint: disable=unused-argument + ): + # We want to do a sequence of trades one at a time, so we keep an internal counter based on + # how many times `action` has been called. + self.counter = 0 + super().__init__(budget, rng, slippage_tolerance) + + def action( + self, hyperdrive: HyperdriveInterface, wallet: HyperdriveWallet + ) -> tuple[list[Trade[HyperdriveMarketAction]], bool]: + # pylint: disable=unused-argument + action_list = [] + done_trading = False + # We make various trades to ensure the wallet has a non-zero withdrawal share + # Valid add liquidity + if self.counter == 0: + # Add liquidity + action_list.append( + Trade( + market_type=MarketType.HYPERDRIVE, + market_action=HyperdriveMarketAction( + action_type=HyperdriveActionType.ADD_LIQUIDITY, + trade_amount=FixedPoint(10000), + wallet=wallet, + ), + ) + ) + # Valid open long + short + elif self.counter == 1: + # Open Long + action_list.append( + Trade( + market_type=MarketType.HYPERDRIVE, + market_action=HyperdriveMarketAction( + action_type=HyperdriveActionType.OPEN_LONG, + trade_amount=FixedPoint(10000), + slippage_tolerance=self.slippage_tolerance, + wallet=wallet, + ), + ), + ) + # Open Short + action_list.append( + Trade( + market_type=MarketType.HYPERDRIVE, + market_action=HyperdriveMarketAction( + action_type=HyperdriveActionType.OPEN_SHORT, + trade_amount=FixedPoint(10000), + slippage_tolerance=self.slippage_tolerance, + wallet=wallet, + ), + ) + ) + # Valid remove liquidity + elif self.counter == 2: + # Remove all liquidity + action_list.append( + Trade( + market_type=MarketType.HYPERDRIVE, + market_action=HyperdriveMarketAction( + action_type=HyperdriveActionType.REMOVE_LIQUIDITY, + trade_amount=wallet.lp_tokens, + wallet=wallet, + ), + ) + ) + elif self.counter == 3: + # Attempt to redeem withdrawal shares that are not ready to withdrawal + # since the open trades are not closed + assert wallet.withdraw_shares > FixedPoint(0) + action_list.append( + Trade( + market_type=MarketType.HYPERDRIVE, + market_action=HyperdriveMarketAction( + action_type=HyperdriveActionType.REDEEM_WITHDRAW_SHARE, + trade_amount=wallet.withdraw_shares, + wallet=wallet, + ), + ) + ) + # Last trade, set flag + done_trading = True + self.counter += 1 + + return action_list, done_trading + + +class InvalidRedeemWithdrawFromNonZero(HyperdrivePolicy): + """A agent that submits an invalid remove liquidity share with a non-zero wallet""" + + def __init__( + self, + budget: FixedPoint, + rng: NumpyGenerator | None = None, + slippage_tolerance: FixedPoint | None = None, + # When this policy doesn't have a config and doesn't define a custom config object + # we still need it in the constructor since the object factory still calls with this arg + policy_config: HyperdrivePolicy.Config | None = None, # pylint: disable=unused-argument + ): + # We want to do a sequence of trades one at a time, so we keep an internal counter based on + # how many times `action` has been called. + self.counter = 0 + super().__init__(budget, rng, slippage_tolerance) + + def action( + self, hyperdrive: HyperdriveInterface, wallet: HyperdriveWallet + ) -> tuple[list[Trade[HyperdriveMarketAction]], bool]: + # pylint: disable=unused-argument + action_list = [] + done_trading = False + # We make various trades to ensure the wallet has a non-zero withdrawal share + # Valid add liquidity + if self.counter == 0: + # Add liquidity + action_list.append( + Trade( + market_type=MarketType.HYPERDRIVE, + market_action=HyperdriveMarketAction( + action_type=HyperdriveActionType.ADD_LIQUIDITY, + trade_amount=FixedPoint(10000), + wallet=wallet, + ), + ) + ) + # Valid open long + short + elif self.counter == 1: + # Open Long + action_list.append( + Trade( + market_type=MarketType.HYPERDRIVE, + market_action=HyperdriveMarketAction( + action_type=HyperdriveActionType.OPEN_LONG, + trade_amount=FixedPoint(10000), + slippage_tolerance=self.slippage_tolerance, + wallet=wallet, + ), + ), + ) + # Open Short + action_list.append( + Trade( + market_type=MarketType.HYPERDRIVE, + market_action=HyperdriveMarketAction( + action_type=HyperdriveActionType.OPEN_SHORT, + trade_amount=FixedPoint(10000), + slippage_tolerance=self.slippage_tolerance, + wallet=wallet, + ), + ) + ) + # Valid remove liquidity + elif self.counter == 2: + # Remove all liquidity + action_list.append( + Trade( + market_type=MarketType.HYPERDRIVE, + market_action=HyperdriveMarketAction( + action_type=HyperdriveActionType.REMOVE_LIQUIDITY, + trade_amount=wallet.lp_tokens, + wallet=wallet, + ), + ) + ) + elif self.counter == 3: + # Attempt to redeem withdrawal shares that are not ready to withdrawal + # since the open trades are not closed + assert wallet.withdraw_shares > FixedPoint(0) + action_list.append( + Trade( + market_type=MarketType.HYPERDRIVE, + market_action=HyperdriveMarketAction( + action_type=HyperdriveActionType.REDEEM_WITHDRAW_SHARE, + trade_amount=FixedPoint(20000), + wallet=wallet, + ), + ) + ) + # Last trade, set flag + done_trading = True + self.counter += 1 + + return action_list, done_trading + + +class TestInvalidTrades: + """Tests pipeline from bots making trades to viewing the trades in the db""" + + def _build_and_run_with_funded_bot(self, hyperdrive_pool: DeployedHyperdrivePool, policy: Type[HyperdrivePolicy]): + # Run this test with develop mode on + os.environ["DEVELOP"] = "true" + + env_config = EnvironmentConfig( + delete_previous_logs=True, + halt_on_errors=True, + # We don't want tests to write lots of files + crash_report_to_file=False, + log_filename=".logging/invalid_test.log", + log_level=logging.INFO, + log_stdout=True, + random_seed=1234, + username="test", + ) + + # Get hyperdrive chain info + rpc_uri: URI | None = cast(HTTPProvider, hyperdrive_pool.web3.provider).endpoint_uri + assert rpc_uri is not None + hyperdrive_contract_addresses: HyperdriveAddresses = hyperdrive_pool.hyperdrive_contract_addresses + + # Build agent config + agent_config: list[AgentConfig] = [ + AgentConfig( + policy=policy, + number_of_agents=1, + slippage_tolerance=None, + base_budget_wei=FixedPoint("1_000_000").scaled_value, # 1 million base + eth_budget_wei=FixedPoint("100").scaled_value, # 100 base + policy_config=policy.Config(), + ), + ] + account_key_config = build_account_key_config_from_agent_config(agent_config) + # Build custom eth config pointing to local test chain + eth_config = EthConfig( + # Artifacts_uri isn't used here, as we explicitly set addresses and passed to run_bots + artifacts_uri="not_used", + rpc_uri=rpc_uri, + ) + run_agents( + env_config, + agent_config, + account_key_config, + eth_config=eth_config, + contract_addresses=hyperdrive_contract_addresses, + load_wallet_state=False, + ) + # If this reaches this point, the agent was successful, which means this test should fail + assert False, "Agent was successful with known invalid trade" + + def _build_and_run_with_non_funded_bot( + self, hyperdrive_pool: DeployedHyperdrivePool, policy: Type[HyperdrivePolicy] + ): + # Run this test with develop mode on + os.environ["DEVELOP"] = "true" + + env_config = EnvironmentConfig( + delete_previous_logs=True, + halt_on_errors=True, + # We don't want tests to write lots of files + crash_report_to_file=False, + log_filename=".logging/invalid_test.log", + log_level=logging.INFO, + log_stdout=True, + random_seed=1234, + username="test", + ) + + # Get hyperdrive chain info + rpc_uri: URI | None = cast(HTTPProvider, hyperdrive_pool.web3.provider).endpoint_uri + assert rpc_uri is not None + hyperdrive_contract_addresses: HyperdriveAddresses = hyperdrive_pool.hyperdrive_contract_addresses + + # Build agent config + agent_config: list[AgentConfig] = [ + AgentConfig( + policy=policy, + number_of_agents=1, + slippage_tolerance=None, + base_budget_wei=FixedPoint("10").scaled_value, # 10 base + eth_budget_wei=FixedPoint("100").scaled_value, # 100 base + policy_config=policy.Config(), + ), + ] + account_key_config = build_account_key_config_from_agent_config(agent_config) + # Build custom eth config pointing to local test chain + eth_config = EthConfig( + # Artifacts_uri isn't used here, as we explicitly set addresses and passed to run_bots + artifacts_uri="not_used", + rpc_uri=rpc_uri, + ) + run_agents( + env_config, + agent_config, + account_key_config, + eth_config=eth_config, + contract_addresses=hyperdrive_contract_addresses, + load_wallet_state=False, + ) + # If this reaches this point, the agent was successful, which means this test should fail + assert False, "Agent was successful with known invalid trade" + + def test_not_enough_base( + self, + local_hyperdrive_pool: DeployedHyperdrivePool, + ): + """Tests when making a trade with not enough base in wallet""" + try: + self._build_and_run_with_non_funded_bot(local_hyperdrive_pool, InvalidRemoveLiquidityFromNonZero) + + except ContractCallException as exc: + # Expected error due to illegal trade + # We do add an argument for invalid balance to the args, so check for that here + assert "Invalid balance:" in exc.args[0] + # Fails on add liquidity + assert exc.function_name_or_signature == "addLiquidity" + # This throws panic error under the hood + assert exc.orig_exception is not None + assert isinstance(exc.orig_exception, ContractPanicError) + assert ( + exc.orig_exception.args[0] == "Panic error 0x11: Arithmetic operation results in underflow or overflow." + ) + + def test_invalid_remove_liquidity_from_zero( + self, + local_hyperdrive_pool: DeployedHyperdrivePool, + ): + """Tests when making a invalid remove liquidity with zero lp tokens""" + try: + self._build_and_run_with_funded_bot(local_hyperdrive_pool, InvalidRemoveLiquidityFromZero) + + except ContractCallException as exc: + # Expected error due to illegal trade + # We do add an argument for invalid balance to the args, so check for that here + assert "Invalid balance:" in exc.args[0] + # Fails on remove liquidity + assert exc.function_name_or_signature == "removeLiquidity" + # This throws panic error under the hood + assert exc.orig_exception is not None + assert isinstance(exc.orig_exception, ContractPanicError) + assert ( + exc.orig_exception.args[0] == "Panic error 0x11: Arithmetic operation results in underflow or overflow." + ) + + def test_invalid_close_long_from_zero( + self, + local_hyperdrive_pool: DeployedHyperdrivePool, + ): + """Tests when making a invalid close long with zero long tokens""" + try: + self._build_and_run_with_funded_bot(local_hyperdrive_pool, InvalidCloseLongFromZero) + + except ContractCallException as exc: + # Expected error due to illegal trade + # We do add an argument for invalid balance to the args, so check for that here + assert "Invalid balance:" in exc.args[0] + assert "long token not found in wallet" in exc.args[0] + # Fails on close long + assert exc.function_name_or_signature == "closeLong" + # This throws panic error under the hood + assert exc.orig_exception is not None + assert isinstance(exc.orig_exception, ContractPanicError) + assert ( + exc.orig_exception.args[0] == "Panic error 0x11: Arithmetic operation results in underflow or overflow." + ) + + def test_invalid_close_short_from_zero( + self, + local_hyperdrive_pool: DeployedHyperdrivePool, + ): + """Tests when making a invalid close long with zero long tokens""" + try: + self._build_and_run_with_funded_bot(local_hyperdrive_pool, InvalidCloseShortFromZero) + + except ContractCallException as exc: + # Expected error due to illegal trade + # We do add an argument for invalid balance to the args, so check for that here + assert "Invalid balance:" in exc.args[0] + assert "short token not found in wallet" in exc.args[0] + # Fails on close long + assert exc.function_name_or_signature == "closeShort" + # This throws panic error under the hood + assert exc.orig_exception is not None + assert isinstance(exc.orig_exception, ContractPanicError) + assert ( + exc.orig_exception.args[0] == "Panic error 0x11: Arithmetic operation results in underflow or overflow." + ) + + def test_invalid_redeem_withdraw_share_from_zero( + self, + local_hyperdrive_pool: DeployedHyperdrivePool, + ): + """Tests when making a invalid redeem withdrawal shares with zero withdrawal tokens""" + try: + self._build_and_run_with_funded_bot(local_hyperdrive_pool, InvalidRedeemWithdrawFromZero) + + # This is catching a value error, since this transaction is actually valid on the chain + # We're explicitly catching this and throwing a value error in redeem withdraw shares + except ValueError as exc: + # Expected error due to illegal trade + # We do add an argument for invalid balance to the args, so check for that here + assert "Invalid balance:" in exc.args[0] + # Error message should print out the balance of withdraw shares here + assert "balance of " in exc.args[0] + assert exc.args[1] == "Preview call for redeem withdrawal shares returned 0 for non-zero input trade amount" + + def test_invalid_remove_liquidity_from_nonzero( + self, + local_hyperdrive_pool: DeployedHyperdrivePool, + ): + """Tests when making a invalid remove liquidity trade with nonzero lp tokens""" + try: + self._build_and_run_with_funded_bot(local_hyperdrive_pool, InvalidRemoveLiquidityFromNonZero) + + except ContractCallException as exc: + # Expected error due to illegal trade + # We do add an argument for invalid balance to the args, so check for that here + assert "Invalid balance:" in exc.args[0] + # Fails on remove liquidity + assert exc.function_name_or_signature == "removeLiquidity" + # This throws panic error under the hood + assert exc.orig_exception is not None + assert isinstance(exc.orig_exception, ContractPanicError) + assert ( + exc.orig_exception.args[0] == "Panic error 0x11: Arithmetic operation results in underflow or overflow." + ) + + def test_invalid_close_long_from_nonzero( + self, + local_hyperdrive_pool: DeployedHyperdrivePool, + ): + """Tests when making a invalid close long with nonzero long tokens""" + try: + self._build_and_run_with_funded_bot(local_hyperdrive_pool, InvalidCloseLongFromNonZero) + + except ContractCallException as exc: + # Expected error due to illegal trade + # We do add an argument for invalid balance to the args, so check for that here + assert "Invalid balance:" in exc.args[0] + # Fails on closeLong + assert exc.function_name_or_signature == "closeLong" + # This throws panic error under the hood + assert exc.orig_exception is not None + assert isinstance(exc.orig_exception, ContractPanicError) + assert ( + exc.orig_exception.args[0] == "Panic error 0x11: Arithmetic operation results in underflow or overflow." + ) + + def test_invalid_close_short_from_nonzero( + self, + local_hyperdrive_pool: DeployedHyperdrivePool, + ): + """Tests when making a invalid close short with nonzero short tokens""" + try: + self._build_and_run_with_funded_bot(local_hyperdrive_pool, InvalidCloseShortFromNonZero) + + except ContractCallException as exc: + # Expected error due to illegal trade + # We do add an argument for invalid balance to the args, so check for that here + assert "Invalid balance:" in exc.args[0] + # Fails on closeShort + assert exc.function_name_or_signature == "closeShort" + # This throws panic error under the hood + assert exc.orig_exception is not None + assert isinstance(exc.orig_exception, ContractPanicError) + assert ( + exc.orig_exception.args[0] == "Panic error 0x11: Arithmetic operation results in underflow or overflow." + ) + + def test_invalid_redeem_withdraw_from_nonzero( + self, + local_hyperdrive_pool: DeployedHyperdrivePool, + ): + """Tests when making a invalid close short with nonzero short tokens""" + try: + self._build_and_run_with_funded_bot(local_hyperdrive_pool, InvalidRedeemWithdrawFromNonZero) + + # This is catching a value error, since this transaction is actually valid on the chain + # We're explicitly catching this and throwing a value error in redeem withdraw shares + except ValueError as exc: + # Expected error due to illegal trade + # We do add an argument for invalid balance to the args, so check for that here + assert "Invalid balance:" in exc.args[0] + # Error message should print out the balance of withdraw shares here + assert "balance of " in exc.args[0] + assert exc.args[1] == "Preview call for redeem withdrawal shares returned 0 for non-zero input trade amount" + + def test_invalid_redeem_withdraw_in_pool( + self, + local_hyperdrive_pool: DeployedHyperdrivePool, + ): + """Tests when making a invalid close short with nonzero short tokens""" + try: + self._build_and_run_with_funded_bot(local_hyperdrive_pool, InvalidRedeemWithdrawInPool) + + # This is catching a value error, since this transaction is actually valid on the chain + # We're explicitly catching this and throwing a value error in redeem withdraw shares + except ValueError as exc: + # Expected error due to illegal trade + # We do add an argument for invalid balance to the args, so check for that here + assert "Invalid balance:" in exc.args[0] + assert "not enough ready to withdraw" in exc.args[0] + assert exc.args[1] == "Preview call for redeem withdrawal shares returned 0 for non-zero input trade amount" From aa482479d592800f3f9b561b73c516ed7e753aed Mon Sep 17 00:00:00 2001 From: Sheng Lundquist Date: Thu, 9 Nov 2023 14:46:17 -0800 Subject: [PATCH 8/9] Removing invalid trade in multi_trade_per_block_test in favor of new test --- tests/multi_trade_per_block_test.py | 39 ++++++----------------------- 1 file changed, 8 insertions(+), 31 deletions(-) diff --git a/tests/multi_trade_per_block_test.py b/tests/multi_trade_per_block_test.py index ac509218e..7771910e8 100644 --- a/tests/multi_trade_per_block_test.py +++ b/tests/multi_trade_per_block_test.py @@ -16,7 +16,6 @@ from elfpy.types import MarketType, Trade from eth_typing import URI from ethpy import EthConfig -from ethpy.base.errors import ContractCallException from fixedpointmath import FixedPoint from numpy.random._generator import Generator as NumpyGenerator from sqlalchemy.orm import Session @@ -99,19 +98,6 @@ def action( ) ) - # Illegal trade - # TODO this trade is currently returning an uninformative assertion error - action_list.append( - Trade( - market_type=MarketType.HYPERDRIVE, - market_action=HyperdriveMarketAction( - action_type=HyperdriveActionType.REDEEM_WITHDRAW_SHARE, - trade_amount=FixedPoint(99999999999), - wallet=wallet, - ), - ) - ) - self.made_trade = True return action_list, True @@ -181,20 +167,13 @@ def test_multi_trade_per_block( # Using default abi dir ) - try: - run_agents( - env_config, - agent_config, - account_key_config, - eth_config=eth_config, - contract_addresses=hyperdrive_contract_addresses, - ) - # If this reaches this point, the agent was successful, which means this test should fail - assert False, "Agent was successful with known invalid trade" - except ContractCallException as exc: - # Expected error due to illegal trade - # We do add an argument for invalid balance to the args, so check for that here - assert "Invalid balance:" in exc.args[0] + run_agents( + env_config, + agent_config, + account_key_config, + eth_config=eth_config, + contract_addresses=hyperdrive_contract_addresses, + ) # Run acquire data to get data from chain to db acquire_data( @@ -222,14 +201,12 @@ def test_multi_trade_per_block( # 3. openShort of 33333 bonds db_transaction_info: pd.DataFrame = get_transactions(db_session, coerce_float=False) - # TODO transactions is logging the failed trade? Is this desired? - assert len(db_transaction_info == 4) + assert len(db_transaction_info == 3) # Checking without order trxs = db_transaction_info["input_method"].to_list() assert "addLiquidity" in trxs assert "openLong" in trxs assert "openShort" in trxs - assert "redeemWithdrawalShares" in trxs db_ticker: pd.DataFrame = get_ticker(db_session, coerce_float=False) assert len(db_ticker == 3) From 40fb7a296515353071372b586de00a3aae03c01d Mon Sep 17 00:00:00 2001 From: Sheng Lundquist Date: Thu, 9 Nov 2023 14:46:50 -0800 Subject: [PATCH 9/9] Adding volume pruning on cleanup --- lib/chainsync/chainsync/test_fixtures/db_session.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/chainsync/chainsync/test_fixtures/db_session.py b/lib/chainsync/chainsync/test_fixtures/db_session.py index ef2f373e8..d6a0fb853 100644 --- a/lib/chainsync/chainsync/test_fixtures/db_session.py +++ b/lib/chainsync/chainsync/test_fixtures/db_session.py @@ -8,14 +8,13 @@ import docker import pytest +from chainsync import PostgresConfig +from chainsync.db.base import Base, initialize_engine from docker.errors import DockerException from pytest_postgresql.janitor import DatabaseJanitor from sqlalchemy import Engine from sqlalchemy.orm import Session, sessionmaker -from chainsync import PostgresConfig -from chainsync.db.base import Base, initialize_engine - @pytest.fixture(scope="session") def psql_docker() -> Iterator[PostgresConfig]: @@ -77,7 +76,10 @@ def psql_docker() -> Iterator[PostgresConfig]: yield postgres_config # Docker doesn't play nice with types + # Remove the container along with volume container.kill() # type:ignore + # Prune volumes + client.volumes.prune() @pytest.fixture(scope="session")