From 13f06a1aca766e07d1c51b45ea9635e336dfb7aa Mon Sep 17 00:00:00 2001 From: Serhii Volovyk Date: Fri, 11 Aug 2023 14:48:31 +0300 Subject: [PATCH 01/10] atomic account creation tests --- integration-tests/tests/lib.rs | 22 +++++++ integration-tests/tests/mpc/negative.rs | 82 +++++++++++++++++++++++-- 2 files changed, 100 insertions(+), 4 deletions(-) diff --git a/integration-tests/tests/lib.rs b/integration-tests/tests/lib.rs index 783afe4f0..f05c0b513 100644 --- a/integration-tests/tests/lib.rs +++ b/integration-tests/tests/lib.rs @@ -230,6 +230,7 @@ trait MpcCheck { fn assert_ok(self) -> anyhow::Result; fn assert_bad_request_contains(self, expected: &str) -> anyhow::Result; fn assert_unauthorized_contains(self, expected: &str) -> anyhow::Result; + fn assert_internal_error_contains(self, expected: &str) -> anyhow::Result; fn assert_bad_request(self) -> anyhow::Result where @@ -243,6 +244,12 @@ trait MpcCheck { { self.assert_unauthorized_contains("") } + fn assert_internal_error(self) -> anyhow::Result + where + Self: Sized, + { + self.assert_internal_error_contains("") + } } // Presumes that $response::Err has a `msg: String` field. @@ -296,6 +303,21 @@ macro_rules! impl_mpc_check { }; assert!(msg.contains(expected)); + Ok(response) + } else { + anyhow::bail!("expected 401, but got {status_code} with response: {response:?}"); + } + } + fn assert_internal_error_contains(self, expected: &str) -> anyhow::Result { + let status_code = self.0; + let response = self.1; + + if status_code == StatusCode::INTERNAL_SERVER_ERROR { + let $response::Err { ref msg, .. } = response else { + anyhow::bail!("unexpected Ok with a 500 http code"); + }; + assert!(msg.contains(expected)); + Ok(response) } else { anyhow::bail!("expected 401, but got {status_code} with response: {response:?}"); diff --git a/integration-tests/tests/mpc/negative.rs b/integration-tests/tests/mpc/negative.rs index 467aad493..e9c3dfacd 100644 --- a/integration-tests/tests/mpc/negative.rs +++ b/integration-tests/tests/mpc/negative.rs @@ -209,11 +209,11 @@ async fn negative_front_running_protection() -> anyhow::Result<()> { // Relayer is wasting a token even if account was not created // ctx.leader_node // .new_account_with_helper( - // account_id.to_string(), - // user_public_key.clone(), + // &account_id.to_string(), + // &user_public_key, // None, - // user_secret_key.clone(), - // oidc_token_1.clone(), + // &user_secret_key, + // &oidc_token_1, // ) // .await? // .assert_unauthorized_contains("was not claimed")?; @@ -633,3 +633,77 @@ async fn test_malformed_raw_create_account() -> anyhow::Result<()> { }) .await } + +#[test(tokio::test)] +async fn test_account_creation_should_work_on_second_attempt() -> anyhow::Result<()> { + with_nodes(2, |ctx| { + Box::pin(async move { + let account_id = account::random(ctx.worker)?; + let user_secret_key = key::random_sk(); + let user_public_key = user_secret_key.public_key(); + let oidc_token = token::valid_random(); + + ctx.leader_node + .new_account_with_helper( + &account_id.to_string(), + &user_public_key, + None, + &user_secret_key, + &oidc_token, + ) + .await? + .assert_unauthorized_contains("was not claimed")?; + + register_account( + &ctx, + &account_id, + &user_secret_key, + &user_public_key, + &oidc_token, + None, + ) + .await?; + + Ok(()) + }) + }) + .await +} + +#[test(tokio::test)] +async fn test_creation_of_two_account_with_the_same_oidc_should_not_be_possible( +) -> anyhow::Result<()> { + with_nodes(2, |ctx| { + Box::pin(async move { + let account_id = account::random(ctx.worker)?; + let account_id_2 = account::random(ctx.worker)?; + let user_secret_key = key::random_sk(); + let user_public_key = user_secret_key.public_key(); + let oidc_token = token::valid_random(); + + register_account( + &ctx, + &account_id, + &user_secret_key, + &user_public_key, + &oidc_token, + None, + ) + .await?; + + ctx.leader_node + .new_account_with_helper( + &account_id_2.to_string(), + &user_public_key, + None, + &user_secret_key, + &oidc_token, + ) + .await? + .assert_internal_error_contains("Yu can only register 1 account per oath_token")?; + + Ok(()) + }) + }) + .await +} From 5c35b23810eb301fc214b1ff1c03a5a88dd286e8 Mon Sep 17 00:00:00 2001 From: Serhii Volovyk Date: Thu, 14 Sep 2023 15:37:24 +0300 Subject: [PATCH 02/10] register_account_atomic (broken) --- integration-tests/tests/lib.rs | 1 + integration-tests/tests/mpc/negative.rs | 30 ++++++------ mpc-recovery/src/leader_node/mod.rs | 20 ++++---- mpc-recovery/src/relayer/mod.rs | 62 ++++++++++++++++++++++++- mpc-recovery/src/relayer/msg.rs | 10 ++++ 5 files changed, 95 insertions(+), 28 deletions(-) diff --git a/integration-tests/tests/lib.rs b/integration-tests/tests/lib.rs index 0bef67ad0..6605348b8 100644 --- a/integration-tests/tests/lib.rs +++ b/integration-tests/tests/lib.rs @@ -300,6 +300,7 @@ macro_rules! impl_mpc_check { ); } } + // ideally we should not have situations where we can get INTERNAL_SERVER_ERROR fn assert_internal_error_contains(self, expected: &str) -> anyhow::Result { let status_code = self.0; let response = self.1; diff --git a/integration-tests/tests/mpc/negative.rs b/integration-tests/tests/mpc/negative.rs index 5a3be1b91..104b5d10e 100644 --- a/integration-tests/tests/mpc/negative.rs +++ b/integration-tests/tests/mpc/negative.rs @@ -206,18 +206,16 @@ async fn negative_front_running_protection() -> anyhow::Result<()> { let wrong_oidc_token = OidcToken::random(); // Create account before claiming OIDC token - // This part of the test is commented since account creation is not atomic (known issue) - // Relayer is wasting a token even if account was not created - // ctx.leader_node - // .new_account_with_helper( - // &account_id.to_string(), - // &user_public_key, - // None, - // &user_secret_key, - // &oidc_token_1, - // ) - // .await? - // .assert_unauthorized_contains("was not claimed")?; + ctx.leader_node + .new_account_with_helper( + &account_id, + &user_public_key, + None, + &user_secret_key, + &oidc_token_1, + ) + .await? + .assert_unauthorized_contains("was not claimed")?; // Get user recovery PK before claiming OIDC token ctx.leader_node @@ -613,11 +611,11 @@ async fn test_account_creation_should_work_on_second_attempt() -> anyhow::Result let account_id = account::random(ctx.worker)?; let user_secret_key = key::random_sk(); let user_public_key = user_secret_key.public_key(); - let oidc_token = token::valid_random(); + let oidc_token = OidcToken::random(); ctx.leader_node .new_account_with_helper( - &account_id.to_string(), + &account_id, &user_public_key, None, &user_secret_key, @@ -651,7 +649,7 @@ async fn test_creation_of_two_account_with_the_same_oidc_should_not_be_possible( let account_id_2 = account::random(ctx.worker)?; let user_secret_key = key::random_sk(); let user_public_key = user_secret_key.public_key(); - let oidc_token = token::valid_random(); + let oidc_token = OidcToken::random(); register_account( &ctx, @@ -665,7 +663,7 @@ async fn test_creation_of_two_account_with_the_same_oidc_should_not_be_possible( ctx.leader_node .new_account_with_helper( - &account_id_2.to_string(), + &account_id_2, &user_public_key, None, &user_secret_key, diff --git a/mpc-recovery/src/leader_node/mod.rs b/mpc-recovery/src/leader_node/mod.rs index 706da2e9e..c7836b6e4 100644 --- a/mpc-recovery/src/leader_node/mod.rs +++ b/mpc-recovery/src/leader_node/mod.rs @@ -7,7 +7,7 @@ use crate::msg::{ SignRequest, SignResponse, UserCredentialsRequest, UserCredentialsResponse, }; use crate::oauth::OAuthTokenVerifier; -use crate::relayer::msg::RegisterAccountRequest; +use crate::relayer::msg::{RegisterAccountAtomicRequest, RegisterAccountRequest}; use crate::relayer::NearRpcAndRelayerClient; use crate::transaction::{ get_create_account_delegate_action, get_local_signed_delegated_action, get_mpc_signature, @@ -355,15 +355,6 @@ async fn process_new_account( .map_err(LeaderNodeError::SignatureVerificationFailed)?; tracing::debug!("user credentials digest signature verified for {new_user_account_id:?}"); - state - .client - .register_account(RegisterAccountRequest { - account_id: new_user_account_id.clone(), - allowance: 300_000_000_000_000, - oauth_token: internal_acc_id.clone(), - }) - .await?; - nar::retry(|| async { // Get nonce and recent block hash let (_hash, block_height, nonce) = state @@ -409,7 +400,14 @@ async fn process_new_account( ); // Send delegate action to relayer - let result = state.client.send_meta_tx(signed_delegate_action).await; + let request = RegisterAccountAtomicRequest { + account_id: new_user_account_id.clone(), + allowance: 300_000_000_000_000, + oauth_token: internal_acc_id.clone(), + signed_delegate_action, + }; + + let result = state.client.register_account_atomic(request).await; if let Err(err) = &result { let err_str = format!("{:?}", err); state diff --git a/mpc-recovery/src/relayer/mod.rs b/mpc-recovery/src/relayer/mod.rs index 3ad95f720..baa2062d6 100644 --- a/mpc-recovery/src/relayer/mod.rs +++ b/mpc-recovery/src/relayer/mod.rs @@ -10,7 +10,9 @@ use near_primitives::types::{AccountId, BlockHeight, Nonce}; use near_primitives::views::FinalExecutionStatus; use self::error::RelayerError; -use self::msg::{RegisterAccountRequest, SendMetaTxRequest, SendMetaTxResponse}; +use self::msg::{ + RegisterAccountAtomicRequest, RegisterAccountRequest, SendMetaTxRequest, SendMetaTxResponse, +}; use crate::nar::{self, CachedAccessKeyNonces}; @@ -99,6 +101,64 @@ impl NearRpcAndRelayerClient { } } + #[tracing::instrument(level = "debug", skip_all, fields(account_id = request.account_id.to_string()))] + pub async fn register_account_atomic( + &self, + request: RegisterAccountAtomicRequest, + ) -> Result { + let mut req = Request::builder() + .method(Method::POST) + .uri(format!("{}/register_account_atomic", self.relayer_url)) + .header("content-type", "application/json"); + + if let Some(api_key) = &self.api_key { + req = req.header("x-api-key", api_key); + }; + + let request = req + .body(Body::from( + serde_json::to_vec(&request) + .map_err(|e| RelayerError::DataConversionFailure(e.into()))?, + )) + .map_err(|e| RelayerError::NetworkFailure(e.into()))?; + + tracing::debug!("constructed http request to {}", self.relayer_url); + let client = Client::new(); + let response = client + .request(request) + .await + .map_err(|e| RelayerError::NetworkFailure(e.into()))?; + + let status = response.status(); + let response_body = hyper::body::to_bytes(response.into_body()) + .await + .map_err(|e| RelayerError::NetworkFailure(e.into()))?; + let msg = std::str::from_utf8(&response_body) + .map_err(|e| RelayerError::DataConversionFailure(e.into()))?; + + if status.is_success() { + tracing::debug!(response_body = msg, "got response"); + let response: SendMetaTxResponse = serde_json::from_slice(&response_body) + .map_err(|e| RelayerError::DataConversionFailure(e.into()))?; + match response.status { + FinalExecutionStatus::NotStarted | FinalExecutionStatus::Started => { + Err(RelayerError::TxNotReady) + } + FinalExecutionStatus::Failure(e) => Err(RelayerError::TxExecutionFailure(e)), + FinalExecutionStatus::SuccessValue(ref value) => { + tracing::debug!( + value = std::str::from_utf8(value) + .map_err(|e| RelayerError::DataConversionFailure(e.into()))?, + "success" + ); + Ok(response) + } + } + } else { + Err(RelayerError::RequestFailure(status, msg.to_string())) + } + } + #[tracing::instrument(level = "debug", skip_all, fields(receiver_id = request.delegate_action.receiver_id.to_string()))] pub async fn send_meta_tx( &self, diff --git a/mpc-recovery/src/relayer/msg.rs b/mpc-recovery/src/relayer/msg.rs index 9ce77d0a2..a20b082e2 100644 --- a/mpc-recovery/src/relayer/msg.rs +++ b/mpc-recovery/src/relayer/msg.rs @@ -14,6 +14,16 @@ pub struct RegisterAccountRequest { pub oauth_token: String, } +#[derive(Clone, Debug, Serialize, Deserialize)] +pub struct RegisterAccountAtomicRequest { + pub account_id: AccountId, + pub allowance: u64, + // This is actually an InternalAccountId. + // TODO: rename it to internal_account_id on the relayer side + pub oauth_token: String, + pub signed_delegate_action: SignedDelegateAction, +} + pub type SendMetaTxRequest = SignedDelegateAction; #[derive(Clone, Debug, Serialize, Deserialize)] From 2240b3135ea4a3d5c9364b54ce793d1dcc08183f Mon Sep 17 00:00:00 2001 From: Phuong Nguyen Date: Mon, 18 Sep 2023 08:41:36 -0700 Subject: [PATCH 03/10] Rebase off of latest relayer --- integration-tests/src/containers.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/integration-tests/src/containers.rs b/integration-tests/src/containers.rs index b72ed0cdf..0f222bbfd 100644 --- a/integration-tests/src/containers.rs +++ b/integration-tests/src/containers.rs @@ -276,12 +276,12 @@ impl<'a> Relayer<'a> { .with_env_var("RELAYER_RPC_URL", near_rpc) .with_env_var("RELAYER_ACCOUNT_ID", relayer_account_id.to_string()) .with_env_var("REDIS_HOST", redis_hostname) + .with_env_var("OVERRIDE_RPC_CONF", "true") .with_env_var("PUBLIC_KEY", relayer_account_sk.public_key().to_string()) .with_env_var("PRIVATE_KEY", relayer_account_sk.to_string()) - .with_env_var( - "RELAYER_WHITELISTED_CONTRACT", - creator_account_id.to_string(), - ) + .with_env_var("KEYS_FILENAMES", format!("{relayer_account_id}.json")) + .with_env_var("WHITELISTED_CONTRACT", creator_account_id.to_string()) + .with_env_var("WHITELISTED_RECEIVER_IDS", creator_account_id.to_string()) .with_env_var("CUSTOM_SOCIAL_DB_ID", social_db_id.to_string()) .with_env_var("STORAGE_ACCOUNT_ID", social_account_id.to_string()) .with_env_var( From ac787a778d7ca2e3a6616b96edf0065ae9215ce4 Mon Sep 17 00:00:00 2001 From: Serhii Volovyk Date: Tue, 19 Sep 2023 13:34:42 +0300 Subject: [PATCH 04/10] NUM_KEYS set to 1 --- integration-tests/src/containers.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/integration-tests/src/containers.rs b/integration-tests/src/containers.rs index 0f222bbfd..803b066da 100644 --- a/integration-tests/src/containers.rs +++ b/integration-tests/src/containers.rs @@ -288,7 +288,9 @@ impl<'a> Relayer<'a> { "STORAGE_PUBLIC_KEY", social_account_sk.public_key().to_string(), ) - .with_env_var("STORAGE_PRIVATE_KEY", social_account_sk.to_string()); + .with_env_var("STORAGE_PRIVATE_KEY", social_account_sk.to_string()) + .with_env_var("NUM_KEYS", "1"); + let image: RunnableImage = image.into(); let image = image.with_network(network); let container = docker_client.cli.run(image); From a50c2f946825f31d5795dfc90ff0ccb3d64aa583 Mon Sep 17 00:00:00 2001 From: Serhii Volovyk Date: Tue, 19 Sep 2023 15:19:38 +0300 Subject: [PATCH 05/10] atomic url fixed --- mpc-recovery/src/leader_node/mod.rs | 6 +++--- mpc-recovery/src/relayer/mod.rs | 8 ++++---- mpc-recovery/src/relayer/msg.rs | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/mpc-recovery/src/leader_node/mod.rs b/mpc-recovery/src/leader_node/mod.rs index f7a1abf3a..1520d23c0 100644 --- a/mpc-recovery/src/leader_node/mod.rs +++ b/mpc-recovery/src/leader_node/mod.rs @@ -7,7 +7,7 @@ use crate::msg::{ SignRequest, SignResponse, UserCredentialsRequest, UserCredentialsResponse, }; use crate::oauth::OAuthTokenVerifier; -use crate::relayer::msg::{RegisterAccountAtomicRequest, RegisterAccountRequest}; +use crate::relayer::msg::{CreateAccountAtomicRequest, RegisterAccountRequest}; use crate::relayer::NearRpcAndRelayerClient; use crate::transaction::{ get_create_account_delegate_action, get_local_signed_delegated_action, get_mpc_signature, @@ -401,7 +401,7 @@ async fn process_new_account( ); // Send delegate action to relayer - let request = RegisterAccountAtomicRequest { + let request = CreateAccountAtomicRequest { account_id: new_user_account_id.clone(), allowance: 300_000_000_000_000, oauth_token: internal_acc_id.clone(), @@ -415,7 +415,7 @@ async fn process_new_account( let result = state .client - .register_account_atomic(request, partner.relayer) + .create_account_atomic(request, partner.relayer) .await; if let Err(err) = &result { diff --git a/mpc-recovery/src/relayer/mod.rs b/mpc-recovery/src/relayer/mod.rs index 2639c5ac8..41bcb07af 100644 --- a/mpc-recovery/src/relayer/mod.rs +++ b/mpc-recovery/src/relayer/mod.rs @@ -11,7 +11,7 @@ use near_primitives::views::FinalExecutionStatus; use self::error::RelayerError; use self::msg::{ - RegisterAccountAtomicRequest, RegisterAccountRequest, SendMetaTxRequest, SendMetaTxResponse, + CreateAccountAtomicRequest, RegisterAccountRequest, SendMetaTxRequest, SendMetaTxResponse, }; use crate::firewall::allowed::DelegateActionRelayer; @@ -98,14 +98,14 @@ impl NearRpcAndRelayerClient { } #[tracing::instrument(level = "debug", skip_all, fields(account_id = request.account_id.to_string()))] - pub async fn register_account_atomic( + pub async fn create_account_atomic( &self, - request: RegisterAccountAtomicRequest, + request: CreateAccountAtomicRequest, relayer: DelegateActionRelayer, ) -> Result { let mut req = Request::builder() .method(Method::POST) - .uri(format!("{}/register_account_atomic", relayer.url)) + .uri(format!("{}/create_account_atomic", relayer.url)) .header("content-type", "application/json"); if let Some(api_key) = relayer.api_key { diff --git a/mpc-recovery/src/relayer/msg.rs b/mpc-recovery/src/relayer/msg.rs index a20b082e2..9357814cd 100644 --- a/mpc-recovery/src/relayer/msg.rs +++ b/mpc-recovery/src/relayer/msg.rs @@ -15,7 +15,7 @@ pub struct RegisterAccountRequest { } #[derive(Clone, Debug, Serialize, Deserialize)] -pub struct RegisterAccountAtomicRequest { +pub struct CreateAccountAtomicRequest { pub account_id: AccountId, pub allowance: u64, // This is actually an InternalAccountId. From 0c666a2fc9e9509456dc0fdbff1182d3b0b10b4a Mon Sep 17 00:00:00 2001 From: Serhii Volovyk Date: Tue, 19 Sep 2023 17:16:09 +0300 Subject: [PATCH 06/10] return type in atomic acc creation fixed --- integration-tests/tests/mpc/negative.rs | 24 +++++++++++++++--------- mpc-recovery/src/leader_node/mod.rs | 24 +++++++----------------- mpc-recovery/src/relayer/mod.rs | 21 +++------------------ 3 files changed, 25 insertions(+), 44 deletions(-) diff --git a/integration-tests/tests/mpc/negative.rs b/integration-tests/tests/mpc/negative.rs index 104b5d10e..f452ccd06 100644 --- a/integration-tests/tests/mpc/negative.rs +++ b/integration-tests/tests/mpc/negative.rs @@ -651,15 +651,21 @@ async fn test_creation_of_two_account_with_the_same_oidc_should_not_be_possible( let user_public_key = user_secret_key.public_key(); let oidc_token = OidcToken::random(); - register_account( - &ctx, - &account_id, - &user_secret_key, - &user_public_key, - &oidc_token, - None, - ) - .await?; + ctx.leader_node + .claim_oidc_with_helper(&oidc_token, &user_public_key, &user_secret_key) + .await? + .assert_ok()?; + + ctx.leader_node + .new_account_with_helper( + &account_id, + &user_public_key, + None, + &user_secret_key, + &oidc_token, + ) + .await? + .assert_ok()?; ctx.leader_node .new_account_with_helper( diff --git a/mpc-recovery/src/leader_node/mod.rs b/mpc-recovery/src/leader_node/mod.rs index 1520d23c0..017ccb039 100644 --- a/mpc-recovery/src/leader_node/mod.rs +++ b/mpc-recovery/src/leader_node/mod.rs @@ -32,7 +32,6 @@ use near_crypto::SecretKey; use near_primitives::delegate_action::{DelegateAction, NonDelegateAction}; use near_primitives::transaction::{Action, DeleteKeyAction}; use near_primitives::types::AccountId; -use near_primitives::views::FinalExecutionStatus; use prometheus::{Encoder, TextEncoder}; use rand::{distributions::Alphanumeric, Rng}; use std::net::SocketAddr; @@ -422,7 +421,7 @@ async fn process_new_account( let err_str = format!("{:?}", err); state .client - .invalidate_cache_if_tx_failed( + .invalidate_cache_if_acc_creation_failed( &( state.account_creator_id.clone(), state.account_creator_sk.public_key(), @@ -431,21 +430,12 @@ async fn process_new_account( ) .await; } - let response = result?; - - // TODO: Probably need to check more fields - if matches!(response.status, FinalExecutionStatus::SuccessValue(_)) { - Ok(NewAccountResponse::Ok { - create_account_options: new_account_options, - user_recovery_public_key: mpc_user_recovery_pk, - near_account_id: new_user_account_id.clone(), - }) - } else { - Err(LeaderNodeError::Other(anyhow::anyhow!( - "transaction failed with {:?}", - response.status - ))) - } + + Ok(NewAccountResponse::Ok { + create_account_options: new_account_options, + user_recovery_public_key: mpc_user_recovery_pk, + near_account_id: new_user_account_id.clone(), + }) }) .await } diff --git a/mpc-recovery/src/relayer/mod.rs b/mpc-recovery/src/relayer/mod.rs index 41bcb07af..5b2c0f696 100644 --- a/mpc-recovery/src/relayer/mod.rs +++ b/mpc-recovery/src/relayer/mod.rs @@ -102,7 +102,7 @@ impl NearRpcAndRelayerClient { &self, request: CreateAccountAtomicRequest, relayer: DelegateActionRelayer, - ) -> Result { + ) -> Result<(), RelayerError> { let mut req = Request::builder() .method(Method::POST) .uri(format!("{}/create_account_atomic", relayer.url)) @@ -135,22 +135,7 @@ impl NearRpcAndRelayerClient { if status.is_success() { tracing::debug!(response_body = msg, "got response"); - let response: SendMetaTxResponse = serde_json::from_slice(&response_body) - .map_err(|e| RelayerError::DataConversionFailure(e.into()))?; - match response.status { - FinalExecutionStatus::NotStarted | FinalExecutionStatus::Started => { - Err(RelayerError::TxNotReady) - } - FinalExecutionStatus::Failure(e) => Err(RelayerError::TxExecutionFailure(e)), - FinalExecutionStatus::SuccessValue(ref value) => { - tracing::debug!( - value = std::str::from_utf8(value) - .map_err(|e| RelayerError::DataConversionFailure(e.into()))?, - "success" - ); - Ok(response) - } - } + Ok(()) } else { Err(RelayerError::RequestFailure(status, msg.to_string())) } @@ -210,7 +195,7 @@ impl NearRpcAndRelayerClient { } } - pub(crate) async fn invalidate_cache_if_tx_failed( + pub(crate) async fn invalidate_cache_if_acc_creation_failed( &self, cache_key: &(AccountId, PublicKey), err_str: &str, From 5cb22104ce957c1db5967b91920050092bbb6f71 Mon Sep 17 00:00:00 2001 From: Serhii Volovyk Date: Tue, 19 Sep 2023 17:55:56 +0300 Subject: [PATCH 07/10] atomic acc creation status error fxed --- integration-tests/tests/lib.rs | 18 +++++++++- integration-tests/tests/mpc/negative.rs | 4 ++- mpc-recovery/src/leader_node/mod.rs | 45 +++++++++++++++---------- 3 files changed, 47 insertions(+), 20 deletions(-) diff --git a/integration-tests/tests/lib.rs b/integration-tests/tests/lib.rs index ffab403b3..5e7a78ce8 100644 --- a/integration-tests/tests/lib.rs +++ b/integration-tests/tests/lib.rs @@ -218,6 +218,7 @@ trait MpcCheck { fn assert_bad_request_contains(self, expected: &str) -> anyhow::Result; fn assert_unauthorized_contains(self, expected: &str) -> anyhow::Result; fn assert_internal_error_contains(self, expected: &str) -> anyhow::Result; + fn assert_dependency_error_contains(self, expected: &str) -> anyhow::Result; fn assert_bad_request(self) -> anyhow::Result where @@ -311,7 +312,7 @@ macro_rules! impl_mpc_check { if status_code == StatusCode::INTERNAL_SERVER_ERROR { let $response::Err { ref msg, .. } = response else { - anyhow::bail!("unexpected Ok with a 500 http code"); + anyhow::bail!("unexpected error with a 401 http code"); }; assert!(msg.contains(expected)); @@ -320,6 +321,21 @@ macro_rules! impl_mpc_check { anyhow::bail!("expected 401, but got {status_code} with response: {response:?}"); } } + fn assert_dependency_error_contains(self, expected: &str) -> anyhow::Result { + let status_code = self.0; + let response = self.1; + + if status_code == StatusCode::FAILED_DEPENDENCY { + let $response::Err { ref msg, .. } = response else { + anyhow::bail!("unexpected error with a 424 http code"); + }; + assert!(msg.contains(expected)); + + Ok(response) + } else { + anyhow::bail!("expected 424, but got {status_code} with response: {response:?}"); + } + } } }; } diff --git a/integration-tests/tests/mpc/negative.rs b/integration-tests/tests/mpc/negative.rs index f452ccd06..b3323a293 100644 --- a/integration-tests/tests/mpc/negative.rs +++ b/integration-tests/tests/mpc/negative.rs @@ -676,7 +676,9 @@ async fn test_creation_of_two_account_with_the_same_oidc_should_not_be_possible( &oidc_token, ) .await? - .assert_internal_error_contains("Yu can only register 1 account per oath_token")?; + .assert_dependency_error_contains( + "You can only register 1 account per oauth_token", + )?; Ok(()) }) diff --git a/mpc-recovery/src/leader_node/mod.rs b/mpc-recovery/src/leader_node/mod.rs index 017ccb039..2898096d9 100644 --- a/mpc-recovery/src/leader_node/mod.rs +++ b/mpc-recovery/src/leader_node/mod.rs @@ -417,25 +417,34 @@ async fn process_new_account( .create_account_atomic(request, partner.relayer) .await; - if let Err(err) = &result { - let err_str = format!("{:?}", err); - state - .client - .invalidate_cache_if_acc_creation_failed( - &( - state.account_creator_id.clone(), - state.account_creator_sk.public_key(), - ), - &err_str, - ) - .await; + match result { + Ok(_) => { + tracing::info!( + "account creation succeeded: {new_user_account_id:?}", + new_user_account_id = new_user_account_id + ); + Ok(NewAccountResponse::Ok { + create_account_options: new_account_options, + user_recovery_public_key: mpc_user_recovery_pk, + near_account_id: new_user_account_id.clone(), + }) + } + Err(err) => { + tracing::error!("account creation failed: {err}"); + let err_str = format!("{:?}", err); + state + .client + .invalidate_cache_if_acc_creation_failed( + &( + state.account_creator_id.clone(), + state.account_creator_sk.public_key(), + ), + &err_str, + ) + .await; + return Err(LeaderNodeError::RelayerError(err)); + } } - - Ok(NewAccountResponse::Ok { - create_account_options: new_account_options, - user_recovery_public_key: mpc_user_recovery_pk, - near_account_id: new_user_account_id.clone(), - }) }) .await } From d5159bc04617ad26a5e862b6fdfefc2a46ddcf69 Mon Sep 17 00:00:00 2001 From: Serhii Volovyk Date: Tue, 19 Sep 2023 18:03:22 +0300 Subject: [PATCH 08/10] fmt --- integration-tests/tests/lib.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/integration-tests/tests/lib.rs b/integration-tests/tests/lib.rs index 5e7a78ce8..12fe65f94 100644 --- a/integration-tests/tests/lib.rs +++ b/integration-tests/tests/lib.rs @@ -306,13 +306,18 @@ macro_rules! impl_mpc_check { } } // ideally we should not have situations where we can get INTERNAL_SERVER_ERROR - fn assert_internal_error_contains(self, expected: &str) -> anyhow::Result { + fn assert_internal_error_contains( + self, + expected: &str + ) -> anyhow::Result { let status_code = self.0; let response = self.1; if status_code == StatusCode::INTERNAL_SERVER_ERROR { let $response::Err { ref msg, .. } = response else { - anyhow::bail!("unexpected error with a 401 http code"); + anyhow::bail!( + "unexpected error with a 401 http code" + ); }; assert!(msg.contains(expected)); @@ -321,7 +326,10 @@ macro_rules! impl_mpc_check { anyhow::bail!("expected 401, but got {status_code} with response: {response:?}"); } } - fn assert_dependency_error_contains(self, expected: &str) -> anyhow::Result { + fn assert_dependency_error_contains( + self, + expected: &str + ) -> anyhow::Result { let status_code = self.0; let response = self.1; @@ -333,7 +341,9 @@ macro_rules! impl_mpc_check { Ok(response) } else { - anyhow::bail!("expected 424, but got {status_code} with response: {response:?}"); + anyhow::bail!( + "expected 424, but got {status_code} with response: {response:?}" + ); } } } From d039d3d507a1233ecb4e2cbf6bddda0ee0b1f9e3 Mon Sep 17 00:00:00 2001 From: Serhii Volovyk Date: Tue, 19 Sep 2023 18:17:49 +0300 Subject: [PATCH 09/10] fmt2 --- integration-tests/tests/lib.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/integration-tests/tests/lib.rs b/integration-tests/tests/lib.rs index 12fe65f94..a5e1a5fd8 100644 --- a/integration-tests/tests/lib.rs +++ b/integration-tests/tests/lib.rs @@ -308,27 +308,27 @@ macro_rules! impl_mpc_check { // ideally we should not have situations where we can get INTERNAL_SERVER_ERROR fn assert_internal_error_contains( self, - expected: &str + expected: &str, ) -> anyhow::Result { let status_code = self.0; let response = self.1; if status_code == StatusCode::INTERNAL_SERVER_ERROR { let $response::Err { ref msg, .. } = response else { - anyhow::bail!( - "unexpected error with a 401 http code" - ); + anyhow::bail!("unexpected error with a 401 http code"); }; assert!(msg.contains(expected)); Ok(response) } else { - anyhow::bail!("expected 401, but got {status_code} with response: {response:?}"); + anyhow::bail!( + "expected 401, but got {status_code} with response: {response:?}" + ); } } fn assert_dependency_error_contains( self, - expected: &str + expected: &str, ) -> anyhow::Result { let status_code = self.0; let response = self.1; From 4d486dd7f9512d12fceda1211db23216bb070291 Mon Sep 17 00:00:00 2001 From: Serhii Volovyk Date: Tue, 19 Sep 2023 18:27:25 +0300 Subject: [PATCH 10/10] fmt3 --- mpc-recovery/src/leader_node/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mpc-recovery/src/leader_node/mod.rs b/mpc-recovery/src/leader_node/mod.rs index 2898096d9..2d484b04c 100644 --- a/mpc-recovery/src/leader_node/mod.rs +++ b/mpc-recovery/src/leader_node/mod.rs @@ -442,7 +442,7 @@ async fn process_new_account( &err_str, ) .await; - return Err(LeaderNodeError::RelayerError(err)); + Err(LeaderNodeError::RelayerError(err)) } } })