Pool cycle audit
Every mimir-controlled PoolCycle
blocks (currently about every 3 days), the set of Available
and Staged
pools is updated. Available
pools not meeting basic requirements are demoted to Staged
. Staged
pools are charged a fee. And depending on MaxAvailablePools
, one Staged
pool may be promoted to Available
. If the cap is reached, one Available
pool may be demoted. The Staged
pool with the highest RUNE depth is chosen for promotion; the Available
pool with the lowest RUNE depth is chosen for demotion.
Bugs/Issues/Refactors
-
The logic for cycling pools is split between
module.go
andhelpers.go
, which is not particularly obvious to find. I propose we promote pool cycling to a newPoolManager
, like the other functionality called bymodule.go
. (https://gitlab.com/thorchain/thornode/-/blob/develop/x/thorchain/module.go#L212-233), (https://gitlab.com/thorchain/thornode/-/blob/develop/x/thorchain/helpers.go#L515-733) -
If a pool is to be abandoned entirely, we remove the asset from all vaults and remove the pool and lps from the store. Any remaining assets are abandoned and left behind on the next churn. If any LP has pending RUNE, this is also abandoned. That RUNE is easy to reclaim. I propose we transfer any RUNE pending on the pool to the reserve. (https://gitlab.com/thorchain/thornode/-/blob/develop/x/thorchain/helpers.go#L615-632)
-
In cases where an
Available
pool is demoted toStaged
for not meeting availability criteria, the pool is added to the chopping block, even though it's already been chopped/demoted. If the pool remains on the block and is then chopped, it will callsetPool
twice and emit a duplicate pool status event. Also if for some reason chopping multiple pools would be desired, such a pool would prevent a more appropriate candidate from cycling out. I propose we do not add newly demoted pools to the chopping block, freeing up that slot in case it's needed and avoiding duplicate events. (https://gitlab.com/thorchain/thornode/-/blob/develop/x/thorchain/helpers.go#L571-587) -
Liquidity fees generated by a pool are accumulated on a per-cycle basis in
RollingLiquidityFees
. This is reset at each cycle. We update the rolling fees for all pools, however we only reset the fees for pools that are not gas assets. I propose we either stop updating rolling fees for gas assets (better performance), or reset rolling fees regardless of whether it's a gas asset. (https://gitlab.com/thorchain/thornode/-/blob/develop/x/thorchain/module.go#L212-233) -
If a pool is demoted due to not generating the minimum trading fees for the cycle, it will likely cycle back in next time. This means such a pool will continuously cycle in/out every 3 days. On cycles in which the pool is
Staged
, it will be charged a staging fee. I propose, instead of demoting these pools toStaged
, we simply charge a fee. The effect is the same but eliminates the in/out cycles. Further the minimum fee could be considered a quota and anyAvailable
pool could be chargedquota - rollingFees
.
(Edit: adding two more)
-
Rolling fees are updated each time a liquidity fee is collected. But we already track all fees for an asset in a block. We could instead update the rolling fees during end block, adding the block-level fees to rolling. This would be a once-per-block update as opposed to a once-per-swap update.
-
We only demote one
Available
pool if we've exceededMaxAvailablePools
. This works in the steady state with a constantMaxAvailablePools
. But if that were to be lowered to less than the current available pool count, sayn-k
wheren
is number of pools, it would takek
cycles to settle to the desired cap. This is a rare and unlikely scenario. But I think it would make sense to immediately trim down to the newMaxAvailablePools
. But perhaps there's an argument to do it slowly, one per cycle.
(Edit: one more, the "demote for low trading" feature is currently broken)
- The fee keeper which updates the per-cycle rolling fees miscalculates the rolling fee. Instead of adding the new
fee
to the current rolling total, it adds the total fees for that asset so far in the blockpoolFees
. If you have two fees taken in the same blockf0
andf1
, the first fee will setpoolFees = 0 + f0
androlling = 0 + (0 + f0)
, all good. But for the second fee,poolFees = f0 + f1
androlling = f0 + (f0 + f1)
. Double countingf0
. (https://gitlab.com/thorchain/thornode/-/blob/develop/x/thorchain/keeper/v1/keeper_liquidity_fees.go#L41)