delvtech / hyperdrive-rs

Rust SDK for the Hyperdrive AMM.
https://docs.rs/crate/hyperdrive-math/latest
Apache License 2.0
2 stars 0 forks source link

Add error handling to Rust trade logic #33

Open dpaiton opened 6 months ago

dpaiton commented 6 months ago

Tasks:

Another issue (https://github.com/delvtech/hyperdrive-rs/issues/45) will address these:

When these functions are added (https://github.com/delvtech/hyperdrive/issues/873 https://github.com/delvtech/hyperdrive-rs/issues/24 https://github.com/delvtech/hyperdrive-rs/issues/22) we should ensure they also have matching errors:

Details:

The Solidity code includes error handling which the Rust SDK lacks. For instance, in Solidity, openLong reverts with a NegativeInterest error, while in Rust it incorrectly returns a valid FixedPoint number. This discrepancy can lead to issues where users might pay gas for transactions that are destined to fail. It also can introduce bugs in Python bot logic that relies on these outputs for choosing subsequent trades.

Here is a python script to reproduce the given example:

"""Demonstrate failure of rust module to produce the same errors as the solidity contracts."""
from fixedpointmath import FixedPoint
from agent0.hyperdrive.interactive import LocalChain
from agent0.interactive_fuzz.helpers import setup_fuzz

if __name__ == "__main__":
    # Setup the environment
    chain_config = LocalChain.Config
    log_to_stdout = False
    log_filename = ".logging/check_rust_error.log"
    chain, random_seed, rng, interactive_hyperdrive = setup_fuzz(
        log_filename,
        chain_config,
        log_to_stdout,
        fuzz_test_name="check_rust_error",
        flat_fee=FixedPoint(0),
        curve_fee=FixedPoint(0.001),  # 0.1%
        governance_lp_fee=FixedPoint(0),
        governance_zombie_fee=FixedPoint(0),
        var_interest=FixedPoint(0.0),
    )

    current_pool_state = interactive_hyperdrive.hyperdrive_interface.current_pool_state
    max_long = interactive_hyperdrive.hyperdrive_interface.calc_max_long(FixedPoint(1e15), current_pool_state)
    trade_amount = max_long + FixedPoint(100)  # trade more than the max
    long_agent = interactive_hyperdrive.init_agent(base=FixedPoint(1e15), eth=FixedPoint(1e3), name="joe")

    # rust version
    bonds_purchased_rust = interactive_hyperdrive.hyperdrive_interface.calc_open_long(trade_amount, current_pool_state)
    # Rust code succeeds:
    # bonds_purchased_rust = FixedPoint("99793307.650604214737873666")

    # smart contract version
    open_long_event = long_agent.open_long(trade_amount)
    # Contract throws an error: web3.exceptions.ContractCustomError: ('0x512095c7', 'ContractCustomError NegativeInterest raised.')

    # Cant assign this because it fails
    # bonds_purchased_sol = open_long_event.bond_amount

    # TODO: assert solidity error == rust error
dpaiton commented 5 months ago

We also wrap calc_max_long and calc_max_short in the random bot policy in agent0 with a try/catch because the calc_max functions fail without clear messaging or in a situation that could be easily avoided with knowledge of the pool state. We're seeing failures in the hyperdrive experiment flow as well as fuzz testing.

The calc_max process is approximate, and has a high error rate when the network is close to an extreme state where the max would be close to zero. This is easily reproducible by calling calcmax[long/short], executing the trade, and then calling it again. We want to catch this situation and return a max of zero at the rust level, so that the value returned is safe to pass directly to the smart contracts.

Related issues with crash report for this are https://github.com/delvtech/hyperdrive/issues/687 and https://github.com/delvtech/hyperdrive/issues/686

slundqui commented 5 months ago

Solving open long issue in this pr: https://github.com/delvtech/hyperdrive/pull/754

dpaiton commented 3 months ago

Upper bound (max) errors on close long & short require checkpointing ( https://github.com/delvtech/hyperdrive-rs/issues/23 ), so we are blocked on that for now.

dpaiton commented 3 months ago

I'm moving this to High priority, removing the deadline, and removing @slundqui 's assignment.

He achieved all that was possible in delvtech/hyperdrive#916 . We are blocked on checkpointing & a MockHyperdrive test utility; so the full resolution of this issue will need to be delayed.