Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Dev #9

Open
wants to merge 38 commits into
base: main
Choose a base branch
from
Open

Dev #9

wants to merge 38 commits into from

Conversation

0xHaku
Copy link

@0xHaku 0xHaku commented Oct 31, 2023

No description provided.

github-advanced-security[bot]
github-advanced-security bot found potential problems Oct 31, 2023
View reviewed changes
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Semgrep found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.

github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 13, 2024
View reviewed changes
contracts/Kompu.sol
Comment on lines +841 to +842
if (msg.sender != governor && msg.sender != pauseGuardian)
revert Unauthorized();

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.
contracts/Kompu.sol
Comment on lines +962 to +968
if (initialUserBalance == 0 && finalUserBalance != 0) {
// set bit for asset
userBasic[account].assetsIn |= (uint16(1) << assetInfo.offset);
} else if (initialUserBalance != 0 && finalUserBalance == 0) {
// clear bit for asset
userBasic[account].assetsIn &= ~(uint16(1) << assetInfo.offset);
}

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.
contracts/Kompu.sol
Comment on lines +965 to +968
} else if (initialUserBalance != 0 && finalUserBalance == 0) {
// clear bit for asset
userBasic[account].assetsIn &= ~(uint16(1) << assetInfo.offset);
}

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.
contracts/Kompu.sol
Comment on lines +1583 to +1584
if (reserves >= 0 && uint(reserves) >= targetReserves)
revert NotForSale();

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.
contracts/Kompu.sol
Comment on lines +140 to +232
constructor(Configuration memory config) {
// Sanity checks
uint8 decimals_ = ERC20(config.baseToken).decimals();
if (decimals_ > MAX_BASE_DECIMALS) revert BadDecimals();
if (config.storeFrontPriceFactor > FACTOR_SCALE) revert BadDiscount();
if (config.assetConfigs.length > MAX_ASSETS) revert TooManyAssets();
if (config.baseMinForRewards == 0) revert BadMinimum();
if (
IPriceFeed(config.baseTokenPriceFeed).decimals() !=
PRICE_FEED_DECIMALS
) revert BadDecimals();

// Copy configuration
unchecked {
governor = config.governor;
pauseGuardian = config.pauseGuardian;
baseToken = config.baseToken;
baseTokenPriceFeed = config.baseTokenPriceFeed;
extensionDelegate = config.extensionDelegate;
storeFrontPriceFactor = config.storeFrontPriceFactor;

decimals = decimals_;
baseScale = uint64(10 ** decimals_);
trackingIndexScale = config.trackingIndexScale;
if (baseScale < BASE_ACCRUAL_SCALE) revert BadDecimals();
accrualDescaleFactor = baseScale / BASE_ACCRUAL_SCALE;

baseMinForRewards = config.baseMinForRewards;
rewardKink = config.rewardKink;
baseTrackingRewardSpeed = config.baseTrackingRewardSpeed;

baseBorrowMin = config.baseBorrowMin;
targetReserves = config.targetReserves;
}

// Set interest rate model configs
unchecked {
supplyKink = config.supplyKink;
supplyPerSecondInterestRateSlopeLow =
config.supplyPerYearInterestRateSlopeLow /
SECONDS_PER_YEAR;
supplyPerSecondInterestRateSlopeHigh =
config.supplyPerYearInterestRateSlopeHigh /
SECONDS_PER_YEAR;
supplyPerSecondInterestRateBase =
config.supplyPerYearInterestRateBase /
SECONDS_PER_YEAR;
borrowKink = config.borrowKink;
borrowPerSecondInterestRateSlopeLow =
config.borrowPerYearInterestRateSlopeLow /
SECONDS_PER_YEAR;
borrowPerSecondInterestRateSlopeHigh =
config.borrowPerYearInterestRateSlopeHigh /
SECONDS_PER_YEAR;
borrowPerSecondInterestRateBase =
config.borrowPerYearInterestRateBase /
SECONDS_PER_YEAR;
}

// Set asset info
numAssets = uint8(config.assetConfigs.length);

(asset00_a, asset00_b) = getPackedAssetInternal(config.assetConfigs, 0);
(asset01_a, asset01_b) = getPackedAssetInternal(config.assetConfigs, 1);
(asset02_a, asset02_b) = getPackedAssetInternal(config.assetConfigs, 2);
(asset03_a, asset03_b) = getPackedAssetInternal(config.assetConfigs, 3);
(asset04_a, asset04_b) = getPackedAssetInternal(config.assetConfigs, 4);
(asset05_a, asset05_b) = getPackedAssetInternal(config.assetConfigs, 5);
(asset06_a, asset06_b) = getPackedAssetInternal(config.assetConfigs, 6);
(asset07_a, asset07_b) = getPackedAssetInternal(config.assetConfigs, 7);
(asset08_a, asset08_b) = getPackedAssetInternal(config.assetConfigs, 8);
(asset09_a, asset09_b) = getPackedAssetInternal(config.assetConfigs, 9);
(asset10_a, asset10_b) = getPackedAssetInternal(
config.assetConfigs,
10
);
(asset11_a, asset11_b) = getPackedAssetInternal(
config.assetConfigs,
11
);
(asset12_a, asset12_b) = getPackedAssetInternal(
config.assetConfigs,
12
);
(asset13_a, asset13_b) = getPackedAssetInternal(
config.assetConfigs,
13
);
(asset14_a, asset14_b) = getPackedAssetInternal(
config.assetConfigs,
14
);
}

Check notice

Code scanning / Semgrep

Semgrep Finding: rules.solidity.performance.non-payable-constructor Note

Consider making costructor payable to save gas.
contracts/Kompu.sol
);
}
unchecked {
i++;

Check notice

Code scanning / Semgrep

Semgrep Finding: rules.solidity.performance.use-prefix-increment-not-postfix Note

Consider using the prefix increment expression whenever the return value is not needed.
The prefix increment expression is cheaper in terms of gas.
contracts/pricefeeds/UniChainlinkPriceFeed.sol
Comment on lines +49 to +71
constructor(
address priceFeedUniswap_,
address uniswapUnderlying_,
address priceFeedChainlink_,
uint8 decimals_,
string memory description_
) {
priceFeedUniswap = priceFeedUniswap_;
uniswapUnderlying = uniswapUnderlying_;
priceFeedChainlink = priceFeedChainlink_;
uint8 priceFeedUniswapDecimals = 18;
uint8 priceFeedChainlinkDecimals = AggregatorV3Interface(
priceFeedChainlink_
).decimals();
combinedScale = signed256(
10 ** (priceFeedUniswapDecimals + priceFeedChainlinkDecimals)
);

if (decimals_ > 18) revert BadDecimals();
decimals = decimals_;
description = description_;
priceFeedScale = int256(10 ** decimals);
}

Check warning

Code scanning / Semgrep

Semgrep Finding: compound.solidity.missing-constructor-sanity-checks Warning

There're no sanity checks for the constructor argument description_.
contracts/pricefeeds/UniChainlinkPriceFeed.sol
Comment on lines +49 to +71
constructor(
address priceFeedUniswap_,
address uniswapUnderlying_,
address priceFeedChainlink_,
uint8 decimals_,
string memory description_
) {
priceFeedUniswap = priceFeedUniswap_;
uniswapUnderlying = uniswapUnderlying_;
priceFeedChainlink = priceFeedChainlink_;
uint8 priceFeedUniswapDecimals = 18;
uint8 priceFeedChainlinkDecimals = AggregatorV3Interface(
priceFeedChainlink_
).decimals();
combinedScale = signed256(
10 ** (priceFeedUniswapDecimals + priceFeedChainlinkDecimals)
);

if (decimals_ > 18) revert BadDecimals();
decimals = decimals_;
description = description_;
priceFeedScale = int256(10 ** decimals);
}

Check warning

Code scanning / Semgrep

Semgrep Finding: compound.solidity.missing-constructor-sanity-checks Warning

There're no sanity checks for the constructor argument priceFeedChainlink_.
contracts/pricefeeds/UniChainlinkPriceFeed.sol
Comment on lines +49 to +71
constructor(
address priceFeedUniswap_,
address uniswapUnderlying_,
address priceFeedChainlink_,
uint8 decimals_,
string memory description_
) {
priceFeedUniswap = priceFeedUniswap_;
uniswapUnderlying = uniswapUnderlying_;
priceFeedChainlink = priceFeedChainlink_;
uint8 priceFeedUniswapDecimals = 18;
uint8 priceFeedChainlinkDecimals = AggregatorV3Interface(
priceFeedChainlink_
).decimals();
combinedScale = signed256(
10 ** (priceFeedUniswapDecimals + priceFeedChainlinkDecimals)
);

if (decimals_ > 18) revert BadDecimals();
decimals = decimals_;
description = description_;
priceFeedScale = int256(10 ** decimals);
}

Check warning

Code scanning / Semgrep

Semgrep Finding: compound.solidity.missing-constructor-sanity-checks Warning

There're no sanity checks for the constructor argument priceFeedUniswap_.
contracts/pricefeeds/UniChainlinkPriceFeed.sol
Comment on lines +49 to +71
constructor(
address priceFeedUniswap_,
address uniswapUnderlying_,
address priceFeedChainlink_,
uint8 decimals_,
string memory description_
) {
priceFeedUniswap = priceFeedUniswap_;
uniswapUnderlying = uniswapUnderlying_;
priceFeedChainlink = priceFeedChainlink_;
uint8 priceFeedUniswapDecimals = 18;
uint8 priceFeedChainlinkDecimals = AggregatorV3Interface(
priceFeedChainlink_
).decimals();
combinedScale = signed256(
10 ** (priceFeedUniswapDecimals + priceFeedChainlinkDecimals)
);

if (decimals_ > 18) revert BadDecimals();
decimals = decimals_;
description = description_;
priceFeedScale = int256(10 ** decimals);
}

Check warning

Code scanning / Semgrep

Semgrep Finding: compound.solidity.missing-constructor-sanity-checks Warning

There're no sanity checks for the constructor argument uniswapUnderlying_.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@0xHaku @norwegianneko