Cliff does not work as expected

[telezhnaya]

Looks like we have a bug in calculation of unrelease_amount:

let unreleased_amount = U256::from(lockup_amount) * time_left / U256::from(release_duration);

Let's suppose that we have classic contract 4-years long in total, with 1-year cliff.
It does not clear, what release_duration means. Doc says it's the duration when the full lockup amount will be available. Does it mean 3 or 4 years for our case? Anyway, we will have problems further.

1. Let's suppose that release_duration == 3 years for such case.
   It means that unreleased_amount will not take into account cliff. Our lockup_amount will start unlocking after we pass the cliff without unlocking 25% at the early beginning. To fix that, the formula should look like:

let unreleased_amount = U256::from(lockup_amount) * time_left / U256::from(release_duration + self.lockup_information.lockup_duration);

2. Let's suppose that release_duration == 4 years for such case.
   It means that our contract will last 5 years instead of 4 because of this line. To fix that, we should also apply this change:

let end_timestamp = lockup_timestamp.saturating_add(release_duration - self.lockup_information.lockup_duration);

(code could be simplified, I just tried to explain the idea)

[frol]

UPDATE: The details below reflect the 3.0.0 version of lockup contract! The currently deploy contracts are not affected by this bug due to a happy coincidence, see detail in a new comment below!

Well, simply saying the problem is that the lockup contract does not have a concept of a cliff at all; it just has a linear unlock from the starting date (either phase 2 launch date + lockup_duration OR lockup_timestamp) to the end date (start date + release_duration).

@evgenykuzyakov @SkidanovAlex @ilblackdragon @eriktrautman Is this intended?

I checked my lockup contract and the unlock schedule there is going to be (according to the 3.0.0 contract implementation):

• 2021-10-13 I will start getting the tokens unlocked (not 25%, but just a linear unlock)
• 2022-10-13 my unlocked tokens will reach 25%
• 2025-10-13 I will have all my tokens unlocked (notice, it is 5 years from the phase 2 launch)

Ultimately, would we deploy 3.0.0 version of lockup contract team tokens would be locked for 5 years since phase 2 launch and will get linearly unlocked starting 2021-10-13 (no 25% cliff)

[marcinbodnar]

@telezhnaya This problem exists in the current version of the lockup contract. There is also another problem, as I explained here #140 .

I checked my lockup contract and the unlock schedule there is going to be (according to the contract implementation):

@frol It will not affect team-tokens lockup contracts, and they will release properly. Yes, the starting date is calculated wrongly but it's not affecting end_timestamp and the calculations will be proper. That's what I was trying to explain here near/near-wallet#1668

If you check the lockup contract before version 3.0.0 of core-contracts you will see that end_timestamp is always taking transfers_timestamp to calculate, so the time_left values of unlocked tokens will be proper, and whole calculation will be proper:

core-contracts/lockup/src/getters.rs

Line 81 in 890ad50

let end_timestamp = transfers_timestamp.0.saturating_add(release_duration);

I added this repo:
https://marcinbodnar.github.io/account-lookup-core/
to simulate how the core-contracts will calculate the locked amount of tokens (same logic as lockup-contract before 3.0.0), it is set to simulate the date 20/04/2022 (one year from yesterday).

So team tokens contracts are fine, but the current version of lockup contracts has two problems, please check #140 .

[marcinbodnar]

I confirmed that the release_duration is 4 years (via view-state on my lockup), and my understanding of the code is identical to what @frol outlined above: as implemented now, lockup releases 0% at 1yr mark, and then releases linearly from year 1 until year 5.
I initiated the discussion with NF on how this can be mitigated.

EDIT: @marcinbodnar, are you suggesting that the bug Olga is pointing to above is not present in the contracts deployed on the mainnet, and was introduced later?

@SkidanovAlex yes, the situation that @frol described is present in the current version of lockup contract implementation, the contracts before 3.0.0 (before #136) are fine.

In the previous version (before 3.0.0) of the lockup contract the release start date is calculated wrongly, but it's not affecting end_timestamp, and time_left - and these two are used to calculate locked and unlocked values - so after one year from phase 2 25% will be released (for contracts with cliff).

[maxzaver]

CCing @mikedotexe since core contracts are responsibility of DevPlatform.

[maxzaver]

Added banner to the readme file #142

[telezhnaya]

Added tags to track versions easier, see previous realisation here
https://github.com/near/core-contracts/blob/lockup-v2.0.0/lockup/src/getters.rs#L65

[frol]

TL;DR: The lockup contracts deployed for the team tokens distribution (4 years lockups) are not affected due to a lucky bug that we "resolved" only recently in #136!

• Before 2021-10-13 the lockup contract 2.0.0 will not pass through:
  

  core-contracts/lockup/src/getters.rs

  Line 78 in 56972b1

  if lockup_timestamp <= block_timestamp {

  Thus, all the tokens will remain locked.

• After 2021-10-13 the check will be passed, and in lockup contract 2.0.0 we compute the end_timestamp based on the Phase-2 timestamp:

  core-contracts/lockup/src/getters.rs

  Line 81 in 56972b1

  let end_timestamp = transfers_timestamp.0.saturating_add(release_duration);

  Thus, the end_timestamp will be 2024-10-13, and the unreleased_amount will ultimately be total_locked_tokens * 3 years / 4 years, so I will get 25% tokens unlocked on 2021-10-13

• 2024-10-13 I will have all the tokens unlocked!

The cliff is there and the total lockup duration matches what was promised! 🎉

P.S. If we have non-4-year lockups with cliffs, those won't be 25% unlocked on the cliff-day, e.g. if there is a 2-year lockup with 1 year "cliff", it will unlock 50% on the "cliff" day.

[marcinbodnar]

@frol correct, as explained in my comment here:
#141 (comment)
and #140

[robert-zaremba]

What's the ETA? We are looking to use this contract.

[telezhnaya]

We have 2 conflicting definitions for release_duration field:

• https://github.com/near/core-contracts/blob/master/lockup/src/lib.rs#L173
• https://github.com/near/core-contracts/blob/master/lockup/src/types.rs#L40

[telezhnaya]

@robert-zaremba I am working on it right now. I am afraid to give any time estimate.
You could have a look at the pull request that I just created.

TLDR:
We don't have cliff logic on lockup layer, while we have correct implementation on vesting level. It only means that we need to update the documentation. Let's see if the reviewers agree with it.

[robert-zaremba]

OK, so the logic works (i didn't audit it) if cliff=0?

[telezhnaya]

@robert-zaremba Yes

[telezhnaya]

In previous version, there was an issue with calculating start date. It means that we have an error for these contracts:
problem_lockups.txt

(58 lines)
Example of data:

creation month, id,expected start date
dec20,9b4476caf43b49f4f7b163253dce5c06bd3b31ae.lockup.near,2020-11-18 03:00:00
dec20,f04ef81b98b53db24fbc955085d48d7f596e265f.lockup.near,2020-11-25 03:00:00
jan21,f02bcf25be00d528e8009bc3ffd1d0aded87df0e.lockup.near,2020-10-21 01:00:00
jan21,6d9f7b0d735fefb5521a69157b7c95e1d9dfe549.lockup.near,2020-12-01 03:00:00
jan21,3ad67bc0afba9e460328cade87f48d6ed7b1c3cc.lockup.near,2020-10-26 13:00:00

For them, we wanted to start the contract from the date in third column, but the start is actually the moment of enabling transfers.