update salt generation to include parameters of vesting plan

contracts/factories/PrivateOfferFactory.sol

@@ -53,19 +53,10 @@ contract PrivateOfferFactory {
         address _vestingContractOwner,
         address trustedForwarder
     ) external returns (address) {
-        bytes32 salt = _getSalt(
-            _rawSalt,
-            _arguments,
-            _vestingStart,
-            _vestingCliff,
-            _vestingDuration,
-            _vestingContractOwner
-        );
-
         // deploy the vesting contract
         Vesting vesting = Vesting(
             vestingCloneFactory.createVestingCloneWithLockupPlan(
-                salt,
+                _rawSalt,
                 trustedForwarder,
                 _vestingContractOwner,
                 address(_arguments.token),
@@ -110,19 +101,16 @@ contract PrivateOfferFactory {
         address _vestingContractOwner,
         address trustedForwarder
     ) public view returns (address, address) {
-        bytes32 salt = _getSalt(
+        address vestingAddress = vestingCloneFactory.predictCloneAddressWithLockupPlan(
             _rawSalt,
-            _arguments,
+            trustedForwarder,
+            _vestingContractOwner,
+            address(_arguments.token),
+            _arguments.tokenAmount,
+            _arguments.tokenReceiver,
             _vestingStart,
             _vestingCliff,
-            _vestingDuration,
-            _vestingContractOwner
-        );
-        address vestingAddress = vestingCloneFactory.predictCloneAddress(
-            salt,
-            trustedForwarder,
-            address(vestingCloneFactory),
-            address(_arguments.token)
+            _vestingDuration
         );
 
         // since the vesting contracts address will be used as the token receiver, we need to use it for the prediction

contracts/factories/VestingCloneFactory.sol

@@ -61,8 +61,22 @@ contract VestingCloneFactory is CloneFactory {
         uint64 _cliff,
         uint64 _duration
     ) external returns (address) {
+        // generate salt from all parameters
+        bytes32 salt = keccak256(
+            abi.encode(
+                _rawSalt,
+                _trustedForwarder,
+                _owner,
+                _token,
+                _allocation,
+                _beneficiary,
+                _start,
+                _cliff,
+                _duration
+            )
+        );
         // deploy the vesting contract
-        Vesting vesting = Vesting(createVestingClone(_rawSalt, _trustedForwarder, address(this), _token));
+        Vesting vesting = Vesting(createVestingClone(salt, _trustedForwarder, address(this), _token));
 
         // create the vesting plan
         vesting.createVesting(_allocation, _beneficiary, _start, _cliff, _duration, false); // this plan is not mintable
@@ -101,4 +115,48 @@ contract VestingCloneFactory is CloneFactory {
         bytes32 salt = keccak256(abi.encode(_rawSalt, _trustedForwarder, _owner, _token));
         return Clones.predictDeterministicAddress(implementation, salt);
     }
+
+    /**
+     * Calculate the address a clone will have using the given parameters of the contract and the lockup plan
+     * @param _rawSalt value that influences the address of the clone, but not the initialization
+     * @param _trustedForwarder the trusted forwarder (ERC2771) can not be changed, but is checked for security
+     * @param _owner future owner of the vesting contract. If 0, the contract will not have an owner.
+     * @param _token token to vest
+     * @param _allocation amount of tokens to vest
+     * @param _beneficiary address receiving the tokens
+     * @param _start start date of the vesting
+     * @param _cliff cliff duration
+     * @param _duration total duration
+     */
+    function predictCloneAddressWithLockupPlan(
+        bytes32 _rawSalt,
+        address _trustedForwarder,
+        address _owner,
+        address _token,
+        uint256 _allocation,
+        address _beneficiary,
+        uint64 _start,
+        uint64 _cliff,
+        uint64 _duration
+    ) external view returns (address) {
+        require(
+            Vesting(implementation).isTrustedForwarder(_trustedForwarder),
+            "VestingCloneFactory: Unexpected trustedForwarder"
+        );
+        bytes32 salt = keccak256(
+            abi.encode(
+                _rawSalt,
+                _trustedForwarder,
+                _owner,
+                _token,
+                _allocation,
+                _beneficiary,
+                _start,
+                _cliff,
+                _duration
+            )
+        );
+        salt = keccak256(abi.encode(salt, _trustedForwarder, address(this), _token));
+        return Clones.predictDeterministicAddress(implementation, salt);
+    }
 }

test/PrivateOfferFactory.t.sol

@@ -143,6 +143,9 @@ contract PrivateOfferFactoryTest is Test {
             trustedForwarder
         );
 
+        console.log("expectedPrivateOffer", expectedPrivateOffer);
+        console.log("expectedVesting", expectedVesting);
+
         // make sure no contract lives here yet
         assertFalse(Address.isContract(expectedPrivateOffer), "Private Offer address already contains contract");
         assertFalse(Address.isContract(expectedVesting), "Vesting address already contains contract");

test/PrivateOfferOffchainPaymentTimeLock.t.sol

@@ -66,7 +66,7 @@ contract PrivateOfferOffchainPaymentTimeLockTest is Test {
      * @param attemptTime try to release tokens after this amount of time
      * @param releaseDuration how long the releasing of tokens should take
      */
-    function testPrivateOfferWithTimeLock(
+    function testPrivateOfferOffchainPaymentTimeLock(
         bytes32 salt,
         uint64 releaseStartTime,
         uint64 releaseDuration,
@@ -90,11 +90,16 @@ contract PrivateOfferOffchainPaymentTimeLockTest is Test {
         // as the payment happens off-chain, we just assume it happened
 
         // predict addresses
-        address expectedTimeLockAddress = vestingCloneFactory.predictCloneAddress(
+        address expectedTimeLockAddress = vestingCloneFactory.predictCloneAddressWithLockupPlan(
             salt,
             trustedForwarder,
-            address(vestingCloneFactory),
-            address(token)
+            address(0), // no owner
+            address(token),
+            tokenAmount,
+            tokenReceiver,
+            releaseStartTime,
+            releaseDuration,
+            releaseDuration
         );
 
         // add time lock and token receiver to the allow list

test/VestingCloneFactory.t.sol

@@ -35,6 +35,188 @@ contract VestingCloneFactoryTest is Test {
         assertEq(expected1, actual, "address prediction failed");
     }
 
+    function testLockUpAddressPrediction(
+        bytes32 _rawSalt,
+        address _trustedForwarder,
+        address _owner,
+        address _token,
+        uint256 _allocation,
+        address _beneficiary,
+        uint64 _start,
+        uint64 _cliff,
+        uint64 _duration
+    ) public {
+        vm.assume(_owner != address(1));
+        vm.assume(
+            _trustedForwarder != address(0) &&
+                _trustedForwarder != address(1) &&
+                _trustedForwarder != address(this) &&
+                _trustedForwarder != address(factory)
+        );
+        vm.assume(_token != address(0) && _token != address(1));
+        vm.assume(_allocation != 0 && _allocation != 1);
+        vm.assume(_beneficiary != address(0) && _beneficiary != address(1));
+        vm.assume(_start != 0 && _start != 1);
+        vm.assume(_cliff != 0 && _cliff != 1);
+        vm.assume(_duration != 0 && _duration != 1);
+        vm.assume(_rawSalt != bytes32("a"));
+        Vesting _implementation = new Vesting(_trustedForwarder);
+        VestingCloneFactory _factory = new VestingCloneFactory(address(_implementation));
+
+        address expected = _factory.predictCloneAddressWithLockupPlan(
+            _rawSalt,
+            _trustedForwarder,
+            _owner,
+            _token,
+            _allocation,
+            _beneficiary,
+            _start,
+            _cliff,
+            _duration
+        );
+
+        vm.expectEmit(true, true, true, true, address(_factory));
+        emit NewClone(expected);
+        address actual = _factory.createVestingCloneWithLockupPlan(
+            _rawSalt,
+            _trustedForwarder,
+            _owner,
+            _token,
+            _allocation,
+            _beneficiary,
+            _start,
+            _cliff,
+            _duration
+        );
+        assertEq(expected, actual, "address prediction failed: expected != actual");
+
+        // test changing the salt changes the address
+        address changedAddress = _factory.predictCloneAddressWithLockupPlan(
+            bytes32("a"),
+            _trustedForwarder,
+            _owner,
+            _token,
+            _allocation,
+            _beneficiary,
+            _start,
+            _cliff,
+            _duration
+        );
+        assertNotEq(actual, changedAddress, "address prediction failed: salt");
+
+        // ensure changing the trustedForwarder reverts
+        vm.expectRevert("VestingCloneFactory: Unexpected trustedForwarder");
+        _factory.predictCloneAddressWithLockupPlan(
+            _rawSalt,
+            address(1),
+            _owner,
+            _token,
+            _allocation,
+            _beneficiary,
+            _start,
+            _cliff,
+            _duration
+        );
+
+        // test changing the owner changes the address
+        changedAddress = _factory.predictCloneAddressWithLockupPlan(
+            _rawSalt,
+            _trustedForwarder,
+            address(1),
+            _token,
+            _allocation,
+            _beneficiary,
+            _start,
+            _cliff,
+            _duration
+        );
+        assertNotEq(actual, changedAddress, "address prediction failed: owner");
+
+        // test changing the token changes the address
+        changedAddress = _factory.predictCloneAddressWithLockupPlan(
+            _rawSalt,
+            _trustedForwarder,
+            _owner,
+            address(1),
+            _allocation,
+            _beneficiary,
+            _start,
+            _cliff,
+            _duration
+        );
+        assertNotEq(actual, changedAddress, "address prediction failed: token");
+
+        // test changing the allocation changes the address
+        changedAddress = _factory.predictCloneAddressWithLockupPlan(
+            _rawSalt,
+            _trustedForwarder,
+            _owner,
+            _token,
+            1,
+            _beneficiary,
+            _start,
+            _cliff,
+            _duration
+        );
+        assertNotEq(actual, changedAddress, "address prediction failed: allocation");
+
+        // test changing the beneficiary changes the address
+        changedAddress = _factory.predictCloneAddressWithLockupPlan(
+            _rawSalt,
+            _trustedForwarder,
+            _owner,
+            _token,
+            _allocation,
+            address(1),
+            _start,
+            _cliff,
+            _duration
+        );
+        assertNotEq(actual, changedAddress, "address prediction failed: beneficiary");
+
+        // test changing the start changes the address
+        changedAddress = _factory.predictCloneAddressWithLockupPlan(
+            _rawSalt,
+            _trustedForwarder,
+            _owner,
+            _token,
+            _allocation,
+            _beneficiary,
+            1,
+            _cliff,
+            _duration
+        );
+        assertNotEq(actual, changedAddress, "address prediction failed: start");
+
+        // test changing the cliff changes the address
+        changedAddress = _factory.predictCloneAddressWithLockupPlan(
+            _rawSalt,
+            _trustedForwarder,
+            _owner,
+            _token,
+            _allocation,
+            _beneficiary,
+            _start,
+            1,
+            _duration
+        );
+        assertNotEq(actual, changedAddress, "address prediction failed: cliff");
+
+        // test changing the duration changes the address
+        changedAddress = _factory.predictCloneAddressWithLockupPlan(
+            _rawSalt,
+            _trustedForwarder,
+            _owner,
+            _token,
+            _allocation,
+            _beneficiary,
+            _start,
+            _cliff,
+            1
+        );
+        assertNotEq(actual, changedAddress, "address prediction failed: duration");
+    }
+
     function testWrongTrustedForwarderReverts(address _wrongTrustedForwarder) public {
         vm.assume(_wrongTrustedForwarder != address(0));
         vm.assume(_wrongTrustedForwarder != trustedForwarder);