✨ Implement 2-step feeTo address update (#169)

valorem-labs-inc · Dec 22, 2022 · 83a9ac8 · 83a9ac8
1 parent 0cc7818
commit 83a9ac8
Show file tree

Hide file tree

Showing 3 changed files with 91 additions and 22 deletions.
diff --git a/src/OptionSettlementEngine.sol b/src/OptionSettlementEngine.sol
@@ -109,6 +109,9 @@ contract OptionSettlementEngine is ERC1155, IOptionSettlementEngine {
     /// @notice Details about the option, buckets, and claims per option type.
     mapping(uint160 => OptionTypeState) private optionTypeStates;
 
+    /// @notice The new feeTo address, pending explicit acceptance by this address.
+    address private pendingFeeTo;
+
     /*//////////////////////////////////////////////////////////////
     //  State Variables - Public
     //////////////////////////////////////////////////////////////*/
@@ -633,9 +636,29 @@ contract OptionSettlementEngine is ERC1155, IOptionSettlementEngine {
         if (newFeeTo == address(0)) {
             revert InvalidAddress(address(0));
         }
-        feeTo = newFeeTo;
+        pendingFeeTo = newFeeTo;
+    }
+
+    /// @inheritdoc IOptionSettlementEngine
+    function acceptFeeTo() external {
+        if (msg.sender != pendingFeeTo) {
+            revert AccessControlViolation(msg.sender, pendingFeeTo);
+        }
+
+        feeTo = msg.sender;
+        pendingFeeTo = address(0);
+
+        emit FeeToUpdated(feeTo);
+    }
+
+    /// @inheritdoc IOptionSettlementEngine
+    function setTokenURIGenerator(address newTokenURIGenerator) external onlyFeeTo {
+        if (newTokenURIGenerator == address(0)) {
+            revert InvalidAddress(address(0));
+        }
+        tokenURIGenerator = ITokenURIGenerator(newTokenURIGenerator);
 
-        emit FeeToUpdated(newFeeTo);
+        emit TokenURIGeneratorUpdated(newTokenURIGenerator);
     }
 
     /// @inheritdoc IOptionSettlementEngine
@@ -662,16 +685,6 @@ contract OptionSettlementEngine is ERC1155, IOptionSettlementEngine {
         }
     }
 
-    /// @inheritdoc IOptionSettlementEngine
-    function setTokenURIGenerator(address newTokenURIGenerator) external onlyFeeTo {
-        if (newTokenURIGenerator == address(0)) {
-            revert InvalidAddress(address(0));
-        }
-        tokenURIGenerator = ITokenURIGenerator(newTokenURIGenerator);
-
-        emit TokenURIGeneratorUpdated(newTokenURIGenerator);
-    }
-
     /*//////////////////////////////////////////////////////////////
     //  Private Views
     //////////////////////////////////////////////////////////////*/

diff --git a/src/interfaces/IOptionSettlementEngine.sol b/src/interfaces/IOptionSettlementEngine.sol
@@ -360,7 +360,8 @@ interface IOptionSettlementEngine {
     function feeBalance(address token) external view returns (uint256);
 
     /**
-     * @return fee The protocol fee, expressed in basis points.
+     * @notice Gets the protocol fee, expressed in basis points.
+     * @return fee The protocol fee.
      */
     function feeBps() external view returns (uint8 fee);
 
@@ -371,8 +372,8 @@ interface IOptionSettlementEngine {
     function feesEnabled() external view returns (bool enabled);
 
     /**
-     * @notice Returns the address which protocol fees are swept to.
-     * @return The address which fees are swept to.
+     * @notice Returns the address to which protocol fees are swept.
+     * @return The address to which fees are swept.
      */
     function feeTo() external view returns (address);
 
@@ -462,11 +463,19 @@ interface IOptionSettlementEngine {
     function setFeesEnabled(bool enabled) external;
 
     /**
-     * @notice Updates the address fees are swept to.
-     * @param newFeeTo The new address which fees are swept to.
+     * @notice Nominates a new address to which fees should be swept, requiring
+     * the new feeTo address to accept before the update is complete. See also
+     * acceptFeeTo().
+     * @param newFeeTo The new address to which fees should be swept.
      */
     function setFeeTo(address newFeeTo) external;
 
+    /**
+     * @notice Accepts the new feeTo address and completes the update.
+     * See also setFeeTo(address newFeeTo).
+     */
+    function acceptFeeTo() external;
+
     /**
      * @notice Updates the contract address for generating token URIs for tokens.
      * @param newTokenURIGenerator The address of the new ITokenURIGenerator contract.

diff --git a/test/OptionSettlementEngine.unit.t.sol b/test/OptionSettlementEngine.unit.t.sol
@@ -839,20 +839,54 @@ contract OptionSettlementUnitTest is BaseEngineTest {
     }
 
     /*//////////////////////////////////////////////////////////////
-    // function setFeeTo(address newFeeTo) external
+    // function setFeeTo(address newFeeTo) external + function acceptFeeTo() external
     //////////////////////////////////////////////////////////////*/
 
-    function test_unitSetFeeTo() public {
+    function test_unitSetFeeToAndAcceptFeeTo() public {
+        address newFeeTo = address(0xCAFE);
+
         // precondition check
         assertEq(engine.feeTo(), FEE_TO);
 
+        vm.prank(FEE_TO);
+        engine.setFeeTo(newFeeTo);
+
         vm.expectEmit(true, true, true, true);
-        emit FeeToUpdated(address(0xCAFE));
+        emit FeeToUpdated(newFeeTo);
+
+        vm.prank(newFeeTo);
+        engine.acceptFeeTo();
+
+        assertEq(engine.feeTo(), newFeeTo);
+    }
 
+    function test_unitSetFeeToAndAcceptFeeToTwoTimes() public {
+        address newFeeTo = address(0xCAFE);
+        address newNewFeeTo = address(0xBEEF);
+
+        // First time around.
         vm.prank(FEE_TO);
-        engine.setFeeTo(address(0xCAFE));
+        engine.setFeeTo(newFeeTo);
+
+        vm.expectEmit(true, true, true, true);
+        emit FeeToUpdated(newFeeTo);
+
+        vm.prank(newFeeTo);
+        engine.acceptFeeTo();
+
+        assertEq(engine.feeTo(), newFeeTo);
 
-        assertEq(engine.feeTo(), address(0xCAFE));
+        // Second time around.
+        vm.prank(newFeeTo);
+        engine.setFeeTo(newNewFeeTo);
+
+        vm.expectEmit(true, true, true, true);
+        emit FeeToUpdated(newNewFeeTo);
+
+        vm.prank(newNewFeeTo);
+        engine.acceptFeeTo();
+
+        assertEq(engine.feeTo(), newNewFeeTo);
     }
 
     function test_unitSetFeeToRevertWhenNotCurrentFeeTo() public {
@@ -867,6 +901,19 @@ contract OptionSettlementUnitTest is BaseEngineTest {
         engine.setFeeTo(address(0));
     }
 
+    function test_unitAcceptFeeToRevertWhenNotPendingFeeTo() public {
+        address newFeeTo = address(0xCAFE);
+
+        vm.prank(FEE_TO);
+        engine.setFeeTo(newFeeTo);
+
+        vm.expectRevert(
+            abi.encodeWithSelector(IOptionSettlementEngine.AccessControlViolation.selector, ALICE, newFeeTo)
+        );
+        vm.prank(ALICE);
+        engine.acceptFeeTo();
+    }
+
     /*//////////////////////////////////////////////////////////////
     // function setTokenURIGenerator(address newTokenURIGenerator) external
     //////////////////////////////////////////////////////////////*/