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.

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

Сomet with extended asset list #904

Conversation

MishaShWoof
Copy link
Contributor

No description provided.

MishaShWoof and others added 30 commits August 9, 2024 23:45

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
…tware/comet into woof-software/collateral-extension
@dmitriy-woof-software
Copy link
Contributor

Comet Asset Extension Migration Audit.pdf
Comet Asset Extension Audit.pdf

There were performed 2 security audit. The first one audited the updated contracts whereas the second one audited migrations.

The last audited commit from the contract side is f4860cf where the following commit 94131ad optimizes tests and b6ff78e merge the audit updates into the current branch. NO CODE CHANGES HAPPEN AFTER THE MERGE.

For the second audit, the last commit is 5fe26a2

After this commit, we fixed all found issues in the report and updated the protocol to the new version to support 24 collaterals.

Currently, all the markets support 24 collaterals and all new markets will be deployed with 24 collaterals version.

@dmitriy-woof-software
Copy link
Contributor

The last commit updates all enact to true as the protocol is updated.

Important! After merging this PR that protocol should not accept any Contracts changes. It doesn't concern to update of the Governor interface, as the Governor interface has changed and will be updated in the following PRs.

All the following PRs include deployment of new markets, collaterals, and scenarios updates, but not contract changes.

Copy link
Contributor

@torreyatcitty torreyatcitty left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines +75 to +103
constructor(CometConfiguration.AssetConfig[] memory assetConfigs) {
uint8 _numAssets = uint8(assetConfigs.length);
numAssets = _numAssets;

(asset00_a, asset00_b) = getPackedAssetInternal(assetConfigs, 0);
(asset01_a, asset01_b) = getPackedAssetInternal(assetConfigs, 1);
(asset02_a, asset02_b) = getPackedAssetInternal(assetConfigs, 2);
(asset03_a, asset03_b) = getPackedAssetInternal(assetConfigs, 3);
(asset04_a, asset04_b) = getPackedAssetInternal(assetConfigs, 4);
(asset05_a, asset05_b) = getPackedAssetInternal(assetConfigs, 5);
(asset06_a, asset06_b) = getPackedAssetInternal(assetConfigs, 6);
(asset07_a, asset07_b) = getPackedAssetInternal(assetConfigs, 7);
(asset08_a, asset08_b) = getPackedAssetInternal(assetConfigs, 8);
(asset09_a, asset09_b) = getPackedAssetInternal(assetConfigs, 9);
(asset10_a, asset10_b) = getPackedAssetInternal(assetConfigs, 10);
(asset11_a, asset11_b) = getPackedAssetInternal(assetConfigs, 11);
(asset12_a, asset12_b) = getPackedAssetInternal(assetConfigs, 12);
(asset13_a, asset13_b) = getPackedAssetInternal(assetConfigs, 13);
(asset14_a, asset14_b) = getPackedAssetInternal(assetConfigs, 14);
(asset15_a, asset15_b) = getPackedAssetInternal(assetConfigs, 15);
(asset16_a, asset16_b) = getPackedAssetInternal(assetConfigs, 16);
(asset17_a, asset17_b) = getPackedAssetInternal(assetConfigs, 17);
(asset18_a, asset18_b) = getPackedAssetInternal(assetConfigs, 18);
(asset19_a, asset19_b) = getPackedAssetInternal(assetConfigs, 19);
(asset20_a, asset20_b) = getPackedAssetInternal(assetConfigs, 20);
(asset21_a, asset21_b) = getPackedAssetInternal(assetConfigs, 21);
(asset22_a, asset22_b) = getPackedAssetInternal(assetConfigs, 22);
(asset23_a, asset23_b) = getPackedAssetInternal(assetConfigs, 23);
}

Check warning

Code scanning / Semgrep OSS

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

There're no sanity checks for the constructor argument assetConfigs.
Comment on lines +75 to +103
constructor(CometConfiguration.AssetConfig[] memory assetConfigs) {
uint8 _numAssets = uint8(assetConfigs.length);
numAssets = _numAssets;

(asset00_a, asset00_b) = getPackedAssetInternal(assetConfigs, 0);
(asset01_a, asset01_b) = getPackedAssetInternal(assetConfigs, 1);
(asset02_a, asset02_b) = getPackedAssetInternal(assetConfigs, 2);
(asset03_a, asset03_b) = getPackedAssetInternal(assetConfigs, 3);
(asset04_a, asset04_b) = getPackedAssetInternal(assetConfigs, 4);
(asset05_a, asset05_b) = getPackedAssetInternal(assetConfigs, 5);
(asset06_a, asset06_b) = getPackedAssetInternal(assetConfigs, 6);
(asset07_a, asset07_b) = getPackedAssetInternal(assetConfigs, 7);
(asset08_a, asset08_b) = getPackedAssetInternal(assetConfigs, 8);
(asset09_a, asset09_b) = getPackedAssetInternal(assetConfigs, 9);
(asset10_a, asset10_b) = getPackedAssetInternal(assetConfigs, 10);
(asset11_a, asset11_b) = getPackedAssetInternal(assetConfigs, 11);
(asset12_a, asset12_b) = getPackedAssetInternal(assetConfigs, 12);
(asset13_a, asset13_b) = getPackedAssetInternal(assetConfigs, 13);
(asset14_a, asset14_b) = getPackedAssetInternal(assetConfigs, 14);
(asset15_a, asset15_b) = getPackedAssetInternal(assetConfigs, 15);
(asset16_a, asset16_b) = getPackedAssetInternal(assetConfigs, 16);
(asset17_a, asset17_b) = getPackedAssetInternal(assetConfigs, 17);
(asset18_a, asset18_b) = getPackedAssetInternal(assetConfigs, 18);
(asset19_a, asset19_b) = getPackedAssetInternal(assetConfigs, 19);
(asset20_a, asset20_b) = getPackedAssetInternal(assetConfigs, 20);
(asset21_a, asset21_b) = getPackedAssetInternal(assetConfigs, 21);
(asset22_a, asset22_b) = getPackedAssetInternal(assetConfigs, 22);
(asset23_a, asset23_b) = getPackedAssetInternal(assetConfigs, 23);
}

Check notice

Code scanning / Semgrep OSS

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

Consider making costructor payable to save gas.
Comment on lines +16 to +18
constructor(ExtConfiguration memory config, address assetListFactoryAddress) CometExt(config) {
assetListFactory = assetListFactoryAddress;
}

Check warning

Code scanning / Semgrep OSS

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

There're no sanity checks for the constructor argument assetListFactoryAddress.
Comment on lines +16 to +18
constructor(ExtConfiguration memory config, address assetListFactoryAddress) CometExt(config) {
assetListFactory = assetListFactoryAddress;
}

Check warning

Code scanning / Semgrep OSS

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

There're no sanity checks for the constructor argument config.
Comment on lines +16 to +18
constructor(ExtConfiguration memory config, address assetListFactoryAddress) CometExt(config) {
assetListFactory = assetListFactoryAddress;
}

Check notice

Code scanning / Semgrep OSS

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

Consider making costructor payable to save gas.
accrueInternal();
for (uint i = 0; i < accounts.length; ) {
absorbInternal(absorber, accounts[i]);
unchecked { i++; }

Check notice

Code scanning / Semgrep OSS

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.
// Using gas price instead of base fee would more accurately reflect spend,
// but is also subject to abuse if refunds were to be given automatically.
LiquidatorPoints memory points = liquidatorPoints[absorber];
points.numAbsorbs++;

Check notice

Code scanning / Semgrep OSS

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.
uint8 _reserved = accountUser._reserved;

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

Check notice

Code scanning / Semgrep OSS

Semgrep Finding: rules.solidity.performance.init-variables-with-default-value Note

Uninitialized variables are assigned with the types default value.
Explicitly initializing a variable with its default value costs unnecessary gas.

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

Check notice

Code scanning / Semgrep OSS

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.
if (isBuyPaused()) revert Paused();

int reserves = getReserves();
if (reserves >= 0 && uint(reserves) >= targetReserves) revert NotForSale();

Check notice

Code scanning / Semgrep OSS

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.
@torreyatcitty torreyatcitty merged commit 059c119 into compound-finance:main Mar 6, 2025
3 of 31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants