Skip to content

Commit

Permalink
FOLLOW-244: Stop claiming neurons in the nns-dapp canister (#5738)
Browse files Browse the repository at this point in the history
# 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
  • Loading branch information
dskloetd authored Nov 7, 2024
1 parent babf022 commit 36f052e
Show file tree
Hide file tree
Showing 7 changed files with 16 additions and 117 deletions.
1 change: 1 addition & 0 deletions .config/spellcheck.dic
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,4 @@ M2
protobuf
se
TVL
backend
2 changes: 1 addition & 1 deletion CHANGELOG-Nns-Dapp-unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
36 changes: 2 additions & 34 deletions rs/backend/src/accounts_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -251,7 +250,6 @@ pub enum TransactionType {
Transfer,
Approve,
TransferFrom,
StakeNeuron,
StakeNeuronNotification,
CreateCanister,
TopUpCanister(CanisterId),
Expand Down Expand Up @@ -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);
}
}
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand All @@ -996,7 +971,6 @@ impl AccountsStore {
principal: PrincipalId,
from: AccountIdentifier,
to: AccountIdentifier,
memo: Memo,
block_height: BlockIndex,
) {
match transaction_type {
Expand All @@ -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,
Expand Down
32 changes: 0 additions & 32 deletions rs/backend/src/accounts_store/tests.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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();
Expand Down
24 changes: 4 additions & 20 deletions rs/backend/src/canisters/governance.rs
Original file line number Diff line number Diff line change
@@ -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<ClaimOrRefreshNeuronFromAccountResponse, String> {
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;
Expand All @@ -28,7 +10,9 @@ type GetMetricsCallResult = Result<Result<GovernanceCachedMetrics, GovernanceErr

#[cfg(not(test))]
mod prod {
use super::{candid, GetMetricsCallResult, GOVERNANCE_CANISTER_ID};
use super::GetMetricsCallResult;
use dfn_candid::candid;
use ic_nns_constants::GOVERNANCE_CANISTER_ID;

pub async fn get_metrics() -> GetMetricsCallResult {
dfn_core::call(GOVERNANCE_CANISTER_ID, "get_metrics", candid, ())
Expand Down
2 changes: 2 additions & 0 deletions rs/backend/src/multi_part_transactions_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
36 changes: 6 additions & 30 deletions rs/backend/src/periodic_tasks_runner.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
}
Expand All @@ -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| {
Expand Down Expand Up @@ -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<NeuronId, String> {
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,
Expand Down

0 comments on commit 36f052e

Please sign in to comment.