Gearbox-protocol / core-v3

Other
28 stars 4 forks source link

fix: quota limit is bounded by `type(int96).max` #267

Closed lekhovitsky closed 1 month ago

lekhovitsky commented 1 month ago

Fixes:

StErMi commented 1 month ago

@lekhovitsky do you have any PoolQuotaKeeper instances already deployed that have any totalQuotaParams[token].limit greater than type(int96).max?

In general, I think that it would make sense to include in the deployment script a set of instructions that updates every deployed PoolQuotaKeeper and for each token in the quotaTokensSet set the totalQuotaParams[token].limit to type(int96).max if the current limit is above that value.

lekhovitsky commented 1 month ago

@StErMi No, values close to this are always a misconfig, added this constraint for formal correctness

StErMi commented 1 month ago

The new implementation applies an upper max bound to the total quota (for a token) that can be purchased by users

    /// @dev Implementation of `setTokenLimit`
    function _setTokenLimit(TokenQuotaParams storage tokenQuotaParams, address token, uint96 limit) internal {
        // ... other code

        if (limit > uint96(type(int96).max)) {
            revert IncorrectParameterException(); // U:[PQK-12]
        }

        // ... other code
    }

by doing that, all the reported casts/conversions/negations will not revert or create side effects.

Some final notes:

1) It's fine to apply this upper bound to just the _setTokenLimit function, given that when a token is added to the PQK it will be automatically added with a limit equal to 0. The PQK controller must call setTokenLimit explicitly to set a limit and enable quota purchase. 2) Gearbox must verify, for each already deployed PoolQuotaKeeper that the limit, for each token, respects the new upper bound of uint96(type(int96).max). If that's not the case, Gearbox must update the limit and verify that the amount of quota purchased for such token is not already above the new upper bound.