Skip to content

Commit

Permalink
Represent present value with 256 bits instead of 104 (#454)
Browse files Browse the repository at this point in the history 
* Represent present value with 256 bits instead of 104

This internally uses 256 bits for present values instead of artificially constraining them to 104 bits.
This avoids an unsafe cast we were doing previously to save gas, and also opens up some other gas optimizations / contract size reductions.
However, this also removes some previously unchecked math, which makes it a slight tradeoff in terms of gas.

* Up price to 256 bits as well

* Remove extraneous safe unsigned cast since we know sign of value

As pointed out by ChainSecurity.
  • Loading branch information
@jflatow
jflatow committed Jul 18, 2022
1 parent c33bed4 commit aa92a19
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 67 deletions.
76 changes: 35 additions & 41 deletions contracts/Comet.sol
View file
Expand Up @@ -106,17 +106,15 @@ contract Comet is CometMainInterface {
/// @dev uint64
uint public override immutable baseTrackingBorrowSpeed;

/// @notice The minimum amount of base wei for rewards to accrue
/// @notice The minimum amount of base principal wei for rewards to accrue
/// @dev This must be large enough so as to prevent division by base wei from overflowing the 64 bit indices
/// @dev uint104
uint public override immutable baseMinForRewards;

/// @notice The minimum base amount required to initiate a borrow
/// @dev uint104
uint public override immutable baseBorrowMin;

/// @notice The minimum base token reserves which must be held before collateral is hodled
/// @dev uint104
uint public override immutable targetReserves;

/// @notice The number of decimals for wrapped base token
Expand Down Expand Up @@ -498,10 +496,10 @@ contract Comet is CometMainInterface {
* @param priceFeed The address of a price feed
* @return The price, scaled by `PRICE_SCALE`
*/
function getPrice(address priceFeed) override public view returns (uint128) {
function getPrice(address priceFeed) override public view returns (uint256) {
(, int price, , , ) = AggregatorV3Interface(priceFeed).latestRoundData();
if (price <= 0 || price > type(int128).max) revert BadPrice();
return uint128(int128(price));
if (price <= 0) revert BadPrice();
return uint256(price);
}

/**
Expand All @@ -510,9 +508,9 @@ contract Comet is CometMainInterface {
function getReserves() override public view returns (int) {
(uint64 baseSupplyIndex_, uint64 baseBorrowIndex_) = accruedInterestIndices(getNowInternal() - lastAccrualTime);
uint balance = ERC20(baseToken).balanceOf(address(this));
uint104 totalSupply_ = presentValueSupply(baseSupplyIndex_, totalSupplyBase);
uint104 totalBorrow_ = presentValueBorrow(baseBorrowIndex_, totalBorrowBase);
return signed256(balance) - signed104(totalSupply_) + signed104(totalBorrow_);
uint totalSupply_ = presentValueSupply(baseSupplyIndex_, totalSupplyBase);
uint totalBorrow_ = presentValueBorrow(baseBorrowIndex_, totalBorrowBase);
return signed256(balance) - signed256(totalSupply_) + signed256(totalBorrow_);
}

/**
Expand Down Expand Up @@ -711,19 +709,15 @@ contract Comet is CometMainInterface {
/**
* @dev Multiply a `fromScale` quantity by a price, returning a common price quantity
*/
function mulPrice(uint128 n, uint128 price, uint64 fromScale) internal pure returns (uint) {
unchecked {
return uint256(n) * price / fromScale;
}
function mulPrice(uint n, uint price, uint64 fromScale) internal pure returns (uint) {
return n * price / fromScale;
}

/**
* @dev Multiply a signed `fromScale` quantity by a price, returning a common price quantity
*/
function signedMulPrice(int128 n, uint128 price, uint64 fromScale) internal pure returns (int) {
unchecked {
return n * signed256(price) / signed256(fromScale);
}
function signedMulPrice(int n, uint price, uint64 fromScale) internal pure returns (int) {
return n * signed256(price) / int256(uint256(fromScale));
}

/**
Expand Down Expand Up @@ -840,7 +834,7 @@ contract Comet is CometMainInterface {
if (amount == type(uint256).max) {
amount = borrowBalanceOf(dst);
}
return supplyBase(from, dst, safe104(amount));
return supplyBase(from, dst, amount);
} else {
return supplyCollateral(from, dst, asset, safe128(amount));
}
Expand All @@ -849,14 +843,14 @@ contract Comet is CometMainInterface {
/**
* @dev Supply an amount of base asset from `from` to dst
*/
function supplyBase(address from, address dst, uint104 amount) internal {
function supplyBase(address from, address dst, uint256 amount) internal {
doTransferIn(baseToken, from, amount);

accrueInternal();

UserBasic memory dstUser = userBasic[dst];
int104 dstPrincipal = dstUser.principal;
int104 dstBalance = presentValue(dstPrincipal) + signed104(amount);
int256 dstBalance = presentValue(dstPrincipal) + signed256(amount);
int104 dstPrincipalNew = principalValue(dstBalance);

(uint104 repayAmount, uint104 supplyAmount) = repayAndSupplyAmount(dstPrincipal, dstPrincipalNew);
Expand Down Expand Up @@ -952,7 +946,7 @@ contract Comet is CometMainInterface {
if (amount == type(uint256).max) {
amount = balanceOf(src);
}
return transferBase(src, dst, safe104(amount));
return transferBase(src, dst, amount);
} else {
return transferCollateral(src, dst, asset, safe128(amount));
}
Expand All @@ -961,16 +955,16 @@ contract Comet is CometMainInterface {
/**
* @dev Transfer an amount of base asset from src to dst, borrowing if possible/necessary
*/
function transferBase(address src, address dst, uint104 amount) internal {
function transferBase(address src, address dst, uint256 amount) internal {
accrueInternal();

UserBasic memory srcUser = userBasic[src];
UserBasic memory dstUser = userBasic[dst];

int104 srcPrincipal = srcUser.principal;
int104 dstPrincipal = dstUser.principal;
int104 srcBalance = presentValue(srcPrincipal) - signed104(amount);
int104 dstBalance = presentValue(dstPrincipal) + signed104(amount);
int256 srcBalance = presentValue(srcPrincipal) - signed256(amount);
int256 dstBalance = presentValue(dstPrincipal) + signed256(amount);
int104 srcPrincipalNew = principalValue(srcBalance);
int104 dstPrincipalNew = principalValue(dstBalance);

Expand All @@ -985,7 +979,7 @@ contract Comet is CometMainInterface {
updateBasePrincipal(dst, dstUser, dstPrincipalNew);

if (srcBalance < 0) {
if (uint104(-srcBalance) < baseBorrowMin) revert BorrowTooSmall();
if (uint256(-srcBalance) < baseBorrowMin) revert BorrowTooSmall();
if (!isBorrowCollateralized(src)) revert NotCollateralized();
}

Expand Down Expand Up @@ -1062,7 +1056,7 @@ contract Comet is CometMainInterface {
if (amount == type(uint256).max) {
amount = balanceOf(src);
}
return withdrawBase(src, to, safe104(amount));
return withdrawBase(src, to, amount);
} else {
return withdrawCollateral(src, to, asset, safe128(amount));
}
Expand All @@ -1071,12 +1065,12 @@ contract Comet is CometMainInterface {
/**
* @dev Withdraw an amount of base asset from src to `to`, borrowing if possible/necessary
*/
function withdrawBase(address src, address to, uint104 amount) internal {
function withdrawBase(address src, address to, uint256 amount) internal {
accrueInternal();

UserBasic memory srcUser = userBasic[src];
int104 srcPrincipal = srcUser.principal;
int104 srcBalance = presentValue(srcPrincipal) - signed104(amount);
int256 srcBalance = presentValue(srcPrincipal) - signed256(amount);
int104 srcPrincipalNew = principalValue(srcBalance);

(uint104 withdrawAmount, uint104 borrowAmount) = withdrawAndBorrowAmount(srcPrincipal, srcPrincipalNew);
Expand All @@ -1087,7 +1081,7 @@ contract Comet is CometMainInterface {
updateBasePrincipal(src, srcUser, srcPrincipalNew);

if (srcBalance < 0) {
if (uint104(-srcBalance) < baseBorrowMin) revert BorrowTooSmall();
if (uint256(-srcBalance) < baseBorrowMin) revert BorrowTooSmall();
if (!isBorrowCollateralized(src)) revert NotCollateralized();
}

Expand Down Expand Up @@ -1156,11 +1150,11 @@ contract Comet is CometMainInterface {

UserBasic memory accountUser = userBasic[account];
int104 oldPrincipal = accountUser.principal;
int104 oldBalance = presentValue(oldPrincipal);
int256 oldBalance = presentValue(oldPrincipal);
uint16 assetsIn = accountUser.assetsIn;

uint128 basePrice = getPrice(baseTokenPriceFeed);
uint deltaValue = 0;
uint256 basePrice = getPrice(baseTokenPriceFeed);
uint256 deltaValue = 0;

for (uint8 i = 0; i < numAssets; ) {
if (isInAsset(assetsIn, i)) {
Expand All @@ -1170,16 +1164,16 @@ contract Comet is CometMainInterface {
userCollateral[account][asset].balance = 0;
userCollateral[address(this)][asset].balance += seizeAmount;

uint value = mulPrice(seizeAmount, getPrice(assetInfo.priceFeed), assetInfo.scale);
uint256 value = mulPrice(seizeAmount, getPrice(assetInfo.priceFeed), assetInfo.scale);
deltaValue += mulFactor(value, assetInfo.liquidationFactor);

emit AbsorbCollateral(absorber, account, asset, seizeAmount, value);
}
unchecked { i++; }
}

uint104 deltaBalance = safe104(divPrice(deltaValue, basePrice, uint64(baseScale)));
int104 newBalance = oldBalance + signed104(deltaBalance);
uint256 deltaBalance = divPrice(deltaValue, basePrice, uint64(baseScale));
int256 newBalance = oldBalance + signed256(deltaBalance);
// New balance will not be negative, all excess debt absorbed by reserves
if (newBalance < 0) {
newBalance = 0;
Expand All @@ -1198,8 +1192,8 @@ contract Comet is CometMainInterface {
totalSupplyBase += supplyAmount;
totalBorrowBase -= repayAmount;

uint104 basePaidOut = unsigned104(newBalance - oldBalance);
uint valueOfBasePaidOut = mulPrice(basePaidOut, basePrice, uint64(baseScale));
uint256 basePaidOut = unsigned256(newBalance - oldBalance);
uint256 valueOfBasePaidOut = mulPrice(basePaidOut, basePrice, uint64(baseScale));
emit AbsorbDebt(absorber, account, basePaidOut, valueOfBasePaidOut);
}

Expand Down Expand Up @@ -1238,12 +1232,12 @@ contract Comet is CometMainInterface {
*/
function quoteCollateral(address asset, uint baseAmount) override public view returns (uint) {
AssetInfo memory assetInfo = getAssetInfoByAddress(asset);
uint128 assetPrice = getPrice(assetInfo.priceFeed);
uint256 assetPrice = getPrice(assetInfo.priceFeed);
// Store front discount is derived from the collateral asset's liquidationFactor and storeFrontPriceFactor
// discount = storeFrontPriceFactor * (1e18 - liquidationFactor)
uint discountFactor = mulFactor(storeFrontPriceFactor, FACTOR_SCALE - assetInfo.liquidationFactor);
uint128 assetPriceDiscounted = uint128(mulFactor(assetPrice, FACTOR_SCALE - discountFactor));
uint128 basePrice = getPrice(baseTokenPriceFeed);
uint256 discountFactor = mulFactor(storeFrontPriceFactor, FACTOR_SCALE - assetInfo.liquidationFactor);
uint256 assetPriceDiscounted = mulFactor(assetPrice, FACTOR_SCALE - discountFactor);
uint256 basePrice = getPrice(baseTokenPriceFeed);
// # of collateral assets
// = (TotalValueOfBaseAmount / DiscountedPriceOfCollateralAsset) * assetScale
// = ((basePrice * baseAmount / baseScale) / assetPriceDiscounted) * assetScale
Expand Down
33 changes: 16 additions & 17 deletions contracts/CometCore.sol
View file
Expand Up @@ -69,54 +69,53 @@ abstract contract CometCore is CometConfiguration, CometStorage, CometMath {

/**
* @dev The positive present supply balance if positive or the negative borrow balance if negative
* Note This will overflow at 2^103/1e18=~10 trillion for assets with 18 decimals.
*/
function presentValue(int104 principalValue_) internal view returns (int104) {
function presentValue(int104 principalValue_) internal view returns (int256) {
if (principalValue_ >= 0) {
return signed104(presentValueSupply(baseSupplyIndex, unsigned104(principalValue_)));
return signed256(presentValueSupply(baseSupplyIndex, uint104(principalValue_)));
} else {
return -signed104(presentValueBorrow(baseBorrowIndex, unsigned104(-principalValue_)));
return -signed256(presentValueBorrow(baseBorrowIndex, uint104(-principalValue_)));
}
}

/**
* @dev The principal amount projected forward by the supply index
* Note This will overflow at 2^104/1e18=~20 trillion for assets with 18 decimals.
*/
function presentValueSupply(uint64 baseSupplyIndex_, uint104 principalValue_) internal pure returns (uint104) {
return uint104(uint(principalValue_) * baseSupplyIndex_ / BASE_INDEX_SCALE);
function presentValueSupply(uint64 baseSupplyIndex_, uint104 principalValue_) internal pure returns (uint256) {
return uint256(principalValue_) * baseSupplyIndex_ / BASE_INDEX_SCALE;
}

/**
* @dev The principal amount projected forward by the borrow index
* Note This will overflow at 2^104/1e18=~20 trillion for assets with 18 decimals.
*/
function presentValueBorrow(uint64 baseBorrowIndex_, uint104 principalValue_) internal pure returns (uint104) {
return uint104(uint(principalValue_) * baseBorrowIndex_ / BASE_INDEX_SCALE);
function presentValueBorrow(uint64 baseBorrowIndex_, uint104 principalValue_) internal pure returns (uint256) {
return uint256(principalValue_) * baseBorrowIndex_ / BASE_INDEX_SCALE;
}

/**
* @dev The positive principal if positive or the negative principal if negative
*/
function principalValue(int104 presentValue_) internal view returns (int104) {
function principalValue(int256 presentValue_) internal view returns (int104) {
if (presentValue_ >= 0) {
return signed104(principalValueSupply(baseSupplyIndex, unsigned104(presentValue_)));
return signed104(principalValueSupply(baseSupplyIndex, uint256(presentValue_)));
} else {
return -signed104(principalValueBorrow(baseBorrowIndex, unsigned104(-presentValue_)));
return -signed104(principalValueBorrow(baseBorrowIndex, uint256(-presentValue_)));
}
}

/**
* @dev The present value projected backward by the supply index (rounded down)
* Note: This will overflow (revert) at 2^104/1e18=~20 trillion principal for assets with 18 decimals.
*/
function principalValueSupply(uint64 baseSupplyIndex_, uint104 presentValue_) internal pure returns (uint104) {
return uint104((uint(presentValue_) * BASE_INDEX_SCALE) / baseSupplyIndex_);
function principalValueSupply(uint64 baseSupplyIndex_, uint256 presentValue_) internal pure returns (uint104) {
return safe104((presentValue_ * BASE_INDEX_SCALE) / baseSupplyIndex_);
}

/**
* @dev The present value projected backward by the borrow index (rounded up)
* Note: This will overflow (revert) at 2^104/1e18=~20 trillion principal for assets with 18 decimals.
*/
function principalValueBorrow(uint64 baseBorrowIndex_, uint104 presentValue_) internal pure returns (uint104) {
return uint104((uint(presentValue_) * BASE_INDEX_SCALE + baseBorrowIndex_ - 1) / baseBorrowIndex_);
function principalValueBorrow(uint64 baseBorrowIndex_, uint256 presentValue_) internal pure returns (uint104) {
return safe104((presentValue_ * BASE_INDEX_SCALE + baseBorrowIndex_ - 1) / baseBorrowIndex_);
}
}
18 changes: 9 additions & 9 deletions contracts/CometMainInterface.sol
View file
Expand Up @@ -9,19 +9,19 @@ import "./CometCore.sol";
* @author Compound
*/
abstract contract CometMainInterface is CometCore {
event Supply(address indexed from, address indexed dst, uint256 amount);
event Transfer(address indexed from, address indexed to, uint256 amount);
event Withdraw(address indexed src, address indexed to, uint256 amount);
event Supply(address indexed from, address indexed dst, uint amount);
event Transfer(address indexed from, address indexed to, uint amount);
event Withdraw(address indexed src, address indexed to, uint amount);

event SupplyCollateral(address indexed from, address indexed dst, address indexed asset, uint256 amount);
event TransferCollateral(address indexed from, address indexed to, address indexed asset, uint256 amount);
event WithdrawCollateral(address indexed src, address indexed to, address indexed asset, uint256 amount);
event SupplyCollateral(address indexed from, address indexed dst, address indexed asset, uint amount);
event TransferCollateral(address indexed from, address indexed to, address indexed asset, uint amount);
event WithdrawCollateral(address indexed src, address indexed to, address indexed asset, uint amount);

/// @notice Event emitted when a borrow position is absorbed by the protocol
event AbsorbDebt(address indexed absorber, address indexed borrower, uint104 basePaidOut, uint usdValue);
event AbsorbDebt(address indexed absorber, address indexed borrower, uint basePaidOut, uint usdValue);

/// @notice Event emitted when a user's collateral is absorbed by the protocol
event AbsorbCollateral(address indexed absorber, address indexed borrower, address indexed asset, uint128 collateralAbsorbed, uint usdValue);
event AbsorbCollateral(address indexed absorber, address indexed borrower, address indexed asset, uint collateralAbsorbed, uint usdValue);

/// @notice Event emitted when a collateral asset is purchased from the protocol
event BuyCollateral(address indexed buyer, address indexed asset, uint baseAmount, uint collateralAmount);
Expand Down Expand Up @@ -56,7 +56,7 @@ abstract contract CometMainInterface is CometCore {
function getAssetInfo(uint8 i) virtual public view returns (AssetInfo memory);
function getAssetInfoByAddress(address asset) virtual public view returns (AssetInfo memory);
function getReserves() virtual public view returns (int);
function getPrice(address priceFeed) virtual public view returns (uint128);
function getPrice(address priceFeed) virtual public view returns (uint);

function isBorrowCollateralized(address account) virtual public view returns (bool);
function isLiquidatable(address account) virtual public view returns (bool);
Expand Down

0 comments on commit aa92a19

Please sign in to comment.