From a0f90ebaa95974ca4fed4a23cd8d04ac86003f88 Mon Sep 17 00:00:00 2001 From: Kaspar Kallas Date: Thu, 27 Jun 2024 14:44:45 +0300 Subject: [PATCH 01/15] refactor: explicit functions - _claim - _exececuteAtSingleTransfer - _getTotalVestedAmount --- .../contracts/VestingSchedulerV2.sol | 108 +++++++++++------- 1 file changed, 66 insertions(+), 42 deletions(-) diff --git a/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol b/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol index 40faf33056..15f32858e5 100644 --- a/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol +++ b/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol @@ -513,45 +513,10 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { VestingSchedule memory schedule = vestingSchedules[configHash]; if (schedule.claimValidityDate > 0) { - // Ensure that the caller is the sender or the receiver if the vesting schedule requires claiming. - if (msg.sender != sender && msg.sender != receiver) revert CannotClaimScheduleOnBehalf(); - if (schedule.claimValidityDate < block.timestamp) - revert TimeWindowInvalid(); // TODO: Use more explicit Expired error? Add a good test. - - delete vestingSchedules[configHash].claimValidityDate; - emit VestingClaimed(superToken, sender, receiver, msg.sender); + _claim(superToken, sender, receiver); if (block.timestamp >= schedule.endDate - END_DATE_VALID_BEFORE) { - uint256 totalVestedAmount = - schedule.cliffAmount + - schedule.remainderAmount + - (schedule.endDate - schedule.cliffAndFlowDate) * SafeCast.toUint256(schedule.flowRate); - - delete vestingSchedules[configHash]; // TODO: Add a good test. - - // Note: Super Tokens revert, not return false, i.e. we expect always true here. - assert(superToken.transferFrom(sender, receiver, totalVestedAmount)); - - emit VestingCliffAndFlowExecuted( - superToken, - sender, - receiver, - schedule.cliffAndFlowDate, - 0, - schedule.cliffAmount, - 0 - ); - - emit VestingEndExecuted( - superToken, - sender, - receiver, - schedule.endDate, - totalVestedAmount, - false - ); - - return true; + return _executeScheduleAsSingleTransfer(superToken, sender, receiver); } else { return _executeCliffAndFlow(superToken, sender, receiver, schedule.claimValidityDate); } @@ -561,6 +526,60 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { } } + function _executeScheduleAsSingleTransfer( + ISuperToken superToken, + address sender, + address receiver + ) private returns (bool success) { + bytes32 configHash = keccak256(abi.encodePacked(superToken, sender, receiver)); + VestingSchedule memory schedule = vestingSchedules[configHash]; + + delete vestingSchedules[configHash]; // TODO: Add a good test. + + uint256 totalVestedAmount = _getTotalVestedAmount(schedule); + + // Note: Super Tokens revert, not return false, i.e. we expect always true here. + assert(superToken.transferFrom(sender, receiver, totalVestedAmount)); + + emit VestingCliffAndFlowExecuted( + superToken, + sender, + receiver, + schedule.cliffAndFlowDate, + 0, + schedule.cliffAmount, + 0 + ); + + emit VestingEndExecuted( + superToken, + sender, + receiver, + schedule.endDate, + totalVestedAmount, + false + ); + + return true; + } + + function _claim( + ISuperToken superToken, + address sender, + address receiver + ) private { + bytes32 configHash = keccak256(abi.encodePacked(superToken, sender, receiver)); + VestingSchedule memory schedule = vestingSchedules[configHash]; + + // Ensure that the caller is the sender or the receiver if the vesting schedule requires claiming. + if (msg.sender != sender && msg.sender != receiver) revert CannotClaimScheduleOnBehalf(); + if (schedule.claimValidityDate < block.timestamp) + revert TimeWindowInvalid(); // TODO: Use more explicit Expired error? Add a good test. + + delete vestingSchedules[configHash].claimValidityDate; + emit VestingClaimed(superToken, sender, receiver, msg.sender); + } + /// @dev IVestingScheduler.executeCliffAndFlow implementation. function _executeCliffAndFlow( ISuperToken superToken, @@ -778,11 +797,7 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { maxFlowDelayCompensationAmount + maxEarlyEndCompensationAmount; } else if (schedule.claimValidityDate >= schedule.endDate - END_DATE_VALID_BEFORE) { - uint256 totalVestedAmount = - schedule.cliffAmount + - schedule.remainderAmount + - (schedule.endDate - schedule.cliffAndFlowDate) * SafeCast.toUint256(schedule.flowRate); - return totalVestedAmount; + return _getTotalVestedAmount(schedule); } else { return schedule.cliffAmount + schedule.remainderAmount + @@ -790,4 +805,13 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { maxEarlyEndCompensationAmount; } } + + function _getTotalVestedAmount( + VestingSchedule memory schedule + ) private pure returns (uint256) { + return + schedule.cliffAmount + + schedule.remainderAmount + + (schedule.endDate - schedule.cliffAndFlowDate) * SafeCast.toUint256(schedule.flowRate); + } } From 56e49bd3a2833c9647b58f4c73f381cfa20f3541 Mon Sep 17 00:00:00 2001 From: Kaspar Kallas Date: Thu, 27 Jun 2024 15:07:47 +0300 Subject: [PATCH 02/15] chore: test that schedule is deleted in more places --- .../contracts/VestingSchedulerV2.sol | 2 +- .../scheduler/test/VestingSchedulerV2.t.sol | 57 ++++++++++++++----- 2 files changed, 43 insertions(+), 16 deletions(-) diff --git a/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol b/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol index 15f32858e5..ece6f5c94f 100644 --- a/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol +++ b/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol @@ -534,7 +534,7 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { bytes32 configHash = keccak256(abi.encodePacked(superToken, sender, receiver)); VestingSchedule memory schedule = vestingSchedules[configHash]; - delete vestingSchedules[configHash]; // TODO: Add a good test. + delete vestingSchedules[configHash]; uint256 totalVestedAmount = _getTotalVestedAmount(schedule); diff --git a/packages/automation-contracts/scheduler/test/VestingSchedulerV2.t.sol b/packages/automation-contracts/scheduler/test/VestingSchedulerV2.t.sol index 16fb892651..d388787863 100644 --- a/packages/automation-contracts/scheduler/test/VestingSchedulerV2.t.sol +++ b/packages/automation-contracts/scheduler/test/VestingSchedulerV2.t.sol @@ -172,6 +172,37 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { vm.stopPrank(); } + function assertAreScheduleCreationParamsEqual( + IVestingSchedulerV2.ScheduleCreationParams memory params1, + IVestingSchedulerV2.ScheduleCreationParams memory params2 + ) internal pure { + require(params1.superToken == params2.superToken, "SuperToken mismatch"); + require(params1.receiver == params2.receiver, "Receiver mismatch"); + require(params1.startDate == params2.startDate, "StartDate mismatch"); + require(params1.claimValidityDate == params2.claimValidityDate, "ClaimValidityDate mismatch"); + require(params1.cliffDate == params2.cliffDate, "CliffDate mismatch"); + require(params1.flowRate == params2.flowRate, "FlowRate mismatch"); + require(params1.cliffAmount == params2.cliffAmount, "CliffAmount mismatch"); + require(params1.endDate == params2.endDate, "EndDate mismatch"); + require(params1.remainderAmount == params2.remainderAmount, "RemainderAmount mismatch"); + } + + function testAssertScheduleDoesNotExist( + address superToken, + address sender, + address receiver + ) public { + VestingSchedulerV2.VestingSchedule memory schedule = vestingScheduler.getVestingSchedule(superToken, sender, receiver); + VestingSchedulerV2.VestingSchedule memory deletedSchedule; + + assertEq(schedule.cliffAndFlowDate, deletedSchedule.cliffAndFlowDate, "cliffAndFlowDate mismatch"); + assertEq(schedule.endDate, deletedSchedule.endDate, "endDate mismatch"); + assertEq(schedule.flowRate, deletedSchedule.flowRate, "flowRate mismatch"); + assertEq(schedule.cliffAmount, deletedSchedule.cliffAmount, "cliffAmount mismatch"); + assertEq(schedule.remainderAmount, deletedSchedule.remainderAmount, "remainderAmount mismatch"); + assertEq(schedule.claimValidityDate, deletedSchedule.claimValidityDate, "claimValidityDate mismatch"); + } + /// TESTS function testCreateVestingSchedule() public { @@ -418,6 +449,7 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { vm.expectEmit(true, true, true, true); emit VestingScheduleDeleted(superToken, alice, bob); vestingScheduler.deleteVestingSchedule(superToken, bob, EMPTY_CTX); + testAssertScheduleDoesNotExist(address(superToken), alice, bob); } function testCannotDeleteVestingScheduleIfDataDontExist() public { @@ -1042,21 +1074,8 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { vm.warp(type(uint32).max); assertEq(afterSenderBalance, superToken.balanceOf(alice), "After the schedule has ended, the sender's balance should never change."); - } - function assertAreScheduleCreationParamsEqual( - IVestingSchedulerV2.ScheduleCreationParams memory params1, - IVestingSchedulerV2.ScheduleCreationParams memory params2 - ) internal pure { - require(params1.superToken == params2.superToken, "SuperToken mismatch"); - require(params1.receiver == params2.receiver, "Receiver mismatch"); - require(params1.startDate == params2.startDate, "StartDate mismatch"); - require(params1.claimValidityDate == params2.claimValidityDate, "ClaimValidityDate mismatch"); - require(params1.cliffDate == params2.cliffDate, "CliffDate mismatch"); - require(params1.flowRate == params2.flowRate, "FlowRate mismatch"); - require(params1.cliffAmount == params2.cliffAmount, "CliffAmount mismatch"); - require(params1.endDate == params2.endDate, "EndDate mismatch"); - require(params1.remainderAmount == params2.remainderAmount, "RemainderAmount mismatch"); + testAssertScheduleDoesNotExist(address(superToken), alice, bob); } // Claimable Vesting Schedules tests @@ -1634,6 +1653,8 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { uint256 aliceShouldStream = (END_DATE-CLIFF_DATE) * uint96(FLOW_RATE) + CLIFF_TRANSFER_AMOUNT ; assertEq(aliceInitialBalance - aliceFinalBalance, aliceShouldStream, "(sender) wrong final balance"); assertEq(bobFinalBalance, bobInitialBalance + aliceShouldStream, "(receiver) wrong final balance"); + + testAssertScheduleDoesNotExist(address(superToken), alice, bob); } function test_executeCliffAndFlow_claimAfterEndDate(uint256 delayAfterEndDate, uint256 claimDate) public { @@ -1645,7 +1666,7 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { SafeCast.toUint256(FLOW_RATE); delayAfterEndDate = bound(delayAfterEndDate, 1, 1e8); - claimDate = bound(claimDate, END_DATE - vestingScheduler.END_DATE_VALID_BEFORE() + 1, END_DATE + delayAfterEndDate); + claimDate = bound(claimDate, END_DATE - vestingScheduler.END_DATE_VALID_BEFORE(), END_DATE + delayAfterEndDate); _createClaimableVestingScheduleWithClaimDateAfterEndDate(alice, bob, delayAfterEndDate); vm.prank(alice); @@ -1675,6 +1696,8 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { assertEq(superToken.balanceOf(alice), aliceInitialBalance - totalExpectedAmount); assertEq(superToken.balanceOf(bob), bobInitialBalance + totalExpectedAmount); + + testAssertScheduleDoesNotExist(address(superToken), alice, bob); } function test_executeCliffAndFlow_claimableScheduleWithCliffAmount_senderClaim() public { @@ -1920,6 +1943,8 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { assertEq(superToken.allowance(alice, address(vestingScheduler)), 0, "No allowance should be left"); (,,,int96 flowRateAllowance) = superToken.getFlowPermissions(alice, address(vestingScheduler)); assertEq(flowRateAllowance, 0, "No flow rate allowance should be left"); + + testAssertScheduleDoesNotExist(address(superToken), alice, bob); } function test_getMaximumNeededTokenAllowance_with_claim_should_end_with_zero_if_extreme_ranges_are_used( @@ -2020,5 +2045,7 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { assertEq(superToken.allowance(alice, address(vestingScheduler)), 0, "No allowance should be left"); (,,,int96 flowRateAllowance) = superToken.getFlowPermissions(alice, address(vestingScheduler)); assertEq(flowRateAllowance, 0, "No flow rate allowance should be left"); + + testAssertScheduleDoesNotExist(address(superToken), alice, bob); } } From bc422036e707c0ad4afb8c60698aa18c781bc8a2 Mon Sep 17 00:00:00 2001 From: Kaspar Kallas Date: Thu, 27 Jun 2024 15:23:21 +0300 Subject: [PATCH 03/15] refactor: use more foundry bound in tests --- .../scheduler/test/VestingSchedulerV2.t.sol | 158 +++++++++--------- 1 file changed, 79 insertions(+), 79 deletions(-) diff --git a/packages/automation-contracts/scheduler/test/VestingSchedulerV2.t.sol b/packages/automation-contracts/scheduler/test/VestingSchedulerV2.t.sol index d388787863..2fffb9d619 100644 --- a/packages/automation-contracts/scheduler/test/VestingSchedulerV2.t.sol +++ b/packages/automation-contracts/scheduler/test/VestingSchedulerV2.t.sol @@ -203,6 +203,69 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { assertEq(schedule.claimValidityDate, deletedSchedule.claimValidityDate, "claimValidityDate mismatch"); } + function _getExpectedSchedule( + uint32 startDate, + uint32 cliffDate, + int96 flowRate, + uint256 cliffAmount, + uint32 endDate + ) public view returns (IVestingSchedulerV2.VestingSchedule memory expectedSchedule) { + if (startDate == 0) { + startDate = uint32(block.timestamp); + } + + uint32 cliffAndFlowDate = cliffDate == 0 ? startDate : cliffDate; + + expectedSchedule = IVestingSchedulerV2.VestingSchedule({ + cliffAndFlowDate: cliffAndFlowDate, + endDate: endDate, + claimValidityDate: 0, + flowRate: flowRate, + cliffAmount: cliffAmount, + remainderAmount: 0 + }); + } + + function _getExpectedScheduleFromAmountAndDuration( + uint256 totalAmount, + uint32 totalDuration, + uint32 cliffPeriod, + uint32 startDate, + uint32 claimPeriod + ) public view returns (IVestingSchedulerV2.VestingSchedule memory expectedSchedule) { + if (startDate == 0) { + startDate = uint32(block.timestamp); + } + + int96 flowRate = SafeCast.toInt96(SafeCast.toInt256(totalAmount / totalDuration)); + + uint32 cliffDate; + uint32 cliffAndFlowDate; + uint256 cliffAmount; + if (cliffPeriod > 0) { + cliffDate = startDate + cliffPeriod; + cliffAmount = cliffPeriod * SafeCast.toUint256(flowRate); + cliffAndFlowDate = cliffDate; + } else { + cliffDate = 0; + cliffAmount = 0; + cliffAndFlowDate = startDate; + } + + uint32 endDate = startDate + totalDuration; + + uint96 remainderAmount = SafeCast.toUint96(totalAmount - SafeCast.toUint256(flowRate) * totalDuration); + + expectedSchedule = IVestingSchedulerV2.VestingSchedule({ + cliffAndFlowDate: cliffAndFlowDate, + endDate: endDate, + flowRate: flowRate, + cliffAmount: cliffAmount, + remainderAmount: remainderAmount, + claimValidityDate: claimPeriod == 0 ? 0 : startDate + claimPeriod + }); + } + /// TESTS function testCreateVestingSchedule() public { @@ -908,7 +971,7 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { uint256 beforeReceiverBalance; } - function test_createScheduleFromAmountAndDuration_executeCliffAndFlow_executeEndVesting( + function test_createScheduleFromAmountAndDuration_executeCliffAndFlow_executeEndVesting_withoutClaim( uint256 totalAmount, uint32 totalDuration, uint32 cliffPeriod, @@ -916,10 +979,11 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { uint8 randomizer ) public { // Assume - vm.assume(randomizer != 0); + randomizer = SafeCast.toUint8(bound(randomizer, 1, type(uint8).max)); - vm.assume(startDate == 0 || startDate >= block.timestamp); - vm.assume(startDate < 2524600800 /* year 2050 */); + if (startDate != 0) { + startDate = SafeCast.toUint32(bound(startDate, block.timestamp, 2524600800)); + } totalDuration = SafeCast.toUint32(bound(totalDuration, vestingScheduler.MIN_VESTING_DURATION(), 18250 days)); vm.assume(cliffPeriod <= totalDuration - vestingScheduler.MIN_VESTING_DURATION()); @@ -929,10 +993,9 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { $.beforeSenderBalance = superToken.balanceOf(alice); $.beforeReceiverBalance = superToken.balanceOf(bob); - vm.assume(totalAmount > 1); + totalAmount = bound(totalAmount, 1, $.beforeSenderBalance); vm.assume(totalAmount >= totalDuration); vm.assume(totalAmount / totalDuration <= SafeCast.toUint256(type(int96).max)); - vm.assume(totalAmount <= $.beforeSenderBalance); assertTrue(vestingScheduler.getVestingSchedule(address(superToken), alice, bob).endDate == 0, "Schedule should not exist"); @@ -1795,69 +1858,6 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { vm.stopPrank(); } - function _getExpectedSchedule( - uint32 startDate, - uint32 cliffDate, - int96 flowRate, - uint256 cliffAmount, - uint32 endDate - ) public view returns (IVestingSchedulerV2.VestingSchedule memory expectedSchedule) { - if (startDate == 0) { - startDate = uint32(block.timestamp); - } - - uint32 cliffAndFlowDate = cliffDate == 0 ? startDate : cliffDate; - - expectedSchedule = IVestingSchedulerV2.VestingSchedule({ - cliffAndFlowDate: cliffAndFlowDate, - endDate: endDate, - claimValidityDate: 0, - flowRate: flowRate, - cliffAmount: cliffAmount, - remainderAmount: 0 - }); - } - - function _getExpectedScheduleFromAmountAndDuration( - uint256 totalAmount, - uint32 totalDuration, - uint32 cliffPeriod, - uint32 startDate, - uint32 claimPeriod - ) public view returns (IVestingSchedulerV2.VestingSchedule memory expectedSchedule) { - if (startDate == 0) { - startDate = uint32(block.timestamp); - } - - int96 flowRate = SafeCast.toInt96(SafeCast.toInt256(totalAmount / totalDuration)); - - uint32 cliffDate; - uint32 cliffAndFlowDate; - uint256 cliffAmount; - if (cliffPeriod > 0) { - cliffDate = startDate + cliffPeriod; - cliffAmount = cliffPeriod * SafeCast.toUint256(flowRate); - cliffAndFlowDate = cliffDate; - } else { - cliffDate = 0; - cliffAmount = 0; - cliffAndFlowDate = startDate; - } - - uint32 endDate = startDate + totalDuration; - - uint96 remainderAmount = SafeCast.toUint96(totalAmount - SafeCast.toUint256(flowRate) * totalDuration); - - expectedSchedule = IVestingSchedulerV2.VestingSchedule({ - cliffAndFlowDate: cliffAndFlowDate, - endDate: endDate, - flowRate: flowRate, - cliffAmount: cliffAmount, - remainderAmount: remainderAmount, - claimValidityDate: claimPeriod == 0 ? 0 : startDate + claimPeriod - }); - } - function test_getMaximumNeededTokenAllowance_should_end_with_zero_if_extreme_ranges_are_used( uint256 totalAmount, uint32 totalDuration, @@ -1866,20 +1866,20 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { uint8 randomizer ) public { // Assume - vm.assume(randomizer != 0); + randomizer = SafeCast.toUint8(bound(randomizer, 1, type(uint8).max)); - vm.assume(startDate == 0 || startDate >= block.timestamp); - vm.assume(startDate < 2524600800 /* year 2050 */); + if (startDate != 0) { + startDate = SafeCast.toUint32(bound(startDate, block.timestamp, 2524600800)); + } totalDuration = SafeCast.toUint32(bound(totalDuration, vestingScheduler.MIN_VESTING_DURATION(), 18250 days)); vm.assume(cliffPeriod <= totalDuration - vestingScheduler.MIN_VESTING_DURATION()); uint256 beforeSenderBalance = superToken.balanceOf(alice); - vm.assume(totalAmount > 1); + totalAmount = bound(totalAmount, 1, beforeSenderBalance); vm.assume(totalAmount >= totalDuration); vm.assume(totalAmount / totalDuration <= SafeCast.toUint256(type(int96).max)); - vm.assume(totalAmount <= beforeSenderBalance); // Arrange IVestingSchedulerV2.VestingSchedule memory expectedSchedule = _getExpectedScheduleFromAmountAndDuration( @@ -1956,10 +1956,11 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { uint8 randomizer ) public { // Assume - vm.assume(randomizer != 0); + randomizer = SafeCast.toUint8(bound(randomizer, 1, type(uint8).max)); - vm.assume(startDate == 0 || startDate >= block.timestamp); - vm.assume(startDate < 2524600800 /* year 2050 */); + if (startDate != 0) { + startDate = SafeCast.toUint32(bound(startDate, block.timestamp, 2524600800)); + } claimPeriod = SafeCast.toUint32(bound(claimPeriod, 1, 18250 days)); vm.assume(claimPeriod >= cliffPeriod); @@ -1969,10 +1970,9 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { uint256 beforeSenderBalance = superToken.balanceOf(alice); - vm.assume(totalAmount > 1); + totalAmount = bound(totalAmount, 1, beforeSenderBalance); vm.assume(totalAmount >= totalDuration); vm.assume(totalAmount / totalDuration <= SafeCast.toUint256(type(int96).max)); - vm.assume(totalAmount <= beforeSenderBalance); // Arrange IVestingSchedulerV2.VestingSchedule memory expectedSchedule = _getExpectedScheduleFromAmountAndDuration( From 0a4a44312726dd17b383ea2496ed9a54e555d75d Mon Sep 17 00:00:00 2001 From: Kaspar Kallas Date: Thu, 27 Jun 2024 16:11:14 +0300 Subject: [PATCH 04/15] chore: test better the scenario where the schedule is not ended on time --- .../scheduler/test/VestingSchedulerV2.t.sol | 46 ++++++++++++------- 1 file changed, 29 insertions(+), 17 deletions(-) diff --git a/packages/automation-contracts/scheduler/test/VestingSchedulerV2.t.sol b/packages/automation-contracts/scheduler/test/VestingSchedulerV2.t.sol index 2fffb9d619..511f59c425 100644 --- a/packages/automation-contracts/scheduler/test/VestingSchedulerV2.t.sol +++ b/packages/automation-contracts/scheduler/test/VestingSchedulerV2.t.sol @@ -1117,32 +1117,44 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { assertEq(actualSchedule.endDate, expectedSchedule.endDate, "schedule started: endDate not expected"); assertEq(actualSchedule.remainderAmount, expectedSchedule.remainderAmount, "schedule started: remainderAmount not expected"); - // Act - vm.warp(expectedSchedule.endDate - (vestingScheduler.END_DATE_VALID_BEFORE() - (vestingScheduler.END_DATE_VALID_BEFORE() / randomizer))); - assertTrue(vestingScheduler.executeEndVesting(superToken, alice, bob)); + uint256 afterSenderBalance; + uint256 afterReceiverBalance; + if (randomizer % 7 != 0) { + // # Test end execution on time. - actualSchedule = vestingScheduler.getVestingSchedule(address(superToken), alice, bob); - assertEq(actualSchedule.cliffAndFlowDate, 0, "schedule ended: cliffAndFlowDate not expected"); - assertEq(actualSchedule.cliffAmount, 0, "schedule ended: cliffAmount not expected"); - assertEq(actualSchedule.flowRate, 0, "schedule ended: flowRate not expected"); - assertEq(actualSchedule.endDate, 0, "schedule ended: endDate not expected"); - assertEq(actualSchedule.remainderAmount, 0, "schedule ended: remainderAmount not expected"); + // Act + vm.warp(expectedSchedule.endDate - (vestingScheduler.END_DATE_VALID_BEFORE() - (vestingScheduler.END_DATE_VALID_BEFORE() / randomizer))); + assertTrue(vestingScheduler.executeEndVesting(superToken, alice, bob)); - // Assert - uint256 afterSenderBalance = superToken.balanceOf(alice); - uint256 afterReceiverBalance = superToken.balanceOf(bob); + // Assert + afterSenderBalance = superToken.balanceOf(alice); + afterReceiverBalance = superToken.balanceOf(bob); - assertEq(afterSenderBalance, $.beforeSenderBalance - totalAmount, "Sender balance should decrease by totalAmount"); - assertEq(afterReceiverBalance, $.beforeReceiverBalance + totalAmount, "Receiver balance should increase by totalAmount"); + assertEq(afterSenderBalance, $.beforeSenderBalance - totalAmount, "Sender balance should decrease by totalAmount"); + assertEq(afterReceiverBalance, $.beforeReceiverBalance + totalAmount, "Receiver balance should increase by totalAmount"); + } else { + // # Test end execution delayed. - vm.warp(type(uint32).max); - assertEq(afterSenderBalance, superToken.balanceOf(alice), "After the schedule has ended, the sender's balance should never change."); + // Act + vm.warp(expectedSchedule.endDate + (totalDuration / randomizer * 3)); + assertTrue(vestingScheduler.executeEndVesting(superToken, alice, bob)); + + // Assert + afterSenderBalance = superToken.balanceOf(alice); + afterReceiverBalance = superToken.balanceOf(bob); + + assertLt(afterSenderBalance, $.beforeSenderBalance - totalAmount + expectedSchedule.remainderAmount, "Sender balance should decrease by at least totalAmount"); + assertGt(afterReceiverBalance, $.beforeReceiverBalance + totalAmount - expectedSchedule.remainderAmount, "Receiver balance should increase by at least totalAmount"); + } testAssertScheduleDoesNotExist(address(superToken), alice, bob); + + vm.warp(type(uint32).max); + assertEq(afterSenderBalance, superToken.balanceOf(alice), "After the schedule has ended, the sender's balance should never change."); } // Claimable Vesting Schedules tests - + function test_createClaimableVestingSchedule() public { vm.expectEmit(true, true, true, true); From 774a2f2039d5c10583d799b7c3cb2ccee24aa567 Mon Sep 17 00:00:00 2001 From: Kaspar Kallas Date: Fri, 28 Jun 2024 00:39:37 +0300 Subject: [PATCH 05/15] refactor: refactor to using aggregate object - make executeCliffAndFlow public --- .../contracts/VestingSchedulerV2.sol | 296 +++++++++++------- .../scheduler/test/VestingSchedulerV2.t.sol | 7 +- 2 files changed, 180 insertions(+), 123 deletions(-) diff --git a/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol b/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol index ece6f5c94f..5a8b13786a 100644 --- a/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol +++ b/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol @@ -19,6 +19,14 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { uint32 public constant START_DATE_VALID_AFTER = 3 days; uint32 public constant END_DATE_VALID_BEFORE = 1 days; + struct VestingScheduleAggregate { + ISuperToken superToken; + address sender; + address receiver; + bytes32 id; + VestingSchedule schedule; + } + constructor(ISuperfluid host) { cfaV1 = CFAv1Library.InitData( host, @@ -52,7 +60,7 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { uint32 endDate, bytes memory ctx ) external returns (bytes memory newCtx) { - newCtx = _createVestingSchedule( + newCtx = _validateAndCreateVestingSchedule( ScheduleCreationParams( superToken, receiver, @@ -78,7 +86,7 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { uint256 cliffAmount, uint32 endDate ) external { - _createVestingSchedule( + _validateAndCreateVestingSchedule( ScheduleCreationParams( superToken, receiver, @@ -94,7 +102,7 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { ); } - function _createVestingSchedule( + function _validateAndCreateVestingSchedule( ScheduleCreationParams memory params, bytes memory ctx ) private returns (bytes memory newCtx) { @@ -129,10 +137,10 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { if (params.claimValidityDate != 0 && params.claimValidityDate < cliffAndFlowDate) revert TimeWindowInvalid(); - bytes32 hashConfig = keccak256(abi.encodePacked(params.superToken, sender, params.receiver)); - if (vestingSchedules[hashConfig].endDate != 0) revert ScheduleAlreadyExists(); + bytes32 id = keccak256(abi.encodePacked(params.superToken, sender, params.receiver)); + if (vestingSchedules[id].endDate != 0) revert ScheduleAlreadyExists(); - vestingSchedules[hashConfig] = VestingSchedule( + vestingSchedules[id] = VestingSchedule( cliffAndFlowDate, params.endDate, params.flowRate, @@ -165,7 +173,7 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { uint32 startDate, bytes memory ctx ) external returns (bytes memory newCtx) { - newCtx = _createVestingSchedule( + newCtx = _validateAndCreateVestingSchedule( getCreateVestingScheduleParamsFromAmountAndDuration( superToken, receiver, @@ -188,7 +196,7 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { uint32 cliffPeriod, uint32 startDate ) external { - _createVestingSchedule( + _validateAndCreateVestingSchedule( getCreateVestingScheduleParamsFromAmountAndDuration( superToken, receiver, @@ -210,7 +218,7 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { uint32 totalDuration, uint32 cliffPeriod ) external { - _createVestingSchedule( + _validateAndCreateVestingSchedule( getCreateVestingScheduleParamsFromAmountAndDuration( superToken, receiver, @@ -231,7 +239,7 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { uint256 totalAmount, uint32 totalDuration ) external { - _createVestingSchedule( + _validateAndCreateVestingSchedule( getCreateVestingScheduleParamsFromAmountAndDuration( superToken, receiver, @@ -286,7 +294,7 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { uint32 totalDuration, bytes memory ctx ) private returns (bytes memory newCtx) { - newCtx = _createVestingSchedule( + newCtx = _validateAndCreateVestingSchedule( getCreateVestingScheduleParamsFromAmountAndDuration( superToken, receiver, @@ -300,7 +308,7 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { ); address sender = _getSender(ctx); - assert(_executeCliffAndFlow(superToken, sender, receiver, uint32(block.timestamp))); + assert(executeCliffAndFlow(superToken, sender, receiver)); } /// @dev IVestingScheduler.createClaimableVestingSchedule implementation. @@ -315,7 +323,7 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { uint32 endDate, bytes memory ctx ) external returns (bytes memory newCtx) { - newCtx = _createVestingSchedule( + newCtx = _validateAndCreateVestingSchedule( ScheduleCreationParams( superToken, receiver, @@ -342,7 +350,7 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { uint256 cliffAmount, uint32 endDate ) external { - _createVestingSchedule( + _validateAndCreateVestingSchedule( ScheduleCreationParams( superToken, receiver, @@ -369,7 +377,7 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { uint32 startDate, bytes memory ctx ) external returns (bytes memory newCtx) { - newCtx = _createVestingSchedule( + newCtx = _validateAndCreateVestingSchedule( getCreateVestingScheduleParamsFromAmountAndDuration( superToken, receiver, @@ -393,7 +401,7 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { uint32 cliffPeriod, uint32 startDate ) external { - _createVestingSchedule( + _validateAndCreateVestingSchedule( getCreateVestingScheduleParamsFromAmountAndDuration( superToken, receiver, @@ -416,7 +424,7 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { uint32 claimPeriod, uint32 cliffPeriod ) external { - _createVestingSchedule( + _validateAndCreateVestingSchedule( getCreateVestingScheduleParamsFromAmountAndDuration( superToken, receiver, @@ -438,7 +446,7 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { uint32 totalDuration, uint32 claimPeriod ) external { - _createVestingSchedule( + _validateAndCreateVestingSchedule( getCreateVestingScheduleParamsFromAmountAndDuration( superToken, receiver, @@ -461,8 +469,8 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { newCtx = ctx; address sender = _getSender(ctx); - bytes32 configHash = keccak256(abi.encodePacked(superToken, sender, receiver)); - VestingSchedule memory schedule = vestingSchedules[configHash]; + bytes32 id = keccak256(abi.encodePacked(superToken, sender, receiver)); + VestingSchedule memory schedule = vestingSchedules[id]; if (endDate <= block.timestamp) revert TimeWindowInvalid(); @@ -471,9 +479,9 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { // Only allow an update if 1. vesting exists 2. executeCliffAndFlow() has been called if (schedule.cliffAndFlowDate != 0 || schedule.endDate == 0) revert ScheduleNotFlowing(); - vestingSchedules[configHash].endDate = endDate; + vestingSchedules[id].endDate = endDate; // Note: Nullify the remainder amount when complexity of updates is introduced. - vestingSchedules[configHash].remainderAmount = 0; + vestingSchedules[id].remainderAmount = 0; emit VestingScheduleUpdated( superToken, @@ -481,7 +489,7 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { receiver, schedule.endDate, endDate, - vestingSchedules[configHash].remainderAmount + vestingSchedules[id].remainderAmount ); } @@ -493,10 +501,10 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { ) external returns (bytes memory newCtx) { newCtx = ctx; address sender = _getSender(ctx); - bytes32 configHash = keccak256(abi.encodePacked(superToken, sender, receiver)); + bytes32 id = keccak256(abi.encodePacked(superToken, sender, receiver)); - if (vestingSchedules[configHash].endDate != 0) { - delete vestingSchedules[configHash]; + if (vestingSchedules[id].endDate != 0) { + delete vestingSchedules[id]; emit VestingScheduleDeleted(superToken, sender, receiver); } else { revert ScheduleDoesNotExist(); @@ -508,138 +516,147 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { ISuperToken superToken, address sender, address receiver - ) external returns (bool success) { - bytes32 configHash = keccak256(abi.encodePacked(superToken, sender, receiver)); - VestingSchedule memory schedule = vestingSchedules[configHash]; - - if (schedule.claimValidityDate > 0) { - _claim(superToken, sender, receiver); - - if (block.timestamp >= schedule.endDate - END_DATE_VALID_BEFORE) { - return _executeScheduleAsSingleTransfer(superToken, sender, receiver); + ) public returns (bool success) { + VestingScheduleAggregate memory aggr = _getVestingScheduleAggregate(superToken, sender, receiver); + + if (aggr.schedule.claimValidityDate != 0) { + _validateAndClaim(aggr); + _validateBeforeCliffAndFlow(aggr.schedule, true); + if (block.timestamp >= _minDateToExecuteEndInclusive(aggr.schedule)) { + _validateBeforeEndVesting(aggr.schedule, true); + success = _executeVestingAsSingleTransfer(aggr); } else { - return _executeCliffAndFlow(superToken, sender, receiver, schedule.claimValidityDate); + success = _executeCliffAndFlow(aggr); } } else { - return _executeCliffAndFlow( - superToken, sender, receiver, schedule.cliffAndFlowDate + START_DATE_VALID_AFTER); + _validateBeforeCliffAndFlow(aggr.schedule, false); + success = _executeCliffAndFlow(aggr); } } - function _executeScheduleAsSingleTransfer( - ISuperToken superToken, - address sender, - address receiver - ) private returns (bool success) { - bytes32 configHash = keccak256(abi.encodePacked(superToken, sender, receiver)); - VestingSchedule memory schedule = vestingSchedules[configHash]; - - delete vestingSchedules[configHash]; - - uint256 totalVestedAmount = _getTotalVestedAmount(schedule); - - // Note: Super Tokens revert, not return false, i.e. we expect always true here. - assert(superToken.transferFrom(sender, receiver, totalVestedAmount)); - - emit VestingCliffAndFlowExecuted( - superToken, - sender, - receiver, - schedule.cliffAndFlowDate, - 0, - schedule.cliffAmount, - 0 - ); - - emit VestingEndExecuted( - superToken, - sender, - receiver, - schedule.endDate, - totalVestedAmount, - false - ); + function _validateAndClaim( + VestingScheduleAggregate memory aggr + ) private { + // Ensure that the caller is the sender or the receiver if the vesting schedule requires claiming. + if (msg.sender != aggr.sender && msg.sender != aggr.receiver) + revert CannotClaimScheduleOnBehalf(); - return true; + if (aggr.schedule.claimValidityDate < block.timestamp) + revert TimeWindowInvalid(); // TODO: Use more explicit Expired error? Add a good test. + + emit VestingClaimed(aggr.superToken, aggr.sender, aggr.receiver, msg.sender); + delete vestingSchedules[aggr.id].claimValidityDate; } - function _claim( - ISuperToken superToken, - address sender, - address receiver - ) private { - bytes32 configHash = keccak256(abi.encodePacked(superToken, sender, receiver)); - VestingSchedule memory schedule = vestingSchedules[configHash]; + function _validateBeforeCliffAndFlow( + VestingSchedule memory schedule, + bool disableClaimCheck + ) private view { + if (schedule.cliffAndFlowDate == 0) + revert AlreadyExecuted(); - // Ensure that the caller is the sender or the receiver if the vesting schedule requires claiming. - if (msg.sender != sender && msg.sender != receiver) revert CannotClaimScheduleOnBehalf(); - if (schedule.claimValidityDate < block.timestamp) - revert TimeWindowInvalid(); // TODO: Use more explicit Expired error? Add a good test. + if (!disableClaimCheck && schedule.claimValidityDate != 0) + revert ScheduleNotClaimed(); - delete vestingSchedules[configHash].claimValidityDate; - emit VestingClaimed(superToken, sender, receiver, msg.sender); + // Ensure that that the claming date is after the cliff/flow date and before the claim validity date + if (schedule.cliffAndFlowDate > block.timestamp || + _maxDateToExecuteStartInclusive(schedule) < block.timestamp) + revert TimeWindowInvalid(); } /// @dev IVestingScheduler.executeCliffAndFlow implementation. function _executeCliffAndFlow( - ISuperToken superToken, - address sender, - address receiver, - uint32 latestExecutionDate + VestingScheduleAggregate memory aggr ) private returns (bool success) { - bytes32 configHash = keccak256(abi.encodePacked(superToken, sender, receiver)); - VestingSchedule memory schedule = vestingSchedules[configHash]; - - if (schedule.cliffAndFlowDate == 0) revert AlreadyExecuted(); - - // Ensure that that the claming date is after the cliff/flow date and before the claim validity date - if (schedule.cliffAndFlowDate > block.timestamp || - latestExecutionDate < block.timestamp - ) revert TimeWindowInvalid(); - // Invalidate configuration straight away -- avoid any chance of re-execution or re-entry. - delete vestingSchedules[configHash].cliffAndFlowDate; - delete vestingSchedules[configHash].cliffAmount; + delete vestingSchedules[aggr.id].cliffAndFlowDate; + delete vestingSchedules[aggr.id].cliffAmount; // Compensate for the fact that flow will almost always be executed slightly later than scheduled. - uint256 flowDelayCompensation = (block.timestamp - schedule.cliffAndFlowDate) * uint96(schedule.flowRate); + uint256 flowDelayCompensation = + (block.timestamp - aggr.schedule.cliffAndFlowDate) * uint96(aggr.schedule.flowRate); // If there's cliff or compensation then transfer that amount. - if (schedule.cliffAmount != 0 || flowDelayCompensation != 0) { + if (aggr.schedule.cliffAmount != 0 || flowDelayCompensation != 0) { // Note: Super Tokens revert, not return false, i.e. we expect always true here. - assert(superToken.transferFrom(sender, receiver, schedule.cliffAmount + flowDelayCompensation)); + assert( + aggr.superToken.transferFrom( + aggr.sender, aggr.receiver, aggr.schedule.cliffAmount + flowDelayCompensation)); } // Create a flow according to the vesting schedule configuration. - cfaV1.createFlowByOperator(sender, receiver, superToken, schedule.flowRate); + cfaV1.createFlowByOperator(aggr.sender, aggr.receiver, aggr.superToken, aggr.schedule.flowRate); emit VestingCliffAndFlowExecuted( - superToken, - sender, - receiver, - schedule.cliffAndFlowDate, - schedule.flowRate, - schedule.cliffAmount, + aggr.superToken, + aggr.sender, + aggr.receiver, + aggr.schedule.cliffAndFlowDate, + aggr.schedule.flowRate, + aggr.schedule.cliffAmount, flowDelayCompensation ); return true; } + function _executeVestingAsSingleTransfer( + VestingScheduleAggregate memory aggr + ) private returns (bool success) { + delete vestingSchedules[aggr.id]; + + uint256 totalVestedAmount = _getTotalVestedAmount(aggr.schedule); + + // Note: Super Tokens revert, not return false, i.e. we expect always true here. + assert(aggr.superToken.transferFrom(aggr.sender, aggr.receiver, totalVestedAmount)); + + emit VestingCliffAndFlowExecuted( + aggr.superToken, + aggr.sender, + aggr.receiver, + aggr.schedule.cliffAndFlowDate, + 0, + aggr.schedule.cliffAmount, + 0 + ); + + emit VestingEndExecuted( + aggr.superToken, + aggr.sender, + aggr.receiver, + aggr.schedule.endDate, + totalVestedAmount, + false + ); + + return true; + } + + function _validateBeforeEndVesting( + VestingSchedule memory schedule, + bool disableClaimCheck + ) private view { + if (schedule.endDate == 0) + revert AlreadyExecuted(); + + if (!disableClaimCheck && schedule.claimValidityDate != 0) + revert ScheduleNotClaimed(); + + if (_minDateToExecuteEndInclusive(schedule) > block.timestamp) + revert TimeWindowInvalid(); + } + /// @dev IVestingScheduler.executeEndVesting implementation. function executeEndVesting( ISuperToken superToken, address sender, address receiver ) external returns (bool success) { - bytes32 configHash = keccak256(abi.encodePacked(superToken, sender, receiver)); - VestingSchedule memory schedule = vestingSchedules[configHash]; + bytes32 id = keccak256(abi.encodePacked(superToken, sender, receiver)); + VestingSchedule memory schedule = vestingSchedules[id]; - if (schedule.claimValidityDate != 0) revert ScheduleNotClaimed(); - if (schedule.endDate == 0) revert AlreadyExecuted(); - if (schedule.endDate - END_DATE_VALID_BEFORE > block.timestamp) - revert TimeWindowInvalid(); + _validateBeforeEndVesting(schedule, false); // Invalidate configuration straight away -- avoid any chance of re-execution or re-entry. - delete vestingSchedules[configHash]; + delete vestingSchedules[id]; // If vesting is not running, we can't do anything, just emit failing event. if (_isFlowOngoing(superToken, sender, receiver)) { @@ -679,11 +696,26 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { /// @dev IVestingScheduler.getVestingSchedule implementation. function getVestingSchedule( - address supertoken, + address superToken, address sender, address receiver ) public view returns (VestingSchedule memory) { - return vestingSchedules[keccak256(abi.encodePacked(supertoken, sender, receiver))]; + return vestingSchedules[keccak256(abi.encodePacked(superToken, sender, receiver))]; + } + + function _getVestingScheduleAggregate( + ISuperToken superToken, + address sender, + address receiver + ) public view returns (VestingScheduleAggregate memory) { + bytes32 id = keccak256(abi.encodePacked(superToken, sender, receiver)); + return VestingScheduleAggregate({ + superToken: superToken, + sender: sender, + receiver: receiver, + id: id, + schedule: vestingSchedules[id] + }); } /// @dev get sender of transaction from Superfluid Context or transaction itself. @@ -796,7 +828,7 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { schedule.remainderAmount + maxFlowDelayCompensationAmount + maxEarlyEndCompensationAmount; - } else if (schedule.claimValidityDate >= schedule.endDate - END_DATE_VALID_BEFORE) { + } else if (schedule.claimValidityDate >= _minDateToExecuteEndInclusive(schedule)) { return _getTotalVestedAmount(schedule); } else { return schedule.cliffAmount + @@ -814,4 +846,26 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { schedule.remainderAmount + (schedule.endDate - schedule.cliffAndFlowDate) * SafeCast.toUint256(schedule.flowRate); } + + function _maxDateToExecuteStartInclusive( + VestingSchedule memory schedule + ) private pure returns (uint32) { + if (schedule.cliffAndFlowDate == 0) + revert TimeWindowInvalid(); + + if (schedule.claimValidityDate != 0) { + return schedule.claimValidityDate; + } else { + return schedule.cliffAndFlowDate + START_DATE_VALID_AFTER; + } + } + + function _minDateToExecuteEndInclusive( + VestingSchedule memory schedule + ) private pure returns (uint32) { + if (schedule.endDate == 0) + revert TimeWindowInvalid(); + + return schedule.endDate - END_DATE_VALID_BEFORE; + } } diff --git a/packages/automation-contracts/scheduler/test/VestingSchedulerV2.t.sol b/packages/automation-contracts/scheduler/test/VestingSchedulerV2.t.sol index 511f59c425..b98e6f98b1 100644 --- a/packages/automation-contracts/scheduler/test/VestingSchedulerV2.t.sol +++ b/packages/automation-contracts/scheduler/test/VestingSchedulerV2.t.sol @@ -985,7 +985,7 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { startDate = SafeCast.toUint32(bound(startDate, block.timestamp, 2524600800)); } - totalDuration = SafeCast.toUint32(bound(totalDuration, vestingScheduler.MIN_VESTING_DURATION(), 18250 days)); + totalDuration = SafeCast.toUint32(bound(totalDuration, vestingScheduler.MIN_VESTING_DURATION(), 9125 days)); vm.assume(cliffPeriod <= totalDuration - vestingScheduler.MIN_VESTING_DURATION()); BigTestData memory $; @@ -1106,6 +1106,7 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { assertEq(actualSchedule.remainderAmount, expectedSchedule.remainderAmount, "schedule created: remainderAmount not expected"); // Act + console.log("Executing cliff and flow."); vm.warp(expectedSchedule.cliffAndFlowDate + (vestingScheduler.START_DATE_VALID_AFTER() - (vestingScheduler.START_DATE_VALID_AFTER() / randomizer))); assertTrue(vestingScheduler.executeCliffAndFlow(superToken, alice, bob)); @@ -1123,6 +1124,7 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { // # Test end execution on time. // Act + console.log("Executing end vesting early."); vm.warp(expectedSchedule.endDate - (vestingScheduler.END_DATE_VALID_BEFORE() - (vestingScheduler.END_DATE_VALID_BEFORE() / randomizer))); assertTrue(vestingScheduler.executeEndVesting(superToken, alice, bob)); @@ -1136,7 +1138,8 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { // # Test end execution delayed. // Act - vm.warp(expectedSchedule.endDate + (totalDuration / randomizer * 3)); + console.log("Executing end vesting late."); + vm.warp(expectedSchedule.endDate + (totalDuration / randomizer)); // There is some chance of overflow here. assertTrue(vestingScheduler.executeEndVesting(superToken, alice, bob)); // Assert From 0706afdeb50e8f2f60310b02abe764dadbde448f Mon Sep 17 00:00:00 2001 From: Kaspar Kallas Date: Fri, 28 Jun 2024 10:05:41 +0300 Subject: [PATCH 06/15] chore: improve the test further --- .../contracts/VestingSchedulerV2.sol | 98 +++++++++---------- .../scheduler/test/VestingSchedulerV2.t.sol | 52 +++++++--- 2 files changed, 87 insertions(+), 63 deletions(-) diff --git a/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol b/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol index 5a8b13786a..0da27ae308 100644 --- a/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol +++ b/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol @@ -19,7 +19,7 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { uint32 public constant START_DATE_VALID_AFTER = 3 days; uint32 public constant END_DATE_VALID_BEFORE = 1 days; - struct VestingScheduleAggregate { + struct ScheduleAggregate { ISuperToken superToken; address sender; address receiver; @@ -517,35 +517,35 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { address sender, address receiver ) public returns (bool success) { - VestingScheduleAggregate memory aggr = _getVestingScheduleAggregate(superToken, sender, receiver); - - if (aggr.schedule.claimValidityDate != 0) { - _validateAndClaim(aggr); - _validateBeforeCliffAndFlow(aggr.schedule, true); - if (block.timestamp >= _minDateToExecuteEndInclusive(aggr.schedule)) { - _validateBeforeEndVesting(aggr.schedule, true); - success = _executeVestingAsSingleTransfer(aggr); + ScheduleAggregate memory agg = _getVestingScheduleAggregate(superToken, sender, receiver); + + if (agg.schedule.claimValidityDate != 0) { + _validateAndClaim(agg); + _validateBeforeCliffAndFlow(agg.schedule, true); + if (block.timestamp >= _minDateToExecuteEndInclusive(agg.schedule)) { + _validateBeforeEndVesting(agg.schedule, true); + success = _executeVestingAsSingleTransfer(agg); } else { - success = _executeCliffAndFlow(aggr); + success = _executeCliffAndFlow(agg); } } else { - _validateBeforeCliffAndFlow(aggr.schedule, false); - success = _executeCliffAndFlow(aggr); + _validateBeforeCliffAndFlow(agg.schedule, false); + success = _executeCliffAndFlow(agg); } } function _validateAndClaim( - VestingScheduleAggregate memory aggr + ScheduleAggregate memory agg ) private { // Ensure that the caller is the sender or the receiver if the vesting schedule requires claiming. - if (msg.sender != aggr.sender && msg.sender != aggr.receiver) + if (msg.sender != agg.sender && msg.sender != agg.receiver) revert CannotClaimScheduleOnBehalf(); - if (aggr.schedule.claimValidityDate < block.timestamp) - revert TimeWindowInvalid(); // TODO: Use more explicit Expired error? Add a good test. + if (agg.schedule.claimValidityDate < block.timestamp) + revert TimeWindowInvalid(); - emit VestingClaimed(aggr.superToken, aggr.sender, aggr.receiver, msg.sender); - delete vestingSchedules[aggr.id].claimValidityDate; + emit VestingClaimed(agg.superToken, agg.sender, agg.receiver, msg.sender); + delete vestingSchedules[agg.id].claimValidityDate; } function _validateBeforeCliffAndFlow( @@ -566,32 +566,32 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { /// @dev IVestingScheduler.executeCliffAndFlow implementation. function _executeCliffAndFlow( - VestingScheduleAggregate memory aggr + ScheduleAggregate memory agg ) private returns (bool success) { // Invalidate configuration straight away -- avoid any chance of re-execution or re-entry. - delete vestingSchedules[aggr.id].cliffAndFlowDate; - delete vestingSchedules[aggr.id].cliffAmount; + delete vestingSchedules[agg.id].cliffAndFlowDate; + delete vestingSchedules[agg.id].cliffAmount; // Compensate for the fact that flow will almost always be executed slightly later than scheduled. uint256 flowDelayCompensation = - (block.timestamp - aggr.schedule.cliffAndFlowDate) * uint96(aggr.schedule.flowRate); + (block.timestamp - agg.schedule.cliffAndFlowDate) * uint96(agg.schedule.flowRate); // If there's cliff or compensation then transfer that amount. - if (aggr.schedule.cliffAmount != 0 || flowDelayCompensation != 0) { + if (agg.schedule.cliffAmount != 0 || flowDelayCompensation != 0) { // Note: Super Tokens revert, not return false, i.e. we expect always true here. assert( - aggr.superToken.transferFrom( - aggr.sender, aggr.receiver, aggr.schedule.cliffAmount + flowDelayCompensation)); + agg.superToken.transferFrom( + agg.sender, agg.receiver, agg.schedule.cliffAmount + flowDelayCompensation)); } // Create a flow according to the vesting schedule configuration. - cfaV1.createFlowByOperator(aggr.sender, aggr.receiver, aggr.superToken, aggr.schedule.flowRate); + cfaV1.createFlowByOperator(agg.sender, agg.receiver, agg.superToken, agg.schedule.flowRate); emit VestingCliffAndFlowExecuted( - aggr.superToken, - aggr.sender, - aggr.receiver, - aggr.schedule.cliffAndFlowDate, - aggr.schedule.flowRate, - aggr.schedule.cliffAmount, + agg.superToken, + agg.sender, + agg.receiver, + agg.schedule.cliffAndFlowDate, + agg.schedule.flowRate, + agg.schedule.cliffAmount, flowDelayCompensation ); @@ -599,30 +599,30 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { } function _executeVestingAsSingleTransfer( - VestingScheduleAggregate memory aggr + ScheduleAggregate memory agg ) private returns (bool success) { - delete vestingSchedules[aggr.id]; + delete vestingSchedules[agg.id]; - uint256 totalVestedAmount = _getTotalVestedAmount(aggr.schedule); + uint256 totalVestedAmount = _getTotalVestedAmount(agg.schedule); // Note: Super Tokens revert, not return false, i.e. we expect always true here. - assert(aggr.superToken.transferFrom(aggr.sender, aggr.receiver, totalVestedAmount)); + assert(agg.superToken.transferFrom(agg.sender, agg.receiver, totalVestedAmount)); emit VestingCliffAndFlowExecuted( - aggr.superToken, - aggr.sender, - aggr.receiver, - aggr.schedule.cliffAndFlowDate, + agg.superToken, + agg.sender, + agg.receiver, + agg.schedule.cliffAndFlowDate, 0, - aggr.schedule.cliffAmount, + agg.schedule.cliffAmount, 0 ); emit VestingEndExecuted( - aggr.superToken, - aggr.sender, - aggr.receiver, - aggr.schedule.endDate, + agg.superToken, + agg.sender, + agg.receiver, + agg.schedule.endDate, totalVestedAmount, false ); @@ -707,9 +707,9 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { ISuperToken superToken, address sender, address receiver - ) public view returns (VestingScheduleAggregate memory) { + ) public view returns (ScheduleAggregate memory) { bytes32 id = keccak256(abi.encodePacked(superToken, sender, receiver)); - return VestingScheduleAggregate({ + return ScheduleAggregate({ superToken: superToken, sender: sender, receiver: receiver, @@ -851,7 +851,7 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { VestingSchedule memory schedule ) private pure returns (uint32) { if (schedule.cliffAndFlowDate == 0) - revert TimeWindowInvalid(); + revert AlreadyExecuted(); if (schedule.claimValidityDate != 0) { return schedule.claimValidityDate; @@ -864,7 +864,7 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { VestingSchedule memory schedule ) private pure returns (uint32) { if (schedule.endDate == 0) - revert TimeWindowInvalid(); + revert AlreadyExecuted(); return schedule.endDate - END_DATE_VALID_BEFORE; } diff --git a/packages/automation-contracts/scheduler/test/VestingSchedulerV2.t.sol b/packages/automation-contracts/scheduler/test/VestingSchedulerV2.t.sol index b98e6f98b1..d68e859a68 100644 --- a/packages/automation-contracts/scheduler/test/VestingSchedulerV2.t.sol +++ b/packages/automation-contracts/scheduler/test/VestingSchedulerV2.t.sol @@ -969,6 +969,8 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { struct BigTestData { uint256 beforeSenderBalance; uint256 beforeReceiverBalance; + uint32 expectedCliffDate; + uint32 expectedStartDate; } function test_createScheduleFromAmountAndDuration_executeCliffAndFlow_executeEndVesting_withoutClaim( @@ -1007,8 +1009,8 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { startDate, 0 ); - uint32 expectedCliffDate = cliffPeriod == 0 ? 0 : expectedSchedule.cliffAndFlowDate; - uint32 expectedStartDate = startDate == 0 ? uint32(block.timestamp) : startDate; + $.expectedCliffDate = cliffPeriod == 0 ? 0 : expectedSchedule.cliffAndFlowDate; + $.expectedStartDate = startDate == 0 ? uint32(block.timestamp) : startDate; // Assume we're not getting liquidated at the end: vm.assume($.beforeSenderBalance >= totalAmount + vestingScheduler.END_DATE_VALID_BEFORE() * SafeCast.toUint256(expectedSchedule.flowRate)); @@ -1018,8 +1020,8 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { console.log("Cliff period: %s", cliffPeriod); console.log("Start date: %s", startDate); console.log("Randomizer: %s", randomizer); - console.log("Expected start date: %s", expectedStartDate); - console.log("Expected cliff date: %s", expectedCliffDate); + console.log("Expected start date: %s", $.expectedStartDate); + console.log("Expected cliff date: %s", $.expectedCliffDate); console.log("Expected cliff & flow date: %s", expectedSchedule.cliffAndFlowDate); console.log("Expected end date: %s", expectedSchedule.endDate); console.log("Expected flow rate: %s", SafeCast.toUint256(expectedSchedule.flowRate)); @@ -1042,17 +1044,14 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { superToken.approve(address(vestingScheduler), vestingScheduler.getMaximumNeededTokenAllowance(expectedSchedule)); vm.stopPrank(); - vm.expectEmit(); - emit VestingScheduleCreated(superToken, alice, bob, expectedStartDate, expectedCliffDate, expectedSchedule.flowRate, expectedSchedule.endDate, expectedSchedule.cliffAmount, 0, expectedSchedule.remainderAmount); - // Intermediary `getCreateVestingScheduleParamsFromAmountAndDuration` test assertAreScheduleCreationParamsEqual( IVestingSchedulerV2.ScheduleCreationParams( superToken, bob, - expectedStartDate, + $.expectedStartDate, expectedSchedule.claimValidityDate, - expectedCliffDate, + $.expectedCliffDate, expectedSchedule.flowRate, expectedSchedule.cliffAmount, expectedSchedule.endDate, @@ -1060,6 +1059,9 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { ), vestingScheduler.getCreateVestingScheduleParamsFromAmountAndDuration(superToken, bob, totalAmount, totalDuration, cliffPeriod, startDate, 0)); + vm.expectEmit(); + emit VestingScheduleCreated(superToken, alice, bob, $.expectedStartDate, $.expectedCliffDate, expectedSchedule.flowRate, expectedSchedule.endDate, expectedSchedule.cliffAmount, 0, expectedSchedule.remainderAmount); + // Act vm.startPrank(alice); if (startDate == 0 && randomizer % 2 == 0) { @@ -1107,7 +1109,10 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { // Act console.log("Executing cliff and flow."); - vm.warp(expectedSchedule.cliffAndFlowDate + (vestingScheduler.START_DATE_VALID_AFTER() - (vestingScheduler.START_DATE_VALID_AFTER() / randomizer))); + uint32 randomFlowDelay = (vestingScheduler.START_DATE_VALID_AFTER() - (vestingScheduler.START_DATE_VALID_AFTER() / randomizer)); + vm.warp(expectedSchedule.cliffAndFlowDate + randomFlowDelay); + vm.expectEmit(); + emit VestingCliffAndFlowExecuted(superToken, alice, bob, expectedSchedule.cliffAndFlowDate, expectedSchedule.flowRate, expectedSchedule.cliffAmount, randomFlowDelay * SafeCast.toUint256(expectedSchedule.flowRate)); assertTrue(vestingScheduler.executeCliffAndFlow(superToken, alice, bob)); // Assert @@ -1123,9 +1128,14 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { if (randomizer % 7 != 0) { // # Test end execution on time. - // Act console.log("Executing end vesting early."); - vm.warp(expectedSchedule.endDate - (vestingScheduler.END_DATE_VALID_BEFORE() - (vestingScheduler.END_DATE_VALID_BEFORE() / randomizer))); + uint32 randomEarlyEndTime = (vestingScheduler.END_DATE_VALID_BEFORE() - (vestingScheduler.END_DATE_VALID_BEFORE() / randomizer)); + vm.warp(expectedSchedule.endDate - randomEarlyEndTime); + vm.expectEmit(); + uint256 earlyEndCompensation = randomEarlyEndTime * SafeCast.toUint256(expectedSchedule.flowRate) + expectedSchedule.remainderAmount; + emit VestingEndExecuted(superToken, alice, bob, expectedSchedule.endDate, earlyEndCompensation, false); + + // Act assertTrue(vestingScheduler.executeEndVesting(superToken, alice, bob)); // Assert @@ -1137,9 +1147,23 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { } else { // # Test end execution delayed. - // Act console.log("Executing end vesting late."); - vm.warp(expectedSchedule.endDate + (totalDuration / randomizer)); // There is some chance of overflow here. + uint32 randomLateEndDelay = (totalDuration / randomizer); + vm.warp(expectedSchedule.endDate + randomLateEndDelay); // There is some chance of overflow here. + + if (randomizer % 13 == 0) { + vm.startPrank(alice); + superToken.deleteFlow(alice, bob); + vm.stopPrank(); + + vm.expectEmit(); + emit VestingEndFailed(superToken, alice, bob, expectedSchedule.endDate); + } else { + vm.expectEmit(); + emit VestingEndExecuted(superToken, alice, bob, expectedSchedule.endDate, 0, true); + } + + // Act assertTrue(vestingScheduler.executeEndVesting(superToken, alice, bob)); // Assert From f5a41485d1a4f19f5dc30d2e0c08414f937e8cb8 Mon Sep 17 00:00:00 2001 From: Kaspar Kallas Date: Fri, 28 Jun 2024 10:24:59 +0300 Subject: [PATCH 07/15] refactor: use aggregate in all places --- .../contracts/VestingSchedulerV2.sol | 88 +++++++++++-------- 1 file changed, 49 insertions(+), 39 deletions(-) diff --git a/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol b/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol index 0da27ae308..b82250d94d 100644 --- a/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol +++ b/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol @@ -261,7 +261,7 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { uint32 totalDuration, bytes memory ctx ) external returns (bytes memory newCtx) { - newCtx = _createAndExecuteVestingScheduleFromAmountAndDuration( + newCtx = _validateAndCreateAndExecuteVestingScheduleFromAmountAndDuration( superToken, receiver, totalAmount, @@ -277,7 +277,7 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { uint256 totalAmount, uint32 totalDuration ) external { - _createAndExecuteVestingScheduleFromAmountAndDuration( + _validateAndCreateAndExecuteVestingScheduleFromAmountAndDuration( superToken, receiver, totalAmount, @@ -287,7 +287,7 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { } /// @dev IVestingScheduler.createAndExecuteVestingScheduleFromAmountAndDuration. - function _createAndExecuteVestingScheduleFromAmountAndDuration( + function _validateAndCreateAndExecuteVestingScheduleFromAmountAndDuration( ISuperToken superToken, address receiver, uint256 totalAmount, @@ -307,8 +307,11 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { ctx ); - address sender = _getSender(ctx); - assert(executeCliffAndFlow(superToken, sender, receiver)); + address sender = _getSender(newCtx); + ScheduleAggregate memory agg = _getVestingScheduleAggregate(superToken, sender, receiver); + + _validateBeforeCliffAndFlow(agg.schedule, /* disableClaimCheck: */ false); + assert(_executeCliffAndFlow(agg)); } /// @dev IVestingScheduler.createClaimableVestingSchedule implementation. @@ -468,9 +471,8 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { ) external returns (bytes memory newCtx) { newCtx = ctx; address sender = _getSender(ctx); - - bytes32 id = keccak256(abi.encodePacked(superToken, sender, receiver)); - VestingSchedule memory schedule = vestingSchedules[id]; + ScheduleAggregate memory agg = _getVestingScheduleAggregate(superToken, sender, receiver); + VestingSchedule memory schedule = agg.schedule; if (endDate <= block.timestamp) revert TimeWindowInvalid(); @@ -479,9 +481,9 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { // Only allow an update if 1. vesting exists 2. executeCliffAndFlow() has been called if (schedule.cliffAndFlowDate != 0 || schedule.endDate == 0) revert ScheduleNotFlowing(); - vestingSchedules[id].endDate = endDate; + vestingSchedules[agg.id].endDate = endDate; // Note: Nullify the remainder amount when complexity of updates is introduced. - vestingSchedules[id].remainderAmount = 0; + vestingSchedules[agg.id].remainderAmount = 0; emit VestingScheduleUpdated( superToken, @@ -489,7 +491,7 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { receiver, schedule.endDate, endDate, - vestingSchedules[id].remainderAmount + 0 ); } @@ -501,10 +503,11 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { ) external returns (bytes memory newCtx) { newCtx = ctx; address sender = _getSender(ctx); - bytes32 id = keccak256(abi.encodePacked(superToken, sender, receiver)); + ScheduleAggregate memory agg = _getVestingScheduleAggregate(superToken, sender, receiver); + VestingSchedule memory schedule = agg.schedule; - if (vestingSchedules[id].endDate != 0) { - delete vestingSchedules[id]; + if (schedule.endDate != 0) { + delete vestingSchedules[agg.id]; emit VestingScheduleDeleted(superToken, sender, receiver); } else { revert ScheduleDoesNotExist(); @@ -516,20 +519,21 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { ISuperToken superToken, address sender, address receiver - ) public returns (bool success) { + ) external returns (bool success) { ScheduleAggregate memory agg = _getVestingScheduleAggregate(superToken, sender, receiver); + VestingSchedule memory schedule = agg.schedule; - if (agg.schedule.claimValidityDate != 0) { + if (schedule.claimValidityDate != 0) { _validateAndClaim(agg); - _validateBeforeCliffAndFlow(agg.schedule, true); - if (block.timestamp >= _minDateToExecuteEndInclusive(agg.schedule)) { - _validateBeforeEndVesting(agg.schedule, true); + _validateBeforeCliffAndFlow(schedule, /* disableClaimCheck: */ true); + if (block.timestamp >= _minDateToExecuteEndInclusive(schedule)) { + _validateBeforeEndVesting(schedule, /* disableClaimCheck: */ true); success = _executeVestingAsSingleTransfer(agg); } else { success = _executeCliffAndFlow(agg); } } else { - _validateBeforeCliffAndFlow(agg.schedule, false); + _validateBeforeCliffAndFlow(schedule, /* disableClaimCheck: */ false); success = _executeCliffAndFlow(agg); } } @@ -537,15 +541,17 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { function _validateAndClaim( ScheduleAggregate memory agg ) private { + VestingSchedule memory schedule = agg.schedule; + // Ensure that the caller is the sender or the receiver if the vesting schedule requires claiming. if (msg.sender != agg.sender && msg.sender != agg.receiver) revert CannotClaimScheduleOnBehalf(); - if (agg.schedule.claimValidityDate < block.timestamp) + if (schedule.claimValidityDate < block.timestamp) revert TimeWindowInvalid(); - emit VestingClaimed(agg.superToken, agg.sender, agg.receiver, msg.sender); delete vestingSchedules[agg.id].claimValidityDate; + emit VestingClaimed(agg.superToken, agg.sender, agg.receiver, msg.sender); } function _validateBeforeCliffAndFlow( @@ -568,30 +574,32 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { function _executeCliffAndFlow( ScheduleAggregate memory agg ) private returns (bool success) { + VestingSchedule memory schedule = agg.schedule; + // Invalidate configuration straight away -- avoid any chance of re-execution or re-entry. delete vestingSchedules[agg.id].cliffAndFlowDate; delete vestingSchedules[agg.id].cliffAmount; // Compensate for the fact that flow will almost always be executed slightly later than scheduled. uint256 flowDelayCompensation = - (block.timestamp - agg.schedule.cliffAndFlowDate) * uint96(agg.schedule.flowRate); + (block.timestamp - schedule.cliffAndFlowDate) * uint96(schedule.flowRate); // If there's cliff or compensation then transfer that amount. - if (agg.schedule.cliffAmount != 0 || flowDelayCompensation != 0) { + if (schedule.cliffAmount != 0 || flowDelayCompensation != 0) { // Note: Super Tokens revert, not return false, i.e. we expect always true here. assert( agg.superToken.transferFrom( - agg.sender, agg.receiver, agg.schedule.cliffAmount + flowDelayCompensation)); + agg.sender, agg.receiver, schedule.cliffAmount + flowDelayCompensation)); } // Create a flow according to the vesting schedule configuration. - cfaV1.createFlowByOperator(agg.sender, agg.receiver, agg.superToken, agg.schedule.flowRate); + cfaV1.createFlowByOperator(agg.sender, agg.receiver, agg.superToken, schedule.flowRate); emit VestingCliffAndFlowExecuted( agg.superToken, agg.sender, agg.receiver, - agg.schedule.cliffAndFlowDate, - agg.schedule.flowRate, - agg.schedule.cliffAmount, + schedule.cliffAndFlowDate, + schedule.flowRate, + schedule.cliffAmount, flowDelayCompensation ); @@ -601,9 +609,11 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { function _executeVestingAsSingleTransfer( ScheduleAggregate memory agg ) private returns (bool success) { + VestingSchedule memory schedule = agg.schedule; + delete vestingSchedules[agg.id]; - uint256 totalVestedAmount = _getTotalVestedAmount(agg.schedule); + uint256 totalVestedAmount = _getTotalVestedAmount(schedule); // Note: Super Tokens revert, not return false, i.e. we expect always true here. assert(agg.superToken.transferFrom(agg.sender, agg.receiver, totalVestedAmount)); @@ -612,9 +622,9 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { agg.superToken, agg.sender, agg.receiver, - agg.schedule.cliffAndFlowDate, + schedule.cliffAndFlowDate, 0, - agg.schedule.cliffAmount, + schedule.cliffAmount, 0 ); @@ -622,7 +632,7 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { agg.superToken, agg.sender, agg.receiver, - agg.schedule.endDate, + schedule.endDate, totalVestedAmount, false ); @@ -650,13 +660,13 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { address sender, address receiver ) external returns (bool success) { - bytes32 id = keccak256(abi.encodePacked(superToken, sender, receiver)); - VestingSchedule memory schedule = vestingSchedules[id]; + ScheduleAggregate memory agg = _getVestingScheduleAggregate(superToken, sender, receiver); + VestingSchedule memory schedule = agg.schedule; - _validateBeforeEndVesting(schedule, false); + _validateBeforeEndVesting(schedule, /* disableClaimCheck: */ false); // Invalidate configuration straight away -- avoid any chance of re-execution or re-entry. - delete vestingSchedules[id]; + delete vestingSchedules[agg.id]; // If vesting is not running, we can't do anything, just emit failing event. if (_isFlowOngoing(superToken, sender, receiver)) { @@ -801,12 +811,12 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { address sender, address receiver ) public view override returns (uint256) { - VestingSchedule memory vestingSchedule = getVestingSchedule( + VestingSchedule memory schedule = getVestingSchedule( superToken, sender, receiver ); - return getMaximumNeededTokenAllowance(vestingSchedule); + return getMaximumNeededTokenAllowance(schedule); } /// @dev IVestingScheduler.getMaximumNeededTokenAllowance implementation. From 9ccce34f3eaae4e1ed4c6ae8ed7db7ca6f985b7f Mon Sep 17 00:00:00 2001 From: Kaspar Kallas Date: Fri, 28 Jun 2024 10:45:02 +0300 Subject: [PATCH 08/15] refactor: re-order some functions based on visibility --- .../contracts/VestingSchedulerV2.sol | 66 +++++++++---------- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol b/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol index b82250d94d..dcbf949692 100644 --- a/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol +++ b/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol @@ -713,39 +713,6 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { return vestingSchedules[keccak256(abi.encodePacked(superToken, sender, receiver))]; } - function _getVestingScheduleAggregate( - ISuperToken superToken, - address sender, - address receiver - ) public view returns (ScheduleAggregate memory) { - bytes32 id = keccak256(abi.encodePacked(superToken, sender, receiver)); - return ScheduleAggregate({ - superToken: superToken, - sender: sender, - receiver: receiver, - id: id, - schedule: vestingSchedules[id] - }); - } - - /// @dev get sender of transaction from Superfluid Context or transaction itself. - function _getSender(bytes memory ctx) internal view returns (address sender) { - if (ctx.length != 0) { - if (msg.sender != address(cfaV1.host)) revert HostInvalid(); - sender = cfaV1.host.decodeCtx(ctx).msgSender; - } else { - sender = msg.sender; - } - // This is an invariant and should never happen. - assert(sender != address(0)); - } - - /// @dev get flowRate of stream - function _isFlowOngoing(ISuperToken superToken, address sender, address receiver) internal view returns (bool) { - (,int96 flowRate,,) = cfaV1.cfa.getFlow(superToken, sender, receiver); - return flowRate != 0; - } - /// @dev IVestingScheduler.getCreateVestingScheduleParamsFromAmountAndDuration implementation. function getCreateVestingScheduleParamsFromAmountAndDuration( ISuperToken superToken, @@ -848,6 +815,39 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { } } + function _getVestingScheduleAggregate( + ISuperToken superToken, + address sender, + address receiver + ) public view returns (ScheduleAggregate memory) { + bytes32 id = keccak256(abi.encodePacked(superToken, sender, receiver)); + return ScheduleAggregate({ + superToken: superToken, + sender: sender, + receiver: receiver, + id: id, + schedule: vestingSchedules[id] + }); + } + + /// @dev get sender of transaction from Superfluid Context or transaction itself. + function _getSender(bytes memory ctx) internal view returns (address sender) { + if (ctx.length != 0) { + if (msg.sender != address(cfaV1.host)) revert HostInvalid(); + sender = cfaV1.host.decodeCtx(ctx).msgSender; + } else { + sender = msg.sender; + } + // This is an invariant and should never happen. + assert(sender != address(0)); + } + + /// @dev get flowRate of stream + function _isFlowOngoing(ISuperToken superToken, address sender, address receiver) internal view returns (bool) { + (,int96 flowRate,,) = cfaV1.cfa.getFlow(superToken, sender, receiver); + return flowRate != 0; + } + function _getTotalVestedAmount( VestingSchedule memory schedule ) private pure returns (uint256) { From f99689d9f7139c868cd79cd12b32dc5b261faec6 Mon Sep 17 00:00:00 2001 From: Kaspar Kallas Date: Fri, 28 Jun 2024 10:50:15 +0300 Subject: [PATCH 09/15] chore: add small comment --- .../scheduler/contracts/VestingSchedulerV2.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol b/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol index dcbf949692..bf5423b3c1 100644 --- a/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol +++ b/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol @@ -491,7 +491,7 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { receiver, schedule.endDate, endDate, - 0 + 0 // remainderAmount ); } From 340a7fb49a075812c2a8cf4aedd1d86167e92c4b Mon Sep 17 00:00:00 2001 From: Kaspar Kallas Date: Fri, 28 Jun 2024 10:54:23 +0300 Subject: [PATCH 10/15] refactor: small whitespace fix --- .../scheduler/test/VestingSchedulerV2.t.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/automation-contracts/scheduler/test/VestingSchedulerV2.t.sol b/packages/automation-contracts/scheduler/test/VestingSchedulerV2.t.sol index d68e859a68..11cf53a213 100644 --- a/packages/automation-contracts/scheduler/test/VestingSchedulerV2.t.sol +++ b/packages/automation-contracts/scheduler/test/VestingSchedulerV2.t.sol @@ -203,7 +203,7 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { assertEq(schedule.claimValidityDate, deletedSchedule.claimValidityDate, "claimValidityDate mismatch"); } - function _getExpectedSchedule( + function _getExpectedSchedule( uint32 startDate, uint32 cliffDate, int96 flowRate, From c4192fd8b15e308e8134f92e2b82a92f5eb37d9d Mon Sep 17 00:00:00 2001 From: Kaspar Kallas Date: Fri, 28 Jun 2024 12:00:35 +0300 Subject: [PATCH 11/15] refactor: use named parameters when using structs --- .../contracts/VestingSchedulerV2.sol | 148 +++++++++--------- 1 file changed, 74 insertions(+), 74 deletions(-) diff --git a/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol b/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol index bf5423b3c1..21422f5128 100644 --- a/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol +++ b/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol @@ -61,17 +61,17 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { bytes memory ctx ) external returns (bytes memory newCtx) { newCtx = _validateAndCreateVestingSchedule( - ScheduleCreationParams( - superToken, - receiver, - startDate, - 0, // claimValidityDate - cliffDate, - flowRate, - cliffAmount, - endDate, - 0 // remainderAmount - ), + ScheduleCreationParams({ + superToken: superToken, + receiver: receiver, + startDate: startDate, + claimValidityDate: 0, + cliffDate: cliffDate, + flowRate: flowRate, + cliffAmount: cliffAmount, + endDate: endDate, + remainderAmount: 0 + }), ctx ); } @@ -87,17 +87,17 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { uint32 endDate ) external { _validateAndCreateVestingSchedule( - ScheduleCreationParams( - superToken, - receiver, - startDate, - 0, // claimValidityDate - cliffDate, - flowRate, - cliffAmount, - endDate, - 0 // remainderAmount - ), + ScheduleCreationParams({ + superToken: superToken, + receiver: receiver, + startDate: startDate, + claimValidityDate: 0, + cliffDate: cliffDate, + flowRate: flowRate, + cliffAmount: cliffAmount, + endDate: endDate, + remainderAmount: 0 + }), bytes("") ); } @@ -140,14 +140,14 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { bytes32 id = keccak256(abi.encodePacked(params.superToken, sender, params.receiver)); if (vestingSchedules[id].endDate != 0) revert ScheduleAlreadyExists(); - vestingSchedules[id] = VestingSchedule( - cliffAndFlowDate, - params.endDate, - params.flowRate, - params.cliffAmount, - params.remainderAmount, - params.claimValidityDate - ); + vestingSchedules[id] = VestingSchedule({ + cliffAndFlowDate: cliffAndFlowDate, + endDate: params.endDate, + flowRate: params.flowRate, + cliffAmount: params.cliffAmount, + remainderAmount: params.remainderAmount, + claimValidityDate: params.claimValidityDate + }); emit VestingScheduleCreated( params.superToken, @@ -327,17 +327,17 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { bytes memory ctx ) external returns (bytes memory newCtx) { newCtx = _validateAndCreateVestingSchedule( - ScheduleCreationParams( - superToken, - receiver, - startDate, - claimValidityDate, - cliffDate, - flowRate, - cliffAmount, - endDate, - 0 /* remainderAmount */ - ), + ScheduleCreationParams({ + superToken: superToken, + receiver: receiver, + startDate: startDate, + claimValidityDate: claimValidityDate, + cliffDate: cliffDate, + flowRate: flowRate, + cliffAmount: cliffAmount, + endDate: endDate, + remainderAmount: 0 + }), ctx ); } @@ -354,17 +354,17 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { uint32 endDate ) external { _validateAndCreateVestingSchedule( - ScheduleCreationParams( - superToken, - receiver, - startDate, - claimValidityDate, - cliffDate, - flowRate, - cliffAmount, - endDate, - 0 /* remainderAmount */ - ), + ScheduleCreationParams({ + superToken: superToken, + receiver: receiver, + startDate: startDate, + claimValidityDate: claimValidityDate, + cliffDate: cliffDate, + flowRate: flowRate, + cliffAmount: cliffAmount, + endDate: endDate, + remainderAmount: 0 + }), bytes("") ); } @@ -741,34 +741,34 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { ); if (cliffPeriod == 0) { - result = ScheduleCreationParams( - superToken, - receiver, - startDate, - claimValidityDate, - 0 /* cliffDate */, - flowRate, - 0 /* cliffAmount */, - endDate, - remainderAmount - ); + result = ScheduleCreationParams({ + superToken: superToken, + receiver: receiver, + startDate: startDate, + claimValidityDate: claimValidityDate, + cliffDate: 0 /* cliffDate */, + flowRate: flowRate, + cliffAmount: 0 /* cliffAmount */, + endDate: endDate, + remainderAmount: remainderAmount + }); } else { uint32 cliffDate = startDate + cliffPeriod; uint256 cliffAmount = SafeMath.mul( cliffPeriod, SafeCast.toUint256(flowRate) ); - result = ScheduleCreationParams( - superToken, - receiver, - startDate, - claimValidityDate, - cliffDate, - flowRate, - cliffAmount, - endDate, - remainderAmount - ); + result = ScheduleCreationParams({ + superToken: superToken, + receiver: receiver, + startDate: startDate, + claimValidityDate: claimValidityDate, + cliffDate: cliffDate, + flowRate: flowRate, + cliffAmount: cliffAmount, + endDate: endDate, + remainderAmount: remainderAmount + }); } } From a6bbc62278c9639ecd662d1053ee3a2ff3d90fe3 Mon Sep 17 00:00:00 2001 From: Kaspar Kallas Date: Fri, 28 Jun 2024 12:02:45 +0300 Subject: [PATCH 12/15] refactor: remove unnecessary comments --- .../scheduler/contracts/VestingSchedulerV2.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol b/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol index 21422f5128..1630837c84 100644 --- a/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol +++ b/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol @@ -746,9 +746,9 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { receiver: receiver, startDate: startDate, claimValidityDate: claimValidityDate, - cliffDate: 0 /* cliffDate */, + cliffDate: 0, flowRate: flowRate, - cliffAmount: 0 /* cliffAmount */, + cliffAmount: 0, endDate: endDate, remainderAmount: remainderAmount }); From 6ae1368cd3cc59a636dc862ee230bc0929be3a26 Mon Sep 17 00:00:00 2001 From: Kaspar Kallas Date: Fri, 28 Jun 2024 12:48:56 +0300 Subject: [PATCH 13/15] fix: change type in log event --- .../scheduler/contracts/interface/IVestingSchedulerV2.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/automation-contracts/scheduler/contracts/interface/IVestingSchedulerV2.sol b/packages/automation-contracts/scheduler/contracts/interface/IVestingSchedulerV2.sol index 751a15e160..2bbefc5e1c 100644 --- a/packages/automation-contracts/scheduler/contracts/interface/IVestingSchedulerV2.sol +++ b/packages/automation-contracts/scheduler/contracts/interface/IVestingSchedulerV2.sol @@ -87,7 +87,7 @@ interface IVestingSchedulerV2 { uint32 endDate, uint256 cliffAmount, uint32 claimValidityDate, - uint256 remainderAmount + uint96 remainderAmount ); /** From 5b9945b0d87820fc53161b26311d43762611d3de Mon Sep 17 00:00:00 2001 From: Kaspar Kallas Date: Fri, 28 Jun 2024 17:23:05 +0300 Subject: [PATCH 14/15] chore: test claim event --- .../scheduler/test/VestingSchedulerV2.t.sol | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/packages/automation-contracts/scheduler/test/VestingSchedulerV2.t.sol b/packages/automation-contracts/scheduler/test/VestingSchedulerV2.t.sol index 11cf53a213..cf825f5765 100644 --- a/packages/automation-contracts/scheduler/test/VestingSchedulerV2.t.sol +++ b/packages/automation-contracts/scheduler/test/VestingSchedulerV2.t.sol @@ -26,7 +26,7 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { uint32 endDate, uint256 cliffAmount, uint32 claimValidityDate, - uint256 remainderAmount + uint96 remainderAmount ); event VestingScheduleUpdated( @@ -70,6 +70,13 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { uint32 endDate ); + event VestingClaimed( + ISuperToken indexed superToken, + address indexed sender, + address indexed receiver, + address claimer + ); + event Transfer(address indexed from, address indexed to, uint256 value); /// @dev This is required by solidity for using the SuperTokenV1Library in the tester @@ -1759,7 +1766,9 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { testAssertScheduleDoesNotExist(address(superToken), alice, bob); } - function test_executeCliffAndFlow_claimAfterEndDate(uint256 delayAfterEndDate, uint256 claimDate) public { + function test_executeCliffAndFlow_claimAfterEndDate(uint256 delayAfterEndDate, uint256 claimDate, uint8 randomizer) public { + randomizer = SafeCast.toUint8(bound(randomizer, 1, type(uint8).max)); + uint256 aliceInitialBalance = superToken.balanceOf(alice); uint256 bobInitialBalance = superToken.balanceOf(bob); @@ -1776,6 +1785,12 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { vm.warp(claimDate); + address claimer = randomizer % 2 == 0 ? bob : alice; + vm.expectEmit(true, true, true, false); + emit VestingClaimed( + superToken, alice, bob, claimer + ); + vm.expectEmit(true, true, true, true); emit Transfer(alice, bob, totalExpectedAmount); @@ -1793,7 +1808,7 @@ contract VestingSchedulerV2Tests is FoundrySuperfluidTester { assertEq(vestingScheduler.getMaximumNeededTokenAllowance(schedule), totalExpectedAmount); assertEq(vestingScheduler.getMaximumNeededTokenAllowance(address(superToken), alice, bob), totalExpectedAmount); - vm.prank(bob); + vm.prank(claimer); assertTrue(vestingScheduler.executeCliffAndFlow(superToken, alice, bob)); assertEq(superToken.balanceOf(alice), aliceInitialBalance - totalExpectedAmount); From 71ffdbe290cfee39aeeb923b507cad5aa0ed2295 Mon Sep 17 00:00:00 2001 From: Kaspar Kallas Date: Fri, 28 Jun 2024 18:39:23 +0300 Subject: [PATCH 15/15] chore: add version --- .../scheduler/contracts/VestingSchedulerV2.sol | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol b/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol index 1630837c84..d7fa89a936 100644 --- a/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol +++ b/packages/automation-contracts/scheduler/contracts/VestingSchedulerV2.sol @@ -11,6 +11,8 @@ import { SafeMath } from "@openzeppelin/contracts/utils/math/SafeMath.sol"; import { SafeCast } from "@openzeppelin/contracts/utils/math/SafeCast.sol"; contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { + string public constant VERSION = "2.0.0-beta"; + using CFAv1Library for CFAv1Library.InitData; CFAv1Library.InitData public cfaV1; mapping(bytes32 => VestingSchedule) public vestingSchedules; // id = keccak(supertoken, sender, receiver) @@ -583,7 +585,7 @@ contract VestingSchedulerV2 is IVestingSchedulerV2, SuperAppBase { // Compensate for the fact that flow will almost always be executed slightly later than scheduled. uint256 flowDelayCompensation = (block.timestamp - schedule.cliffAndFlowDate) * uint96(schedule.flowRate); - + // If there's cliff or compensation then transfer that amount. if (schedule.cliffAmount != 0 || flowDelayCompensation != 0) { // Note: Super Tokens revert, not return false, i.e. we expect always true here.