fix: use borsh-serialized type for sign requests (#275)

Dockerfile

@@ -12,7 +12,7 @@ COPY . .
 RUN sed -i 's#"integration-tests",##' Cargo.toml
 RUN cargo build --release --package mpc-recovery
 
-FROM debian:bullseye-slim as runtime
+FROM debian:bookworm-slim as runtime
 RUN apt-get update && apt-get install --assume-yes libssl-dev ca-certificates curl
 RUN update-ca-certificates
 COPY --from=builder /usr/src/app/target/release/mpc-recovery /usr/local/bin/mpc-recovery

README.md

@@ -111,7 +111,7 @@ signed by the key you used to claim the oidc token. This does not have to be the
 
     URL: /sign
     Request parameters: {
-        delegate_action: DelegateAction,
+        delegate_action: String, // Base64-encoded borsh serialization of DelegateAction
         oidc_token: String,
         frp_signature: Signature,
         user_credentials_frp_signature: Signature,
@@ -163,8 +163,9 @@ The expected flow for the client is next:
 1. Client uses `/user_credentials` endpoint to get the recovery PK.
 2. Client fetches latest nonce, block hash using obtained recovery PK.
 3. Client creates a delegate action with desired actions, such as add or delete key.
-4. Client gets the signature from the MPC system using `/sign` endpoint.
-5. Client sends the same delegate action to the relayer with obtained signature.
+4. Client serializes the delegate action and encodes it into Base64.
+5. Client gets the signature from the MPC system using `/sign` endpoint.
+6. Client sends the same delegate action to the relayer with obtained signature.
 
 ### Client integration
 

integration-tests/src/containers.rs

@@ -23,6 +23,7 @@ use mpc_recovery::{
 };
 use multi_party_eddsa::protocols::ExpandedKeyPair;
 use near_crypto::{PublicKey, SecretKey};
+use near_primitives::borsh::BorshSerialize;
 use near_primitives::transaction::DeleteKeyAction;
 use near_primitives::types::{BlockHeight, Nonce};
 use near_primitives::{
@@ -703,7 +704,7 @@ impl LeaderNodeApi {
         let user_credentials_frp_signature = sign_digest(&user_credentials_request_digest, frp_sk)?;
 
         let sign_request = SignRequest {
-            delegate_action: add_key_delegate_action.clone(),
+            delegate_action: add_key_delegate_action.try_to_vec()?,
             oidc_token: oidc_token.clone(),
             frp_signature,
             user_credentials_frp_signature,
@@ -759,7 +760,7 @@ impl LeaderNodeApi {
         let user_credentials_frp_signature = sign_digest(&user_credentials_request_digest, frp_sk)?;
 
         let sign_request = SignRequest {
-            delegate_action: delete_key_delegate_action.clone(),
+            delegate_action: delete_key_delegate_action.try_to_vec()?,
             oidc_token: oidc_token.clone(),
             frp_signature,
             user_credentials_frp_signature,
@@ -802,7 +803,7 @@ impl LeaderNodeApi {
         let user_credentials_frp_signature = sign_digest(&user_credentials_request_digest, frp_sk)?;
 
         let sign_request = SignRequest {
-            delegate_action: delegate_action.clone(),
+            delegate_action: delegate_action.try_to_vec()?,
             oidc_token: oidc_token.clone(),
             frp_signature,
             user_credentials_frp_signature,

integration-tests/tests/lib.rs

@@ -249,7 +249,9 @@ macro_rules! impl_mpc_check {
                     let $response::Err { .. } = response else {
                         anyhow::bail!("unexpected Ok with a non-200 http code ({status_code})");
                     };
-                    anyhow::bail!("expected 200, but got {status_code} with response: {response:?}");
+                    anyhow::bail!(
+                        "expected 200, but got {status_code} with response: {response:?}"
+                    );
                 }
             }
 
@@ -265,11 +267,16 @@ macro_rules! impl_mpc_check {
 
                     Ok(response)
                 } else {
-                    anyhow::bail!("expected 400, but got {status_code} with response: {response:?}");
+                    anyhow::bail!(
+                        "expected 400, but got {status_code} with response: {response:?}"
+                    );
                 }
             }
 
-            fn assert_unauthorized_contains(self, expected: &str) -> anyhow::Result<Self::Response> {
+            fn assert_unauthorized_contains(
+                self,
+                expected: &str,
+            ) -> anyhow::Result<Self::Response> {
                 let status_code = self.0;
                 let response = self.1;
 
@@ -281,7 +288,9 @@ macro_rules! impl_mpc_check {
 
                     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:?}"
+                    );
                 }
             }
         }

integration-tests/tests/mpc/negative.rs

@@ -480,7 +480,7 @@ async fn test_reject_new_pk_set() -> anyhow::Result<()> {
                 })
                 .await?;
             assert_eq!(status_code, StatusCode::BAD_REQUEST);
-            assert!(matches!(result, Err(_)));
+            assert!(result.is_err());
 
             Ok(())
         })

integration-tests/tests/mpc/positive.rs

@@ -178,7 +178,7 @@ async fn test_accept_existing_pk_set() -> anyhow::Result<()> {
                 })
                 .await?;
             assert_eq!(status_code, StatusCode::OK);
-            assert!(matches!(result, Ok(_)));
+            assert!(result.is_ok());
 
             Ok(())
         })

mpc-recovery/Cargo.toml

@@ -31,6 +31,7 @@ rand8 = { package = "rand", version = "0.8" }
 reqwest = { version = "0.11.16", features = ["blocking"] }
 serde = { version = "1", features = ["derive"] }
 serde_json = "1"
+serde_with = "3.3.0"
 thiserror = "1"
 tokio = { version = "1.28", features = ["full"] }
 tokio-retry = "0.3"

mpc-recovery/src/error.rs

@@ -64,6 +64,8 @@ pub enum LeaderNodeError {
     AggregateSigningFailed(#[from] AggregateSigningError),
     #[error("malformed account id: {0}")]
     MalformedAccountId(String, ParseAccountError),
+    #[error("malformed delegate action: {0}")]
+    MalformedDelegateAction(std::io::Error),
     #[error("failed to verify signature: {0}")]
     SignatureVerificationFailed(anyhow::Error),
     #[error("failed to verify oidc token: {0}")]
@@ -92,6 +94,7 @@ impl LeaderNodeError {
             LeaderNodeError::SignatureVerificationFailed(_) => StatusCode::BAD_REQUEST,
             LeaderNodeError::OidcVerificationFailed(_) => StatusCode::UNAUTHORIZED,
             LeaderNodeError::MalformedAccountId(_, _) => StatusCode::BAD_REQUEST,
+            LeaderNodeError::MalformedDelegateAction(_) => StatusCode::BAD_REQUEST,
             LeaderNodeError::RelayerError(_) => StatusCode::FAILED_DEPENDENCY,
             LeaderNodeError::TimeoutGatheringPublicKeys => StatusCode::INTERNAL_SERVER_ERROR,
             LeaderNodeError::RecoveryKeyCanNotBeDeleted(_) => StatusCode::BAD_REQUEST,

mpc-recovery/src/leader_node/mod.rs

@@ -25,9 +25,10 @@ use axum::{
     Extension, Json, Router,
 };
 use axum_extra::extract::WithRejection;
+use borsh::BorshDeserialize;
 use curv::elliptic::curves::{Ed25519, Point};
 use near_crypto::SecretKey;
-use near_primitives::delegate_action::NonDelegateAction;
+use near_primitives::delegate_action::{DelegateAction, NonDelegateAction};
 use near_primitives::transaction::{Action, DeleteKeyAction};
 use near_primitives::types::AccountId;
 use near_primitives::views::FinalExecutionStatus;
@@ -469,13 +470,17 @@ async fn process_sign<T: OAuthTokenVerifier>(
     state: LeaderState,
     request: SignRequest,
 ) -> Result<SignResponse, LeaderNodeError> {
+    // Deserialize the included delegate action via borsh
+    let delegate_action = DelegateAction::try_from_slice(&request.delegate_action)
+        .map_err(LeaderNodeError::MalformedDelegateAction)?;
+
     // Check OIDC token
     T::verify_token(&request.oidc_token, &state.pagoda_firebase_audience_id)
         .await
         .map_err(LeaderNodeError::OidcVerificationFailed)?;
 
     // Prevent recovery key delition
-    let requested_delegate_actions: &Vec<NonDelegateAction> = &request.delegate_action.actions;
+    let requested_delegate_actions: &Vec<NonDelegateAction> = &delegate_action.actions;
 
     let requested_actions: &Vec<Action> = &requested_delegate_actions
         .iter()
@@ -527,7 +532,7 @@ async fn process_sign<T: OAuthTokenVerifier>(
             &state.reqwest_client,
             &state.sign_nodes,
             &request.oidc_token,
-            request.delegate_action.clone(),
+            delegate_action.clone(),
             request.frp_signature,
             &request.frp_public_key,
         )

mpc-recovery/src/msg.rs

@@ -1,11 +1,12 @@
+use crate::sign_node::oidc::{OidcHash, OidcToken};
+use crate::transaction::CreateAccountOptions;
 use curv::elliptic::curves::{Ed25519, Point};
 use ed25519_dalek::Signature;
 use near_primitives::delegate_action::DelegateAction;
 use near_primitives::types::AccountId;
 use serde::{Deserialize, Serialize};
-
-use crate::sign_node::oidc::{OidcHash, OidcToken};
-use crate::transaction::CreateAccountOptions;
+use serde_with::base64::Base64;
+use serde_with::serde_as;
 
 #[derive(Serialize, Deserialize, Debug, Clone)]
 pub struct MpcPkRequest {}
@@ -116,9 +117,11 @@ impl NewAccountResponse {
     }
 }
 
+#[serde_as]
 #[derive(Serialize, Deserialize, Debug, Clone)]
 pub struct SignRequest {
-    pub delegate_action: DelegateAction,
+    #[serde_as(as = "Base64")]
+    pub delegate_action: Vec<u8>,
     pub oidc_token: OidcToken,
     #[serde(with = "hex_signature")]
     pub frp_signature: Signature,