From 6bfa0db7ef608b8b09a6a823f9ad3d93fce22d46 Mon Sep 17 00:00:00 2001
From: Shahul Hameed <10547529+shahthepro@users.noreply.github.com>
Date: Mon, 14 Oct 2024 12:16:23 +0400
Subject: [PATCH] Slither fixes and a bit more tests

---
 contracts/contracts/vault/OETHVaultCore.sol   |  7 +-
 contracts/contracts/vault/VaultStorage.sol    |  3 +
 .../test/vault/oethb-vault.base.fork-test.js  | 75 ++++++++++++++++++-
 3 files changed, 81 insertions(+), 4 deletions(-)

diff --git a/contracts/contracts/vault/OETHVaultCore.sol b/contracts/contracts/vault/OETHVaultCore.sol
index 719f660d1e..f3055d0f9b 100644
--- 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
index 82ed3fd6f7..638f2e8b23 100644
--- 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
index 462a25c26c..18d88bad06 100644
--- 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 () {