Skip to content

Commit

Permalink
Replace signatures in IdentityUpdate (#1051)
Browse files Browse the repository at this point in the history
## tl;dr

- #1033
- Replaces the signatures in `IdentityUpdate` struct to use the new `VerifiedSignature`
- For clients that want to control signature verification, we will need a `MultiChainSmartContractWalletVerifier` that uses different RPC endpoints for each supported `chain_id`
- We can probably refactor the `SignatureRequest` to contain a reference to the verifier, so that it doesn't have to be passed in as part of `add_signature` in the bindings.

## Does this mean SCW signatures work

Not yet. There are still some big TODO items in here, which will cause smart contract wallet signature verification to fail (which it also is in `main`). We don't really want to use the `RpcSmartContractWalletVerifier` in normal clients. Instead we will call a (still unbuilt) RPC method to verify these signatures remotely.
  • Loading branch information
neekolas committed Sep 11, 2024
1 parent dacdf4f commit 09366ec
Show file tree
Hide file tree
Showing 24 changed files with 883 additions and 1,339 deletions.
1 change: 1 addition & 0 deletions bindings_ffi/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions bindings_ffi/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ path = "src/bin.rs"

[dev-dependencies]
ethers = "2.0.13"
rand = "0.8.5"
tempfile = "3.5.0"
tokio = { version = "1.28.1", features = ["full"] }
tokio-test = "0.4"
Expand Down
71 changes: 41 additions & 30 deletions bindings_ffi/src/mls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,15 @@ 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;
use xmtp_id::scw_verifier::RpcSmartContractWalletVerifier;
use xmtp_id::scw_verifier::SmartContractSignatureVerifier;
use xmtp_id::{
associations::{
builder::SignatureRequest, generate_inbox_id as xmtp_id_generate_inbox_id,
RecoverableEcdsaSignature, SmartContractWalletSignature,
},
associations::{builder::SignatureRequest, generate_inbox_id as xmtp_id_generate_inbox_id},
InboxId,
};
use xmtp_mls::groups::group_mutable_metadata::MetadataField;
Expand Down Expand Up @@ -179,17 +182,23 @@ pub struct FfiSignatureRequest {
inner: Arc<Mutex<SignatureRequest>>,
}

// TODO:nm store the verifier on the request from the client
fn signature_verifier() -> impl SmartContractSignatureVerifier {
RpcSmartContractWalletVerifier::new("http://www.fake.com".to_string())
}

#[uniffi::export(async_runtime = "tokio")]
impl FfiSignatureRequest {
// Signature that's signed by EOA wallet
pub async fn add_ecdsa_signature(&self, signature_bytes: Vec<u8>) -> Result<(), GenericError> {
let mut inner = self.inner.lock().await;
let signature_text = inner.signature_text();
inner
.add_signature(Box::new(RecoverableEcdsaSignature::new(
signature_text,
signature_bytes,
)))
.add_signature(
UnverifiedSignature::RecoverableEcdsa(UnverifiedRecoverableEcdsaSignature::new(
signature_bytes,
)),
&signature_verifier(),
)
.await?;

Ok(())
Expand All @@ -200,17 +209,20 @@ impl FfiSignatureRequest {
&self,
signature_bytes: Vec<u8>,
address: String,
chain_rpc_url: String,
chain_id: u64,
block_number: u64,
) -> Result<(), GenericError> {
let mut inner = self.inner.lock().await;
let signature = SmartContractWalletSignature::new_with_rpc(
inner.signature_text(),
let account_id = AccountId::new_evm(chain_id, address);

let signature = UnverifiedSignature::Erc6492(UnverifiedErc6492Signature::new(
signature_bytes,
address,
chain_rpc_url,
)
.await?;
inner.add_signature(Box::new(signature)).await?;
account_id,
block_number,
));
inner
.add_signature(signature, &signature_verifier())
.await?;
Ok(())
}

Expand Down Expand Up @@ -1560,30 +1572,29 @@ impl FfiGroupPermissions {

#[cfg(test)]
mod tests {
use super::{create_client, signature_verifier, FfiMessage, FfiMessageCallback, FfiXmtpClient};
use crate::{
get_inbox_id_for_address, inbox_owner::SigningError, logger::FfiLogger,
FfiConversationCallback, FfiCreateGroupOptions, FfiGroup, FfiGroupMessageKind,
FfiGroupPermissionsOptions, FfiInboxOwner, FfiListConversationsOptions,
FfiListMessagesOptions, FfiMetadataField, FfiPermissionPolicy, FfiPermissionPolicySet,
FfiPermissionUpdateType,
};
use ethers::utils::hex;
use rand::distributions::{Alphanumeric, DistString};
use std::{
env,
sync::{
atomic::{AtomicU32, Ordering},
Arc, Mutex,
},
};

use super::{create_client, FfiMessage, FfiMessageCallback, FfiXmtpClient};
use ethers::core::rand::{
self,
distributions::{Alphanumeric, DistString},
};
use ethers::utils::hex;
use tokio::{sync::Notify, time::error::Elapsed};
use xmtp_cryptography::{signature::RecoverableSignature, utils::rng};
use xmtp_id::associations::generate_inbox_id;
use xmtp_id::associations::{
generate_inbox_id,
unverified::{UnverifiedRecoverableEcdsaSignature, UnverifiedSignature},
};
use xmtp_mls::{
groups::{GroupError, MlsGroup},
storage::EncryptionKey,
Expand Down Expand Up @@ -1908,12 +1919,12 @@ mod tests {
.inner
.lock()
.await
.add_signature(Box::new(
xmtp_id::associations::RecoverableEcdsaSignature::new(
signature_text,
.add_signature(
UnverifiedSignature::RecoverableEcdsa(UnverifiedRecoverableEcdsaSignature::new(
wallet_signature,
),
))
)),
&signature_verifier(),
)
.await
.unwrap();
}
Expand Down
39 changes: 17 additions & 22 deletions bindings_node/src/mls_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ 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::{
AccountId, MemberIdentifier, RecoverableEcdsaSignature, Signature, SmartContractWalletSignature,
use xmtp_id::associations::unverified::{
UnverifiedErc6492Signature, UnverifiedRecoverableEcdsaSignature, UnverifiedSignature,
};
use xmtp_id::associations::{AccountId, MemberIdentifier};
use xmtp_mls::api::ApiClientWrapper;
use xmtp_mls::builder::ClientBuilder;
use xmtp_mls::identity::IdentityStrategy;
Expand All @@ -22,7 +23,7 @@ pub type RustXmtpClient = MlsClient<TonicApiClient>;
#[napi]
pub struct NapiClient {
inner_client: Arc<RustXmtpClient>,
signatures: HashMap<MemberIdentifier, Box<dyn Signature>>,
signatures: HashMap<MemberIdentifier, UnverifiedSignature>,
pub account_address: String,
}

Expand Down Expand Up @@ -152,15 +153,9 @@ impl NapiClient {
));
}

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

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

self.signatures.insert(
MemberIdentifier::Address(self.account_address.clone().to_lowercase()),
Expand All @@ -176,7 +171,8 @@ impl NapiClient {
signature_bytes: Uint8Array,
chain_id: BigInt,
account_address: String,
chain_rpc_url: String,
// TODO:nm remove this
_chain_rpc_url: String,
block_number: BigInt,
) -> Result<()> {
if self.is_registered() {
Expand All @@ -185,20 +181,13 @@ impl NapiClient {
));
}

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

let (_, chain_id_u64, _) = chain_id.get_u64();

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

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

Expand Down Expand Up @@ -233,7 +222,13 @@ impl NapiClient {
// apply added signatures to the signature request
for signature in self.signatures.values() {
signature_request
.add_signature(signature.clone())
.add_signature(
signature.clone(),
self
.inner_client
.smart_contract_signature_verifier()
.as_ref(),
)
.await
.map_err(|e| Error::from_reason(format!("{}", e)))?;
}
Expand Down
30 changes: 18 additions & 12 deletions bindings_wasm/src/mls_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ 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::{
AccountId, MemberIdentifier, RecoverableEcdsaSignature, Signature, SmartContractWalletSignature,
use xmtp_id::associations::unverified::{
UnverifiedErc6492Signature, UnverifiedRecoverableEcdsaSignature, UnverifiedSignature,
};
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 All @@ -22,7 +24,7 @@ pub type RustXmtpClient = MlsClient<XmtpHttpApiClient>;
pub struct WasmClient {
account_address: String,
inner_client: Arc<RustXmtpClient>,
signatures: HashMap<MemberIdentifier, Box<dyn Signature>>,
signatures: HashMap<MemberIdentifier, UnverifiedSignature>,
}

#[wasm_bindgen]
Expand Down Expand Up @@ -153,10 +155,9 @@ impl WasmClient {
None => return Err(JsError::new("No signature text found")),
};

let signature = Box::new(RecoverableEcdsaSignature::new(
signature_text.into(),
signature_bytes.to_vec(),
));
let signature = UnverifiedSignature::RecoverableEcdsa(
UnverifiedRecoverableEcdsaSignature::new(signature_bytes.to_vec()),
);

self.signatures.insert(
MemberIdentifier::Address(self.account_address.clone().to_lowercase()),
Expand All @@ -172,7 +173,8 @@ impl WasmClient {
signature_bytes: Uint8Array,
chain_id: u64,
account_address: String,
chain_rpc_url: String,
// TODO:nm Remove this
_chain_rpc_url: String,
block_number: u64,
) -> Result<(), JsError> {
if self.is_registered() {
Expand All @@ -188,11 +190,9 @@ impl WasmClient {

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

let signature = Box::new(SmartContractWalletSignature::new(
signature_text,
let signature = UnverifiedSignature::Erc6492(UnverifiedErc6492Signature::new(
signature_bytes.to_vec(),
account_id,
chain_rpc_url,
block_number,
));

Expand Down Expand Up @@ -227,7 +227,13 @@ impl WasmClient {
// apply added signatures to the signature request
for signature in self.signatures.values() {
signature_request
.add_signature(signature.clone())
.add_signature(
signature.clone(),
self
.inner_client
.smart_contract_signature_verifier()
.as_ref(),
)
.await
.map_err(|e| JsError::new(format!("{}", e).as_str()))?;
}
Expand Down
21 changes: 12 additions & 9 deletions examples/cli/cli-client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use clap::{Parser, Subcommand, ValueEnum};
use ethers::signers::{coins_bip39::English, LocalWallet, MnemonicBuilder};
use kv_log_macro::{error, info};
use prost::Message;
use xmtp_id::associations::RecoverableEcdsaSignature;
use xmtp_id::associations::unverified::{UnverifiedRecoverableEcdsaSignature, UnverifiedSignature};
use xmtp_mls::groups::message_history::MessageHistoryContent;
use xmtp_mls::storage::group_message::GroupMessageKind;

Expand Down Expand Up @@ -457,14 +457,17 @@ async fn register(cli: &Cli, maybe_seed_phrase: Option<String>) -> Result<(), Cl
)
.await?;
let mut signature_request = client.identity().signature_request().unwrap();
let signature = RecoverableEcdsaSignature::new(
signature_request.signature_text(),
w.sign(signature_request.signature_text().as_str())
.unwrap()
.into(),
);
let sig_bytes: Vec<u8> = w
.sign(signature_request.signature_text().as_str())
.unwrap()
.into();
let signature =
UnverifiedSignature::RecoverableEcdsa(UnverifiedRecoverableEcdsaSignature::new(sig_bytes));
signature_request
.add_signature(Box::new(signature))
.add_signature(
signature,
client.smart_contract_signature_verifier().as_ref(),
)
.await
.unwrap();

Expand Down Expand Up @@ -509,7 +512,7 @@ fn format_messages(
if text.is_none() {
continue;
}
// TODO:nm use inbox ID

let sender = if msg.sender_inbox_id == my_account_address {
"Me".to_string()
} else {
Expand Down
Loading

0 comments on commit 09366ec

Please sign in to comment.