Skip to content

Commit

Permalink
Fix validator set bug (MystenLabs#3594)
Browse files Browse the repository at this point in the history
  • Loading branch information
lxfind authored Jul 29, 2022
1 parent 2d9f99a commit bf990b6
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 16 deletions.

Large diffs are not rendered by default.

12 changes: 6 additions & 6 deletions crates/sui-framework/sources/governance/sui_system.move
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ module sui::sui_system {
ctx: &mut TxContext,
) {
assert!(
validator_set::total_validator_candidate_count(&self.validators) < self.parameters.max_validator_candidate_count,
validator_set::next_epoch_validator_count(&self.validators) < self.parameters.max_validator_candidate_count,
0
);
let stake_amount = coin::value(&stake);
Expand Down Expand Up @@ -226,7 +226,7 @@ module sui::sui_system {
delegation::undelegate(delegation, self.epoch, ctx)
}

// Switch delegation from the current validator to a new one.
// Switch delegation from the current validator to a new one.
public entry fun request_switch_delegation(
self: &mut SuiSystemState,
delegation: &mut Delegation,
Expand Down Expand Up @@ -316,17 +316,17 @@ module sui::sui_system {
self.epoch
}

/// Returns the amount of stake delegated to `validator_addr`.
/// Returns the amount of stake delegated to `validator_addr`.
/// Aborts if `validator_addr` is not an active validator.
public fun validator_delegate_amount(self: &SuiSystemState, validator_addr: address): u64 {
validator_set::validator_delegate_amount(&self.validators, validator_addr)
}
}

/// Returns the amount of delegators who have delegated to `validator_addr`.
/// Returns the amount of delegators who have delegated to `validator_addr`.
/// Aborts if `validator_addr` is not an active validator.
public fun validator_delegator_count(self: &SuiSystemState, validator_addr: address): u64 {
validator_set::validator_delegator_count(&self.validators, validator_addr)
}
}

#[test_only]
public fun set_epoch_for_testing(self: &mut SuiSystemState, epoch_num: u64) {
Expand Down
17 changes: 12 additions & 5 deletions crates/sui-framework/sources/governance/validator_set.move
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,9 @@ module sui::validator_set {
validators
}

/// Get the total number of candidates that might become validators in the next epoch.
public(friend) fun total_validator_candidate_count(self: &ValidatorSet): u64 {
vector::length(&self.active_validators)
+ vector::length(&self.pending_validators)
- vector::length(&self.pending_removals)
/// Get the total number of validators in the next epoch.
public(friend) fun next_epoch_validator_count(self: &ValidatorSet): u64 {
vector::length(&self.next_epoch_validators)
}

/// Called by `SuiSystem`, add a new validator to `pending_validators`, which will be
Expand Down Expand Up @@ -448,6 +446,15 @@ module sui::validator_set {
vector::push_back(&mut result, *metadata);
active_count = active_count - 1;
};
let i = 0;
let pending_count = vector::length(&self.pending_validators);
while (i < pending_count) {
let metadata = validator::metadata(
vector::borrow(&self.pending_validators, i),
);
vector::push_back(&mut result, *metadata);
i = i + 1;
};
result
}

Expand Down
8 changes: 4 additions & 4 deletions crates/sui-framework/tests/validator_set_tests.move
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ module sui::validator_set_tests {

// Create a validator set with only the first validator in it.
let validator_set = validator_set::new(vector[validator1]);
assert!(validator_set::total_validator_candidate_count(&validator_set) == 1, 0);
assert!(validator_set::next_epoch_validator_count(&validator_set) == 1, 0);
assert!(validator_set::total_validator_stake(&validator_set) == 100, 0);

// Add the other 3 validators one by one.
Expand All @@ -35,7 +35,7 @@ module sui::validator_set_tests {
validator2,
);
// Adding validator during the epoch should not affect stake and quorum threshold.
assert!(validator_set::total_validator_candidate_count(&validator_set) == 2, 0);
assert!(validator_set::next_epoch_validator_count(&validator_set) == 2, 0);
assert!(validator_set::total_validator_stake(&validator_set) == 100, 0);

validator_set::request_add_validator(
Expand Down Expand Up @@ -83,15 +83,15 @@ module sui::validator_set_tests {
let ctx1 = test_scenario::ctx(&mut scenario);
validator_set::advance_epoch(&mut validator_set, &mut reward, ctx1);
// The total stake and quorum should reflect 4 validators.
assert!(validator_set::total_validator_candidate_count(&validator_set) == 4, 0);
assert!(validator_set::next_epoch_validator_count(&validator_set) == 4, 0);
assert!(validator_set::total_validator_stake(&validator_set) == 1000, 0);

validator_set::request_remove_validator(
&mut validator_set,
ctx1,
);
// Total validator candidate count changes, but total stake remains during epoch.
assert!(validator_set::total_validator_candidate_count(&validator_set) == 3, 0);
assert!(validator_set::next_epoch_validator_count(&validator_set) == 3, 0);
assert!(validator_set::total_validator_stake(&validator_set) == 1000, 0);
validator_set::advance_epoch(&mut validator_set, &mut reward, ctx1);
// Validator1 is gone.
Expand Down

0 comments on commit bf990b6

Please sign in to comment.