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

wen_restart: Fix the epoch_stakes used in calculation. #2376

Merged
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
777f739
wen_restart: Fix the epoch_stakes used in calculation.
wen-coding Jul 31, 2024
6fdf31e
Merge branch 'master' into wen_restart_use_appropriate_epoch_stakes
wen-coding Jul 31, 2024
a5cc18e
Fix a bad merge.
wen-coding Jul 31, 2024
2bef89b
Remove EpochStakesCache, it only caches epoch stakes from root_bank, …
wen-coding Aug 2, 2024
4e85977
Split aggregate into smaller functions.
wen-coding Aug 2, 2024
20614e9
Merge branch 'master' into wen_restart_use_appropriate_epoch_stakes
wen-coding Aug 2, 2024
77df63b
Merge branch 'master' into wen_restart_use_appropriate_epoch_stakes
wen-coding Aug 2, 2024
dfcc6cc
Merge branch 'master' into wen_restart_use_appropriate_epoch_stakes
wen-coding Aug 3, 2024
2a30886
Switch to node_id_to_stake which is simpler.
wen-coding Aug 3, 2024
ec895a4
Rename update_slots_stake_map and switch to epoch_total_stake().
wen-coding Aug 6, 2024
fffffb7
Merge branch 'master' into wen_restart_use_appropriate_epoch_stakes
wen-coding Aug 6, 2024
0c0b574
Remove unnecessary utility functions.
wen-coding Aug 6, 2024
78598f4
Do not modify epoch_info_vec, just init it with two epochs we will co…
wen-coding Aug 7, 2024
0cef6f2
Merge branch 'master' into wen_restart_use_appropriate_epoch_stakes
wen-coding Aug 7, 2024
a601722
Merge branch 'master' into wen_restart_use_appropriate_epoch_stakes
wen-coding Aug 7, 2024
1f74e80
Switch to epoch_node_id_to_stake()
wen-coding Aug 7, 2024
ac97e1e
Add test for divergence at Epoch boundary.
wen-coding Aug 9, 2024
67848af
Merge branch 'master' into wen_restart_use_appropriate_epoch_stakes
wen-coding Aug 9, 2024
4958a24
Make linter happy.
wen-coding Aug 9, 2024
a236422
Merge branch 'master' into wen_restart_use_appropriate_epoch_stakes
wen-coding Aug 9, 2024
5df389e
- wait for the new Epoch if > 1/3 of the validators voted for some
wen-coding Aug 13, 2024
198237c
Merge branch 'master' into wen_restart_use_appropriate_epoch_stakes
wen-coding Aug 13, 2024
13a6b25
Fix a bad merge.
wen-coding Aug 13, 2024
bafbced
Merge branch 'master' into wen_restart_use_appropriate_epoch_stakes
wen-coding Aug 16, 2024
1c96e52
Fix a bad merge.
wen-coding Aug 16, 2024
1202248
Merge branch 'master' into wen_restart_use_appropriate_epoch_stakes
wen-coding Aug 19, 2024
0b4857a
Change constant format.
wen-coding Aug 22, 2024
944a357
Do not loop through the whole table.
wen-coding Aug 23, 2024
d733f4a
Address reviewer feedback.
wen-coding Aug 23, 2024
9242a56
Address reviewer comments.
wen-coding Aug 23, 2024
6c0cd01
Merge branch 'master' into wen_restart_use_appropriate_epoch_stakes
wen-coding Aug 23, 2024
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
9 changes: 8 additions & 1 deletion wen-restart/proto/wen_restart.proto
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,16 @@ message LastVotedForkSlotsAggregateRecord {
optional LastVotedForkSlotsAggregateFinal final_result = 2;
}

message LastVotedForkSlotsEpochInfoRecord {
uint64 epoch = 1;
uint64 total_active_stake = 2;
uint64 total_stake = 3;
bool considered_during_exit = 4;
Copy link

Choose a reason for hiding this comment

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

hmm do we need this extra flag, can we just ignore LastVotedForkSlots that are not in the current or next epoch?

Copy link
Author

Choose a reason for hiding this comment

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

We need this extra flag because root_epoch + 1 is always in epoch_info_vec now, but we don't always consider root_epoch + 1 for exit criteria. For example, we won't consider it at all if no one has ever voted in the next epoch.

We then need this extra flag in the proto file because the proto file is a snapshot, we can restart from this snapshot. Then we need this calculated correctly.

Copy link

Choose a reason for hiding this comment

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

For logging purposes and cleaner implementation instead of the considered_during_exit flag, we could track for epoch N and N+1 two fields, voted_percent and voted_for_this_epoch_percent

voted_percent is the % stake of the epoch that has voted for either epoch
voted_for_this_epoch_percent is the % stake of this epoch that has voted for a slot in this epoch.

This might be useful for debugging as well if we log this information.

We then need this extra flag in the proto file because the proto file is a snapshot, we can restart from this snapshot. Then we need this calculated correctly.

This seems extractable from the bank on restart

Copy link
Author

Choose a reason for hiding this comment

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

Changed to voted_percent and voted_for_this_epoch_percent.

}

message LastVotedForkSlotsAggregateFinal {
map<uint64, uint64> slots_stake_map = 1;
uint64 total_active_stake = 2;
repeated LastVotedForkSlotsEpochInfoRecord epoch_infos = 2;
}

message HeaviestForkRecord {
Expand Down
4 changes: 2 additions & 2 deletions wen-restart/src/heaviest_fork_aggregate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ pub(crate) struct HeaviestForkAggregate {
supermajority_threshold: f64,
my_shred_version: u16,
my_pubkey: Pubkey,
// TODO(wen): using local root's EpochStakes, need to fix if crossing Epoch boundary.
// We use the epoch_stakes of the Epoch our heaviest bank is in. Proceed and exit only if
// enough validator agree with me.
epoch_stakes: EpochStakes,
heaviest_forks: HashMap<Pubkey, RestartHeaviestFork>,
block_stake_map: HashMap<(Slot, Hash), u64>,
Expand Down Expand Up @@ -171,7 +172,6 @@ impl HeaviestForkAggregate {
Some(record)
}

// TODO(wen): use better epoch stake and add a test later.
pub(crate) fn total_active_stake(&self) -> u64 {
self.active_peers.iter().fold(0, |sum: u64, pubkey| {
sum.saturating_add(self.epoch_stakes.node_id_to_stake(pubkey).unwrap_or(0))
Expand Down
Loading