Skip to content

Commit

Permalink
refactor: mapping "salts" to "getNextSalt" and "nextSalts"
Browse files Browse the repository at this point in the history
chore: temporarily disable loading of deploy tasks
docs: add requirements in natspec of "deploy" and "deployFor" functions
docs: document all params in natspec of constant functions
docs: rewrite top-level natspec for IPRBProxyRegistry
feat: mapping "lastSalts" and "getLastSalt" in PRBProxyRegistry
feat: function "getProxy" in PRBProxyRegistry
fix: load proper address for PRBProxyFactory in "dist/addresses-computeProxyAddress"
refactor: emit "PRBProxyRegistry__ProxyAlreadyExists" custom error in PRBProxyRegistry
refactor: mapping "_isProxy" to "proxies" in PRBProxyFactory
refactor: var "salt" to "finalSalt"
test: fix order of context blocks
test: new custom error "PRBProxyRegistry__ProxyAlreadyExists"
test: new helper constant SALT_TWO
test: refactor "test/shared/create2-computeProxyAddress" to load next salt from contract
test: update tests to conform to latest contracts
test: when the deployer interacts with the factory while interacting with the registry
  • Loading branch information
PaulRBerg committed Sep 11, 2021
1 parent 18a8cb5 commit a94f1de
Show file tree
Hide file tree
Showing 15 changed files with 199 additions and 93 deletions.
12 changes: 9 additions & 3 deletions contracts/IPRBProxyFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,17 @@ interface IPRBProxyFactory {

/// PUBLIC CONSTANT FUNCTIONS ///

/// @notice Gets the next salt that will be used to deploy the proxy.
/// @param eoa The externally owned account which deployed proxies.
function getNextSalt(address eoa) external view returns (bytes32 result);

/// @notice The address of the implementation of PRBProxy.
function implementation() external view returns (IPRBProxy proxy);

/// @notice Mapping to track all deployed proxies.
/// @param proxy The address of the proxy to make the check for.
function isProxy(address proxy) external view returns (bool result);

/// @notice Mapping to track used salts per EOA.
function salts(address eoa) external view returns (bytes32 salt);

/// PUBLIC NON-CONSTANT FUNCTIONS ///

/// @notice Deploys a new proxy as an EIP-1167 clone deployed via CREATE2.
Expand All @@ -30,6 +32,10 @@ interface IPRBProxyFactory {
function deploy() external returns (address payable proxy);

/// @notice Deploys a new proxy as an EIP-1167 clone deployed via CREATE2, for a specific owner.
///
/// @dev Requirements:
/// - The CREATE2 must not have been used.
///
/// @param owner The owner of the proxy.
/// @return proxy The address of the newly deployed proxy contract.
function deployFor(address owner) external returns (address payable proxy);
Expand Down
30 changes: 25 additions & 5 deletions contracts/IPRBProxyRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,44 @@ import "./IPRBProxyFactory.sol";

/// @title IPRBProxyRegistry
/// @author Paul Razvan Berg
/// @notice Deploys new proxy instances via the proxy factory and keeps a registry of owners to proxies.
/// @notice Deploys new proxy instances via the proxy factory and keeps a registry of owners to proxies. Owners can only
/// have one proxy at a time.
interface IPRBProxyRegistry {
/// PUBLIC CONSTANT FUNCTIONS ///

/// @notice Mapping of owners to salts to proxies.
function proxies(address owner, bytes32 salt) external view returns (IPRBProxy proxy);

/// @notice Proxy factory contract.
function factory() external view returns (IPRBProxyFactory proxyFactory);

/// @notice Gets the last proxy that belongs to the given owner.
/// @param owner The address of the owner of the proxy.
function getLastProxy(address owner) external view returns (IPRBProxy proxy);

/// @notice Gets the last salt that was used to deploy the proxy.
/// @dev It is possible for this to grow by more than 1 between deployments. Users can call the factory directly.
/// @param owner The address of the owner of the proxy.
function getLastSalt(address owner) external view returns (bytes32 nextSalt);

/// @notice Gets the proxy for the given owner and salt.
/// @param owner The address of the owner of the proxy.
/// @param salt The data used as an additional input to CREATE2.
function getProxy(address owner, bytes32 salt) external view returns (IPRBProxy proxy);

/// PUBLIC NON-CONSTANT FUNCTIONS ///

/// @notice Deploys a new proxy instance via the proxy factory.
/// @dev Sets "msg.sender" as the owner of the proxy.
///
/// Requirements:
/// - All from "deployFor".
///
/// @return proxy The address of the newly deployed proxy contract.
function deploy() external returns (address payable proxy);

/// @notice Deploys a new proxy instance via the proxy factory, for a specific owner.
/// @notice Deploys a new proxy instance via the proxy factory, for the given owner.
///
/// @dev Requirements:
/// - The proxy must either not exist or its ownership must have been transferred by the owner.
///
/// @param owner The owner of the proxy.
/// @return proxy The address of the newly deployed proxy contract.
function deployFor(address owner) external returns (address payable proxy);
Expand Down
28 changes: 14 additions & 14 deletions contracts/PRBProxyFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import "./IPRBProxy.sol";
import "./IPRBProxyFactory.sol";

/// @notice Emitted when the deployment of an EIP-1167 clone with CREATE2 fails.
error PRBProxyFactory__CloneFailed(bytes32 salt);
error PRBProxyFactory__CloneFailed(bytes32 finalSalt);

/// @title PRBProxyFactory
/// @author Paul Razvan Berg
Expand All @@ -18,10 +18,10 @@ contract PRBProxyFactory is IPRBProxyFactory {
/// INTERNAL STORAGE ///

/// @dev Internal mapping to track all deployed proxies.
mapping(address => bool) internal _isProxy;
mapping(address => bool) internal proxies;

/// @dev Internal mapping to track used salts per EOA.
mapping(address => bytes32) internal _salts;
/// @dev Internal mapping to track the next salt to be used by an EOA.
mapping(address => bytes32) internal nextSalts;

/// CONSTRUCTOR ///

Expand All @@ -32,13 +32,13 @@ contract PRBProxyFactory is IPRBProxyFactory {
/// PUBLIC CONSTANT FUNCTIONS ///

/// @inheritdoc IPRBProxyFactory
function isProxy(address proxy) external view override returns (bool result) {
result = _isProxy[proxy];
function getNextSalt(address eoa) external view override returns (bytes32 nextSalt) {
nextSalt = nextSalts[eoa];
}

/// @inheritdoc IPRBProxyFactory
function salts(address eoa) external view override returns (bytes32 salt) {
salt = _salts[eoa];
function isProxy(address proxy) external view override returns (bool result) {
result = proxies[proxy];
}

/// PUBLIC NON-CONSTANT FUNCTIONS ///
Expand All @@ -50,7 +50,7 @@ contract PRBProxyFactory is IPRBProxyFactory {

/// @inheritdoc IPRBProxyFactory
function deployFor(address owner) public override returns (address payable proxy) {
bytes32 salt = _salts[tx.origin];
bytes32 salt = nextSalts[tx.origin];

// Prevent front-running the salt by hashing the concatenation of "tx.origin" and the user-provided salt.
bytes32 finalSalt = keccak256(abi.encode(tx.origin, salt));
Expand All @@ -62,11 +62,11 @@ contract PRBProxyFactory is IPRBProxyFactory {
IPRBProxy(proxy).initialize(owner);

// Mark the proxy as deployed.
_isProxy[proxy] = true;
proxies[proxy] = true;

// Increment the salt.
unchecked {
_salts[tx.origin] = bytes32(uint256(salt) + 1);
nextSalts[tx.origin] = bytes32(uint256(salt) + 1);
}

// Log the proxy via en event.
Expand All @@ -76,17 +76,17 @@ contract PRBProxyFactory is IPRBProxyFactory {
/// INTERNAL NON-CONSTANT FUNCTIONS ///

/// @dev Deploys an EIP-1167 clone that mimics the behavior of `implementation`.
function clone(bytes32 salt) internal returns (address payable proxy) {
function clone(bytes32 finalSalt) internal returns (address payable proxy) {
bytes20 impl = bytes20(address(implementation));
assembly {
let bytecode := mload(0x40)
mstore(bytecode, 0x3d602d80600a3d3981f3363d3d373d3d3d363d73000000000000000000000000)
mstore(add(bytecode, 0x14), impl)
mstore(add(bytecode, 0x28), 0x5af43d82803e903d91602b57fd5bf30000000000000000000000000000000000)
proxy := create2(0, bytecode, 0x37, salt)
proxy := create2(0, bytecode, 0x37, finalSalt)
}
if (proxy == address(0)) {
revert PRBProxyFactory__CloneFailed(salt);
revert PRBProxyFactory__CloneFailed(finalSalt);
}
}
}
36 changes: 31 additions & 5 deletions contracts/PRBProxyRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ import "./IPRBProxy.sol";
import "./IPRBProxyFactory.sol";
import "./IPRBProxyRegistry.sol";

/// @notice Emitted when a proxy already exists for the given owner.
error PRBProxyRegistry__ProxyAlreadyExists(address owner);

/// @title PRBProxyRegistry
/// @author Paul Razvan Berg
contract PRBProxyRegistry is IPRBProxyRegistry {
Expand All @@ -16,7 +19,10 @@ contract PRBProxyRegistry is IPRBProxyRegistry {
/// INTERNAL STORAGE ///

/// @notice Internal mapping of owners to salts to proxies.
mapping(address => mapping(bytes32 => IPRBProxy)) internal _proxies;
mapping(address => mapping(bytes32 => IPRBProxy)) internal proxies;

/// @dev Internal mapping to track the last used by an EOA.
mapping(address => bytes32) internal lastSalts;

/// CONSTRUCTOR ///

Expand All @@ -27,8 +33,19 @@ contract PRBProxyRegistry is IPRBProxyRegistry {
/// PUBLIC CONSTANT FUNCTIONS ///

/// @inheritdoc IPRBProxyRegistry
function proxies(address owner, bytes32 salt) external view override returns (IPRBProxy proxy) {
proxy = _proxies[owner][salt];
function getLastProxy(address owner) public view override returns (IPRBProxy proxy) {
bytes32 lastSalt = lastSalts[owner];
proxy = proxies[owner][lastSalt];
}

/// @inheritdoc IPRBProxyRegistry
function getLastSalt(address owner) external view override returns (bytes32 lastSalt) {
lastSalt = lastSalts[owner];
}

/// @inheritdoc IPRBProxyRegistry
function getProxy(address owner, bytes32 salt) external view override returns (IPRBProxy proxy) {
proxy = proxies[owner][salt];
}

/// PUBLIC NON-CONSTANT FUNCTIONS ///
Expand All @@ -40,12 +57,21 @@ contract PRBProxyRegistry is IPRBProxyRegistry {

/// @inheritdoc IPRBProxyRegistry
function deployFor(address owner) public override returns (address payable proxy) {
bytes32 salt = bytes32(factory.salts(tx.origin));
IPRBProxy lastProxy = getLastProxy(owner);

// Do not deploy if the proxy already exists and the owner is the same.
if (address(lastProxy) != address(0) && lastProxy.owner() == owner) {
revert PRBProxyRegistry__ProxyAlreadyExists(owner);
}
bytes32 salt = bytes32(factory.getNextSalt(tx.origin));

// Deploy the proxy via the factory.
proxy = factory.deployFor(owner);

// Save the proxy in the mapping.
_proxies[owner][salt] = IPRBProxy(proxy);
proxies[owner][salt] = IPRBProxy(proxy);

// Save the last salt.
lastSalts[owner] = salt;
}
}
4 changes: 2 additions & 2 deletions contracts/test/GodModePRBProxyFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ contract GodModePRBProxyFactory is PRBProxyFactory {
// solhint-disable-previous-line no-empty-blocks
}

function __godMode_clone(bytes32 salt) external returns (address payable proxy) {
proxy = clone(salt);
function __godMode_clone(bytes32 finalSalt) external returns (address payable proxy) {
proxy = clone(finalSalt);
}
}
2 changes: 1 addition & 1 deletion dist/addresses.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const addresses = {
export function computeProxyAddress(this: Mocha.Context, deployer: string, salt: string): string {
const cloneBytecode: string[] = ["3d602d80600a3d3981f3363d3d373d3d3d363d73", "5af43d82803e903d91602b57fd5bf3"];
return getCreate2Address(
this.contracts.prbProxyFactory.address,
addresses.PRBProxyFactory,
computeFinalSalt(deployer, salt),
solidityKeccak256(["bytes"], ["0x" + cloneBytecode[0] + addresses.PRBProxy + cloneBytecode[1]]),
);
Expand Down
1 change: 0 additions & 1 deletion hardhat.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import "hardhat-gas-reporter";
import "hardhat-packager";
import "solidity-coverage";

import "./tasks/deploy";
import "./tasks/clean";

import { resolve } from "path";
Expand Down
2 changes: 2 additions & 0 deletions helpers/constants.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
// See https://github.com/Zoltu/deterministic-deployment-proxy
export const DETERMINISTIC_DEPLOYMENT_PROXY_ADDRESS: string = "0x7A0D94F55792C434d74a40883C6ed8545E406D12";
export const SALT_ONE: string = "0x0000000000000000000000000000000000000000000000000000000000000001";
export const SALT_THREE: string = "0x0000000000000000000000000000000000000000000000000000000000000003";
export const SALT_TWO: string = "0x0000000000000000000000000000000000000000000000000000000000000002";
export const SALT_ZERO: string = "0x0000000000000000000000000000000000000000000000000000000000000000";
4 changes: 2 additions & 2 deletions test/integration/prbProxyFactory/effects/clone.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ export default function shouldBehaveLikeClone(): void {
let proxyAddress: string;
let finalSalt: string;

beforeEach(function () {
beforeEach(async function () {
const deployer: SignerWithAddress = this.signers.alice;
finalSalt = computeFinalSalt(this.signers.alice.address, SALT_ZERO);
proxyAddress = computeProxyAddress.call(this, deployer.address, SALT_ZERO);
proxyAddress = await computeProxyAddress.call(this, deployer.address);
});

context("when the final salt was used", function () {
Expand Down
16 changes: 8 additions & 8 deletions test/integration/prbProxyFactory/effects/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ export default function shouldBehaveLikeDeploy(): void {
let deployer: SignerWithAddress;
let proxyAddress: string;

beforeEach(function () {
beforeEach(async function () {
deployer = this.signers.alice;
proxyAddress = computeProxyAddress.call(this, deployer.address, SALT_ZERO);
proxyAddress = await computeProxyAddress.call(this, deployer.address);
});

it("deploys the proxy", async function () {
Expand All @@ -22,16 +22,16 @@ export default function shouldBehaveLikeDeploy(): void {
expect(deployedBytecode).to.equal(expectedBytecode);
});

it("updates the isProxy mapping", async function () {
it("updates the nextSalts mapping", async function () {
await this.contracts.prbProxyFactory.connect(deployer).deploy();
const isProxy: boolean = await this.contracts.prbProxyFactory.isProxy(proxyAddress);
expect(isProxy).to.equal(true);
const nextSalt: string = await this.contracts.prbProxyFactory.getNextSalt(deployer.address);
expect(nextSalt).to.equal(SALT_ONE);
});

it("updates the salts mapping", async function () {
it("updates the proxies mapping", async function () {
await this.contracts.prbProxyFactory.connect(deployer).deploy();
const salt: string = await this.contracts.prbProxyFactory.salts(deployer.address);
expect(salt).to.equal(SALT_ONE);
const isProxy: boolean = await this.contracts.prbProxyFactory.isProxy(proxyAddress);
expect(isProxy).to.equal(true);
});

it("emits a DeployProxy event", async function () {
Expand Down
16 changes: 8 additions & 8 deletions test/integration/prbProxyFactory/effects/deployFor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ export default function shouldBehaveLikeDeployFor(): void {
let owner: SignerWithAddress;
let proxyAddress: string;

beforeEach(function () {
beforeEach(async function () {
deployer = this.signers.alice;
expectedBytecode = getCloneDeployedBytecode(this.contracts.prbProxyImplementation.address);
owner = this.signers.bob;
proxyAddress = computeProxyAddress.call(this, deployer.address, SALT_ZERO);
proxyAddress = await computeProxyAddress.call(this, deployer.address);
});

context("when the owner is the zero address", function () {
Expand All @@ -45,16 +45,16 @@ export default function shouldBehaveLikeDeployFor(): void {
expect(deployedBytecode).to.equal(expectedBytecode);
});

it("updates the isProxy mapping", async function () {
it("updates the nextSalts mapping", async function () {
await this.contracts.prbProxyFactory.connect(deployer).deployFor(owner.address);
const isProxy: boolean = await this.contracts.prbProxyFactory.isProxy(proxyAddress);
expect(isProxy).to.equal(true);
const nextSalt: string = await this.contracts.prbProxyFactory.getNextSalt(deployer.address);
expect(nextSalt).to.equal(SALT_ONE);
});

it("updates the salts mapping", async function () {
it("updates the proxies mapping", async function () {
await this.contracts.prbProxyFactory.connect(deployer).deployFor(owner.address);
const salt: string = await this.contracts.prbProxyFactory.salts(deployer.address);
expect(salt).to.equal(SALT_ONE);
const isProxy: boolean = await this.contracts.prbProxyFactory.isProxy(proxyAddress);
expect(isProxy).to.equal(true);
});

it("emits a DeployProxy event", async function () {
Expand Down
14 changes: 10 additions & 4 deletions test/integration/prbProxyRegistry/effects/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ export default function shouldBehaveLikeDeploy(): void {
let deployer: SignerWithAddress;
let proxyAddress: string;

beforeEach(function () {
beforeEach(async function () {
deployer = this.signers.alice;
proxyAddress = computeProxyAddress.call(this, deployer.address, SALT_ZERO);
proxyAddress = await computeProxyAddress.call(this, deployer.address);
});

it("deploys the proxy", async function () {
Expand All @@ -24,7 +24,13 @@ export default function shouldBehaveLikeDeploy(): void {

it("updates the proxies mapping", async function () {
await this.contracts.prbProxyRegistry.connect(deployer).deploy();
const newProxyAddress: string = await this.contracts.prbProxyRegistry.proxies(deployer.address, SALT_ZERO);
expect(proxyAddress).to.equal(newProxyAddress);
const proxy: string = await this.contracts.prbProxyRegistry.getProxy(deployer.address, SALT_ZERO);
expect(proxyAddress).to.equal(proxy);
});

it("updates the lastSalts mapping", async function () {
await this.contracts.prbProxyRegistry.connect(deployer).deploy();
const lastSalt: string = await this.contracts.prbProxyRegistry.getLastSalt(deployer.address);
expect(lastSalt).to.equal(SALT_ZERO);
});
}
Loading

0 comments on commit a94f1de

Please sign in to comment.