Skip to content

Commit

Permalink
Merge pull request #100 from woof-software/woof-software/collateral-e…
Browse files Browse the repository at this point in the history
…xtension-audit-fix

Audit fixes to collateral extension
  • Loading branch information
MishaShWoof authored Jan 17, 2025
2 parents 94131ad + f4860cf commit b6ff78e
Show file tree
Hide file tree
Showing 12 changed files with 218 additions and 37 deletions.
198 changes: 182 additions & 16 deletions contracts/AssetList.sol
Original file line number Diff line number Diff line change
Expand Up @@ -20,29 +20,100 @@ contract AssetList {
/// @dev The max value for a collateral factor (1)
uint64 internal constant MAX_COLLATERAL_FACTOR = FACTOR_SCALE;

uint256[] internal assets_a;
uint256[] internal assets_b;
uint256 internal immutable asset00_a;
uint256 internal immutable asset00_b;
uint256 internal immutable asset01_a;
uint256 internal immutable asset01_b;
uint256 internal immutable asset02_a;
uint256 internal immutable asset02_b;
uint256 internal immutable asset03_a;
uint256 internal immutable asset03_b;
uint256 internal immutable asset04_a;
uint256 internal immutable asset04_b;
uint256 internal immutable asset05_a;
uint256 internal immutable asset05_b;
uint256 internal immutable asset06_a;
uint256 internal immutable asset06_b;
uint256 internal immutable asset07_a;
uint256 internal immutable asset07_b;
uint256 internal immutable asset08_a;
uint256 internal immutable asset08_b;
uint256 internal immutable asset09_a;
uint256 internal immutable asset09_b;
uint256 internal immutable asset10_a;
uint256 internal immutable asset10_b;
uint256 internal immutable asset11_a;
uint256 internal immutable asset11_b;
uint256 internal immutable asset12_a;
uint256 internal immutable asset12_b;
uint256 internal immutable asset13_a;
uint256 internal immutable asset13_b;
uint256 internal immutable asset14_a;
uint256 internal immutable asset14_b;
uint256 internal immutable asset15_a;
uint256 internal immutable asset15_b;
uint256 internal immutable asset16_a;
uint256 internal immutable asset16_b;
uint256 internal immutable asset17_a;
uint256 internal immutable asset17_b;
uint256 internal immutable asset18_a;
uint256 internal immutable asset18_b;
uint256 internal immutable asset19_a;
uint256 internal immutable asset19_b;
uint256 internal immutable asset20_a;
uint256 internal immutable asset20_b;
uint256 internal immutable asset21_a;
uint256 internal immutable asset21_b;
uint256 internal immutable asset22_a;
uint256 internal immutable asset22_b;
uint256 internal immutable asset23_a;
uint256 internal immutable asset23_b;

/// @notice The number of assets this contract actually supports
uint8 public immutable numAssets;

constructor(CometConfiguration.AssetConfig[] memory assetConfigs) {
uint8 _numAssets = uint8(assetConfigs.length);
numAssets = _numAssets;
uint256[] memory _assets_a = new uint256[](_numAssets);
uint256[] memory _assets_b = new uint256[](_numAssets);
for (uint i = 0; i < _numAssets; ) {
(uint256 asset_a, uint256 asset_b) = getPackedAssetInternal(assetConfigs, i);
_assets_a[i] = asset_a;
_assets_b[i] = asset_b;
unchecked { i++; }
}
assets_a = _assets_a;
assets_b = _assets_b;

(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.

Check notice

Code scanning / Semgrep OSS

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

Consider making costructor payable to save gas.

/**
* @dev Checks and gets the packed asset info for storage
* @dev Checks and gets the packed asset info for storage in 2 variables
* - in first variable, the asset address is stored in the lower 160 bits (address can be interpreted as uint160),
* the borrow collateral factor in the next 16 bits,
* the liquidate collateral factor in the next 16 bits,
* and the liquidation factor in the next 16 bits
* - in the second variable, the price feed address is stored in the lower 160 bits,
* the asset decimals in the next 8 bits,
* and the supply cap in the next 64 bits
* @param assetConfigs The asset configurations
* @param i The index of the asset info to get
* @return The packed asset info
*/
function getPackedAssetInternal(CometConfiguration.AssetConfig[] memory assetConfigs, uint i) internal view returns (uint256, uint256) {
CometConfiguration.AssetConfig memory assetConfig;
Expand Down Expand Up @@ -102,9 +173,104 @@ contract AssetList {
*/
function getAssetInfo(uint8 i) public view returns (CometCore.AssetInfo memory) {
if (i >= numAssets) revert CometMainInterface.BadAsset();

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

address asset = address(uint160(word_a & type(uint160).max));
uint64 rescale = FACTOR_SCALE / 1e4;
Expand Down
5 changes: 5 additions & 0 deletions contracts/AssetListFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ import "./AssetList.sol";
contract AssetListFactory {
event AssetListCreated(address indexed assetList, CometCore.AssetConfig[] assetConfigs);

/**
* @notice Create a new asset list
* @param assetConfigs The asset configurations
* @return assetList The address of the new asset list
*/
function createAssetList(CometCore.AssetConfig[] memory assetConfigs) external returns (address assetList) {
assetList = address(new AssetList(assetConfigs));
emit AssetListCreated(assetList, assetConfigs);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
// SPDX-License-Identifier: BUSL-1.1
pragma solidity 0.8.15;

import "./CometExtendedAssetList.sol";
import "./CometWithExtendedAssetList.sol";
import "./CometConfiguration.sol";

contract CometFactoryExtendedAssetList is CometConfiguration {
contract CometFactoryWithExtendedAssetList is CometConfiguration {
function clone(Configuration calldata config) external returns (address) {
return address(new CometExtendedAssetList(config));
return address(new CometWithExtendedAssetList(config));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import "./IAssetList.sol";
* @notice An efficient monolithic money market protocol
* @author Compound
*/
contract CometExtendedAssetList is CometMainInterface {
contract CometWithExtendedAssetList is CometMainInterface {
/** General configuration constants **/

/// @notice The admin of the protocol
Expand Down
1 change: 1 addition & 0 deletions contracts/IAssetList.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@ import "./CometCore.sol";
*/
interface IAssetList {
function getAssetInfo(uint8 i) external view returns (CometCore.AssetInfo memory);
function numAssets() external view returns (uint8);
}
5 changes: 5 additions & 0 deletions contracts/IAssetListFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,10 @@ import "./CometCore.sol";
* @author Compound
*/
interface IAssetListFactory {
/**
* @notice Create a new asset list
* @param assetConfigs The asset configurations
* @return assetList The address of the new asset list
*/
function createAssetList(CometCore.AssetConfig[] memory assetConfigs) external returns (address assetList);
}
4 changes: 4 additions & 0 deletions contracts/IAssetListFactoryHolder.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,9 @@ pragma solidity 0.8.15;
* @author Compound
*/
interface IAssetListFactoryHolder {
/**
* @notice Get the asset list factory
* @return assetListFactory The asset list factory address
*/
function assetListFactory() external view returns (address);
}
6 changes: 3 additions & 3 deletions contracts/test/CometHarnessExtendedAssetList.sol
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
// SPDX-License-Identifier: BUSL-1.1
pragma solidity 0.8.15;

import "../CometExtendedAssetList.sol";
import "../CometWithExtendedAssetList.sol";

contract CometHarnessExtendedAssetList is CometExtendedAssetList {
contract CometHarnessExtendedAssetList is CometWithExtendedAssetList {
uint public nowOverride;

constructor(Configuration memory config) CometExtendedAssetList(config) {}
constructor(Configuration memory config) CometWithExtendedAssetList(config) {}

function getNowInternal() override internal view returns (uint40) {
return nowOverride > 0 ? uint40(nowOverride) : super.getNowInternal();
Expand Down
2 changes: 1 addition & 1 deletion test/absorb-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,7 @@ describe('absorb', function () {
},
reward: 'COMP',
});
const { cometExtendedAssetList : comet, tokens: {
const { cometWithExtendedAssetList : comet, tokens: {
COMP,
WETH,
}, users: [absorber, underwater] } = protocol;
Expand Down
6 changes: 3 additions & 3 deletions test/asset-info-test-asset-list-comet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { expect, exp, makeConfigurator, ONE, makeProtocol } from './helpers';

describe('asset info', function () {
it('initializes protocol', async () => {
const { cometExtendedAssetList: comet, tokens } = await makeConfigurator({
const { cometWithExtendedAssetList: comet, tokens } = await makeConfigurator({
assets: {
USDC: {},
ASSET1: {},
Expand Down Expand Up @@ -68,8 +68,8 @@ describe('asset info', function () {
});

it('reverts if index is greater than numAssets', async () => {
const { cometExtendedAssetList } = await makeConfigurator();
await expect(cometExtendedAssetList.getAssetInfo(3)).to.be.revertedWith("custom error 'BadAsset()'");
const { cometWithExtendedAssetList } = await makeConfigurator();
await expect(cometWithExtendedAssetList.getAssetInfo(3)).to.be.revertedWith("custom error 'BadAsset()'");
});

it('reverts if collateral factors are out of range', async () => {
Expand Down
4 changes: 2 additions & 2 deletions test/bulker-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ describe('bulker', function () {
},
reward: 'COMP',
});
const { cometExtendedAssetList : comet, tokens: {
const { cometWithExtendedAssetList : comet, tokens: {
COMP,
WETH,
USDC,
Expand Down Expand Up @@ -392,7 +392,7 @@ describe('bulker', function () {
},
reward: 'COMP',
});
const { cometExtendedAssetList : comet, tokens: {
const { cometWithExtendedAssetList : comet, tokens: {
COMP,
WETH,
}, users: [alice] } = protocol;
Expand Down
Loading

0 comments on commit b6ff78e

Please sign in to comment.