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

Fix timestamp overflow #7886

Merged
merged 4 commits into from
Jan 21, 2020

Conversation

CriesofCarrots
Copy link
Contributor

Problem

On a testnet with very high stakes, solana get-block-time returns unexpected, unrealistic value (see #7781 ).
Stake-weighted blocktime calculation accumulators currently use u64 integer types; stake quantities are represented in lamports, and also use the u64 integer type. Thus, it is easy to make the stake-weighted timestamp calculation overflow when multiplying a moderately high stake number with a UnixTimestamp (i64).

(This bug produces a skewed timestamp on testnet, instead of panicking the node as it does in the unit test; TIL Rust only checks for integer overflow in debug builds.)

Summary of Changes

  • Use u128 integers to handle stake-weighting timestamps
  • Also sneak in fix to Vote error occurring frequently in slot 1

Fixes #7781

@codecov
Copy link

codecov bot commented Jan 20, 2020

Codecov Report

Merging #7886 into master will increase coverage by <.1%.
The diff coverage is 97.8%.

@@           Coverage Diff            @@
##           master   #7886     +/-   ##
========================================
+ Coverage    81.8%   81.9%   +<.1%     
========================================
  Files         244     244             
  Lines       51902   51983     +81     
========================================
+ Hits        42506   42585     +79     
- Misses       9396    9398      +2

@CriesofCarrots CriesofCarrots merged commit 21d5fe6 into solana-labs:master Jan 21, 2020
@ryoqun
Copy link
Member

ryoqun commented Jan 21, 2020

@CriesofCarrots Thanks for fixing this!!

@CriesofCarrots CriesofCarrots deleted the timestamp-overflow branch February 21, 2020 22:13
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.

Block time calculation gets warped with high stakes
3 participants