Skip to content

Commit

Permalink
contracts/: fix feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@bun919tw
bun919tw committed Apr 22, 2021
1 parent 2448b1e commit f634f8f
Show file tree
Hide file tree
Showing 5 changed files with 117 additions and 24 deletions.
34 changes: 29 additions & 5 deletions contracts/CCapableErc20.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ contract CCapableErc20 is CToken, CCapableErc20Interface {
* @param params The other parameters
*/
function flashLoan(address receiver, uint amount, bytes calldata params) external nonReentrant {
require(amount > 0, "flashLoan amount should be greater than zero");
require(accrueInterest() == uint(Error.NO_ERROR), "accrue interest failed");

uint cashOnChainBefore = getCashOnChain();
Expand Down Expand Up @@ -289,13 +290,12 @@ contract CCapableErc20 is CToken, CCapableErc20Interface {
}

/* Do the calculations, checking for {under,over}flow */
uint allowanceNew = sub_(startingAllowance, tokens);
accountTokens[src] = sub_(accountTokens[src], tokens);
accountTokens[dst] = add_(accountTokens[dst], tokens);

/* Eat some of the allowance (if necessary) */
if (startingAllowance != uint(-1)) {
transferAllowances[src][spender] = allowanceNew;
transferAllowances[src][spender] = sub_(startingAllowance, tokens);
}

/* We emit a Transfer event */
Expand Down Expand Up @@ -335,6 +335,14 @@ contract CCapableErc20 is CToken, CCapableErc20Interface {
return (failOpaque(Error.COMPTROLLER_REJECTION, FailureInfo.MINT_COMPTROLLER_REJECTION, allowed), 0);
}

/*
* Return if mintAmount is zero.
* Put behind `mintAllowed` for accuring potential COMP rewards.
*/
if (mintAmount == 0) {
return (uint(Error.NO_ERROR), 0);
}

/* Verify market's block number equals current block number */
if (accrualBlockNumber != getBlockNumber()) {
return (fail(Error.MARKET_NOT_FRESH, FailureInfo.MINT_FRESHNESS_CHECK), 0);
Expand Down Expand Up @@ -393,10 +401,10 @@ contract CCapableErc20 is CToken, CCapableErc20Interface {

/**
* @notice User redeems cTokens in exchange for the underlying asset
* @dev Assumes interest has already been accrued up to the current block
* @dev Assumes interest has already been accrued up to the current block. Only one of redeemTokensIn or redeemAmountIn may be non-zero and it would do nothing if both are zero.
* @param redeemer The address of the account which is redeeming the tokens
* @param redeemTokensIn The number of cTokens to redeem into underlying (only one of redeemTokensIn or redeemAmountIn may be non-zero)
* @param redeemAmountIn The number of underlying tokens to receive from redeeming cTokens (only one of redeemTokensIn or redeemAmountIn may be non-zero)
* @param redeemTokensIn The number of cTokens to redeem into underlying
* @param redeemAmountIn The number of underlying tokens to receive from redeeming cTokens
* @return uint 0=success, otherwise a failure (see ErrorReporter.sol for details)
*/
function redeemFresh(address payable redeemer, uint redeemTokensIn, uint redeemAmountIn) internal returns (uint) {
Expand Down Expand Up @@ -432,6 +440,14 @@ contract CCapableErc20 is CToken, CCapableErc20Interface {
return failOpaque(Error.COMPTROLLER_REJECTION, FailureInfo.REDEEM_COMPTROLLER_REJECTION, allowed);
}

/*
* Return if redeemTokensIn and redeemAmountIn are zero.
* Put behind `redeemAllowed` for accuring potential COMP rewards.
*/
if (redeemTokensIn == 0 && redeemAmountIn == 0) {
return uint(Error.NO_ERROR);
}

/* Verify market's block number equals current block number */
if (accrualBlockNumber != getBlockNumber()) {
return fail(Error.MARKET_NOT_FRESH, FailureInfo.REDEEM_FRESHNESS_CHECK);
Expand Down Expand Up @@ -493,6 +509,14 @@ contract CCapableErc20 is CToken, CCapableErc20Interface {
return failOpaque(Error.COMPTROLLER_REJECTION, FailureInfo.LIQUIDATE_SEIZE_COMPTROLLER_REJECTION, allowed);
}

/*
* Return if seizeTokens is zero.
* Put behind `seizeAllowed` for accuring potential COMP rewards.
*/
if (seizeTokens == 0) {
return uint(Error.NO_ERROR);
}

/* Fail if borrower = liquidator */
if (borrower == liquidator) {
return fail(Error.INVALID_ACCOUNT_PAIR, FailureInfo.LIQUIDATE_SEIZE_LIQUIDATOR_IS_BORROWER);
Expand Down
38 changes: 31 additions & 7 deletions contracts/CCollateralCapErc20.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ contract CCollateralCapErc20 is CToken, CCollateralCapErc20Interface {
* @param params The other parameters
*/
function flashLoan(address receiver, uint amount, bytes calldata params) external nonReentrant {
require(amount > 0, "flashLoan amount should be greater than zero");
require(accrueInterest() == uint(Error.NO_ERROR), "accrue interest failed");

uint cashOnChainBefore = getCashOnChain();
Expand Down Expand Up @@ -356,7 +357,7 @@ contract CCollateralCapErc20 is CToken, CCollateralCapErc20Interface {
uint bufferTokens = sub_(accountTokens[src], accountCollateralTokens[src]);
uint collateralTokens = 0;
if (tokens > bufferTokens) {
collateralTokens = sub_(tokens, bufferTokens);
collateralTokens = tokens - bufferTokens;
}

/**
Expand All @@ -382,7 +383,6 @@ contract CCollateralCapErc20 is CToken, CCollateralCapErc20Interface {
}

/* Do the calculations, checking for {under,over}flow */
uint allowanceNew = sub_(startingAllowance, tokens);
accountTokens[src] = sub_(accountTokens[src], tokens);
accountTokens[dst] = add_(accountTokens[dst], tokens);
if (collateralTokens > 0) {
Expand All @@ -395,7 +395,7 @@ contract CCollateralCapErc20 is CToken, CCollateralCapErc20Interface {

/* Eat some of the allowance (if necessary) */
if (startingAllowance != uint(-1)) {
transferAllowances[src][spender] = allowanceNew;
transferAllowances[src][spender] = sub_(startingAllowance, tokens);
}

/* We emit a Transfer event */
Expand Down Expand Up @@ -460,6 +460,14 @@ contract CCollateralCapErc20 is CToken, CCollateralCapErc20Interface {
function decreaseUserCollateralInternal(address account, uint amount) internal {
require(comptroller.redeemAllowed(address(this), account, amount) == 0, "comptroller rejection");

/*
* Return if amount is zero.
* Put behind `redeemAllowed` for accuring potential COMP rewards.
*/
if (amount == 0) {
return;
}

totalCollateralTokens = sub_(totalCollateralTokens, amount);
accountCollateralTokens[account] = sub_(accountCollateralTokens[account], amount);

Expand Down Expand Up @@ -489,6 +497,14 @@ contract CCollateralCapErc20 is CToken, CCollateralCapErc20Interface {
return (failOpaque(Error.COMPTROLLER_REJECTION, FailureInfo.MINT_COMPTROLLER_REJECTION, allowed), 0);
}

/*
* Return if mintAmount is zero.
* Put behind `mintAllowed` for accuring potential COMP rewards.
*/
if (mintAmount == 0) {
return (uint(Error.NO_ERROR), 0);
}

/* Verify market's block number equals current block number */
if (accrualBlockNumber != getBlockNumber()) {
return (fail(Error.MARKET_NOT_FRESH, FailureInfo.MINT_FRESHNESS_CHECK), 0);
Expand Down Expand Up @@ -552,10 +568,10 @@ contract CCollateralCapErc20 is CToken, CCollateralCapErc20Interface {

/**
* @notice User redeems cTokens in exchange for the underlying asset
* @dev Assumes interest has already been accrued up to the current block
* @dev Assumes interest has already been accrued up to the current block. Only one of redeemTokensIn or redeemAmountIn may be non-zero and it would do nothing if both are zero.
* @param redeemer The address of the account which is redeeming the tokens
* @param redeemTokensIn The number of cTokens to redeem into underlying (only one of redeemTokensIn or redeemAmountIn may be non-zero)
* @param redeemAmountIn The number of underlying tokens to receive from redeeming cTokens (only one of redeemTokensIn or redeemAmountIn may be non-zero)
* @param redeemTokensIn The number of cTokens to redeem into underlying
* @param redeemAmountIn The number of underlying tokens to receive from redeeming cTokens
* @return uint 0=success, otherwise a failure (see ErrorReporter.sol for details)
*/
function redeemFresh(address payable redeemer, uint redeemTokensIn, uint redeemAmountIn) internal returns (uint) {
Expand Down Expand Up @@ -597,7 +613,7 @@ contract CCollateralCapErc20 is CToken, CCollateralCapErc20Interface {
uint bufferTokens = sub_(accountTokens[redeemer], accountCollateralTokens[redeemer]);
uint collateralTokens = 0;
if (vars.redeemTokens > bufferTokens) {
collateralTokens = sub_(vars.redeemTokens, bufferTokens);
collateralTokens = vars.redeemTokens - bufferTokens;
}

/* Verify market's block number equals current block number */
Expand Down Expand Up @@ -668,6 +684,14 @@ contract CCollateralCapErc20 is CToken, CCollateralCapErc20Interface {
return failOpaque(Error.COMPTROLLER_REJECTION, FailureInfo.LIQUIDATE_SEIZE_COMPTROLLER_REJECTION, allowed);
}

/*
* Return if seizeTokens is zero.
* Put behind `seizeAllowed` for accuring potential COMP rewards.
*/
if (seizeTokens == 0) {
return uint(Error.NO_ERROR);
}

/* Fail if borrower = liquidator */
if (borrower == liquidator) {
return fail(Error.INVALID_ACCOUNT_PAIR, FailureInfo.LIQUIDATE_SEIZE_LIQUIDATOR_IS_BORROWER);
Expand Down
33 changes: 28 additions & 5 deletions contracts/CErc20.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,13 +225,12 @@ contract CErc20 is CToken, CErc20Interface {
}

/* Do the calculations, checking for {under,over}flow */
uint allowanceNew = sub_(startingAllowance, tokens);
accountTokens[src] = sub_(accountTokens[src], tokens);
accountTokens[dst] = add_(accountTokens[dst], tokens);

/* Eat some of the allowance (if necessary) */
if (startingAllowance != uint(-1)) {
transferAllowances[src][spender] = allowanceNew;
transferAllowances[src][spender] = sub_(startingAllowance, tokens);
}

/* We emit a Transfer event */
Expand Down Expand Up @@ -271,6 +270,14 @@ contract CErc20 is CToken, CErc20Interface {
return (failOpaque(Error.COMPTROLLER_REJECTION, FailureInfo.MINT_COMPTROLLER_REJECTION, allowed), 0);
}

/*
* Return if mintAmount is zero.
* Put behind `mintAllowed` for accuring potential COMP rewards.
*/
if (mintAmount == 0) {
return (uint(Error.NO_ERROR), 0);
}

/* Verify market's block number equals current block number */
if (accrualBlockNumber != getBlockNumber()) {
return (fail(Error.MARKET_NOT_FRESH, FailureInfo.MINT_FRESHNESS_CHECK), 0);
Expand Down Expand Up @@ -329,10 +336,10 @@ contract CErc20 is CToken, CErc20Interface {

/**
* @notice User redeems cTokens in exchange for the underlying asset
* @dev Assumes interest has already been accrued up to the current block
* @dev Assumes interest has already been accrued up to the current block. Only one of redeemTokensIn or redeemAmountIn may be non-zero and it would do nothing if both are zero.
* @param redeemer The address of the account which is redeeming the tokens
* @param redeemTokensIn The number of cTokens to redeem into underlying (only one of redeemTokensIn or redeemAmountIn may be non-zero)
* @param redeemAmountIn The number of underlying tokens to receive from redeeming cTokens (only one of redeemTokensIn or redeemAmountIn may be non-zero)
* @param redeemTokensIn The number of cTokens to redeem into underlying
* @param redeemAmountIn The number of underlying tokens to receive from redeeming cTokens
* @return uint 0=success, otherwise a failure (see ErrorReporter.sol for details)
*/
function redeemFresh(address payable redeemer, uint redeemTokensIn, uint redeemAmountIn) internal returns (uint) {
Expand Down Expand Up @@ -368,6 +375,14 @@ contract CErc20 is CToken, CErc20Interface {
return failOpaque(Error.COMPTROLLER_REJECTION, FailureInfo.REDEEM_COMPTROLLER_REJECTION, allowed);
}

/*
* Return if redeemTokensIn and redeemAmountIn are zero.
* Put behind `redeemAllowed` for accuring potential COMP rewards.
*/
if (redeemTokensIn == 0 && redeemAmountIn == 0) {
return uint(Error.NO_ERROR);
}

/* Verify market's block number equals current block number */
if (accrualBlockNumber != getBlockNumber()) {
return fail(Error.MARKET_NOT_FRESH, FailureInfo.REDEEM_FRESHNESS_CHECK);
Expand Down Expand Up @@ -429,6 +444,14 @@ contract CErc20 is CToken, CErc20Interface {
return failOpaque(Error.COMPTROLLER_REJECTION, FailureInfo.LIQUIDATE_SEIZE_COMPTROLLER_REJECTION, allowed);
}

/*
* Return if seizeTokens is zero.
* Put behind `seizeAllowed` for accuring potential COMP rewards.
*/
if (seizeTokens == 0) {
return uint(Error.NO_ERROR);
}

/* Fail if borrower = liquidator */
if (borrower == liquidator) {
return fail(Error.INVALID_ACCOUNT_PAIR, FailureInfo.LIQUIDATE_SEIZE_LIQUIDATOR_IS_BORROWER);
Expand Down
18 changes: 18 additions & 0 deletions contracts/CToken.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,15 @@ contract CToken is CTokenInterface, Exponential, TokenErrorReporter {
return failOpaque(Error.COMPTROLLER_REJECTION, FailureInfo.BORROW_COMPTROLLER_REJECTION, allowed);
}

/*
* Return if borrowAmount is zero.
* Put behind `borrowAllowed` for accuring potential COMP rewards.
*/
if (borrowAmount == 0) {
accountBorrows[borrower].interestIndex = borrowIndex;
return uint(Error.NO_ERROR);
}

/* Verify market's block number equals current block number */
if (accrualBlockNumber != getBlockNumber()) {
return fail(Error.MARKET_NOT_FRESH, FailureInfo.BORROW_FRESHNESS_CHECK);
Expand Down Expand Up @@ -545,6 +554,15 @@ contract CToken is CTokenInterface, Exponential, TokenErrorReporter {
return (failOpaque(Error.COMPTROLLER_REJECTION, FailureInfo.REPAY_BORROW_COMPTROLLER_REJECTION, allowed), 0);
}

/*
* Return if repayAmount is zero.
* Put behind `repayBorrowAllowed` for accuring potential COMP rewards.
*/
if (repayAmount == 0) {
accountBorrows[borrower].interestIndex = borrowIndex;
return (uint(Error.NO_ERROR), 0);
}

/* Verify market's block number equals current block number */
if (accrualBlockNumber != getBlockNumber()) {
return (fail(Error.MARKET_NOT_FRESH, FailureInfo.REPAY_BORROW_FRESHNESS_CHECK), 0);
Expand Down
18 changes: 11 additions & 7 deletions contracts/Comptroller.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ contract Comptroller is ComptrollerV5Storage, ComptrollerInterface, ComptrollerE
return failOpaque(Error.REJECTION, FailureInfo.EXIT_MARKET_REJECTION, allowed);
}

Market storage marketToExit = markets[address(cToken)];
Market storage marketToExit = markets[cTokenAddress];

/* Return true if the sender is not already ‘in’ the market */
if (!marketToExit.accountMembership[msg.sender]) {
Expand Down Expand Up @@ -210,7 +210,9 @@ contract Comptroller is ComptrollerV5Storage, ComptrollerInterface, ComptrollerE

// copy last item in list to location of item to be removed, reduce length by 1
CToken[] storage storedList = accountAssets[msg.sender];
storedList[assetIndex] = storedList[storedList.length - 1];
if (assetIndex != storedList.length - 1){
storedList[assetIndex] = storedList[storedList.length - 1];
}
storedList.length--;

if (marketToExit.version == Version.COLLATERALCAP) {
Expand All @@ -235,9 +237,6 @@ contract Comptroller is ComptrollerV5Storage, ComptrollerInterface, ComptrollerE
// Pausing is a very serious situation - we revert to sound the alarms
require(!mintGuardianPaused[cToken], "mint is paused");

// Shh - currently unused
minter;

if (!markets[cToken].isListed) {
return uint(Error.MARKET_NOT_LISTED);
}
Expand Down Expand Up @@ -367,6 +366,9 @@ contract Comptroller is ComptrollerV5Storage, ComptrollerInterface, ComptrollerE
if (err != Error.NO_ERROR) {
return uint(err);
}
if (markets[cToken].version == Version.COLLATERALCAP) {
CCollateralCapErc20Interface(cToken).registerCollateral(borrower);
}

// it should be impossible to break the important invariant
assert(markets[cToken].accountMembership[borrower]);
Expand Down Expand Up @@ -998,7 +1000,8 @@ contract Comptroller is ComptrollerV5Storage, ComptrollerInterface, ComptrollerE

/**
* @notice Set the given supply caps for the given cToken markets. Supplying that brings total supplys to or above supply cap will revert.
* @dev Admin or supplyCapGuardian function to set the supply caps. A supply cap of 0 corresponds to unlimited supplying.
* @dev Admin or supplyCapGuardian function to set the supply caps. A supply cap of 0 corresponds to unlimited supplying. If the total borrows
* already exceeded the cap, it will prevent anyone to borrow.
* @param cTokens The addresses of the markets (tokens) to change the supply caps for
* @param newSupplyCaps The new supply cap values in underlying to be set. A value of 0 corresponds to unlimited supplying.
*/
Expand All @@ -1018,7 +1021,8 @@ contract Comptroller is ComptrollerV5Storage, ComptrollerInterface, ComptrollerE

/**
* @notice Set the given borrow caps for the given cToken markets. Borrowing that brings total borrows to or above borrow cap will revert.
* @dev Admin or borrowCapGuardian function to set the borrow caps. A borrow cap of 0 corresponds to unlimited borrowing.
* @dev Admin or borrowCapGuardian function to set the borrow caps. A borrow cap of 0 corresponds to unlimited borrowing. If the total supplies
* already exceeded the cap, it will prevent anyone to mint.
* @param cTokens The addresses of the markets (tokens) to change the borrow caps for
* @param newBorrowCaps The new borrow cap values in underlying to be set. A value of 0 corresponds to unlimited borrowing.
*/
Expand Down

0 comments on commit f634f8f

Please sign in to comment.