Address OZ's feedback on USDT comet support

contracts/Comet.sol

@@ -202,7 +202,7 @@
     }
 
     /**
-     * @dev Non-reentrant modifier
+     * @dev Prevents marked functions from being reentered 
      */
     modifier nonReentrant() {
         nonReentrantBefore();
@@ -211,28 +211,28 @@
     }
 
     /**
-     * @dev This will set the flag before the call and revert if it is already set
+     * @dev Checks that the reentrancy flag is not set and then sets the flag
      */
     function nonReentrantBefore() internal {
         bytes32 slot = REENTRANCY_GUARD_FLAG_SLOT;
         uint256 status;
         assembly ("memory-safe") {
             status := sload(slot)
         }
 
         if (status == REENTRANCY_GUARD_ENTERED) revert ReentrantCallBlocked();
         assembly ("memory-safe") {
             sstore(slot, REENTRANCY_GUARD_ENTERED)
         }
     }
 
     /**
-     * @dev This will unset the flag after the call
+     * @dev Unsets the reentrancy flag
      */
     function nonReentrantAfter() internal {
         bytes32 slot = REENTRANCY_GUARD_FLAG_SLOT;
         uint256 status;
         assembly ("memory-safe") {
             sstore(slot, REENTRANCY_GUARD_NOT_ENTERED)
         }
     }
@@ -260,7 +260,7 @@
     function getPackedAssetInternal(AssetConfig[] memory assetConfigs, uint i) internal view returns (uint256, uint256) {
         AssetConfig memory assetConfig;
         if (i < assetConfigs.length) {
             assembly {
                 assetConfig := mload(add(add(assetConfigs, 0x20), mul(i, 0x20)))
             }
         } else {
@@ -803,7 +803,7 @@
         uint256 preTransferBalance = IERC20NonStandard(asset).balanceOf(address(this));
         IERC20NonStandard(asset).transferFrom(from, address(this), amount);
         bool success;
         assembly ("memory-safe") {
             switch returndatasize()
                 case 0 {                       // This is a non-standard ERC-20
                     success := not(0)          // set success to true
@@ -827,7 +827,7 @@
     function doTransferOut(address asset, address to, uint amount) internal {
         IERC20NonStandard(asset).transfer(to, amount);
         bool success;
         assembly ("memory-safe") {
             switch returndatasize()
                 case 0 {                       // This is a non-standard ERC-20
                     success := not(0)          // set success to true
@@ -1380,9 +1380,9 @@
     /**
      * @notice Fallback to calling the extension delegate for everything else
      */
     fallback() external payable {
         address delegate = extensionDelegate;
         assembly {
             calldatacopy(0, 0, calldatasize())
             let result := delegatecall(gas(), delegate, 0, calldatasize(), 0, 0)
             returndatacopy(0, 0, returndatasize())

contracts/test/EvilToken.sol

@@ -13,7 +13,8 @@ contract EvilToken is FaucetToken {
     enum AttackType {
         TRANSFER_FROM,
         WITHDRAW_FROM,
-        SUPPLY_FROM
+        SUPPLY_FROM,
+        BUY_COLLATERAL
     }
 
     struct ReentryAttack {
@@ -23,6 +24,7 @@ contract EvilToken is FaucetToken {
         address asset;
         uint amount;
         uint maxCalls;
+        address collateralAsset;
     }
 
     ReentryAttack public attack;
@@ -40,7 +42,8 @@ contract EvilToken is FaucetToken {
             destination: address(this),
             asset: address(this),
             amount: 1e6,
-            maxCalls: type(uint).max
+            maxCalls: type(uint).max,
+            collateralAsset: address(0)
         });
     }
 
@@ -92,6 +95,13 @@ contract EvilToken is FaucetToken {
                 reentryAttack.asset,
                 reentryAttack.amount
             );
+        }  else if (reentryAttack.attackType == AttackType.BUY_COLLATERAL) {
+            Comet(payable(msg.sender)).buyCollateral(
+                reentryAttack.collateralAsset,
+                0,
+                reentryAttack.amount,
+                reentryAttack.destination
+            );
         } else {
             revert("invalid reentry attack");
         }

deployments/goerli/usdt/migrations/1691022234_configurate_and_ens.ts

@@ -0,0 +1,206 @@
+import { DeploymentManager, migration } from '../../../../plugins/deployment_manager';
+import { exp, getConfigurationStruct, proposal } from '../../../../src/deploy';
+import { diffState, getCometConfig } from '../../../../plugins/deployment_manager/DiffState';
+import { expect } from 'chai';
+
+const COMPAddress = '0x3587b2F7E0E2D6166d6C14230e7Fe160252B0ba4';
+const ENSName = 'compound-community-licenses.eth';
+const ENSResolverAddress = '0x19c2d5D0f035563344dBB7bE5fD09c8dad62b001';
+const ENSSubdomainLabel = 'v3-additional-grants';
+const ENSSubdomain = `${ENSSubdomainLabel}.${ENSName}`;
+const ENSTextRecordKey = 'v3-official-markets';
+const USDCAmountToSeed = exp(5, 6);
+
+export default migration('1691022234_configurate_and_ens', {
+    async prepare(deploymentManager: DeploymentManager) {
+        const cometFactory = await deploymentManager.deploy('cometFactory', 'CometFactory.sol', [], true);
+        return { newFactoryAddress: cometFactory.address };
+    },
+
+    enact: async (deploymentManager: DeploymentManager, govDeploymentManager: DeploymentManager, { newFactoryAddress }) => {
+        const trace = deploymentManager.tracer();
+
+        // Import shared contracts from cUSDCv3
+
+        const AllContracts = await deploymentManager.getContracts();
+        const {
+            governor,
+            comet,
+            configurator,
+            cometAdmin,
+            rewards,
+            USDT,
+        } = AllContracts;
+
+        const configuration = await getConfigurationStruct(deploymentManager);
+
+        const ENSResolver = await deploymentManager.existing('ENSResolver', ENSResolverAddress, 'goerli');
+        const subdomainHash = ethers.utils.namehash(ENSSubdomain);
+        const chainId = (await deploymentManager.hre.ethers.provider.getNetwork()).chainId.toString();
+        const newMarketObject = { baseSymbol: 'USDT', cometAddress: comet.address };
+        const officialMarketsJSON = JSON.parse(await ENSResolver.text(subdomainHash, ENSTextRecordKey));
+        if (officialMarketsJSON[chainId]) {
+            officialMarketsJSON[chainId].push(newMarketObject);
+        } else {
+            officialMarketsJSON[chainId] = [newMarketObject];
+        }
+
+        const actions = [
+            // 1. Set the factory in the Configurator
+            {
+                contract: configurator,
+                signature: 'setFactory(address,address)',
+                args: [comet.address, newFactoryAddress],
+            },
+
+            // 2. Set the configuration in the Configurator
+            {
+                contract: configurator,
+                signature: 'setConfiguration(address,(address,address,address,address,address,uint64,uint64,uint64,uint64,uint64,uint64,uint64,uint64,uint64,uint64,uint64,uint64,uint104,uint104,uint104,(address,address,uint8,uint64,uint64,uint64,uint128)[]))',
+                args: [comet.address, configuration],
+            },
+
+            // 3. Deploy and upgrade to a new version of Comet
+            {
+                contract: cometAdmin,
+                signature: "deployAndUpgradeTo(address,address)",
+                args: [configurator.address, comet.address],
+            },
+
+            // 4. Set the rewards configuration to COMP
+            {
+                contract: rewards,
+                signature: "setRewardConfig(address,address)",
+                args: [comet.address, COMPAddress],
+            },
+
+            // 5. Set the official markets text record on the subdomain
+            {
+                target: ENSResolverAddress,
+                signature: 'setText(bytes32,string,string)',
+                calldata: ethers.utils.defaultAbiCoder.encode(
+                    ['bytes32', 'string', 'string'],
+                    [subdomainHash, ENSTextRecordKey, JSON.stringify(officialMarketsJSON)]
+                )
+            },
+
+            // 6. Send USDT from Timelock to Comet
+            // XXX assert that funds have been transferred by diffing the balances before and after
+            {
+                contract: USDT,
+                signature: "transfer(address,uint256)",
+                args: [comet.address, exp(20_000_000, 6)],
+            },
+        ];
+
+        const description = "# Initialize cUSDTv3 on Goerli"
+        const txn = await deploymentManager.retry(
+            async () => trace((await governor.propose(...await proposal(actions, description))))
+        );
+
+        const event = txn.events.find(event => event.event === 'ProposalCreated');
+        const [proposalId] = event.args;
+
+        trace(`Created proposal ${proposalId}.`);
+    },
+
+    async verify(deploymentManager: DeploymentManager, govDeploymentManager: DeploymentManager, preMigrationBlockNumber: number) {
+        const ethers = deploymentManager.hre.ethers;
+        await deploymentManager.spider(); // Pull in Arbitrum COMP now that reward config has been set
+        const {
+            comet,
+            rewards,
+        } = await deploymentManager.getContracts();
+
+        const config = await rewards.rewardConfig(comet.address);
+
+        // Verify state changes
+        // const stateChanges = await diffState(comet, getCometConfig, preMigrationBlockNumber);
+        // TODO: Will uncomment once the comet has been deployed
+        // expect(stateChanges).to.deep.equal({
+        //   COMP: {
+        //     supplyCap: exp(500_000, 18)
+        //   },
+        //   WBTC: {
+        //     supplyCap: exp(35_000, 8)
+        //   },
+        //   WETH: {
+        //     supplyCap: exp(1_000_000, 18)
+        //   },
+        //   baseTrackingSupplySpeed: exp(34.74 / 86400, 15, 18),
+        //   baseTrackingBorrowSpeed: exp(34.74 / 86400, 15, 18),
+        // });
+
+        expect(config.token).to.be.equal(COMPAddress);
+        expect(config.rescaleFactor).to.be.equal(exp(1, 12));
+        expect(config.shouldUpscale).to.be.equal(true);
+
+        // Verify the seeded USDT reaches Comet reserve
+        expect(await comet.getReserves()).to.be.equal(exp(20_000_000, 6));
+
+        // Verify the official markets are updated
+        const ENSResolver = await deploymentManager.existing('ENSResolver', ENSResolverAddress, 'goerli');
+        const subdomainHash = ethers.utils.namehash(ENSSubdomain);
+        const officialMarkets = JSON.parse(await ENSResolver.text(subdomainHash, ENSTextRecordKey));
+
+        expect(officialMarkets).to.deep.equal({
+            5: [
+                {
+                    baseSymbol: 'USDC',
+                    cometAddress: '0x3EE77595A8459e93C2888b13aDB354017B198188',
+                },
+                {
+                    baseSymbol: 'WETH',
+                    cometAddress: '0x9A539EEc489AAA03D588212a164d0abdB5F08F5F',
+                },
+                {
+                    baseSymbol: 'USDT',
+                    cometAddress: comet.address,
+                }
+            ],
+
+            420: [
+                {
+                    baseSymbol: 'USDC',
+                    cometAddress: '0xb8F2f9C84ceD7bBCcc1Db6FB7bb1F19A9a4adfF4'
+                }
+            ],
+
+            421613: [
+                {
+                    baseSymbol: 'USDC.e',
+                    cometAddress: '0x1d573274E19174260c5aCE3f2251598959d24456',
+                },
+                {
+                    baseSymbol: 'USDC',
+                    cometAddress: '0x0C94d3F9D7F211630EDecAF085718Ac80821A6cA',
+                },
+            ],
+
+            59140: [
+                {
+                    baseSymbol: 'USDC',
+                    cometAddress: '0xa84b24A43ba1890A165f94Ad13d0196E5fD1023a'
+                }
+            ],
+
+            84531: [
+                {
+                    baseSymbol: 'USDC',
+                    cometAddress: '0xe78Fc55c884704F9485EDa042fb91BfE16fD55c1'
+                },
+                {
+                    baseSymbol: 'WETH',
+                    cometAddress: '0xED94f3052638620fE226a9661ead6a39C2a265bE'
+                }
+            ],
+
+            80001: [
+                {
+                    baseSymbol: 'USDC',
+                    cometAddress: '0xF09F0369aB0a875254fB565E52226c88f10Bc839'
+                },
+            ]
+        });
+    }
+});

test/buy-collateral-test.ts

@@ -409,7 +409,7 @@ describe('buyCollateral', function () {
   });
 
   describe('reentrancy', function() {
-    it('is blocked during reentrant supply', async () => {
+    it('is not broken by reentrancy supply', async () => {
       const wethArgs = {
         initial: 1e4,
         decimals: 18,
@@ -563,7 +563,7 @@ describe('buyCollateral', function () {
       expect(evilBobPortfolio.internal.EVIL).to.equal(0);
     });
 
-    it('reentrant supply is reverted', async () => {
+    it('reentrant buyCollateral is reverted', async () => {
       const wethArgs = {
         initial: 1e4,
         decimals: 18,
@@ -596,12 +596,13 @@ describe('buyCollateral', function () {
 
       // add attack to EVIL token
       const attack = Object.assign({}, await EVIL.getAttack(), {
-        attackType: ReentryAttack.SupplyFrom,
+        attackType: ReentryAttack.BuyCollateral,
         source: evilAlice.address,
         destination: evilBob.address,
         asset: EVIL.address,
         amount: 3000e6,
-        maxCalls: 1
+        maxCalls: 1,
+        collateralAsset: evilWETH.address,
       });
       await EVIL.setAttack(attack);
 
@@ -615,7 +616,8 @@ describe('buyCollateral', function () {
 
       // authorize EVIL, since callback will originate from EVIL token address
       await evilComet.connect(evilAlice).allow(EVIL.address, true);
-      // call buyCollateral; supplyFrom is called in in callback
+
+      // call buyCollateral; supplyFrom is called in callback
       await expect(evilComet
         .connect(evilAlice)
         .buyCollateral(

test/helpers.ts

@@ -44,7 +44,8 @@ export type Numeric = number | bigint;
 export enum ReentryAttack {
   TransferFrom = 0,
   WithdrawFrom = 1,
-  SupplyFrom = 2
+  SupplyFrom = 2,
+  BuyCollateral = 3,
 }
 
 export type ProtocolOpts = {