replay: send duplicate proofs from blockstore to state machine #32962
Conversation
Codecov Report
@@ Coverage Diff @@
## master #32962 +/- ##
=========================================
- Coverage 82.0% 82.0% -0.1%
=========================================
Files 785 784 -1
Lines 210819 212607 +1788
=========================================
+ Hits 173055 174423 +1368
- Misses 37764 38184 +420 |
AshwinSekar
force-pushed
the
duplicate-shred-proof-replay
branch
2 times, most recently
from
d0d056b to
c3fbcd0
Compare
| let duplicate_slots = blockstore | ||
| .duplicate_slots_iterator(bank_forks.root_bank().slot()) | ||
| .unwrap(); | ||
| let duplicate_slot_hashes = duplicate_slots | ||
| .filter_map(|slot| bank_forks.bank_hash(slot).map(|hash| (slot, hash))); |
There was a problem hiding this comment.
This can be moved outside of this block scope using just the returned root_bank, since it doesn't rely on holding the bank_forks lock
There was a problem hiding this comment.
You still need the bank forks lock to filter get the hashes for the duplicate slots:
.filter_map(|slot| bank_forks.bank_hash(slot).map(|hash| (slot, hash)));
There was a problem hiding this comment.
ah yeah, I'm dumb, ignore this
|
|
||
| if blockstore.is_full(child) { | ||
| let shreds = blockstore | ||
| .get_data_shreds_for_slot(child, 0) | ||
| .expect("Child is full"); | ||
| let mut our_block = true; | ||
| for shred in shreds { | ||
| our_block &= shred.verify(&target_pubkey); | ||
| } | ||
| if our_block { | ||
| done = true; | ||
| } | ||
| checked_children.insert(child); |
There was a problem hiding this comment.
We still might build a block off the duplicate block instead of the parent in the new proposed change to deal with the votes not landing right?
Would it be better to check that our next vote is not on the duplicate fork heaviest (have to guarantee that the duplicate fork is the heaviest though, i.e. votes have landed on it)?
There was a problem hiding this comment.
Even with the new proposed change, if you have an invalid fork with no alternative valid fork you will always create a new fork, which is the case here since we are only 1 fork. So although after we create the new fork we might go back to building off the duplicate fork, since we check all children this will still work.
Would it be better to check that our next vote is not on the duplicate fork heaviest (have to guarantee that the duplicate fork is the heaviest though, i.e. votes have landed on it)?
This could be possible although it would require a really complicated setup, because as is the validator would not be able to ever switch onto the non duplicate fork, so we would need a switching threshold stake to both be not on the duplicate fork but also not have created their own fork already.
I'm don't think this setup would test the intended code path in a more rigorous way, and is probably better for a duplicate switch rollback test.
AshwinSekar
force-pushed
the
duplicate-shred-proof-replay
branch
from
c3fbcd0 to
a06d82a
Compare
AshwinSekar
force-pushed
the
duplicate-shred-proof-replay
branch
from
a06d82a to
33a3711
Compare
3 participants
Problem
After restart, any slots that were previously marked duplicate will not be marked invalid in fork choice.
Summary of Changes
When processing the result of replay (dead or frozen), also check if blockstore contains a duplicate proof. If it does, let the state machine know.
Split from #32862