GalloDaSballo - `updateFundingRate` is not accruing previous changes

[sherlock-admin]

GalloDaSballo

high

updateFundingRate is not accruing previous changes

Summary

updateFundingRate will be called often, but because intermediary values do not accrue, positions will end up not paying the appropriate amount of funding rates.

This means that accounts are not being paid by the other side

Vulnerability Detail

Intuitively: You need a gain of 25% to recoup from a loss of 20% meaning that not accruing intermediary values is not making each user paying appropriate maintenance fees

If funding rate was properly applied to all positions, then each change in funding rate should alter their value

https://github.com/sherlock-audit/2023-04-jojo/blob/main/smart-contract-EVM/contracts/impl/Perpetual.sol#L72-L86

    function balanceOf(
        address trader
    ) external view returns (int256 paper, int256 credit) {
        paper = int256(balanceMap[trader].paper);
        credit =
            paper.decimalMul(fundingRate) +
            int256(balanceMap[trader].reducedCredit);
    }

    function updateFundingRate(int256 newFundingRate) external onlyOwner {
        int256 oldFundingRate = fundingRate;
        fundingRate = newFundingRate;
        emit UpdateFundingRate(oldFundingRate, newFundingRate);
    }

More specifically:

• Up, down up

Can result in different behaviour, instead of the exact new value it should be

-> Accrue only when convenient via no-ops to get free money / accrue better yield -> You'd instead need to apply a mulitplier at index (and perhaps cover the old ones) as a way to ensure accrual has happened as intend

Impact

Code Snippet

https://github.com/sherlock-audit/2023-04-jojo/blob/main/smart-contract-EVM/contracts/lib/Operation.sol#L76-L93

https://github.com/sherlock-audit/2023-04-jojo/blob/main/smart-contract-EVM/contracts/fundingRateKeeper/FundingRateUpdateLimiter.sol#L43-L48

Tool used

Manual Review

Recommendation

Store a global index multiplier, with a given timestamp, that can be used to compare the last index from each account

This will ensure that compound funding rate changes are applied as intended and don't leak value

On High Severity

While Med may be considered, updating fundingRate is not a "optional" setting, it's a core part of the protocol, meaning the incorrect Math and the opportunity to profit from it will present itself very frequently

Coded POC

// SPDX-License-Identifier: MIT
pragma solidity 0.8.10;

import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
import {ReentrancyGuard} from "@openzeppelin/contracts/security/ReentrancyGuard.sol";

library SignedDecimalMath {
    int256 constant SignedONE = 10 ** 18;

    function decimalMul(int256 a, int256 b) internal pure returns (int256) {
        return (a * b) / SignedONE;
    }
}

contract MyContract {
    using SignedDecimalMath for int256;

    int256 public rate = 1e18;
    int256 public paper = 1e18;
    int256 public reducedCredit = 1e18;

    function setRate(int256 newRate) public {
        rate = newRate;
    }

    function neweStep() public {
        int256 credit = int256(paper).decimalMul(rate) + int256(reducedCredit) + 0;
        int128 newPaper = int128(paper) + int128(0);
        int128 newReducedCredit = int128(credit - int256(newPaper).decimalMul(rate));

        paper = int256(newPaper);
        reducedCredit = int256(newReducedCredit);
    }
}

```python

c = MyContract.deploy({"from": a[0]})

print(c.neweStep())
print(c.rate())
print(c.paper())
print(c.reducedCredit())

c.setRate(1.1e18, {"from": a[0]})
c.neweStep()
## We got a gain
c.neweStep()
print(c.rate())
print(c.paper())
print(c.reducedCredit())

## Simulate a loss
c.setRate(9e17, {"from": a[0]})

## Skip
c.neweStep()
print(c.rate())
print(c.paper())
print(c.reducedCredit())

## Back to a gain
c.setRate(1.1e18, {"from": a[0]})

c.neweStep()
print(c.rate())
print(c.paper())
print(c.reducedCredit())

As you can see changes are not compounded which means that fees are not being paid to the account

[JoscelynFarr]

If a position is opened and closed between two update Funding Rate update frequencies, the user does not need to pay the funding rate