fix clock bankhash mismatch #10078
fix clock bankhash mismatch #10078
Conversation
|
@ryoqun fyi |
Codecov Report
@@ Coverage Diff @@
## master #10078 +/- ##
=========================================
- Coverage 81.5% 81.4% -0.2%
=========================================
Files 283 283
Lines 65797 65827 +30
=========================================
- Hits 53659 53611 -48
- Misses 12138 12216 +78 |
|
Cool!!! So, we can just avoid a rather costly transition entirely. :) Could you manually test this actually fixes vote mismatch errors according to these steps?: #9992 (comment) |
|
And extra bonus is that this pr could contain a bank_hash deterministic test like this if you have extra bandwidth :) : test_bank_hash() {
genesis_bank = ... # some deterministic way with warm-up epoch
next_bank = Bank::new_from_parent(genesis_bank)
assert_eq!(genesis_bank.hash(), "81KbyqL4FmPEwLxdg4...........")
# fast forward to next epoch
while/loop/whatever {
bank = new_from_parent(bank)
}
....
assert_eq!(bank_in_next_epoch.bank_hash(), "4Q8t8qk4qqAjiqs..........");
}Which is actually on my due. ;) |
|
Yup, I did verify it via those steps, thanks for the good writeup. I'll look into a representative test |
| @@ -7005,4 +7010,46 @@ mod tests { | |||
| } | |||
| info!("results: {:?}", results); | |||
| } | |||
|
|
|||
| #[test] | |||
There was a problem hiding this comment.
@ryoqun how about this test? I ran it across different scenarios (with the owner in the hash, without, with my archiver's removed w and w/o this PR) and it looks like it accomplishes what we need. Let me know what you think.
There was a problem hiding this comment.
perfect! That's what I'm looking for. :)
runtime/src/bank.rs
Outdated
| "4kUw6sn94XLCjh5dBhtJec5W6qbzSuTHfzvXKbXtJtc7" | ||
| ); | ||
| } | ||
| if bank.slot == 4 { |
There was a problem hiding this comment.
nits: why we stop at slot 4? I mean, why not 3 or 5? Is there specific reason? Some slots just to be safe? one line comment or const SOME_SLOT = 4; would be useful for future maintenance.
There was a problem hiding this comment.
Just check some slots, I modified it to cross a epoch boundary
ryoqun
added
automerge
Problem
Deprecating segment from the clock sysval broke bankhash
Summary of Changes
Continue to calculate the correct values
follow-up to: #9992