agent0 icon indicating copy to clipboard operation
agent0 copied to clipboard

improve interface calc functions to not require deepcopy(PoolState)

Open dpaiton opened this issue 1 year ago • 0 comments

Problem description

Presently if we want to test making changes to a PoolState object we have to deepcopy it before applying an update. This is required to then use functions such as interface.calc_fixed_rate(pool_state).

Example

For example:

current_pool_state = interface.current_pool_state

# keep iterating until we come up with a good trade
trade_is_bad = True
while trade_is_bad:
  # compute deltas for some trade attempt
  delta_bonds, delta_shares = get_mock_deltas(trade_parameters)
  # we don't want to actually apply the update
  temp_pool_state = apply_update(deepcopy(current_pool_state))
  # fixed rate calculation does not use the whole state, but requires it as input anyway
  new_fixed_rate = interface.calc_fixed_rate(temp_pool_state)
  # some check using the interface calculation
  trade_is_bad = check_trade_goodness(new_fixed_rate)
  # update parameters if we need to try again
  if trade_is_bad:
    trade_parameters = some_new_parameters()

In this example, the apply_update function is only done locally to facilitate calculating a fixed rate on a temporary updated pool state.

Reasons for the current implementation

  • The initial decision to have interface.calc_* functions take pool_state as input was to match up with the underlying hyperdrive-rs, which implements all public functions for a unified State object.
  • This has an added benefit of simplifying and unifying the arguments across all calc_* functions
  • It relieves the user from having to know what variables are required for any given calculation
  • If we wanted to change away from this setup to more explicit arguments, and we want to "do it right", then we would should go all the way back to hyperdrive-rs and propagate the change up to the Interface.

Reasons to change

  • The primary reason is cited in the example: it is cumbersome and inefficient to duplicate and update a PoolState object in a temporary setting.
  • With good docstrings, variable names, and an IDE the user should be able to infer what PoolState variables they need. So hiding it "under the hood" is not much of a befefit.
  • More explicit arguments (e.g. calc_fixed_rate(position_duraiton, spot_price)) might result in easier-to-read code.

Possible solutions

One solution is to refactor the whole codebase to be more explicit in the arguments used, which is a good amount of work.

The only way I can think of to productively improve the situation without a refactor is to add slots to the PoolState object to speed up deepcopy.

Two other solutions that are (I think) purely cosmetic would be to:

  • create a PoolState.return_updated_state(**kwargs) member function that instantiates a new PoolState with only kwarg attributes modified. This would provide a more consistent interface (user no longer has to know when it's a good idea to deepcopy), but is probably not any faster.
  • write override functions in the ReadInterface to build a PoolState whenever a version of the function is called using explicit arguments instead of a PoolState object.

dpaiton avatar Mar 11 '24 07:03 dpaiton