test storage persistence after registry upgrade #2
Conversation
src/VerifiableFactory.sol
Outdated
| // reconstruct the address using CREATE2 and verify it matches | ||
| bytes32 outerSalt = keccak256(abi.encode(msg.sender, salt)); | ||
| function _verifyContract(address proxy, address creator, address owner, uint256 salt) private view returns (bool) { | ||
| // verify the creator matches this factory |
There was a problem hiding this comment.
Is there any need for this check, or for the proxy to be able to return the creator? If it doesn't match, the address will not validate.
There was a problem hiding this comment.
oh right, since creator is the part of the address creation, this is unnecessary. gonna remove it.
| @@ -9,8 +9,6 @@ interface IProxy { | |||
| function salt() external view returns (uint256); | |||
|
|
|||
| function owner() external view returns (address); | |||
|
|
|||
| function creator() external view returns (address); | |||
There was a problem hiding this comment.
It looks like you deleted it from the interface but not from the code?
Thinking about it, it may still be useful to have; otherwise there's no way to know which factory to check if all you have is an instance.
There was a problem hiding this comment.
the interface here is only under the VerifiableFactory scope, the actual creator is a public immutable variable which will have the auto-generated getter interface, do we need to have it explicitly under VerifiableFactory if not in use?
There was a problem hiding this comment.
Ah, good point. No, but we should be having the proxy implement the same interface we use here - otherwise there could be a mismatch between what the proxy implements and the interface specifies and we wouldn't know at compile-time.
There was a problem hiding this comment.
surely, both salt and the owner implements exactly as stated here. Then I will create a common interface shared between Proxy contract and Factory contract, just to be sure there won't be any mismatch for the future changes.
No description provided.