Skip to content

Commit

Permalink
refactor(sns): Reset cached upgrade steps upon detecting inconsistenc…
Browse files Browse the repository at this point in the history
…ies (#2802)

This PR refactors the SNS Governance to reset cached upgrade steps
earlier, as soon as an inconsistency or missing data is detected. This
simplifies the implementation of a few periodic functions since they can
rely on the data being available.

The behavior is slightly affected, as the first upgrade journal entry
for a newly deployed SNS is expected to be a cache _reset_ rather than a
cache refresh.

---------

Co-authored-by: Andre Popovitch <andre@popovit.ch>
  • Loading branch information
aterga and anchpop authored Nov 29, 2024
1 parent 3a123cb commit 3025f56
Show file tree
Hide file tree
Showing 11 changed files with 854 additions and 375 deletions.
19 changes: 14 additions & 5 deletions rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1343,6 +1343,7 @@ pub mod sns {

pub mod governance {
use super::*;
use assert_matches::assert_matches;
use ic_crypto_sha2::Sha256;
use ic_nervous_system_agent::sns::governance::GovernanceCanister;
use ic_sns_governance::pb::v1::get_neuron_response;
Expand Down Expand Up @@ -1719,13 +1720,21 @@ pub mod sns {
sns_governance_canister_id: PrincipalId,
expected_entries: &[sns_pb::upgrade_journal_entry::Event],
) {
let sns_pb::GetUpgradeJournalResponse {
upgrade_journal, ..
} = sns::governance::get_upgrade_journal(pocket_ic, sns_governance_canister_id).await;
let response =
sns::governance::get_upgrade_journal(pocket_ic, sns_governance_canister_id).await;

let upgrade_journal = upgrade_journal.unwrap().entries;
let journal_entries = assert_matches!(
response,
sns_pb::GetUpgradeJournalResponse {
upgrade_journal: Some(sns_pb::UpgradeJournal {
entries,
..
}),
..
} => entries
);

for (index, either_or_both) in upgrade_journal
for (index, either_or_both) in journal_entries
.iter()
.zip_longest(expected_entries.iter())
.enumerate()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use ic_nervous_system_integration_tests::{
},
};
use ic_nns_test_utils::sns_wasm::create_modified_sns_wasm;
use ic_sns_governance::pb::v1::upgrade_journal_entry::UpgradeStepsReset;
use ic_sns_governance::{
governance::UPGRADE_STEPS_INTERVAL_REFRESH_BACKOFF_SECONDS,
pb::v1 as sns_pb,
Expand Down Expand Up @@ -67,9 +68,10 @@ async fn test_get_upgrade_journal() {
// Step 1: Check that the upgrade journal contains the initial version right after SNS creation.
let mut expected_upgrade_journal_entries = vec![];
{
expected_upgrade_journal_entries.push(Event::UpgradeStepsRefreshed(
UpgradeStepsRefreshed::new(vec![initial_sns_version.clone()]),
));
expected_upgrade_journal_entries.push(Event::UpgradeStepsReset(UpgradeStepsReset::new(
"this message will be redacted to keep the test spec more abstract".to_string(),
vec![initial_sns_version.clone()],
)));

sns::governance::assert_upgrade_journal(
&pocket_ic,
Expand Down Expand Up @@ -228,7 +230,9 @@ async fn test_get_upgrade_journal() {

expected_upgrade_journal_entries.push(
sns_pb::upgrade_journal_entry::Event::UpgradeOutcome(
sns_pb::upgrade_journal_entry::UpgradeOutcome::success("redacted".to_string()),
sns_pb::upgrade_journal_entry::UpgradeOutcome::success(
"this message will be redacted to keep the test spec more abstract".to_string(),
),
),
);

Expand All @@ -243,7 +247,9 @@ async fn test_get_upgrade_journal() {

expected_upgrade_journal_entries.push(
sns_pb::upgrade_journal_entry::Event::UpgradeOutcome(
sns_pb::upgrade_journal_entry::UpgradeOutcome::success("redacted".to_string()),
sns_pb::upgrade_journal_entry::UpgradeOutcome::success(
"this message will be redacted to keep the test spec more abstract".to_string(),
),
),
);

Expand Down
8 changes: 7 additions & 1 deletion rs/sns/governance/canister/canister.rs
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,13 @@ fn advance_target_version(request: AdvanceTargetVersionRequest) -> AdvanceTarget
async fn refresh_cached_upgrade_steps(
_: RefreshCachedUpgradeStepsRequest,
) -> RefreshCachedUpgradeStepsResponse {
governance_mut().refresh_cached_upgrade_steps().await;
let goverance = governance_mut();
let deployed_version = goverance
.try_temporarily_lock_refresh_cached_upgrade_steps()
.unwrap();
goverance
.refresh_cached_upgrade_steps(deployed_version)
.await;
RefreshCachedUpgradeStepsResponse {}
}

Expand Down
Loading

0 comments on commit 3025f56

Please sign in to comment.