Openzeppelin synth fixes

src/Synths/ESynth.sol

@@ -22,9 +22,14 @@ contract ESynth is ERC20Collateral, Ownable {
         uint128 minted;
     }
 
+    /// @notice contains the minting capacity and minted amount for each minter.
     mapping(address => MinterData) public minters;
+    /// @notice contains the list of addresses to ignore for the total supply.
     EnumerableSet.AddressSet internal ignoredForTotalSupply;
 
+    /// @notice Emitted when the minting capacity for a minter is set.
+    /// @param minter The address of the minter.
+    /// @param capacity The capacity set for the minter.
     event MinterCapacitySet(address indexed minter, uint256 capacity);
 
     error E_CapacityReached();
@@ -121,8 +126,8 @@ contract ESynth is ERC20Collateral, Ownable {
     /// @dev Overriden due to the conflict with the Context definition.
     /// @dev This function returns the account on behalf of which the current operation is being performed, which is
     /// either msg.sender or the account authenticated by the EVC.
-    /// @return The address of the message sender.
-    function _msgSender() internal view virtual override (ERC20Collateral, Context) returns (address) {
+    /// @return msgSender The address of the message sender.
+    function _msgSender() internal view virtual override (ERC20Collateral, Context) returns (address msgSender) {
         return ERC20Collateral._msgSender();
     }
 
@@ -144,24 +149,25 @@ contract ESynth is ERC20Collateral, Ownable {
 
     /// @notice Checks if an account is ignored for the total supply.
     /// @param account The account to check.
-    function isIgnoredForTotalSupply(address account) public view returns (bool) {
+    /// @return isIgnored True if the account is ignored for the total supply. False otherwise.
+    function isIgnoredForTotalSupply(address account) external view returns (bool isIgnored) {
         return ignoredForTotalSupply.contains(account);
     }
 
     /// @notice Retrieves all the accounts ignored for the total supply.
-    /// @return The list of accounts ignored for the total supply.
-    function getAllIgnoredForTotalSupply() public view returns (address[] memory) {
+    /// @return accounts List of accounts ignored for the total supply.
+    function getAllIgnoredForTotalSupply() external view returns (address[] memory accounts) {
         return ignoredForTotalSupply.values();
     }
 
     /// @notice Retrieves the total supply of the token.
     /// @dev Overriden to exclude the ignored accounts from the total supply.
-    /// @return The total supply of the token.
-    function totalSupply() public view override returns (uint256) {
-        uint256 total = super.totalSupply();
+    /// @return total Total supply of the token.
+    function totalSupply() public view override returns (uint256 total) {
+        total = super.totalSupply();
 
         uint256 ignoredLength = ignoredForTotalSupply.length(); // cache for efficiency
-        for (uint256 i = 0; i < ignoredLength; i++) {
+        for (uint256 i = 0; i < ignoredLength; ++i) {
             total -= balanceOf(ignoredForTotalSupply.at(i));
         }
         return total;

src/Synths/EulerSavingsRate.sol

@@ -24,7 +24,11 @@ contract EulerSavingsRate is EVCUtil, ERC4626 {
     uint8 internal constant UNLOCKED = 1;
     uint8 internal constant LOCKED = 2;
 
+    /// @notice The virtual amount added to total shares and total assets.
     uint256 internal constant VIRTUAL_AMOUNT = 1e6;
+    /// @notice At least 10 times the virtual amount of shares should exist for gulp to be enabled
+    uint256 internal constant MIN_SHARES_FOR_GULP = VIRTUAL_AMOUNT * 10;
+
     uint256 public constant INTEREST_SMEAR = 2 weeks;
 
     struct ESRSlot {
@@ -34,11 +38,16 @@ contract EulerSavingsRate is EVCUtil, ERC4626 {
         uint8 locked;
     }
 
+    /// @notice Multiple state variables stored in a single storage slot.
     ESRSlot internal esrSlot;
+    /// @notice The total assets accounted for in the vault.
     uint256 internal _totalAssets;
 
     error Reentrancy();
 
+    event Gulped(uint256 gulped, uint256 interestLeft);
+    event InterestUpdated(uint256 interestAccrued, uint256 interestLeft);
+
     /// @notice Modifier to require an account status check on the EVC.
     /// @dev Calls `requireAccountStatusCheck` function from EVC for the specified account after the function body.
     /// @param account The address of the account to check.
@@ -69,6 +78,11 @@ contract EulerSavingsRate is EVCUtil, ERC4626 {
         return _totalAssets + interestAccrued();
     }
 
+    /// @notice Returns the maximum amount of shares that can be redeemed by the specified address.
+    /// @dev If the account has a controller set it's possible the withdrawal will be reverted by the controller, thus
+    /// we return 0.
+    /// @param owner The account owner.
+    /// @return The maximum amount of shares that can be redeemed.
     function maxRedeem(address owner) public view override returns (uint256) {
         // If account has borrows, withdrawal might be reverted by the controller during account status checks.
         // The vault has no way to verify or enforce the behaviour of the controller, which the account owner
@@ -83,6 +97,11 @@ contract EulerSavingsRate is EVCUtil, ERC4626 {
         return super.maxRedeem(owner);
     }
 
+    /// @notice Returns the maximum amount of assets that can be withdrawn by the specified address.
+    /// @dev If the account has a controller set it's possible the withdrawal will be reverted by the controller, thus
+    /// we return 0.
+    /// @param owner The account owner.
+    /// @return The maximum amount of assets that can be withdrawn.
     function maxWithdraw(address owner) public view override returns (uint256) {
         // If account has borrows, withdrawal might be reverted by the controller during account status checks.
         // The vault has no way to verify or enforce the behaviour of the controller, which the account owner
@@ -137,14 +156,17 @@ contract EulerSavingsRate is EVCUtil, ERC4626 {
     /// @notice Mints a certain amount of shares to the account.
     /// @param shares The amount of assets to mint.
     /// @param receiver The account to mint the shares to.
-    /// @return The amount of assets spend.
+    /// @return The amount of assets spent.
     function mint(uint256 shares, address receiver) public override nonReentrant returns (uint256) {
         return super.mint(shares, receiver);
     }
 
-    /// @notice Deposits a certain amount of assets to the vault.
-    /// @param assets The amount of assets to deposit.
+    /// @notice Withdraws a certain amount of assets to the vault.
+    /// @dev Overwritten to not call maxWithdraw which would return 0 if there is a controller set, update the accrued
+    /// interest and update _totalAssets.
+    /// @param assets The amount of assets to withdraw.
     /// @param receiver The recipient of the shares.
+    /// @param owner The account from which the assets are withdrawn
     /// @return The amount of shares minted.
     function withdraw(uint256 assets, address receiver, address owner)
         public
@@ -155,12 +177,25 @@ contract EulerSavingsRate is EVCUtil, ERC4626 {
     {
         // Move interest to totalAssets
         updateInterestAndReturnESRSlotCache();
-        return super.withdraw(assets, receiver, owner);
+
+        uint256 maxAssets = _convertToAssets(balanceOf(owner), Math.Rounding.Floor);
+        if (maxAssets < assets) {
+            revert ERC4626ExceededMaxWithdraw(owner, assets, maxAssets);
+        }
+
+        uint256 shares = previewWithdraw(assets);
+        _withdraw(_msgSender(), receiver, owner, assets, shares);
+        _totalAssets = _totalAssets - assets;
+
+        return shares;
     }
 
     /// @notice Redeems a certain amount of shares for assets.
+    /// @dev Overwritten to not call maxRedeem which would return 0 if there is a controller set, update the accrued
+    /// interest and update _totalAssets.
     /// @param shares The amount of shares to redeem.
     /// @param receiver The recipient of the assets.
+    /// @param owner The account from which the shares are redeemed.
     /// @return The amount of assets redeemed.
     function redeem(uint256 shares, address receiver, address owner)
         public
@@ -171,7 +206,17 @@ contract EulerSavingsRate is EVCUtil, ERC4626 {
     {
         // Move interest to totalAssets
         updateInterestAndReturnESRSlotCache();
-        return super.redeem(shares, receiver, owner);
+
+        uint256 maxShares = balanceOf(owner);
+        if (maxShares < shares) {
+            revert ERC4626ExceededMaxRedeem(owner, shares, maxShares);
+        }
+
+        uint256 assets = previewRedeem(shares);
+        _withdraw(_msgSender(), receiver, owner, assets, shares);
+        _totalAssets = _totalAssets - assets;
+
+        return assets;
     }
 
     function _convertToShares(uint256 assets, Math.Rounding rounding) internal view override returns (uint256) {
@@ -183,22 +228,17 @@ contract EulerSavingsRate is EVCUtil, ERC4626 {
     }
 
     function _deposit(address caller, address receiver, uint256 assets, uint256 shares) internal override {
-        _totalAssets = _totalAssets + assets;
         super._deposit(caller, receiver, assets, shares);
-    }
-
-    function _withdraw(address caller, address receiver, address owner, uint256 assets, uint256 shares)
-        internal
-        override
-    {
-        _totalAssets = _totalAssets - assets;
-        super._withdraw(caller, receiver, owner, assets, shares);
+        _totalAssets = _totalAssets + assets;
     }
 
     /// @notice Smears any donations to this vault as interest.
     function gulp() public nonReentrant {
         ESRSlot memory esrSlotCache = updateInterestAndReturnESRSlotCache();
 
+        // Do not gulp if total supply is too low
+        if (totalSupply() < MIN_SHARES_FOR_GULP) return;
+
         uint256 assetBalance = IERC20(asset()).balanceOf(address(this));
         uint256 toGulp = assetBalance - _totalAssets - esrSlotCache.interestLeft;
 
@@ -210,6 +250,8 @@ contract EulerSavingsRate is EVCUtil, ERC4626 {
 
         // write esrSlotCache back to storage in a single SSTORE
         esrSlot = esrSlotCache;
+
+        emit Gulped(toGulp, esrSlotCache.interestLeft);
     }
 
     /// @notice Updates the interest and returns the ESR storage slot cache.
@@ -226,10 +268,13 @@ contract EulerSavingsRate is EVCUtil, ERC4626 {
         // Move interest accrued to totalAssets
         _totalAssets = _totalAssets + accruedInterest;
 
+        emit InterestUpdated(accruedInterest, esrSlotCache.interestLeft);
+
         return esrSlotCache;
     }
 
     /// @notice Returns the amount of interest accrued.
+    /// @return The amount of interest accrued.
     function interestAccrued() public view returns (uint256) {
         return interestAccruedFromCache(esrSlot);
     }
@@ -253,6 +298,7 @@ contract EulerSavingsRate is EVCUtil, ERC4626 {
     }
 
     /// @notice Returns the ESR storage slot as a struct.
+    /// @return The ESR storage slot as a struct.
     function getESRSlot() public view returns (ESRSlot memory) {
         return esrSlot;
     }

src/Synths/IRMSynth.sol

@@ -2,8 +2,8 @@
 
 pragma solidity ^0.8.0;
 
-import "../InterestRateModels/IIRM.sol";
-import "../interfaces/IPriceOracle.sol";
+import {IIRM} from "../InterestRateModels/IIRM.sol";
+import {IPriceOracle} from "../interfaces/IPriceOracle.sol";
 import {IERC20} from "../EVault/IEVault.sol";
 
 /// @title IRMSynth
@@ -20,10 +20,15 @@ contract IRMSynth is IIRM {
     uint216 public constant ADJUST_ONE = 1.0e18;
     uint216 public constant ADJUST_INTERVAL = 1 hours;
 
+    /// @notice The address of the synthetic asset.
     address public immutable synth;
+    /// @notice The address of the reference asset.
     address public immutable referenceAsset;
+    /// @notice The address of the oracle.
     IPriceOracle public immutable oracle;
+    /// @notice The target quote which the IRM will try to maintain.
     uint256 public immutable targetQuote;
+    /// @notice The amount of the quote asset to use for the quote.
     uint256 public immutable quoteAmount;
 
     struct IRMData {
@@ -36,6 +41,8 @@ contract IRMSynth is IIRM {
     error E_ZeroAddress();
     error E_InvalidQuote();
 
+    event InterestUpdated(uint256 rate);
+
     constructor(address synth_, address referenceAsset_, address oracle_, uint256 targetQuoute_) {
         if (synth_ == address(0) || referenceAsset_ == address(0) || oracle_ == address(0)) {
             revert E_ZeroAddress();
@@ -54,21 +61,26 @@ contract IRMSynth is IIRM {
         }
 
         irmStorage = IRMData({lastUpdated: uint40(block.timestamp), lastRate: BASE_RATE});
+
+        emit InterestUpdated(BASE_RATE);
     }
 
+    /// @notice Computes the interest rate and updates the storage if necessary.
+    /// @return The interest rate.
     function computeInterestRate(address, uint256, uint256) external override returns (uint256) {
-        IRMData memory irmCache = irmStorage;
-        (uint216 rate, bool updated) = _computeRate(irmCache);
+        (uint216 rate, bool updated) = _computeRate(irmStorage);
 
         if (updated) {
             irmStorage = IRMData({lastUpdated: uint40(block.timestamp), lastRate: rate});
+            emit InterestUpdated(rate);
         }
 
         return rate;
     }
 
-    function computeInterestRateView(address, uint256, uint256) external view override returns (uint256) {
-        (uint216 rate,) = _computeRate(irmStorage);
+    /// @return rate The new interest rate
+    function computeInterestRateView(address, uint256, uint256) external view override returns (uint256 rate) {
+        (rate,) = _computeRate(irmStorage);
         return rate;
     }
 
@@ -103,6 +115,8 @@ contract IRMSynth is IIRM {
         return (rate, updated);
     }
 
+    /// @notice Retrieves the packed IRM data as a struct.
+    /// @return The IRM data.
     function getIRMData() external view returns (IRMData memory) {
         return irmStorage;
     }

src/Synths/PegStabilityModule.sol

@@ -22,11 +22,16 @@ contract PegStabilityModule is EVCUtil {
     uint256 public constant BPS_SCALE = 100_00;
     uint256 public constant PRICE_SCALE = 1e18;
 
+    /// @notice The synthetic asset.
     ESynth public immutable synth;
+    /// @notice The underlying asset.
     IERC20 public immutable underlying;
+    /// @notice The conversion price between the synthetic and underlying asset.
     uint256 public immutable conversionPrice; // 1e18 = 1 SYNTH == 1 UNDERLYING, 0.01e18 = 1 SYNTH == 0.01 UNDERLYING
 
+    /// @notice The fee for swapping to the underlying asset in basis points.
     uint256 public immutable TO_UNDERLYING_FEE;
+    /// @notice The fee for swapping to the synthetic asset in basis points.
     uint256 public immutable TO_SYNTH_FEE;
 
     error E_ZeroAddress();

test/unit/esr/ESR.Fuzz.t.sol

@@ -38,6 +38,7 @@ contract ESRFuzzTest is ESRTest {
 
     // this tests shows that when you have a very small deposit and a very large interestAmount minted to the contract
     function testFuzz_gulp_under_uint168(uint256 interestAmount, uint256 depositAmount) public {
+        uint256 MIN_SHARES_FOR_GULP = 10 * 1e6;
         depositAmount = bound(depositAmount, 0, type(uint112).max);
         interestAmount = bound(interestAmount, 0, type(uint256).max - depositAmount); // this makes sure that the mint
             // won't cause overflow
@@ -49,10 +50,14 @@ contract ESRFuzzTest is ESRTest {
 
         EulerSavingsRate.ESRSlot memory esrSlot = esr.updateInterestAndReturnESRSlotCache();
 
-        if (interestAmount <= type(uint168).max) {
-            assertEq(esrSlot.interestLeft, interestAmount);
+        if (depositAmount >= MIN_SHARES_FOR_GULP) {
+            if (interestAmount <= type(uint168).max) {
+                assertEq(esrSlot.interestLeft, interestAmount);
+            } else {
+                assertEq(esrSlot.interestLeft, type(uint168).max);
+            }
         } else {
-            assertEq(esrSlot.interestLeft, type(uint168).max);
+            assertEq(esrSlot.interestLeft, 0);
         }
     }