From 44ee600a671dd1bcc6e96985d83a703fccfd7022 Mon Sep 17 00:00:00 2001 From: kphed Date: Fri, 12 Jan 2024 12:06:35 -0500 Subject: [PATCH] Resolve L-04 (1 of 2): Add minShares to custom deposit method --- script/BrrETH.s.sol | 2 +- script/BrrETHRedeemHelper.s.sol | 5 ++- src/BrrETH.sol | 13 ++++++-- src/BrrETHRedeemHelper.sol | 9 ++++-- test/BrrETH.t.sol | 54 ++++++++++++++++++++++++++++----- test/BrrETHRedeemHelper.t.sol | 23 +++++++------- test/Helper.sol | 4 +++ 7 files changed, 83 insertions(+), 27 deletions(-) diff --git a/script/BrrETH.s.sol b/script/BrrETH.s.sol index 5482ce8..f3f06ce 100644 --- a/script/BrrETH.s.sol +++ b/script/BrrETH.s.sol @@ -14,7 +14,7 @@ contract BrrETHScript is Script { BrrETH brrETH = new BrrETH(vm.envAddress("OWNER")); - brrETH.deposit{value: _INITIAL_DEPOSIT_AMOUNT}(_BURN_ADDRESS); + brrETH.deposit{value: _INITIAL_DEPOSIT_AMOUNT}(_BURN_ADDRESS, 1); vm.stopBroadcast(); } diff --git a/script/BrrETHRedeemHelper.s.sol b/script/BrrETHRedeemHelper.s.sol index a172e8e..e1e93ad 100644 --- a/script/BrrETHRedeemHelper.s.sol +++ b/script/BrrETHRedeemHelper.s.sol @@ -5,9 +5,12 @@ import "forge-std/Script.sol"; import {BrrETHRedeemHelper} from "src/BrrETHRedeemHelper.sol"; contract BrrETHRedeemHelperScript is Script { + address private constant _BRR_ETH = + 0xf1288441F094d0D73bcA4E57dDd07829B34de681; + function run() public { vm.broadcast(vm.envUint("PRIVATE_KEY")); - new BrrETHRedeemHelper(); + new BrrETHRedeemHelper(_BRR_ETH); } } diff --git a/src/BrrETH.sol b/src/BrrETH.sol index 8962c3f..621c2e3 100644 --- a/src/BrrETH.sol +++ b/src/BrrETH.sol @@ -51,6 +51,7 @@ contract BrrETH is Ownable, ERC4626 { event SetProtocolFeeReceiver(address); event SetFeeDistributor(address); + error InsufficientSharesMinted(); error InsufficientAssetBalance(); error InvalidCometRewards(); error InvalidRouter(); @@ -148,10 +149,14 @@ contract BrrETH is Ownable, ERC4626 { /** * @notice Mints `shares` Vault shares to `to` by depositing `assets` received from supplying ETH. - * @param to address Address to mint shares to. - * @return shares uint256 Amount of shares minted. + * @param to address Address to mint shares to. + * @param minShares uint256 The minimum amount of shares that must be minted. + * @return shares uint256 Amount of shares minted. */ - function deposit(address to) external payable returns (uint256 shares) { + function deposit( + address to, + uint256 minShares + ) external payable returns (uint256 shares) { IWETH(_WETH).deposit{value: msg.value}(); uint256 totalAssetsBefore = totalAssets(); @@ -161,6 +166,8 @@ contract BrrETH is Ownable, ERC4626 { uint256 assets = totalAssets() - totalAssetsBefore; shares = convertToShares(assets, totalSupply(), totalAssetsBefore); + if (shares < minShares) revert InsufficientSharesMinted(); + _deposit(msg.sender, to, assets, shares); } diff --git a/src/BrrETHRedeemHelper.sol b/src/BrrETHRedeemHelper.sol index eb2949f..0a45d25 100644 --- a/src/BrrETHRedeemHelper.sol +++ b/src/BrrETHRedeemHelper.sol @@ -9,15 +9,18 @@ import {IWETH} from "src/interfaces/IWETH.sol"; contract BrrETHRedeemHelper { using SafeTransferLib for address; - IBrrETH private constant _BRR_ETH = - IBrrETH(0xf1288441F094d0D73bcA4E57dDd07829B34de681); IComet private constant _COMET = IComet(0x46e6b214b524310239732D51387075E0e70970bf); IWETH private constant _WETH = IWETH(0x4200000000000000000000000000000000000006); + IBrrETH public immutable brrETH; receive() external payable {} + constructor(address _brrETH) { + brrETH = IBrrETH(_brrETH); + } + /** * @notice Redeem brrETH for ETH. * @param shares uint256 Amount of shares to redeem. @@ -25,7 +28,7 @@ contract BrrETHRedeemHelper { */ function redeem(uint256 shares, address to) external { // Requires approval from the caller to spend their brrETH balance. - _BRR_ETH.redeem(shares, address(this), msg.sender); + brrETH.redeem(shares, address(this), msg.sender); // Comet's alias for an "entire balance" is `type(uint256).max`. _COMET.withdraw(address(_WETH), type(uint256).max); diff --git a/test/BrrETH.t.sol b/test/BrrETH.t.sol index 695e6f4..02d1d33 100644 --- a/test/BrrETH.t.sol +++ b/test/BrrETH.t.sol @@ -17,8 +17,6 @@ contract BrrETHTest is Helper { using SafeTransferLib for address; using FixedPointMathLib for uint256; - address public immutable owner = address(this); - BrrETH public immutable vault = new BrrETH(address(this)); address[10] public anvilAccounts = [ address(0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266), address(0x70997970C51812dc3A010C7d01b50e0d17dc79C8), @@ -150,9 +148,33 @@ contract BrrETHTest is Helper { deposit (ETH) //////////////////////////////////////////////////////////////*/ + function testCannotDepositETHInsufficientSharesMinted() external { + uint256 assets = 0; + address to = address(this); + uint256 minShares = 1; + + assertLt( + vault.convertToShares( + assets, + vault.totalSupply(), + vault.totalAssets() + ), + minShares + ); + + vm.expectRevert(BrrETH.InsufficientSharesMinted.selector); + + vault.deposit{value: assets}(to, minShares); + } + function testDepositETH() external { uint256 assets = 1 ether; address to = address(this); + uint256 minShares = vault.convertToShares( + assets - _COMET_ROUNDING_ERROR_MARGIN, + vault.totalSupply(), + vault.totalAssets() + ); uint256 totalSupplyBefore = vault.totalSupply(); uint256 totalAssetsBefore = vault.totalAssets(); @@ -161,7 +183,7 @@ contract BrrETHTest is Helper { emit ERC4626.Deposit(address(this), to, assets, 0); - uint256 shares = vault.deposit{value: assets}(to); + uint256 shares = vault.deposit{value: assets}(to, minShares); uint256 totalSupplyAfter = vault.totalSupply(); uint256 totalAssetsAfter = vault.totalAssets(); uint256 expectedShares = vault.convertToShares( @@ -170,6 +192,7 @@ contract BrrETHTest is Helper { totalAssetsBefore ); + assertLe(minShares, shares); assertEq(expectedShares, shares); assertEq(shares, totalSupplyAfter - totalSupplyBefore); assertEq(shares, vault.balanceOf(to)); @@ -183,6 +206,11 @@ contract BrrETHTest is Helper { for (uint256 i = 0; i < anvilAccounts.length; ++i) { uint256 assets = baseAsset * (i + 1); + uint256 minShares = vault.convertToShares( + assets - _COMET_ROUNDING_ERROR_MARGIN, + vault.totalSupply(), + vault.totalAssets() + ); uint256 totalSupplyBefore = vault.totalSupply(); uint256 totalAssetsBefore = vault.totalAssets(); @@ -190,7 +218,10 @@ contract BrrETHTest is Helper { emit ERC4626.Deposit(address(this), anvilAccounts[i], assets, 0); - uint256 shares = vault.deposit{value: assets}(anvilAccounts[i]); + uint256 shares = vault.deposit{value: assets}( + anvilAccounts[i], + minShares + ); uint256 totalSupplyAfter = vault.totalSupply(); uint256 totalAssetsAfter = vault.totalAssets(); uint256 expectedShares = vault.convertToShares( @@ -201,7 +232,7 @@ contract BrrETHTest is Helper { totalSupply += totalSupplyAfter - totalSupplyBefore; totalAssets += totalAssetsAfter - totalAssetsBefore; - assertLt(0, shares); + assertLe(minShares, shares); assertEq(expectedShares, shares); assertEq(shares, totalSupplyAfter - totalSupplyBefore); assertEq(shares, vault.balanceOf(anvilAccounts[i])); @@ -213,14 +244,21 @@ contract BrrETHTest is Helper { } function testDepositETHFuzz(uint80 assets, address to) external { + vm.assume(assets >= _COMET_ROUNDING_ERROR_MARGIN); + uint256 totalSupplyBefore = vault.totalSupply(); uint256 totalAssetsBefore = vault.totalAssets(); + uint256 minShares = vault.convertToShares( + uint256(assets) - _COMET_ROUNDING_ERROR_MARGIN, + vault.totalSupply(), + vault.totalAssets() + ); vm.expectEmit(true, true, true, false, address(vault)); emit ERC4626.Deposit(address(this), to, assets, 0); - uint256 shares = vault.deposit{value: assets}(to); + uint256 shares = vault.deposit{value: assets}(to, minShares); uint256 totalSupplyAfter = vault.totalSupply(); uint256 totalAssetsAfter = vault.totalAssets(); uint256 expectedShares = vault.convertToShares( @@ -541,7 +579,7 @@ contract BrrETHTest is Helper { assertTrue(cometRewards != address(vault.cometRewards())); // Deposit and accrue enough time to ensure `harvest` is called (i.e. emits `Harvest` event). - vault.deposit{value: 1 ether}(address(this)); + vault.deposit{value: 1 ether}(address(this), 1); skip(1 days); @@ -571,7 +609,7 @@ contract BrrETHTest is Helper { assertTrue(cometRewards != address(vault.cometRewards())); if (shouldHarvest) { - vault.deposit{value: 1 ether}(address(this)); + vault.deposit{value: 1 ether}(address(this), 1); skip(1 days); diff --git a/test/BrrETHRedeemHelper.t.sol b/test/BrrETHRedeemHelper.t.sol index 382d282..fa938a5 100644 --- a/test/BrrETHRedeemHelper.t.sol +++ b/test/BrrETHRedeemHelper.t.sol @@ -5,20 +5,21 @@ import "forge-std/Test.sol"; import {SafeTransferLib} from "solady/utils/SafeTransferLib.sol"; import {BrrETH} from "src/BrrETH.sol"; import {BrrETHRedeemHelper} from "src/BrrETHRedeemHelper.sol"; +import {Helper} from "test/Helper.sol"; -contract BrrETHRedeemHelperTest is Test { +contract BrrETHRedeemHelperTest is Test, Helper { using SafeTransferLib for address; - BrrETH public constant BRR_ETH = - BrrETH(0xf1288441F094d0D73bcA4E57dDd07829B34de681); address public constant COMET = 0x46e6b214b524310239732D51387075E0e70970bf; address public constant WETH = 0x4200000000000000000000000000000000000006; - BrrETHRedeemHelper public immutable redeemHelper = new BrrETHRedeemHelper(); + BrrETHRedeemHelper public immutable redeemHelper; receive() external payable {} constructor() { - BRR_ETH.approve(address(redeemHelper), type(uint256).max); + redeemHelper = new BrrETHRedeemHelper(address(vault)); + + vault.approve(address(redeemHelper), type(uint256).max); } /*////////////////////////////////////////////////////////////// @@ -26,10 +27,10 @@ contract BrrETHRedeemHelperTest is Test { //////////////////////////////////////////////////////////////*/ function testRedeem() external { - uint256 shares = BRR_ETH.deposit{value: 1 ether}(address(this)); + uint256 shares = vault.deposit{value: 1 ether}(address(this), 1); // The amount of cWETH that will be redeemed from brrETH. - uint256 assets = BRR_ETH.convertToAssets(shares); + uint256 assets = vault.convertToAssets(shares); uint256 ethBalanceBefore = address(this).balance; @@ -39,7 +40,7 @@ contract BrrETHRedeemHelperTest is Test { assertEq(assets - 1, address(this).balance - ethBalanceBefore); // The redeem helper should not maintain balances for any of the tokens it handles. - assertEq(0, BRR_ETH.balanceOf(address(redeemHelper))); + assertEq(0, vault.balanceOf(address(redeemHelper))); assertEq(0, COMET.balanceOf(address(redeemHelper))); assertEq(0, WETH.balanceOf(address(redeemHelper))); assertEq(0, address(redeemHelper).balance); @@ -47,8 +48,8 @@ contract BrrETHRedeemHelperTest is Test { function testRedeemFuzz(uint8 ethMultiplier) external { uint256 msgValue = 1 ether * (uint256(ethMultiplier) + 1); - uint256 shares = BRR_ETH.deposit{value: msgValue}(address(this)); - uint256 assets = BRR_ETH.convertToAssets(shares); + uint256 shares = vault.deposit{value: msgValue}(address(this), 1); + uint256 assets = vault.convertToAssets(shares); uint256 ethBalanceBefore = address(this).balance; @@ -57,7 +58,7 @@ contract BrrETHRedeemHelperTest is Test { // Account for Comet rounding down and compare against the ETH amount received. assertLe(assets - 2, address(this).balance - ethBalanceBefore); - assertEq(0, BRR_ETH.balanceOf(address(redeemHelper))); + assertEq(0, vault.balanceOf(address(redeemHelper))); assertEq(0, COMET.balanceOf(address(redeemHelper))); assertEq(0, WETH.balanceOf(address(redeemHelper))); assertEq(0, address(redeemHelper).balance); diff --git a/test/Helper.sol b/test/Helper.sol index 3e986f4..ce0cbbd 100644 --- a/test/Helper.sol +++ b/test/Helper.sol @@ -3,8 +3,11 @@ pragma solidity ^0.8.0; import "forge-std/Test.sol"; import {ICometRewards} from "src/interfaces/ICometRewards.sol"; +import {BrrETH} from "src/BrrETH.sol"; contract Helper is Test { + address public immutable owner = address(this); + BrrETH public immutable vault = new BrrETH(address(this)); string internal constant _NAME = "Brrito ETH"; string internal constant _SYMBOL = "brrETH"; address internal constant _WETH = @@ -19,4 +22,5 @@ contract Helper is Test { ICometRewards(0x123964802e6ABabBE1Bc9547D72Ef1B69B00A6b1); uint256 internal constant _FEE_BASE = 10_000; uint256 internal constant _SWAP_FEE_DEDUCTED = 9_998; + uint256 internal constant _COMET_ROUNDING_ERROR_MARGIN = 2; }