feat: remove omnipool stable position #1300
Conversation
|
Crate versions that have not been updated:
Crate versions that have been updated:
Runtime version has been increased. |
|
Quick benchmark at commit e1e25b3 has been executed successfully. |
| [package] | ||
| name = "pallet-omnipool-liquidity-mining" | ||
| version = "2.7.1" | ||
| version = "2.8.0" |
There was a problem hiding this comment.
it should be really be 3.0.0 because it breaks the api by adding extra parameter.
There was a problem hiding this comment.
does it really break the API? I mean, it is just an optional param in the end, AFAIK it doesn't break API.
| /// other pallets to call it and receive the transferred amount. | ||
| #[require_transactional] | ||
| pub fn do_remove_liquidity_with_limit( | ||
| origin: OriginFor<T>, |
There was a problem hiding this comment.
account instead of origin. origin can be ensured just before.
also usage of this in the omnipool-lm favors this approach.
There was a problem hiding this comment.
the thing is that on_liquidity_changed requires origin, so i think we need to keep it as is, unfortunately.
| fn remove_liquidity_one_asset( | ||
| who: AccountId, | ||
| pool_id: AssetId, | ||
| asset_id: AssetId, | ||
| share_amount: Balance, | ||
| min_amount_out: Balance, | ||
| ) -> Result<Balance, DispatchError>; | ||
|
|
||
| fn remove_liquidity( | ||
| who: AccountId, | ||
| pool_id: AssetId, | ||
| share_amount: Balance, | ||
| min_amounts_out: Vec<AssetAmount<AssetId>>, | ||
| ) -> Result<(), DispatchError>; |
There was a problem hiding this comment.
this does not make much sense here. the trait is called AddLiquidity.
There was a problem hiding this comment.
good point. Since this trait is only used in omnipool liqudity mining, i would rename it to "StableswapLiquidityMutation'
Fixes: #1294
TODOS: