Skip to content
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

1.4.1 Feature: Backport migration contracts to 1.4.1 #795

Merged
merged 4 commits into from
Jul 23, 2024

Commits on Jul 22, 2024

  1. Generic migration contract (#793)

    Fixes: #787
    
    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.
    
    - 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.net>
    Co-authored-by: Shebin John <admin@remedcu.com>
    Co-authored-by: Mikhail <16622558+mmv08@users.noreply.github.com>
    4 people committed Jul 22, 2024
    Configuration menu
    Copy the full SHA
    d6b9753 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    dc43b09 View commit details
    Browse the repository at this point in the history
  3. Add contract to migrate a Safe from not L2 to L2 (#685)

    * Add contract to migrate a Safe from not L2 to L2
    * Related to safe-global/safe-transaction-service#1703
    Uxio0 authored and mmv08 committed Jul 22, 2024
    Configuration menu
    Copy the full SHA
    8e28148 View commit details
    Browse the repository at this point in the history
  4. Configuration menu
    Copy the full SHA
    f1b87d7 View commit details
    Browse the repository at this point in the history