Skip to content

Commit

Permalink
add low level support to approveThis as well
Browse files Browse the repository at this point in the history
  • Loading branch information
Hans Wang committed Nov 9, 2023
1 parent ad99f2e commit f2707f2
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 4 deletions.
9 changes: 6 additions & 3 deletions contracts/Comet.sol
Original file line number Diff line number Diff line change
Expand Up @@ -783,7 +783,7 @@ contract Comet is CometMainInterface {
function doTransferIn(address asset, address from, uint amount) nonReentrant internal returns (uint) {

Check warning on line 783 in contracts/Comet.sol

View workflow job for this annotation

GitHub Actions / Contract linter

Visibility modifier must be first in list of modifiers
uint256 preTransferBalance = ERC20(asset).balanceOf(address(this));
(bool success, bytes memory returndata) = asset.call(abi.encodeWithSelector(ERC20.transferFrom.selector, from, address(this), amount));

Check warning on line 785 in contracts/Comet.sol

View workflow job for this annotation

GitHub Actions / Contract linter

Avoid to use low level calls
if (!success || !(returndata.length == 0 || abi.decode(returndata, (bool)))) {
if (!success || (returndata.length != 0 && !abi.decode(returndata, (bool)))) {
revert TransferInFailed();
}

Check notice

Code scanning / Semgrep

Semgrep Finding: rules.solidity.performance.use-nested-if Note

Using nested is cheaper than using && multiple check combinations.
There are more advantages, such as easier to read code and better coverage reports.
return ERC20(asset).balanceOf(address(this)) - preTransferBalance;
Expand All @@ -795,7 +795,7 @@ contract Comet is CometMainInterface {
*/
function doTransferOut(address asset, address to, uint amount) nonReentrant internal {

Check warning on line 796 in contracts/Comet.sol

View workflow job for this annotation

GitHub Actions / Contract linter

Visibility modifier must be first in list of modifiers
(bool success, bytes memory returndata) = asset.call(abi.encodeWithSelector(ERC20.transfer.selector, to, amount));

Check warning on line 797 in contracts/Comet.sol

View workflow job for this annotation

GitHub Actions / Contract linter

Avoid to use low level calls
if (!success || !(returndata.length == 0 || abi.decode(returndata, (bool)))) {
if (!success || (returndata.length != 0 && !abi.decode(returndata, (bool)))) {
revert TransferOutFailed();
}

Check notice

Code scanning / Semgrep

Semgrep Finding: rules.solidity.performance.use-nested-if Note

Using nested is cheaper than using && multiple check combinations.
There are more advantages, such as easier to read code and better coverage reports.
}
Expand Down Expand Up @@ -1287,7 +1287,10 @@ contract Comet is CometMainInterface {
function approveThis(address manager, address asset, uint amount) override external {
if (msg.sender != governor) revert Unauthorized();

ERC20(asset).approve(manager, amount);
(bool success, bytes memory returndata) = asset.call(abi.encodeWithSelector(ERC20.approve.selector, manager, amount));

Check warning on line 1290 in contracts/Comet.sol

View workflow job for this annotation

GitHub Actions / Contract linter

Avoid to use low level calls
if (!success || (returndata.length != 0 && !abi.decode(returndata, (bool)))) {
revert ApproveFailed();
}

Check notice

Code scanning / Semgrep

Semgrep Finding: rules.solidity.performance.use-nested-if Note

Using nested is cheaper than using && multiple check combinations.
There are more advantages, such as easier to read code and better coverage reports.
}

/**
Expand Down
3 changes: 2 additions & 1 deletion contracts/CometMainInterface.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import "./CometCore.sol";
abstract contract CometMainInterface is CometCore {
error Absurd();
error AlreadyInitialized();
error ApproveFailed();
error BadAsset();
error BadDecimals();
error BadDiscount();
Expand All @@ -25,14 +26,14 @@ abstract contract CometMainInterface is CometCore {
error NotForSale();
error NotLiquidatable();
error Paused();
error ReentrantCallBlocked();
error SupplyCapExceeded();
error TimestampTooLarge();
error TooManyAssets();
error TooMuchSlippage();
error TransferInFailed();
error TransferOutFailed();
error Unauthorized();
error ReentrantCallBlocked();

event Supply(address indexed from, address indexed dst, uint amount);
event Transfer(address indexed from, address indexed to, uint amount);
Expand Down

0 comments on commit f2707f2

Please sign in to comment.