Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Locking to handle upcoming block.time change #538

Closed
4 tasks
baroooo opened this issue Nov 1, 2024 · 0 comments · Fixed by #542
Closed
4 tasks

Update Locking to handle upcoming block.time change #538

baroooo opened this issue Nov 1, 2024 · 0 comments · Fixed by #542
Assignees

Comments

@baroooo
Copy link
Contributor

baroooo commented Nov 1, 2024

Description

With Celo transitioning to a L2, the block time will be reduced from 5 seconds to 1 second. This change impacts our Locking and Governance mechanisms, which currently depend on a block time of 5 seconds, calculating a "week" as 120960 blocks. With the new block time, a week will be equal 604800 blocks.

To ensure the continuity of functionality after the L2 upgrade, we need to update the WEEK constant and adjust startingPointWeek and roundTimestamp such that the week calculation in the Locking contract remains unchanged post-upgrade.

  • Add a NEW_WEEK constant in the contract to accommodate the new block time (1s/block) 604,800 blocks per week.
  • Create variables: mentoLabsMultisig address, l2Block(uint256), newStartingWeek(uint256), oldEpochShift(constant uint32), newEpochShift(uint32) and paused(bool)
  • Setters for these vars, should be only callable by the mentoLabsMultisig
  • setL2TransitionBlock, which allows us to specify the exact block number for the L2 transition once it is known, will:
    set l2Block no
    pause the contract
  • roundTimestamp should be pausable, and it should pause all functionality after the l2Block is set
  • roundTimestamp should use the correct WEEK, startingPointWeek, and shift vars depending on the param it is called with. it should also handle the case where we need to add the starting point week instead of subtraction to handle the underflow

Testing

  • Conduct forked tests to validate functionality and ensure correct values across a range of block times and scenarios.
  • setL2TransitionBlock
  • Calculate the correct newStartingPointWeek and set it
  • Calculate the correct newShift and set it
  • Write tests to cover: totalSupply and pastTotalSupply, balanceOf, Voting power and past voting power, lockedAmount and withdrawableAmount for multiple lock owners
  • Update the _gap array to ensure storage slots are not affected

Acceptance Criteria:

  • Locking contracts accurately handle both 5s and 1s block times.
  • setL2TransitionBlock function successfully updates and registers L2 block number.
  • roundTimestamp() correctly adjusts for week calculations post and pre transition.
  • Tests confirm correctness across total supply, balances, voting power, locked and withdraw-able amounts for multiple lock owners at various points in time.
@baroooo baroooo self-assigned this Nov 8, 2024
baroooo added a commit that referenced this issue Dec 6, 2024
### Description

Updates Locking Contracts to handle upcoming block.time change

Mainly changes the roundTimestamp() to have a forked implementation that
uses different values to calculate week no for the blocks before and
after l2 transition. Week numbers that return from roundTimestamp before
and after should be continues and consistent.

### Other changes

No

### Tested

Unit and fork tests

### Related issues

- Fixes #538

---------

Co-authored-by: philbow61 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant