-
Notifications
You must be signed in to change notification settings - Fork 973
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
Add contract to migrate a Safe from not L2 to L2 #685
Merged
Merged
Changes from 8 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
07c8a93
Add contract to migrate a Safe from not L2 to L2
Uxio0 3ac8ea6
Fix linting
Uxio0 00b1369
Add tests
Uxio0 03bd68d
Remove dead code
Uxio0 c1abd94
Update contracts/libraries/SafeToL2Migration.sol
Uxio0 036adfd
Adding docs
Uxio0 e85b404
Fix typo
Uxio0 9fd1b87
Add test for migrating from v1.4.1 to v1.4.1 L2
Uxio0 00ab5ea
Add support to migrate from v1.1.1
Uxio0 1417a08
Fix typo
Uxio0 7885971
Add modifier only nonce zero
Uxio0 7bef949
Add script to deploy L2 migration
Uxio0 58ca94e
Rename deploy migrations script
Uxio0 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,96 @@ | ||
// SPDX-License-Identifier: LGPL-3.0-only | ||
/* solhint-disable one-contract-per-file */ | ||
pragma solidity >=0.7.0 <0.9.0; | ||
|
||
import {SafeStorage} from "../libraries/SafeStorage.sol"; | ||
import {Enum} from "../common/Enum.sol"; | ||
|
||
interface ISafe { | ||
// solhint-disable-next-line | ||
function VERSION() external view returns (string memory); | ||
} | ||
|
||
/** | ||
* @title Migration Contract for updating a Safe from 1.3.0/1.4.1 version to a L2 version. Useful when replaying a Safe from a non L2 network in a L2 network. | ||
* @notice This contract facilitates the migration of a Safe contract from version 1.3.0 to 1.3.0L2 or from 1.4.1 to 1.4.1L2 | ||
* Older versions are not supported | ||
* @dev IMPORTANT: The migration will only work with proxies that store the implementation address in the storage slot 0. | ||
*/ | ||
contract SafeToL2Migration is SafeStorage { | ||
// Address of this contract | ||
address public immutable MIGRATION_SINGLETON; | ||
|
||
/** | ||
* @notice Constructor | ||
* @dev Initializes the migrationSingleton with the contract's own address. | ||
*/ | ||
constructor() { | ||
MIGRATION_SINGLETON = address(this); | ||
} | ||
|
||
/** | ||
* @notice Event indicating a change of master copy address. | ||
* @param singleton New master copy address | ||
*/ | ||
event ChangedMasterCopy(address singleton); | ||
|
||
event SafeMultiSigTransaction( | ||
address to, | ||
uint256 value, | ||
bytes data, | ||
Enum.Operation operation, | ||
uint256 safeTxGas, | ||
uint256 baseGas, | ||
uint256 gasPrice, | ||
address gasToken, | ||
address payable refundReceiver, | ||
bytes signatures, | ||
// We combine nonce, sender and threshold into one to avoid stack too deep | ||
// Dev note: additionalInfo should not contain `bytes`, as this complicates decoding | ||
bytes additionalInfo | ||
); | ||
|
||
/** | ||
* @notice Migrate from Safe 1.3.0/1.4.1 Singleton (L1) to the same version provided L2 singleton | ||
* Safe is required to have nonce 0 so backend can support it after the migration | ||
* @dev This function should only be called via a delegatecall to perform the upgrade. | ||
* Singletons versions will be compared, so it implies that contracts exist | ||
*/ | ||
function migrateToL2(address l2Singleton) public { | ||
require(address(this) != MIGRATION_SINGLETON, "Migration should only be called via delegatecall"); | ||
require(address(singleton) != l2Singleton, "Safe is already using the singleton"); | ||
// Nonce is increased before executing a tx, so first executed tx will have nonce=1 | ||
require(nonce == 1, "Safe must have not executed any tx"); | ||
bytes32 oldSingletonVersion = keccak256(abi.encodePacked(ISafe(singleton).VERSION())); | ||
mmv08 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
bytes32 newSingletonVersion = keccak256(abi.encodePacked(ISafe(l2Singleton).VERSION())); | ||
|
||
require(oldSingletonVersion == newSingletonVersion, "L2 singleton must match current version singleton"); | ||
// There's no way to make sure if address is a valid singleton, unless we cofigure the contract for every chain | ||
require( | ||
newSingletonVersion == keccak256(abi.encodePacked("1.3.0")) || newSingletonVersion == keccak256(abi.encodePacked("1.4.1")), | ||
"Provided singleton version is not supported" | ||
); | ||
|
||
singleton = l2Singleton; | ||
|
||
// Simulate a L2 transaction so indexer picks up the Safe | ||
// 0xef2624ae - keccak("migrateToL2(address)") | ||
bytes memory data = abi.encodeWithSelector(0xef2624ae, l2Singleton); | ||
// nonce, sender, threshold | ||
bytes memory additionalInfo = abi.encode(nonce - 1, msg.sender, threshold); | ||
emit SafeMultiSigTransaction( | ||
mmv08 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
MIGRATION_SINGLETON, | ||
0, | ||
data, | ||
Enum.Operation.DelegateCall, | ||
0, | ||
0, | ||
0, | ||
address(0), | ||
address(0), | ||
"", // We cannot detect signatures | ||
additionalInfo | ||
); | ||
emit ChangedMasterCopy(singleton); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,256 @@ | ||
import { expect } from "chai"; | ||
import hre, { ethers, deployments } from "hardhat"; | ||
import { AddressZero } from "@ethersproject/constants"; | ||
import { getSafeWithSingleton, getSafeSingletonAt, getMock } from "../utils/setup"; | ||
import deploymentData from "../json/safeDeployment.json"; | ||
import safeRuntimeBytecode from "../json/safeRuntimeBytecode.json"; | ||
import { buildSafeTransaction, executeContractCallWithSigners, executeTxWithSigners } from "../../src/utils/execution"; | ||
|
||
const SAFE_SINGLETON_141_ADDRESS = "0x3E5c63644E683549055b9Be8653de26E0B4CD36E"; | ||
|
||
const SAFE_SINGLETON_141_L2_ADDRESS = "0xfb1bffC9d739B8D520DaF37dF666da4C687191EA"; | ||
|
||
const SAFE_SINGLETON_150_L2_ADDRESS = "0x0Ee37514644683f7EB9745a5726C722DeBa77e52"; | ||
|
||
const FALLBACK_HANDLER_STORAGE_SLOT = "0x6c9a6c4a39284e37ed1cf53d337577d14212a4870fb976a4366c693b939918d5"; | ||
|
||
const GUARD_STORAGE_SLOT = "0x4a204f620c8c5ccdca3fd54d003badd85ba500436a431f0cbda4f558c93c34c8"; | ||
|
||
describe("SafeToL2Migration library", () => { | ||
const migratedInterface = new ethers.Interface(["function masterCopy() view returns(address)"]); | ||
|
||
const setupTests = deployments.createFixture(async ({ deployments }) => { | ||
await deployments.fixture(); | ||
|
||
// Set the runtime code for hardcoded addresses, so the expected events are emitted | ||
await hre.network.provider.send("hardhat_setCode", [SAFE_SINGLETON_141_ADDRESS, safeRuntimeBytecode.safe141]); | ||
await hre.network.provider.send("hardhat_setCode", [SAFE_SINGLETON_141_L2_ADDRESS, safeRuntimeBytecode.safe141l2]); | ||
await hre.network.provider.send("hardhat_setCode", [SAFE_SINGLETON_150_L2_ADDRESS, safeRuntimeBytecode.safe150l2]); | ||
|
||
const signers = await ethers.getSigners(); | ||
const [user1] = signers; | ||
const singleton130Address = (await (await user1.sendTransaction({ data: deploymentData.safe130 })).wait())?.contractAddress; | ||
const singleton130L2Address = (await (await user1.sendTransaction({ data: deploymentData.safe130l2 })).wait())?.contractAddress; | ||
|
||
if (!singleton130Address || !singleton130L2Address) { | ||
throw new Error("Could not deploy Safe130 or Safe130L2"); | ||
} | ||
const singleton130 = await getSafeSingletonAt(singleton130Address); | ||
const singleton141 = await getSafeSingletonAt(SAFE_SINGLETON_141_ADDRESS); | ||
|
||
const guardContract = await hre.ethers.getContractAt("Guard", AddressZero); | ||
const guardEip165Calldata = guardContract.interface.encodeFunctionData("supportsInterface", ["0x945b8148"]); | ||
const validGuardMock = await getMock(); | ||
await validGuardMock.givenCalldataReturnBool(guardEip165Calldata, true); | ||
|
||
const invalidGuardMock = await getMock(); | ||
await invalidGuardMock.givenCalldataReturnBool(guardEip165Calldata, false); | ||
|
||
const safeWith1967Proxy = await getSafeSingletonAt( | ||
await hre.ethers | ||
.getContractFactory("UpgradeableProxy") | ||
.then((factory) => | ||
factory.deploy( | ||
singleton130Address, | ||
singleton130.interface.encodeFunctionData("setup", [ | ||
[user1.address], | ||
1, | ||
AddressZero, | ||
"0x", | ||
AddressZero, | ||
AddressZero, | ||
0, | ||
AddressZero, | ||
]), | ||
), | ||
) | ||
.then((proxy) => proxy.getAddress()), | ||
); | ||
const safeToL2MigrationContract = await hre.ethers.getContractFactory("SafeToL2Migration"); | ||
const migration = await safeToL2MigrationContract.deploy(); | ||
return { | ||
safe130: await getSafeWithSingleton(singleton130, [user1.address]), | ||
safe141: await getSafeWithSingleton(singleton141, [user1.address]), | ||
safeWith1967Proxy, | ||
migration, | ||
signers, | ||
validGuardMock, | ||
invalidGuardMock, | ||
singleton130Address, | ||
singleton130L2Address, | ||
}; | ||
}); | ||
|
||
describe("migrateToL2", () => { | ||
it("reverts if the singleton is not set", async () => { | ||
const { | ||
migration, | ||
safeWith1967Proxy, | ||
signers: [user1], | ||
singleton130L2Address, | ||
} = await setupTests(); | ||
|
||
await expect( | ||
executeContractCallWithSigners(safeWith1967Proxy, migration, "migrateToL2", [singleton130L2Address], [user1], true), | ||
).to.be.revertedWith("GS013"); | ||
}); | ||
|
||
it("reverts if new singleton is the same as the old one", async () => { | ||
const { | ||
safe130, | ||
migration, | ||
signers: [user1], | ||
singleton130Address, | ||
} = await setupTests(); | ||
await expect( | ||
executeContractCallWithSigners(safe130, migration, "migrateToL2", [singleton130Address], [user1], true), | ||
).to.be.revertedWith("GS013"); | ||
}); | ||
|
||
it("reverts if new singleton is not supported", async () => { | ||
const { | ||
safe130, | ||
migration, | ||
signers: [user1], | ||
} = await setupTests(); | ||
await expect( | ||
executeContractCallWithSigners(safe130, migration, "migrateToL2", [SAFE_SINGLETON_150_L2_ADDRESS], [user1], true), | ||
).to.be.revertedWith("GS013"); | ||
}); | ||
|
||
it("reverts if nonce > 0", async () => { | ||
const { | ||
safe130, | ||
migration, | ||
signers: [user1], | ||
singleton130Address, | ||
singleton130L2Address, | ||
} = await setupTests(); | ||
const safeAddress = await safe130.getAddress(); | ||
expect(await safe130.nonce()).to.be.eq(0); | ||
|
||
// Increase nonce by sending eth | ||
await user1.sendTransaction({ to: safeAddress, value: ethers.parseEther("1") }); | ||
const nonce = 0; | ||
const safeTx = buildSafeTransaction({ to: user1.address, value: ethers.parseEther("1"), nonce }); | ||
await executeTxWithSigners(safe130, safeTx, [user1]); | ||
|
||
expect(await safe130.nonce()).to.be.eq(1); | ||
await expect( | ||
executeContractCallWithSigners(safe130, migration, "migrateToL2", [singleton130L2Address], [user1], true), | ||
).to.be.revertedWith("GS013"); | ||
|
||
const singletonResp = await user1.call({ to: safeAddress, data: migratedInterface.encodeFunctionData("masterCopy") }); | ||
expect(migratedInterface.decodeFunctionResult("masterCopy", singletonResp)[0]).to.eq(singleton130Address); | ||
}); | ||
|
||
it("migrates from singleton 1.3.0 to 1.3.0L2", async () => { | ||
const { | ||
safe130, | ||
migration, | ||
signers: [user1], | ||
singleton130L2Address, | ||
} = await setupTests(); | ||
const safeAddress = await safe130.getAddress(); | ||
// The emit matcher checks the address, which is the Safe as delegatecall is used | ||
const migrationSafe = migration.attach(safeAddress); | ||
const migrationAddress = await migration.getAddress(); | ||
|
||
const functionName = "migrateToL2"; | ||
const expectedData = migration.interface.encodeFunctionData(functionName, [singleton130L2Address]); | ||
const safeThreshold = await safe130.getThreshold(); | ||
const additionalInfo = hre.ethers.AbiCoder.defaultAbiCoder().encode( | ||
["uint256", "address", "uint256"], | ||
[0, user1.address, safeThreshold], | ||
); | ||
await expect(executeContractCallWithSigners(safe130, migration, functionName, [singleton130L2Address], [user1], true)) | ||
.to.emit(migrationSafe, "ChangedMasterCopy") | ||
.withArgs(singleton130L2Address) | ||
.to.emit(migrationSafe, "SafeMultiSigTransaction") | ||
.withArgs( | ||
migrationAddress, | ||
0, | ||
expectedData, | ||
1, | ||
0, | ||
0, | ||
0, | ||
AddressZero, | ||
AddressZero, | ||
"0x", // We cannot detect signatures | ||
additionalInfo, | ||
); | ||
|
||
const singletonResp = await user1.call({ to: safeAddress, data: migratedInterface.encodeFunctionData("masterCopy") }); | ||
expect(migratedInterface.decodeFunctionResult("masterCopy", singletonResp)[0]).to.eq(singleton130L2Address); | ||
expect(await safe130.nonce()).to.be.eq(1); | ||
}); | ||
|
||
it("migrates from singleton 1.4.1 to 1.4.1L2", async () => { | ||
const { | ||
safe141, | ||
migration, | ||
signers: [user1], | ||
} = await setupTests(); | ||
const safeAddress = await safe141.getAddress(); | ||
// The emit matcher checks the address, which is the Safe as delegatecall is used | ||
const migrationSafe = migration.attach(safeAddress); | ||
const migrationAddress = await migration.getAddress(); | ||
|
||
const functionName = "migrateToL2"; | ||
const expectedData = migration.interface.encodeFunctionData(functionName, [SAFE_SINGLETON_141_L2_ADDRESS]); | ||
const safeThreshold = await safe141.getThreshold(); | ||
const additionalInfo = hre.ethers.AbiCoder.defaultAbiCoder().encode( | ||
["uint256", "address", "uint256"], | ||
[0, user1.address, safeThreshold], | ||
); | ||
await expect(executeContractCallWithSigners(safe141, migration, functionName, [SAFE_SINGLETON_141_L2_ADDRESS], [user1], true)) | ||
.to.emit(migrationSafe, "ChangedMasterCopy") | ||
.withArgs(SAFE_SINGLETON_141_L2_ADDRESS) | ||
.to.emit(migrationSafe, "SafeMultiSigTransaction") | ||
.withArgs( | ||
migrationAddress, | ||
0, | ||
expectedData, | ||
1, | ||
0, | ||
0, | ||
0, | ||
AddressZero, | ||
AddressZero, | ||
"0x", // We cannot detect signatures | ||
additionalInfo, | ||
); | ||
|
||
const singletonResp = await user1.call({ to: safeAddress, data: migratedInterface.encodeFunctionData("masterCopy") }); | ||
expect(migratedInterface.decodeFunctionResult("masterCopy", singletonResp)[0]).to.eq(SAFE_SINGLETON_141_L2_ADDRESS); | ||
expect(await safe141.nonce()).to.be.eq(1); | ||
}); | ||
|
||
it("doesn't touch important storage slots", async () => { | ||
const { | ||
safe130, | ||
migration, | ||
signers: [user1], | ||
singleton130L2Address, | ||
} = await setupTests(); | ||
const safeAddress = await safe130.getAddress(); | ||
|
||
const ownerCountBeforeMigration = await hre.ethers.provider.getStorage(safeAddress, 3); | ||
const thresholdBeforeMigration = await hre.ethers.provider.getStorage(safeAddress, 4); | ||
const nonceBeforeMigration = await hre.ethers.provider.getStorage(safeAddress, 5); | ||
const guardBeforeMigration = await hre.ethers.provider.getStorage(safeAddress, GUARD_STORAGE_SLOT); | ||
const fallbackHandlerBeforeMigration = await hre.ethers.provider.getStorage(safeAddress, FALLBACK_HANDLER_STORAGE_SLOT); | ||
|
||
await expect(executeContractCallWithSigners(safe130, migration, "migrateToL2", [singleton130L2Address], [user1], true)); | ||
|
||
expect(await hre.ethers.provider.getStorage(safeAddress, 3)).to.be.eq(ownerCountBeforeMigration); | ||
expect(await hre.ethers.provider.getStorage(safeAddress, 4)).to.be.eq(thresholdBeforeMigration); | ||
expect(await hre.ethers.provider.getStorage(safeAddress, 5)).to.be.eq(nonceBeforeMigration); | ||
expect(await hre.ethers.provider.getStorage(safeAddress, GUARD_STORAGE_SLOT)).to.be.eq(guardBeforeMigration); | ||
expect(await hre.ethers.provider.getStorage(safeAddress, FALLBACK_HANDLER_STORAGE_SLOT)).to.be.eq( | ||
fallbackHandlerBeforeMigration, | ||
); | ||
}); | ||
}); | ||
}); |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
comment: Completely unrelated to this PR, but maybe it would make sense to group all migration contracts in a
contracts/migrations
root directory? Not suggesting doing this as part of this PR, but if I were to jump into the codebase without any context, I would find looking for migration contracts in a separate root slightly more natural and intuitive. Food for thought...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.
Makes total sense, I found it weird but I put it on the same directory as the rest of the migrations