-
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
Added transfer of RAD tokens to ScopeLift upon successful upgrade to governor Bravo #19
Conversation
Current dependencies on/for this PR:
This stack of pull requests is managed by Graphite. |
test/RadworksGovernor.t.sol
Outdated
uint256 _timelockTokenBalance = _token.balanceOf(TIMELOCK); | ||
|
||
// bound by the number of tokens the timelock currently controls | ||
_amount = bound(_amount, 0, _timelockTokenBalance); | ||
uint256 _initialTokenBalance = _token.balanceOf(_receiver); | ||
|
||
_upgradeToBravoGovernor(); |
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.
It may better to keep this test as is and assume
the token does not equal the rad token, and create a new test specifically for the RAD token which subtracts the expected ScopeLift amount from the _timelockTokenBalance
. In this setup a test will fail if an unexpected amount of tokens is transferred during the upgrade where as I think this will not.
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.
Good idea! Thanks!
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.
Done: 1bdb4f1
b2af40e
to
354cfbc
Compare
1bdb4f1
to
f25f7d4
Compare
354cfbc
to
bf76141
Compare
f25f7d4
to
4d030e5
Compare
bf76141
to
fe8a3aa
Compare
1b6cb61
to
eec664b
Compare
eec664b
to
689b261
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.
LGTM, as long as the build is passing
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.
a bunch of nits, otherwise lgtm, thanks @jferas
This PR implements a transfer of RAD tokens to ScopeLift as part of the transaction that upgrades to Governor Bravo.
Resolves #17