Lp extension #192
Lp extension #192
Conversation
…ty is not specified
Added basic test for remove_liquidity at exact added price
…d are unspecified
There was a problem hiding this comment.
One higher-level critique: I don't know that it makes sense to have these two parallel systems for holding pool shares: nfts and the old system of ('omnipool', tkn). I think the nfts are a better and more flexible solution, and adding all this overhead to maintain backward compatibility seems worse in the long run than just refactoring things to use the new system. This works for now, but I propose simplifying in the next PR.
| delta_R = init_agent.holdings[tkn] / 2 | ||
|
|
||
| single_add_state_1, single_add_agent_1 = oamm.simulate_add_liquidity(initial_state, init_agent, delta_R, tkn) | ||
| single_add_state_2, single_add_agent_2 = oamm.simulate_add_liquidity(initial_state, init_agent, delta_R, tkn, |
There was a problem hiding this comment.
since n=1 is already tested, this is probably redundant
There was a problem hiding this comment.
This is testing adding as an NFT to adding to holdings? I don't think that's redundant?
| @@ -29,6 +30,7 @@ def __init__(self, | |||
| self.trade_strategy = trade_strategy | |||
| self.asset_list = list(self.holdings.keys()) | |||
| self.unique_id = unique_id | |||
| self.nfts = nfts or {} | |||
|
|
|||
| def __repr__(self): | |||
| precision = 10 | |||
There was a problem hiding this comment.
Would be nice to fix repr() so it can correctly display nfts
There was a problem hiding this comment.
I'd prefer to address this in a future PR
Adds the ability for LPs to have multiple LP positions in Omnipool in the same asset, by making LP positions NFTs. Backward compatible so that LP positions can still be represented by (pool_name, tkn_name) assets in agent holdings.