Note: All funds are safe. No urgent action is required.
This afternoon @TylerEther messaged me that he might have found a bug related to setting a compSpeed for already listed markets with a current speed of 0. @elee and @TylerEther were able to diagnose the bug and provide a fix.
Bug 1:
When attempting to run the scenario that checks adding a compSpeed for cMKR and the mint cMKR we get an integer underflow error for the mint.
This was happening on line 1161, here compound-protocol/Comptroller.sol at master · compound-finance/compound-protocol · GitHub
function distributeSupplierComp(address cToken, address supplier) internal {
CompMarketState storage supplyState = compSupplyState[cToken];
Double memory supplyIndex = Double({mantissa: supplyState.index});
Double memory supplierIndex = Double({mantissa: compSupplierIndex[cToken][supplier]});
compSupplierIndex[cToken][supplier] = supplyIndex.mantissa;
if (supplierIndex.mantissa == 0 && supplyIndex.mantissa > 0) {
supplierIndex.mantissa = compInitialIndex;
}
Double memory deltaIndex = sub_(supplyIndex, supplierIndex); <<< RIGHT HERE!!!!
uint supplierTokens = CToken(cToken).balanceOf(supplier);
uint supplierDelta = mul_(supplierTokens, deltaIndex);
uint supplierAccrued = add_(compAccrued[supplier], supplierDelta);
compAccrued[supplier] = supplierAccrued;
emit DistributedSupplierComp(CToken(cToken), supplier, supplierDelta, supplyIndex.mantissa);
}
This happened because the supplyIndex was smaller than the supplierIndex.
Why? Looking at the code, supplyIndex is obtained through these two lines.
CompMarketState storage supplyState = compSupplyState[cToken]; <<< RIGHT HERE!!!!
Double memory supplyIndex = Double({mantissa: supplyState.index});
After some looking, we found the issue is supplyState.index was not being set to the correct number because when supplyState/borrowState already has an entry with block information, and the current compSpeed is equal to zero, we do not update the market state.
Here is the function where supplyState is set: compound-protocol/Comptroller.sol at master · compound-finance/compound-protocol · GitHub
function setCompSpeedInternal(CToken cToken, uint compSpeed) internal {
uint currentCompSpeed = compSpeeds[address(cToken)];
if (currentCompSpeed != 0) {
// note that COMP speed could be set to 0 to halt liquidity rewards for a market
Exp memory borrowIndex = Exp({mantissa: cToken.borrowIndex()});
updateCompSupplyIndex(address(cToken));
updateCompBorrowIndex(address(cToken), borrowIndex);
} else if (compSpeed != 0) {
// Add the COMP market
Market storage market = markets[address(cToken)];
require(market.isListed == true, "comp market is not listed");
if (compSupplyState[address(cToken)].index == 0 && compSupplyState[address(cToken)].block == 0) {
compSupplyState[address(cToken)] = CompMarketState({
index: compInitialIndex,
block: safe32(getBlockNumber(), "block number exceeds 32 bits")
});
}
if (compBorrowState[address(cToken)].index == 0 && compBorrowState[address(cToken)].block == 0) {
compBorrowState[address(cToken)] = CompMarketState({
index: compInitialIndex,
block: safe32(getBlockNumber(), "block number exceeds 32 bits")
});
}
}
if (currentCompSpeed != compSpeed) {
compSpeeds[address(cToken)] = compSpeed;
emit CompSpeedUpdated(cToken, compSpeed);
}
}
Importantly, look at these statements.
if (currentCompSpeed != 0) {
// note that COMP speed could be set to 0 to halt liquidity rewards for a market
Exp memory borrowIndex = Exp({mantissa: cToken.borrowIndex()});
updateCompSupplyIndex(address(cToken));
updateCompBorrowIndex(address(cToken), borrowIndex);
}
if (compSupplyState[address(cToken)].index == 0 && compSupplyState[address(cToken)].block == 0) {
compSupplyState[address(cToken)] = CompMarketState({
index: compInitialIndex,
block: safe32(getBlockNumber(), "block number exceeds 32 bits")
});
}
If the block isn’t zero, it won’t set a compMarketState and since currentCompSpeed is 0, it does not update it either.
This can be fixed with the following change (@TylerEther)
if (compSupplyState[address(cToken)].index == 0) {
compSupplyState[address(cToken)] = CompMarketState({
index: compInitialIndex,
block: safe32(getBlockNumber(), "block number exceeds 32 bits")
});
} else {
compSupplyState[address(cToken)].block = safe32(getBlockNumber(), "block number exceeds 32 bits");
}
Instead of not setting the entry when block != 0, we instead update the entry.
Bug 2
This patch also fixes another separate but related bug.
Previously, if the compSpeed for a token was set above 0 at block [A], set back to 0 at block [B], then set to positive number R at block [C], since there would have already been an index & block set, the block number would get updated with market activity, but the index would remain at the index of [B].
This meant that at block [C], rewards would be accrued from the block of the last market activity prior to [C] using the rate R.
This was incorrect, as COMP accrued at rate R should only be calculated from when it was set, aka block [C].
As the patch now properly updates the block value of the market state to [C], interest will then be corrected properly, using rate R between blocks [C] and [C]+N.