From e1d715ed028a2404d3b1d2609e3acaa6394c606a Mon Sep 17 00:00:00 2001
From: Dmitry <dymitr.paremski@thesis.co>
Date: Fri, 3 May 2024 14:30:50 +0200
Subject: [PATCH 1/2] Adding check for max basis points on update

Added additional verification on updating entry and exit basis points so
that it won't exceed max basis points which is 10000 = 100%.
---
 solidity/contracts/lib/ERC4626Fees.sol |  2 +-
 solidity/contracts/stBTC.sol           |  9 +++++++++
 solidity/test/stBTC.test.ts            | 24 ++++++++++++++++++++++--
 3 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/solidity/contracts/lib/ERC4626Fees.sol b/solidity/contracts/lib/ERC4626Fees.sol
index 0bc0ee1c4..5f8c2391c 100644
--- a/solidity/contracts/lib/ERC4626Fees.sol
+++ b/solidity/contracts/lib/ERC4626Fees.sol
@@ -13,7 +13,7 @@ import {Math} from "@openzeppelin/contracts/utils/math/Math.sol";
 abstract contract ERC4626Fees is ERC4626Upgradeable {
     using Math for uint256;
 
-    uint256 private constant _BASIS_POINT_SCALE = 1e4;
+    uint256 internal constant _BASIS_POINT_SCALE = 1e4;
 
     // === Overrides ===
 
diff --git a/solidity/contracts/stBTC.sol b/solidity/contracts/stBTC.sol
index 2ce9cf8d9..25f2c5c8d 100644
--- a/solidity/contracts/stBTC.sol
+++ b/solidity/contracts/stBTC.sol
@@ -74,6 +74,9 @@ contract stBTC is ERC4626Fees, PausableOwnable {
     /// Reverts if the address is disallowed.
     error DisallowedAddress();
 
+    /// Reverts if the fee basis points exceed the maximum value.
+    error ExceedsMaxFeeBasisPoints();
+
     /// @custom:oz-upgrades-unsafe-allow constructor
     constructor() {
         _disableInitializers();
@@ -148,6 +151,9 @@ contract stBTC is ERC4626Fees, PausableOwnable {
     function updateEntryFeeBasisPoints(
         uint256 newEntryFeeBasisPoints
     ) external onlyOwner {
+        if (newEntryFeeBasisPoints > _BASIS_POINT_SCALE) {
+            revert ExceedsMaxFeeBasisPoints();
+        }
         entryFeeBasisPoints = newEntryFeeBasisPoints;
 
         emit EntryFeeBasisPointsUpdated(newEntryFeeBasisPoints);
@@ -158,6 +164,9 @@ contract stBTC is ERC4626Fees, PausableOwnable {
     function updateExitFeeBasisPoints(
         uint256 newExitFeeBasisPoints
     ) external onlyOwner {
+        if (newExitFeeBasisPoints > _BASIS_POINT_SCALE) {
+            revert ExceedsMaxFeeBasisPoints();
+        }
         exitFeeBasisPoints = newExitFeeBasisPoints;
 
         emit ExitFeeBasisPointsUpdated(newExitFeeBasisPoints);
diff --git a/solidity/test/stBTC.test.ts b/solidity/test/stBTC.test.ts
index 860d6b3fe..f778adf7a 100644
--- a/solidity/test/stBTC.test.ts
+++ b/solidity/test/stBTC.test.ts
@@ -1846,7 +1846,7 @@ describe("stBTC", () => {
 
     const validEntryFeeBasisPoints = 100n // 1%
 
-    context("when is called by governance", () => {
+    context("when called by the governance", () => {
       context("when entry fee basis points are valid", () => {
         beforeAfterSnapshotWrapper()
 
@@ -1888,6 +1888,16 @@ describe("stBTC", () => {
           )
         })
       })
+
+      context("when entry fee basis points exceed 10000", () => {
+        beforeAfterSnapshotWrapper()
+
+        it("should revert", async () => {
+          await expect(
+            stbtc.connect(governance).updateEntryFeeBasisPoints(10001n),
+          ).to.be.revertedWithCustomError(stbtc, "ExceedsMaxFeeBasisPoints")
+        })
+      })
     })
 
     context("when is called by non-governance", () => {
@@ -1904,7 +1914,7 @@ describe("stBTC", () => {
 
     const validExitFeeBasisPoints = 100n // 1%
 
-    context("when is called by governance", () => {
+    context("when called by the governance", () => {
       context("when exit fee basis points are valid", () => {
         beforeAfterSnapshotWrapper()
 
@@ -1929,6 +1939,16 @@ describe("stBTC", () => {
         })
       })
 
+      context("when exit fee basis points exceed 10000", () => {
+        beforeAfterSnapshotWrapper()
+
+        it("should revert", async () => {
+          await expect(
+            stbtc.connect(governance).updateExitFeeBasisPoints(10001n),
+          ).to.be.revertedWithCustomError(stbtc, "ExceedsMaxFeeBasisPoints")
+        })
+      })
+
       context("when exit fee basis points are 0", () => {
         beforeAfterSnapshotWrapper()
 

From 4d181042786d332684a52b364cb45ed5b78e9692 Mon Sep 17 00:00:00 2001
From: Dmitry <dymitr.paremski@thesis.co>
Date: Fri, 3 May 2024 14:36:36 +0200
Subject: [PATCH 2/2] Linting function order

---
 solidity/contracts/stBTC.sol | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/solidity/contracts/stBTC.sol b/solidity/contracts/stBTC.sol
index 25f2c5c8d..1ea83bc3b 100644
--- a/solidity/contracts/stBTC.sol
+++ b/solidity/contracts/stBTC.sol
@@ -172,13 +172,6 @@ contract stBTC is ERC4626Fees, PausableOwnable {
         emit ExitFeeBasisPointsUpdated(newExitFeeBasisPoints);
     }
 
-    /// @notice Returns the total amount of assets held by the vault across all
-    ///         allocations and this contract.
-    function totalAssets() public view override returns (uint256) {
-        return
-            IERC20(asset()).balanceOf(address(this)) + dispatcher.totalAssets();
-    }
-
     /// @notice Calls `receiveApproval` function on spender previously approving
     ///         the spender to withdraw from the caller multiple times, up to
     ///         the `amount` amount. If this function is called again, it
@@ -292,6 +285,13 @@ contract stBTC is ERC4626Fees, PausableOwnable {
         return super.redeem(shares, receiver, owner);
     }
 
+    /// @notice Returns the total amount of assets held by the vault across all
+    ///         allocations and this contract.
+    function totalAssets() public view override returns (uint256) {
+        return
+            IERC20(asset()).balanceOf(address(this)) + dispatcher.totalAssets();
+    }
+
     /// @notice Returns value of assets that would be exchanged for the amount of
     ///         shares owned by the `account`.
     /// @param account Owner of shares.