From 042e60f1d7b76ff02a5aa7d9709c246504587694 Mon Sep 17 00:00:00 2001
From: jasonz-dfinity <133917836+jasonz-dfinity@users.noreply.github.com>
Date: Fri, 31 Jan 2025 13:42:43 -0800
Subject: [PATCH] Remove everything related to
 accounts_db_stats_recomputed_on_upgrade (#6301)

# Motivation

`accounts_db_stats_recomputed_on_upgrade` is no longer useful after the
migration of accounts to stable structures. It is always `opt false`,
which can be verified by `dfx canister --ic call
qoctq-giaaa-aaaaa-aaaea-cai get_stats | grep
accounts_db_stats_recomputed_on_upgrade`

# Changes

Remove everything related to `accounts_db_stats_recomputed_on_upgrade`

# Tests

N/A

# Related PRs

[Previous](https://github.com/dfinity/nns-dapp/pull/6298) |
[Next](https://github.com/dfinity/nns-dapp/pull/6302)
---
 rs/backend/nns-dapp.did                  |  1 -
 rs/backend/src/accounts_store.rs         | 20 --------------------
 rs/backend/src/stats.rs                  |  2 --
 scripts/nns-dapp/migration-test.canister | 19 -------------------
 4 files changed, 42 deletions(-)

diff --git a/rs/backend/nns-dapp.did b/rs/backend/nns-dapp.did
index 8f343ba54ca..a8fcf09616f 100644
--- a/rs/backend/nns-dapp.did
+++ b/rs/backend/nns-dapp.did
@@ -140,7 +140,6 @@ type Stats =
         stable_memory_size_bytes: opt nat64;
         wasm_memory_size_bytes: opt nat64;
         migration_countdown: opt nat32;
-        accounts_db_stats_recomputed_on_upgrade: opt bool;
     };
 
 type PerformanceCount =
diff --git a/rs/backend/src/accounts_store.rs b/rs/backend/src/accounts_store.rs
index 67eedbb104c..82d4fa44a5d 100644
--- a/rs/backend/src/accounts_store.rs
+++ b/rs/backend/src/accounts_store.rs
@@ -47,25 +47,8 @@ pub struct AccountsStore {
     accounts_db: schema::proxy::AccountsDbAsProxy,
 
     accounts_db_stats: AccountsDbStats,
-    accounts_db_stats_recomputed_on_upgrade: IgnoreEq<Option<bool>>,
 }
 
-/// A wrapper around a value that returns true for `PartialEq` and `Eq` equality checks, regardless of the value.
-///
-/// This is intended to be used on incidental, volatile fields.  A structure containing such a field will typically wish to disregard the field in any comparison.
-#[derive(Default)]
-struct IgnoreEq<T>(T)
-where
-    T: Default;
-
-impl<T: Default> PartialEq for IgnoreEq<T> {
-    fn eq(&self, _: &Self) -> bool {
-        true
-    }
-}
-
-impl<T: Default> Eq for IgnoreEq<T> {}
-
 impl fmt::Debug for AccountsStore {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         write!(
@@ -684,7 +667,6 @@ impl AccountsStore {
         stats.sub_accounts_count = self.accounts_db_stats.sub_accounts_count;
         stats.hardware_wallet_accounts_count = self.accounts_db_stats.hardware_wallet_accounts_count;
         stats.migration_countdown = Some(self.accounts_db.migration_countdown());
-        stats.accounts_db_stats_recomputed_on_upgrade = self.accounts_db_stats_recomputed_on_upgrade.0;
     }
 
     #[must_use]
@@ -792,7 +774,6 @@ impl StableState for AccountsStore {
             Option<AccountsDbStats>,
         ) = Candid::from_bytes(bytes).map(|c| c.0)?;
 
-        let accounts_db_stats_recomputed_on_upgrade = IgnoreEq(Some(accounts_db_stats_maybe.is_none()));
         let Some(accounts_db_stats) = accounts_db_stats_maybe else {
             return Err("Accounts DB stats should be present since the stable structures migration.".to_string());
         };
@@ -803,7 +784,6 @@ impl StableState for AccountsStore {
             // State::from(Partitions) so it doesn't matter what we set here.
             accounts_db: AccountsDbAsProxy::default(),
             accounts_db_stats,
-            accounts_db_stats_recomputed_on_upgrade,
         })
     }
 }
diff --git a/rs/backend/src/stats.rs b/rs/backend/src/stats.rs
index 0e71c85215b..36e607b580d 100644
--- a/rs/backend/src/stats.rs
+++ b/rs/backend/src/stats.rs
@@ -38,8 +38,6 @@ pub struct Stats {
     pub stable_memory_size_bytes: Option<u64>,
     pub wasm_memory_size_bytes: Option<u64>,
     pub migration_countdown: Option<u32>, // When non-zero, a migration is in progress.
-    /// Whether account stats were recomputed on upgrade.
-    pub accounts_db_stats_recomputed_on_upgrade: Option<bool>,
 }
 
 /// Encodes the metrics into the format scraped by the monitoring system.
diff --git a/scripts/nns-dapp/migration-test.canister b/scripts/nns-dapp/migration-test.canister
index c22d5d72dc9..ae2294274b4 100755
--- a/scripts/nns-dapp/migration-test.canister
+++ b/scripts/nns-dapp/migration-test.canister
@@ -35,20 +35,6 @@ get_upgrade_invariant_stats() {
   dfx canister call nns-dapp get_stats | idl2json | jq '{accounts_count, sub_accounts_count}'
 }
 
-# Verifies that accounts were not recomputed on upgrade.
-# Recomputing on upgrade is very expensive for stable memory.
-assert_accounts_db_stats_are_not_recomputed_on_upgrade() {
-  local expected_value actual_value
-  expected_value="[false]"
-  actual_value="$(dfx canister call nns-dapp get_stats | idl2json | jq -c '.accounts_db_stats_recomputed_on_upgrade')"
-  [[ "$expected_value" == "$actual_value" ]] || {
-    echo "ERROR: Recompute stats on upgrade is not as expected."
-    echo "Expected: $expected_value"
-    echo "Actual:   $actual_value"
-    exit 1
-  }
-}
-
 get_accounts_count() {
   dfx canister call nns-dapp get_stats | idl2json | jq -r .accounts_count
 }
@@ -152,11 +138,6 @@ upgrade_nnsdapp() {
       exit 1
     } >&2
   fi
-
-  # Going forwards, this should always be false.  Historically it was always true.
-  #
-  # TODO: Delete the code path for recomputing stats on upgrade.
-  assert_accounts_db_stats_are_not_recomputed_on_upgrade
 }
 
 # This file is intended to be sourced, but if called directly, offer some help