Atomic account creation

integration-tests/src/containers.rs

@@ -276,19 +276,21 @@ 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(
                 "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<GenericImage> = image.into();
         let image = image.with_network(network);
         let container = docker_client.cli.run(image);

integration-tests/tests/lib.rs

@@ -217,6 +217,8 @@ trait MpcCheck {
     fn assert_ok(self) -> anyhow::Result<Self::Response>;
     fn assert_bad_request_contains(self, expected: &str) -> anyhow::Result<Self::Response>;
     fn assert_unauthorized_contains(self, expected: &str) -> anyhow::Result<Self::Response>;
+    fn assert_internal_error_contains(self, expected: &str) -> anyhow::Result<Self::Response>;
+    fn assert_dependency_error_contains(self, expected: &str) -> anyhow::Result<Self::Response>;
 
     fn assert_bad_request(self) -> anyhow::Result<Self::Response>
     where
@@ -230,6 +232,12 @@ trait MpcCheck {
     {
         self.assert_unauthorized_contains("")
     }
+    fn assert_internal_error(self) -> anyhow::Result<Self::Response>
+    where
+        Self: Sized,
+    {
+        self.assert_internal_error_contains("")
+    }
 }
 
 // Presumes that $response::Err has a `msg: String` field.
@@ -297,6 +305,47 @@ 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<Self::Response> {
+                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");
+                    };
+                    assert!(msg.contains(expected));
+
+                    Ok(response)
+                } else {
+                    anyhow::bail!(
+                        "expected 401, but got {status_code} with response: {response:?}"
+                    );
+                }
+            }
+            fn assert_dependency_error_contains(
+                self,
+                expected: &str,
+            ) -> anyhow::Result<Self::Response> {
+                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:?}"
+                    );
+                }
+            }
         }
     };
 }

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.clone(),
-            //         None,
-            //         user_secret_key.clone(),
-            //         oidc_token_1.clone(),
-            //     )
-            //     .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
@@ -605,3 +603,85 @@ 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 = OidcToken::random();
+
+            ctx.leader_node
+                .new_account_with_helper(
+                    &account_id,
+                    &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 = OidcToken::random();
+
+            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(
+                    &account_id_2,
+                    &user_public_key,
+                    None,
+                    &user_secret_key,
+                    &oidc_token,
+                )
+                .await?
+                .assert_dependency_error_contains(
+                    "You can only register 1 account per oauth_token",
+                )?;
+
+            Ok(())
+        })
+    })
+    .await
+}

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::{CreateAccountAtomicRequest, RegisterAccountRequest};
 use crate::relayer::NearRpcAndRelayerClient;
 use crate::transaction::{
     get_create_account_delegate_action, get_local_signed_delegated_action, get_mpc_signature,
@@ -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;
@@ -356,22 +355,6 @@ async fn process_new_account<T: OAuthTokenVerifier>(
         .map_err(LeaderNodeError::SignatureVerificationFailed)?;
     tracing::debug!("user credentials digest signature verified for {new_user_account_id:?}");
 
-    let partner = state
-        .partners
-        .find(&oidc_token_claims.iss, &oidc_token_claims.aud)?;
-
-    state
-        .client
-        .register_account(
-            RegisterAccountRequest {
-                account_id: new_user_account_id.clone(),
-                allowance: 300_000_000_000_000,
-                oauth_token: internal_acc_id.clone(),
-            },
-            partner.relayer,
-        )
-        .await?;
-
     nar::retry(|| async {
         // Get nonce and recent block hash
         let (_hash, block_height, nonce) = state
@@ -417,40 +400,50 @@ async fn process_new_account<T: OAuthTokenVerifier>(
         );
 
         // Send delegate action to relayer
+        let request = CreateAccountAtomicRequest {
+            account_id: new_user_account_id.clone(),
+            allowance: 300_000_000_000_000,
+            oauth_token: internal_acc_id.clone(),
+            signed_delegate_action,
+        };
+
+        // TODO: move error message from here to this place
         let partner = state
             .partners
             .find(&oidc_token_claims.iss, &oidc_token_claims.aud)?;
+
         let result = state
             .client
-            .send_meta_tx(signed_delegate_action, partner.relayer)
+            .create_account_atomic(request, partner.relayer)
             .await;
-        if let Err(err) = &result {
-            let err_str = format!("{:?}", err);
-            state
-                .client
-                .invalidate_cache_if_tx_failed(
-                    &(
-                        state.account_creator_id.clone(),
-                        state.account_creator_sk.public_key(),
-                    ),
-                    &err_str,
-                )
-                .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
-            )))
+
+        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;
+                Err(LeaderNodeError::RelayerError(err))
+            }
         }
     })
     .await