Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(erc4626): make decimals offset adapt #342

Merged
merged 4 commits into from
Dec 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/foundry.yml
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ jobs:
- uses: ./.github/actions/install-cache

- name: Run tests in ${{ matrix.type }} mode
run: yarn test:forge
run: yarn test:forge -vvv
env:
FOUNDRY_FUZZ_RUNS: ${{ matrix.fuzz-runs }}
FOUNDRY_FUZZ_MAX_TEST_REJECTS: ${{ matrix.max-test-rejects }}
Expand Down
14 changes: 10 additions & 4 deletions src/MetaMorpho.sol
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph
/// @notice The address of the Morpho contract.
IMorpho public immutable MORPHO;

/// @notice OpenZeppelin decimals offset used by the ERC4626 implementation.
/// @dev Calculated to be max(0, 18 - underlyingDecimals) at construction, so the initial conversion rate maximizes
/// precision between shares and assets.
uint8 public immutable DECIMALS_OFFSET;

/* STORAGE */

/// @notice The address of the curator.
Expand Down Expand Up @@ -126,6 +131,7 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph
if (morpho == address(0)) revert ErrorsLib.ZeroAddress();

MORPHO = IMorpho(morpho);
DECIMALS_OFFSET = uint8(uint256(18).zeroFloorSub(IERC20Metadata(_asset).decimals()));
QGarchery marked this conversation as resolved.
Show resolved Hide resolved

_checkTimelockBounds(initialTimelock);
_setTimelock(initialTimelock);
Expand Down Expand Up @@ -619,8 +625,8 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph
/* ERC4626 (INTERNAL) */

/// @inheritdoc ERC4626
function _decimalsOffset() internal pure override returns (uint8) {
return ConstantsLib.DECIMALS_OFFSET;
function _decimalsOffset() internal view override returns (uint8) {
return DECIMALS_OFFSET;
}

/// @dev Returns the maximum amount of asset (`assets`) that the `owner` can withdraw from the vault, as well as the
Expand Down Expand Up @@ -675,7 +681,7 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph
uint256 newTotalSupply,
uint256 newTotalAssets,
Math.Rounding rounding
) internal pure returns (uint256) {
) internal view returns (uint256) {
return assets.mulDiv(newTotalSupply + 10 ** _decimalsOffset(), newTotalAssets + 1, rounding);
}

Expand All @@ -686,7 +692,7 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph
uint256 newTotalSupply,
uint256 newTotalAssets,
Math.Rounding rounding
) internal pure returns (uint256) {
) internal view returns (uint256) {
return shares.mulDiv(newTotalAssets + 1, newTotalSupply + 10 ** _decimalsOffset(), rounding);
}

Expand Down
1 change: 1 addition & 0 deletions src/interfaces/IMetaMorpho.sol
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ interface IOwnable {
/// @dev Consider using the IMetaMorpho interface instead of this one.
interface IMetaMorphoBase {
function MORPHO() external view returns (IMorpho);
function DECIMALS_OFFSET() external view returns (uint8);

function curator() external view returns (address);
function isAllocator(address target) external view returns (bool);
Expand Down
3 changes: 0 additions & 3 deletions src/libraries/ConstantsLib.sol
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,6 @@ library ConstantsLib {
/// @dev The minimum delay of a timelock.
uint256 internal constant MIN_TIMELOCK = 1 days;

/// @dev OpenZeppelin's decimals offset used in MetaMorpho's ERC4626 implementation.
uint8 internal constant DECIMALS_OFFSET = 6;

/// @dev The maximum number of markets in the supply/withdraw queue.
uint256 internal constant MAX_QUEUE_LENGTH = 30;

Expand Down
28 changes: 18 additions & 10 deletions test/forge/ERC4626Test.sol
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,14 @@ contract ERC4626Test is IntegrationTest, IMorphoFlashLoanCallback {
_sortSupplyQueueIdleLast();
}

function testDecimals() public {
assertEq(vault.decimals(), loanToken.decimals() + ConstantsLib.DECIMALS_OFFSET, "decimals");
Rubilmax marked this conversation as resolved.
Show resolved Hide resolved
function testDecimals(uint8 decimals) public {
vm.mockCall(address(loanToken), abi.encodeWithSignature("decimals()"), abi.encode(decimals));

vault = IMetaMorpho(
address(new MetaMorpho(OWNER, address(morpho), TIMELOCK, address(loanToken), "MetaMorpho Vault", "MMV"))
);

assertEq(vault.decimals(), Math.max(18, decimals), "decimals");
}

function testMint(uint256 assets) public {
Expand Down Expand Up @@ -116,16 +122,18 @@ contract ERC4626Test is IntegrationTest, IMorphoFlashLoanCallback {
function testRedeemTooMuch(uint256 deposited) public {
deposited = bound(deposited, MIN_TEST_ASSETS, MAX_TEST_ASSETS);

loanToken.setBalance(SUPPLIER, deposited);
loanToken.setBalance(SUPPLIER, deposited * 2);

vm.prank(SUPPLIER);
uint256 shares = vault.deposit(deposited, ONBEHALF);
vm.startPrank(SUPPLIER);
uint256 shares = vault.deposit(deposited, SUPPLIER);
vault.deposit(deposited, ONBEHALF);
vm.stopPrank();

vm.prank(ONBEHALF);
vm.prank(SUPPLIER);
vm.expectRevert(
abi.encodeWithSelector(IERC20Errors.ERC20InsufficientBalance.selector, ONBEHALF, shares, shares + 1)
abi.encodeWithSelector(IERC20Errors.ERC20InsufficientBalance.selector, SUPPLIER, shares, shares + 1)
);
vault.redeem(shares + 1, RECEIVER, ONBEHALF);
vault.redeem(shares + 1, RECEIVER, SUPPLIER);
}

function testWithdrawAll(uint256 assets) public {
Expand Down Expand Up @@ -270,7 +278,7 @@ contract ERC4626Test is IntegrationTest, IMorphoFlashLoanCallback {
vm.prank(SUPPLIER);
vault.deposit(deposited, ONBEHALF);

assets = bound(assets, deposited + 1, type(uint256).max / (deposited + 10 ** ConstantsLib.DECIMALS_OFFSET));
assets = bound(assets, deposited + 1, type(uint256).max / (deposited + 1));

vm.prank(ONBEHALF);
vm.expectRevert(ErrorsLib.NotEnoughLiquidity.selector);
Expand All @@ -285,7 +293,7 @@ contract ERC4626Test is IntegrationTest, IMorphoFlashLoanCallback {
vm.prank(SUPPLIER);
vault.deposit(deposited, ONBEHALF);

assets = bound(assets, deposited + 1, type(uint256).max / (deposited + 10 ** ConstantsLib.DECIMALS_OFFSET));
assets = bound(assets, deposited + 1, type(uint256).max / (deposited + 1));

collateralToken.setBalance(BORROWER, type(uint128).max);

Expand Down
22 changes: 6 additions & 16 deletions test/forge/FeeTest.sol
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,7 @@ contract FeeTest is IntegrationTest {
uint256 interest = totalAssetsAfter - vault.lastTotalAssets();
uint256 feeAssets = interest.mulDiv(FEE, WAD);

return feeAssets.mulDiv(
vault.totalSupply() + 10 ** ConstantsLib.DECIMALS_OFFSET,
totalAssetsAfter - feeAssets + 1,
Math.Rounding.Floor
);
return feeAssets.mulDiv(vault.totalSupply() + 1, totalAssetsAfter - feeAssets + 1, Math.Rounding.Floor);
}

function testAccrueFeeWithinABlock(uint256 deposited, uint256 withdrawn) public {
Expand Down Expand Up @@ -312,11 +308,8 @@ contract FeeTest is IntegrationTest {
_forward(blocks);

uint256 feeShares = _feeShares();
uint256 expectedShares = assets.mulDiv(
vault.totalSupply() + feeShares + 10 ** ConstantsLib.DECIMALS_OFFSET,
vault.totalAssets() + 1,
Math.Rounding.Floor
);
uint256 expectedShares =
assets.mulDiv(vault.totalSupply() + feeShares + 1, vault.totalAssets() + 1, Math.Rounding.Floor);
uint256 shares = vault.convertToShares(assets);

assertEq(shares, expectedShares, "shares");
Expand All @@ -325,7 +318,7 @@ contract FeeTest is IntegrationTest {

function testConvertToSharesWithFeeAndInterest(uint256 deposited, uint256 shares, uint256 blocks) public {
deposited = bound(deposited, MIN_TEST_ASSETS, MAX_TEST_ASSETS);
shares = bound(shares, 10 ** ConstantsLib.DECIMALS_OFFSET, MAX_TEST_ASSETS);
shares = bound(shares, 1, MAX_TEST_ASSETS);
blocks = _boundBlocks(blocks);

loanToken.setBalance(SUPPLIER, deposited);
Expand All @@ -338,11 +331,8 @@ contract FeeTest is IntegrationTest {
_forward(blocks);

uint256 feeShares = _feeShares();
uint256 expectedAssets = shares.mulDiv(
vault.totalAssets() + 1,
vault.totalSupply() + feeShares + 10 ** ConstantsLib.DECIMALS_OFFSET,
Math.Rounding.Floor
);
uint256 expectedAssets =
shares.mulDiv(vault.totalAssets() + 1, vault.totalSupply() + feeShares + 1, Math.Rounding.Floor);
uint256 assets = vault.convertToAssets(shares);

assertEq(assets, expectedAssets, "assets");
Expand Down