-
Notifications
You must be signed in to change notification settings - Fork 15
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
[VEN-2100]: Feat timestamp based contracts #324
Conversation
f22b237
to
f44e34c
Compare
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.
Thank you for this effort, it was probably quite tedious. I marked some comments with emojis so that you don't spend too much time resolving my "would be nice to have it this way" things 😅
[🤔] – some ideas and thoughts we could discuss, definitely not a blocker for the PR in any way
[🤓] – nerdy nitpicks you might want to address or ignore
I think we should move the TimeManager contract to https://github.com/VenusProtocol/solidity-utilities. I will be needing the contract for Prime also and we shouldn't import isolated-pools in venus-protocol repo. Also I think we will need different solidity versions of the contract if we are deploying some of the venus-protocol repo contracts in other chains. For example: XVSVault. |
Maybe the time has come for vault v0.8? |
As per my opinion, having a new v0.8 Vault will increase the new feature implementation overheads. I don't see any benefits of having two versions. Even if we create a new version of XVSVault we might still need old version of TimeManager because many other contracts may also need the older version. If there is any other major benefits of having a v0.8 XVSVault then I would vote for it. |
[VEN-2258]: replace the local TimeManager with the contracts imported from the solidity-utilities package
|
Description
Resolves #VEN-2100
This PR contains contracts based on Timestamp changes
Checklist