morpho-org / metamorpho Goto Github PK
View Code? Open in Web Editor NEWMetaMorpho
Home Page: https://morpho.org
License: Other
MetaMorpho
Home Page: https://morpho.org
License: Other
it will make it easier to verify the deployment configuration, and also make it more accessible to external users to check it.
the downside is of course deployment gas, but it seems that vault operator will have to spend over time quite a lot of gas on operation, so idk
could be optimized
Originally posted by @MerlinEgalite in #31 (comment)
Idk id it's an issue or not
Issue is only valid if idle supply can be capped
It could ease vault manager's life you'll need to store somewhere the bytes32 only and not the whole market config.
Add a timelock to:
could be optimized
Originally posted by @MerlinEgalite in #31 (comment)
It seems that by being able to update the lastTotalAssets several times within a block the feeRecipient
can generate instant revenu not based on interest accrual.
Whereas we could bypass the accept
step when timelock is zero
I have some questions about the fee mechanism:
lastTotalAssets
, remove the fee recipient so that lastTotalAssets
is not updated for a while. Then, add a new fee recipient so that lastTotalAssets
is the totalInterest
generated is very (very) big. This needs to be clarified on my side but I think the fee recipient could basically steal users' funds.But what would meta-txs be useful for?
If size is limited, gas may be saved by saving queues as 32-bytes bit arrays
When first reading the code, I'd argue that understanding what functions named setParameter
are used for because in the case parameters require a pending submission first, these functions don't expect any input parameter
Moreover, it may be not very clear with other functions named alike for parameters that don't require a pending submission
Perhaps acceptParameter
is a better name
I think it is good for vault operators to have a getRiskManagers() and getAllocators() function. could help prevent keeping stale operators, which in the long run could have some security implications.
I am aware that technically vault managers can read the events, but it requires much more effort.
When withdrawing too much, 2 errors can be raised depending on how much is withdrawn:
withdraw order failed
ERC20: burn amount exceeds balance
We should harmonize this behavior.
How to redistribute tokens transferred to the vaults (claim on Blue, for example) to the users ?
The easiest way to redistribute tokens is to use a Merkle tree, with an offchain distribution
The Universal Rewards Distributor is using ERC20 allowance and a treasury transaction to redistribute the tokens.
The flow is as following
The vault has to accept the treasury. We can do it in the constructor or, if the vault is owned, by adding an owner function.
contract Vault {
address constant URD = 0x0000;
uint256 immutable distributionId;
constructor(uint256 _distributionId) {
distributionId = _distributionId;
}
/// can be called by everyone, the distributionId is immutable
function acceptTreasuryRole() public {
IURD(URD).acceptAsTreasury(distributionId);
}
}
Then, the vault has to set the allowance to the rewards distributor.
Usually, the vault must not contain any ERC20. So we can create this public function:
function setURDAllowance(address token) public {
uint256 balance = ERC20(token).balanceOf(address(this));
if(token == VAULT_UNDERLYING)
balance = balance - idle
ERC20(token).approve(URD, balance);
}
supply
does not conform to EIP4626 naming. deposit
does instead
OZ has contracts for access control management that we could use.
There's a DEFAULT_ADMIN_ROLE
(a sort of master admin) having the power to grant or revoke other roles (in our case RISK_MANAGER
and ALLOCATOR
).
Pros:
onlyRiskManager
and onlyAllocator
modifierssetIsRiskManager
and setIsAllocator
functionsisRiskManager
and isAllocator
from storageCons:
There's AccessControlDefaultAdminRules
adding a delay for changing DEFAULT_ADMIN_ROLE
. This can be interesting for legal reasons (to be discussed in a product call).
Can be helpful on etherscan
Following last product discussion, we'll remove supply and withdraw strategies. Instead, we'll have setters to set ordered list of whitelisted markets for the supply and withdraw side.
The allocator can still allocate arbitrarily the funds among the whitelisted markets.
Issue is only valid if idle can be capped
In totalAssets
we use balanceOf(address(this))
to get access to the idle balance of the vault.
Because of passed events we know that it can be an attack vector for integrators on top. Thus, I'd advocate for tracking the idle liquidity instead even if it implies losing some money when someone donates to the vault.
I tried to remove the remapping in morpho-blue but it still fails when running yarn test:hardhat
Currently: 2 days
Is it enough? 3 days
? 7 days
In the end, I don't think it's that important but I'd go for 3 days
Reference: morpho-org/morpho-blue#426
Could be easier to submit cap zero for infinite instead of having to submit 2 ** 128 - 1
This is not an issue to keep the current behavior but you could do the following thing: enable a market, disable it and reenable it before if it fits in the timelock
It's cumbersome to have to cast it to IERC20
instead of having it expected as an address
Whereas they could be declared at the top level to be used by integrators
Maybe 50?
Should not exceed 30 from interviews
I'd argue either submitParameter
or setPendingParameter
is enough, submitPendingParameter
is overwhelming
Having the invariant isRiskManager => isAllocator
would be more efficient (reading only from isAllocator
)
### Tasks
- [x] Deposit
- [x] Withdraw
- [x] Transfer
- [ ] Fees
Need to quantify how much
To prevent the inflation attack OZ added _decimalsOffset
that can be overriden. By default _decimalsOffset
is 0
Idle supply is not tracked yet but it's necessary for allocations and the URD
There is no constraint on the withdrawal queue pushed by the allocator, so in the case users withdraw all liquidity available from all markets of the withdrawal queue and from the idle supply, there may be available liquidity on markets that are not in the withdrawal queue... which makes the vault illiquid whereas it should be
A way to fix this is to check that all enabled markets are part of the withdrawal queue upon change
setCap
doesn't enforce the timelock
This would unlock 1 click move from MM V1 to V2
Should we name it MetaMorpho
?
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.