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

Reduce cap by rent's leftover as temporary measure #12111

Merged
merged 2 commits into from
Sep 8, 2020

Conversation

ryoqun
Copy link
Member

@ryoqun ryoqun commented Sep 8, 2020

Problem

testnet's cap is busted now.

Summary of Changes

This is temporary fix, which can go without gating.

The underlying bug is now identified and the bug even resides in mainnet-beta/v1.2......

Context

found via capitalization work and blocking #11927

@codecov
Copy link

codecov bot commented Sep 8, 2020

Codecov Report

Merging #12111 into master will increase coverage by 0.0%.
The diff coverage is 100.0%.

@@           Coverage Diff           @@
##           master   #12111   +/-   ##
=======================================
  Coverage    82.0%    82.0%           
=======================================
  Files         337      337           
  Lines       79220    79223    +3     
=======================================
+ Hits        65012    65025   +13     
+ Misses      14208    14198   -10     

@ryoqun
Copy link
Member Author

ryoqun commented Sep 8, 2020

  • reset capitalization on snapshot load at least on testnet to fix busted cap in snapshots....

mvines
mvines previously approved these changes Sep 8, 2020
Copy link
Member

@mvines mvines left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

self.distribute_rent_to_validators(&self.vote_accounts(), rent_to_be_distributed);
let leftover =
self.distribute_rent_to_validators(&self.vote_accounts(), rent_to_be_distributed);
self.capitalization.fetch_sub(leftover, Ordering::Relaxed);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to confirm, this is ok because capitalization is not a part of the bank hash right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to confirm, this is ok because capitalization is not a part of the bank hash right?

Exactly. :)

@ryoqun ryoqun force-pushed the rent-leftover-cap-reduction branch from f6a123f to b00f1a5 Compare September 8, 2020 17:18
@mergify mergify bot dismissed mvines’s stale review September 8, 2020 17:19

Pull request has been modified.

@ryoqun
Copy link
Member Author

ryoqun commented Sep 8, 2020

* [x]  reset capitalization on snapshot load at least on testnet to fix busted cap in snapshots....

@mvines FYI, this is done. b00f1a5

@ryoqun ryoqun removed the v1.2 label Sep 8, 2020
@ryoqun ryoqun merged commit 5b2442d into solana-labs:master Sep 8, 2020
mergify bot pushed a commit that referenced this pull request Sep 8, 2020
* Reduce cap by rent's leftover as temporary measure

* Reset testnet cap. on start and more logs

(cherry picked from commit 5b2442d)
mergify bot added a commit that referenced this pull request Sep 8, 2020
* Reduce cap by rent's leftover as temporary measure

* Reset testnet cap. on start and more logs

(cherry picked from commit 5b2442d)

Co-authored-by: Ryo Onodera <[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 this pull request may close these issues.

2 participants