Skip to content

Commit

Permalink
Merge branch 'main' of https://github.com/safe-global/safe-contracts …
Browse files Browse the repository at this point in the history 
…into feat/improve-proxy-factory
  • Loading branch information
@mmv08
mmv08 committed Jan 11, 2023
2 parents 7e2da4b + 4c4baef commit 1f8f495
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 97 deletions.
4 changes: 2 additions & 2 deletions benchmark/utils/setup.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const generateTarget = async (owners: Wallet[], threshold: number, guardAddress:
export const configs = [
{ name: "single owner", signers: [user1], threshold: 1 },
{ name: "single owner and guard", signers: [user1], threshold: 1, useGuard: true },
{ name: "2 out of 23", signers: [user1, user2], threshold: 2 },
{ name: "2 out of 2", signers: [user1, user2], threshold: 2 },
{ name: "3 out of 3", signers: [user1, user2, user3], threshold: 3 },
{ name: "3 out of 5", signers: [user1, user2, user3, user4, user5], threshold: 3 },
]
Expand All @@ -33,7 +33,7 @@ export const setupBenchmarkContracts = (benchmarkFixture?: () => Promise<any>, l
await deployments.fixture();
const guardFactory = await hre.ethers.getContractFactory("DelegateCallTransactionGuard");
const guard = await guardFactory.deploy(AddressZero)
const targets = []
const targets: Contract[] = []
for (const config of configs) {
targets.push(await generateTarget(config.signers, config.threshold, config.useGuard ? guard.address : AddressZero, logGasUsage))
}
Expand Down
74 changes: 17 additions & 57 deletions contracts/proxies/GnosisSafeProxyFactory.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,28 +19,13 @@ contract GnosisSafeProxyFactory {
return type(GnosisSafeProxy).creationCode;
}

/// @dev Allows to create new proxy contact using CREATE2 but it doesn't run the initializer.
/// @dev Allows to create a new proxy contact using CREATE2.
/// This method is only meant as an utility to be called from other methods
/// @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,
bool includeChainIdInSalt
) internal returns (GnosisSafeProxy proxy) {
/// @param _singleton Address of singleton contract. Must be deployed at the time of execution.
/// @param salt Create2 salt to use for calculating the address of the new proxy contract.
function deployProxy(address _singleton, bytes32 salt) 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;
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 @@ -49,16 +34,18 @@ contract GnosisSafeProxyFactory {
require(address(proxy) != address(0), "Create2 call failed");
}

/// @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.
/// @dev Allows to create a new proxy contact and execute a message call to the new proxy within one transaction.
/// @param _singleton Address of singleton contract. Must be deployed at the time of execution.
/// @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 createProxyWithNonce(
address _singleton,
bytes memory initializer,
uint256 saltNonce
) public returns (GnosisSafeProxy proxy) {
proxy = deployProxyWithNonce(_singleton, initializer, saltNonce, false);
// 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));
proxy = deployProxy(_singleton, salt);
if (initializer.length > 0)
// solhint-disable-next-line no-inline-assembly
assembly {
Expand All @@ -69,16 +56,19 @@ contract GnosisSafeProxyFactory {
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.
/// @dev Allows to create a new proxy contact that should exist only on 1 network (e.g. specific governance or admin accounts)
/// by including the chain id in the create2 salt. Such proxies cannot be created on other networks by replaying the transaction.
/// @param _singleton Address of singleton contract. Must be deployed at the time of execution.
/// @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 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, getChainId()));
proxy = deployProxy(_singleton, salt);
if (initializer.length > 0)
// solhint-disable-next-line no-inline-assembly
assembly {
Expand All @@ -89,8 +79,8 @@ contract GnosisSafeProxyFactory {
emit ProxyCreation(proxy, _singleton);
}

/// @dev Allows to create new proxy contact, execute a message call to the new proxy and call a specified callback within one transaction
/// @param _singleton Address of singleton contract.
/// @dev Allows to create a new proxy contact, execute a message call to the new proxy and call a specified callback within one transaction
/// @param _singleton Address of singleton contract. Must be deployed at the time of execution.
/// @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 callback Callback that will be invoked after the new proxy contract has been successfully deployed and initialized.
Expand All @@ -105,36 +95,6 @@ contract GnosisSafeProxyFactory {
if (address(callback) != address(0)) callback.proxyCreated(proxy, _singleton, initializer, saltNonce);
}

/// @dev Allows to get the address for a new proxy contact created via `createProxyWithNonce`
/// 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 calculateCreateProxyWithNonceAddress(
address _singleton,
bytes calldata initializer,
uint256 saltNonce
) external returns (GnosisSafeProxy proxy) {
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) {
Expand Down
41 changes: 3 additions & 38 deletions test/factory/ProxyFactory.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,9 @@ describe("ProxyFactory", async () => {

it('should revert if singleton address is not a contract', async () => {
const { factory } = await setupTests()
const randomAddress = ethers.utils.getAddress(ethers.utils.hexlify(ethers.utils.randomBytes(20)))
await expect(
factory.createProxyWithNonce(AddressZero, "0x", saltNonce)
factory.createProxyWithNonce(randomAddress, "0x", saltNonce)
).to.be.revertedWith("Singleton contract not deployed")
})

Expand Down Expand Up @@ -110,7 +111,7 @@ describe("ProxyFactory", async () => {

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

const saltNonce = 73
const saltNonce = 42

it('should revert if singleton address is not a contract', async () => {
const { factory } = await setupTests()
Expand Down Expand Up @@ -234,40 +235,4 @@ describe("ProxyFactory", async () => {
expect(await hre.ethers.provider.getCode(proxyAddress)).to.be.eq(await factory.proxyRuntimeCode())
})
})

describe("calculateCreateProxyWithNonceAddress", 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 calculateProxyAddress(factory, singleton.address, initCode, saltNonce)
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("calculateCreateProxyWithNonceAddress", [singleton.address, initCode, saltNonce])
const response = await singleton.callStatic.forward(factory.address, data)
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)))
})
})
})

0 comments on commit 1f8f495

Please sign in to comment.