Submitted by eierina, also found by 0x52, 0xA5DF, 0xdapper, ElKu, obront, pauliax, pcarranzav, pedroais, rbserver, Ruhum, RustyRabbit, and TomJ
VTVLVesting.sol#L418
VTVLVesting.sol#L147-L151
VTVLVesting.sol#L364
Vesting is a legal term that means the point in time where property is earned or gained by some person.
The VTVLVesting contract defines:
The entitled user can either withdraw after each interval elapses, or after the whole vesting period is over or any variant of the two options.
The administrator of the contract can revoke the claim for a user at any time, which for vesting assets is expected. For example an employee with a vesting stock allocation of 1000 shares vesting at each quarter over a period of 4 years, may resign after 2 years and therefore the only half of the shares would be vested and therefore sold by the employee. The employee can either sell them at each quarter, or before, or after resigning, in any case the half of the shares have vested and are by legal right owned by the employee.
The VTVLContract revoke has the following defects:
Raising as high impact because it actually causes:
The following two tests prove the behaviour difference when the order by which revokeClaim vs withdraw are called, whch shows that the vesting right is not guaranteed.
Below are, in order, a test and a diff/patch for a proposed fix. The proposed fix is just an idea at how to fix, or in other words, a way to preserve the already vested amount when claim is revoked.
The diff/patch add a deactivationTimestamp to claim, and a new revokeClaimProper that shall replace the revokeClaim function to correct the behaviour.
The deactivationTimestamp is used to track the deactivation time for the claim in order to preserve the amount vested so far and allow the user to withdraw the amount righteously earned so far. The _baseVestedAmount and hasActiveClaim have been updated to do proper math when isActive is false but deactivationTimestamp is greater than 0.
The finalVestedAmount has been update to show the what would be amount if the vesting would have reached the claim endTimestamp while the finalClaimableAmount takes into consideration the deactivationTimestamp if the claim has been revoked.
The test shows that the already vested amount (cliff + half way linear vesting) is preserved.
lawrencehui (VTVL) confirmed and commented:
0xean (judge) commented:
