Description

In the getDeployed function of the VaultFactory contract, there is a comment claiming that the function hashes the provided salt with the deployer's address to give each deployer its own namespace. However, the implementation does not actually include the deployer's address in the hash calculation. It only hashes the salt parameter itself without incorporating any deployer-specific information.

function getDeployed(bytes32 salt) external view returns (address) {
    // hash salt with the deployer address to give each deployer its own namespace
    salt = keccak256(abi.encodePacked(salt));
    return CREATE3.predictDeterministicAddress(salt);
}

Impact

This inconsistency between the comment and the actual implementation could lead to misunderstandings by developers or auditors reviewing the code. It also may create a false sense of security regarding namespace isolation between different deployers.

Recommendations

To align the implementation with the comment, modify the getDeployed function to include the deployer's address in the salt hashing:

function getDeployed(bytes32 salt) external view returns (address) {
    // hash salt with the deployer address to give each deployer its own namespace
    salt = keccak256(abi.encodePacked(msg.sender, salt));
    return CREATE3.predictDeterministicAddress(salt);
}

Alternatively, if the intention is not to include the deployer's address, update the comment to accurately reflect the current implementation:

function getDeployed(bytes32 salt) external view returns (address) {
    // hash salt to get a deterministic address
    salt = keccak256(abi.encodePacked(salt));
    return CREATE3.predictDeterministicAddress(salt);
}

Remediation

This issue has been acknowledged by Orderly Network, and a fix was implemented in commit aa898b99↗.