feat: snapShotExecutor oracle deadman switch and transfer

Author: Detoo

scripts/yearnBorg.s.sol

@@ -44,6 +44,7 @@ contract YearnBorgDeployScript is Script {
     uint256 snapShotWaitingPeriod = 3 days; // TODO Is it still necessary?
     uint256 snapShotCancelPeriod = 2 days;
     uint256 snapShotPendingProposalLimit = 3;
+    uint256 snapShotTtl = 30 days;
     address oracle = 0xf00c0dE09574805389743391ada2A0259D6b7a00;
 
     SafeTxHelper safeTxHelper;
@@ -108,7 +109,7 @@ contract YearnBorgDeployScript is Script {
         // Create SnapShotExecutor
 
         executorAuth = new BorgAuth();
-        snapShotExecutor = new SnapShotExecutor(executorAuth, address(oracle), snapShotWaitingPeriod, snapShotCancelPeriod, snapShotPendingProposalLimit);
+        snapShotExecutor = new SnapShotExecutor(executorAuth, address(oracle), snapShotWaitingPeriod, snapShotCancelPeriod, snapShotPendingProposalLimit, snapShotTtl);
 
         // Add modules
 

src/libs/governance/snapShotExecutor.sol

@@ -5,12 +5,15 @@ import "../auth.sol";
 import "openzeppelin/contracts/utils/Address.sol";
 
 contract SnapShotExecutor is BorgAuthACL {
+    uint256 public immutable ORACLE_TTL;
 
     address public oracle;
+    address public pendingOracle;
     uint256 public waitingPeriod;
     uint256 public cancelPeriod;
     uint256 public pendingProposalCount;
     uint256 public pendingProposalLimit;
+    uint256 public lastOraclePingTimestamp;
 
     struct proposal {
         address target;
@@ -25,6 +28,7 @@ contract SnapShotExecutor is BorgAuthACL {
     error SnapShotExecutor_WaitingPeriod();
     error SnapShotExeuctor_InvalidParams();
     error SnapShotExecutor_TooManyPendingProposals();
+    error SnapShotExecutor_OracleNotDead();
 
     //events
     event ProposalCreated(bytes32 indexed proposalId, address indexed target, uint256 value, bytes cdata, string description, uint256 timestamp);
@@ -33,18 +37,36 @@ contract SnapShotExecutor is BorgAuthACL {
 
     mapping(bytes32 => proposal) public pendingProposals;
 
+    /// @dev Check if `msg.sender` is either the oracle or is pending to be one. If it's the latter, transfer it. Also ping for TTL checks.
     modifier onlyOracle() {
-        if (msg.sender != oracle) revert SnapShotExecutor_NotAuthorized();
+        if (msg.sender != oracle) {
+            if (msg.sender == pendingOracle) {
+                // Pending oracle can accept the transfer
+                oracle = pendingOracle;
+                pendingOracle = address(0);
+            } else {
+                // Not authorized if neither oracle nor pending oracle
+                revert SnapShotExecutor_NotAuthorized();
+            }
+        }
+        lastOraclePingTimestamp = block.timestamp;
         _;
     }
 
-    constructor(BorgAuth _auth, address _oracle, uint256 _waitingPeriod, uint256 _cancelPeriod, uint256 _pendingProposals) BorgAuthACL(_auth) {
+    modifier onlyDeadOracle() {
+        if (block.timestamp < lastOraclePingTimestamp + ORACLE_TTL) revert SnapShotExecutor_OracleNotDead();
+        _;
+    }
+
+    constructor(BorgAuth _auth, address _oracle, uint256 _waitingPeriod, uint256 _cancelPeriod, uint256 _pendingProposals, uint256 _oracleTtl) BorgAuthACL(_auth) {
         oracle = _oracle;
         if(_waitingPeriod < 1 minutes) revert SnapShotExeuctor_InvalidParams();
         waitingPeriod = _waitingPeriod;
         if(_cancelPeriod < 1 minutes) revert SnapShotExeuctor_InvalidParams();
         cancelPeriod = _cancelPeriod;
         pendingProposalLimit = _pendingProposals;
+        ORACLE_TTL = _oracleTtl;
+        lastOraclePingTimestamp = block.timestamp;
     }
 
     function propose(address target, uint256 value, bytes calldata cdata, string memory description) external onlyOracle() returns (bytes32) {
@@ -75,4 +97,9 @@ contract SnapShotExecutor is BorgAuthACL {
         emit ProposalCanceled(proposalId, p.target, p.value, p.cdata, p.description, p.timestamp);
     }
 
-}
+    function transferOracle(address newOracle) external onlyOwner() onlyDeadOracle() {
+        pendingOracle = newOracle;
+    }
+
+    function ping() external onlyOracle() {}
+}

test/snapShotExecutor.t.sol

@@ -13,7 +13,8 @@ contract SnapShotExecutorTest is Test {
 
     address owner = vm.addr(1);
     address oracle = vm.addr(2);
-    address alice = vm.addr(3);
+    address newOracle = vm.addr(3);
+    address alice = vm.addr(4);
 
     BorgAuth auth;
     SnapShotExecutor snapShotExecutor;
@@ -29,7 +30,8 @@ contract SnapShotExecutorTest is Test {
             oracle,
             3 days, // waitingPeriod
             2 days, // cancelPeriod
-            3 // pendingProposalLimit
+            3, // pendingProposalLimit
+            30 days // ttl
         );
 
         // Transferring auth ownership
@@ -40,10 +42,13 @@ contract SnapShotExecutorTest is Test {
     /// @dev Metadata should meet specs
     function testMeta() public view {
         assertEq(snapShotExecutor.oracle(), oracle, "Unexpected oracle address");
+        assertEq(snapShotExecutor.pendingOracle(), address(0), "Unexpected pending oracle address");
         assertEq(snapShotExecutor.waitingPeriod(), 3 days, "Unexpected waitingPeriod");
         assertEq(snapShotExecutor.cancelPeriod(), 2 days, "Unexpected cancelPeriod");
         assertEq(snapShotExecutor.pendingProposalCount(), 0, "Unexpected pendingProposalCount");
         assertEq(snapShotExecutor.pendingProposalLimit(), 3, "Unexpected pendingProposalLimit");
+        assertEq(snapShotExecutor.ORACLE_TTL(), 30 days, "Unexpected ORACLE_TTL");
+        assertEq(snapShotExecutor.lastOraclePingTimestamp(), block.timestamp, "Unexpected lastOraclePingTimestamp");
     }
 
     /// @dev BorgAuth instances should be properly assigned and configured
@@ -203,4 +208,63 @@ contract SnapShotExecutorTest is Test {
 
         vm.stopPrank();
     }
+
+    /// @dev Ping timestamp should update when oracle is working
+    function testPing() public {
+        uint256 lastOraclePingTimestamp = snapShotExecutor.lastOraclePingTimestamp();
+
+        // Non-oracle shouldn't be able to ping
+        vm.expectRevert(abi.encodeWithSelector(SnapShotExecutor.SnapShotExecutor_NotAuthorized.selector));
+        snapShotExecutor.ping();
+
+        // Last timestamp should update after a successful ping
+        skip(1 days);
+        vm.prank(oracle);
+        snapShotExecutor.ping();
+        assertEq(snapShotExecutor.lastOraclePingTimestamp(), lastOraclePingTimestamp + 1 days);
+
+        // Propose should also ping
+        skip(1 days);
+        vm.prank(oracle);
+        snapShotExecutor.propose(
+            address(alice), // target
+            0, // value
+            "", // cdata
+            "Arbitrary instruction"
+        );
+        assertEq(snapShotExecutor.lastOraclePingTimestamp(), lastOraclePingTimestamp + 2 days);
+    }
+
+    /// @dev Owner should be able to replace oracle if it's dead
+    function testTransferOracle() public {
+        skip(snapShotExecutor.ORACLE_TTL());
+        vm.prank(owner);
+        snapShotExecutor.transferOracle(newOracle);
+
+        // Old oracle should still work when the transfer is pending
+        vm.prank(oracle);
+        snapShotExecutor.ping();
+        assertEq(snapShotExecutor.oracle(), oracle);
+
+        // Non-oracle should still be unauthorized
+        vm.expectRevert(abi.encodeWithSelector(SnapShotExecutor.SnapShotExecutor_NotAuthorized.selector));
+        snapShotExecutor.ping();
+
+        // Transfer should be done after the new oracle interacts
+        vm.prank(newOracle);
+        snapShotExecutor.ping();
+        assertEq(snapShotExecutor.oracle(), newOracle);
+        assertEq(snapShotExecutor.pendingOracle(), address(0));
+        // Old oracle should no longer be authorized
+        vm.expectRevert(abi.encodeWithSelector(SnapShotExecutor.SnapShotExecutor_NotAuthorized.selector));
+        vm.prank(oracle);
+        snapShotExecutor.ping();
+    }
+
+    /// @dev Owner should not be able to replace oracle if it's not dead
+    function test_RevertIf_SetOracleNotDead() public {
+        vm.expectRevert(abi.encodeWithSelector(SnapShotExecutor.SnapShotExecutor_OracleNotDead.selector));
+        vm.prank(owner);
+        snapShotExecutor.transferOracle(newOracle);
+    }
 }

test/yearnBorgAcceptance.t.sol

@@ -157,6 +157,7 @@ contract YearnBorgAcceptanceTest is Test {
         assertEq(snapShotExecutor.waitingPeriod(), 3 days, "Unexpected waitingPeriod");
         assertEq(snapShotExecutor.cancelPeriod(), 2 days, "Unexpected cancelPeriod");
         assertEq(snapShotExecutor.pendingProposalLimit(), 3, "Unexpected pendingProposalLimit");
+        assertEq(snapShotExecutor.ORACLE_TTL(), 30 days, "Unexpected ORACLE_TTL");
     }
 
     function testEjectImplantMeta() public {