refactor: replace shotgun auth testing with an incremental approach

makerdao · Dec 18, 2023 · 6a3c6b2 · 6a3c6b2
1 parent dcff008
commit 6a3c6b2
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 29 deletions.
diff --git a/src/DssSpell.t.base.sol b/src/DssSpell.t.base.sol
@@ -1410,48 +1410,71 @@ contract DssSpellTestBase is Config, DssTest {
         );
     }
 
+    /**
+     * @dev Checks if the deployer of a contract has not kept `wards` access to the contract.
+     * Notice that it depends on `deployers` being kept up-to-date.
+     */
     function _checkWards(address _addr, string memory contractName) internal {
         for (uint256 i = 0; i < deployers.count(); i ++) {
             address deployer = deployers.addr(i);
-            (bool ok, bytes memory data) = _addr.call(
-                abi.encodeWithSignature("wards(address)", deployer)
-            );
+            if(_skipWards(_addr, deployer)) {
+                continue;
+            }
+
+            (bool ok, bytes memory data) = _addr.call(abi.encodeWithSignature("wards(address)", deployer));
             if (!ok || data.length != 32) return;
+
             uint256 ward = abi.decode(data, (uint256));
             if (ward > 0) {
-                if (_skipWards(_addr, deployer)) continue;
-                emit log("Error: Bad Auth");
                 emit log_named_address("   Deployer Address", deployer);
                 emit log_named_string("  Affected Contract", contractName);
-                fail();
+                fail("Error: Bad Auth");
             }
         }
     }
 
-    function _checkSource(address _addr, string memory contractName) internal {
-        (bool ok, bytes memory data) =
-            _addr.call(abi.encodeWithSignature("src()"));
+    /**
+     * @dev Same as `_checkWards`, but for OSMs' underlying Median contracts.
+     */
+    function _checkOsmSrcWards(address _addr, string memory contractName) internal {
+        (bool ok, bytes memory data) = _addr.call(abi.encodeWithSignature("src()"));
         if (!ok || data.length != 32) return;
+
         address source = abi.decode(data, (address));
-        string memory sourceName = string(
-            abi.encodePacked("source of ", contractName)
-        );
+        string memory sourceName = string(abi.encodePacked("src of ", contractName));
         _checkWards(source, sourceName);
     }
 
-    function _checkAuth(bool onlySource) internal {
+    /**
+     * @notice Checks if the deployer of contracts being added to the chainlog has not kept `wards` access to it.
+     * @dev Reverts if any non-existent keys are present the list.
+     * @param keys A list of chainlog keys.
+     */
+    function _checkAuth(bytes32[] memory keys) internal {
         _vote(address(spell));
         _scheduleWaitAndCast(address(spell));
         assertTrue(spell.done(), "TestError/spell-not-done");
 
-        bytes32[] memory contractNames = chainLog.list();
-        for(uint256 i = 0; i < contractNames.length; i++) {
-            address _addr = chainLog.getAddress(contractNames[i]);
-            string memory contractName = string(
-                abi.encodePacked(contractNames[i])
-            );
-            if (onlySource) _checkSource(_addr, contractName);
-            else _checkWards(_addr, contractName);
+        for(uint256 i = 0; i < keys.length; i++) {
+            _checkWards(chainLog.getAddress(keys[i]), string(abi.encodePacked(keys[i])));
+        }
+    }
+
+    /**
+     * @notice Same as `_checkAuth`, but specific for OSMs.
+     * @dev Reverts if any non-existent keys are present the list.
+     * @param keys A list of chainlog keys for OSMs.
+     */
+    function _checkOsmAuth(bytes32[] memory keys) internal {
+        _vote(address(spell));
+        _scheduleWaitAndCast(address(spell));
+        assertTrue(spell.done(), "TestError/spell-not-done");
+
+        for(uint256 i = 0; i < keys.length; i++) {
+            address _addr = chainLog.getAddress(keys[i]);
+            string memory _key = string(abi.encodePacked(keys[i]));
+            _checkWards(_addr, _key);
+            _checkOsmSrcWards(_addr, _key);
         }
     }
 

diff --git a/src/DssSpell.t.sol b/src/DssSpell.t.sol
@@ -101,14 +101,6 @@ contract DssSpellTest is DssSpellTestBase {
         _testUseEta();
     }
 
-    function testAuth() public {
-        _checkAuth(false);
-    }
-
-    function testAuthInSources() public {
-        _checkAuth(true);
-    }
-
     function testBytecodeMatches() public {
         _testBytecodeMatches();
     }
@@ -235,6 +227,23 @@ contract DssSpellTest is DssSpellTestBase {
         //assertEq(OsmAbstract(0xF15993A5C5BE496b8e1c9657Fd2233b579Cd3Bc6).wards(ORACLE_WALLET01), 1);
     }
 
+    function testAuth() private {  // make private to disable
+        bytes32[] memory keys = new bytes32[](2);
+        keys[0] = "MCD_JOIN_TOKEN_X";
+        keys[1] = "MCD_CLIP_TOKEN_X";
+
+        _checkAuth(keys);
+    }
+
+    function testOsmAuth() private {  // make private to disable
+        bytes32[] memory keys = new bytes32[](3);
+        keys[0] = "PIP_XXX";
+        keys[1] = "PIP_YYY";
+        keys[2] = "PIP_ZZZ";
+
+        _checkOsmAuth(keys);
+    }
+
     function testRemoveChainlogValues() private { // make private to disable
         _vote(address(spell));
         _scheduleWaitAndCast(address(spell));