0x37 - Penalized funding received token will be locked in the contract

[sherlock-admin4]

0x37

Medium

Penalized funding received token will be locked in the contract

Summary

If one position's funding received is penalized, these funding received will be locked in the contract.

Vulnerability Detail

When we close one position, we will check the remaining collateral after we deduct the borrowing fee and possible funding paid. If there is not left collateral, the funding received will be set to 0 as one penalty.
In this case, the trader will not receive the funding received. And the funding_received_want will not be deducted from base_collateral considering one long position.
The problem is that traders with short position will still pay funding fees. This will cause funding_received_want will be left and anyone cannot withdraw this part.

@external
@view
def calc_fees(id: uint256) -> FeesPaid:
  pos             : PositionState = Positions(self).lookup(id)
  pool            : PoolState     = self.POOLS.lookup(pos.pool)
  # Here collateral is this position's collateral.
  fees            : SumFees       = self.FEES.calc(
                                    pos.pool, pos.long, pos.collateral, pos.opened_at)
  # @audit-fp funding_paid, borrowing_paid will be paid via collateral.
  c0              : uint256       = pos.collateral
  # Funding paid and borrowing paid will be paid via the collateral.
  c1              : Val           = self.deduct(c0,           fees.funding_paid)
  c2              : Val           = self.deduct(c1.remaining, fees.borrowing_paid)
  # Funding fees prioritized over borrowing fees.
  # deduct funding fee at first, and then deduct borrowing fee.
  funding_paid    : uint256       = c1.deducted
  borrowing_paid  : uint256       = c2.deducted
  # collateral - funding paid fee - borrowing fee
  remaining       : uint256       = c2.remaining
  funding_received: uint256       = 0 if remaining == 0 else (
                                    min(fees.funding_received, avail) )

@external
@view
def value(id: uint256, ctx: Ctx) -> PositionValue:
  # Get position state.
  pos   : PositionState = Positions(self).lookup(id)
  # All positions will eventually become liquidatable due to fees.
  fees  : FeesPaid      = Positions(self).calc_fees(id)
  pnl   : PnL           = Positions(self).calc_pnl(id, ctx, fees.remaining)

  deltas: Deltas        = Deltas({
    base_interest   : [self.MATH.MINUS(pos.interest)], # unlock LP tokens.
    quote_interest  : [],
    base_transfer   : [self.MATH.PLUS(pnl.payout),
                       self.MATH.PLUS(fees.funding_received)],
    base_reserves   : [self.MATH.MINUS(pnl.payout)], # base reserve: when traders win, they will get LP's base token,
                                                     # funding fee is not related with LP holders.

@>    base_collateral : [self.MATH.MINUS(fees.funding_received)], # ->
  ...
  }) if pos.long else ...

Impact

Penalized funding received token will be locked in the contract

Code Snippet

https://github.com/sherlock-audit/2024-08-velar-artha/blob/main/gl-sherlock/contracts/positions.vy#L242-L272

Tool used

Manual Review

Recommendation

Once the funding received is penalized, we can consider to transfer this part funds to collector directly.

Duplicate of #83

[ami0x226]

This is low severity because the position should be liquidated before collateral becomes zero by liquidation bot like #93

[kennedy1030]

Escalate

This issue should be low/info because the position should be liquidated before collateral becomes zero by liquidation bot like #93 .

[sherlock-admin3]

Escalate

This issue should be low/info because the position should be liquidated before collateral becomes zero by liquidation bot like #93 .

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[WangSecurity]

For now, I'll consider the question about bots a bit more.
But, who should've received the funding fees in this case? Or they're just stuck?

[rickkk137]

invalid
in short cases funding_paid will be added to base_collateral and and long cases which will be added to quote_collateral

def value(id: uint256, ctx: Ctx) -> PositionValue:
  ...   
  deltas: Deltas        = Deltas({
    base_interest   : [self.MATH.MINUS(pos.interest)],
    quote_interest  : [],

    base_transfer   : [self.MATH.PLUS(pnl.payout),
                       # funding_received is capped at collateral and we
                       # already have those tokens
                       self.MATH.PLUS(fees.funding_received)],
    base_reserves   : [self.MATH.MINUS(pnl.payout)],
    base_collateral : [self.MATH.MINUS(fees.funding_received)], # ->

    quote_transfer  : [],
                      # in the worst case describe above, reserves
                      # dont change
    quote_reserves  : [self.MATH.PLUS(pos.collateral), #does not need min()
                       self.MATH.MINUS(fees.funding_paid)],
@>>>>    quote_collateral: [self.MATH.PLUS(fees.funding_paid),
                       self.MATH.MINUS(pos.collateral)],
  }) if pos.long else  Deltas({
    base_interest   : [],
    quote_interest  : [self.MATH.MINUS(pos.interest)],

    base_transfer   : [],
    base_reserves   : [self.MATH.PLUS(pos.collateral),
                       self.MATH.MINUS(fees.funding_paid)],
@>>>    base_collateral : [self.MATH.PLUS(fees.funding_paid), # <-
                       self.MATH.MINUS(pos.collateral)],

    quote_transfer  : [self.MATH.PLUS(pnl.payout),
                       self.MATH.PLUS(fees.funding_received)],
    quote_reserves  : [self.MATH.MINUS(pnl.payout)],
    quote_collateral: [self.MATH.MINUS(fees.funding_received)],
  })

when quote_collateral and base_collateral will be increases, hence other position's funding_received will be increased

paid_long_term      : uint256 = self.apply(fs.long_collateral, fs.funding_long * new_terms)

paid_short_term     : uint256 = self.apply(fs.short_collateral, fs.funding_short * new_terms)

[WangSecurity]

Hm, as I understand, the above comment is correct and this issue is invalid. Correct us if we're wrong. Planning to accept the escalation and invalidate this issue.

[WangSecurity]

Result:
Invalid
Unique

[sherlock-admin4]

Escalations have been resolved successfully!

Escalation status:

• kennedy1030: accepted

[WangSecurity]

This issue is duplicated with #83, look into this comment for details #83 (comment)