Skip to content

Commit

Permalink
Tidy and add convenience methods (#1052)
Browse files Browse the repository at this point in the history
## tl;dr

- Adds some convenience methods for creating unverified signatures
- Improves tests for serialization and deserialization
- Adds `PartialEq` trait to `UnverifiedIdentityUpdate`
  • Loading branch information
neekolas committed Sep 11, 2024
1 parent 09366ec commit 4a78a26
Show file tree
Hide file tree
Showing 9 changed files with 124 additions and 94 deletions.
10 changes: 3 additions & 7 deletions bindings_ffi/src/mls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ use std::convert::TryInto;
use std::sync::Arc;
use tokio::{sync::Mutex, task::AbortHandle};
use xmtp_api_grpc::grpc_api_helper::Client as TonicApiClient;
use xmtp_id::associations::unverified::UnverifiedErc6492Signature;
use xmtp_id::associations::unverified::UnverifiedRecoverableEcdsaSignature;
use xmtp_id::associations::unverified::UnverifiedSignature;
use xmtp_id::associations::AccountId;
use xmtp_id::associations::AssociationState;
Expand Down Expand Up @@ -194,9 +192,7 @@ impl FfiSignatureRequest {
let mut inner = self.inner.lock().await;
inner
.add_signature(
UnverifiedSignature::RecoverableEcdsa(UnverifiedRecoverableEcdsaSignature::new(
signature_bytes,
)),
UnverifiedSignature::new_recoverable_ecdsa(signature_bytes),
&signature_verifier(),
)
.await?;
Expand All @@ -215,11 +211,11 @@ impl FfiSignatureRequest {
let mut inner = self.inner.lock().await;
let account_id = AccountId::new_evm(chain_id, address);

let signature = UnverifiedSignature::Erc6492(UnverifiedErc6492Signature::new(
let signature = UnverifiedSignature::new_smart_contract_wallet(
signature_bytes,
account_id,
block_number,
));
);
inner
.add_signature(signature, &signature_verifier())
.await?;
Expand Down
12 changes: 4 additions & 8 deletions bindings_node/src/mls_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@ use std::sync::Arc;
pub use xmtp_api_grpc::grpc_api_helper::Client as TonicApiClient;
use xmtp_cryptography::signature::ed25519_public_key_to_address;
use xmtp_id::associations::generate_inbox_id as xmtp_id_generate_inbox_id;
use xmtp_id::associations::unverified::{
UnverifiedErc6492Signature, UnverifiedRecoverableEcdsaSignature, UnverifiedSignature,
};
use xmtp_id::associations::unverified::UnverifiedSignature;
use xmtp_id::associations::{AccountId, MemberIdentifier};
use xmtp_mls::api::ApiClientWrapper;
use xmtp_mls::builder::ClientBuilder;
Expand Down Expand Up @@ -153,9 +151,7 @@ impl NapiClient {
));
}

let signature = UnverifiedSignature::RecoverableEcdsa(
UnverifiedRecoverableEcdsaSignature::new(signature_bytes.deref().to_vec()),
);
let signature = UnverifiedSignature::new_recoverable_ecdsa(signature_bytes.deref().to_vec());

self.signatures.insert(
MemberIdentifier::Address(self.account_address.clone().to_lowercase()),
Expand Down Expand Up @@ -185,11 +181,11 @@ impl NapiClient {

let account_id = AccountId::new_evm(chain_id_u64, account_address.clone());

let signature = UnverifiedSignature::Erc6492(UnverifiedErc6492Signature::new(
let signature = UnverifiedSignature::new_smart_contract_wallet(
signature_bytes.deref().to_vec(),
account_id,
block_number.get_u64().1,
));
);

self.signatures.insert(
MemberIdentifier::Address(account_address.clone().to_lowercase()),
Expand Down
26 changes: 6 additions & 20 deletions bindings_wasm/src/mls_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,10 @@ use wasm_bindgen::prelude::{wasm_bindgen, JsError};
use wasm_bindgen::JsValue;
use xmtp_api_http::XmtpHttpApiClient;
use xmtp_cryptography::signature::ed25519_public_key_to_address;
use xmtp_id::associations::generate_inbox_id as xmtp_id_generate_inbox_id;
use xmtp_id::associations::unverified::{
UnverifiedErc6492Signature, UnverifiedRecoverableEcdsaSignature, UnverifiedSignature,
use xmtp_id::associations::{
generate_inbox_id as xmtp_id_generate_inbox_id, unverified::UnverifiedSignature, AccountId,
MemberIdentifier,
};
use xmtp_id::associations::{AccountId, MemberIdentifier};
use xmtp_id::scw_verifier::{RpcSmartContractWalletVerifier, SmartContractSignatureVerifier};
use xmtp_mls::api::ApiClientWrapper;
use xmtp_mls::builder::ClientBuilder;
use xmtp_mls::identity::IdentityStrategy;
Expand Down Expand Up @@ -150,14 +148,7 @@ impl WasmClient {
));
}

let signature_text = match self.signature_text() {
Some(text) => text,
None => return Err(JsError::new("No signature text found")),
};

let signature = UnverifiedSignature::RecoverableEcdsa(
UnverifiedRecoverableEcdsaSignature::new(signature_bytes.to_vec()),
);
let signature = UnverifiedSignature::new_recoverable_ecdsa(signature_bytes.to_vec());

self.signatures.insert(
MemberIdentifier::Address(self.account_address.clone().to_lowercase()),
Expand All @@ -183,18 +174,13 @@ impl WasmClient {
));
}

let signature_text = match self.signature_text() {
Some(text) => text,
None => return Err(JsError::new("No signature text found")),
};

let account_id = AccountId::new_evm(chain_id, account_address.clone());

let signature = UnverifiedSignature::Erc6492(UnverifiedErc6492Signature::new(
let signature = UnverifiedSignature::new_smart_contract_wallet(
signature_bytes.to_vec(),
account_id,
block_number,
));
);

self.signatures.insert(
MemberIdentifier::Address(account_address.clone().to_lowercase()),
Expand Down
2 changes: 1 addition & 1 deletion xmtp_id/src/associations/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ impl SignatureRequest {
return Err(SignatureRequestError::UnknownSigner);
}

self.signatures.insert(signer_identity.clone(), signature);
self.signatures.insert(verified_sig.signer, signature);

Ok(())
}
Expand Down
65 changes: 47 additions & 18 deletions xmtp_id/src/associations/serialization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ use super::{
},
unverified::{
UnverifiedAction, UnverifiedAddAssociation, UnverifiedChangeRecoveryAddress,
UnverifiedCreateInbox, UnverifiedErc6492Signature, UnverifiedIdentityUpdate,
UnverifiedInstallationKeySignature, UnverifiedLegacyDelegatedSignature,
UnverifiedRecoverableEcdsaSignature, UnverifiedRevokeAssociation, UnverifiedSignature,
UnverifiedCreateInbox, UnverifiedIdentityUpdate, UnverifiedInstallationKeySignature,
UnverifiedLegacyDelegatedSignature, UnverifiedRecoverableEcdsaSignature,
UnverifiedRevokeAssociation, UnverifiedSignature, UnverifiedSmartContractWalletSignature,
},
verified_signature::VerifiedSignature,
MemberIdentifier, SignatureError,
Expand Down Expand Up @@ -168,13 +168,13 @@ impl TryFrom<SignatureWrapperProto> for UnverifiedSignature {
SignatureKindProto::InstallationKey(sig) => UnverifiedSignature::InstallationKey(
UnverifiedInstallationKeySignature::new(sig.bytes, sig.public_key),
),
SignatureKindProto::Erc6492(sig) => {
UnverifiedSignature::Erc6492(UnverifiedErc6492Signature::new(
SignatureKindProto::Erc6492(sig) => UnverifiedSignature::SmartContractWallet(
UnverifiedSmartContractWalletSignature::new(
sig.signature,
sig.account_id.try_into()?,
sig.block_number,
))
}
),
),
};

Ok(unverified_sig)
Expand Down Expand Up @@ -252,7 +252,7 @@ impl From<UnverifiedAction> for IdentityActionProto {
impl From<UnverifiedSignature> for SignatureWrapperProto {
fn from(value: UnverifiedSignature) -> Self {
let signature = match value {
UnverifiedSignature::Erc6492(sig) => {
UnverifiedSignature::SmartContractWallet(sig) => {
SignatureKindProto::Erc6492(SmartContractWalletSignatureProto {
account_id: sig.account_id.into(),
block_number: sig.block_number,
Expand Down Expand Up @@ -565,15 +565,42 @@ mod tests {
let identity_update = UnverifiedIdentityUpdate::new(
inbox_id,
client_timestamp_ns,
vec![UnverifiedAction::CreateInbox(UnverifiedCreateInbox {
initial_address_signature: UnverifiedSignature::RecoverableEcdsa(
UnverifiedRecoverableEcdsaSignature::new(signature_bytes),
),
unsigned_action: UnsignedCreateInbox {
nonce,
account_address,
},
})],
vec![
UnverifiedAction::CreateInbox(UnverifiedCreateInbox {
initial_address_signature: UnverifiedSignature::RecoverableEcdsa(
UnverifiedRecoverableEcdsaSignature::new(signature_bytes),
),
unsigned_action: UnsignedCreateInbox {
nonce,
account_address,
},
}),
UnverifiedAction::AddAssociation(UnverifiedAddAssociation {
new_member_signature: UnverifiedSignature::new_recoverable_ecdsa(vec![1, 2, 3]),
existing_member_signature: UnverifiedSignature::new_recoverable_ecdsa(vec![
4, 5, 6,
]),
unsigned_action: UnsignedAddAssociation {
new_member_identifier: rand_string().into(),
},
}),
UnverifiedAction::ChangeRecoveryAddress(UnverifiedChangeRecoveryAddress {
recovery_address_signature: UnverifiedSignature::new_recoverable_ecdsa(vec![
7, 8, 9,
]),
unsigned_action: UnsignedChangeRecoveryAddress {
new_recovery_address: rand_string(),
},
}),
UnverifiedAction::RevokeAssociation(UnverifiedRevokeAssociation {
recovery_address_signature: UnverifiedSignature::new_recoverable_ecdsa(vec![
10, 11, 12,
]),
unsigned_action: UnsignedRevokeAssociation {
revoked_member: rand_string().into(),
},
}),
],
);

let serialized_update = IdentityUpdateProto::from(identity_update.clone());
Expand All @@ -582,13 +609,15 @@ mod tests {
serialized_update.client_timestamp_ns,
identity_update.client_timestamp_ns
);
assert_eq!(serialized_update.actions.len(), 1);
assert_eq!(serialized_update.actions.len(), 4);

let deserialized_update: UnverifiedIdentityUpdate = serialized_update
.clone()
.try_into()
.expect("deserialization error");

assert_eq!(deserialized_update, identity_update);

let reserialized = IdentityUpdateProto::from(deserialized_update);

assert_eq!(serialized_update, reserialized);
Expand Down
2 changes: 1 addition & 1 deletion xmtp_id/src/associations/signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ impl std::fmt::Display for SignatureKind {
}
}
// CAIP-10[https://github.com/ChainAgnostic/CAIPs/blob/main/CAIPs/caip-10.md]
#[derive(Debug, Clone)]
#[derive(Debug, Clone, PartialEq)]
pub struct AccountId {
pub(crate) chain_id: String,
pub(crate) account_address: String,
Expand Down
26 changes: 8 additions & 18 deletions xmtp_id/src/associations/test_utils.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
use super::{
builder::SignatureRequest,
unsigned_actions::UnsignedCreateInbox,
unverified::{
UnverifiedAction, UnverifiedCreateInbox, UnverifiedInstallationKeySignature,
UnverifiedRecoverableEcdsaSignature, UnverifiedSignature,
},
unverified::{UnverifiedAction, UnverifiedCreateInbox, UnverifiedSignature},
AccountId,
};
use crate::{
Expand Down Expand Up @@ -66,9 +63,9 @@ impl SmartContractSignatureVerifier for MockSmartContractSignatureVerifier {
pub async fn add_wallet_signature(signature_request: &mut SignatureRequest, wallet: &LocalWallet) {
let signature_text = signature_request.signature_text();
let sig = wallet.sign_message(signature_text).await.unwrap().to_vec();
let unverified_sig =
UnverifiedSignature::RecoverableEcdsa(UnverifiedRecoverableEcdsaSignature::new(sig));
let unverified_sig = UnverifiedSignature::new_recoverable_ecdsa(sig);
let scw_verifier = MockSmartContractSignatureVerifier::new(false);

signature_request
.add_signature(unverified_sig, &scw_verifier)
.await
Expand All @@ -87,11 +84,10 @@ pub async fn add_installation_key_signature(
let sig = installation_key
.sign_prehashed(prehashed, Some(INSTALLATION_KEY_SIGNATURE_CONTEXT))
.unwrap();
let unverified_sig =
UnverifiedSignature::InstallationKey(UnverifiedInstallationKeySignature::new(
sig.to_bytes().to_vec(),
verifying_key.as_bytes().to_vec(),
));
let unverified_sig = UnverifiedSignature::new_installation_key(
sig.to_bytes().to_vec(),
verifying_key.as_bytes().to_vec(),
);

signature_request
.add_signature(
Expand All @@ -102,20 +98,14 @@ pub async fn add_installation_key_signature(
.expect("should succeed");
}

impl UnverifiedSignature {
pub fn new_test_recoverable_ecdsa(signature: Vec<u8>) -> Self {
Self::RecoverableEcdsa(UnverifiedRecoverableEcdsaSignature::new(signature))
}
}

impl UnverifiedAction {
pub fn new_test_create_inbox(account_address: &str, nonce: &u64) -> Self {
Self::CreateInbox(UnverifiedCreateInbox::new(
UnsignedCreateInbox {
account_address: account_address.to_owned(),
nonce: *nonce,
},
UnverifiedSignature::new_test_recoverable_ecdsa(vec![1, 2, 3]),
UnverifiedSignature::new_recoverable_ecdsa(vec![1, 2, 3]),
))
}
}
12 changes: 6 additions & 6 deletions xmtp_id/src/associations/unsigned_actions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ pub trait SignatureTextCreator {
fn signature_text(&self) -> String;
}

#[derive(Clone, Debug)]
#[derive(Clone, Debug, PartialEq)]
pub struct UnsignedCreateInbox {
pub nonce: u64,
pub account_address: String,
Expand All @@ -23,7 +23,7 @@ impl SignatureTextCreator for UnsignedCreateInbox {
}
}

#[derive(Clone, Debug)]
#[derive(Clone, Debug, PartialEq)]
pub struct UnsignedAddAssociation {
pub new_member_identifier: MemberIdentifier,
}
Expand All @@ -40,7 +40,7 @@ impl SignatureTextCreator for UnsignedAddAssociation {
}
}

#[derive(Clone, Debug)]
#[derive(Clone, Debug, PartialEq)]
pub struct UnsignedRevokeAssociation {
pub revoked_member: MemberIdentifier,
}
Expand All @@ -57,7 +57,7 @@ impl SignatureTextCreator for UnsignedRevokeAssociation {
}
}

#[derive(Clone, Debug)]
#[derive(Clone, Debug, PartialEq)]
pub struct UnsignedChangeRecoveryAddress {
pub new_recovery_address: String,
}
Expand All @@ -73,7 +73,7 @@ impl SignatureTextCreator for UnsignedChangeRecoveryAddress {
}

#[allow(dead_code)]
#[derive(Clone, Debug)]
#[derive(Clone, Debug, PartialEq)]
pub enum UnsignedAction {
CreateInbox(UnsignedCreateInbox),
AddAssociation(UnsignedAddAssociation),
Expand All @@ -92,7 +92,7 @@ impl SignatureTextCreator for UnsignedAction {
}
}

#[derive(Clone)]
#[derive(Clone, Debug, PartialEq)]
pub struct UnsignedIdentityUpdate {
pub inbox_id: String,
pub client_timestamp_ns: u64,
Expand Down
Loading

0 comments on commit 4a78a26

Please sign in to comment.