safe-global / safe-smart-account / 10010897109

94%

push

github

Generic migration contract (#793)

Fixes: #787 

## Summary

This PR adds a general migration contract that takes address of the
Safe, SafeL2 and fallback handler contracts during deployment. The
contract allows Safe to update the Singleton at `address(0)`.

- Uses error strings rather than error types because Solidity version
0.7.6 doesn't support it.

As of now tests cover below migration paths: 
- 1.3.0 to 1.5.0
- 1.3.0 to 1.4.1
- 14.1 to 1.5.0

See `SafeMigration.spec.ts` to see how tests are organised. Do share if
you any thoughts to better run same tests on different migration paths.

The migration contract stores address of the Safe singletons and
fallback handler rather than using code hash and requiring the user to
provide singleton address as described in the issue. The reason being as
follows:
 
Checking codehash of the target singleton means user has to provide the
address of the target singleton. Also, checking code hash has higher gas
costs.

The only argument for using code hash for upgrades is that it also
allows unofficial singletons to be used for migration using official
migration contract. But, users/projects can also deploy their own
version of migration contract by providing singleton addresses in the
constructor and have similar security guarantees as the official
migration contract.

## Changes in PR
- Create a general migration contract which is not tightly bound to any
specific Safe version
- Update tests
- Remove other migration contract as this PR supersedes it
 
Unlike `Safe150Migration.sol`, this new contract does not check if
slot(0) of the contract stores an address having some non-empty code. I
think this check is not need because this contract is not intended to be
used in general by other proxy contracts and checking slot(0) value is
only a partially correct way. Would like to know thought of others.

---------

Co-authored-by: Nicholas Rodrigues Lordello <n@lordello.n... (continued)

259 of 292 branches covered (88.7%)

Branch coverage included in aggregate %.

20 of 20 new or added lines in 1 file covered. (100.0%)

386 of 398 relevant lines covered (96.98%)

102.98 hits per line