Closed i-stam closed 4 years ago
I would assume this upgradable proxy could be used with the 'library' approach that we spoke about few days ago - and could also be simplified - since at the moment we are adding more contracts on top of the already complex wallet contract.
If there are test that are missing, we should add them , feel free to suggest. All the previous tests are running successfully and just the basic tests for the upgradeable contracts are added. But this was supposed to be fast so we can send it to the auditor, we can add more tests in the meanwhile.
P.S. Can we make the
externals/upgradeability
directory calledexternals/upgradable
or something cleaner/simpler? I think there is no problem there, we can change the name to something else. This is actually the name that openZeppeling uses so I kept it the same as we do for all the external contracts
@i-stam the other thing which I would like to get in in this PR is a link to the commit which we took the the open-zepplin upgradebale. Would be good to have the git commit captured
I reviewed the latest changes, I think it's good that we remove the unnecessary code from the proxy.
Uses the
upgradeability
package offered by openZeppelinUses one external
initialize()
function for the wallet with the appropriateinitializer
modifier, the rest of theinitialize()
functions are internal and manual care should be taken for invoking them.The rest of the contract code was left untouched (initializing contract parameters e.g. ENS nodes, should be done in a separate PR)
The openZeppelin contracts are considered safe by default and no additional testing is performed. We just tested if they were initialized and used properly and slightly modified them to allow the admin (aka wallet owner) to delegate calls. Repo commit hash:9baca3afb5649b6defc3a75eeb69f4930852180f (https://github.com/OpenZeppelin/openzeppelin-sdk/commit/9baca3afb5649b6defc3a75eeb69f4930852180f)