-
Notifications
You must be signed in to change notification settings - Fork 0
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
F: Adds versions and upgrade tests to the contracts #54
base: develop
Are you sure you want to change the base?
Conversation
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.
Let's change the link and think about if it's worth it having a different setup rather than the same
|
||
# Notes from the Halborn Audit | ||
|
||
https://www.halborn.com/portal/reports/ve-governance-updates |
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.
Externals can't get this link. Let's change it directly to the one we are uploading in this PR.
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.
thanks will fix
f: adds support for forge 1.0.0 in test cases
F/regression test
Poc/versioned tests
// voter.upgradeTo(address(voterV1_1_0)); | ||
|
||
// safe upgrade | ||
_safeUpgradeVoter(address(voter)); |
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'd still use upgradeTo
instead of upgradeProxy
helper. The reason being is in the actual upgrade, we don't use upgradeProxy
. To keep things as close as possible, both the tests and the actual upgrade must follow the same flow.
Storage compatibility can still be tested without upgradeProxy
.
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.
that's a fair point. Ideally we'd use a single upgrade factory
No description provided.