chore: audit fixes

contracts/DepositVault.sol

@@ -210,7 +210,6 @@ contract DepositVault is ManageableVault, IDepositVault {
      */
     function safeBulkApproveRequestAtSavedRate(uint256[] calldata requestIds)
         external
-        onlyContractAdmin
     {
         _safeBulkApproveRequest(requestIds, 0, true, false);
     }
@@ -341,16 +340,7 @@ contract DepositVault is ManageableVault, IDepositVault {
             if (isRequestRate) {
                 newOutRate = mintRequests[requestIds[i]].tokenOutRate;
             }
-            bool success = _approveRequest(
-                requestIds[i],
-                newOutRate,
-                true,
-                isAvgRate
-            );
-
-            if (!success) {
-                continue;
-            }
+            _approveRequest(requestIds[i], newOutRate, true, isAvgRate);
         }
     }
 
@@ -427,7 +417,7 @@ contract DepositVault is ManageableVault, IDepositVault {
 
         _instantTransferTokensToTokensReceiver(
             tokenIn,
-            result.amountTokenWithoutFee + result.feeTokenAmount,
+            amountToken,
             result.tokenDecimals
         );
 
@@ -590,21 +580,13 @@ contract DepositVault is ManageableVault, IDepositVault {
      * - safely validates max supply cap
      * - requires variation tolerance
      * @param isAvgRate if true, newOutRate is avg rate
-     *
-     * @return success true if success, otherwise false if isSafe flag is true,
-     * or revert if isSafe flag is false
      */
     function _approveRequest(
         uint256 requestId,
         uint256 newOutRate,
         bool isSafe,
         bool isAvgRate
-    )
-        private
-        returns (
-            bool /* success */
-        )
-    {
+    ) private {
         Request memory request = mintRequests[requestId];
 
         _validateRequest(requestId, request.recipient, request.status);
@@ -620,6 +602,10 @@ contract DepositVault is ManageableVault, IDepositVault {
                 newOutRate
             );
 
+            if (request.depositedInstantUsdAmount > 0) {
+                require(avgRate > 0, InvalidAvgRate());
+            }
+
             if (avgRate != 0) {
                 newOutRate = avgRate;
             }
@@ -641,7 +627,7 @@ contract DepositVault is ManageableVault, IDepositVault {
                 !isSafe
             ) || !_validateAndUpdateNextRequestIdToProcess(requestId, !isSafe)
         ) {
-            return false;
+            return;
         }
 
         upcomingSupply -= upcomingSupplyDecrease;
@@ -657,8 +643,6 @@ contract DepositVault is ManageableVault, IDepositVault {
         mintRequests[requestId] = request;
 
         emit ApproveRequest(requestId, newOutRate, isSafe, isAvgRate);
-
-        return true;
     }
 
     /**

contracts/RedemptionVault.sol

@@ -209,7 +209,6 @@ contract RedemptionVault is ManageableVault, IRedemptionVault {
      */
     function safeBulkApproveRequestAtSavedRate(uint256[] calldata requestIds)
         external
-        onlyContractAdmin
     {
         _safeBulkApproveRequest(requestIds, 0, true, false);
     }
@@ -415,16 +414,7 @@ contract RedemptionVault is ManageableVault, IRedemptionVault {
                 newOutRate = redeemRequests[requestIds[i]].mTokenRate;
             }
 
-            bool success = _approveRequest(
-                requestIds[i],
-                newOutRate,
-                true,
-                isAvgRate
-            );
-
-            if (!success) {
-                continue;
-            }
+            _approveRequest(requestIds[i], newOutRate, true, isAvgRate);
         }
     }
 
@@ -441,21 +431,13 @@ contract RedemptionVault is ManageableVault, IRedemptionVault {
      * - safely validates if there is enough liquidity
      * - requires variation tolerance
      * @param isAvgRate if true, calculates holdback part rate from avg rate
-     *
-     * @return success true if success, otherwise false if isSafe flag is true,
-     * or revert if isSafe flag is false
      */
     function _approveRequest(
         uint256 requestId,
         uint256 newMTokenRate,
         bool isSafe,
         bool isAvgRate
-    )
-        private
-        returns (
-            bool /* success */
-        )
-    {
+    ) private {
         Request memory request = redeemRequests[requestId];
 
         _validateRequest(requestId, request.recipient, request.status);
@@ -472,6 +454,10 @@ contract RedemptionVault is ManageableVault, IRedemptionVault {
                 newMTokenRate
             );
 
+            if (request.amountMTokenInstant > 0) {
+                require(avgRate > 0, InvalidAvgRate());
+            }
+
             if (avgRate != 0) {
                 newMTokenRate = avgRate;
             }
@@ -497,7 +483,7 @@ contract RedemptionVault is ManageableVault, IRedemptionVault {
                     .convertFromBase18(calcResult.tokenOutDecimals)) ||
             !_validateAndUpdateNextRequestIdToProcess(requestId, !isSafe)
         ) {
-            return false;
+            return;
         }
 
         _tokenTransferFromTo(
@@ -519,8 +505,6 @@ contract RedemptionVault is ManageableVault, IRedemptionVault {
         redeemRequests[requestId] = request;
 
         emit ApproveRequest(requestId, newMTokenRate, isSafe, isAvgRate);
-
-        return true;
     }
 
     /**

contracts/RedemptionVaultWithMToken.sol

@@ -16,7 +16,6 @@ import {RedemptionSwapperHelpersLibrary} from "./libraries/RedemptionSwapperHelp
 /**
  * @title RedemptionVaultWithMToken
  * @notice Smart contract that handles redemptions using mToken RedemptionVault withdrawals
- * @dev Storage layout is preserved for safe upgrades from RedemptionVaultWithSwapper
  * @author RedDuck Software
  */
 contract RedemptionVaultWithMToken is RedemptionVault {

contracts/RedemptionVaultWithUSTB.sol

@@ -20,16 +20,6 @@ contract RedemptionVaultWithUSTB is RedemptionVault {
     using DecimalsCorrectionLibrary for uint256;
     using SafeERC20 for IERC20;
 
-    /**
-     * @custom:oz-upgrades-renamed-from minBuidlToRedeem
-     */
-    uint256 private __deprecatedStorageSlot1;
-
-    /**
-     * @custom:oz-upgrades-renamed-from minBuidlBalance
-     */
-    uint256 private __deprecatedStorageSlot2;
-
     /**
      * @notice USTB redemption contract address
      * @dev Used to handle USTB redemptions when vault has insufficient USDC

contracts/abstract/ManageableVault.sol

@@ -30,7 +30,7 @@
  * @author RedDuck Software
  * @notice Contract with base Vault methods
  */
 abstract contract ManageableVault is
     IManageableVault,
     Blacklistable,
     Greenlistable,
@@ -197,8 +197,8 @@
             _commonVaultInitParams.sanctionsList
         );
 
-        _validateAddress(_commonVaultInitParams.mToken, false);
-        _validateAddress(_commonVaultInitParams.mTokenDataFeed, false);
+        _validateAddress(_commonVaultInitParams.mToken, true);
+        _validateAddress(_commonVaultInitParams.mTokenDataFeed, true);
         _validateAddress(_commonVaultInitParams.tokensReceiver, true);
         _validateFee(_commonVaultInitParams.variationTolerance, true);
         _validateFee(_commonVaultInitParams.instantFee, false);

contracts/access/MidasTimelockManager.sol

@@ -37,6 +37,7 @@
         bool isSetCouncilOperation;
         uint32 createdAt;
         uint32 executionApprovedAt;
+        uint32 pausedAt;
         address operationProposer;
         address pauser;
     }
@@ -76,7 +77,7 @@
     /**
      * @notice hard cap for max pending operations per proposer
      */
-    uint256 public constant MAX_PENDING_OPERATIONS_PER_PROPOSER = 100;
+    uint256 public constant MAX_OF_MAX_PENDING_OPERATIONS_PER_PROPOSER = 100;
 
     /**
      * @inheritdoc IMidasTimelockManager
@@ -268,17 +269,19 @@
             TimelockOperationNotReady()
         );
 
-        _timelock.execute(target, 0, data, bytes32(0), bytes32(dataHashIndex));
-
         _resetPendingSetCouncilOperation(opDetails);
 
-        // updating state after execution to be able to verify tx against current context
-        // in case of reentrancy timelock.execute will revert
         opDetails.status = TimelockOperationStatus.Executed;
-        dataHashIndexes[dataHash] = dataHashIndex + 1;
         --proposerPendingOperationsCount[opDetails.operationProposer];
         require(_pendingOperations.remove(operationId), OperationNotPending());
 
+        _timelock.execute(target, 0, data, bytes32(0), bytes32(dataHashIndex));
+
+        // updating hash after execution to be able to get current proposer
+        // from the executing operation
+        // in case of reentrancy timelock.execute will revert
+        dataHashIndexes[dataHash] = dataHashIndex + 1;
+
         emit ExecuteTimelockOperation(msg.sender, operationId);
     }
 
@@ -306,6 +309,7 @@
         opDetails.pauseReasonCode = pauseReasonCode;
         opDetails.councilVersion = councilVersion;
         opDetails.pauser = msg.sender;
+        opDetails.pausedAt = uint32(block.timestamp);
 
         emit PauseTimelockOperation(
             msg.sender,
@@ -477,6 +481,7 @@
         result.votesForExecution = uint8(opDetails.votersForExecution.length());
         result.votesForVeto = uint8(opDetails.votersForVeto.length());
         result.isSetCouncilOperation = opDetails.isSetCouncilOperation;
+        result.pausedAt = opDetails.pausedAt;
     }
 
     /**
@@ -745,7 +750,7 @@
         require(
             _maxPendingOperationsPerProposer > 0 &&
                 _maxPendingOperationsPerProposer <=
-                MAX_PENDING_OPERATIONS_PER_PROPOSER,
+                MAX_OF_MAX_PENDING_OPERATIONS_PER_PROPOSER,
             InvalidMaxPendingOperationsPerProposer()
         );
         maxPendingOperationsPerProposer = _maxPendingOperationsPerProposer;
@@ -868,7 +873,7 @@
         bytes4 selector;
 
         // getting the selector of custom error
         assembly {
             selector := mload(add(err, 32))
         }
 
@@ -878,7 +883,7 @@
             InvalidPreflightError(err)
         );
 
         assembly {
             role := mload(add(err, 36))
             overrideDelay := mload(add(err, 68))
             roleIsFunctionOperator := mload(add(err, 100))

contracts/interfaces/IManageableVault.sol

@@ -255,6 +255,11 @@ interface IManageableVault {
      */
     error InvalidNewMTokenRate();
 
+    /**
+     * @notice Avg rate must be greater than zero
+     */
+    error InvalidAvgRate();
+
     /**
      * @notice Request does not exist
      * @param requestId request id

contracts/interfaces/IMidasTimelockManager.sol

@@ -35,6 +35,8 @@ struct GetOperationStatusResult {
     address operationProposer;
     /// @notice address that paused the operation
     address pauser;
+    /// @notice block timestamp when operation was paused
+    uint32 pausedAt;
     /// @notice hash of target, value and data
     bytes32 dataHash;
     /// @notice number of council votes for execution

contracts/libraries/RateLimitLibrary.sol

@@ -206,12 +206,7 @@ library RateLimitLibrary {
     ) private view returns (uint256 inFlight, uint256 remaining) {
         uint256 elapsed = block.timestamp - lastUpdated;
 
-        uint256 decay = Math.mulDiv(
-            limit,
-            elapsed,
-            window > 0 ? window : 1,
-            Math.Rounding.Down
-        );
+        uint256 decay = Math.mulDiv(limit, elapsed, window, Math.Rounding.Down);
 
         inFlight = amountInFlight <= decay ? 0 : amountInFlight - decay;
 

contracts/mToken.sol

@@ -114,7 +114,7 @@ contract mToken is ERC20PausableUpgradeable, Blacklistable, IMToken {
         string memory symbol_
     ) external {
         _initializeV1(_accessControl, name_, symbol_);
-        initializeV2(_clawbackReceiver);
+        initializeV3(_clawbackReceiver);
     }
 
     /**
@@ -133,10 +133,10 @@ contract mToken is ERC20PausableUpgradeable, Blacklistable, IMToken {
     }
 
     /**
-     * @dev v2 initializer
+     * @dev v3 initializer
      * @param _clawbackReceiver address to which clawback tokens will be sent
      */
-    function initializeV2(address _clawbackReceiver)
+    function initializeV3(address _clawbackReceiver)
         public
         virtual
         reinitializer(3)
@@ -204,7 +204,8 @@ contract mToken is ERC20PausableUpgradeable, Blacklistable, IMToken {
      * @inheritdoc IMToken
      */
     function burn(address from, uint256 amount)
-        external
+        public
+        virtual
         onlyRoleNoTimelock(burnerRole(), false)
     {
         _onlyNotBlacklisted(from);

contracts/mTokenPermissioned.sol

@@ -49,6 +49,16 @@ contract mTokenPermissioned is mToken {
         return _GREENLISTED_ROLE;
     }
 
+    /**
+     * @notice Burns tokens from a given address
+     * @param from address to burn tokens from
+     * @param amount amount of tokens to burn
+     */
+    function burn(address from, uint256 amount) public override {
+        _onlyGreenlisted(from);
+        super.burn(from, amount);
+    }
+
     /**
      * @dev overrides _beforeTokenTransfer function to allow
      * greenlisted users to use the token transfers functions

test/common/timelock-manager.helpers.ts

@@ -244,6 +244,7 @@ const validateOperationDetails = async ({
   expect(details.votesForVeto).to.be.equal(0);
   expect(details.createdAt).to.be.equal(blockTimestamp);
   expect(details.executionApprovedAt).to.be.equal(0);
+  expect(details.pausedAt).to.be.equal(0);
   expect(details.pauseReasonCode).to.be.equal(0);
   expect(details.councilVersion).to.be.equal(councilVersionBefore);
   expect(details.isSetCouncilOperation).to.be.equal(isSetCouncilOperation);
@@ -317,6 +318,7 @@ export const executeTimelockOperationTester = async (
   expect(detailsAfter.executionApprovedAt).to.be.equal(
     detailsBefore.executionApprovedAt,
   );
+  expect(detailsAfter.pausedAt).to.be.equal(detailsBefore.pausedAt);
   expect(detailsAfter.pauseReasonCode).to.be.equal(
     detailsBefore.pauseReasonCode,
   );
@@ -409,6 +411,7 @@ export const pauseTimelockOperationTest = async (
   expect(details.status).to.be.equal(2);
   expect(details.pauser).to.be.equal(from.address);
   expect(details.pauseReasonCode).to.be.equal(pauseReasonCode);
+  expect(details.pausedAt).to.be.equal(await getCurrentBlockTimestamp());
   expect(details.councilVersion).to.be.equal(detailsBefore.councilVersion);
   expect(details.operationProposer).to.be.equal(
     detailsBefore.operationProposer,
@@ -455,6 +458,7 @@ export const voteForVetoTest = async (
   const details = await timelockManager.getOperationDetails(operationId);
   expect(details.pauser).to.be.equal(detailsBefore.pauser);
   expect(details.pauseReasonCode).to.be.equal(detailsBefore.pauseReasonCode);
+  expect(details.pausedAt).to.be.equal(detailsBefore.pausedAt);
   expect(details.councilVersion).to.be.equal(detailsBefore.councilVersion);
   expect(details.operationProposer).to.be.equal(
     detailsBefore.operationProposer,
@@ -504,6 +508,7 @@ export const voteForExecutionTest = async (
   const details = await timelockManager.getOperationDetails(operationId);
   expect(details.pauser).to.be.equal(detailsBefore.pauser);
   expect(details.pauseReasonCode).to.be.equal(detailsBefore.pauseReasonCode);
+  expect(details.pausedAt).to.be.equal(detailsBefore.pausedAt);
   expect(details.councilVersion).to.be.equal(detailsBefore.councilVersion);
   expect(details.operationProposer).to.be.equal(
     detailsBefore.operationProposer,
@@ -606,6 +611,7 @@ export const abortOperationTest = async (
   expect(details.executionApprovedAt).to.be.equal(
     detailsBefore.executionApprovedAt,
   );
+  expect(details.pausedAt).to.be.equal(detailsBefore.pausedAt);
   expect(await timelock.isOperation(operationId)).to.be.false;
 
   if (details.isSetCouncilOperation) {