Contract pausing

contracts/Account.sol

@@ -7,13 +7,14 @@ import "./Managed.sol";
 import "./common/UUPSOwnableUpgradeable.sol";
 import "./common/UsingRegistryUpgradeable.sol";
 import "./interfaces/IAccount.sol";
+import "./Pausable.sol";
 
 /**
  * @title A contract that facilitates voting on behalf of StakedCelo.sol.
  * @notice This contract depends on the Manager to decide how to distribute votes and how to
  * keep track of ownership of CELO voted via this contract.
  */
-contract Account is UUPSOwnableUpgradeable, UsingRegistryUpgradeable, Managed, IAccount {
+contract Account is UUPSOwnableUpgradeable, UsingRegistryUpgradeable, Managed, IAccount, Pausable {
     /**
      * @notice Used to keep track of a pending withdrawal. A similar data structure
      * exists within LockedGold.sol, but it only keeps track of pending withdrawals
@@ -64,6 +65,11 @@ contract Account is UUPSOwnableUpgradeable, UsingRegistryUpgradeable, Managed, I
      */
     uint256 public totalScheduledWithdrawals;
 
+    /**
+     * @notice Controls whether the contract is paused.
+     */
+    PausedRecord public paused;
+
     /**
      * @notice Emitted when CELO is scheduled for voting for a given group.
      * @param group The validator group the CELO is intended to vote for.
@@ -225,6 +231,23 @@ contract Account is UUPSOwnableUpgradeable, UsingRegistryUpgradeable, Managed, I
         }
     }
 
+    /**
+     * @notice Pauses external access to the contract.
+     * @dev Functions with the `onlyWhenNotPaused` modifier will be paused. This
+     * should be all the non-permissioned (i.e. not `onlyOwner` or * `onlyManager`)
+     * external/public functions.
+     */
+    function pause() external onlyOwner {
+        _pause(paused);
+    }
+
+    /**
+     * @notice Unpauses the contract if it was previously paused using `pause()`.
+     */
+    function unpause() external onlyOwner {
+        _unpause(paused);
+    }
+
     /**
      * @notice Deposits CELO sent via msg.value as unlocked CELO intended as
      * votes for groups.
@@ -238,6 +261,7 @@ contract Account is UUPSOwnableUpgradeable, UsingRegistryUpgradeable, Managed, I
         external
         payable
         onlyManager
+        onlyWhenNotPaused(paused)
     {
         if (groups.length != votes.length) {
             revert GroupsAndVotesArrayLengthsMismatch();
@@ -268,7 +292,7 @@ contract Account is UUPSOwnableUpgradeable, UsingRegistryUpgradeable, Managed, I
         uint256[] calldata fromVotes,
         address[] calldata toGroups,
         uint256[] calldata toVotes
-    ) external onlyManager {
+    ) external onlyManager onlyWhenNotPaused(paused) {
         if (fromGroups.length != fromVotes.length || toGroups.length != toVotes.length) {
             revert GroupsAndVotesArrayLengthsMismatch();
         }
@@ -301,7 +325,7 @@ contract Account is UUPSOwnableUpgradeable, UsingRegistryUpgradeable, Managed, I
         address beneficiary,
         address[] calldata groups,
         uint256[] calldata withdrawals
-    ) external onlyManager {
+    ) external onlyManager onlyWhenNotPaused(paused) {
         if (groups.length != withdrawals.length) {
             revert GroupsAndVotesArrayLengthsMismatch();
         }
@@ -355,7 +379,7 @@ contract Account is UUPSOwnableUpgradeable, UsingRegistryUpgradeable, Managed, I
         address lesserAfterActiveRevoke,
         address greaterAfterActiveRevoke,
         uint256 index
-    ) external returns (uint256) {
+    ) external onlyWhenNotPaused(paused) returns (uint256) {
         uint256 withdrawalAmount = scheduledVotes[group].toWithdrawFor[beneficiary];
         if (withdrawalAmount == 0) {
             revert NoScheduledWithdrawal(beneficiary, group);
@@ -431,7 +455,7 @@ contract Account is UUPSOwnableUpgradeable, UsingRegistryUpgradeable, Managed, I
         address group,
         address voteLesser,
         address voteGreater
-    ) external {
+    ) external onlyWhenNotPaused(paused) {
         IElection election = getElection();
 
         // The amount of unlocked CELO for group that we want to lock and vote with.
@@ -491,7 +515,7 @@ contract Account is UUPSOwnableUpgradeable, UsingRegistryUpgradeable, Managed, I
         address beneficiary,
         uint256 localPendingWithdrawalIndex,
         uint256 lockedGoldPendingWithdrawalIndex
-    ) external returns (uint256 amount) {
+    ) external onlyWhenNotPaused(paused) returns (uint256 amount) {
         (uint256 value, uint256 timestamp) = validatePendingWithdrawalRequest(
             beneficiary,
             localPendingWithdrawalIndex,
@@ -544,7 +568,7 @@ contract Account is UUPSOwnableUpgradeable, UsingRegistryUpgradeable, Managed, I
         uint256 yesVotes,
         uint256 noVotes,
         uint256 abstainVotes
-    ) external onlyManager {
+    ) external onlyManager onlyWhenNotPaused(paused) {
         bool voteResult = getGovernance().votePartially(
             proposalId,
             index,
@@ -680,6 +704,14 @@ contract Account is UUPSOwnableUpgradeable, UsingRegistryUpgradeable, Managed, I
         return scheduledVotes[group].toWithdrawFor[beneficiary];
     }
 
+    /**
+     * @notice Returns the paused status of the contract.
+     * @return `true` if the contract is paused, `false` otherwise.
+     */
+    function isPaused() external view returns (bool) {
+        return _isPaused(paused);
+    }
+
     /**
      * @notice Returns the storage, major, minor, and patch version of the contract.
      * @return Storage version of the contract.
@@ -697,7 +729,7 @@ contract Account is UUPSOwnableUpgradeable, UsingRegistryUpgradeable, Managed, I
             uint256
         )
     {
-        return (1, 2, 0, 0);
+        return (1, 2, 1, 0);
     }
 
     /**
@@ -728,7 +760,7 @@ contract Account is UUPSOwnableUpgradeable, UsingRegistryUpgradeable, Managed, I
         address lesserAfterActiveRevoke,
         address greaterAfterActiveRevoke,
         uint256 index
-    ) public {
+    ) public onlyWhenNotPaused(paused) {
         (, uint256 revokeAmount) = getAndUpdateToVoteAndToRevoke(group, 0, 0);
 
         if (revokeAmount == 0) {

contracts/Pausable.sol

@@ -0,0 +1,68 @@
+//SPDX-License-Identifier: LGPL-3.0-only
+pragma solidity 0.8.11;
+
+/**
+ * @title A helper contract to add pasuing functionality to a contract.
+ * @notice Used to prevent/mitigate damage in case an exploit is found in the
+ * extending contract.
+ */
+contract Pausable {
+    /**
+     * @notice Struct wrapping a bool that determines whether the contract
+     * should be paused.
+     * @dev We can't store the flag in this contract itself, since that would
+     * mangle storage of inheriting contracts. Instead, we provide a
+     * struct-wrapped bool type, to leverage the typechecker to help ensure the
+     * correct value is used.
+     */
+    struct PausedRecord {
+        bool paused;
+    }
+
+    /**
+     * @notice Used when an `onlyWhenNotPaused` function is called while the
+     * contract is paused.
+     */
+    error Paused();
+
+    /**
+     * @notice Reverts if the contract is paused.
+     * @param paused The `PausedRecord` struct containing the flag.
+     */
+    modifier onlyWhenNotPaused(PausedRecord storage paused) {
+        if (paused.paused) {
+            revert Paused();
+        }
+
+        _;
+    }
+
+    /**
+     * @notice Pauses the contract by setting the wrapped bool to `true`.
+     * @param paused The PausedRecord to modify.
+     * @dev The implementing contract should likely wrap this function in a
+     * permissioned (e.g. `onlyOwner`) `pause()` function.
+     */
+    function _pause(PausedRecord storage paused) internal {
+        paused.paused = true;
+    }
+
+    /**
+     * @notice Unpauses the contract by setting the wrapped bool to `false`.
+     * @param paused The PausedRecord to modify.
+     * @dev The implementing contract should likely wrap this function in a
+     * permissioned (e.g. `onlyOwner`) `unpause()` function.
+     */
+    function _unpause(PausedRecord storage paused) internal {
+        paused.paused = false;
+    }
+
+    /**
+     * @notice Returns whether or not the contract is paused.
+     * @param paused The PauseRecord to check.
+     * @return `true` if the contract is paused, `false` otherwise.
+     */
+    function _isPaused(PausedRecord storage paused) internal view returns (bool) {
+        return paused.paused;
+    }
+}

contracts/test/PausableTest.sol

@@ -0,0 +1,29 @@
+//SPDX-License-Identifier: LGPL-3.0-only
+pragma solidity 0.8.11;
+
+import "../Pausable.sol";
+
+contract PausableTest is Pausable {
+    PausedRecord paused;
+    uint256 public numberCalls;
+
+    function callPausable() external onlyWhenNotPaused(paused) {
+        numberCalls++;
+    }
+
+    function callAlways() external {
+        numberCalls++;
+    }
+
+    function pause() external {
+        _pause(paused);
+    }
+
+    function unpause() external {
+        _unpause(paused);
+    }
+
+    function isPaused() external view returns (bool) {
+        return _isPaused(paused);
+    }
+}

deploy/test/pausable.ts

@@ -0,0 +1,15 @@
+import { DeployFunction } from "@celo/staked-celo-hardhat-deploy/types";
+import { HardhatRuntimeEnvironment } from "hardhat/types";
+
+const func: DeployFunction = async function (hre: HardhatRuntimeEnvironment) {
+  const { deploy } = hre.deployments;
+  const { deployer, owner } = await hre.getNamedAccounts();
+  await deploy("PausableTest", {
+    from: deployer,
+    log: true,
+  });
+};
+
+func.id = "deploy_test_pausable";
+func.tags = ["TestPausable"];
+export default func;

test-ts/account.test.ts

@@ -37,6 +37,7 @@ describe("Account", () => {
 
   let account: Account;
 
+  let owner: SignerWithAddress;
   let manager: SignerWithAddress;
   let nonManager: SignerWithAddress;
   let beneficiary: SignerWithAddress;
@@ -84,7 +85,7 @@ describe("Account", () => {
 
   beforeEach(async () => {
     await hre.deployments.fixture("TestAccount");
-    const owner = await hre.ethers.getNamedSigner("owner");
+    owner = await hre.ethers.getNamedSigner("owner");
     account = await hre.ethers.getContract("Account");
     await account.connect(owner).setManager(manager.address);
     governance = await hre.ethers.getContract("MockGovernance");
@@ -2015,4 +2016,113 @@ describe("Account", () => {
       expect(await governance.abstainVotes()).to.eq(abstain);
     });
   });
+
+  describe("#pause", () => {
+    it("can be called by the owner", async () => {
+      await account.connect(owner).pause();
+      const isPaused = await account.isPaused();
+      expect(isPaused).to.be.true;
+    });
+
+    it("cannot be called by a random account", async () => {
+      await expect(account.connect(beneficiary).pause()).revertedWith(
+        "Ownable: caller is not the owner"
+      );
+      const isPaused = await account.isPaused();
+      expect(isPaused).to.be.false;
+    });
+  });
+
+  describe("#unpause", () => {
+    beforeEach(async () => {
+      await account.connect(owner).pause();
+    });
+
+    it("can be called by the owner", async () => {
+      await account.connect(owner).unpause();
+      const isPaused = await account.isPaused();
+      expect(isPaused).to.be.false;
+    });
+
+    it("cannot be called by a random account", async () => {
+      await expect(account.connect(beneficiary).unpause()).revertedWith(
+        "Ownable: caller is not the owner"
+      );
+      const isPaused = await account.isPaused();
+      expect(isPaused).to.be.true;
+    });
+  });
+
+  describe("when paused", () => {
+    beforeEach(async () => {
+      await account.connect(owner).pause();
+    });
+
+    it("can't call scheduleVotes", async () => {
+      await expect(
+        account.connect(manager).scheduleVotes([groupAddresses[0]], [100], { value: "100" })
+      ).revertedWith("Paused()");
+    });
+
+    it("can't call scheduleTransfer", async () => {
+      await expect(
+        account
+          .connect(manager)
+          .scheduleTransfer([groupAddresses[0]], [1], [groupAddresses[1]], [1])
+      ).revertedWith("Paused()");
+    });
+
+    it("can't call scheduleWithdrawals", async () => {
+      await expect(
+        account
+          .connect(manager)
+          .scheduleWithdrawals(beneficiary.address, [groupAddresses[0]], [260])
+      ).revertedWith("Paused()");
+    });
+
+    it("can't call withdraw", async () => {
+      await expect(
+        account
+          .connect(manager)
+          .withdraw(
+            beneficiary.address,
+            groupAddresses[0],
+            ADDRESS_ZERO,
+            ADDRESS_ZERO,
+            ADDRESS_ZERO,
+            ADDRESS_ZERO,
+            0
+          )
+      ).revertedWith("Paused()");
+    });
+
+    it("can't call activateAndVote", async () => {
+      await expect(
+        account.activateAndVote(groupAddresses[0], groupAddresses[1], ADDRESS_ZERO)
+      ).revertedWith("Paused()");
+    });
+
+    it("can't call finishPendingWithdrawal", async () => {
+      await expect(account.finishPendingWithdrawal(beneficiary.address, 0, 0)).revertedWith(
+        "Paused()"
+      );
+    });
+
+    it("can't call votePartially", async () => {
+      await expect(account.connect(manager).votePartially(0, 0, 1, 1, 1)).revertedWith("Paused()");
+    });
+
+    it("can't call revokeVotes", async () => {
+      await expect(
+        account.revokeVotes(
+          groupAddresses[0],
+          ADDRESS_ZERO,
+          ADDRESS_ZERO,
+          ADDRESS_ZERO,
+          ADDRESS_ZERO,
+          0
+        )
+      ).revertedWith("Paused()");
+    });
+  });
 });