diff --git a/bindings_ffi/src/mls.rs b/bindings_ffi/src/mls.rs index 7ab18fe12..f8c9ab19a 100644 --- a/bindings_ffi/src/mls.rs +++ b/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, 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 = 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 = 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 diff --git a/bindings_node/src/signatures.rs b/bindings_node/src/signatures.rs index cfc67a81f..709c374ec 100644 --- a/bindings_node/src/signatures.rs +++ b/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 { + pub async fn add_wallet_signature_text(&self, new_wallet_address: String) -> Result { 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; diff --git a/bindings_node/test/Client.test.ts b/bindings_node/test/Client.test.ts index 063b0f603..8942e7b25 100644 --- a/bindings_node/test/Client.test.ts +++ b/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) diff --git a/bindings_wasm/src/signatures.rs b/bindings_wasm/src/signatures.rs index 7c9f27ac7..9fde22378 100644 --- a/bindings_wasm/src/signatures.rs +++ b/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 { 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; diff --git a/xmtp_id/src/associations/builder.rs b/xmtp_id/src/associations/builder.rs index b1e689411..43c0d166a 100644 --- a/xmtp_id/src/associations/builder.rs +++ b/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 { - let signers: HashSet = 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::>() - }) - .collect(); - - let signatures: HashSet = 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 { + 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); } diff --git a/xmtp_id/src/associations/member.rs b/xmtp_id/src/associations/member.rs index 8c7d5cab7..40b162e73 100644 --- a/xmtp_id/src/associations/member.rs +++ b/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> for MemberIdentifier { } } +impl From 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()) diff --git a/xmtp_mls/src/identity_updates.rs b/xmtp_mls/src/identity_updates.rs index 83c0b9c5f..23d08e69a 100644 --- a/xmtp_mls/src/identity_updates.rs +++ b/xmtp_mls/src/identity_updates.rs @@ -7,6 +7,7 @@ use crate::{ }; use futures::future::try_join_all; use thiserror::Error; +use xmtp_cryptography::CredentialSign; use xmtp_id::{ associations::{ apply_update, @@ -15,8 +16,8 @@ use xmtp_id::{ unverified::{ UnverifiedIdentityUpdate, UnverifiedInstallationKeySignature, UnverifiedSignature, }, - AssociationError, AssociationState, AssociationStateDiff, IdentityUpdate, MemberIdentifier, - SignatureError, + AssociationError, AssociationState, AssociationStateDiff, IdentityUpdate, + InstallationKeyContext, MemberIdentifier, SignatureError, }, scw_verifier::SmartContractSignatureVerifier, InboxIdRef, @@ -276,18 +277,32 @@ where } /// Generate a `AssociateWallet` signature request using an existing wallet and a new wallet address - pub fn associate_wallet( + pub async fn associate_wallet( &self, - existing_wallet_address: String, new_wallet_address: String, ) -> Result { tracing::info!("Associating new wallet with inbox_id {}", self.inbox_id()); let inbox_id = self.inbox_id(); let builder = SignatureRequestBuilder::new(inbox_id); + let installation_public_key = self.identity().installation_keys.verifying_key(); + let new_member_identifier: MemberIdentifier = new_wallet_address.into(); + + let mut signature_request = builder + .add_association(new_member_identifier, installation_public_key.into()) + .build(); + + let signature = self + .identity() + .installation_keys + .credential_sign::(signature_request.signature_text())?; + signature_request + .add_signature( + UnverifiedSignature::new_installation_key(signature, installation_public_key), + &self.scw_verifier, + ) + .await?; - Ok(builder - .add_association(new_wallet_address.into(), existing_wallet_address.into()) - .build()) + Ok(signature_request) } /// Revoke the given wallets from the association state for the client's inbox @@ -602,10 +617,10 @@ pub(crate) mod tests { let client = ClientBuilder::new_test_client(&wallet).await; let mut add_association_request = client - .associate_wallet(wallet_address.clone(), wallet_2_address.clone()) + .associate_wallet(wallet_2_address.clone()) + .await .unwrap(); - add_wallet_signature(&mut add_association_request, &wallet).await; add_wallet_signature(&mut add_association_request, &wallet_2).await; client @@ -656,10 +671,10 @@ pub(crate) mod tests { assert_logged!("Wrote association", 1); let mut add_association_request = client - .associate_wallet(wallet_address.clone(), wallet_2_address.clone()) + .associate_wallet(wallet_2_address.clone()) + .await .unwrap(); - add_wallet_signature(&mut add_association_request, &wallet).await; add_wallet_signature(&mut add_association_request, &wallet_2).await; client @@ -737,10 +752,10 @@ pub(crate) mod tests { .unwrap(); let new_wallet = generate_local_wallet(); let mut add_association_request = client - .associate_wallet(wallet.get_address(), new_wallet.get_address()) + .associate_wallet(new_wallet.get_address()) + .await .unwrap(); - add_wallet_signature(&mut add_association_request, &wallet).await; add_wallet_signature(&mut add_association_request, &new_wallet).await; client @@ -817,10 +832,10 @@ pub(crate) mod tests { let client = ClientBuilder::new_test_client(&recovery_wallet).await; let mut add_wallet_signature_request = client - .associate_wallet(recovery_wallet.get_address(), second_wallet.get_address()) + .associate_wallet(second_wallet.get_address()) + .await .unwrap(); - add_wallet_signature(&mut add_wallet_signature_request, &recovery_wallet).await; add_wallet_signature(&mut add_wallet_signature_request, &second_wallet).await; client