Modernize codebase Part 2 #297
Conversation
Many tests were broken and were only accidentally passing because of rampant misuse of global assignments and test inter-dependencies
This test ensures slot positions are retained post-upgrade by manually checking the use of the slots at expected positions.
test/v1/ProxyNegativeTests.js
Outdated
| @@ -96,13 +96,6 @@ async function run_tests(newToken, _accounts) { | |||
| await checkVariables([token], [customVars]); | |||
| }); | |||
|
|
|||
| it("nut007 should fail to call proxy function with non-adminAccount", async () => { | |||
There was a problem hiding this comment.
There was no point in putting ifAdmin modifier for admin() and implementation(). Besides, those were marked as view functions, but if a non-admin invokes those functions, the modifier could cause a delegate call to the functions with the same name in the implementation contract which may not be read-only (i.e. not view). New versions of Solidity compiler correctly catches this and errors out. The implementation contract should also never contain meaningful admin() and implementation() defined that relies on the modifier to be callable.
| require(msg.sender == blacklister); | ||
| require( | ||
| msg.sender == blacklister, | ||
| "Blacklistable: caller is not the blacklister" |
There was a problem hiding this comment.
The format and wording of the error messages follow the convention in OpenZeppelin code.
|
Closing in favor of merging with previous PR #296 |
Requires #296.