Simplify Wallet Association By Using Installation Keys (#1314)

* wip

* wip

* sign with installation

* cleanup

* naming

* more cleanup

* cleanup

* cleanup

* getting closer

* test fix

* remove a param

* cleanup

* fix js tests

* unnecessary clone

bindings_ffi/src/mls.rs

@@ -472,15 +472,15 @@ impl FfiXmtpClient {
         Ok(())
     }
 
-    /// Adds an identity - really a wallet address - to the existing client
+    /// Adds a wallet address to the existing client
     pub async fn add_wallet(
         &self,
-        existing_wallet_address: &str,
         new_wallet_address: &str,
     ) -> Result<Arc<FfiSignatureRequest>, GenericError> {
         let signature_request = self
             .inner_client
-            .associate_wallet(existing_wallet_address.into(), new_wallet_address.into())?;
+            .associate_wallet(new_wallet_address.into())
+            .await?;
         let scw_verifier = self.inner_client.scw_verifier().clone();
         let request = Arc::new(FfiSignatureRequest {
             inner: Arc::new(tokio::sync::Mutex::new(signature_request)),
@@ -2172,27 +2172,28 @@ mod tests {
         assert!(result_errored, "did not error on wrong encryption key")
     }
 
+    trait SignWithWallet {
+        async fn add_wallet_signature(&self, wallet: &xmtp_cryptography::utils::LocalWallet);
+    }
+
     use super::FfiSignatureRequest;
-    async fn sign_with_wallet(
-        wallet: &xmtp_cryptography::utils::LocalWallet,
-        signature_request: &FfiSignatureRequest,
-    ) {
-        let scw_verifier = signature_request.scw_verifier.clone();
-        let signature_text = signature_request.inner.lock().await.signature_text();
-        let wallet_signature: Vec<u8> = wallet.sign(&signature_text.clone()).unwrap().into();
+    impl SignWithWallet for FfiSignatureRequest {
+        async fn add_wallet_signature(&self, wallet: &xmtp_cryptography::utils::LocalWallet) {
+            let signature_text = self.inner.lock().await.signature_text();
+            let wallet_signature: Vec<u8> = wallet.sign(&signature_text.clone()).unwrap().into();
 
-        signature_request
-            .inner
-            .lock()
-            .await
-            .add_signature(
-                UnverifiedSignature::RecoverableEcdsa(UnverifiedRecoverableEcdsaSignature::new(
-                    wallet_signature,
-                )),
-                scw_verifier,
-            )
-            .await
-            .unwrap();
+            self.inner
+                .lock()
+                .await
+                .add_signature(
+                    UnverifiedSignature::RecoverableEcdsa(
+                        UnverifiedRecoverableEcdsaSignature::new(wallet_signature),
+                    ),
+                    &self.scw_verifier,
+                )
+                .await
+                .unwrap();
+        }
     }
 
     use xmtp_cryptography::utils::generate_local_wallet;
@@ -2224,7 +2225,9 @@ mod tests {
         let signature_request = client.signature_request().unwrap().clone();
         register_client(&ffi_inbox_owner, &client).await;
 
-        sign_with_wallet(&ffi_inbox_owner.wallet, &signature_request).await;
+        signature_request
+            .add_wallet_signature(&ffi_inbox_owner.wallet)
+            .await;
 
         let conn = client.inner_client.store().conn().unwrap();
         let state = client
@@ -2236,18 +2239,16 @@ mod tests {
         assert_eq!(state.members().len(), 2);
 
         // Now, add the second wallet to the client
-
         let wallet_to_add = generate_local_wallet();
         let new_account_address = wallet_to_add.get_address();
         println!("second address: {}", new_account_address);
 
         let signature_request = client
-            .add_wallet(&ffi_inbox_owner.get_address(), &new_account_address)
+            .add_wallet(&new_account_address)
             .await
             .expect("could not add wallet");
 
-        sign_with_wallet(&ffi_inbox_owner.wallet, &signature_request).await;
-        sign_with_wallet(&wallet_to_add, &signature_request).await;
+        signature_request.add_wallet_signature(&wallet_to_add).await;
 
         client
             .apply_signature_request(signature_request)
@@ -2290,7 +2291,9 @@ mod tests {
         let signature_request = client.signature_request().unwrap().clone();
         register_client(&ffi_inbox_owner, &client).await;
 
-        sign_with_wallet(&ffi_inbox_owner.wallet, &signature_request).await;
+        signature_request
+            .add_wallet_signature(&ffi_inbox_owner.wallet)
+            .await;
 
         let conn = client.inner_client.store().conn().unwrap();
         let state = client
@@ -2308,12 +2311,11 @@ mod tests {
         println!("second address: {}", new_account_address);
 
         let signature_request = client
-            .add_wallet(&ffi_inbox_owner.get_address(), &new_account_address)
+            .add_wallet(&new_account_address)
             .await
             .expect("could not add wallet");
 
-        sign_with_wallet(&ffi_inbox_owner.wallet, &signature_request).await;
-        sign_with_wallet(&wallet_to_add, &signature_request).await;
+        signature_request.add_wallet_signature(&wallet_to_add).await;
 
         client
             .apply_signature_request(signature_request.clone())
@@ -2334,7 +2336,9 @@ mod tests {
             .await
             .expect("could not revoke wallet");
 
-        sign_with_wallet(&ffi_inbox_owner.wallet, &signature_request).await;
+        signature_request
+            .add_wallet_signature(&ffi_inbox_owner.wallet)
+            .await;
 
         client
             .apply_signature_request(signature_request)
@@ -3835,7 +3839,7 @@ mod tests {
         assert_eq!(client_2_state.installations.len(), 2);
 
         let signature_request = client_1.revoke_all_other_installations().await.unwrap();
-        sign_with_wallet(&wallet, &signature_request).await;
+        signature_request.add_wallet_signature(&wallet).await;
         client_1
             .apply_signature_request(signature_request)
             .await

bindings_node/src/signatures.rs

@@ -35,17 +35,11 @@ impl Client {
   }
 
   #[napi]
-  pub async fn add_wallet_signature_text(
-    &self,
-    existing_wallet_address: String,
-    new_wallet_address: String,
-  ) -> Result<String> {
+  pub async fn add_wallet_signature_text(&self, new_wallet_address: String) -> Result<String> {
     let signature_request = self
       .inner_client()
-      .associate_wallet(
-        existing_wallet_address.to_lowercase(),
-        new_wallet_address.to_lowercase(),
-      )
+      .associate_wallet(new_wallet_address.to_lowercase())
+      .await
       .map_err(ErrorWrapper::from)?;
     let signature_text = signature_request.signature_text();
     let mut signature_requests = self.signature_requests().lock().await;

bindings_node/test/Client.test.ts

@@ -64,23 +64,14 @@ describe('Client', () => {
     const user2 = createUser()
     const client = await createRegisteredClient(user)
     const signatureText = await client.addWalletSignatureText(
-      user.account.address,
       user2.account.address
     )
     expect(signatureText).toBeDefined()
 
-    // sign message
-    const signature = await user.wallet.signMessage({
-      message: signatureText,
-    })
     const signature2 = await user2.wallet.signMessage({
       message: signatureText,
     })
 
-    await client.addSignature(
-      SignatureRequestType.AddWallet,
-      toBytes(signature)
-    )
     await client.addSignature(
       SignatureRequestType.AddWallet,
       toBytes(signature2)
@@ -101,23 +92,15 @@ describe('Client', () => {
     const user2 = createUser()
     const client = await createRegisteredClient(user)
     const signatureText = await client.addWalletSignatureText(
-      user.account.address,
       user2.account.address
     )
     expect(signatureText).toBeDefined()
 
     // sign message
-    const signature = await user.wallet.signMessage({
-      message: signatureText,
-    })
     const signature2 = await user2.wallet.signMessage({
       message: signatureText,
     })
 
-    await client.addSignature(
-      SignatureRequestType.AddWallet,
-      toBytes(signature)
-    )
     await client.addSignature(
       SignatureRequestType.AddWallet,
       toBytes(signature2)

bindings_wasm/src/signatures.rs

@@ -36,15 +36,12 @@ impl Client {
   #[wasm_bindgen(js_name = addWalletSignatureText)]
   pub async fn add_wallet_signature_text(
     &self,
-    existing_wallet_address: String,
     new_wallet_address: String,
   ) -> Result<String, JsError> {
     let signature_request = self
       .inner_client()
-      .associate_wallet(
-        existing_wallet_address.to_lowercase(),
-        new_wallet_address.to_lowercase(),
-      )
+      .associate_wallet(new_wallet_address.to_lowercase())
+      .await
       .map_err(|e| JsError::new(format!("{}", e).as_str()))?;
     let signature_text = signature_request.signature_text();
     let mut signature_requests = self.signature_requests().lock().await;

xmtp_id/src/associations/builder.rs

@@ -2,7 +2,7 @@
 //! resolved into an [`IdentityUpdate`]. An [`IdentityUpdate`] may be used for updating the state
 //! of an XMTP ID according to [XIP-46](https://github.com/xmtp/XIPs/pull/53)
 
-use std::collections::{HashMap, HashSet};
+use std::collections::HashMap;
 
 use crate::{scw_verifier::SmartContractSignatureVerifier, utils::now_ns};
 use thiserror::Error;
@@ -194,29 +194,18 @@ impl SignatureRequest {
         }
     }
 
-    pub fn missing_signatures(&self) -> Vec<MemberIdentifier> {
-        let signers: HashSet<MemberIdentifier> = self
-            .pending_actions
+    pub fn missing_signatures(&self) -> Vec<&MemberIdentifier> {
+        self.pending_actions
             .iter()
-            .flat_map(|pending_action| {
-                pending_action
-                    .pending_signatures
-                    .values()
-                    .cloned()
-                    .collect::<Vec<MemberIdentifier>>()
-            })
-            .collect();
-
-        let signatures: HashSet<MemberIdentifier> = self.signatures.keys().cloned().collect();
-
-        signers.difference(&signatures).cloned().collect()
+            .flat_map(|pending_action| pending_action.pending_signatures.values())
+            .filter(|ident| !self.signatures.contains_key(ident))
+            .collect()
     }
 
-    pub fn missing_address_signatures(&self) -> Vec<MemberIdentifier> {
+    pub fn missing_address_signatures(&self) -> Vec<&MemberIdentifier> {
         self.missing_signatures()
-            .iter()
+            .into_iter()
             .filter(|member| member.kind() == MemberKind::Address)
-            .cloned()
             .collect()
     }
 
@@ -277,7 +266,7 @@ impl SignatureRequest {
             "adding verified signature");
 
         // Make sure the signer is someone actually in the request
-        if !missing_signatures.contains(signer_identity) {
+        if !missing_signatures.contains(&signer_identity) {
             return Err(SignatureRequestError::UnknownSigner);
         }
 

xmtp_id/src/associations/member.rs

@@ -1,3 +1,4 @@
+use ed25519_dalek::VerifyingKey;
 use xmtp_cryptography::XmtpInstallationCredential;
 
 #[derive(Clone, Debug, PartialEq)]
@@ -106,6 +107,12 @@ impl From<Vec<u8>> for MemberIdentifier {
     }
 }
 
+impl From<VerifyingKey> for MemberIdentifier {
+    fn from(installation: VerifyingKey) -> Self {
+        installation.as_bytes().to_vec().into()
+    }
+}
+
 impl<'a> From<&'a XmtpInstallationCredential> for MemberIdentifier {
     fn from(cred: &'a XmtpInstallationCredential) -> MemberIdentifier {
         MemberIdentifier::Installation(cred.public_slice().to_vec())