From 3afae1b0bdfad154928f2ab0f4917e49afed8323 Mon Sep 17 00:00:00 2001 From: Nika Khachiashvili Date: Tue, 1 Oct 2024 15:50:24 -0400 Subject: [PATCH 1/3] fix [HAL-04]: Unintended Burning of Excess Stablecoins in Redeem Function --- src/adapters/MorphoRUSDAdapter.sol | 11 +++++--- test/morpho/MorphoRUSDAdapter.t.sol | 40 +++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/src/adapters/MorphoRUSDAdapter.sol b/src/adapters/MorphoRUSDAdapter.sol index 02f79ac..7568a03 100644 --- a/src/adapters/MorphoRUSDAdapter.sol +++ b/src/adapters/MorphoRUSDAdapter.sol @@ -60,10 +60,15 @@ contract MorphoRUSDAdapter is IAssetAdapter, AccessControl { function redeem(uint256 shares) public onlyRole(CONTROLLER) { Stablecoin underlyingStablecoin = Stablecoin(address(underlying)); + + uint256 initialBalance = underlyingStablecoin.balanceOf(address(this)); + vault.redeem(shares, address(this), address(this)); - underlyingStablecoin.burn( - underlyingStablecoin.balanceOf(address(this)) - ); + + uint256 receivedAmount = underlyingStablecoin.balanceOf(address(this)) - + initialBalance; + + underlyingStablecoin.burn(receivedAmount); emit Redeem(msg.sender, shares, block.timestamp); } diff --git a/test/morpho/MorphoRUSDAdapter.t.sol b/test/morpho/MorphoRUSDAdapter.t.sol index 4f548e7..8db5bca 100644 --- a/test/morpho/MorphoRUSDAdapter.t.sol +++ b/test/morpho/MorphoRUSDAdapter.t.sol @@ -255,6 +255,25 @@ contract MorphoRUSDAdapterTest is Test { assertEq(adapter.fundBalance(), metamorpho.balanceOf(address(adapter))); } + function test_redeem_with_accidental_sent_tokens( + uint256 depositAmount, + uint256 redeemAmount, + uint256 accidentalySentAmount + ) external { + vm.assume(depositAmount <= 1_000_000_000e18); + vm.assume(redeemAmount <= depositAmount); + vm.assume(accidentalySentAmount <= 1_000_000_000e18); + + adapter.deposit(depositAmount); + + deal(address(rusd), address(this), accidentalySentAmount, true); + rusd.transfer(address(adapter), accidentalySentAmount); + + adapter.redeem(redeemAmount); + + assertEq(rusd.balanceOf(address(adapter)), accidentalySentAmount); + } + //! DONT KNOW WHATS THE REAL CAP // function testDepositMoreThenCap(uint256 amount) external { // vm.assume(amount > CAP); @@ -330,4 +349,25 @@ contract MorphoRUSDAdapterTest is Test { vm.expectRevert(); adapter.setFundRiskWeight(riskWeight); } + + function test_recover(uint256 _amount) external { + ERC20 testToken = new ERC20("Test Token", "TTT"); + + deal(address(testToken), address(adapter), _amount); + + assertEq(testToken.balanceOf(address(this)), 0); + assertEq(testToken.balanceOf(address(adapter)), _amount); + + adapter.recover(address(testToken)); + + assertEq(testToken.balanceOf(address(this)), _amount); + assertEq(testToken.balanceOf(address(adapter)), 0); + } + + function test_recover_as_non_owner() external { + adapter.revokeRole(adapter.MANAGER(), address(this)); + + vm.expectRevert(); + adapter.recover(address(rusd)); + } } From 90b3c94fe1e4dc1032599d3abba688c28003b41a Mon Sep 17 00:00:00 2001 From: Nika Khachiashvili Date: Tue, 1 Oct 2024 15:53:18 -0400 Subject: [PATCH 2/3] fix [HAL-03] unsafe type casting --- src/adapters/MorphoRUSDAdapter.sol | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/adapters/MorphoRUSDAdapter.sol b/src/adapters/MorphoRUSDAdapter.sol index 7568a03..352a19f 100644 --- a/src/adapters/MorphoRUSDAdapter.sol +++ b/src/adapters/MorphoRUSDAdapter.sol @@ -6,6 +6,7 @@ import {AccessControl} from "openzeppelin-contracts/contracts/access/AccessContr import {IERC4626} from "openzeppelin-contracts/contracts/interfaces/IERC4626.sol"; import {IERC20} from "openzeppelin-contracts/contracts/token/ERC20/IERC20.sol"; +import {SafeCast} from "openzeppelin-contracts/contracts/utils/math/SafeCast.sol"; import {Stablecoin} from "../Stablecoin.sol"; import {IOracle} from "src/interfaces/IOracle.sol"; @@ -192,13 +193,13 @@ contract MorphoRUSDAdapter is IAssetAdapter, AccessControl { { int256 latestAnswer = underlyingPriceOracle.latestAnswer(); - return latestAnswer > 0 ? uint256(latestAnswer) : 0; + return latestAnswer > 0 ? SafeCast.toUint256(latestAnswer) : 0; } function _fundPriceOracleLatestAnswer() private view returns (uint256) { int256 latestAnswer = fundPriceOracle.latestAnswer(); - return latestAnswer > 0 ? uint256(latestAnswer) : 0; + return latestAnswer > 0 ? SafeCast.toUint256(latestAnswer) : 0; } function recover(address _token) external onlyRole(MANAGER) { From afac7ce5615f6225ca17f798dd99d08e42481005 Mon Sep 17 00:00:00 2001 From: Nika Khachiashvili Date: Mon, 28 Oct 2024 20:06:46 +0400 Subject: [PATCH 3/3] fix MorphoRUSDAdapterTest --- test/morpho/MorphoRUSDAdapter.t.sol | 141 ++++++++++++++++++---------- 1 file changed, 91 insertions(+), 50 deletions(-) diff --git a/test/morpho/MorphoRUSDAdapter.t.sol b/test/morpho/MorphoRUSDAdapter.t.sol index 4f548e7..00e364e 100644 --- a/test/morpho/MorphoRUSDAdapter.t.sol +++ b/test/morpho/MorphoRUSDAdapter.t.sol @@ -37,6 +37,12 @@ contract MorphoRUSDAdapterTest is Test { string MAINNET_RPC_URL = vm.envString("MAINNET_RPC_URL"); + // avoid stack too deep errors for testDepositRedeemFlow test + uint256 depositShares1; + uint256 redeemShares1; + uint256 depositShares2; + uint256 redeemShares2; + function setUp() external { vm.createSelectFork(MAINNET_RPC_URL); @@ -112,25 +118,36 @@ contract MorphoRUSDAdapterTest is Test { emit Deposit(address(this), depositAmount1, block.timestamp); adapter.deposit(depositAmount1); - assertEq( + depositShares1 = metamorpho.convertToShares(depositAmount1); + + assertApproxEqAbs( metamorpho.convertToAssets(metamorpho.balanceOf(address(adapter))), - depositAmount1 + depositAmount1, + 10 ); assertEq(rusd.balanceOf(address(adapter)), 0); assertEq(rusd.balanceOf(address(this)), 0); - assertEq(rusd.totalSupply(), initialRusdTotalSupply + depositAmount1); + assertApproxEqAbs( + rusd.totalSupply(), + initialRusdTotalSupply + depositAmount1, + 10 + ); - assertEq( + assertApproxEqAbs( adapter.totalValue(), - (uint256(vaultSharesOracleV2.latestAnswer()) * depositAmount1) / 1e8 + (uint256(vaultSharesOracleV2.latestAnswer()) * depositShares1) / + 1e8, + 10 ); assertEq(adapter.fundTotalValue(), adapter.totalValue()); - assertEq( + + assertApproxEqAbs( adapter.totalRiskValue(), (riskWeight * ((uint256(vaultSharesOracleV2.latestAnswer()) * - depositAmount1) / 1e8)) / 1e6 + depositShares1) / 1e8)) / 1e6, + 10 ); assertEq(adapter.fundTotalRiskValue(), adapter.totalRiskValue()); assertEq(adapter.underlyingTotalRiskValue(), 0); @@ -139,32 +156,43 @@ contract MorphoRUSDAdapterTest is Test { assertEq(adapter.fundBalance(), metamorpho.balanceOf(address(adapter))); vm.expectEmit(true, true, true, true); - emit Redeem(address(this), redeemAmount1, block.timestamp); - adapter.redeem(redeemAmount1); + emit Redeem( + address(this), + metamorpho.convertToShares(redeemAmount1), + block.timestamp + ); + adapter.redeem(metamorpho.convertToShares(redeemAmount1)); - assertEq( + depositShares1 = metamorpho.convertToShares(depositAmount1); + redeemShares1 = metamorpho.convertToShares(redeemAmount1); + + assertApproxEqAbs( metamorpho.convertToAssets(metamorpho.balanceOf(address(adapter))), - depositAmount1 - redeemAmount1 + depositAmount1 - redeemAmount1, + 10 ); assertEq(rusd.balanceOf(address(adapter)), 0); assertEq(rusd.balanceOf(address(this)), 0); - assertEq( + assertApproxEqAbs( rusd.totalSupply(), - initialRusdTotalSupply + depositAmount1 - redeemAmount1 + initialRusdTotalSupply + depositAmount1 - redeemAmount1, + 10 ); - assertEq( + assertApproxEqAbs( adapter.totalValue(), (uint256(vaultSharesOracleV2.latestAnswer()) * - (depositAmount1 - redeemAmount1)) / 1e8 + (depositShares1 - redeemShares1)) / 1e8, + 10 ); assertEq(adapter.fundTotalValue(), adapter.totalValue()); - assertEq( + assertApproxEqAbs( adapter.totalRiskValue(), (riskWeight * ((uint256(vaultSharesOracleV2.latestAnswer()) * - (depositAmount1 - redeemAmount1)) / 1e8)) / 1e6 + (depositShares1 - redeemShares1)) / 1e8)) / 1e6, + 10 ); assertEq(adapter.fundTotalRiskValue(), adapter.totalRiskValue()); assertEq(adapter.underlyingTotalRiskValue(), 0); @@ -176,33 +204,41 @@ contract MorphoRUSDAdapterTest is Test { emit Deposit(address(this), depositAmount2, block.timestamp); adapter.deposit(depositAmount2); - assertEq( + depositShares1 = metamorpho.convertToShares(depositAmount1); + redeemShares1 = metamorpho.convertToShares(redeemAmount1); + depositShares2 = metamorpho.convertToShares(depositAmount2); + + assertApproxEqAbs( metamorpho.convertToAssets(metamorpho.balanceOf(address(adapter))), - depositAmount1 - redeemAmount1 + depositAmount2 + depositAmount1 - redeemAmount1 + depositAmount2, + 10 ); assertEq(rusd.balanceOf(address(adapter)), 0); assertEq(rusd.balanceOf(address(this)), 0); - assertEq( + assertApproxEqAbs( rusd.totalSupply(), initialRusdTotalSupply + depositAmount1 - redeemAmount1 + - depositAmount2 + depositAmount2, + 10 ); - assertEq( + assertApproxEqAbs( adapter.totalValue(), (uint256(vaultSharesOracleV2.latestAnswer()) * - (depositAmount1 - redeemAmount1 + depositAmount2)) / 1e8 + (depositShares1 - redeemShares1 + depositShares2)) / 1e8, + 10 ); assertEq(adapter.fundTotalValue(), adapter.totalValue()); - assertEq( + assertApproxEqAbs( adapter.totalRiskValue(), (riskWeight * ((uint256(vaultSharesOracleV2.latestAnswer()) * - (depositAmount1 - redeemAmount1 + depositAmount2)) / 1e8)) / - 1e6 + (depositShares1 - redeemShares1 + depositShares2)) / 1e8)) / + 1e6, + 10 ); assertEq(adapter.fundTotalRiskValue(), adapter.totalRiskValue()); assertEq(adapter.underlyingTotalRiskValue(), 0); @@ -211,42 +247,55 @@ contract MorphoRUSDAdapterTest is Test { assertEq(adapter.fundBalance(), metamorpho.balanceOf(address(adapter))); vm.expectEmit(true, true, true, true); - emit Redeem(address(this), redeemAmount2, block.timestamp); - adapter.redeem(redeemAmount2); + emit Redeem( + address(this), + metamorpho.convertToShares(redeemAmount2), + block.timestamp + ); + adapter.redeem(metamorpho.convertToShares(redeemAmount2)); - assertEq( + depositShares1 = metamorpho.convertToShares(depositAmount1); + redeemShares1 = metamorpho.convertToShares(redeemAmount1); + depositShares2 = metamorpho.convertToShares(depositAmount2); + redeemShares2 = metamorpho.convertToShares(redeemAmount2); + + assertApproxEqAbs( metamorpho.convertToAssets(metamorpho.balanceOf(address(adapter))), - depositAmount1 - redeemAmount1 + depositAmount2 - redeemAmount2 + depositAmount1 - redeemAmount1 + depositAmount2 - redeemAmount2, + 10 ); assertEq(rusd.balanceOf(address(adapter)), 0); assertEq(rusd.balanceOf(address(this)), 0); - assertEq( + assertApproxEqAbs( rusd.totalSupply(), initialRusdTotalSupply + depositAmount1 - redeemAmount1 + depositAmount2 - - redeemAmount2 + redeemAmount2, + 10 ); - assertEq( + assertApproxEqAbs( adapter.totalValue(), (uint256(vaultSharesOracleV2.latestAnswer()) * - (depositAmount1 - - redeemAmount1 + - depositAmount2 - - redeemAmount2)) / 1e8 + (depositShares1 - + redeemShares1 + + depositShares2 - + redeemShares2)) / 1e8, + 10 ); assertEq(adapter.fundTotalValue(), adapter.totalValue()); - assertEq( + assertApproxEqAbs( adapter.totalRiskValue(), (riskWeight * ((uint256(vaultSharesOracleV2.latestAnswer()) * - (depositAmount1 - - redeemAmount1 + - depositAmount2 - - redeemAmount2)) / 1e8)) / 1e6 + (depositShares1 - + redeemShares1 + + depositShares2 - + redeemShares2)) / 1e8)) / 1e6, + 10 ); assertEq(adapter.fundTotalRiskValue(), adapter.totalRiskValue()); assertEq(adapter.underlyingTotalRiskValue(), 0); @@ -255,14 +304,6 @@ contract MorphoRUSDAdapterTest is Test { assertEq(adapter.fundBalance(), metamorpho.balanceOf(address(adapter))); } - //! DONT KNOW WHATS THE REAL CAP - // function testDepositMoreThenCap(uint256 amount) external { - // vm.assume(amount > CAP); - - // vm.expectRevert(); - // adapter.deposit(amount); - // } - function testDepositUnauthorized(uint256 amount) external { adapter.revokeRole(adapter.CONTROLLER(), address(this));