fix: use owner as CREATE2 salt #130
Conversation
docs: improve writing in README refactor: delete "nextSeeds" refactor: delete stale CREATE2 seeds refactor: remove "origin" in "DeployProxy" event test: remove unused constants test: update tests in light of new contract logic test: remove stale "origin" fuzzing
|
🤖 OpenAI High-level SummaryThe changes involve significant modifications to the logic and functionality of the code. They introduce improvements to the PRBProxy contract, including the use of CREATE2 for pre-computing the proxy contract's address, generating CREATE2 salts to prevent front-running, and ensuring immutability of the proxy owner during DELEGATECALL. Unnecessary operations and constants are removed. The test contracts are also updated to reflect the changes in the logic and functionality. Additionally, an event signature is modified. The changes require a thorough review to ensure correctness and avoid unintended consequences. File Summaries
The files have been triaged as Chat with 🤖 OpenAI Bot (
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| address payable eve; | ||
| } | ||
|
|
||
| /*////////////////////////////////////////////////////////////////////////// | ||
| CONSTANTS | ||
| //////////////////////////////////////////////////////////////////////////*/ | ||
|
|
||
| uint256 internal constant DEFAULT_MIN_GAS_RESERVE = 5000; | ||
| bytes32 internal constant SEED_ONE = bytes32(uint256(0x01)); | ||
| bytes32 internal constant SEED_TWO = bytes32(uint256(0x02)); | ||
| bytes32 internal constant SEED_ZERO = bytes32(uint256(0x00)); | ||
|
|
||
| /*////////////////////////////////////////////////////////////////////////// | ||
| VARIABLES | ||
| //////////////////////////////////////////////////////////////////////////*/ |
There was a problem hiding this comment.
🤖 OpenAI
The constant declarations DEFAULT_MIN_GAS_RESERVE, SEED_ONE, SEED_TWO, and SEED_ZERO have been removed. Please ensure that these constants are no longer used in the codebase and update any references accordingly.
| deployRegistryConditionally(); | ||
|
|
||
| // Make Alice both the caller and the origin. | ||
| vm.startPrank({ msgSender: users.alice, txOrigin: users.alice }); | ||
| vm.startPrank({ msgSender: users.alice }); | ||
| } | ||
|
|
||
| /*////////////////////////////////////////////////////////////////////////// | ||
| HELPERS | ||
| //////////////////////////////////////////////////////////////////////////*/ | ||
|
|
||
| /// @dev Computes the proxy address without deploying it. | ||
| function computeProxyAddress(address origin, bytes32 seed) internal returns (address) { | ||
| bytes32 salt = keccak256(abi.encode(origin, seed)); | ||
| function computeProxyAddress(address owner) internal returns (address) { | ||
| bytes32 salt = bytes32(abi.encodePacked(owner)); | ||
| bytes32 creationBytecodeHash = keccak256(getProxyBytecode()); | ||
| // Use the Create2 utility from Forge Std. | ||
| return computeCreate2Address({ salt: salt, initcodeHash: creationBytecodeHash, deployer: address(registry) }); |
There was a problem hiding this comment.
🤖 OpenAI
The function computeProxyAddress has been modified. The parameter origin has been removed, and the parameter owner has been added. The computation of the salt has also changed from keccak256(abi.encode(origin, seed)) to bytes32(abi.encodePacked(owner)). Please verify that this change is intentional and update any references to origin accordingly.
| /// @inheritdoc IPRBProxyRegistry | ||
| ConstructorParams public override constructorParams; | ||
|
|
||
| /// @inheritdoc IPRBProxyRegistry | ||
| mapping(address origin => bytes32 seed) public override nextSeeds; | ||
|
|
||
| /*////////////////////////////////////////////////////////////////////////// | ||
| INTERNAL STORAGE | ||
| //////////////////////////////////////////////////////////////////////////*/ |
There was a problem hiding this comment.
🤖 OpenAI
The nextSeeds mapping has been removed from the contract. This change eliminates the need for the mapping(address origin => bytes32 seed) declaration and the associated code that uses it. The removal of this mapping simplifies the contract logic and reduces storage usage.
| noProxy(msg.sender) | ||
| returns (IPRBProxy proxy) | ||
| { | ||
| // Load the next seed. | ||
| bytes32 seed = nextSeeds[tx.origin]; | ||
|
|
||
| // Prevent front-running the salt by hashing the concatenation of "tx.origin" and the user-provided seed. | ||
| bytes32 salt = keccak256(abi.encode(tx.origin, seed)); | ||
|
|
||
| // Set the constructor params. | ||
| // Use the address of the owner as the CREATE2 salt. | ||
| address owner = msg.sender; | ||
| constructorParams = ConstructorParams({ owner: owner, target: target, data: data }); | ||
| bytes32 salt = bytes32(abi.encodePacked(owner)); | ||
|
|
||
| // Deploy the proxy with CREATE2. | ||
| // Deploy the proxy with CREATE2, and execute the delegate call in the constructor. | ||
| constructorParams = ConstructorParams({ owner: owner, target: target, data: data }); | ||
| proxy = new PRBProxy{ salt: salt }(); | ||
| delete constructorParams; | ||
|
|
||
| // Associate the the owner with the proxy in the mapping. | ||
| // Associate the owner and the proxy. | ||
| _proxies[owner] = proxy; | ||
|
|
||
| // Increment the seed. | ||
| // Using unchecked arithmetic here because this cannot realistically overflow, ever. | ||
| unchecked { | ||
| nextSeeds[tx.origin] = bytes32(uint256(seed) + 1); | ||
| } | ||
|
|
||
| // Log the proxy via en event. | ||
| // forgefmt: disable-next-line | ||
| emit DeployProxy({ | ||
| origin: tx.origin, | ||
| operator: msg.sender, | ||
| owner: owner, | ||
| seed: seed, | ||
| salt: salt, | ||
| proxy: proxy | ||
| }); | ||
| // Log the creation of the proxy. | ||
| emit DeployProxy({ operator: msg.sender, owner: owner, proxy: proxy }); | ||
| } |
There was a problem hiding this comment.
🤖 OpenAI
The function _deploy has been modified to use the owner's address as the CREATE2 salt instead of using a combination of tx.origin and a seed. This change improves security by preventing front-running attacks. Additionally, the constructorParams struct is updated to include the owner, target, and data values. The constructor parameters are set before deploying the proxy contract with CREATE2. After deployment, the constructor parameters are deleted, and the owner is associated with the proxy in the _proxies mapping. Finally, an event is emitted to log the creation of the proxy.
| proxy = new PRBProxy{ salt: salt }(); | ||
| delete constructorParams; | ||
|
|
||
| // Associate the the owner with the proxy in the mapping. | ||
| // Associate the owner and the proxy. | ||
| _proxies[owner] = proxy; | ||
|
|
||
| // Increment the seed. | ||
| // We're using unchecked arithmetic here because this cannot realistically overflow, ever. | ||
| unchecked { | ||
| nextSeeds[tx.origin] = bytes32(uint256(seed) + 1); | ||
| } | ||
|
|
||
| // Log the proxy via en event. | ||
| // forgefmt: disable-next-line | ||
| emit DeployProxy({ | ||
| origin: tx.origin, | ||
| operator: msg.sender, | ||
| owner: owner, | ||
| seed: seed, | ||
| salt: salt, | ||
| proxy: proxy | ||
| }); | ||
| // Log the creation of the proxy. | ||
| emit DeployProxy({ operator: msg.sender, owner: owner, proxy: proxy }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🤖 OpenAI
Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [227-245]
The same changes made in the previous section (lines 136-153) have been applied to the _deploy function in this section as well. The function now uses the owner's address as the CREATE2 salt, sets the constructor parameters, deploys the proxy contract, associates the owner with the proxy, and emits an event to log the creation of the proxy.
No substantive issues were detected in the provided code changes.
| └── when the owner does not have a proxy | ||
| ├── it should generate the correct address | ||
| ├── it should initialize the owner | ||
| ├── it should update the next seeds mapping | ||
| ├── it should update the proxies mapping |
There was a problem hiding this comment.
🤖 OpenAI
The comment chains indicate that the nextSeeds mapping has been removed and replaced with the proxies mapping. However, the code in the new hunk still references the old next seeds mapping. Please update the code to use the correct proxies mapping instead.
---new_hunk---
4: └── when the owner does not have a proxy
5: ├── it should generate the correct address
6: ├── it should initialize the owner
7: ├── it should update the proxies mapping
8: ├── it should delegate call to the target contract
9: ├── it should emit a {DeployProxy} event
---old_hunk---
└── when the owner does not have a proxy
├── it should generate the correct address
├── it should initialize the owner
├── it should update the next seeds mapping
├── it should update the proxies mapping
├── it should delegate call to the target contract
├── it should emit a {DeployProxy} event
---comment_chains---
// Comment chain 1
// Removed nextSeeds mapping
// Comment chain 2
// Replaced nextSeeds with proxies
---end_change_section---
| └── when the owner does not have a proxy | ||
| ├── it should generate the correct address | ||
| ├── it should initialize the owner | ||
| ├── it should update the next seeds mapping | ||
| ├── it should update the proxies mapping | ||
| └── it should emit a {DeployProxy} event |
There was a problem hiding this comment.
🤖 OpenAI
The comment chain indicates that the nextSeeds mapping has been removed and replaced with the proxies mapping. However, the code snippet still references the old mapping name next seeds mapping. Please update the code to reflect the correct mapping name.
4: └── when the owner does not have a proxy
5: ├── it should generate the correct address
6: ├── it should initialize the owner
7: ├── it should update the proxies mapping
8: └── it should emit a {DeployProxy} eventNo changes needed.
| //////////////////////////////////////////////////////////////////////////*/ | ||
|
|
||
| /// @notice Emitted when a new proxy is deployed. | ||
| event DeployProxy( | ||
| address indexed origin, | ||
| address indexed operator, | ||
| address indexed owner, | ||
| bytes32 seed, | ||
| bytes32 salt, | ||
| IPRBProxy proxy | ||
| ); | ||
| event DeployProxy(address indexed operator, address indexed owner, IPRBProxy proxy); | ||
|
|
||
| /// @notice Emitted when a plugin is installed. | ||
| event InstallPlugin(address indexed owner, IPRBProxy indexed proxy, IPRBProxyPlugin indexed plugin); |
There was a problem hiding this comment.
🤖 OpenAI
The event signatures in IPRBProxyRegistry.sol have been modified. The DeployProxy event no longer includes the origin, seed, and salt parameters, and the InstallPlugin event remains unchanged. Please ensure that these changes are reflected in the event emissions throughout the codebase.
| /// @param owner The user address to make the query for. | ||
| function getProxy(address owner) external view returns (IPRBProxy proxy); | ||
|
|
There was a problem hiding this comment.
🤖 OpenAI
The nextSeeds function has been removed from the interface. Please remove this function from the implementation as well.
| /// @return proxy The address of the newly deployed proxy contract. | ||
| function deploy() external returns (IPRBProxy proxy); | ||
|
|
||
| /// @notice Deploys a new proxy via CREATE2 by setting the caller as the owner, and delegate calls to the provided | ||
| /// target contract by forwarding the data. It returns the data it gets back, and bubbles up any potential revert. | ||
| /// @notice Deploys a new proxy via CREATE2, using the caller as the owner. It delegate calls to the provided | ||
| /// target contract by forwarding the data. Then, it returns the data it gets back, and bubbles up any potential | ||
| /// revert. | ||
| /// | ||
| /// @dev Emits a {DeployProxy} and an {Execute} event. | ||
| /// |
There was a problem hiding this comment.
🤖 OpenAI
The deploy function has been updated to include additional functionality. Please review the changes and ensure that the new implementation is correct.
| /// @dev Emits an {InstallPlugin} event. | ||
| /// | ||
| /// Notes: | ||
| /// - Installing a plugin is a potentially dangerous operation, because anyone will be able to run the plugin. | ||
| /// - Plugin methods that have the same selector as {PRBProxy.execute} will be installed, but they will never be run | ||
| /// by the proxy. | ||
| /// - Installing a plugin is a potentially dangerous operation, because anyone can run the plugin. | ||
| /// - Plugin methods that have the same selector as {PRBProxy.execute} can be installed, but they can never be run. | ||
| /// | ||
| /// Requirements: | ||
| /// - The caller must have a proxy. |
There was a problem hiding this comment.
🤖 OpenAI
The comments for the installPlugin function have been updated. Please review the changes and ensure that the comments accurately reflect the behavior of the function.
| /// Requirements: | ||
| /// - The caller must have a proxy. | ||
| /// | ||
| /// @param envoy The address of the envoy account. | ||
| /// @param envoy The address of the account given permission to call the target contract. | ||
| /// @param target The address of the target contract. | ||
| /// @param permission The boolean permission to set. | ||
| function setPermission(address envoy, address target, bool permission) external; |
There was a problem hiding this comment.
🤖 OpenAI
The comments for the setPermission function have been updated. Please review the changes and ensure that the comments accurately reflect the behavior of the function.
| /// Requirements: | |
| /// - The caller must have a proxy. | |
| /// | |
| /// @param envoy The address of the envoy account. | |
| /// @param envoy The address of the account given permission to call the target contract. | |
| /// @param target The address of the target contract. | |
| /// @param permission The boolean permission to set. | |
| function setPermission(address envoy, address target, bool permission) external; | |
| //////////////////////////////////////////////////////////////////////////*/ | |
| /// @notice Emitted when a new proxy is deployed. | |
| event DeployProxy(address indexed operator, address indexed owner, IPRBProxy proxy); | |
| /// @notice Emitted when a plugin is installed. | |
| event InstallPlugin(address indexed owner, IPRBProxy indexed proxy, IPRBProxyPlugin indexed plugin); |
| /// Requirements: | |
| /// - The caller must have a proxy. | |
| /// | |
| /// @param envoy The address of the envoy account. | |
| /// @param envoy The address of the account given permission to call the target contract. | |
| /// @param target The address of the target contract. | |
| /// @param permission The boolean permission to set. | |
| function setPermission(address envoy, address target, bool permission) external; | |
| /// @param owner The user address to make the query for. | |
| function getProxy(address owner) external view returns (IPRBProxy proxy); | |
| /*////////////////////////////////////////////////////////////////////////// | |
| NON-CONSTANT FUNCTIONS | |
| //////////////////////////////////////////////////////////////////////////*/ | |
| /// @notice Deploys a new proxy with CREATE2, using the caller as the owner. | |
| /// | |
| /// @dev Emits a {DeployProxy} event. | |
| /// |
| /// Requirements: | |
| /// - The caller must have a proxy. | |
| /// | |
| /// @param envoy The address of the envoy account. | |
| /// @param envoy The address of the account given permission to call the target contract. | |
| /// @param target The address of the target contract. | |
| /// @param permission The boolean permission to set. | |
| function setPermission(address envoy, address target, bool permission) external; | |
| /// @return proxy The address of the newly deployed proxy contract. | |
| function deploy() external returns (IPRBProxy proxy); | |
| /// @notice Deploys a new proxy via CREATE2, using the caller as the owner. It delegate calls to the provided | |
| /// target contract by forwarding the data. Then, it returns the data it gets back, and bubbles up any potential | |
| /// revert. | |
| /// | |
| /// @dev Emits a {DeployProxy} and an {Execute} event. | |
| /// |
| /// Requirements: | |
| /// - The caller must have a proxy. | |
| /// | |
| /// @param envoy The address of the envoy account. | |
| /// @param envoy The address of the account given permission to call the target contract. | |
| /// @param target The address of the target contract. | |
| /// @param permission The boolean permission to set. | |
| function setPermission(address envoy, address target, bool permission) external; | |
| /// @dev Emits an {InstallPlugin} event. | |
| /// | |
| /// Notes: | |
| /// - Installing a plugin is a potentially dangerous operation, because anyone can run the plugin. | |
| /// - Plugin methods that have the same selector as {PRBProxy.execute} can be installed, but they can never be run. | |
| /// | |
| /// Requirements: | |
| /// - The caller must have a proxy. |
| /// Requirements: | |
| /// - The caller must have a proxy. | |
| /// | |
| /// @param envoy The address of the envoy account. | |
| /// @param envoy The address of the account given permission to call the target contract. | |
| /// @param target The address of the target contract. | |
| /// @param permission The boolean permission to set. | |
| function setPermission(address envoy, address target, bool permission) external; | |
| /// Requirements: | |
| /// - The caller must have a proxy. | |
| /// | |
| /// @param envoy The address of the account given permission to call the target contract. | |
| /// @param target The address of the target contract. | |
| /// @param permission The boolean permission to set. | |
| function setPermission(address envoy, address target, bool permission) external; |
| function testFuzz_DeployFor_ProxyAddress(address operator, address owner) external whenOwnerDoesNotHaveProxy { | ||
| changePrank({ msgSender: operator }); | ||
| address actualProxy = address(registry.deployFor(owner)); |
There was a problem hiding this comment.
🤖 OpenAI
The changePrank function is called with the msgSender parameter, but it should be called with the operator parameter instead. This could lead to incorrect behavior in the test case.
| function testFuzz_DeployFor_ProxyAddress(address operator, address owner) external whenOwnerDoesNotHaveProxy { | |
| changePrank({ msgSender: operator }); | |
| address actualProxy = address(registry.deployFor(owner)); | |
| function testFuzz_DeployFor_ProxyAddress(address operator, address owner) external whenOwnerDoesNotHaveProxy { | |
| changePrank({ msgSender: operator }); | |
| address actualProxy = address(registry.deployFor(owner)); | |
| address expectedProxy = computeProxyAddress(owner); | |
| assertEq(actualProxy, expectedProxy, "deployed proxy address mismatch"); | |
| } |
| function testFuzz_DeployFor_ProxyOwner(address operator, address owner) external whenOwnerDoesNotHaveProxy { | ||
| changePrank({ msgSender: operator }); | ||
| IPRBProxy proxy = registry.deployFor(owner); |
There was a problem hiding this comment.
🤖 OpenAI
The changePrank function is called with the msgSender parameter, but it should be called with the operator parameter instead. This could lead to incorrect behavior in the test case.
| function testFuzz_DeployFor_ProxyOwner(address operator, address owner) external whenOwnerDoesNotHaveProxy { | |
| changePrank({ msgSender: operator }); | |
| IPRBProxy proxy = registry.deployFor(owner); | |
| function testFuzz_DeployFor_ProxyOwner(address operator, address owner) external whenOwnerDoesNotHaveProxy { | |
| changePrank({ msgSender: operator }); | |
| IPRBProxy proxy = registry.deployFor(owner); | |
| address actualOwner = proxy.owner(); | |
| address expectedOwner = owner; | |
| assertEq(actualOwner, expectedOwner, "proxy owner mismatch"); | |
| } |
| function testFuzz_DeployFor_UpdateProxies(address operator, address owner) external whenOwnerDoesNotHaveProxy { | ||
| changePrank({ msgSender: operator }); | ||
| registry.deployFor(owner); |
There was a problem hiding this comment.
🤖 OpenAI
The changePrank function is called with the msgSender parameter, but it should be called with the operator parameter instead. This could lead to incorrect behavior in the test case.
| function testFuzz_DeployFor_UpdateProxies(address operator, address owner) external whenOwnerDoesNotHaveProxy { | |
| changePrank({ msgSender: operator }); | |
| registry.deployFor(owner); | |
| function testFuzz_DeployFor_UpdateProxies(address operator, address owner) external whenOwnerDoesNotHaveProxy { | |
| changePrank({ msgSender: operator }); | |
| registry.deployFor(owner); | |
| address actualProxyAddress = address(registry.getProxy(owner)); | |
| address expectedProxyAddress = computeProxyAddress(owner); | |
| assertEq(actualProxyAddress, expectedProxyAddress, "proxy address mismatch"); | |
| } |
| function testFuzz_DeployFor_Event(address operator, address owner) external whenOwnerDoesNotHaveProxy { | ||
| changePrank({ msgSender: operator }); | ||
|
|
||
| vm.expectEmit({ emitter: address(registry) }); | ||
| emit DeployProxy({ | ||
| origin: origin, | ||
| operator: operator, | ||
| owner: owner, | ||
| seed: SEED_ZERO, | ||
| salt: keccak256(abi.encode(origin, SEED_ZERO)), | ||
| proxy: IPRBProxy(computeProxyAddress(origin, SEED_ZERO)) | ||
| }); | ||
| emit DeployProxy({ operator: operator, owner: owner, proxy: IPRBProxy(computeProxyAddress(owner)) }); | ||
| registry.deployFor(owner); | ||
| } |
There was a problem hiding this comment.
🤖 OpenAI
The changePrank function is called with the msgSender parameter, but it should be called with the operator parameter instead. This could lead to incorrect behavior in the test case.
| function testFuzz_DeployFor_Event(address operator, address owner) external whenOwnerDoesNotHaveProxy { | |
| changePrank({ msgSender: operator }); | |
| vm.expectEmit({ emitter: address(registry) }); | |
| emit DeployProxy({ | |
| origin: origin, | |
| operator: operator, | |
| owner: owner, | |
| seed: SEED_ZERO, | |
| salt: keccak256(abi.encode(origin, SEED_ZERO)), | |
| proxy: IPRBProxy(computeProxyAddress(origin, SEED_ZERO)) | |
| }); | |
| emit DeployProxy({ operator: operator, owner: owner, proxy: IPRBProxy(computeProxyAddress(owner)) }); | |
| registry.deployFor(owner); | |
| } | |
| function testFuzz_DeployFor_Event(address operator, address owner) external whenOwnerDoesNotHaveProxy { | |
| changePrank({ msgSender: operator }); | |
| vm.expectEmit({ emitter: address(registry) }); | |
| emit DeployProxy({ operator: operator, owner: owner, proxy: IPRBProxy(computeProxyAddress(owner)) }); | |
| registry.deployFor(owner); | |
| } |
Everything else looks good.
| function testFuzz_DeployAndExecute_ProxyAddress(address owner) external whenOwnerDoesNotHaveProxy { | ||
| changePrank({ msgSender: owner }); | ||
| IPRBProxy actualProxy = registry.deployAndExecute(target, data); | ||
| address expectedProxy = computeProxyAddress(origin, SEED_ZERO); | ||
| address expectedProxy = computeProxyAddress(owner); | ||
| assertEq(address(actualProxy), expectedProxy, "deployed proxy address mismatch"); | ||
| } |
There was a problem hiding this comment.
🤖 OpenAI
The function testFuzz_DeployAndExecute_ProxyAddress has been modified to accept an owner parameter instead of the origin and owner parameters. The changePrank function is called with { msgSender: owner } instead of { txOrigin: origin, msgSender: owner }. This change simplifies the function signature and removes the need for the origin parameter. No further issues are detected in this range.
| function testFuzz_DeployAndExecute_ProxyOwner(address owner) external whenOwnerDoesNotHaveProxy { | ||
| changePrank({ msgSender: owner }); | ||
| IPRBProxy proxy = registry.deployAndExecute(target, data); | ||
| address actualOwner = proxy.owner(); | ||
| address expectedOwner = owner; | ||
| assertEq(actualOwner, expectedOwner, "proxy owner mismatch"); |
There was a problem hiding this comment.
🤖 OpenAI
The function testFuzz_DeployAndExecute_ProxyOwner has been modified to accept an owner parameter instead of the origin and owner parameters. The changePrank function is called with { msgSender: owner } instead of { txOrigin: origin, msgSender: owner }. This change simplifies the function signature and removes the need for the origin parameter. No further issues are detected in this range.
| function testFuzz_DeployAndExecute_UpdateProxies(address owner) external whenOwnerDoesNotHaveProxy { | ||
| changePrank({ msgSender: owner }); | ||
| registry.deployAndExecute(target, data); | ||
|
|
||
| address actualProxyAddress = address(registry.getProxy(owner)); | ||
| address expectedProxyAddress = computeProxyAddress(origin, SEED_ZERO); | ||
| address expectedProxyAddress = computeProxyAddress(owner); | ||
| assertEq(actualProxyAddress, expectedProxyAddress, "proxy address mismatch"); | ||
| } |
There was a problem hiding this comment.
🤖 OpenAI
The function testFuzz_DeployAndExecute_UpdateProxies has been modified to accept an owner parameter instead of the origin and owner parameters. The changePrank function is called with { msgSender: owner } instead of { txOrigin: origin, msgSender: owner }. This change simplifies the function signature and removes the need for the origin parameter. No further issues are detected in this range.
| function testFuzz_DeployAndExecute_Event_DeployProxy(address owner) external whenOwnerDoesNotHaveProxy { | ||
| changePrank({ msgSender: owner }); | ||
|
|
||
| vm.expectEmit({ emitter: address(registry) }); | ||
| emit DeployProxy({ | ||
| origin: origin, | ||
| operator: owner, | ||
| owner: owner, | ||
| seed: SEED_ZERO, | ||
| salt: keccak256(abi.encode(origin, SEED_ZERO)), | ||
| proxy: IPRBProxy(computeProxyAddress(origin, SEED_ZERO)) | ||
| }); | ||
| emit DeployProxy({ operator: owner, owner: owner, proxy: IPRBProxy(computeProxyAddress(owner)) }); | ||
| registry.deployAndExecute(target, data); | ||
| } |
There was a problem hiding this comment.
🤖 OpenAI
The function testFuzz_DeployAndExecute_Event_DeployProxy has been modified to accept an owner parameter instead of the origin and owner parameters. The changePrank function is called with { msgSender: owner } instead of { txOrigin: origin, msgSender: owner }. This change simplifies the function signature and removes the need for the origin parameter. No further issues are detected in this range.
| function testFuzz_Deploy_ProxyAddress(address owner) external whenOwnerDoesNotHaveProxy { | ||
| changePrank({ msgSender: owner }); | ||
| address actualProxy = address(registry.deploy()); | ||
| address expectedProxy = computeProxyAddress({ origin: origin, seed: SEED_ZERO }); | ||
| address expectedProxy = computeProxyAddress(owner); |
There was a problem hiding this comment.
🤖 OpenAI
The testFuzz_Deploy_ProxyAddress function has been modified to accept an owner parameter instead of origin. The computeProxyAddress function is called with the owner parameter instead of origin. This change aligns with the use of owner as the CREATE2 salt in the codebase. No further issues are detected in this range.
| function testFuzz_Deploy_ProxyOwner(address owner) external whenOwnerDoesNotHaveProxy { | ||
| changePrank({ msgSender: owner }); | ||
| IPRBProxy proxy = registry.deploy(); | ||
| address actualOwner = proxy.owner(); | ||
| address expectedOwner = owner; | ||
| assertEq(actualOwner, expectedOwner, "proxy owner mismatch"); |
There was a problem hiding this comment.
🤖 OpenAI
The testFuzz_Deploy_ProxyOwner function has been modified to accept an owner parameter instead of origin. The actualOwner variable is assigned the value of proxy.owner() and the expectedOwner variable is assigned the owner parameter. No further issues are detected in this range.
| function testFuzz_Deploy_UpdateProxies(address owner) external whenOwnerDoesNotHaveProxy { | ||
| changePrank({ msgSender: owner }); | ||
| registry.deploy(); | ||
| address actualProxy = address(registry.getProxy(owner)); | ||
| address expectedProxy = computeProxyAddress({ origin: origin, seed: SEED_ZERO }); | ||
| address expectedProxy = computeProxyAddress({ owner: owner }); | ||
| assertEq(actualProxy, expectedProxy, "proxy mapping mismatch"); | ||
| } |
There was a problem hiding this comment.
🤖 OpenAI
The testFuzz_Deploy_UpdateProxies function has been modified to accept an owner parameter instead of origin. The registry.getProxy function is called with the owner parameter. The computeProxyAddress function is called with { owner: owner } instead of { origin: origin, seed: SEED_ZERO }. No further issues are detected in this range.
| function testFuzz_Deploy_Event(address owner) external whenOwnerDoesNotHaveProxy { | ||
| changePrank({ msgSender: owner }); | ||
| vm.expectEmit({ emitter: address(registry) }); | ||
| emit DeployProxy({ | ||
| origin: origin, | ||
| operator: owner, | ||
| owner: owner, | ||
| seed: SEED_ZERO, | ||
| salt: keccak256(abi.encode(origin, SEED_ZERO)), | ||
| proxy: IPRBProxy(computeProxyAddress({ origin: origin, seed: SEED_ZERO })) | ||
| }); | ||
| emit DeployProxy({ operator: owner, owner: owner, proxy: IPRBProxy(computeProxyAddress(owner)) }); | ||
| registry.deploy(); | ||
| } |
There was a problem hiding this comment.
🤖 OpenAI
The testFuzz_Deploy_Event function has been modified to accept an owner parameter instead of origin. The emit DeployProxy statement has been updated to remove the origin, seed, and salt parameters. The computeProxyAddress function is called with { owner: owner } instead of { origin: origin, seed: SEED_ZERO }. No further issues are detected in this range.
Overall, the changes made to the test/registry/deploy/deploy.t.sol file look good.
| event DeployProxy(address indexed operator, address indexed owner, IPRBProxy proxy); | ||
|
|
||
| event InstallPlugin(address indexed owner, IPRBProxy indexed proxy, IPRBProxyPlugin indexed plugin); |
There was a problem hiding this comment.
🤖 OpenAI
The event signature in the Events contract has been modified. The origin, seed, and salt parameters have been removed. Please ensure that this change does not affect any other parts of the codebase that rely on these parameters.
| event DeployProxy(address indexed operator, address indexed owner, IPRBProxy proxy); | |
| event InstallPlugin(address indexed owner, IPRBProxy indexed proxy, IPRBProxyPlugin indexed plugin); | |
| event DeployProxy(address indexed operator, address indexed owner, IPRBProxy proxy); |
Addresses https://github.com/cantinasec/review-sablier2/issues/25.
Summary by OpenAI
Release Notes:
nextSeedsmapping and replaced salt generation logic with owner's address as CREATE2 salt.originparameter and useownerparameter instead in assertions and event emissions.origin,seed, andsaltparameters.