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

Correct Bank timestamp drift #12681

Closed

Conversation

CriesofCarrots
Copy link
Contributor

@CriesofCarrots CriesofCarrots commented Oct 6, 2020

Problem

The timestamp returned by Bank::unix_timestamp() ends up in the clock sysvar and is used to assess time-based stake account lockouts. However it's based on a theoretical slots-per-second instead of reality, so it's quite inaccurate. This will pose a problem for lockups, since the accounts will not register as lockup-free on (or anytime near) the date the lockup is set to expire.

Block times are estimated using a validator timestamp oracle; this data provides an opportunity to align the bank timestamp more closely with real-world time.

Summary of Changes

  • Add feature-gated EpochTimestamps sysvar that stores:
    1. The timestamp at the beginning of the epoch. Use it for updates to the clock sysvar, so that drift only happens from the beginning of the epoch. (Bank::unix_timestamp_from_epoch)
    2. A selection of slots in the upcoming epoch to sample.
  • Add Bank method to get a bank's stake-weighted timestamp estimate using epoch vote accounts last_timestamp.
  • Add Bank::update_epoch_timestamps:
    1. On Bank::new_from_parent, check the expected timestamp sample slots, and if one has passed, populate the sample with the stake-weighted timestamp.
    2. On epoch boundary, use populated sample timestamps to set a new epoch base timestamp in EpochTimestamps sysvar. Right now, all timestamp samples receive equal weight, regardless of when in the epoch they occurred. (TODO: Set sample slots for the upcoming epoch)

Right now, even with the feature activated this code is a no-op for the timestamp in the clock sysvar. Because the expected samples map is never populated, the timestamp at the epoch boundary defaults to the Bank::unix_timestamp_from_genesis().

TODO:

  • Propose a method of selecting sampling slots
  • Implement that method in Bank::update_epoch_timestamps

Fixes #9874
Toward #10093 (but requires some additional consideration of inflation timing)

@codecov
Copy link

codecov bot commented Oct 6, 2020

Codecov Report

Merging #12681 into master will increase coverage by 0.0%.
The diff coverage is 97.9%.

@@           Coverage Diff            @@
##           master   #12681    +/-   ##
========================================
  Coverage    81.9%    82.0%            
========================================
  Files         359      361     +2     
  Lines       84158    84260   +102     
========================================
+ Hits        68994    69130   +136     
+ Misses      15164    15130    -34     

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 this pull request may close these issues.

Correct bank::unix_timestamp() drift
1 participant