-
Notifications
You must be signed in to change notification settings - Fork 224
feat: vault hub solvency tests #1028
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
base: feat/vaults-fuzzing
Are you sure you want to change the base?
Conversation
Hardhat Unit Tests Coverage Summary
Diff against master
Results for commit: 9e7f14a Minimum allowed coverage is ♻️ This comment has been updated with latest results |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👀 🔥 💪
depositContract = new DepositContract__MockForStakingVault(); | ||
|
||
lido = new LidoMock(7810237 * 10 ** 18, 9365361 * 10 ** 18, 0); | ||
LidoLocatorMock lidoLocator = new LidoLocatorMock(depositor, accounting, treasury); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need a mock here? Can't we use the real one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The real object includes many members and extra methods, but we only need three of them, so using a mock here is, in my view, the simplest solution.
|
||
uint256 valuationThreshold = (_stakingVault.valuation() * (TOTAL_BASIS_POINTS - socket.rebalanceThresholdBP)) / | ||
TOTAL_BASIS_POINTS; | ||
if (valuationThreshold < lido.getPooledEthByShares(socket.sharesMinted)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to allow it to go to the unhealthy state and then, on the next interaction, force rebalance and disconnect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't do this because forceRebalance function reverts if vault is in a bad dept state
console2.log("Total shares burned", totalSharesBurned); | ||
console2.log("Shares leftover", sharesLeftover); | ||
|
||
assertEq(totalSharesMinted, totalSharesBurned + sharesLeftover); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 maybe assert vault valuation is enough to cover minted shares
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, it looks solid — great job!
A short summary of the changes.
Context
Create solvency tests for VaultHub
Problem
The VaultHub with connected vaults has a complex state. Solvency tests can help to find issues (if they exist) by repeating many operations with random parameteres.
Solution
These tests create and connect Vault to the VaultHub in a random time and then call mint/burn/rebalance/receive_rewards/receive_penalty operations on them. At the same time simulate core protocol staking and rewards mechanism.