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.

Sign up for GitHub

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

Jump to bottom

Refactor interfaces #240

Merged
merged 7 commits into from
Oct 25, 2023
Merged

Refactor interfaces #240

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d7452be
refactor: split interfaces
QGarchery Oct 19, 2023
49e7092
refactor: rename MM interfaces
QGarchery Oct 20, 2023
2c1cd15
docs: dev comments for MM interfaces
QGarchery Oct 24, 2023
c563892
fix: casing of Multicall
QGarchery Oct 24, 2023
e9a0962
Merge remote-tracking branch 'origin/cantina-review' into refactor/in…
QGarchery Oct 25, 2023
a12c07c
Merge remote-tracking branch 'origin/cantina-review' into refactor/in…
QGarchery Oct 25, 2023
443fe2d
docs: specify signatures of functions
QGarchery Oct 25, 2023
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
24 changes: 8 additions & 16 deletions src/MetaMorpho.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,44 +452,40 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph
/* ERC4626 (PUBLIC) */

/// @inheritdoc IERC20Metadata
function decimals() public view override(IERC20Metadata, ERC20, ERC4626) returns (uint8) {
function decimals() public view override(ERC20, ERC4626) returns (uint8) {
return ERC4626.decimals();
}

/// @inheritdoc IERC4626
function maxWithdraw(address owner) public view override(IERC4626, ERC4626) returns (uint256 assets) {
function maxWithdraw(address owner) public view override returns (uint256 assets) {
(assets,,) = _maxWithdraw(owner);
}

/// @inheritdoc IERC4626
function maxRedeem(address owner) public view override(IERC4626, ERC4626) returns (uint256) {
function maxRedeem(address owner) public view override returns (uint256) {
(uint256 assets, uint256 newTotalSupply, uint256 newTotalAssets) = _maxWithdraw(owner);

return _convertToSharesWithFeeAccrued(assets, newTotalSupply, newTotalAssets, Math.Rounding.Floor);
}

/// @inheritdoc IERC4626
function deposit(uint256 assets, address receiver) public override(IERC4626, ERC4626) returns (uint256 shares) {
function deposit(uint256 assets, address receiver) public override returns (uint256 shares) {
uint256 newTotalAssets = _accrueFee();

shares = _convertToSharesWithFeeAccrued(assets, totalSupply(), newTotalAssets, Math.Rounding.Floor);
_deposit(_msgSender(), receiver, assets, shares);
}

/// @inheritdoc IERC4626
function mint(uint256 shares, address receiver) public override(IERC4626, ERC4626) returns (uint256 assets) {
function mint(uint256 shares, address receiver) public override returns (uint256 assets) {
uint256 newTotalAssets = _accrueFee();

assets = _convertToAssetsWithFeeAccrued(shares, totalSupply(), newTotalAssets, Math.Rounding.Ceil);
_deposit(_msgSender(), receiver, assets, shares);
}

/// @inheritdoc IERC4626
function withdraw(uint256 assets, address receiver, address owner)
public
override(IERC4626, ERC4626)
returns (uint256 shares)
{
function withdraw(uint256 assets, address receiver, address owner) public override returns (uint256 shares) {
uint256 newTotalAssets = _accrueFee();

// Do not call expensive `maxWithdraw` and optimistically withdraw assets.
Expand All @@ -499,11 +495,7 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph
}

/// @inheritdoc IERC4626
function redeem(uint256 shares, address receiver, address owner)
public
override(IERC4626, ERC4626)
returns (uint256 assets)
{
function redeem(uint256 shares, address receiver, address owner) public override returns (uint256 assets) {
uint256 newTotalAssets = _accrueFee();

// Do not call expensive `maxRedeem` and optimistically redeem shares.
Expand All @@ -513,7 +505,7 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph
}

/// @inheritdoc IERC4626
function totalAssets() public view override(IERC4626, ERC4626) returns (uint256 assets) {
function totalAssets() public view override returns (uint256 assets) {
for (uint256 i; i < withdrawQueue.length; ++i) {
assets += _supplyBalance(_marketParams(withdrawQueue[i]));
}
Expand Down
36 changes: 27 additions & 9 deletions src/interfaces/IMetaMorpho.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ pragma solidity >=0.5.0;

import {IMorpho, Id, MarketParams} from "@morpho-blue/interfaces/IMorpho.sol";
import {IERC4626} from "@openzeppelin/interfaces/IERC4626.sol";
import {IERC20Permit} from "@openzeppelin/token/ERC20/extensions/IERC20Permit.sol";

struct MarketConfig {
/// @notice The maximum amount of assets that can be allocated to the market.
Expand Down Expand Up @@ -35,7 +36,19 @@ struct MarketAllocation {
uint256 shares;
}

interface IMetaMorpho is IERC4626 {
interface IMultiCall {
QGarchery marked this conversation as resolved.
Show resolved Hide resolved
function multicall(bytes[] calldata) external returns (bytes[] memory);
}

interface IOwnable {
QGarchery marked this conversation as resolved.
Show resolved Hide resolved
function owner() external returns (address);
function transferOwnership(address) external;
function renounceOwnership() external;
function acceptOwnership() external;
function pendingOwner() external view returns (address);
}

interface IMetaMorphoSpecific {
function MORPHO() external view returns (IMorpho);

function curator() external view returns (address);
Expand All @@ -50,29 +63,24 @@ interface IMetaMorpho is IERC4626 {
function supplyQueueSize() external view returns (uint256);
function withdrawQueue(uint256) external view returns (Id);
function withdrawQueueSize() external view returns (uint256);
function config(Id) external view returns (uint192 cap, uint64 withdrawRank);

function idle() external view returns (uint256);
function lastTotalAssets() external view returns (uint256);

function submitTimelock(uint256 newTimelock) external;
function acceptTimelock() external;
function revokeTimelock() external;
function pendingTimelock() external view returns (uint192 value, uint64 submittedAt);

function submitCap(MarketParams memory marketParams, uint256 marketCap) external;
function acceptCap(Id id) external;
function revokeCap(Id id) external;
function pendingCap(Id) external view returns (uint192 value, uint64 submittedAt);

function submitFee(uint256 newFee) external;
function acceptFee() external;
function pendingFee() external view returns (uint192 value, uint64 submittedAt);

function submitGuardian(address newGuardian) external;
function acceptGuardian() external;
function revokeGuardian() external;
function pendingGuardian() external view returns (address guardian, uint96 submittedAt);

function transferRewards(address) external;

Expand All @@ -86,8 +94,18 @@ interface IMetaMorpho is IERC4626 {
function reallocate(MarketAllocation[] calldata withdrawn, MarketAllocation[] calldata supplied) external;
}

interface IPending {
function pendingTimelock() external view returns (PendingUint192 memory);
function pendingCap(Id) external view returns (PendingUint192 memory);
interface IMetaMorpho is IMetaMorphoSpecific {
QGarchery marked this conversation as resolved.
Show resolved Hide resolved
function config(Id) external view returns (uint192 cap, uint64 withdrawRank);
function pendingGuardian() external view returns (address guardian, uint96 submittedAt);
function pendingCap(Id) external view returns (uint192 value, uint64 submittedAt);
function pendingTimelock() external view returns (uint192 value, uint64 submittedAt);
function pendingFee() external view returns (uint192 value, uint64 submittedAt);
}

interface IMetaMorphoFull is IMetaMorphoSpecific, IERC4626, IERC20Permit, IOwnable, IMultiCall {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But then IMetaMorpho does not inherit IERC4626 and thus the documentation won't appear in etherscan because the following comments won't work: /// @inheritdoc IERC4626

Copy link
Contributor Author

@QGarchery QGarchery Oct 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it works because MetaMorpho inherits from ERC4626 which itself inherits from IERC4626 (btw this is one other advantage of this refactor: function signatures are imported only once, which is clearer). In this test PR, you can see that if you remove is ICounterGetNumber you get an error in the inheritdoc comment, which means that the inheritdoc comments are aware of inherited interfaces of inherited contracts

function config(Id) external view returns (MarketConfig memory);
function pendingGuardian() external view returns (PendingAddress memory);
function pendingCap(Id) external view returns (PendingUint192 memory);
function pendingTimelock() external view returns (PendingUint192 memory);
function pendingFee() external view returns (PendingUint192 memory);
}
30 changes: 15 additions & 15 deletions test/forge/GuardianTest.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,16 @@ contract GuardianTest is IntegrationTest {
vm.warp(block.timestamp + elapsed);

vm.expectEmit();
emit EventsLib.RevokeTimelock(GUARDIAN, IPending(address(vault)).pendingTimelock());
emit EventsLib.RevokeTimelock(GUARDIAN, vault.pendingTimelock());
vm.prank(GUARDIAN);
vault.revokeTimelock();

uint256 newTimelock = vault.timelock();
(uint256 pendingTimelock, uint64 submittedAt) = vault.pendingTimelock();
PendingUint192 memory pendingTimelock = vault.pendingTimelock();

assertEq(newTimelock, TIMELOCK, "newTimelock");
assertEq(pendingTimelock, 0, "pendingTimelock");
assertEq(submittedAt, 0, "submittedAt");
assertEq(pendingTimelock.value, 0, "pendingTimelock.value");
assertEq(pendingTimelock.submittedAt, 0, "pendingTimelock.submittedAt");
}

function testRevokeCapIncreased(uint256 seed, uint256 cap, uint256 elapsed) public {
Expand All @@ -63,17 +63,17 @@ contract GuardianTest is IntegrationTest {
Id id = marketParams.id();

vm.expectEmit();
emit EventsLib.RevokeCap(GUARDIAN, id, IPending(address(vault)).pendingCap(id));
emit EventsLib.RevokeCap(GUARDIAN, id, vault.pendingCap(id));
vm.prank(GUARDIAN);
vault.revokeCap(id);

(uint192 newCap, uint64 withdrawRank) = vault.config(id);
(uint256 pendingCap, uint64 submittedAt) = vault.pendingCap(id);
MarketConfig memory marketConfig = vault.config(id);
PendingUint192 memory pendingCap = vault.pendingCap(id);

assertEq(newCap, 0, "newCap");
assertEq(withdrawRank, 0, "withdrawRank");
assertEq(pendingCap, 0, "pendingCap");
assertEq(submittedAt, 0, "submittedAt");
assertEq(marketConfig.cap, 0, "marketConfig.cap");
assertEq(marketConfig.withdrawRank, 0, "marketConfig.withdrawRank");
assertEq(pendingCap.value, 0, "pendingCap.value");
assertEq(pendingCap.submittedAt, 0, "pendingCap.submittedAt");
}

function testRevokeGuardian(uint256 elapsed) public {
Expand All @@ -87,15 +87,15 @@ contract GuardianTest is IntegrationTest {
vm.warp(block.timestamp + elapsed);

vm.expectEmit();
emit EventsLib.RevokeGuardian(GUARDIAN, IPending(address(vault)).pendingGuardian());
emit EventsLib.RevokeGuardian(GUARDIAN, vault.pendingGuardian());
vm.prank(GUARDIAN);
vault.revokeGuardian();

address newGuardian = vault.guardian();
(address pendingGuardian, uint96 submittedAt) = vault.pendingGuardian();
PendingAddress memory pendingGuardian = vault.pendingGuardian();

assertEq(newGuardian, GUARDIAN, "newGuardian");
assertEq(pendingGuardian, address(0), "pendingGuardian");
assertEq(submittedAt, 0, "submittedAt");
assertEq(pendingGuardian.value, address(0), "pendingGuardian.value");
assertEq(pendingGuardian.submittedAt, 0, "pendingGuardian.submittedAt");
}
}
Loading