Add entropy to MerkleLockup CREATE2 salt

[smol-ninja]

Currently in v2.2, the following parameters are used to generate salt:

bytes32 salt = keccak256(
    abi.encodePacked(
        baseParams.initialAdmin,
        baseParams.asset,
        abi.encode(baseParams.ipfsCID),
        bytes32(abi.encodePacked(baseParams.name)),
        baseParams.merkleRoot,
        baseParams.expiration,
        baseParams.cancelable,
        baseParams.transferable,
        lockupLinear,
        abi.encode(streamDurations)
    )
);

Despite name and ipfsCID being considered to be unique for each campaign, it does not eliminate the case when all the details could be the same for two distinct campaigns. As an example, a third-party integrator can choose to use the same name for all the campaigns created through them.

A potential solution is to include a nonce in the salt that gets auto-incremented for new deployments (similar to how gnosis safe works). It will require adding one storage in the SablierV2MerkleLockupFactory mapped against the caller address.

Another solution is to include block.timestamp in the salt generation. It reduces the likelihood of collision but still doesn't rectify it completely as two campaigns can still be included in the same block.

Should we pursue this in the future or the cost of developing this could be higher than the likelihood of this collision? cc @sablier-labs/engineers

Related:

• Add more informations about Airstream Campaigns unicity docs#140

[razgraf]

I'm quite curious to hear pros/cons on if this would defeat (somewhat) the purpose of CREATE2 being easier to reproduce/deploy deterministically.

[smol-ninja]

Interesting! However, I don't think it defeats the purpose of CREATE2.

From OZ doc:

CREATE2 guarantees that if sender ever deploys bytecode using CREATE2 and the provided salt, it will be stored in new_address

That's still true in this case.

[PaulRBerg]

I suggest taking a practical approach and treating this as a problem that can be solved off-chain. On-chain storage should be reserved for critical features and security requirements.

The UI could detect if two campaigns are identical and employ one of the following techniques:

• Increase the expiration by 1 second
• Make a new IPFS deployment to generate a new ipfsCID (for the same recipient data)
• Nudge the user towards renaming the campaign, e.g. "$FOO Campaign Q2 2024" instead of just "$FOO Campaign"

It is also worth noting that:

• I am reluctant to start development work on this given that we are due to hand off the final commit tomorrow
• This is a problem that occurs only when we are wildly successful - and at that time, we will afford to make another deployment
• Non-UI users could add a dummy low-value recipient in the Merkle tree

[PaulRBerg]

I'm closing this discussion now on the basis of the rationale shared above, but this should definitely be mentioned as a protocol assumption in SECURITY.md. Cc @smol-ninja to remember to add this in the changelog PR.