NNS1-2905: Remove transactions fields from accounts (step 1)

CHANGELOG-Nns-Dapp-unreleased.md

@@ -29,6 +29,7 @@ proposal is successful, the changes it released will be moved from this file to
 * Stop encoding the accounts map in the `AccountsStore`.
 * Removed `ENABLE_HIDE_ZERO_BALANCE` feature flag.
 * Proposal filtering by reward status.
+* Intermediate step to remove transactions from accounts stored in nns-dapp.
 
 #### Fixed
 

rs/backend/src/accounts_store.rs

@@ -153,36 +153,111 @@ pub struct Account {
     /// Note: The principal was not stored for early users.  When early users log in, we discover their principal and set this field.
     principal: Option<PrincipalId>,
     account_identifier: AccountIdentifier,
-    default_account_transactions: Vec<TransactionIndex>,
     sub_accounts: HashMap<u8, NamedSubAccount>,
     hardware_wallet_accounts: Vec<NamedHardwareWalletAccount>,
     canisters: Vec<NamedCanister>,
+    // default_account_transactions: Do not reuse this field. There are still accounts in stable memor with this unused field.
 }
 
 impl Storable for Account {
     const BOUND: Bound = Bound::Unbounded;
     fn to_bytes(&self) -> Cow<'_, [u8]> {
-        candid::encode_one(self).expect("Failed to serialize account").into()
+        // Encode Account in a way that can still be restored if we need to
+        // roll back the release.
+        let old_account = OldAccount::from(self.clone());
+        candid::encode_one(old_account)
+            .expect("Failed to serialize account")
+            .into()
+        // TODO(NNS1-2905): Restore direct encoding once OldAccount is deleted.
+        // candid::encode_one(self).expect("Failed to serialize account").into()
     }
     fn from_bytes(bytes: Cow<'_, [u8]>) -> Self {
         candid::decode_one(&bytes).expect("Failed to parse account from store.")
     }
 }
 
+// TODO(NNS1-2905): Delete this after it has been on mainnet for at least 1 release.
+#[derive(CandidType, Deserialize, Debug, Eq, PartialEq, Clone)]
+struct OldAccount {
+    principal: Option<PrincipalId>,
+    account_identifier: AccountIdentifier,
+    default_account_transactions: Vec<TransactionIndex>,
+    sub_accounts: HashMap<u8, OldNamedSubAccount>,
+    hardware_wallet_accounts: Vec<OldNamedHardwareWalletAccount>,
+    canisters: Vec<NamedCanister>,
+}
+
+impl From<Account> for OldAccount {
+    fn from(account: Account) -> Self {
+        OldAccount {
+            principal: account.principal,
+            account_identifier: account.account_identifier,
+            default_account_transactions: Vec::new(),
+            sub_accounts: account
+                .sub_accounts
+                .into_iter()
+                .map(|(id, sa)| (id, sa.into()))
+                .collect(),
+            hardware_wallet_accounts: account
+                .hardware_wallet_accounts
+                .into_iter()
+                .map(NamedHardwareWalletAccount::into)
+                .collect(),
+            canisters: account.canisters,
+        }
+    }
+}
+
 #[derive(CandidType, Deserialize, Debug, Eq, PartialEq, Clone)]
 struct NamedSubAccount {
+    name: String,
+    account_identifier: AccountIdentifier,
+    // transactions: Do not reuse this field. There are still accounts in stable memor with this unused field.
+}
+
+// TODO(NNS1-2905): Delete this after it has been on mainnet for at least 1 release.
+#[derive(CandidType, Deserialize, Debug, Eq, PartialEq, Clone)]
+struct OldNamedSubAccount {
     name: String,
     account_identifier: AccountIdentifier,
     transactions: Vec<TransactionIndex>,
 }
 
+impl From<NamedSubAccount> for OldNamedSubAccount {
+    fn from(sub_account: NamedSubAccount) -> Self {
+        OldNamedSubAccount {
+            name: sub_account.name,
+            account_identifier: sub_account.account_identifier,
+            transactions: Vec::new(),
+        }
+    }
+}
+
 #[derive(CandidType, Deserialize, Debug, Eq, PartialEq, Clone)]
 struct NamedHardwareWalletAccount {
+    name: String,
+    principal: PrincipalId,
+    // transactions: Do not reuse this field. There are still accounts in stable memor with this unused field.
+}
+
+// TODO(NNS1-2905): Delete this after it has been on mainnet for at least 1 release.
+#[derive(CandidType, Deserialize, Debug, Eq, PartialEq, Clone)]
+struct OldNamedHardwareWalletAccount {
     name: String,
     principal: PrincipalId,
     transactions: Vec<TransactionIndex>,
 }
 
+impl From<NamedHardwareWalletAccount> for OldNamedHardwareWalletAccount {
+    fn from(hw_account: NamedHardwareWalletAccount) -> Self {
+        OldNamedHardwareWalletAccount {
+            name: hw_account.name,
+            principal: hw_account.principal,
+            transactions: Vec::new(),
+        }
+    }
+}
+
 #[derive(CandidType, Deserialize, Clone, Debug, PartialEq, Eq)]
 pub struct NamedCanister {
     name: String,
@@ -544,7 +619,6 @@ impl AccountsStore {
                 account.hardware_wallet_accounts.push(NamedHardwareWalletAccount {
                     name: request.name,
                     principal: request.principal,
-                    transactions: Vec::new(),
                 });
                 account
                     .hardware_wallet_accounts
@@ -1204,7 +1278,6 @@ impl Account {
         Account {
             principal: Some(principal),
             account_identifier,
-            default_account_transactions: Vec::new(),
             sub_accounts: HashMap::new(),
             hardware_wallet_accounts: Vec::new(),
             canisters: Vec::new(),
@@ -1217,7 +1290,6 @@ impl NamedSubAccount {
         NamedSubAccount {
             name,
             account_identifier,
-            transactions: Vec::new(),
         }
     }
 }

rs/backend/src/accounts_store/schema/tests.rs

@@ -21,7 +21,6 @@ pub fn toy_account(account_index: u64, num_canisters: u64) -> Account {
     let mut account = Account {
         principal: Some(principal),
         account_identifier,
-        default_account_transactions: Vec::new(),
         sub_accounts: HashMap::new(),
         hardware_wallet_accounts: Vec::new(),
         canisters: Vec::new(),

rs/backend/src/accounts_store/tests.rs

@@ -1067,8 +1067,6 @@ fn accounts_should_implement_storable() {
         ToyAccountSize {
             sub_accounts: 2,
             canisters: 3,
-            default_account_transactions: 4,
-            sub_account_transactions: 5,
             hardware_wallets: 6,
         },
     );
@@ -1077,4 +1075,73 @@ fn accounts_should_implement_storable() {
     assert_eq!(account, parsed);
 }
 
+fn create_new_test_account_for_encoding() -> Account {
+    let principal = PrincipalId::from_str(TEST_ACCOUNT_1).unwrap();
+    let account_identifier = AccountIdentifier::from(principal);
+
+    let sub_principal = PrincipalId::from_str(TEST_ACCOUNT_2).unwrap();
+    let sub_account_identifier = AccountIdentifier::from(sub_principal);
+
+    let hw_principal = PrincipalId::from_str(TEST_ACCOUNT_3).unwrap();
+
+    let new_sub_account = NamedSubAccount {
+        name: "Sub1".to_string(),
+        account_identifier: sub_account_identifier,
+    };
+
+    let mut new_sub_accounts = HashMap::new();
+    new_sub_accounts.insert(1, new_sub_account);
+
+    let new_hw_account = NamedHardwareWalletAccount {
+        name: "HW1".to_string(),
+        principal: hw_principal,
+    };
+
+    Account {
+        principal: Some(principal),
+        account_identifier,
+        sub_accounts: new_sub_accounts,
+        hardware_wallet_accounts: vec![new_hw_account],
+        canisters: vec![],
+    }
+}
+
+#[test]
+fn old_account_can_be_decoded_to_new_account() {
+    // Test that after upgrading to the next release, we are able to decode the
+    // old accounts from stable memory.
+    let expected_new_account = create_new_test_account_for_encoding();
+    let old_account = OldAccount::from(expected_new_account.clone());
+
+    let bytes = Candid((old_account,)).into_bytes().unwrap();
+    let decoded_new_account = Account::from_bytes(Cow::Owned(bytes));
+
+    assert_eq!(decoded_new_account, expected_new_account);
+}
+
+#[test]
+fn new_account_can_be_decoded_to_new_account() {
+    // Test that after upgrading to the next release after the next release, we
+    // are able to decode the new accounts that were encoded as old accounts.
+    let expected_new_account = create_new_test_account_for_encoding();
+
+    let bytes = expected_new_account.to_bytes();
+    let decoded_new_account = Account::from_bytes(bytes);
+
+    assert_eq!(decoded_new_account, expected_new_account);
+}
+
+#[test]
+fn new_account_can_be_decoded_to_old_account() {
+    // Test that, in case we need to roll back after the next release, the
+    // previous version is able to decode accounts written by the next version.
+    let new_account = create_new_test_account_for_encoding();
+    let expected_old_account = OldAccount::from(new_account.clone());
+
+    let bytes = new_account.to_bytes();
+    let (decoded_old_account,): (OldAccount,) = Candid::from_bytes(bytes.into_owned()).map(|c| c.0).unwrap();
+
+    assert_eq!(decoded_old_account, expected_old_account);
+}
+
 crate::accounts_store::schema::tests::test_accounts_db!(AccountsStore::default());

rs/backend/src/accounts_store/toy_data.rs

@@ -25,11 +25,6 @@ pub struct ToyAccountSize {
     pub sub_accounts: usize,
     /// The number of canisters
     pub canisters: usize,
-    /// The number of transaction indices for the main account; transactions are stored outside the
-    /// account.
-    pub default_account_transactions: usize,
-    /// The number of transactions per sub-account.
-    pub sub_account_transactions: usize,
     /// The number of hardware wallets.
     pub hardware_wallets: usize,
 }
@@ -38,25 +33,10 @@ impl From<&Account> for ToyAccountSize {
     fn from(account: &Account) -> Self {
         let sub_accounts = account.sub_accounts.len();
         let canisters = account.canisters.len();
-        let default_account_transactions = account.default_account_transactions.len();
-        // Average number of transactions per sub-account, rounded down.  When creating a toy
-        // account, all sub-accounts have the same number of transactions however that can change.
-        let total_sub_account_transactions: usize = account
-            .sub_accounts
-            .values()
-            .map(|sub_account| sub_account.transactions.len())
-            .sum();
-        let sub_account_transactions: usize = if sub_accounts == 0 {
-            0
-        } else {
-            total_sub_account_transactions / sub_accounts
-        };
         let hardware_wallets = account.hardware_wallet_accounts.len();
         ToyAccountSize {
             sub_accounts,
             canisters,
-            default_account_transactions,
-            sub_account_transactions,
             hardware_wallets,
         }
     }
@@ -76,7 +56,6 @@ pub fn toy_account(account_index: u64, size: ToyAccountSize) -> Account {
     let mut account = Account {
         principal: Some(principal),
         account_identifier,
-        default_account_transactions: Vec::new(),
         sub_accounts: HashMap::new(),
         hardware_wallet_accounts: Vec::new(),
         canisters: Vec::new(),
@@ -87,8 +66,7 @@ pub fn toy_account(account_index: u64, size: ToyAccountSize) -> Account {
         let sub_account_name = format!("sub_account_{account_index}_{sub_account_index}");
         let sub_account = convert_byte_to_sub_account(sub_account_index);
         let sub_account_identifier = AccountIdentifier::new(principal, Some(sub_account));
-        let mut named_sub_account = NamedSubAccount::new(sub_account_name.clone(), sub_account_identifier);
-        named_sub_account.transactions = (0..size.sub_account_transactions as u64).collect();
+        let named_sub_account = NamedSubAccount::new(sub_account_name.clone(), sub_account_identifier);
         account.sub_accounts.insert(sub_account_index, named_sub_account);
     }
     // Attaches canisters to the account.
@@ -100,13 +78,6 @@ pub fn toy_account(account_index: u64, size: ToyAccountSize) -> Account {
         };
         account.canisters.push(canister);
     }
-    for transaction_index in 0..size.default_account_transactions as u64 {
-        // Note: Normally a transaction would be added to the list of transactions in the accounts
-        // store and the index of that transaction would be stored in the account itself.  Given
-        // that we are creating a standalong account, without an account store, the index is
-        // meaningless.
-        account.default_account_transactions.push(transaction_index);
-    }
     for hardware_wallet_index in 0..size.hardware_wallets as u64 {
         // Note: The principal is currently unused but in case it is used in future tests we make a
         // modest attempt to avoid collisions by:
@@ -118,7 +89,6 @@ pub fn toy_account(account_index: u64, size: ToyAccountSize) -> Account {
         let hardware_wallet = NamedHardwareWalletAccount {
             name: format!("hw_wallet_{account_index}_{hardware_wallet_index}"),
             principal,
-            transactions: Vec::new(),
         };
         account.hardware_wallet_accounts.push(hardware_wallet);
     }
@@ -132,8 +102,6 @@ fn toy_account_should_have_the_requested_size() {
     let requested_size = ToyAccountSize {
         sub_accounts: 1,
         canisters: 2,
-        default_account_transactions: 3,
-        sub_account_transactions: 4,
         hardware_wallets: 5,
     };
     let account = toy_account(9, requested_size);