From 36f052e03be8762af754301bdc618180894c46f6 Mon Sep 17 00:00:00 2001 From: David de Kloet <122978264+dskloetd@users.noreply.github.com> Date: Thu, 7 Nov 2024 17:31:48 +0100 Subject: [PATCH] FOLLOW-244: Stop claiming neurons in the nns-dapp canister (#5738) # Motivation Staking a neuron requires 2 steps: transferring the stake and claiming the neuron. Both steps are done in ic-js. But it's possible that the process gets interrupted in between. In this case the process needs to be finished later. This is now done in the frontend so it no longer needs to be done in the backend. # Changes 1. Remove logic to detect stake neuron transaction and claim neurons from the nns-dapp canister code. # Tests 1. Unit test removed. # Todos - [x] Add entry to changelog (if necessary). existing entry updated --- .config/spellcheck.dic | 1 + CHANGELOG-Nns-Dapp-unreleased.md | 2 +- rs/backend/src/accounts_store.rs | 36 ++----------------- rs/backend/src/accounts_store/tests.rs | 32 ----------------- rs/backend/src/canisters/governance.rs | 24 +++---------- .../src/multi_part_transactions_processor.rs | 2 ++ rs/backend/src/periodic_tasks_runner.rs | 36 ++++--------------- 7 files changed, 16 insertions(+), 117 deletions(-) diff --git a/.config/spellcheck.dic b/.config/spellcheck.dic index ed8678670b6..8fca4180ed3 100644 --- a/.config/spellcheck.dic +++ b/.config/spellcheck.dic @@ -56,3 +56,4 @@ M2 protobuf se TVL +backend diff --git a/CHANGELOG-Nns-Dapp-unreleased.md b/CHANGELOG-Nns-Dapp-unreleased.md index ad8ae4bb850..d5b73b6a79a 100644 --- a/CHANGELOG-Nns-Dapp-unreleased.md +++ b/CHANGELOG-Nns-Dapp-unreleased.md @@ -19,7 +19,7 @@ proposal is successful, the changes it released will be moved from this file to #### Changed * Updated the dark theme and page icons. -* Claim unclaimed neurons from the frontend. +* Claim unclaimed neurons from the frontend instead of the backend. #### Deprecated diff --git a/rs/backend/src/accounts_store.rs b/rs/backend/src/accounts_store.rs index 3e65e99b4b6..0d3f1d0cdfe 100644 --- a/rs/backend/src/accounts_store.rs +++ b/rs/backend/src/accounts_store.rs @@ -7,9 +7,8 @@ use candid::CandidType; use dfn_candid::Candid; use histogram::AccountsStoreHistogram; use ic_base_types::{CanisterId, PrincipalId}; -use ic_crypto_sha2::Sha256; use ic_nns_common::types::NeuronId; -use ic_nns_constants::{CYCLES_MINTING_CANISTER_ID, GOVERNANCE_CANISTER_ID}; +use ic_nns_constants::CYCLES_MINTING_CANISTER_ID; use ic_stable_structures::{storable::Bound, Storable}; use icp_ledger::Operation::{self, Approve, Burn, Mint, Transfer}; use icp_ledger::{AccountIdentifier, BlockIndex, Memo, Subaccount}; @@ -251,7 +250,6 @@ pub enum TransactionType { Transfer, Approve, TransferFrom, - StakeNeuron, StakeNeuronNotification, CreateCanister, TopUpCanister(CanisterId), @@ -599,7 +597,7 @@ impl AccountsStore { &canister_ids, default_transaction_type, ); - self.process_transaction_type(transaction_type, principal, from, to, memo, block_height); + self.process_transaction_type(transaction_type, principal, from, to, block_height); } } } @@ -920,8 +918,6 @@ impl AccountsStore { TransactionType::CreateCanister } else if let Some(canister_id) = Self::is_topup_canister_transaction(memo, &to, canister_ids) { TransactionType::TopUpCanister(canister_id) - } else if Self::is_stake_neuron_transaction(memo, &to, principal) { - TransactionType::StakeNeuron } else { default_transaction_type } @@ -964,27 +960,6 @@ impl AccountsStore { None } - fn is_stake_neuron_transaction(memo: Memo, to: &AccountIdentifier, principal: &PrincipalId) -> bool { - if memo.0 > 0 { - let expected_to = Self::generate_stake_neuron_address(principal, memo); - *to == expected_to - } else { - false - } - } - - fn generate_stake_neuron_address(principal: &PrincipalId, memo: Memo) -> AccountIdentifier { - let subaccount = Subaccount({ - let mut state = Sha256::new(); - state.write(&[0x0c]); - state.write(b"neuron-stake"); - state.write(principal.as_slice()); - state.write(&memo.0.to_be_bytes()); - state.finish() - }); - AccountIdentifier::new(GOVERNANCE_CANISTER_ID.get(), Some(subaccount)) - } - /// Certain transaction types require additional processing (Stake Neuron, Create Canister, /// etc). Each time we detect one of these transaction types we need to add the details to the /// `multi_part_transactions_processor` which will work through the required actions in the @@ -996,7 +971,6 @@ impl AccountsStore { principal: PrincipalId, from: AccountIdentifier, to: AccountIdentifier, - memo: Memo, block_height: BlockIndex, ) { match transaction_type { @@ -1006,12 +980,6 @@ impl AccountsStore { MultiPartTransactionToBeProcessed::ParticipateSwap(principal, from, to, swap_canister_id), ); } - TransactionType::StakeNeuron => { - self.multi_part_transactions_processor.push( - block_height, - MultiPartTransactionToBeProcessed::StakeNeuron(principal, memo), - ); - } TransactionType::CreateCanister => { self.multi_part_transactions_processor.push( block_height, diff --git a/rs/backend/src/accounts_store/tests.rs b/rs/backend/src/accounts_store/tests.rs index 8a61c995207..44fee09b03f 100644 --- a/rs/backend/src/accounts_store/tests.rs +++ b/rs/backend/src/accounts_store/tests.rs @@ -1,7 +1,6 @@ use super::histogram::AccountsStoreHistogram; use super::*; use crate::accounts_store::toy_data::{toy_account, ToyAccountSize}; -use crate::multi_part_transactions_processor::MultiPartTransactionToBeProcessed; use icp_ledger::Tokens; use pretty_assertions::assert_eq; use std::str::FromStr; @@ -135,37 +134,6 @@ fn register_hardware_wallet_hardware_wallet_already_registered() { ); } -#[test] -fn maybe_process_transaction_detects_neuron_transactions() { - let mut store = setup_test_store(); - - let block_height = store.get_block_height_synced_up_to().unwrap_or(0) + 1; - - let neuron_principal = PrincipalId::from_str(TEST_ACCOUNT_1).unwrap(); - let neuron_memo = Memo(16656605094239839590); - let neuron_account = AccountsStore::generate_stake_neuron_address(&neuron_principal, neuron_memo); - - let transfer = Transfer { - from: AccountIdentifier::new(neuron_principal, None), - to: neuron_account, - spender: None, - amount: Tokens::from_tokens(1).unwrap(), - fee: Tokens::from_e8s(10000), - }; - store - .maybe_process_transaction(&transfer, neuron_memo, block_height) - .unwrap(); - - if let Some((_, MultiPartTransactionToBeProcessed::StakeNeuron(principal, memo))) = - store.multi_part_transactions_processor.take_next() - { - assert_eq!(principal, neuron_principal); - assert_eq!(memo, neuron_memo); - } else { - panic!(); - } -} - #[test] fn attach_canister_followed_by_get_canisters() { let mut store = setup_test_store(); diff --git a/rs/backend/src/canisters/governance.rs b/rs/backend/src/canisters/governance.rs index f2837592e61..ea116bca680 100644 --- a/rs/backend/src/canisters/governance.rs +++ b/rs/backend/src/canisters/governance.rs @@ -1,22 +1,4 @@ -use dfn_candid::candid; -use ic_nns_constants::GOVERNANCE_CANISTER_ID; -pub use ic_nns_governance::pb::v1::{ - governance::GovernanceCachedMetrics, ClaimOrRefreshNeuronFromAccount, ClaimOrRefreshNeuronFromAccountResponse, - GovernanceError, -}; - -pub async fn claim_or_refresh_neuron_from_account( - request: ClaimOrRefreshNeuronFromAccount, -) -> Result { - dfn_core::call( - GOVERNANCE_CANISTER_ID, - "claim_or_refresh_neuron_from_account", - candid, - (request,), - ) - .await - .map_err(|e| e.1) -} +pub use ic_nns_governance::pb::v1::{governance::GovernanceCachedMetrics, GovernanceError}; #[cfg(not(test))] pub use prod::get_metrics; @@ -28,7 +10,9 @@ type GetMetricsCallResult = Result GetMetricsCallResult { dfn_core::call(GOVERNANCE_CANISTER_ID, "get_metrics", candid, ()) diff --git a/rs/backend/src/multi_part_transactions_processor.rs b/rs/backend/src/multi_part_transactions_processor.rs index ce97a605c53..72c30ec5ee0 100644 --- a/rs/backend/src/multi_part_transactions_processor.rs +++ b/rs/backend/src/multi_part_transactions_processor.rs @@ -12,6 +12,8 @@ pub struct MultiPartTransactionsProcessor { #[derive(Clone, CandidType, Deserialize, Debug, Eq, PartialEq)] pub enum MultiPartTransactionToBeProcessed { + // TODO: Remove StakeNeuron after a version has been released that does not + // add StakeNeuron to the multi-part transaction queue anymore. StakeNeuron(PrincipalId, Memo), CreateCanisterV2(PrincipalId), TopUpCanisterV2(PrincipalId, CanisterId), diff --git a/rs/backend/src/periodic_tasks_runner.rs b/rs/backend/src/periodic_tasks_runner.rs index ff581b2db17..f63b379931b 100644 --- a/rs/backend/src/periodic_tasks_runner.rs +++ b/rs/backend/src/periodic_tasks_runner.rs @@ -1,13 +1,11 @@ -use crate::canisters::{cmc, governance}; +use crate::canisters::cmc; use crate::ledger_sync; use crate::multi_part_transactions_processor::MultiPartTransactionToBeProcessed; use crate::state::with_state_mut; use crate::Cycles; use cycles_minting_canister::{NotifyCreateCanister, NotifyError, NotifyTopUp}; use dfn_core::api::{CanisterId, PrincipalId}; -use ic_nns_common::types::NeuronId; -use ic_nns_governance::pb::v1::{claim_or_refresh_neuron_from_account_response, ClaimOrRefreshNeuronFromAccount}; -use icp_ledger::{BlockIndex, Memo}; +use icp_ledger::BlockIndex; pub async fn run_periodic_tasks() { ledger_sync::sync_transactions().await; @@ -23,9 +21,10 @@ pub async fn run_periodic_tasks() { // DO NOTHING // Handling ParticipateSwap is not supported. } - MultiPartTransactionToBeProcessed::StakeNeuron(principal, memo) => { - handle_stake_neuron(principal, memo).await; - } + // TODO: Remove StakeNeuron after a version has been released that + // does not add StakeNeuron to the multi-part transaction + // queue anymore. + MultiPartTransactionToBeProcessed::StakeNeuron(_principal, _memo) => {} MultiPartTransactionToBeProcessed::CreateCanisterV2(controller) => { handle_create_canister_v2(block_height, controller).await; } @@ -36,13 +35,6 @@ pub async fn run_periodic_tasks() { } } -async fn handle_stake_neuron(principal: PrincipalId, memo: Memo) { - match claim_or_refresh_neuron(principal, memo).await { - Ok(_neuron_id) => (), - Err(_error) => (), - } -} - async fn handle_create_canister_v2(block_height: BlockIndex, controller: PrincipalId) { match create_canister_v2(block_height, controller).await { Ok(Ok(canister_id)) => with_state_mut(|s| { @@ -77,22 +69,6 @@ async fn handle_top_up_canister_v2(block_height: BlockIndex, principal: Principa } } -async fn claim_or_refresh_neuron(principal: PrincipalId, memo: Memo) -> Result { - let request = ClaimOrRefreshNeuronFromAccount { - controller: Some(principal), - memo: memo.0, - }; - - match governance::claim_or_refresh_neuron_from_account(request).await { - Ok(response) => match response.result { - Some(claim_or_refresh_neuron_from_account_response::Result::NeuronId(neuron_id)) => Ok(neuron_id.into()), - Some(claim_or_refresh_neuron_from_account_response::Result::Error(e)) => Err(e.error_message), - None => Err("'response.result' was empty".to_string()), - }, - Err(e) => Err(e), - } -} - async fn create_canister_v2( block_index: BlockIndex, controller: PrincipalId,