LiquidStone Product Audit Remediation (#154)

LiquidStone audit remediation fixes --------- Co-authored-by: Ian Lucas <[email protected]> Co-authored-by: Krishna kumar S <[email protected]> Co-authored-by: Jonathan Lodge <[email protected]> Co-authored-by: Nawar Hisso <[email protected]> Co-authored-by: Chai Somsri <[email protected]>
credbull · Oct 29, 2024 · d860230 · d860230
1 parent a3316f3
commit d860230
Show file tree

Hide file tree

Showing 48 changed files with 7,495 additions and 5,444 deletions.
diff --git a/.github/workflows/ci-dev-ops.yml b/.github/workflows/ci-dev-ops.yml
@@ -39,7 +39,11 @@ jobs:
       - name: Install foundry-toolchain
         uses: foundry-rs/[email protected]
         with:
-          version: nightly
+          # using 2024-10-19 Nightly https://github.com/foundry-rs/foundry/tree/nightly-a8c3e9c1376122e7030dbe5c695b2f1f2a6f389b
+          # latest gives "deserialization error: duplicate field `status` at line 1 column 1085"
+          # see failure: https://github.com/credbull/credbull-defi/actions/runs/11576938626/job/32227114227
+          version: nightly-a8c3e9c1376122e7030dbe5c695b2f1f2a6f389b
+
 
       - name: Install Project Dependencies
         run: yarn install

diff --git a/.github/workflows/ci-dev-sdk.yml b/.github/workflows/ci-dev-sdk.yml
@@ -39,7 +39,11 @@ jobs:
       - name: Install foundry-toolchain
         uses: foundry-rs/[email protected]
         with:
-          version: nightly
+          # using 2024-10-19 Nightly https://github.com/foundry-rs/foundry/tree/nightly-a8c3e9c1376122e7030dbe5c695b2f1f2a6f389b
+          # latest gives "deserialization error: duplicate field `status` at line 1 column 1085"
+          # see failure: https://github.com/credbull/credbull-defi/actions/runs/11576938626/job/32227114227
+          version: nightly-a8c3e9c1376122e7030dbe5c695b2f1f2a6f389b
+
 
       - name: Install Project Dependencies
         run: yarn install

diff --git a/packages/contracts/package.json b/packages/contracts/package.json
@@ -10,7 +10,7 @@
     "dev": "yarn rm-dbdata && anvil --config-out localhost.json & make deploy-local",
     "build": "forge build && yarn gen-types",
     "test": "forge test",
-    "coverage": "forge coverage --report lcov && genhtml lcov.info -o out/test-reports/coverage --ignore-errors inconsistent",
+    "coverage": "forge coverage --report lcov && genhtml lcov.info --branch-coverage -o out/test-reports/coverage --ignore-errors inconsistent",
     "format": "forge fmt && prettier './script/**/*.js' --write",
     "lint": "forge fmt && eslint --fix --ignore-path .gitignore && yarn solhint './*(test|src)/**/*.sol'",
     "db-check": "tsc && node ./script/utils/checkDb.js",

diff --git a/packages/contracts/resource/local.toml b/packages/contracts/resource/local.toml
@@ -24,6 +24,8 @@ asset_manager = "0x976EA74026E726554dB657fA54763abd0C3a0aa9"
 full_rate_bps = 10_00
 # rate in basis points, e.g.  5.5% = 550 bps
 reduced_rate_bps = 5_50
+# January 1, 2024 2:00:00 PM UTC = 1704117600
+vault_start_timestamp = 1704117600
 
 [evm.contracts.upside_vault]
 # 2 decimal place percentage (meaining value divided by 100) as integer.

diff --git a/packages/contracts/resource/mainnet.toml b/packages/contracts/resource/mainnet.toml
@@ -0,0 +1,37 @@
+##
+# The Application Configuration for the TestNet Environment.
+##
+
+[evm]
+# blockchain id, e.g. arbitrumOne=42161
+chain_id = 42161
+deploy_mocks = false
+
+[evm.address]
+# PrimeVault wallets available ONLY on specific EVM chains, e.g.:  Ethereum, Arbitrum, Optimism, Polygon
+# primevault Wallet [0xFa..0519] "Credbull Eng - Owner Role v1.0"
+owner = "0xFa0C9EB3fd284a87c82b9809162DefEa36070519"
+# primevault Wallet [0x5F..bBf1] "Credbull Eng - Operator Role v1.0"
+operator = "0x5FAbE5420116E061D0711D849DAA4788F0d4bBf1"
+# primevault Wallet [0x1E..81b1] "Credbull Eng - Upgrader Role v1.0"
+upgrader = "0x1E2D099F4681394B0c130e7cCcc3F5275eCa81b1"
+# primevault Wallet [0x1D..7BFB] "Credbull Eng - Asset Manager Role v1.0"
+asset_manager = "0x1DA51e4Ab5D029034Db2caB258EC4e22Be647BFB"
+# primevault Wallet [0xce..3FB2] "Credbull DeFi Vault - Treasury v1.0"
+custodian = "0xce694E94e1Ddb734f2bD32B2511D193fF2783FB2"
+
+[evm.contracts.liquid_continuous_multi_token_vault]
+# rate in basis points, e.g.  10% = 1000 bps
+full_rate_bps = 10_00
+# rate in basis points, e.g.  5.5% = 550 bps
+reduced_rate_bps = 5_50
+
+[evm.contracts.upside_vault]
+# upside rate in basis points, e.g. 2% = 200 bps
+collateral_percentage = 200
+
+[services.supabase]
+url = ""
+
+# Save the contract deployment details to the database.
+update_contract_addresses = false
diff --git a/packages/contracts/script/DeployLiquidMultiTokenVault.s.sol b/packages/contracts/script/DeployLiquidMultiTokenVault.s.sol
@@ -28,9 +28,10 @@ contract DeployLiquidMultiTokenVault is TomlConfig {
     using stdToml for string;
 
     string private _tomlConfig;
-    LiquidContinuousMultiTokenVault.VaultAuth public _vaultAuth;
+    LiquidContinuousMultiTokenVault.VaultAuth internal _vaultAuth;
 
     uint256 public constant NOTICE_PERIOD = 1;
+    string public constant CONTRACT_TOML_KEY = ".evm.contracts.liquid_continuous_multi_token_vault";
 
     constructor() {
         _tomlConfig = loadTomlConfiguration();
@@ -103,11 +104,9 @@ contract DeployLiquidMultiTokenVault is TomlConfig {
         IYieldStrategy yieldStrategy,
         IRedeemOptimizer redeemOptimizer
     ) public view returns (LiquidContinuousMultiTokenVault.VaultParams memory vaultParams_) {
-        string memory contractKey = ".evm.contracts.liquid_continuous_multi_token_vault";
-        uint256 fullRateBasisPoints = _tomlConfig.readUint(string.concat(contractKey, ".full_rate_bps"));
-        uint256 reducedRateBasisPoints = _tomlConfig.readUint(string.concat(contractKey, ".reduced_rate_bps"));
-        uint256 startTimestamp =
-            _readUintWithDefault(_tomlConfig, string.concat(contractKey, ".vault_start_timestamp"), block.timestamp);
+        uint256 fullRateBasisPoints = _tomlConfig.readUint(string.concat(CONTRACT_TOML_KEY, ".full_rate_bps"));
+        uint256 reducedRateBasisPoints = _tomlConfig.readUint(string.concat(CONTRACT_TOML_KEY, ".reduced_rate_bps"));
+        uint256 startTimestamp = _startTimestamp();
 
         uint256 scale = 10 ** asset.decimals();
 
@@ -135,6 +134,12 @@ contract DeployLiquidMultiTokenVault is TomlConfig {
         return vaultParams;
     }
 
+    function _startTimestamp() internal view virtual returns (uint256 startTimestamp_) {
+        return _readUintWithDefault(
+            _tomlConfig, string.concat(CONTRACT_TOML_KEY, ".vault_start_timestamp"), block.timestamp
+        );
+    }
+
     function _usdcOrDeployMock(address contractOwner) internal returns (IERC20Metadata asset) {
         bool shouldDeployMocks = _readBoolWithDefault(_tomlConfig, ".evm.deploy_mocks", false);
 

diff --git a/packages/contracts/src/timelock/ITimelockAsyncUnlock.sol b/packages/contracts/src/timelock/ITimelockAsyncUnlock.sol
@@ -31,6 +31,13 @@ interface ITimelockAsyncUnlock {
         external
         returns (uint256[] memory depositPeriods, uint256[] memory amounts);
 
+    /**
+     * @notice Cancel a pending request to unlock
+     * @param owner Owner of the request
+     * @param requestId Discriminator between non-fungible requests
+     */
+    function cancelRequestUnlock(address owner, uint256 requestId) external;
+
     /**
      * @dev Return notice period
      */

diff --git a/packages/contracts/src/timelock/TimelockAsyncUnlock.sol b/packages/contracts/src/timelock/TimelockAsyncUnlock.sol
@@ -22,6 +22,8 @@ abstract contract TimelockAsyncUnlock is Initializable, ITimelockAsyncUnlock, Co
     // cache of user requested unlocks by depositPeriod across ALL requests.  maps account => map(depositPeriod -> unlockAmount)
     mapping(address account => EnumerableMap.UintToUintMap) private _depositPeriodAmountCache;
 
+    event CancelRedeemRequest(address indexed owner, uint256 indexed requestId, address indexed sender);
+
     error TimelockAsyncUnlock__AuthorizeCallerFailed(address caller, address owner);
     error TimelockAsyncUnlock__InvalidArrayLength(uint256 depositPeriodsLength, uint256 amountsLength);
     error TimelockAsyncUnlock__ExceededMaxRequestUnlock(
@@ -33,7 +35,7 @@ abstract contract TimelockAsyncUnlock is Initializable, ITimelockAsyncUnlock, Co
     error TimelockAsyncUnlock__UnlockBeforeDepositPeriod(
         address caller, address owner, uint256 depositPeriod, uint256 unlockPeriod
     );
-    error TimelockAsyncUnlock__UnlockBeforeUnlockPeriod(
+    error TimelockAsyncUnlock__UnlockBeforeCurrentPeriod(
         address caller, address owner, uint256 currentPeriod, uint256 unlockPeriod
     );
 
@@ -181,6 +183,10 @@ abstract contract TimelockAsyncUnlock is Initializable, ITimelockAsyncUnlock, Co
     {
         _authorizeCaller(_msgSender(), owner);
 
+        if (requestId > currentPeriod()) {
+            revert TimelockAsyncUnlock__UnlockBeforeCurrentPeriod(_msgSender(), owner, currentPeriod(), requestId);
+        }
+
         // use copy of the depositPeriods and amounts.  we will be altering the storage in _unlock()
         (depositPeriods, amounts) = unlockRequests(owner, requestId);
 
@@ -200,7 +206,9 @@ abstract contract TimelockAsyncUnlock is Initializable, ITimelockAsyncUnlock, Co
         internal
         virtual
     {
-        _handleUnlockValidation(owner, depositPeriod, requestId);
+        if (requestId < depositPeriod) {
+            revert TimelockAsyncUnlock__UnlockBeforeDepositPeriod(_msgSender(), owner, depositPeriod, requestId);
+        }
 
         EnumerableMap.UintToUintMap storage unlockRequestsForRequestId = _unlockRequests[owner][requestId];
 
@@ -269,19 +277,6 @@ abstract contract TimelockAsyncUnlock is Initializable, ITimelockAsyncUnlock, Co
         }
     }
 
-    /**
-     * @dev An internal function to check if unlock can be performed
-     */
-    function _handleUnlockValidation(address owner, uint256 depositPeriod, uint256 unlockPeriod) internal virtual {
-        if (unlockPeriod > currentPeriod()) {
-            revert TimelockAsyncUnlock__UnlockBeforeUnlockPeriod(_msgSender(), owner, currentPeriod(), unlockPeriod);
-        }
-
-        if (unlockPeriod < depositPeriod) {
-            revert TimelockAsyncUnlock__UnlockBeforeDepositPeriod(_msgSender(), owner, depositPeriod, unlockPeriod);
-        }
-    }
-
     function supportsInterface(bytes4 interfaceId) public view virtual returns (bool) {
         return interfaceId == type(ITimelockAsyncUnlock).interfaceId;
     }

diff --git a/packages/contracts/src/yield/CalcInterestMetadata.sol b/packages/contracts/src/yield/CalcInterestMetadata.sol
@@ -40,4 +40,11 @@ abstract contract CalcInterestMetadata is Initializable, ICalcInterestMetadata {
     function scale() public view virtual returns (uint256 scale_) {
         return SCALE;
     }
+
+    /**
+     * @dev This empty reserved space is put in place to allow future versions to add new
+     * variables without shifting down storage in the inheritance chain.
+     * See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps
+     */
+    uint256[49] private __gap;
 }