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.

Sign up for GitHub

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

includes rent_epoch in vote-accounts sanity checks #29861

Merged
merged 1 commit into from
Jan 24, 2023
+3 −12
Merged

includes rent_epoch in vote-accounts sanity checks #29861

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 3 additions & 12 deletions runtime/src/stakes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,18 +235,9 @@ impl Stakes<StakeAccount> {
None => return Err(Error::VoteAccountNotFound(*pubkey)),
Some(account) => account,
};
// Ignoring rent_epoch until the feature for
// preserve_rent_epoch_for_rent_exempt_accounts is activated.
let vote_account = vote_account.account();
if vote_account.lamports() != account.lamports()
|| vote_account.owner() != account.owner()
|| vote_account.executable() != account.executable()
|| vote_account.data() != account.data()
{
error!(
"vote account mismatch: {}, {:?}, {:?}",
pubkey, vote_account, account
);
if vote_account != &account {
error!("vote account mismatch: {pubkey}, {vote_account:?}, {account:?}");
return Err(Error::VoteAccountMismatch(*pubkey));
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this halt the validator? IIRC, using the cache is not enabled yet on mnb?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does this halt the validator?

It does halt the validator at start up.

This code is only invoked at startup when deserializing bank:
https://github.com/solana-labs/solana/blob/0be194145/runtime/src/bank.rs#L1828-L1841
in order to obtain Stakes<StakeAccount> from Stakes<Delegation> for backward compatibility reasons mentioned in the link. As part of that, it performs these sanity checks.

IIRC, using the cache is not enabled yet on mnb?

We are not using stakes-cache for rewards calculations (yet), but stakes-cache is already used so many other places, including epoch-stakes calculations. Pubkeys for stakes accounts used in rewards calculations are also already obtained from stakes-cache.

Copy link
Contributor

Choose a reason for hiding this comment

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

as a side note, I wonder why we aren't building this cache on startup instead of serializing it. We scan all accounts to generate the index. It seems we could recreate this then. Bank deserialization at load does take a while and all of this is duplicate information derivable from append vecs at startup. Then we would have no inconsistency checks and it seems we could remove code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure where to obtain correct stake_history: StakeHistory field in stakes-cache, but, yeah, as for stake/vote accounts, those can be obtained from accounts-db. It would need a full accounts-db scan though.

I think since we already do serialize it, it would be just easier/faster to go with it.

Then we would have no inconsistency checks and it seems we could remove code.

Any inconsistency spotted by sanity checks here is only a symptom that there is a bug somewhere. The actual culprit of that inconsistency is somewhere else.

}
}
Expand All @@ -266,7 +257,7 @@ impl Stakes<StakeAccount> {
if VoteState::is_correct_size_and_initialized(account.data())
&& VoteAccount::try_from(account.clone()).is_ok()
{
error!("vote account not cached: {}, {:?}", pubkey, account);
error!("vote account not cached: {pubkey}, {account:?}");
return Err(Error::VoteAccountNotCached(pubkey));
}
}
Expand Down