Skip to content

Commit

Permalink
Allow all actions in signing nodes (#361)
Browse files Browse the repository at this point in the history
* all actions allowed in signing nodes, delete acc action blocked on the leader node
  • Loading branch information
volovyks authored Nov 16, 2023
1 parent 3f92c7e commit 211a1c0
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 48 deletions.
31 changes: 13 additions & 18 deletions integration-tests/tests/mpc/negative.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,7 @@ async fn whitlisted_actions_test() -> anyhow::Result<()> {
}

// Performing blacklisted actions
let blacklisted_actions = vec![
ActionType::CreateAccount,
ActionType::DeployContract,
ActionType::FunctionCall,
ActionType::Transfer,
ActionType::Stake,
ActionType::DeleteAccount,
];
let blacklisted_actions = vec![ActionType::DeleteAccount];

for blacklisted_action in blacklisted_actions {
ctx.leader_node
Expand Down Expand Up @@ -141,28 +134,30 @@ async fn whitlisted_actions_test() -> anyhow::Result<()> {
}

pub enum ActionType {
CreateAccount,
DeployContract,
FunctionCall,
Transfer,
Stake,
_CreateAccount,
_DeployContract,
_FunctionCall,
_Transfer,
_Stake,
AddKey,
DeleteKey,
DeleteAccount,
}

fn get_stub_delegate_action(action_type: ActionType) -> anyhow::Result<DelegateAction> {
let action: Action = match action_type {
ActionType::CreateAccount => Action::CreateAccount(CreateAccountAction {}),
ActionType::DeployContract => Action::DeployContract(DeployContractAction { code: vec![] }),
ActionType::FunctionCall => Action::FunctionCall(FunctionCallAction {
ActionType::_CreateAccount => Action::CreateAccount(CreateAccountAction {}),
ActionType::_DeployContract => {
Action::DeployContract(DeployContractAction { code: vec![] })
}
ActionType::_FunctionCall => Action::FunctionCall(FunctionCallAction {
method_name: "test".to_string(),
args: vec![],
gas: 0,
deposit: 0,
}),
ActionType::Transfer => Action::Transfer(TransferAction { deposit: 0 }),
ActionType::Stake => Action::Stake(StakeAction {
ActionType::_Transfer => Action::Transfer(TransferAction { deposit: 0 }),
ActionType::_Stake => Action::Stake(StakeAction {
stake: 0,
public_key: key::random_sk().public_key(),
}),
Expand Down
6 changes: 3 additions & 3 deletions mpc-recovery/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ pub enum LeaderNodeError {
RelayerError(#[from] RelayerError),
#[error("recovery key can not be deleted: {0}")]
RecoveryKeyCanNotBeDeleted(PublicKey),
#[error("action can not be performed, account deletion is not allowed")]
AccountDeletionUnsupported,
#[error("failed to retrieve recovery pk, check digest signature: {0}")]
FailedToRetrieveRecoveryPk(anyhow::Error),
#[error("timeout gathering sign node pks")]
Expand All @@ -94,6 +96,7 @@ impl LeaderNodeError {
LeaderNodeError::RelayerError(_) => StatusCode::FAILED_DEPENDENCY,
LeaderNodeError::TimeoutGatheringPublicKeys => StatusCode::INTERNAL_SERVER_ERROR,
LeaderNodeError::RecoveryKeyCanNotBeDeleted(_) => StatusCode::BAD_REQUEST,
LeaderNodeError::AccountDeletionUnsupported => StatusCode::BAD_REQUEST,
LeaderNodeError::FailedToRetrieveRecoveryPk(_) => StatusCode::UNAUTHORIZED,
LeaderNodeError::NetworkRejection(err) => {
err.status().unwrap_or(StatusCode::REQUEST_TIMEOUT)
Expand All @@ -117,8 +120,6 @@ pub enum SignNodeError {
OidcTokenNotClaimed(OidcDigest),
#[error("aggregate signing failed: {0}")]
AggregateSigningFailed(#[from] AggregateSigningError),
#[error("This kind of action can not be performed")]
UnsupportedAction,
#[error(transparent)]
Other(#[from] anyhow::Error),
}
Expand All @@ -132,7 +133,6 @@ impl SignNodeError {
Self::OidcTokenClaimedWithAnotherKey(_) => StatusCode::UNAUTHORIZED,
Self::OidcTokenNotClaimed(_) => StatusCode::UNAUTHORIZED,
Self::AggregateSigningFailed(err) => err.code(),
Self::UnsupportedAction => StatusCode::BAD_REQUEST,
Self::Other(_) => StatusCode::INTERNAL_SERVER_ERROR,
}
}
Expand Down
17 changes: 16 additions & 1 deletion mpc-recovery/src/leader_node/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use borsh::BorshDeserialize;
use curv::elliptic::curves::{Ed25519, Point};
use near_fetch::signer::KeyRotatingSigner;
use near_primitives::delegate_action::{DelegateAction, NonDelegateAction};
use near_primitives::transaction::{Action, DeleteKeyAction};
use near_primitives::transaction::{Action, DeleteAccountAction, DeleteKeyAction};
use near_primitives::types::AccountId;
use prometheus::{Encoder, TextEncoder};
use std::net::SocketAddr;
Expand Down Expand Up @@ -529,6 +529,21 @@ async fn process_sign(
}
}

// Prevent account deletion
// Note: we can allow account deletionn once we sync that with relayer. With current logic user will not be able to create another account.
let delete_account_actions: Vec<&DeleteAccountAction> = requested_actions
.iter()
.filter_map(|action| match action {
Action::DeleteAccount(delete_account_action) => Some(delete_account_action),
_ => None,
})
.collect();

if !delete_account_actions.is_empty() {
tracing::error!("Account deletion is not allowed");
Err(LeaderNodeError::AccountDeletionUnsupported)?;
}

// Get MPC signature
nar::retry(|| async {
let signature = get_mpc_signature(
Expand Down
26 changes: 0 additions & 26 deletions mpc-recovery/src/sign_node/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,8 @@ use borsh::BorshSerialize;
use curv::elliptic::curves::{Ed25519, Point};
use multi_party_eddsa::protocols::{self, ExpandedKeyPair};

use near_primitives::delegate_action::NonDelegateAction;
use near_primitives::hash::hash;
use near_primitives::signable_message::{SignableMessage, SignableMessageType};
use near_primitives::transaction::{Action, AddKeyAction, DeleteKeyAction};

use std::net::SocketAddr;
use std::sync::Arc;
Expand Down Expand Up @@ -266,30 +264,6 @@ async fn process_commit(
}
};

// Restrict certain types of DelegateActions
let requested_delegate_actions: &Vec<NonDelegateAction> =
&request.delegate_action.actions;

let requested_actions: &Vec<Action> = &requested_delegate_actions
.iter()
.map(|non_delegate_action| Action::from(non_delegate_action.clone()))
.collect();

for action in requested_actions {
match action {
Action::AddKey(AddKeyAction { .. }) => {
tracing::debug!("AddKeyAction is supported");
}
Action::DeleteKey(DeleteKeyAction { .. }) => {
tracing::debug!("DeleteKeyAction is supported");
}
_ => {
tracing::error!("Unsupported action: {:?}", action);
return Err(SignNodeError::UnsupportedAction);
}
}
}

// Get user credentials
let internal_account_id = oidc_token_claims.get_internal_account_id();
let user_credentials = get_or_generate_user_creds(&state, internal_account_id).await?;
Expand Down

0 comments on commit 211a1c0

Please sign in to comment.