Skip to content

Commit 39b69e2

Browse files
rmeissnerrmeissner
authored
Closes #265: Added migration tests (#288)
1 parent 9e6f927 commit 39b69e2

File tree

9 files changed

+267
-14
lines changed

9 files changed

+267
-14
lines changed

‎contracts/libraries/Migrate_1_3_0_to_1_2_0.sol‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,17 @@
11
// SPDX-License-Identifier: LGPL-3.0-only
22
pragma solidity >=0.7.0 <0.9.0;
3-
import "../common/SelfAuthorized.sol";
43

54
/// @title Migration - migrates a Safe contract from 1.3.0 to 1.2.0
65
/// @author Richard Meissner - <richard@gnosis.io>
7-
contract Migration is SelfAuthorized {
6+
contract Migration {
7+
bytes32 constant private GUARD_VALUE = keccak256("migration_130_to_120.guard.bytes32");
88
bytes32 private constant DOMAIN_SEPARATOR_TYPEHASH = 0x035aff83d86937d35b32e04f0ddc6ff469290eef2f1b692d8a815c89404d4749;
99

1010
address public immutable safe120Singleton;
1111
constructor(address targetSingleton) {
1212
require(targetSingleton != address(0), "Invalid singleton address provided");
1313
safe120Singleton = targetSingleton;
14+
guard = GUARD_VALUE;
1415
}
1516

1617
event ChangedMasterCopy(address singleton);
@@ -26,11 +27,14 @@ contract Migration is SelfAuthorized {
2627
// For the migration we need to set the old domain seperator in storage
2728
bytes32 private domainSeparator;
2829

29-
/// @dev Allows to migrate the contract. This can only be done via a Safe transaction.
30+
// Define guard last to avoid conflict with Safe storage layout
31+
bytes32 guard;
32+
33+
/// @dev Allows to migrate the contract. This can only be called via a delegatecall.
3034
function migrate()
3135
public
32-
authorized
3336
{
37+
require(guard != GUARD_VALUE, "Migration should only be called via delegatecall");
3438
// Master copy address cannot be null.
3539
singleton = safe120Singleton;
3640
domainSeparator = keccak256(abi.encode(DOMAIN_SEPARATOR_TYPEHASH, this));

‎test/core/GnosisSafe.Execution.spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ describe("GnosisSafe", async () => {
3737
}
3838
})
3939

40-
describe("execTransactions", async () => {
40+
describe("execTransaction", async () => {
4141

4242
it('should revert if too little gas is provided', async () => {
4343
const { safe } = await setupTests()

‎test/handlers/DefaultCallbackHandler.spec.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ describe("DefaultCallbackHandler", async () => {
1010
});
1111

1212
describe("ERC1155", async () => {
13-
it('should supports ERC1155 interface', async () => {
13+
it('should support ERC1155 interface', async () => {
1414
const handler = await getDefaultCallbackHandler()
1515
await expect(
1616
await handler.callStatic.supportsInterface("0x4e2312e0")
@@ -33,7 +33,7 @@ describe("DefaultCallbackHandler", async () => {
3333
})
3434

3535
describe("ERC721", async () => {
36-
it('should supports ERC721 interface', async () => {
36+
it('should support ERC721 interface', async () => {
3737
const handler = await getDefaultCallbackHandler()
3838
await expect(
3939
await handler.callStatic.supportsInterface("0x150b7a02")
@@ -56,6 +56,13 @@ describe("DefaultCallbackHandler", async () => {
5656
})
5757

5858
describe("ERC165", async () => {
59+
it('should support ERC165 interface', async () => {
60+
const handler = await getDefaultCallbackHandler()
61+
await expect(
62+
await handler.callStatic.supportsInterface("0x01ffc9a7")
63+
).to.be.eq(true)
64+
})
65+
5966
it('should not support random interface', async () => {
6067
const handler = await getDefaultCallbackHandler()
6168
await expect(

‎test/json/safeDeployment.json‎

Lines changed: 4 additions & 0 deletions
Large diffs are not rendered by default.

‎test/libraries/Migration.spec.ts‎

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,27 @@
11
import { expect } from "chai";
2-
import hre, { deployments, waffle } from "hardhat";
2+
import { ethers, deployments, waffle } from "hardhat";
33
import "@nomiclabs/hardhat-ethers";
44
import { AddressZero } from "@ethersproject/constants";
55
import { getSafeWithOwners, getSafeSingleton, migrationContract } from "../utils/setup";
6+
import deploymentData from "../json/safeDeployment.json";
67
import { executeContractCallWithSigners } from "../utils/execution";
78

89
describe("Migration", async () => {
910

11+
const MigratedInterface = new ethers.utils.Interface([
12+
"function domainSeparator() view returns(bytes32)",
13+
"function masterCopy() view returns(address)",
14+
])
15+
1016
const [user1, user2] = waffle.provider.getWallets();
1117

1218
const setupTests = deployments.createFixture(async ({ deployments }) => {
1319
await deployments.fixture();
14-
// TODO: replace with test singleton
15-
const migration = await (await migrationContract()).deploy(user1.address)
20+
const singleton120 = (await (await user1.sendTransaction({ data: deploymentData.safe120 })).wait()).contractAddress
21+
const migration = await (await migrationContract()).deploy(singleton120)
1622
return {
1723
singleton: await getSafeSingleton(),
24+
singleton120,
1825
safe: await getSafeWithOwners([user1.address]),
1926
migration
2027
}
@@ -32,10 +39,33 @@ describe("Migration", async () => {
3239
describe("migrate", async () => {
3340
it('can only be called from Safe itself', async () => {
3441
const { migration } = await setupTests()
35-
await expect(migration.migrate()).to.be.revertedWith("GS031")
42+
await expect(migration.migrate()).to.be.revertedWith("Migration should only be called via delegatecall")
3643
})
3744

38-
it.skip('can migrate', async () => {
45+
it('can migrate', async () => {
46+
const { safe, migration, singleton120 } = await setupTests()
47+
// The emit matcher checks the address, which is the Safe as delegatecall is used
48+
const migrationSafe = migration.attach(safe.address)
49+
50+
await expect(
51+
await ethers.provider.getStorageAt(safe.address, "0x06")
52+
).to.be.eq("0x" + "".padEnd(64, "0"))
53+
54+
await expect(
55+
executeContractCallWithSigners(safe, migration, "migrate", [], [user1], true)
56+
).to.emit(migrationSafe, "ChangedMasterCopy").withArgs(singleton120)
57+
58+
const expectedDomainSeparator = ethers.utils._TypedDataEncoder.hashDomain({ verifyingContract: safe.address })
59+
60+
await expect(
61+
await ethers.provider.getStorageAt(safe.address, "0x06")
62+
).to.be.eq(expectedDomainSeparator)
63+
64+
const respData = await user1.call({to: safe.address, data: MigratedInterface.encodeFunctionData("domainSeparator")})
65+
await expect(MigratedInterface.decodeFunctionResult("domainSeparator", respData)[0]).to.be.eq(expectedDomainSeparator)
66+
67+
const masterCopyResp = await user1.call({to: safe.address, data: MigratedInterface.encodeFunctionData("masterCopy")})
68+
await expect(MigratedInterface.decodeFunctionResult("masterCopy", masterCopyResp)[0]).to.be.eq(singleton120)
3969
})
4070
})
4171
})

‎test/migration/UpgradeFromSafe111.spec.ts‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
import { expect } from "chai";
2+
import hre, { ethers, deployments, waffle } from "hardhat";
3+
import "@nomiclabs/hardhat-ethers";
4+
import { AddressZero } from "@ethersproject/constants";
5+
import { getSafeSingleton, getFactory, getMock, getMultiSend } from "../utils/setup";
6+
import { buildSafeTransaction, executeTx, safeApproveHash } from "../utils/execution";
7+
import { verificationTests } from "./subTests.spec";
8+
import deploymentData from "../json/safeDeployment.json";
9+
import { calculateProxyAddress } from "../utils/proxies";
10+
11+
describe("Upgrade from Safe 1.1.1", () => {
12+
13+
const [user1] = waffle.provider.getWallets();
14+
15+
const ChangeMasterCopyInterface = new ethers.utils.Interface(["function changeMasterCopy(address target)"])
16+
17+
// We migrate the Safe and run the verification tests
18+
const setupTests = deployments.createFixture(async ({ deployments }) => {
19+
await deployments.fixture();
20+
const mock = await getMock()
21+
const singleton111 = (await (await user1.sendTransaction({ data: deploymentData.safe111 })).wait()).contractAddress
22+
const singleton130 = (await getSafeSingleton()).address
23+
const factory = await getFactory()
24+
const saltNonce = 42
25+
const proxyAddress = await calculateProxyAddress(factory, singleton111, "0x", saltNonce)
26+
await factory.createProxyWithNonce(singleton111, "0x", saltNonce).then((tx: any) => tx.wait())
27+
28+
const Safe = await hre.ethers.getContractFactory("GnosisSafe");
29+
const safe = Safe.attach(proxyAddress)
30+
await safe.setup([user1.address], 1, AddressZero, "0x", mock.address, AddressZero, 0, AddressZero)
31+
32+
expect(await safe.VERSION()).to.be.eq("1.1.1")
33+
const nonce = await safe.callStatic.nonce()
34+
const data = ChangeMasterCopyInterface.encodeFunctionData("changeMasterCopy", [singleton130])
35+
const tx = buildSafeTransaction({ to: safe.address, data, nonce })
36+
await executeTx(safe, tx, [await safeApproveHash(user1, safe, tx, true)])
37+
expect(await safe.VERSION()).to.be.eq("1.3.0")
38+
39+
return {
40+
migratedSafe: safe,
41+
mock,
42+
multiSend: await getMultiSend()
43+
}
44+
})
45+
verificationTests(setupTests)
46+
})

‎test/migration/UpgradeFromSafe120.spec.ts‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
import { expect } from "chai";
2+
import hre, { ethers, deployments, waffle } from "hardhat";
3+
import "@nomiclabs/hardhat-ethers";
4+
import { AddressZero } from "@ethersproject/constants";
5+
import { getSafeSingleton, getFactory, getMock, getMultiSend } from "../utils/setup";
6+
import { buildSafeTransaction, executeTx, safeApproveHash } from "../utils/execution";
7+
import { verificationTests } from "./subTests.spec";
8+
import deploymentData from "../json/safeDeployment.json";
9+
import { calculateProxyAddress } from "../utils/proxies";
10+
11+
describe("Upgrade from Safe 1.2.0", () => {
12+
13+
const [user1] = waffle.provider.getWallets();
14+
15+
const ChangeMasterCopyInterface = new ethers.utils.Interface(["function changeMasterCopy(address target)"])
16+
17+
// We migrate the Safe and run the verification tests
18+
const setupTests = deployments.createFixture(async ({ deployments }) => {
19+
await deployments.fixture();
20+
const mock = await getMock()
21+
const singleton120 = (await (await user1.sendTransaction({ data: deploymentData.safe120 })).wait()).contractAddress
22+
const singleton130 = (await getSafeSingleton()).address
23+
const factory = await getFactory()
24+
const saltNonce = 42
25+
const proxyAddress = await calculateProxyAddress(factory, singleton120, "0x", saltNonce)
26+
await factory.createProxyWithNonce(singleton120, "0x", saltNonce).then((tx: any) => tx.wait())
27+
28+
const Safe = await hre.ethers.getContractFactory("GnosisSafe");
29+
const safe = Safe.attach(proxyAddress)
30+
await safe.setup([user1.address], 1, AddressZero, "0x", mock.address, AddressZero, 0, AddressZero)
31+
32+
expect(await safe.VERSION()).to.be.eq("1.2.0")
33+
const nonce = await safe.callStatic.nonce()
34+
const data = ChangeMasterCopyInterface.encodeFunctionData("changeMasterCopy", [singleton130])
35+
const tx = buildSafeTransaction({ to: safe.address, data, nonce })
36+
await executeTx(safe, tx, [await safeApproveHash(user1, safe, tx, true)])
37+
expect(await safe.VERSION()).to.be.eq("1.3.0")
38+
39+
return {
40+
migratedSafe: safe,
41+
mock,
42+
multiSend: await getMultiSend()
43+
}
44+
})
45+
verificationTests(setupTests)
46+
})

‎test/migration/subTests.spec.ts‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
import { BigNumber } from "@ethersproject/bignumber";
2+
import { Contract } from "@ethersproject/contracts"
3+
import { parseEther } from "@ethersproject/units"
4+
import { expect } from "chai";
5+
import hre, { ethers, waffle } from "hardhat";
6+
import { AddressOne } from "../utils/constants";
7+
import { buildSafeTransaction, executeContractCallWithSigners, executeTxWithSigners, MetaTransaction } from "../utils/execution"
8+
import { buildMultiSendSafeTx } from "../utils/multisend";
9+
10+
interface TestSetup {
11+
migratedSafe: Contract,
12+
mock: Contract,
13+
multiSend: Contract
14+
}
15+
16+
export const verificationTests = (setupTests: () => Promise<TestSetup>) => {
17+
18+
const [user1, user2, user3] = waffle.provider.getWallets();
19+
20+
describe("execTransaction", async () => {
21+
it('should be able to transfer ETH', async () => {
22+
const { migratedSafe } = await setupTests()
23+
await user1.sendTransaction({ to: migratedSafe.address, value: parseEther("1") })
24+
const nonce = await migratedSafe.nonce()
25+
const tx = buildSafeTransaction({ to: user2.address, value: parseEther("1"), nonce })
26+
27+
const userBalance = await ethers.provider.getBalance(user2.address)
28+
await expect(await ethers.provider.getBalance(migratedSafe.address)).to.be.deep.eq(parseEther("1"))
29+
30+
await executeTxWithSigners(migratedSafe, tx, [user1])
31+
32+
await expect(await ethers.provider.getBalance(user2.address)).to.be.deep.eq(userBalance.add(parseEther("1")))
33+
await expect(await ethers.provider.getBalance(migratedSafe.address)).to.be.deep.eq(parseEther("0"))
34+
})
35+
})
36+
37+
describe("addOwner", async () => {
38+
it('should add owner and change treshold', async () => {
39+
const { migratedSafe } = await setupTests()
40+
41+
await expect(
42+
executeContractCallWithSigners(migratedSafe, migratedSafe, "addOwnerWithThreshold", [user2.address, 2], [user1])
43+
).to.emit(migratedSafe, "AddedOwner").withArgs(user2.address).and.to.emit(migratedSafe, "ChangedThreshold")
44+
45+
await expect(await migratedSafe.getThreshold()).to.be.deep.eq(BigNumber.from(2))
46+
await expect(await migratedSafe.getOwners()).to.be.deep.equal([user2.address, user1.address])
47+
48+
await expect(
49+
executeContractCallWithSigners(migratedSafe, migratedSafe, "addOwnerWithThreshold", [user3.address, 1], [user1, user2])
50+
).to.emit(migratedSafe, "AddedOwner").withArgs(user3.address).and.to.emit(migratedSafe, "ChangedThreshold")
51+
52+
await expect(await migratedSafe.getThreshold()).to.be.deep.eq(BigNumber.from(1))
53+
await expect(await migratedSafe.getOwners()).to.be.deep.equal([user3.address, user2.address, user1.address])
54+
55+
await expect(await migratedSafe.isOwner(user1.address)).to.be.true
56+
await expect(await migratedSafe.isOwner(user2.address)).to.be.true
57+
await expect(await migratedSafe.isOwner(user3.address)).to.be.true
58+
})
59+
})
60+
61+
describe("enableModule", async () => {
62+
it('should enabled module and be able to use it', async () => {
63+
const { migratedSafe, mock } = await setupTests()
64+
65+
await expect(
66+
executeContractCallWithSigners(migratedSafe, migratedSafe, "enableModule", [user2.address], [user1])
67+
).to.emit(migratedSafe, "EnabledModule").withArgs(user2.address)
68+
69+
await expect(await migratedSafe.isModuleEnabled(user2.address)).to.be.true
70+
await expect(await migratedSafe.getModulesPaginated(AddressOne, 10)).to.be.deep.equal([[user2.address], AddressOne])
71+
72+
const user2Safe = migratedSafe.connect(user2)
73+
await expect(
74+
user2Safe.execTransactionFromModule(mock.address, 0, "0xbaddad", 0)
75+
).to.emit(migratedSafe, "ExecutionFromModuleSuccess").withArgs(user2.address)
76+
expect(await mock.callStatic.invocationCountForCalldata("0xbaddad")).to.be.deep.equals(BigNumber.from(1));
77+
})
78+
})
79+
80+
describe("multiSend", async () => {
81+
it('execute multisend via delegatecall', async () => {
82+
const { migratedSafe, mock, multiSend } = await setupTests()
83+
84+
await user1.sendTransaction({to: migratedSafe.address, value: parseEther("1")})
85+
const userBalance = await hre.ethers.provider.getBalance(user2.address)
86+
await expect(await hre.ethers.provider.getBalance(migratedSafe.address)).to.be.deep.eq(parseEther("1"))
87+
88+
const txs: MetaTransaction[] = [
89+
buildSafeTransaction({to: user2.address, value: parseEther("1"), nonce: 0}),
90+
buildSafeTransaction({to: mock.address, data: "0xbaddad", nonce: 0})
91+
]
92+
const safeTx = buildMultiSendSafeTx(multiSend, txs, await migratedSafe.nonce())
93+
await expect(
94+
executeTxWithSigners(migratedSafe, safeTx, [ user1 ])
95+
).to.emit(migratedSafe, "ExecutionSuccess")
96+
97+
await expect(await hre.ethers.provider.getBalance(migratedSafe.address)).to.be.deep.eq(parseEther("0"))
98+
await expect(await hre.ethers.provider.getBalance(user2.address)).to.be.deep.eq(userBalance.add(parseEther("1")))
99+
expect(await mock.callStatic.invocationCountForCalldata("0xbaddad")).to.be.deep.equals(BigNumber.from(1));
100+
})
101+
})
102+
103+
describe("fallbackHandler", async () => {
104+
it('should be correctly set', async () => {
105+
const { migratedSafe, mock } = await setupTests()
106+
// Check fallback handler
107+
await expect(
108+
await ethers.provider.getStorageAt(migratedSafe.address, "0x6c9a6c4a39284e37ed1cf53d337577d14212a4870fb976a4366c693b939918d5")
109+
).to.be.eq("0x" + mock.address.toLowerCase().slice(2).padStart(64, "0"))
110+
})
111+
})
112+
}

‎test/utils/execution.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,10 +134,14 @@ export const buildContractCall = (contract: Contract, method: string, params: an
134134
}, overrides))
135135
}
136136

137+
export const executeTxWithSigners = async (safe: Contract, tx: SafeTransaction, signers: Wallet[], overrides?: any) => {
138+
const sigs = await Promise.all(signers.map((signer) => safeSignTypedData(signer, safe, tx)))
139+
return executeTx(safe, tx, sigs, overrides)
140+
}
141+
137142
export const executeContractCallWithSigners = async (safe: Contract, contract: Contract, method: string, params: any[], signers: Wallet[], delegateCall?: boolean, overrides?: Partial<SafeTransaction>) => {
138143
const tx = buildContractCall(contract, method, params, await safe.nonce(), delegateCall, overrides)
139-
const sigs = await Promise.all(signers.map((signer) => safeSignTypedData(signer, safe, tx)))
140-
return executeTx(safe, tx, sigs)
144+
return executeTxWithSigners(safe, tx, signers)
141145
}
142146

143147
export const buildSafeTransaction = (template: {

0 commit comments

Comments
 (0)