Pool deletion from a pool stage cost doesn't send PendingInboundRune to the Reserve
Based on this Discord thread:
From the nature of pending RUNE, a stage cost is only taken from BalanceRune and not PendingInboundRune.
(Consider if a stage cost were partly taken from PendingInboundRune, then of two pending providers the first to complete their add liquidity added their full RUNE amount and the second added almost no RUNE, being unfair.)
When a pool stage cost depletes the last of the BalanceRune from a pool (sent from the Pool Module to the Reserve Module) and the pool is deleted,
currently the PendingInboundRune is likewise deleted while the corresponding RUNE is left in the Reserve
(changing Pool Module RUNE solvency state, namely increasing its oversolvency).
https://gitlab.com/thorchain/thornode/-/blob/v1.111.0/x/thorchain/manager_pool_current.go#L145-156
if pool.BalanceRune.IsZero() && !pool.BalanceAsset.IsZero() {
// the staged pool no longer has any rune, abandon the pool
// and liquidity provider, and burn the asset (via zero'ing
// the vaults for the asset, and churning away from the
// tokens)
ctx.Logger().Info("burning pool", "pool", pool.Asset)
// remove LPs
pm.removeLiquidityProviders(ctx, pool.Asset, mgr)
// delete the pool
mgr.Keeper().RemovePool(ctx, pool.Asset)
I welcome feedback on whether an event should be emitted; that aside, I currently propose that all PendingInboundRune be sent to the Reserve when a pool is deleted.
Specifically, after the "burning pool" log,
if !pool.PendingInboundRune.IsZero() {
if err := mgr.Keeper().AddPoolFeeToReserve(ctx, pool.PendingInboundRune); err != nil {
ctx.Logger().Error("fail to transfer pending inbound rune to reserve during pool burning", "from pool", pool.Asset, "err", err)
}
}
(Noting that calling AddPoolFeeToReserve
is slightly simpler than a full SendFromModuleToModule
from AsgardName to ReserveName.)
Specifically: !2938 (merged)
'Transfer PendingInboundRune to Reserve upon pool burning'.
(Thoughts for further discussion: Should there be similar checks for when Ragnaroking a pool or chain?
|
Should transfer of any remaining PendingInboundRune (and/oror BalanceRune) be a version-specific part of RemovePool
rather than done before any instance of RemovePool
?
--Probably not, since RemovePool
takes an Asset rather than Pool argument, and since unfortunate if there were no SetPool
to update a pool's lowered BalanceRune before the RemovePool. \ Thus, plausibly preferable to do PendingInboundRune
AddPoolFeeToReservewhenever
RemovePool` might be done, and if something's missed catch it with an alert about oversolvency (oversolvency itself relatively harmless compared to insolvency) for fixing then.
|
For Ragnarok, perhaps pending RUNE is already dealt with in automatic withdraws, unlike a stage cost where all providers are deleted directly after no RUNE is left.)