Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify Wallet Association By Using Installation Keys #1314

Merged
merged 16 commits into from
Nov 22, 2024
70 changes: 37 additions & 33 deletions bindings_ffi/src/mls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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())
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down
12 changes: 3 additions & 9 deletions bindings_node/src/signatures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
17 changes: 0 additions & 17 deletions bindings_node/test/Client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
7 changes: 2 additions & 5 deletions bindings_wasm/src/signatures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
29 changes: 9 additions & 20 deletions xmtp_id/src/associations/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
}

Expand Down Expand Up @@ -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);
}

Expand Down
7 changes: 7 additions & 0 deletions xmtp_id/src/associations/member.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use ed25519_dalek::VerifyingKey;
use xmtp_cryptography::XmtpInstallationCredential;

#[derive(Clone, Debug, PartialEq)]
Expand Down Expand Up @@ -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())
Expand Down
Loading