Feat/vesting work

[philbow61]

Description

This PR adds the new VestingLock contract between a hedgey vesting plan and a beneficiary.
The contract will lock/relock all tokens released in Years 1&2 into the veMento contract, redeem all potential withdrawable tokens from a lock and transfer them with redeemable tokens from Year 3&4 to the beneficiary.

Other changes

• Extended the ILocking interface
• Added ITokenVestingPlans interface

Tested

• unit tests for the VEstingLock contract
• fork test to test/verify the actual interaction between the 3 contracts are designed and will be written in the next step

Open Questions:

• Not sure whether we want this contract to emit any Events e.g redeemed(amountLocked, amountWithdrawn, lockID)
• So far the lock calculation is not 100% perfect e.g if someone does the first redemption after year 3 this contract would lock up the whole 50% of tokens with a slope of 1 year. This is not super bad but we could add a step in the calculation that: when a redeem with a lock happens after the cliffPeriodEnd, it calculates the amount that would be redeemable from the lock by now and transfers that amount to the beneficiary and only locks the remainder.
  Let me know what you think @bowd @baroooo
  this could look something like this:
  if(amountToLock > 0 && lockingCliffEndWeek < currentWeek):
uint256 denominator = lockingSlopeEnd - lockingCliffEnd
uint256 numerator = lockingSlopeEnd - currentWeek
amountToLock = (amountToLock * numerator) / denominator

Related issues

• Fixes Implement VestingLock.sol #342

Backwards compatibility

Documentation

[openzeppelin-code]

Feat/vesting work

Generated at commit: 677fc679ac0b1dd4e929d524cea66a0a9ae4814a

🚨 Report Summary

	Severity Level	Results
Contracts	Critical High Medium Low Note Total	2 1 0 16 40 59
Dependencies	Critical High Medium Low Note Total	0 0 0 0 0 0

---

For more details view the full report in OpenZeppelin Code Inspector

[baroooo]

what do you think about deploying VestingLock contracts from a factory? So we can keep track of all contracts in a single place and don't need to pass _hedgeyVestingContract and _veMentoLockingContract everytime, we can call with mentoToken address from the factory. And deploying a new contract is simple as calling a function.

[baroooo]

I think it is fine to just add these function in ILocking, instead of creating ILockingExtended

[baroooo]

I would move this line to constructor. because of this, we call setWithdraw with 0 amount in every skipWeek()

[baroooo]

can we add natspec comments that explains the contract and also function params

[baroooo]

I think we can remove this. see my comment on the MockLocking.sol

[baroooo]

lets change params to have trailing underscore for convention

[philbow61]

Similar to the underscore in front of internal functions I was trying to follow how it's done in other Mento contracts e.g BreakerBox and the breakers but I am more than happy to change it if we want trailing underscores

[baroooo]

as we discussed, this is vulnerable to griefing as initializeVestingPlan can be frontrunned by creating a vesting plan for this contract. I don't see any problems with this since it has no financial impact or gain for the attacker as long as we can revoke any plans we have on hedgey side.
a straightforward solution would be, we make this function to take planId as param and is onlyOwner but not sure if it worths it

[baroooo]

Not sure whether we want this contract to emit any Events e.g redeemed(amountLocked, amountWithdrawn, lockID)

Yes, generally we should emit events for all non-view operations

[bayological]

Closing this as we will no longer need this contract. If it turns out we do need this then we can re-open.
@philbow61 @bowd