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

Audit fixes to collateral extension #100

Open
wants to merge 6 commits into
base: woof-software/collateral-extension
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
189 changes: 174 additions & 15 deletions contracts/AssetList.sol
Original file line number Diff line number Diff line change
Expand Up @@ -20,34 +20,98 @@
/// @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);
}

/**
* @dev Checks and gets the packed asset info for storage
* @param assetConfigs The asset configurations
* @param i The index of the asset info to get
* @return The packed asset info
vitalii-woof-software marked this conversation as resolved.
Show resolved Hide resolved
*/
function getPackedAssetInternal(CometConfiguration.AssetConfig[] memory assetConfigs, uint i) internal view returns (uint256, uint256) {
CometConfiguration.AssetConfig memory assetConfig;
if (i < assetConfigs.length) {
assembly {

Check warning on line 114 in contracts/AssetList.sol

View workflow job for this annotation

GitHub Actions / Contract linter

Avoid to use inline assembly. It is acceptable only in rare cases
assetConfig := mload(add(add(assetConfigs, 0x20), mul(i, 0x20)))
}
} else {
Expand Down Expand Up @@ -102,9 +166,104 @@
*/
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 @@
* @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 Expand Up @@ -179,12 +179,12 @@
function nonReentrantBefore() internal {
bytes32 slot = REENTRANCY_GUARD_FLAG_SLOT;
uint256 status;
assembly ("memory-safe") {

Check warning on line 182 in contracts/CometWithExtendedAssetList.sol

View workflow job for this annotation

GitHub Actions / Contract linter

Avoid to use inline assembly. It is acceptable only in rare cases
status := sload(slot)
}

if (status == REENTRANCY_GUARD_ENTERED) revert ReentrantCallBlocked();
assembly ("memory-safe") {

Check warning on line 187 in contracts/CometWithExtendedAssetList.sol

View workflow job for this annotation

GitHub Actions / Contract linter

Avoid to use inline assembly. It is acceptable only in rare cases
sstore(slot, REENTRANCY_GUARD_ENTERED)
}
}
Expand All @@ -194,8 +194,8 @@
*/
function nonReentrantAfter() internal {
bytes32 slot = REENTRANCY_GUARD_FLAG_SLOT;
uint256 status;

Check warning on line 197 in contracts/CometWithExtendedAssetList.sol

View workflow job for this annotation

GitHub Actions / Contract linter

Variable "status" is unused
assembly ("memory-safe") {

Check warning on line 198 in contracts/CometWithExtendedAssetList.sol

View workflow job for this annotation

GitHub Actions / Contract linter

Avoid to use inline assembly. It is acceptable only in rare cases
sstore(slot, REENTRANCY_GUARD_NOT_ENTERED)
}
}
Expand Down Expand Up @@ -660,7 +660,7 @@
uint256 preTransferBalance = IERC20NonStandard(asset).balanceOf(address(this));
IERC20NonStandard(asset).transferFrom(from, address(this), amount);
bool success;
assembly ("memory-safe") {

Check warning on line 663 in contracts/CometWithExtendedAssetList.sol

View workflow job for this annotation

GitHub Actions / Contract linter

Avoid to use inline assembly. It is acceptable only in rare cases
switch returndatasize()
case 0 { // This is a non-standard ERC-20
success := not(0) // set success to true
Expand All @@ -684,7 +684,7 @@
function doTransferOut(address asset, address to, uint amount) internal {
IERC20NonStandard(asset).transfer(to, amount);
bool success;
assembly ("memory-safe") {

Check warning on line 687 in contracts/CometWithExtendedAssetList.sol

View workflow job for this annotation

GitHub Actions / Contract linter

Avoid to use inline assembly. It is acceptable only in rare cases
switch returndatasize()
case 0 { // This is a non-standard ERC-20
success := not(0) // set success to true
Expand Down Expand Up @@ -1239,9 +1239,9 @@
/**
* @notice Fallback to calling the extension delegate for everything else
*/
fallback() external payable {

Check warning on line 1242 in contracts/CometWithExtendedAssetList.sol

View workflow job for this annotation

GitHub Actions / Contract linter

Fallback function must be simple
address delegate = extensionDelegate;
assembly {

Check warning on line 1244 in contracts/CometWithExtendedAssetList.sol

View workflow job for this annotation

GitHub Actions / Contract linter

Avoid to use inline assembly. It is acceptable only in rare cases
calldatacopy(0, 0, calldatasize())
let result := delegatecall(gas(), delegate, 0, calldatasize(), 0, 0)
returndatacopy(0, 0, returndatasize())
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
Copy link
Collaborator

Choose a reason for hiding this comment

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

I recommend that you indicate that this is an 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
25 changes: 16 additions & 9 deletions test/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
BaseBulker,
BaseBulker__factory,
CometExt,
CometExt__factory,
CometExtAssetList__factory,
CometHarness__factory,
CometHarnessInterface as Comet,
Expand Down Expand Up @@ -35,7 +36,7 @@ import {
AssetListFactory,
AssetListFactory__factory,
CometHarnessExtendedAssetList__factory,
CometHarnessInterfaceExtendedAssetList as CometExtendedAssetList,
CometHarnessInterfaceExtendedAssetList as CometWithExtendedAssetList,
} from '../build/types';
import { BigNumber } from 'ethers';
import { TransactionReceipt, TransactionResponse } from '@ethersproject/abstract-provider';
Expand Down Expand Up @@ -102,7 +103,7 @@ export type Protocol = {
base: string;
reward: string;
comet: Comet;
cometExtendedAssetList: CometExtendedAssetList;
cometWithExtendedAssetList: CometWithExtendedAssetList;
assetListFactory: AssetListFactory;
tokens: {
[symbol: string]: FaucetToken | NonStandardFaucetFeeToken;
Expand Down Expand Up @@ -282,8 +283,8 @@ export async function makeProtocol(opts: ProtocolOpts = {}): Promise<Protocol> {

let extensionDelegate = opts.extensionDelegate;
if (extensionDelegate === undefined) {
const CometExtFactory = (await ethers.getContractFactory('CometExtAssetList')) as CometExtAssetList__factory;
extensionDelegate = await CometExtFactory.deploy({ name32, symbol32 }, assetListFactory.address);
const CometExtFactory = (await ethers.getContractFactory('CometExt')) as CometExt__factory;
extensionDelegate = await CometExtFactory.deploy({ name32, symbol32 });
await extensionDelegate.deployed();
}

Expand All @@ -310,7 +311,7 @@ export async function makeProtocol(opts: ProtocolOpts = {}): Promise<Protocol> {
baseBorrowMin,
targetReserves,
assetConfigs: Object.entries(assets).reduce((acc, [symbol, config], _i) => {
if (symbol != base && _i < 12) {
if (symbol != base && _i <= 12) {
acc.push({
asset: tokens[symbol].address,
priceFeed: priceFeeds[symbol].address,
Expand Down Expand Up @@ -341,7 +342,13 @@ export async function makeProtocol(opts: ProtocolOpts = {}): Promise<Protocol> {
}
return acc;
}, []);

let extensionDelegateAssetList = opts.extensionDelegate;
if (extensionDelegateAssetList === undefined) {
const CometExtFactory = (await ethers.getContractFactory('CometExtAssetList')) as CometExtAssetList__factory;
extensionDelegateAssetList = await CometExtFactory.deploy({ name32, symbol32 }, assetListFactory.address);
await extensionDelegateAssetList.deployed();
}
config.extensionDelegate = extensionDelegateAssetList.address;
const CometFactoryExtendedAssetList = (await ethers.getContractFactory('CometHarnessExtendedAssetList')) as CometHarnessExtendedAssetList__factory;

const cometExtendedAssetList = await CometFactoryExtendedAssetList.deploy(config);
Expand All @@ -367,7 +374,7 @@ export async function makeProtocol(opts: ProtocolOpts = {}): Promise<Protocol> {
base,
reward,
comet: await ethers.getContractAt('CometHarnessInterface', comet.address) as Comet,
cometExtendedAssetList: await ethers.getContractAt('CometHarnessInterfaceExtendedAssetList', cometExtendedAssetList.address) as CometExtendedAssetList,
cometWithExtendedAssetList: await ethers.getContractAt('CometHarnessInterfaceExtendedAssetList', cometExtendedAssetList.address) as CometWithExtendedAssetList,
assetListFactory: assetListFactory,
tokens,
unsupportedToken,
Expand All @@ -387,7 +394,7 @@ export async function makeConfigurator(opts: ProtocolOpts = {}): Promise<Configu
base,
reward,
comet,
cometExtendedAssetList,
cometWithExtendedAssetList,
assetListFactory,
tokens,
unsupportedToken,
Expand Down Expand Up @@ -496,7 +503,7 @@ export async function makeConfigurator(opts: ProtocolOpts = {}): Promise<Configu
reward,
proxyAdmin,
comet,
cometExtendedAssetList,
cometWithExtendedAssetList,
assetListFactory,
cometProxy,
configurator,
Expand Down
Loading