TRUST QA-2: DefaultThreshold check for Non-Self referential (#980)

reserve-protocol · Oct 17, 2023 · 29c0f4c · 29c0f4c
1 parent 2de6b0d
commit 29c0f4c
Show file tree

Hide file tree

Showing 37 changed files with 191 additions and 4 deletions.
diff --git a/contracts/plugins/assets/EURFiatCollateral.sol b/contracts/plugins/assets/EURFiatCollateral.sol
@@ -27,6 +27,8 @@ contract EURFiatCollateral is FiatCollateral {
     ) FiatCollateral(config) {
         require(address(targetUnitChainlinkFeed_) != address(0), "missing targetUnit feed");
         require(targetUnitOracleTimeout_ > 0, "targetUnitOracleTimeout zero");
+        require(config.defaultThreshold > 0, "defaultThreshold zero");
+
         targetUnitChainlinkFeed = targetUnitChainlinkFeed_;
         targetUnitOracleTimeout = targetUnitOracleTimeout_;
     }

diff --git a/contracts/plugins/assets/FiatCollateral.sol b/contracts/plugins/assets/FiatCollateral.sol
@@ -75,6 +75,10 @@ contract FiatCollateral is ICollateral, Asset {
         }
         require(config.delayUntilDefault <= 1209600, "delayUntilDefault too long");
 
+        // Note: This contract is designed to allow setting defaultThreshold = 0 to disable
+        // default checks. You can apply the check below to child contracts when required
+        // require(config.defaultThreshold > 0, "defaultThreshold zero");
+
         targetName = config.targetName;
         delayUntilDefault = config.delayUntilDefault;
 

diff --git a/contracts/plugins/assets/L2LSDCollateral.sol b/contracts/plugins/assets/L2LSDCollateral.sol
@@ -30,6 +30,7 @@ abstract contract L2LSDCollateral is AppreciatingFiatCollateral {
     ) AppreciatingFiatCollateral(config, revenueHiding) {
         require(address(_exchangeRateChainlinkFeed) != address(0), "missing exchangeRate feed");
         require(_exchangeRateChainlinkTimeout != 0, "exchangeRateChainlinkTimeout zero");
+        require(config.defaultThreshold > 0, "defaultThreshold zero");
 
         exchangeRateChainlinkFeed = _exchangeRateChainlinkFeed;
         exchangeRateChainlinkTimeout = _exchangeRateChainlinkTimeout;

diff --git a/contracts/plugins/assets/NonFiatCollateral.sol b/contracts/plugins/assets/NonFiatCollateral.sol
@@ -27,6 +27,8 @@ contract NonFiatCollateral is FiatCollateral {
     ) FiatCollateral(config) {
         require(address(targetUnitChainlinkFeed_) != address(0), "missing targetUnit feed");
         require(targetUnitOracleTimeout_ > 0, "targetUnitOracleTimeout zero");
+        require(config.defaultThreshold > 0, "defaultThreshold zero");
+
         targetUnitChainlinkFeed = targetUnitChainlinkFeed_;
         targetUnitOracleTimeout = targetUnitOracleTimeout_;
     }

diff --git a/contracts/plugins/assets/aave-v3/AaveV3FiatCollateral.sol b/contracts/plugins/assets/aave-v3/AaveV3FiatCollateral.sol
@@ -20,7 +20,9 @@ contract AaveV3FiatCollateral is AppreciatingFiatCollateral {
     /// @param revenueHiding {1} A value like 1e-6 that represents the maximum refPerTok to hide
     constructor(CollateralConfig memory config, uint192 revenueHiding)
         AppreciatingFiatCollateral(config, revenueHiding)
-    {}
+    {
+        require(config.defaultThreshold > 0, "defaultThreshold zero");
+    }
 
     // solhint-enable no-empty-blocks
 

diff --git a/contracts/plugins/assets/aave/ATokenFiatCollateral.sol b/contracts/plugins/assets/aave/ATokenFiatCollateral.sol
@@ -41,7 +41,9 @@ contract ATokenFiatCollateral is AppreciatingFiatCollateral {
     /// @param revenueHiding {1} A value like 1e-6 that represents the maximum refPerTok to hide
     constructor(CollateralConfig memory config, uint192 revenueHiding)
         AppreciatingFiatCollateral(config, revenueHiding)
-    {}
+    {
+        require(config.defaultThreshold > 0, "defaultThreshold zero");
+    }
 
     // solhint-enable no-empty-blocks
 

diff --git a/contracts/plugins/assets/ankr/AnkrStakedEthCollateral.sol b/contracts/plugins/assets/ankr/AnkrStakedEthCollateral.sol
@@ -33,6 +33,7 @@ contract AnkrStakedEthCollateral is AppreciatingFiatCollateral {
     ) AppreciatingFiatCollateral(config, revenueHiding) {
         require(address(_targetPerTokChainlinkFeed) != address(0), "missing targetPerTok feed");
         require(_targetPerTokChainlinkTimeout != 0, "targetPerTokChainlinkTimeout zero");
+        require(config.defaultThreshold > 0, "defaultThreshold zero");
 
         targetPerTokChainlinkFeed = _targetPerTokChainlinkFeed;
         targetPerTokChainlinkTimeout = _targetPerTokChainlinkTimeout;

diff --git a/contracts/plugins/assets/cbeth/CBETHCollateral.sol b/contracts/plugins/assets/cbeth/CBETHCollateral.sol
@@ -32,6 +32,7 @@ contract CBEthCollateral is AppreciatingFiatCollateral {
     ) AppreciatingFiatCollateral(config, revenueHiding) {
         require(address(_targetPerTokChainlinkFeed) != address(0), "missing targetPerTok feed");
         require(_targetPerTokChainlinkTimeout != 0, "targetPerTokChainlinkTimeout zero");
+        require(config.defaultThreshold > 0, "defaultThreshold zero");
 
         targetPerTokChainlinkFeed = _targetPerTokChainlinkFeed;
         targetPerTokChainlinkTimeout = _targetPerTokChainlinkTimeout;

diff --git a/contracts/plugins/assets/cbeth/CBETHCollateralL2.sol b/contracts/plugins/assets/cbeth/CBETHCollateralL2.sol
@@ -41,6 +41,7 @@ contract CBEthCollateralL2 is L2LSDCollateral {
     {
         require(address(_targetPerTokChainlinkFeed) != address(0), "missing targetPerTok feed");
         require(_targetPerTokChainlinkTimeout != 0, "targetPerTokChainlinkTimeout zero");
+        require(config.defaultThreshold > 0, "defaultThreshold zero");
 
         targetPerTokChainlinkFeed = _targetPerTokChainlinkFeed;
         targetPerTokChainlinkTimeout = _targetPerTokChainlinkTimeout;

diff --git a/contracts/plugins/assets/compoundv2/CTokenFiatCollateral.sol b/contracts/plugins/assets/compoundv2/CTokenFiatCollateral.sol
@@ -29,6 +29,8 @@ contract CTokenFiatCollateral is AppreciatingFiatCollateral {
     constructor(CollateralConfig memory config, uint192 revenueHiding)
         AppreciatingFiatCollateral(config, revenueHiding)
     {
+        require(config.defaultThreshold > 0, "defaultThreshold zero");
+
         ICToken _cToken = ICToken(address(config.erc20));
         address _underlying = _cToken.underlying();
         uint8 _referenceERC20Decimals;

diff --git a/contracts/plugins/assets/compoundv2/CTokenNonFiatCollateral.sol b/contracts/plugins/assets/compoundv2/CTokenNonFiatCollateral.sol
@@ -30,6 +30,7 @@ contract CTokenNonFiatCollateral is CTokenFiatCollateral {
     ) CTokenFiatCollateral(config, revenueHiding) {
         require(address(targetUnitChainlinkFeed_) != address(0), "missing targetUnit feed");
         require(targetUnitOracleTimeout_ > 0, "targetUnitOracleTimeout zero");
+        require(config.defaultThreshold > 0, "defaultThreshold zero");
         targetUnitChainlinkFeed = targetUnitChainlinkFeed_;
         targetUnitOracleTimeout = targetUnitOracleTimeout_;
     }

diff --git a/contracts/plugins/assets/compoundv3/CTokenV3Collateral.sol b/contracts/plugins/assets/compoundv3/CTokenV3Collateral.sol
@@ -39,6 +39,7 @@ contract CTokenV3Collateral is AppreciatingFiatCollateral {
         uint192 revenueHiding,
         uint256 reservesThresholdIffy_
     ) AppreciatingFiatCollateral(config, revenueHiding) {
+        require(config.defaultThreshold > 0, "defaultThreshold zero");
         rewardERC20 = ICusdcV3Wrapper(address(config.erc20)).rewardERC20();
         comet = IComet(address(ICusdcV3Wrapper(address(erc20)).underlyingComet()));
         reservesThresholdIffy = reservesThresholdIffy_;

diff --git a/contracts/plugins/assets/dsr/SDaiCollateral.sol b/contracts/plugins/assets/dsr/SDaiCollateral.sol
@@ -35,6 +35,7 @@ contract SDaiCollateral is AppreciatingFiatCollateral {
         uint192 revenueHiding,
         IPot _pot
     ) AppreciatingFiatCollateral(config, revenueHiding) {
+        require(config.defaultThreshold > 0, "defaultThreshold zero");
         pot = _pot;
     }
 

diff --git a/contracts/plugins/assets/frax-eth/SFraxEthCollateral.sol b/contracts/plugins/assets/frax-eth/SFraxEthCollateral.sol
@@ -23,7 +23,9 @@ contract SFraxEthCollateral is AppreciatingFiatCollateral {
     /// @param config.chainlinkFeed Feed units: {UoA/target}
     constructor(CollateralConfig memory config, uint192 revenueHiding)
         AppreciatingFiatCollateral(config, revenueHiding)
-    {}
+    {
+        require(config.defaultThreshold > 0, "defaultThreshold zero");
+    }
 
     // solhint-enable no-empty-blocks
 

diff --git a/contracts/plugins/assets/lido/LidoStakedEthCollateral.sol b/contracts/plugins/assets/lido/LidoStakedEthCollateral.sol
@@ -35,6 +35,8 @@ contract LidoStakedEthCollateral is AppreciatingFiatCollateral {
     ) AppreciatingFiatCollateral(config, revenueHiding) {
         require(address(_targetPerRefChainlinkFeed) != address(0), "missing targetPerRef feed");
         require(_targetPerRefChainlinkTimeout > 0, "targetPerRefChainlinkTimeout zero");
+        require(config.defaultThreshold > 0, "defaultThreshold zero");
+
         targetPerRefChainlinkFeed = _targetPerRefChainlinkFeed;
         targetPerRefChainlinkTimeout = _targetPerRefChainlinkTimeout;
     }

diff --git a/contracts/plugins/assets/morpho-aave/MorphoFiatCollateral.sol b/contracts/plugins/assets/morpho-aave/MorphoFiatCollateral.sol
@@ -28,6 +28,7 @@ contract MorphoFiatCollateral is AppreciatingFiatCollateral {
         AppreciatingFiatCollateral(config, revenueHiding)
     {
         require(address(config.erc20) != address(0), "missing erc20");
+        require(config.defaultThreshold > 0, "defaultThreshold zero");
         MorphoTokenisedDeposit vault = MorphoTokenisedDeposit(address(config.erc20));
         oneShare = 10**vault.decimals();
         refDecimals = int8(uint8(IERC20Metadata(vault.asset()).decimals()));

diff --git a/contracts/plugins/assets/rocket-eth/RethCollateral.sol b/contracts/plugins/assets/rocket-eth/RethCollateral.sol
@@ -31,6 +31,7 @@ contract RethCollateral is AppreciatingFiatCollateral {
     ) AppreciatingFiatCollateral(config, revenueHiding) {
         require(address(_targetPerTokChainlinkFeed) != address(0), "missing targetPerTok feed");
         require(_targetPerTokChainlinkTimeout != 0, "targetPerTokChainlinkTimeout zero");
+        require(config.defaultThreshold > 0, "defaultThreshold zero");
 
         targetPerTokChainlinkFeed = _targetPerTokChainlinkFeed;
         targetPerTokChainlinkTimeout = _targetPerTokChainlinkTimeout;

diff --git a/contracts/plugins/assets/stargate/StargatePoolFiatCollateral.sol b/contracts/plugins/assets/stargate/StargatePoolFiatCollateral.sol
@@ -22,6 +22,7 @@ contract StargatePoolFiatCollateral is AppreciatingFiatCollateral {
     constructor(CollateralConfig memory config, uint192 revenueHiding)
         AppreciatingFiatCollateral(config, revenueHiding)
     {
+        require(config.defaultThreshold > 0, "defaultThreshold zero");
         pool = StargateRewardableWrapper(address(config.erc20)).pool();
     }
 

diff --git a/test/plugins/Collateral.test.ts b/test/plugins/Collateral.test.ts
@@ -262,6 +262,44 @@ describe('Collateral contracts', () => {
       ).to.be.revertedWith('targetName missing')
     })
 
+    it('Should not allow 0 defaultThreshold', async () => {
+      // ATokenFiatCollateral
+      await expect(
+        ATokenFiatCollateralFactory.deploy(
+          {
+            priceTimeout: PRICE_TIMEOUT,
+            chainlinkFeed: await tokenCollateral.chainlinkFeed(),
+            oracleError: ORACLE_ERROR,
+            erc20: aToken.address,
+            maxTradeVolume: config.rTokenMaxTradeVolume,
+            oracleTimeout: ORACLE_TIMEOUT,
+            targetName: ethers.utils.formatBytes32String('USD'),
+            defaultThreshold: bn(0),
+            delayUntilDefault: DELAY_UNTIL_DEFAULT,
+          },
+          REVENUE_HIDING
+        )
+      ).to.be.revertedWith('defaultThreshold zero')
+
+      // CTokenFiatCollateral
+      await expect(
+        CTokenFiatCollateralFactory.deploy(
+          {
+            priceTimeout: PRICE_TIMEOUT,
+            chainlinkFeed: await tokenCollateral.chainlinkFeed(),
+            oracleError: ORACLE_ERROR,
+            erc20: cToken.address,
+            maxTradeVolume: config.rTokenMaxTradeVolume,
+            oracleTimeout: ORACLE_TIMEOUT,
+            targetName: ethers.utils.formatBytes32String('USD'),
+            defaultThreshold: bn(0),
+            delayUntilDefault: DELAY_UNTIL_DEFAULT,
+          },
+          REVENUE_HIDING
+        )
+      ).to.be.revertedWith('defaultThreshold zero')
+    })
+
     it('Should not allow missing delayUntilDefault', async () => {
       await expect(
         FiatCollateralFactory.deploy({
@@ -1236,6 +1274,26 @@ describe('Collateral contracts', () => {
       ).to.be.revertedWith('targetUnitOracleTimeout zero')
     })
 
+    it('Should not allow 0 defaultThreshold', async () => {
+      await expect(
+        NonFiatCollFactory.deploy(
+          {
+            priceTimeout: PRICE_TIMEOUT,
+            chainlinkFeed: referenceUnitOracle.address,
+            oracleError: ORACLE_ERROR,
+            erc20: nonFiatToken.address,
+            maxTradeVolume: config.rTokenMaxTradeVolume,
+            oracleTimeout: ORACLE_TIMEOUT,
+            targetName: ethers.utils.formatBytes32String('BTC'),
+            defaultThreshold: bn(0),
+            delayUntilDefault: DELAY_UNTIL_DEFAULT,
+          },
+          targetUnitOracle.address,
+          ORACLE_TIMEOUT
+        )
+      ).to.be.revertedWith('defaultThreshold zero')
+    })
+
     it('Should setup collateral correctly', async function () {
       // Non-Fiat Token
       expect(await nonFiatCollateral.isCollateral()).to.equal(true)
@@ -1530,6 +1588,27 @@ describe('Collateral contracts', () => {
       ).to.be.revertedWith('targetUnitOracleTimeout zero')
     })
 
+    it('Should not allow 0 defaultThreshold', async () => {
+      await expect(
+        CTokenNonFiatFactory.deploy(
+          {
+            priceTimeout: PRICE_TIMEOUT,
+            chainlinkFeed: referenceUnitOracle.address,
+            oracleError: ORACLE_ERROR,
+            erc20: cNonFiatTokenVault.address,
+            maxTradeVolume: config.rTokenMaxTradeVolume,
+            oracleTimeout: ORACLE_TIMEOUT,
+            targetName: ethers.utils.formatBytes32String('BTC'),
+            defaultThreshold: bn(0),
+            delayUntilDefault: DELAY_UNTIL_DEFAULT,
+          },
+          targetUnitOracle.address,
+          ORACLE_TIMEOUT,
+          REVENUE_HIDING
+        )
+      ).to.be.revertedWith('defaultThreshold zero')
+    })
+
     it('Should setup collateral correctly', async function () {
       // Non-Fiat Token
       expect(await cTokenNonFiatCollateral.isCollateral()).to.equal(true)
@@ -2366,6 +2445,26 @@ describe('Collateral contracts', () => {
       ).to.be.revertedWith('targetUnitOracleTimeout zero')
     })
 
+    it('Should not allow 0 defaultThreshold', async () => {
+      await expect(
+        EURFiatCollateralFactory.deploy(
+          {
+            priceTimeout: PRICE_TIMEOUT,
+            chainlinkFeed: referenceUnitOracle.address,
+            oracleError: ORACLE_ERROR,
+            erc20: eurFiatToken.address,
+            maxTradeVolume: config.rTokenMaxTradeVolume,
+            oracleTimeout: ORACLE_TIMEOUT,
+            targetName: ethers.utils.formatBytes32String('BTC'),
+            defaultThreshold: bn(0),
+            delayUntilDefault: DELAY_UNTIL_DEFAULT,
+          },
+          targetUnitOracle.address,
+          ORACLE_TIMEOUT
+        )
+      ).to.be.revertedWith('defaultThreshold zero')
+    })
+
     it('Should not revert during refresh when price2 is 0', async () => {
       const targetFeedAddr = await eurFiatCollateral.targetUnitChainlinkFeed()
       const targetFeed = await ethers.getContractAt('MockV3Aggregator', targetFeedAddr)

diff --git a/test/plugins/individual-collateral/aave-v3/AaveV3FiatCollateral.test.ts b/test/plugins/individual-collateral/aave-v3/AaveV3FiatCollateral.test.ts
@@ -213,6 +213,7 @@ export const stableOpts = {
   itChecksTargetPerRefDefault: it,
   itChecksRefPerTokDefault: it,
   itHasRevenueHiding: it,
+  itChecksNonZeroDefaultThreshold: it,
   itIsPricedByPeg: true,
   chainlinkDefaultAnswer: 1e8,
   itChecksPriceChanges: it,

diff --git a/test/plugins/individual-collateral/aave/ATokenFiatCollateral.test.ts b/test/plugins/individual-collateral/aave/ATokenFiatCollateral.test.ts
@@ -428,7 +428,7 @@ describeFork(`ATokenFiatCollateral - Mainnet Forking P${IMPLEMENTATION}`, functi
     // Validate constructor arguments
     // Note: Adapt it to your plugin constructor validations
     it('Should validate constructor arguments correctly', async () => {
-      // stkAAVEtroller
+      // Missing erc20
       await expect(
         ATokenFiatCollateralFactory.deploy(
           {
@@ -445,6 +445,24 @@ describeFork(`ATokenFiatCollateral - Mainnet Forking P${IMPLEMENTATION}`, functi
           REVENUE_HIDING
         )
       ).to.be.revertedWith('missing erc20')
+
+      // defaultThreshold = 0
+      await expect(
+        ATokenFiatCollateralFactory.deploy(
+          {
+            priceTimeout: PRICE_TIMEOUT,
+            chainlinkFeed: networkConfig[chainId].chainlinkFeeds.DAI as string,
+            oracleError: ORACLE_ERROR,
+            erc20: staticAToken.address,
+            maxTradeVolume: config.rTokenMaxTradeVolume,
+            oracleTimeout: ORACLE_TIMEOUT,
+            targetName: ethers.utils.formatBytes32String('USD'),
+            defaultThreshold: bn(0),
+            delayUntilDefault,
+          },
+          REVENUE_HIDING
+        )
+      ).to.be.revertedWith('defaultThreshold zero')
     })
   })
 

diff --git a/test/plugins/individual-collateral/ankr/AnkrEthCollateralTestSuite.test.ts b/test/plugins/individual-collateral/ankr/AnkrEthCollateralTestSuite.test.ts
@@ -289,6 +289,7 @@ const opts = {
   itChecksRefPerTokDefault: it,
   itChecksPriceChanges: it,
   itHasRevenueHiding: it,
+  itChecksNonZeroDefaultThreshold: it,
   resetFork,
   collateralName: 'AnkrStakedETH',
   chainlinkDefaultAnswer,

diff --git a/test/plugins/individual-collateral/cbeth/CBETHCollateral.test.ts b/test/plugins/individual-collateral/cbeth/CBETHCollateral.test.ts
@@ -246,6 +246,7 @@ const opts = {
   itChecksRefPerTokDefault: it,
   itChecksPriceChanges: it,
   itHasRevenueHiding: it,
+  itChecksNonZeroDefaultThreshold: it,
   resetFork,
   collateralName: 'CBEthCollateral',
   chainlinkDefaultAnswer,

diff --git a/test/plugins/individual-collateral/cbeth/CBETHCollateralL2.test.ts b/test/plugins/individual-collateral/cbeth/CBETHCollateralL2.test.ts
@@ -277,6 +277,7 @@ const opts = {
   itChecksRefPerTokDefault: it,
   itChecksPriceChanges: it,
   itHasRevenueHiding: it,
+  itChecksNonZeroDefaultThreshold: it,
   resetFork,
   collateralName: 'CBEthCollateralL2',
   chainlinkDefaultAnswer,

diff --git a/test/plugins/individual-collateral/collateralTests.ts b/test/plugins/individual-collateral/collateralTests.ts
@@ -61,6 +61,7 @@ export default function fn<X extends CollateralFixtureContext>(
     itChecksTargetPerRefDefault,
     itChecksRefPerTokDefault,
     itChecksPriceChanges,
+    itChecksNonZeroDefaultThreshold,
     itHasRevenueHiding,
     itIsPricedByPeg,
     resetFork,
@@ -110,6 +111,12 @@ export default function fn<X extends CollateralFixtureContext>(
         )
       })
 
+      itChecksNonZeroDefaultThreshold('does not allow 0 defaultThreshold', async () => {
+        await expect(deployCollateral({ defaultThreshold: bn('0') })).to.be.revertedWith(
+          'defaultThreshold zero'
+        )
+      })
+
       describe('collateral-specific tests', collateralSpecificConstructorTests)
     })
 

diff --git a/test/plugins/individual-collateral/compoundv2/CTokenFiatCollateral.test.ts b/test/plugins/individual-collateral/compoundv2/CTokenFiatCollateral.test.ts
@@ -474,6 +474,24 @@ describeFork(`CTokenFiatCollateral - Mainnet Forking P${IMPLEMENTATION}`, functi
           REVENUE_HIDING
         )
       ).to.be.revertedWith('referenceERC20Decimals missing')
+
+      // defaultThreshold = 0
+      await expect(
+        CTokenCollateralFactory.deploy(
+          {
+            priceTimeout: PRICE_TIMEOUT,
+            chainlinkFeed: networkConfig[chainId].chainlinkFeeds.DAI as string,
+            oracleError: ORACLE_ERROR,
+            erc20: cDaiVault.address,
+            maxTradeVolume: config.rTokenMaxTradeVolume,
+            oracleTimeout: ORACLE_TIMEOUT,
+            targetName: ethers.utils.formatBytes32String('USD'),
+            defaultThreshold: bn(0),
+            delayUntilDefault,
+          },
+          REVENUE_HIDING
+        )
+      ).to.be.revertedWith('defaultThreshold zero')
     })
   })