Slither fixes and a bit more tests

OriginProtocol · Oct 14, 2024 · 6bfa0db · 6bfa0db
1 parent 7a63821
commit 6bfa0db
Show file tree

Hide file tree

Showing 3 changed files with 81 additions and 4 deletions.
diff --git a/contracts/contracts/vault/OETHVaultCore.sol b/contracts/contracts/vault/OETHVaultCore.sol
@@ -288,15 +288,16 @@ contract OETHVaultCore is VaultCore {
         internal
         returns (uint256 amount)
     {
-        uint256 _claimDelay = withdrawalClaimDelay;
-        require(_claimDelay > 0, "Async withdrawals not enabled");
+        // Note: When `withdrawalClaimDelay` is set to 0, users can
+        // no longer place withdrawal requests. However, any existing
+        // unclaimed request would become immediately claimable.
 
         // Load the structs from storage into memory
         WithdrawalRequest memory request = withdrawalRequests[requestId];
         WithdrawalQueueMetadata memory queue = withdrawalQueueMetadata;
 
         require(
-            request.timestamp + _claimDelay <= block.timestamp,
+            request.timestamp + withdrawalClaimDelay <= block.timestamp,
             "Claim delay not met"
         );
         // If there isn't enough reserved liquidity in the queue to claim

diff --git a/contracts/contracts/vault/VaultStorage.sol b/contracts/contracts/vault/VaultStorage.sol
@@ -238,7 +238,10 @@ contract VaultStorage is Initializable, Governable {
     mapping(uint256 => WithdrawalRequest) public withdrawalRequests;
 
     /// @notice Used for OETH & superOETHb async withdrawal
+    // slither-disable-start constable-states
+    // slither-disable-next-line uninitialized-state
     uint256 public withdrawalClaimDelay;
+    // slither-disable-end constable-states
 
     // For future use
     uint256[44] private __gap;

diff --git a/contracts/test/vault/oethb-vault.base.fork-test.js b/contracts/test/vault/oethb-vault.base.fork-test.js
@@ -113,7 +113,7 @@ describe("ForkTest: OETHb Vault", function () {
   });
 
   describe("Async withdrawals", function () {
-    it("Should allow 1:1 async withdrawals", async function () {
+    it("Should allow 1:1 async withdrawals", async () => {
       const { rafael, oethbVault } = fixture;
 
       const delayPeriod = await oethbVault.withdrawalClaimDelay();
@@ -136,6 +136,79 @@ describe("ForkTest: OETHb Vault", function () {
       await advanceTime(delayPeriod);
       await oethbVault.connect(rafael).claimWithdrawal(requestId);
     });
+
+    it("Should not allow withdraw before claim delay", async () => {
+      const { rafael, oethbVault } = fixture;
+
+      const delayPeriod = await oethbVault.withdrawalClaimDelay();
+
+      if (delayPeriod == 0) {
+        // Skip when disabled
+        return;
+      }
+
+      const { nextWithdrawalIndex: requestId } =
+        await oethbVault.withdrawalQueueMetadata();
+
+      // Rafael mints 1 superOETHb
+      await _mint(rafael);
+
+      // Rafael places an async withdrawal request
+      await oethbVault.connect(rafael).requestWithdrawal(oethUnits("1"));
+
+      // ... and tries to claim before the withdraw period
+      const tx = oethbVault.connect(rafael).claimWithdrawal(requestId);
+      await expect(tx).to.be.revertedWith("Claim delay not met");
+    });
+
+    it("Should enforce claim delay limits", async () => {
+      const { governor, oethbVault } = fixture;
+
+      // lower bound
+      await oethbVault.connect(governor).setWithdrawalClaimDelay(
+        10 * 60 // 10 mins
+      );
+      expect(await oethbVault.withdrawalClaimDelay()).to.eq(10 * 60);
+
+      // upper bound
+      await oethbVault.connect(governor).setWithdrawalClaimDelay(
+        7 * 24 * 60 * 60 // 7d
+      );
+      expect(await oethbVault.withdrawalClaimDelay()).to.eq(7 * 24 * 60 * 60);
+
+      // below lower bound
+      let tx = oethbVault.connect(governor).setWithdrawalClaimDelay(
+        9 * 60 + 59 // 9 mins 59 sec
+      );
+      await expect(tx).to.be.revertedWith("Invalid claim delay period");
+
+      // above upper bound
+      tx = oethbVault.connect(governor).setWithdrawalClaimDelay(
+        7 * 24 * 60 * 60 + 1 // 7d + 1s
+      );
+      await expect(tx).to.be.revertedWith("Invalid claim delay period");
+    });
+
+    it("Should allow governor to disable withdrawals", async () => {
+      const { governor, oethbVault, rafael } = fixture;
+
+      // Disable it
+      await oethbVault.connect(governor).setWithdrawalClaimDelay(0);
+      expect(await oethbVault.withdrawalClaimDelay()).to.eq(0);
+
+      // No one can make requests
+      const tx = oethbVault.connect(rafael).requestWithdrawal(oethUnits("1"));
+      await expect(tx).to.be.revertedWith("Async withdrawals not enabled");
+    });
+
+    it("Should not allow anyone else to disable withdrawals", async () => {
+      const { oethbVault, rafael, strategist } = fixture;
+
+      for (const signer of [rafael, strategist]) {
+        const tx = oethbVault.connect(signer).setWithdrawalClaimDelay(0);
+        await expect(tx).to.be.revertedWith("Caller is not the Governor");
+      }
+    });
   });
 
   describe("Mint Whitelist", function () {