Pre-pack asset configs (again)

.solhint.json

@@ -5,6 +5,7 @@
     "const-name-snakecase": "off",
     "func-visibility": ["warn", { "ignoreConstructors": true }],
     "no-empty-blocks": "off",
+    "no-inline-assembly": "off",
     "not-rely-on-time": "off",
     "var-name-mixedcase": "off"
   }

contracts/Comet.sol

@@ -249,62 +249,13 @@ contract Comet is CometCore {
     /**
      * @dev Checks and gets the packed asset info for storage
      */
-    function _getPackedAsset(AssetConfig[] memory assetConfigs, uint i) internal view returns (uint256, uint256) {
-        AssetConfig memory assetConfig;
+    function _getPackedAsset(AssetConfig[] memory assetConfigs, uint i) internal view returns (uint256 word_a, uint256 word_b) {
         if (i < assetConfigs.length) {
             assembly {
-                assetConfig := mload(add(add(assetConfigs, 0x20), mul(i, 0x20)))
+                let assetConfig := mload(add(add(assetConfigs, 0x20), mul(i, 0x20)))
+                word_a := mload(assetConfig)
+                word_b := mload(add(assetConfig, 0x20))
             }
-        } else {
-            assetConfig = AssetConfig({
-                asset: address(0),
-                priceFeed: address(0),
-                decimals: uint8(0),
-                borrowCollateralFactor: uint64(0),
-                liquidateCollateralFactor: uint64(0),
-                liquidationFactor: uint64(0),
-                supplyCap: uint128(0)
-            });
-        }
-        address asset = assetConfig.asset;
-        address priceFeed = assetConfig.priceFeed;
-        uint8 decimals_ = assetConfig.decimals;
-
-        // Short-circuit if asset is nil
-        if (asset == address(0)) {
-            return (0, 0);
-        }
-
-        // Sanity check price feed and asset decimals
-        if (AggregatorV3Interface(priceFeed).decimals() != PRICE_FEED_DECIMALS) revert BadDecimals();
-        if (ERC20(asset).decimals() != decimals_) revert BadDecimals();
-
-        // Ensure collateral factors are within range
-        if (assetConfig.borrowCollateralFactor >= assetConfig.liquidateCollateralFactor) revert BorrowCFTooLarge();
-        if (assetConfig.liquidateCollateralFactor > MAX_COLLATERAL_FACTOR) revert LiquidateCFTooLarge();
-
-        unchecked {
-            // Keep 4 decimals for each factor
-            uint descale = FACTOR_SCALE / 1e4;
-            uint16 borrowCollateralFactor = uint16(assetConfig.borrowCollateralFactor / descale);
-            uint16 liquidateCollateralFactor = uint16(assetConfig.liquidateCollateralFactor / descale);
-            uint16 liquidationFactor = uint16(assetConfig.liquidationFactor / descale);
-
-            // Be nice and check descaled values are still within range
-            if (borrowCollateralFactor >= liquidateCollateralFactor) revert BorrowCFTooLarge();
-
-            // Keep whole units of asset for supply cap
-            uint64 supplyCap = uint64(assetConfig.supplyCap / (10 ** decimals_));
-
-            uint256 word_a = (uint160(asset) << 0 |
-                              uint256(borrowCollateralFactor) << 160 |
-                              uint256(liquidateCollateralFactor) << 176 |
-                              uint256(liquidationFactor) << 192);
-            uint256 word_b = (uint160(priceFeed) << 0 |
-                              uint256(decimals_) << 160 |
-                              uint256(supplyCap) << 168);
-
-            return (word_a, word_b);
         }
     }
 

contracts/CometConfiguration.sol

@@ -34,12 +34,7 @@ contract CometConfiguration {
     }
 
     struct AssetConfig {
-        address asset;
-        address priceFeed;
-        uint8 decimals;
-        uint64 borrowCollateralFactor;
-        uint64 liquidateCollateralFactor;
-        uint64 liquidationFactor;
-        uint128 supplyCap;
+        uint256 word_a;
+        uint256 word_b;
     }
 }

src/deploy/Development.ts

@@ -21,6 +21,7 @@ import { ExtConfigurationStruct } from '../../build/types/CometExt';
 import { BigNumberish } from 'ethers';
 export { Comet } from '../../build/types';
 import { DeployedContracts, ProtocolConfiguration } from './index';
+import { packAssetConfig } from './NetworkConfiguration';
 
 async function makeToken(
   deploymentManager: DeploymentManager,
@@ -72,21 +73,21 @@ export async function deployDevelopmentComet(
   let assetConfig0 = {
     asset: gold.address,
     priceFeed: goldPriceFeed.address,
-    decimals: (8).toString(),
-    borrowCollateralFactor: (0.9e18).toString(),
-    liquidateCollateralFactor: (1e18).toString(),
-    liquidationFactor: (0.95e18).toString(),
-    supplyCap: (1000000e8).toString(),
+    decimals: 8,
+    borrowCF: 0.9,
+    liquidateCF: 1,
+    liquidationFactor: 0.95,
+    supplyCap: 1000000e8,
   };
 
   let assetConfig1 = {
     asset: silver.address,
     priceFeed: silverPriceFeed.address,
-    decimals: (10).toString(),
-    borrowCollateralFactor: (0.4e18).toString(),
-    liquidateCollateralFactor: (0.5e18).toString(),
-    liquidationFactor: (0.9e18).toString(),
-    supplyCap: (500000e10).toString(),
+    decimals: 10,
+    borrowCF: 0.4,
+    liquidateCF: 0.5,
+    liquidationFactor: 0.9,
+    supplyCap: 500000e10,
   };
 
   const {
@@ -127,7 +128,7 @@ export async function deployDevelopmentComet(
       baseMinForRewards: 1, // XXX
       baseBorrowMin: 1, // XXX
       targetReserves: 0, // XXX
-      assetConfigs: [assetConfig0, assetConfig1],
+      assetConfigs: [assetConfig0, assetConfig1].map(c => packAssetConfig(c.asset, c)),
     },
     ...configurationOverrides,
   };

src/deploy/NetworkConfiguration.ts

@@ -124,22 +124,35 @@ function getTrackingInfo(tracking: NetworkTrackingConfiguration): TrackingInfo {
   };
 }
 
+export function packAssetConfig(assetAddress: string, assetConfig: NetworkAssetConfiguration): AssetConfigStruct {
+    const descale = (10n**18n) / (10n**4n);
+    let priceFeedAddress = address(assetConfig.priceFeed);
+    let decimals = BigInt(assetConfig.decimals);
+    let borrowCF = BigInt(percentage(assetConfig.borrowCF));
+    let liquidateCF = BigInt(percentage(assetConfig.liquidateCF));
+    let liquidationFactor = BigInt(percentage(assetConfig.liquidationFactor));
+    let supplyCap = BigInt(number(assetConfig.supplyCap)); // TODO: Decimals (what?)
+    return {
+      word_a: (
+        (BigInt(assetAddress)) |
+          ((borrowCF / descale) << 160n) |
+          ((liquidateCF / descale) << 176n) |
+          ((liquidationFactor / descale) << 192n)
+      ),
+      word_b: (
+        (BigInt(priceFeedAddress)) |
+          (decimals << 160n) |
+          ((supplyCap / (10n**decimals)) << 168n)
+      ),
+    };
+}
+
 function getAssetConfigs(
   assets: { [name: string]: NetworkAssetConfiguration },
   contractMap: ContractMap
 ): AssetConfigStruct[] {
   return Object.entries(assets).map(([assetName, assetConfig]) => {
-    let assetAddress = getContractAddress(assetName, contractMap);
-
-    return {
-      asset: assetAddress,
-      priceFeed: address(assetConfig.priceFeed),
-      decimals: number(assetConfig.decimals),
-      borrowCollateralFactor: percentage(assetConfig.borrowCF),
-      liquidateCollateralFactor: percentage(assetConfig.liquidateCF),
-      liquidationFactor: percentage(assetConfig.liquidationFactor),
-      supplyCap: number(assetConfig.supplyCap), // TODO: Decimals
-    };
+    return packAssetConfig(getContractAddress(assetName, contractMap), assetConfig);
   });
 }
 

test/asset-info-test.ts

@@ -65,7 +65,8 @@ describe('asset info', function () {
     await expect(comet.getAssetInfo(3)).to.be.revertedWith("custom error 'BadAsset()'");
   });
 
-  it('reverts if collateral factors are out of range', async () => {
+  // Note: with pre-packing we do not check this
+  it.skip('reverts if collateral factors are out of range', async () => {
     await expect(makeProtocol({
       assets: {
         USDC: {},

test/constructor-test.ts

@@ -14,7 +14,7 @@ describe('constructor', function () {
     expect(await comet.baseBorrowMin()).to.eq(exp(100,6));
   });
 
-  it('verifies asset scales', async function () {
+  it('does not verify configs', async function () {
     const [governor, pauseGuardian] = await ethers.getSigners();
 
     // extension delegate
@@ -27,10 +27,6 @@ describe('constructor', function () {
     // tokens
     const assets = {
       USDC: { decimals: 6 },
-      EVIL: {
-        decimals: 18,
-        packedDecimals: 19,
-      }
     };
     const FaucetFactory = (await ethers.getContractFactory('FaucetToken')) as FaucetToken__factory;
     const tokens = {};
@@ -51,7 +47,7 @@ describe('constructor', function () {
     }
 
     const CometFactory = (await ethers.getContractFactory('CometHarness')) as CometHarness__factory;
-    await expect(CometFactory.deploy({
+    const comet = await CometFactory.deploy({
       governor: governor.address,
       pauseGuardian: pauseGuardian.address,
       extensionDelegate: extensionDelegate.address,
@@ -70,15 +66,11 @@ describe('constructor', function () {
       baseBorrowMin: exp(1, 6),
       targetReserves: 0,
       assetConfigs: [{
-        asset: tokens["EVIL"].address,
-        priceFeed: priceFeeds["EVIL"].address,
-        decimals: assets["EVIL"].packedDecimals, // <-- packed decimals differ from deployed token's decimals
-        borrowCollateralFactor: ONE - 1n,
-        liquidateCollateralFactor: ONE,
-        liquidationFactor: ONE,
-        supplyCap: exp(100, 18),
+        word_a: 0,
+        word_b: 0,
       }],
-    })).to.be.revertedWith("custom error 'BadDecimals()'");
+    });
+    expect((await comet.getAssetInfo(0)).asset).to.be.equal('0x0000000000000000000000000000000000000000');
   });
 
   it('reverts if baseTokenPriceFeed does not have 8 decimals', async () => {
@@ -93,7 +85,8 @@ describe('constructor', function () {
     ).to.be.revertedWith("custom error 'BadDecimals()'");
   });
 
-  it('reverts if asset has a price feed that does not have 8 decimals', async () => {
+  // Note: with pre-packing we do not check this
+  it.skip('reverts if asset has a price feed that does not have 8 decimals', async () => {
     await expect(
       makeProtocol({
         assets: {

test/helpers.ts

@@ -222,15 +222,25 @@ export async function makeProtocol(opts: ProtocolOpts = {}): Promise<Protocol> {
     baseBorrowMin,
     targetReserves,
     assetConfigs: Object.entries(assets).reduce((acc, [symbol, config], i) => {
+      const descale = factorScale / (10n**4n);
+      const decimals = BigInt(dfn(config.decimals, 18));
+      const borrowCF = BigInt(dfn(config.borrowCF, ONE - 1n));
+      const liquidateCF = BigInt(dfn(config.liquidateCF, ONE))
+      const liquidationFactor = BigInt(dfn(config.liquidationFactor, ONE));
+      const supplyCap = BigInt(dfn(config.supplyCap, exp(100, decimals)));
       if (symbol != base) {
         acc.push({
-          asset: tokens[symbol].address,
-          priceFeed: priceFeeds[symbol].address,
-          decimals: dfn(assets[symbol].decimals, 18),
-          borrowCollateralFactor: dfn(config.borrowCF, ONE - 1n),
-          liquidateCollateralFactor: dfn(config.liquidateCF, ONE),
-          liquidationFactor: dfn(config.liquidationFactor, ONE),
-          supplyCap: dfn(config.supplyCap, exp(100, dfn(config.decimals, 18))),
+          word_a: (
+            (BigInt(tokens[symbol].address)) |
+              ((borrowCF / descale) << 160n) |
+              ((liquidateCF / descale) << 176n) |
+              ((liquidationFactor / descale) << 192n)
+          ),
+          word_b: (
+            (BigInt(priceFeeds[symbol].address)) |
+              (decimals << 160n) |
+              ((supplyCap / exp(1, decimals)) << 168n)
+          ),
         });
       }
       return acc;