-
Notifications
You must be signed in to change notification settings - Fork 7
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
Feature: ynBNB mainnet #4
Conversation
src/SingleVault.sol
Outdated
} from "src/Common.sol"; | ||
|
||
import {ISingleVault} from "src/ISingleVault.sol"; | ||
import {SlisBnbModule} from "src/module/SlisBnbModule.sol"; |
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.
This should be functionally equivalent to having the code in contract as before.
The library is deployed external so it doesn't increase the bytecode of the contract, and in theory can be called by other contracts
But must be noted to upgrade the logic in the module we still need to upgrade the whole implementation of the SingleVault.
Also because of how libraries work in solidity, there's no way to have a library interface so it makes this contract import the concrete library so SingleVault can't be generic (as opposed to it depending on a generic IBnbModule contract)
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.
Yeah, deploying a new SingleVault which has an updated SlisBnbModule will require redeploying the updated SlisBnbModule. But this should all be handled by foundry once you deploy the new SingleVault implementation (which imports the new SlisBnbModule), after which you upgrade the proxy to point to this new impl.
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.
Yes, agree. Thanks for this review. I've reverted efforts to make the SingleVault generic and will just deploy the basic functionality needed for ynBNB now.
src/module/SlisBnbModule.sol
Outdated
*/ | ||
function _retrieveERC4626Storage() internal pure returns (ERC4626Storage storage $) { | ||
assembly { | ||
$.slot := 0x0773e532dfede91f04b12a73d3d2acd361424f41f76b4fb79f090161e36b4e00 |
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.
nice find here - this is actually the only way to access storage inside a library without passing a storage reference
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.
I like how Libraries are pure and consume storage from specified slots like this. I'll come back to this research after deploying the base version. Again, I've reverted back to the basic implementation we had previously and will focus on how to generalize later.
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.
Makes sense to keep it minimal for now but good that you explored this direction
381fe45
to
40fd735
Compare
add fuzz test to verify deposit vs withdrawal
This adds a deployment of the vault factory to BSC mainnet with some minor updates to constants, contract addresses, and actors with extra basic tests.
NOTES
This has two contracts, Factory and SingleVault. Vaults are deployed via the factory, and require
1 ether
of asset sent to the factory, which is used to bootstrap deposits into the newly created vault.SingleVault
has TimelockController Admin capabilities that are Handled via theYnBscSecurityCouncil
multisig:https://app.safe.global/home?safe=bnb:0x721688652DEa9Cabec70BD99411EAEAB9485d436
ROADMAP
ynBNB
as a SingleVault withslisBNB
as the underlying asset. Full deposits and withdraws are capable, but we must not deposit into the Karak Vault until we update Karak withdraws.