Skip to content

Commit

Permalink
remove createProxy, add createNetworkSpecificProxy, check that single…
Browse files Browse the repository at this point in the history
…ton is deployed
  • Loading branch information
mmv08 committed Dec 23, 2022
1 parent 35040bf commit 10b24e5
Show file tree
Hide file tree
Showing 8 changed files with 3,394 additions and 2,910 deletions.
2 changes: 1 addition & 1 deletion benchmark/GnosisSafe.Proxy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ benchmark("Proxy", [{
name: "creation",
prepare: async (contracts,_,nonce) => {
const factory = contracts.additions.factory
const data = factory.interface.encodeFunctionData("createProxy", [testTarget, "0x"])
const data = factory.interface.encodeFunctionData("createProxyWithNonce", [testTarget, "0x", 0])
return buildSafeTransaction({ to: factory.address, data, safeTxGas: 1000000, nonce })
},
fixture: async () => {
Expand Down
91 changes: 71 additions & 20 deletions contracts/proxies/GnosisSafeProxyFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,11 @@ pragma solidity >=0.7.0 <0.9.0;
import "./GnosisSafeProxy.sol";
import "./IProxyCreationCallback.sol";

/// @title Proxy Factory - Allows to create new proxy contact and execute a message call to the new proxy within one transaction.
/// @title Proxy Factory - Allows to create a new proxy contract and execute a message call to the new proxy within one transaction.
/// @author Stefan George - <[email protected]>
contract GnosisSafeProxyFactory {
event ProxyCreation(GnosisSafeProxy proxy, address singleton);

/// @dev Allows to create new proxy contact and execute a message call to the new proxy within one transaction.
/// @param singleton Address of singleton contract.
/// @param data Payload for message call sent to new proxy contract.
function createProxy(address singleton, bytes memory data) public returns (GnosisSafeProxy proxy) {
proxy = new GnosisSafeProxy(singleton);
if (data.length > 0)
// solhint-disable-next-line no-inline-assembly
assembly {
if eq(call(gas(), proxy, 0, add(data, 0x20), mload(data), 0, 0), 0) {
revert(0, 0)
}
}
emit ProxyCreation(proxy, singleton);
}

/// @dev Allows to retrieve the runtime code of a deployed Proxy. This can be used to check that the expected Proxy was deployed.
function proxyRuntimeCode() public pure returns (bytes memory) {
return type(GnosisSafeProxy).runtimeCode;
Expand All @@ -39,13 +24,23 @@ contract GnosisSafeProxyFactory {
/// @param _singleton Address of singleton contract.
/// @param initializer Payload for message call sent to new proxy contract.
/// @param saltNonce Nonce that will be used to generate the salt to calculate the address of the new proxy contract.
/// @param includeChainIdInSalt If true, the chain id will be included in the salt to calculate the address of the new proxy contract.
function deployProxyWithNonce(
address _singleton,
bytes memory initializer,
uint256 saltNonce
uint256 saltNonce,
bool includeChainIdInSalt
) internal returns (GnosisSafeProxy proxy) {
require(isContract(_singleton), "Singleton contract not deployed");

// If the initializer changes the proxy address should change too. Hashing the initializer data is cheaper than just concatinating it
bytes32 salt = keccak256(abi.encodePacked(keccak256(initializer), saltNonce));
bytes32 salt;
if (includeChainIdInSalt) {
salt = keccak256(abi.encodePacked(keccak256(initializer), saltNonce, getChainId()));
} else {
salt = keccak256(abi.encodePacked(keccak256(initializer), saltNonce));
}

bytes memory deploymentData = abi.encodePacked(type(GnosisSafeProxy).creationCode, uint256(uint160(_singleton)));
// solhint-disable-next-line no-inline-assembly
assembly {
Expand All @@ -63,7 +58,27 @@ contract GnosisSafeProxyFactory {
bytes memory initializer,
uint256 saltNonce
) public returns (GnosisSafeProxy proxy) {
proxy = deployProxyWithNonce(_singleton, initializer, saltNonce);
proxy = deployProxyWithNonce(_singleton, initializer, saltNonce, false);
if (initializer.length > 0)
// solhint-disable-next-line no-inline-assembly
assembly {
if eq(call(gas(), proxy, 0, add(initializer, 0x20), mload(initializer), 0, 0), 0) {
revert(0, 0)
}
}
emit ProxyCreation(proxy, _singleton);
}

/// @dev Same as `createProxyWithNonce` but includes the chain id in the salt to calculate the address of the new proxy contract.
/// @param _singleton Address of singleton contract.
/// @param initializer Payload for message call sent to new proxy contract.
/// @param saltNonce Nonce that will be used to generate the salt to calculate the address of the new proxy contract.
function createChainSpecificProxyWithNonce(
address _singleton,
bytes memory initializer,
uint256 saltNonce
) public returns (GnosisSafeProxy proxy) {
proxy = deployProxyWithNonce(_singleton, initializer, saltNonce, true);
if (initializer.length > 0)
// solhint-disable-next-line no-inline-assembly
assembly {
Expand Down Expand Up @@ -101,7 +116,43 @@ contract GnosisSafeProxyFactory {
bytes calldata initializer,
uint256 saltNonce
) external returns (GnosisSafeProxy proxy) {
proxy = deployProxyWithNonce(_singleton, initializer, saltNonce);
proxy = deployProxyWithNonce(_singleton, initializer, saltNonce, false);
revert(string(abi.encodePacked(proxy)));
}

/// @dev Allows to get the address for a new proxy contact created via `createChainSpecificProxyWithNonce`
/// This method is only meant for address calculation purpose when you use an initializer that would revert,
/// therefore the response is returned with a revert. When calling this method set `from` to the address of the proxy factory.
/// @param _singleton Address of singleton contract.
/// @param initializer Payload for message call sent to new proxy contract.
/// @param saltNonce Nonce that will be used to generate the salt to calculate the address of the new proxy contract.
function calculateCreateChainSpecificProxyWithNonceAddress(
address _singleton,
bytes calldata initializer,
uint256 saltNonce
) external returns (GnosisSafeProxy proxy) {
proxy = deployProxyWithNonce(_singleton, initializer, saltNonce, true);
revert(string(abi.encodePacked(proxy)));
}

/// @dev Returns true if `account` is a contract.
/// @param account The address being queried
function isContract(address account) internal view returns (bool) {
uint256 size;
// solhint-disable-next-line no-inline-assembly
assembly {
size := extcodesize(account)
}
return size > 0;
}

/// @dev Returns the chain id used by this contract.
function getChainId() public view returns (uint256) {
uint256 id;
// solhint-disable-next-line no-inline-assembly
assembly {
id := chainid()
}
return id;
}
}
14 changes: 7 additions & 7 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@
"devDependencies": {
"@gnosis.pm/mock-contract": "^4.0.0",
"@gnosis.pm/safe-singleton-factory": "^1.0.3",
"@nomiclabs/hardhat-ethers": "^2.0.0",
"@nomiclabs/hardhat-ethers": "2.0.2",
"@nomiclabs/hardhat-etherscan": "^2.1.0",
"@nomiclabs/hardhat-waffle": "^2.0.0",
"@nomiclabs/hardhat-waffle": "2.0.1",
"@openzeppelin/contracts": "^3.4.0",
"@types/chai": "^4.2.14",
"@types/mocha": "^8.2.0",
Expand All @@ -66,21 +66,21 @@
"eslint-plugin-no-only-tests": "^2.4.0",
"eslint-plugin-prettier": "^3.1.4",
"ethereum-waffle": "^3.3.0",
"ethers": "^5.1.4",
"ethers": "5.4.0",
"hardhat": "^2.2.1",
"hardhat-deploy": "0.9.2",
"husky": "^5.1.3",
"prettier": "^2.1.2",
"prettier-plugin-solidity": "^1.0.0-alpha.60",
"prettier-plugin-solidity": "1.0.0-beta.10",
"solc": "0.7.6",
"solhint": "^3.3.2",
"solhint-plugin-prettier": "^0.0.5",
"solhint": "3.3.4",
"solhint-plugin-prettier": "0.0.5",
"solidity-coverage": "^0.7.17",
"ts-node": "^9.1.1",
"typescript": "^4.2.4",
"yargs": "^16.1.1"
},
"peerDependencies": {
"ethers": "^5.1.4"
"ethers": "5.1.4"
}
}
11 changes: 10 additions & 1 deletion src/utils/proxies.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ethers, Contract } from "ethers"
import { ethers, Contract, BigNumberish } from "ethers"

export const calculateProxyAddress = async (factory: Contract, singleton: string, inititalizer: string, nonce: number | string) => {
const deploymentCode = ethers.utils.solidityPack(["bytes", "uint256"], [await factory.proxyCreationCode(), singleton])
Expand All @@ -15,4 +15,13 @@ export const calculateProxyAddressWithCallback = async (factory: Contract, singl
[nonce, callback]
)
return calculateProxyAddress(factory, singleton, inititalizer, saltNonceWithCallback)
}

export const calculateChainSpecificProxyAddress = async (factory: Contract, singleton: string, inititalizer: string, nonce: number | string, chainId: BigNumberish) => {
const deploymentCode = ethers.utils.solidityPack(["bytes", "uint256"], [await factory.proxyCreationCode(), singleton])
const salt = ethers.utils.solidityKeccak256(
["bytes32", "uint256", "uint256"],
[ethers.utils.solidityKeccak256(["bytes"], [inititalizer]), nonce, chainId]
)
return ethers.utils.getCreate2Address(factory.address, salt, ethers.utils.keccak256(deploymentCode))
}
79 changes: 62 additions & 17 deletions test/factory/ProxyFactory.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ import hre, { deployments, waffle, ethers } from "hardhat";
import "@nomiclabs/hardhat-ethers";
import { deployContract, getFactory, getMock, getSafeWithOwners } from "../utils/setup";
import { AddressZero } from "@ethersproject/constants";
import { BigNumber, Contract } from "ethers";
import { calculateProxyAddress, calculateProxyAddressWithCallback } from "../../src/utils/proxies";
import { BigNumber } from "ethers";
import { calculateChainSpecificProxyAddress, calculateProxyAddress, calculateProxyAddressWithCallback } from "../../src/utils/proxies";
import { getAddress } from "ethers/lib/utils";
import { chainId } from './../utils/encoding';

describe("ProxyFactory", async () => {

Expand Down Expand Up @@ -46,28 +47,30 @@ describe("ProxyFactory", async () => {
}
})

describe("createProxy", async () => {
describe("createProxyWithNonce", async () => {

const saltNonce = 42

it('should revert with invalid singleton address', async () => {
it('should revert if singleton address is not a contract', async () => {
const { factory } = await setupTests()
await expect(
factory.createProxy(AddressZero, "0x")
).to.be.revertedWith("Invalid singleton address provided")
factory.createProxyWithNonce(AddressZero, "0x", saltNonce)
).to.be.revertedWith("Singleton contract not deployed")
})

it('should revert with invalid initializer', async () => {
const { factory, singleton } = await setupTests()
await expect(
factory.createProxy(singleton.address, "0x42baddad")
factory.createProxyWithNonce(singleton.address, "0x42baddad", saltNonce)
).to.be.revertedWith("Transaction reverted without a reason")
})

it('should emit event without initializing', async () => {
const { factory, singleton } = await setupTests()
const factoryNonce = await ethers.provider.getTransactionCount(factory.address)
const proxyAddress = ethers.utils.getContractAddress({ from: factory.address, nonce: factoryNonce })
const initCode = "0x"
const proxyAddress = await calculateProxyAddress(factory, singleton.address, initCode, saltNonce)
await expect(
factory.createProxy(singleton.address, "0x")
factory.createProxyWithNonce(singleton.address, initCode, saltNonce)
).to.emit(factory, "ProxyCreation").withArgs(proxyAddress, singleton.address)
const proxy = singleton.attach(proxyAddress)
expect(await proxy.creator()).to.be.eq(AddressZero)
Expand All @@ -79,10 +82,10 @@ describe("ProxyFactory", async () => {

it('should emit event with initializing', async () => {
const { factory, singleton } = await setupTests()
const factoryNonce = await ethers.provider.getTransactionCount(factory.address)
const proxyAddress = ethers.utils.getContractAddress({ from: factory.address, nonce: factoryNonce })
const initCode = singleton.interface.encodeFunctionData("init", [])
const proxyAddress = await calculateProxyAddress(factory, singleton.address, initCode, saltNonce)
await expect(
factory.createProxy(singleton.address, singleton.interface.encodeFunctionData("init", []))
factory.createProxyWithNonce(singleton.address, initCode, saltNonce)
).to.emit(factory, "ProxyCreation").withArgs(proxyAddress, singleton.address)
const proxy = singleton.attach(proxyAddress)
expect(await proxy.creator()).to.be.eq(factory.address)
Expand All @@ -91,17 +94,29 @@ describe("ProxyFactory", async () => {
expect(await singleton.masterCopy()).to.be.eq(AddressZero)
expect(await hre.ethers.provider.getCode(proxyAddress)).to.be.eq(await factory.proxyRuntimeCode())
})

it('should not be able to deploy same proxy twice', async () => {
const { factory, singleton } = await setupTests()
const initCode = singleton.interface.encodeFunctionData("init", [])
const proxyAddress = await calculateProxyAddress(factory, singleton.address, initCode, saltNonce)
await expect(
factory.createProxyWithNonce(singleton.address, initCode, saltNonce)
).to.emit(factory, "ProxyCreation").withArgs(proxyAddress, singleton.address)
await expect(
factory.createProxyWithNonce(singleton.address, initCode, saltNonce)
).to.be.revertedWith("Create2 call failed")
})
})

describe("createProxyWithNonce", async () => {
describe("createChainSpecificProxyWithNonce", async () => {

const saltNonce = 42
const saltNonce = 73

it('should revert with invalid singleton address', async () => {
it('should revert if singleton address is not a contract', async () => {
const { factory } = await setupTests()
await expect(
factory.createProxyWithNonce(AddressZero, "0x", saltNonce)
).to.be.revertedWith("Create2 call failed")
).to.be.revertedWith("Singleton contract not deployed")
})

it('should revert with invalid initializer', async () => {
Expand Down Expand Up @@ -141,6 +156,18 @@ describe("ProxyFactory", async () => {
expect(await hre.ethers.provider.getCode(proxyAddress)).to.be.eq(await factory.proxyRuntimeCode())
})

it('should deploy proxy to create2 address with chainid included in salt', async () => {
const { factory, singleton } = await setupTests()
const provider = hre.ethers.provider
const initCode = singleton.interface.encodeFunctionData("init", [])
const proxyAddress = await calculateChainSpecificProxyAddress(factory, singleton.address, initCode, saltNonce, await chainId())
expect(await provider.getCode(proxyAddress)).to.eq("0x")

await factory.createChainSpecificProxyWithNonce(singleton.address, initCode, saltNonce)

expect(await provider.getCode(proxyAddress)).to.eq(await factory.proxyRuntimeCode())
})

it('should not be able to deploy same proxy twice', async () => {
const { factory, singleton } = await setupTests()
const initCode = singleton.interface.encodeFunctionData("init", [])
Expand Down Expand Up @@ -225,4 +252,22 @@ describe("ProxyFactory", async () => {
expect(proxyAddress).to.be.eq(getAddress(response.slice(138, 178)))
})
})

describe("calculateCreateChainSpecificProxyWithNonceAddress", async () => {

const saltNonce = 4242

it('should return the calculated address in the revert message', async () => {
const { factory, singleton } = await setupTests()
const initCode = "0x"
const proxyAddress = await calculateChainSpecificProxyAddress(factory, singleton.address, initCode, saltNonce, await chainId())
await expect(
factory.callStatic.calculateCreateProxyWithNonceAddress(singleton.address, initCode, saltNonce)
).to.be.reverted
// Currently ethers provides no good way to grab the result directly from the factory
const data = factory.interface.encodeFunctionData("calculateCreateChainSpecificProxyWithNonceAddress", [singleton.address, initCode, saltNonce])
const response = await singleton.callStatic.forward(factory.address, data)
expect(proxyAddress).to.be.eq(getAddress(response.slice(138, 178)))
})
})
})
15 changes: 15 additions & 0 deletions test/utils/numbers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
const getRandomInt = (
min: number = 0,
max: number = Number.MAX_SAFE_INTEGER
): number => {
return Math.floor(Math.random() * (max - min + 1)) + min;
};

const getRandomIntAsString = (
min: number = 0,
max: number = Number.MAX_SAFE_INTEGER
): string => {
return getRandomInt(min, max).toString();
};

export { getRandomInt, getRandomIntAsString };
11 changes: 6 additions & 5 deletions test/utils/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { AddressZero } from "@ethersproject/constants";
import solc from "solc"
import { logGas } from "../../src/utils/execution";
import { safeContractUnderTest } from "./config";
import { getRandomIntAsString } from "./numbers";

export const defaultCallbackHandlerDeployment = async () => {
return await deployments.get("DefaultCallbackHandler");
Expand Down Expand Up @@ -67,17 +68,17 @@ export const getMock = async () => {
return await Mock.deploy();
}

export const getSafeTemplate = async () => {
export const getSafeTemplate = async (saltNumber: string = getRandomIntAsString()) => {
const singleton = await getSafeSingleton()
const factory = await getFactory()
const template = await factory.callStatic.createProxy(singleton.address, "0x")
await factory.createProxy(singleton.address, "0x").then((tx: any) => tx.wait())
const template = await factory.callStatic.createProxyWithNonce(singleton.address, "0x", saltNumber)
await factory.createProxyWithNonce(singleton.address, "0x", saltNumber).then((tx: any) => tx.wait())
const Safe = await hre.ethers.getContractFactory(safeContractUnderTest());
return Safe.attach(template);
}

export const getSafeWithOwners = async (owners: string[], threshold?: number, fallbackHandler?: string, logGasUsage?: boolean) => {
const template = await getSafeTemplate()
export const getSafeWithOwners = async (owners: string[], threshold?: number, fallbackHandler?: string, logGasUsage?: boolean, saltNumber: string = getRandomIntAsString()) => {
const template = await getSafeTemplate(saltNumber)
await logGas(
`Setup Safe with ${owners.length} owner(s)${fallbackHandler && fallbackHandler !== AddressZero ? " and fallback handler" : ""}`,
template.setup(owners, threshold || owners.length, AddressZero, "0x", fallbackHandler || AddressZero, AddressZero, 0, AddressZero),
Expand Down
Loading

0 comments on commit 10b24e5

Please sign in to comment.