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

Fix epoched last update #2667

Merged
merged 3 commits into from
Apr 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions .changelog/unreleased/bug-fixes/2667-fix-last-update-usage.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Fix the setting of the last update field in an Epoched data structure.
([\#2667](https://github.com/anoma/namada/pull/2667))
8 changes: 3 additions & 5 deletions crates/proof_of_stake/src/epoched.rs
Original file line number Diff line number Diff line change
Expand Up @@ -444,13 +444,11 @@ where
// panic!("WARNING: no data existing in
// {new_oldest_epoch}"); }
self.set_oldest_epoch(storage, new_oldest_epoch)?;

// Update the epoch of the last update to the current epoch
let key = self.get_last_update_storage_key();
storage.write(&key, current_epoch)?;
return Ok(());
}
}
// Update the epoch of the last update to the current epoch
let key = self.get_last_update_storage_key();
storage.write(&key, current_epoch)?;

Ok(())
}
Expand Down
25 changes: 16 additions & 9 deletions crates/proof_of_stake/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2205,7 +2205,8 @@
// Promote the next below-capacity validator to consensus
promote_next_below_capacity_validator_to_consensus(
storage,
pipeline_epoch,
current_epoch,
params.pipeline_len,

Check warning on line 2209 in crates/proof_of_stake/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

crates/proof_of_stake/src/lib.rs#L2208-L2209

Added lines #L2208 - L2209 were not covered by tests
)?;
}

Expand Down Expand Up @@ -2290,8 +2291,8 @@
validator_state_handle(validator).set(
storage,
ValidatorState::Jailed,
pipeline_epoch,
0,
current_epoch,
params.pipeline_len,

Check warning on line 2295 in crates/proof_of_stake/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

crates/proof_of_stake/src/lib.rs#L2294-L2295

Added lines #L2294 - L2295 were not covered by tests
)?;
return Ok(());
}
Expand Down Expand Up @@ -2698,10 +2699,14 @@

// Remove the validator from the set starting at the update epoch and up
// thru the pipeline epoch.
let pipeline_epoch = current_epoch + params.pipeline_len;
for epoch in
Epoch::iter_bounds_inclusive(validator_set_update_epoch, pipeline_epoch)
{
let start = validator_set_update_epoch
.0
.checked_sub(current_epoch.0)
Comment on lines +2702 to +2704
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
let start = validator_set_update_epoch
.0
.checked_sub(current_epoch.0)
let start = validator_set_update_epoch
.checked_sub(current_epoch)

.unwrap(); // Safe unwrap
let end = params.pipeline_len;

for offset in start..=end {
let epoch = current_epoch + offset;
let prev_state = validator_state_handle(validator)
.get(storage, epoch, params)?
.expect("Expected to find a valid validator.");
Expand All @@ -2716,9 +2721,11 @@
// For the pipeline epoch only:
// promote the next max inactive validator to the active
// validator set at the pipeline offset
if epoch == pipeline_epoch {
if offset == params.pipeline_len {
promote_next_below_capacity_validator_to_consensus(
storage, epoch,
storage,
current_epoch,
offset,
)?;
}
}
Expand Down
26 changes: 25 additions & 1 deletion crates/proof_of_stake/src/tests/test_validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1320,8 +1320,20 @@ fn test_purge_validator_information_aux(validators: Vec<GenesisValidator>) {

// Check that there is validator data for epochs 0 - pipeline_len
check_is_data(&s, current_epoch, Epoch(params.owned.pipeline_len));
assert_eq!(
consensus_val_set.get_last_update(&s).unwrap().unwrap(),
Epoch(0)
);
assert_eq!(
validator_positions.get_last_update(&s).unwrap().unwrap(),
Epoch(0)
);
assert_eq!(
validator_positions.get_last_update(&s).unwrap().unwrap(),
Epoch(0)
);

// Advance to epoch 1
// Advance to epoch `default_past_epochs`
for _ in 0..default_past_epochs {
current_epoch = advance_epoch(&mut s, &params);
}
Expand All @@ -1333,6 +1345,18 @@ fn test_purge_validator_information_aux(validators: Vec<GenesisValidator>) {
Epoch(0),
Epoch(params.owned.pipeline_len + default_past_epochs),
);
assert_eq!(
consensus_val_set.get_last_update(&s).unwrap().unwrap(),
Epoch(default_past_epochs)
);
assert_eq!(
validator_positions.get_last_update(&s).unwrap().unwrap(),
Epoch(default_past_epochs)
);
assert_eq!(
validator_positions.get_last_update(&s).unwrap().unwrap(),
Epoch(default_past_epochs)
);

current_epoch = advance_epoch(&mut s, &params);
assert_eq!(current_epoch.0, default_past_epochs + 1);
Expand Down
10 changes: 6 additions & 4 deletions crates/proof_of_stake/src/validator_set_update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -521,11 +521,13 @@ where
/// consensus set already.
pub fn promote_next_below_capacity_validator_to_consensus<S>(
storage: &mut S,
epoch: Epoch,
current_epoch: Epoch,
offset: u64,
) -> namada_storage::Result<()>
where
S: StorageRead + StorageWrite,
{
let epoch = current_epoch + offset;
let below_cap_set = below_capacity_validator_set_handle().at(&epoch);
let max_below_capacity_amount =
get_max_below_capacity_validator_amount(&below_cap_set, storage)?;
Expand All @@ -550,8 +552,8 @@ where
validator_state_handle(&promoted_validator).set(
storage,
ValidatorState::Consensus,
epoch,
0,
current_epoch,
offset,
)?;
}

Expand Down Expand Up @@ -828,6 +830,7 @@ where
.at(&val_stake)
.insert(storage, val_position, val_address)?;
}

// Purge consensus and below-capacity validator sets
consensus_validator_set.update_data(storage, params, current_epoch)?;
below_capacity_validator_set.update_data(storage, params, current_epoch)?;
Expand All @@ -847,7 +850,6 @@ where
let prev = new_positions_handle.insert(storage, validator, position)?;
debug_assert!(prev.is_none());
}
validator_set_positions_handle.set_last_update(storage, current_epoch)?;

// Purge old epochs of validator positions
validator_set_positions_handle.update_data(
Expand Down
Loading