Skip to content

Commit

Permalink
Fuzz updates (#1594)
Browse files Browse the repository at this point in the history
- Adding `DistributeExcessIdleFailed` and
`DecreasedPresentValuWhenAddingLiquidity` to ignored errors for pausing
fuzz.
- No longer logging deploy info in fuzz testing (too much info in
rollbar, can't find actual useful information)
- More accurate steth -> lido shares conversion when making trades.
- Fixing rollbar logging for status == 0 issues.
- Wrapping unit fuzz warnings in log level threshold check.
  • Loading branch information
slundqui authored Jul 3, 2024
1 parent 2f14cc4 commit 8fd0563
Show file tree
Hide file tree
Showing 8 changed files with 92 additions and 107 deletions.
19 changes: 18 additions & 1 deletion scripts/local_fuzz_bots.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@


def _fuzz_ignore_errors(exc: Exception) -> bool:
# pylint: disable=too-many-return-statements
# Ignored fuzz exceptions
if isinstance(exc, FuzzAssertionException):
# LP rate invariance check
Expand Down Expand Up @@ -53,7 +54,23 @@ def _fuzz_ignore_errors(exc: Exception) -> bool:
):
return True

# Status == 0, but preview was successful error
# DistributeExcessIdle error
if (
isinstance(orig_exception, ContractCustomError)
and len(orig_exception.args) > 1
and "DistributeExcessIdleFailed raised" in orig_exception.args[1]
):
return True

# DecreasedPresentValueWhenAddingLiquidity error
if (
isinstance(orig_exception, ContractCustomError)
and len(orig_exception.args) > 1
and "DecreasedPresentValueWhenAddingLiquidity raised" in orig_exception.args[1]
):
return True

# Status == 0
if (
# Lots of conditions to check
# pylint: disable=too-many-boolean-expressions
Expand Down
26 changes: 1 addition & 25 deletions src/agent0/core/hyperdrive/interactive/local_hyperdrive.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from __future__ import annotations

import logging
from dataclasses import asdict, dataclass
from dataclasses import dataclass
from typing import TYPE_CHECKING

import pandas as pd
Expand Down Expand Up @@ -291,30 +291,6 @@ def __init__(
chain._add_deployed_pool_to_bookkeeping(self)
self.chain = chain

# Add additional deployment info to crash report additional info
if self._deployed_hyperdrive_factory is not None:
self._crash_report_additional_info.update(
{
"factory_deployer_account": self._deployed_hyperdrive_factory.deployer_account.address,
"factory_contract": self._deployed_hyperdrive_factory.factory_contract.address,
"deployer_coor_contract": self._deployed_hyperdrive_factory.deployer_coordinator_contract.address,
"registry_contract": self._deployed_hyperdrive_factory.registry_contract.address,
"factory_deploy_config": asdict(self._deployed_hyperdrive_factory.factory_deploy_config),
}
)

if self._deployed_hyperdrive_pool is not None:
self._crash_report_additional_info.update(
{
"pool_deployer_account": self._deployed_hyperdrive_pool.deployer_account.address,
"hyperdrive_contract": self._deployed_hyperdrive_pool.hyperdrive_contract.address,
"base_token_contract": self._deployed_hyperdrive_pool.base_token_contract.address,
"vault_shares_token_contract": self._deployed_hyperdrive_pool.vault_shares_token_contract.address,
"deploy_block_number": self._deploy_block_number,
"pool_deploy_config": asdict(self._deployed_hyperdrive_pool.pool_deploy_config),
}
)

# Run the data pipeline in background threads if experimental mode
self.data_pipeline_timeout = self.config.data_pipeline_timeout

Expand Down
40 changes: 18 additions & 22 deletions src/agent0/core/hyperdrive/interactive/local_hyperdrive_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,33 +132,35 @@ def _ensure_event_matches_wallet_delta(
# pylint: disable=too-many-statements
if deploy_type == LocalHyperdrive.DeployType.ERC4626:
# We expect exact matches for erc4626
event_comparison_epsilon = FixedPoint(0)
chain_balance_comparison_epsilon = FixedPoint(0)
short_chain_balance_comparison_epsilon = FixedPoint(0)
balance_comparison_epsilon = FixedPoint(0)
short_balance_comparison_epsilon = FixedPoint(0)
else:
# There's known conversion errors in the steth market
# There's known conversion errors in the steth market.
# This comes from us using the more accurate contract call to convert
# from user input steth to contract expected lido shares,
# but using the less inaccurate conversion in the resulting event.
# TODO this conversion error for chain comparisons
# is due to the inaccuracy of converting from lido shares to steth
# via the vault share price. The more accurate conversion is to do a
# contract call for the conversion.
event_comparison_epsilon = FixedPoint(scaled_value=10)
chain_balance_comparison_epsilon = FixedPoint(scaled_value=int(3e6))
short_chain_balance_comparison_epsilon = FixedPoint(scaled_value=50)
balance_comparison_epsilon = FixedPoint(scaled_value=int(3e6))
# TODO shorts comparison epsilon is much smaller, figure out why
short_balance_comparison_epsilon = FixedPoint(scaled_value=50)

if isinstance(event, AddLiquidity):
assert isclose(trade_input, event.amount, abs_tol=event_comparison_epsilon)
assert isclose(trade_input, event.amount, abs_tol=balance_comparison_epsilon)
assert isclose(
wallet_after.balance.amount - wallet_before.balance.amount,
-event.amount,
abs_tol=chain_balance_comparison_epsilon,
abs_tol=balance_comparison_epsilon,
)
assert wallet_after.lp_tokens - wallet_before.lp_tokens == event.lp_amount
elif isinstance(event, RemoveLiquidity):
assert trade_input == event.lp_amount
assert isclose(
wallet_after.balance.amount - wallet_before.balance.amount,
event.amount,
abs_tol=chain_balance_comparison_epsilon,
abs_tol=balance_comparison_epsilon,
)
assert wallet_after.lp_tokens - wallet_before.lp_tokens == -event.lp_amount
assert wallet_after.withdraw_shares - wallet_before.withdraw_shares == event.withdrawal_share_amount
Expand All @@ -167,15 +169,15 @@ def _ensure_event_matches_wallet_delta(
assert isclose(
wallet_after.balance.amount - wallet_before.balance.amount,
event.amount,
abs_tol=chain_balance_comparison_epsilon,
abs_tol=balance_comparison_epsilon,
)
assert wallet_after.withdraw_shares - wallet_before.withdraw_shares == -event.withdrawal_share_amount
elif isinstance(event, OpenLong):
assert isclose(trade_input, event.amount, abs_tol=event_comparison_epsilon)
assert isclose(trade_input, event.amount, abs_tol=balance_comparison_epsilon)
assert isclose(
wallet_after.balance.amount - wallet_before.balance.amount,
-event.amount,
abs_tol=chain_balance_comparison_epsilon,
abs_tol=balance_comparison_epsilon,
)
# If this long existed before, check the delta
if event.maturity_time in wallet_before.longs:
Expand All @@ -191,7 +193,7 @@ def _ensure_event_matches_wallet_delta(
assert isclose(
wallet_after.balance.amount - wallet_before.balance.amount,
event.amount,
abs_tol=chain_balance_comparison_epsilon,
abs_tol=balance_comparison_epsilon,
)
if event.maturity_time in wallet_after.longs:
assert (
Expand All @@ -202,13 +204,10 @@ def _ensure_event_matches_wallet_delta(
assert wallet_before.longs[event.maturity_time].balance == event.bond_amount
elif isinstance(event, OpenShort):
assert trade_input == event.bond_amount
# Although this one is a chain balance delta, the epsilon here is much smaller,
# so we use a tighter bound here
# TODO figure out why
assert isclose(
wallet_after.balance.amount - wallet_before.balance.amount,
-event.amount,
abs_tol=short_chain_balance_comparison_epsilon,
abs_tol=short_balance_comparison_epsilon,
)
if event.maturity_time in wallet_before.shorts:
assert (
Expand All @@ -219,13 +218,10 @@ def _ensure_event_matches_wallet_delta(
assert wallet_after.shorts[event.maturity_time].balance == event.bond_amount
elif isinstance(event, CloseShort):
assert trade_input == event.bond_amount
# Although this one is a chain balance delta, the epsilon here is much smaller,
# so we use a tighter bound here
# TODO figure out why
assert isclose(
wallet_after.balance.amount - wallet_before.balance.amount,
event.amount,
abs_tol=short_chain_balance_comparison_epsilon,
abs_tol=short_balance_comparison_epsilon,
)
if event.maturity_time in wallet_after.shorts:
assert (
Expand Down
6 changes: 6 additions & 0 deletions src/agent0/ethpy/base/errors/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@ def __init__(
self.block_number = block_number
self.raw_txn = raw_txn

def __repr__(self):
# We overwrite repr here to ensure the orig exception gets printed for
# repr(ContractCallException)
# Since args is already a tuple, no need for outer parenthesis
return f"ContractCallException{self.args + (repr(self.orig_exception),)}"


def decode_error_selector_for_contract(error_selector: str, contract: Contract) -> str:
"""Decode the error selector for a contract,
Expand Down
54 changes: 19 additions & 35 deletions src/agent0/ethpy/base/transactions.py
Original file line number Diff line number Diff line change
Expand Up @@ -509,22 +509,21 @@ async def _async_send_transaction_and_wait_for_receipt(
try:
# Tracing doesn't exist in typing for some reason.
# Doing this in error checking with try/catch.
tx_hash = tx_receipt["transactionHash"]
trace = web3.tracing.trace_transaction(tx_hash) # type: ignore
if len(trace) == 0:
error_message = f"Receipt has status of 0. {tx_hash=}"
error_message = "Receipt has status of 0."
else:
# Trace gives a list of values, the last one should contain the error
error_message = trace[-1].get("error", None)
# If no trace, add back in status == 0 error
if error_message is None:
error_message = f"Receipt has status of 0. {tx_hash=}"
error_message = "Receipt has status of 0."
# TODO does this need to be BaseException?
except Exception as e: # pylint: disable=broad-exception-caught
# Don't crash in crash reporting
logging.warning("Tracing failed for handling failed status: %s", repr(e))
error_message = f"Receipt has status of 0. Error getting trace: {repr(e)}"
raise UnknownBlockError(f"{error_message}", f"{block_number=}", f"{tx_receipt=}")
raise UnknownBlockError(f"{error_message}", f"{block_number=}", f"{tx_hash=}")
return tx_receipt


Expand Down Expand Up @@ -658,7 +657,7 @@ async def _async_build_send_and_wait():
assert "block_number=" in block_number_arg
block_number = int(block_number_arg.split("block_number=")[1])

orig_exception: list[Exception] = [err]
retry_preview_exception: Exception | None = None
try:
smart_contract_preview_transaction(
contract,
Expand All @@ -670,24 +669,17 @@ async def _async_build_send_and_wait():
**fn_kwargs,
)
# If the preview was successful, then we raise this message here
raise AssertionError("Preview was successful.") # pylint: disable=raise-missing-from
raise ValueError("Preview was successful.") # pylint: disable=raise-missing-from

except Exception as preview_err: # pylint: disable=broad-except
if isinstance(preview_err, ContractCallException):
# We add a message to the preview error saying what this error is
preview_err.args = ("Previewing transaction on transaction failure:",) + (
repr(preview_err.orig_exception),
)
else:
preview_err.args = ("Previewing transaction on transaction failure:",) + (repr(preview_err),)

# We report both the exception from the transaction and the exception from the preview
orig_exception.append(preview_err)
preview_err.args = ("Retry preview result: ",) + preview_err.args
retry_preview_exception = preview_err

raise ContractCallException(
"Error in smart_contract_transact: " + err.args[0],
"Retry preview result: " + repr(orig_exception[-1]),
orig_exception=orig_exception,
repr(retry_preview_exception),
orig_exception=err,
contract_call_type=ContractCallType.TRANSACTION,
function_name_or_signature=function_name_or_signature,
fn_args=fn_args,
Expand Down Expand Up @@ -751,22 +743,21 @@ def _send_transaction_and_wait_for_receipt(
try:
# Tracing doesn't exist in typing for some reason.
# Doing this in error checking with try/catch.
tx_hash = tx_receipt["transactionHash"]
trace = web3.tracing.trace_transaction(tx_hash) # type: ignore
if len(trace) == 0:
error_message = f"Receipt has status of 0. {tx_hash=}"
error_message = "Receipt has status of 0."
else:
# Trace gives a list of values, the last one should contain the error
error_message = trace[-1].get("error", None)
# If no trace, add back in status == 0 error
if error_message is None:
error_message = f"Receipt has status of 0. {tx_hash=}"
error_message = "Receipt has status of 0."
# TODO does this need to be BaseException?
except Exception as e: # pylint: disable=broad-exception-caught
# Don't crash in crash reporting
logging.warning("Tracing failed for handling failed status: %s", repr(e))
error_message = f"Receipt has status of 0. Error getting trace: {repr(e)}"
raise UnknownBlockError(f"{error_message}", f"{block_number=}", f"{tx_receipt=}")
raise UnknownBlockError(f"{error_message}", f"{block_number=}", f"{tx_hash=}")
return tx_receipt


Expand Down Expand Up @@ -892,7 +883,7 @@ def _build_send_and_wait():
assert "block_number=" in block_number_arg
block_number = int(block_number_arg.split("block_number=")[1])

orig_exception: list[Exception] = [err]
retry_preview_exception: Exception | None = None
try:
smart_contract_preview_transaction(
contract,
Expand All @@ -904,24 +895,17 @@ def _build_send_and_wait():
**fn_kwargs,
)
# If the preview was successful, then we raise this message here
raise AssertionError("Preview was successful.") # pylint: disable=raise-missing-from
raise ValueError("Preview was successful.") # pylint: disable=raise-missing-from

except Exception as preview_err: # pylint: disable=broad-except
if isinstance(preview_err, ContractCallException):
# We add a message to the preview error saying what this error is
preview_err.args = ("Previewing transaction on transaction failure:",) + (
repr(preview_err.orig_exception),
)
else:
preview_err.args = ("Previewing transaction on transaction failure:",) + (repr(preview_err),)

# We report both the exception from the transaction and the exception from the preview
orig_exception.append(preview_err)
preview_err.args = ("Retry preview result: ",) + preview_err.args
retry_preview_exception = preview_err

raise ContractCallException(
"Error in smart_contract_transact",
"Retry preview result: " + repr(orig_exception[-1]),
orig_exception=orig_exception,
"Error in smart_contract_transact: " + err.args[0],
repr(retry_preview_exception),
orig_exception=err,
contract_call_type=ContractCallType.TRANSACTION,
function_name_or_signature=function_name_or_signature,
fn_args=fn_args,
Expand Down
20 changes: 12 additions & 8 deletions src/agent0/ethpy/hyperdrive/interface/_contract_calls.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,10 +214,12 @@ async def _async_open_long(
# Convert the trade amount from steth to lido shares
# before passing into hyperdrive
if interface.vault_is_steth:
trade_amount = trade_amount / interface.current_pool_state.pool_info.vault_share_price
# TODO the more accurate way to do this is to use the underlying `getPooledEthByShares`
# call to convert steth to shares, or by using
# trade_amount.mul_div_down(getTotalPooledEther(), getTotalShares()).
# Convert input steth into lido shares
trade_amount = FixedPoint(
scaled_value=interface.vault_shares_token_contract.functions.getSharesByPooledEth(
trade_amount.scaled_value
).call()
)

fn_args = (
trade_amount.scaled_value,
Expand Down Expand Up @@ -571,10 +573,12 @@ async def _async_add_liquidity(
# Convert the trade amount from steth to lido shares
# before passing into hyperdrive
if interface.vault_is_steth:
trade_amount = trade_amount / interface.current_pool_state.pool_info.vault_share_price
# TODO the more accurate way to do this is to use the underlying `getPooledEthByShares`
# call to convert steth to shares, or by using
# trade_amount.mul_div_down(getTotalPooledEther(), getTotalShares()).
# Convert input steth into lido shares
trade_amount = FixedPoint(
scaled_value=interface.vault_shares_token_contract.functions.getSharesByPooledEth(
trade_amount.scaled_value
).call()
)

fn_args = (
trade_amount.scaled_value,
Expand Down
30 changes: 18 additions & 12 deletions src/agent0/hyperfuzz/unit_fuzz/fuzz_path_independence.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,12 @@ def fuzz_path_independence(
if starting_checkpoint_id != ending_checkpoint_id:
message = "Trades were not closed in the same checkpoint"
logging.warning(message)
rollbar_data = {"fuzz_random_seed": random_seed}
rollbar.report_message(message, "warning", extra_data=rollbar_data)

# TODO abstract out rollbar logging into a single point in interactive hyperdrive
if chain.config.rollbar_log_level_threshold <= logging.WARNING:
rollbar_data = {"fuzz_random_seed": random_seed}
rollbar.report_message(message, "warning", extra_data=rollbar_data)

continue

# Check the reserve amounts; they should be unchanged now that all of the trades are closed
Expand Down Expand Up @@ -274,16 +278,18 @@ def fuzz_path_independence(
if not invariance_checked:
warning_message = "No invariance checks were performed due to failed paths."
logging.warning(warning_message)
rollbar_data = {
"fuzz_random_seed": random_seed,
"close_random_paths": [[trade for _, trade in path] for path in trade_paths],
"trade_event_paths": trade_event_paths,
}
rollbar.report_message(
warning_message,
"warning",
extra_data=json.loads(json.dumps(rollbar_data, indent=2, cls=ExtendedJSONEncoder)),
)
# TODO abstract out rollbar logging into a single point in interactive hyperdrive
if chain.config.rollbar_log_level_threshold <= logging.WARNING:
rollbar_data = {
"fuzz_random_seed": random_seed,
"close_random_paths": [[trade for _, trade in path] for path in trade_paths],
"trade_event_paths": trade_event_paths,
}
rollbar.report_message(
warning_message,
"warning",
extra_data=json.loads(json.dumps(rollbar_data, indent=2, cls=ExtendedJSONEncoder)),
)

# If any of the path checks broke, we throw an exception at the very end
if latest_error is not None:
Expand Down
Loading

0 comments on commit 8fd0563

Please sign in to comment.