fix: Fix role check in OracleSwapFreezer upkeep function (#380)

aave · Jan 19, 2024 · e488e4d · e488e4d
1 parent e5f196e
commit e488e4d
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 9 deletions.
diff --git a/src/contracts/facilitators/gsm/swapFreezer/OracleSwapFreezer.sol b/src/contracts/facilitators/gsm/swapFreezer/OracleSwapFreezer.sol
@@ -121,15 +121,17 @@ contract OracleSwapFreezer is AutomationCompatibleInterface {
    * @return The action to take (none, freeze, or unfreeze)
    */
   function _getAction() internal view returns (Action) {
-    if (GSM.getIsSeized()) {
-      return Action.NONE;
-    } else if (!GSM.getIsFrozen()) {
-      if (_isActionAllowed(Action.FREEZE)) {
-        return Action.FREEZE;
-      }
-    } else if (_allowUnfreeze) {
-      if (_isActionAllowed(Action.UNFREEZE)) {
-        return Action.UNFREEZE;
+    if (GSM.hasRole(GSM.SWAP_FREEZER_ROLE(), address(this))) {
+      if (GSM.getIsSeized()) {
+        return Action.NONE;
+      } else if (!GSM.getIsFrozen()) {
+        if (_isActionAllowed(Action.FREEZE)) {
+          return Action.FREEZE;
+        }
+      } else if (_allowUnfreeze) {
+        if (_isActionAllowed(Action.UNFREEZE)) {
+          return Action.UNFREEZE;
+        }
       }
     }
     return Action.NONE;

diff --git a/src/test/TestGsmOracleSwapFreezer.t.sol b/src/test/TestGsmOracleSwapFreezer.t.sol
@@ -301,6 +301,22 @@ contract TestGsmOracleSwapFreezer is TestGhoBase {
     assertEq(GHO_GSM.getIsFrozen(), false, 'Unexpected final freeze state for GSM');
   }
 
+  function testCheckUpkeepNoSwapFreezeRole() public {
+    // Move price outside freeze range
+    PRICE_ORACLE.setAssetPrice(address(USDC_TOKEN), DEFAULT_FREEZE_LOWER_BOUND - 1);
+    (bool canPerformUpkeep, ) = swapFreezer.checkUpkeep('');
+    assertEq(canPerformUpkeep, true, 'Unexpected initial upkeep state');
+
+    // Revoke SwapFreezer role
+    GHO_GSM.revokeRole(GSM_SWAP_FREEZER_ROLE, address(swapFreezer));
+
+    // Upkeep shouldn't be possible
+    (canPerformUpkeep, ) = swapFreezer.checkUpkeep('');
+    assertEq(canPerformUpkeep, false, 'Unexpected upkeep state');
+    // Do not revert, it's a no-op execution
+    swapFreezer.performUpkeep('');
+  }
+
   function testGetCanUnfreeze() public {
     assertEq(swapFreezer.getCanUnfreeze(), true, 'Unexpected initial unfreeze state');
     swapFreezer = new OracleSwapFreezer(
@@ -315,4 +331,34 @@ contract TestGsmOracleSwapFreezer is TestGhoBase {
     );
     assertEq(swapFreezer.getCanUnfreeze(), false, 'Unexpected final unfreeze state');
   }
+
+  function testFuzzUpkeepConsistency(uint256 assetPrice, bool grantRole) public {
+    PRICE_ORACLE.setAssetPrice(address(USDC_TOKEN), assetPrice);
+    OracleSwapFreezer agent = new OracleSwapFreezer(
+      GHO_GSM,
+      address(USDC_TOKEN),
+      IPoolAddressesProvider(address(PROVIDER)),
+      DEFAULT_FREEZE_LOWER_BOUND,
+      DEFAULT_FREEZE_UPPER_BOUND,
+      DEFAULT_UNFREEZE_LOWER_BOUND,
+      DEFAULT_UNFREEZE_UPPER_BOUND,
+      true
+    );
+    if (grantRole) {
+      GHO_GSM.grantRole(GSM_SWAP_FREEZER_ROLE, address(agent));
+    }
+
+    // If canPerformUpkeep, there must be a state change
+    bool freezeState = GHO_GSM.getIsFrozen();
+    (bool canPerformUpkeep, ) = agent.checkUpkeep('');
+
+    agent.performUpkeep('');
+    if (canPerformUpkeep) {
+      // state change
+      assertEq(freezeState, !GHO_GSM.getIsFrozen(), 'no state change after performUpkeep');
+    } else {
+      // no state change
+      assertEq(freezeState, GHO_GSM.getIsFrozen(), 'state change after performUpkeep');
+    }
+  }
 }