Detect vote hash mismatch even at run-sanity.sh

[ryoqun]

Problem

#12670 (comment)

Summary of Changes

note that this test expected to fail.

[codecov]

Codecov Report

Merging #12690 (21d3059) into master (5d72e52) will increase coverage by 0.0%.
The diff coverage is 100.0%.

@@           Coverage Diff           @@
##           master   #12690   +/-   ##
=======================================
  Coverage    82.0%    82.1%           
=======================================
  Files         378      378           
  Lines       90905    90905           
=======================================
+ Hits        74632    74637    +5     
+ Misses      16273    16268    -5     

[sakridge]

Would it better to have a flag to panic on this case?

[mvines]

Would it better to have a flag to panic on this case?

Flag would be cool. Plumbing that flag from solana-validator into the vote state seems really gnarly though, I don't know how we'd do that nicely

[sakridge]

Would it better to have a flag to panic on this case?

Flag would be cool. Plumbing that flag from solana-validator into the vote state seems really gnarly though, I don't know how we'd do that nicely

That's true. Plumbing would be a pain. Also security reviews don't like these kinds of flags to turn off checks.

On the other hand, these kind of log checks are kind of brittle and have a habit of being disabled without anyone noticing. Also seems like more of a pain to enable in more test situations. Generally we would always want to panic in this situation unless it's a specifically malicious test case to insert duplicate conflicting blocks into the system.

[ryoqun]

Would it better to have a flag to panic on this case?

Flag would be cool. Plumbing that flag from solana-validator into the vote state seems really gnarly though, I don't know how we'd do that nicely

That's true. Plumbing would be a pain. Also security reviews don't like these kinds of flags to turn off checks.

On the other hand, these kind of log checks are kind of brittle and have a habit of being disabled without anyone noticing. Also seems like more of a pain to enable in more test situations. Generally we would always want to panic in this situation unless it's a specifically malicious test case to insert duplicate conflicting blocks into the system.

Yeah, I don't like the current state of this. After some thought, I think I'll just add a validator kill-switch if the validator's voted slot hash is mismatched to the supermajority rest of cluster.

To begin with, as bank hashes are chained, there is no point to continue running validator once mismatched is started.

This will be like combination of #11927 and #11727, which is done at the snapshot interval.

Also, this will make #12697 like this more visible when testing/benchmarking.

[ryoqun]

@mvines @sakridge (no rush) please take a look at the more proper solution for this in my previous comment.

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[stale]

This stale pull request has been automatically closed. Thank you for your contributions.

[sakridge]

@ryoqun is there something wrong with this? Can we prioritize enabling this checking?

[ryoqun]

@sakridge There is nothing wrong with this. it's just that I couldn't get my hands on it. Currently, I'm focusing on various inflation and staking bugs. I talked about this a bit with @carllin, we can implement this with VoteTracker.

[sakridge]

@sakridge There is nothing wrong with this. it's just that I couldn't get my hands on it. Currently, I'm focusing on various inflation and staking bugs. I talked about this a bit with @carllin, we can implement this with VoteTracker.

Is the builtins part of this change needed for the vote hash mismatch?

[ryoqun]

@sakridge There is nothing wrong with this. it's just that I couldn't get my hands on it. Currently, I'm focusing on various inflation and staking bugs. I talked about this a bit with @carllin, we can implement this with VoteTracker.

Is the builtins part of this change needed for the vote hash mismatch?

@sakridge oops, sorry. these was rebase leftover; fixed.

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[stale]

This stale pull request has been automatically closed. Thank you for your contributions.

[stale]

This stale pull request has been automatically closed. Thank you for your contributions.