From 71b47a20020ecece139cdc0d56ab356d3f6637f1 Mon Sep 17 00:00:00 2001 From: Naomi Plasterer Date: Fri, 1 Nov 2024 07:32:25 -0700 Subject: [PATCH] Enforce 42 characters for address on inboxId create (#1202) * enforce 42 characters for address * add the error * update all of the tests * try and get closer to a solution * simple fix * one more error fix * fix up all the tests * fmt * fix the lint * fix another lint issue * update the test * cargo fmt * fix up the test * fix up another test * more test fixing --- bindings_ffi/src/lib.rs | 2 ++ bindings_ffi/src/mls.rs | 22 +++++++------- bindings_node/src/inbox_id.rs | 5 ++-- bindings_wasm/src/inbox_id.rs | 6 ++-- examples/cli/cli-client.rs | 6 ++-- mls_validation_service/src/handlers.rs | 4 +-- xmtp_id/src/associations/association_log.rs | 8 +++-- xmtp_id/src/associations/builder.rs | 10 +++---- xmtp_id/src/associations/hashes.rs | 23 ++++++++++++-- xmtp_id/src/associations/mod.rs | 15 ++++++---- xmtp_id/src/associations/serialization.rs | 2 +- xmtp_id/src/associations/state.rs | 20 ++++++++----- xmtp_id/src/associations/test_utils.rs | 14 +++++---- xmtp_id/src/associations/unsigned_actions.rs | 19 ++++++------ xmtp_id/src/associations/unverified.rs | 2 +- xmtp_mls/src/builder.rs | 30 +++++++++---------- xmtp_mls/src/identity.rs | 10 +++++-- xmtp_mls/src/identity_updates.rs | 2 +- .../encrypted_store/association_state.rs | 14 +++++++-- xmtp_mls/src/utils/test/mod.rs | 4 +-- 20 files changed, 136 insertions(+), 82 deletions(-) diff --git a/bindings_ffi/src/lib.rs b/bindings_ffi/src/lib.rs index a7e04c29a..fa9203c69 100755 --- a/bindings_ffi/src/lib.rs +++ b/bindings_ffi/src/lib.rs @@ -44,6 +44,8 @@ pub enum GenericError { Verifier(#[from] xmtp_id::scw_verifier::VerifierError), #[error("Failed to convert to u32")] FailedToConvertToU32, + #[error("Association error: {0}")] + Association(#[from] xmtp_id::associations::AssociationError), #[error(transparent)] DeviceSync(#[from] xmtp_mls::groups::device_sync::DeviceSyncError), } diff --git a/bindings_ffi/src/mls.rs b/bindings_ffi/src/mls.rs index 6f3399411..58b82d801 100644 --- a/bindings_ffi/src/mls.rs +++ b/bindings_ffi/src/mls.rs @@ -170,8 +170,8 @@ pub async fn get_inbox_id_for_address( #[allow(unused)] #[uniffi::export] -pub fn generate_inbox_id(account_address: String, nonce: u64) -> String { - xmtp_id_generate_inbox_id(&account_address, &nonce) +pub fn generate_inbox_id(account_address: String, nonce: u64) -> Result { + Ok(xmtp_id_generate_inbox_id(&account_address, &nonce)?) } #[derive(uniffi::Object)] @@ -1817,7 +1817,7 @@ mod tests { ) -> Arc { let ffi_inbox_owner = LocalWalletInboxOwner::with_wallet(wallet); let nonce = 1; - let inbox_id = generate_inbox_id(&ffi_inbox_owner.get_address(), &nonce); + let inbox_id = generate_inbox_id(&ffi_inbox_owner.get_address(), &nonce).unwrap(); let client = create_client( Box::new(MockLogger {}), @@ -1875,7 +1875,7 @@ mod tests { let account_address = "0x0bD00B21aF9a2D538103c3AAf95Cb507f8AF1B28".to_lowercase(); let legacy_keys = hex::decode("0880bdb7a8b3f6ede81712220a20ad528ea38ce005268c4fb13832cfed13c2b2219a378e9099e48a38a30d66ef991a96010a4c08aaa8e6f5f9311a430a41047fd90688ca39237c2899281cdf2756f9648f93767f91c0e0f74aed7e3d3a8425e9eaa9fa161341c64aa1c782d004ff37ffedc887549ead4a40f18d1179df9dff124612440a403c2cb2338fb98bfe5f6850af11f6a7e97a04350fc9d37877060f8d18e8f66de31c77b3504c93cf6a47017ea700a48625c4159e3f7e75b52ff4ea23bc13db77371001").unwrap(); let nonce = 0; - let inbox_id = generate_inbox_id(&account_address, &nonce); + let inbox_id = generate_inbox_id(&account_address, &nonce).unwrap(); let client = create_client( Box::new(MockLogger {}), @@ -1899,7 +1899,7 @@ mod tests { async fn test_create_client_with_storage() { let ffi_inbox_owner = LocalWalletInboxOwner::new(); let nonce = 1; - let inbox_id = generate_inbox_id(&ffi_inbox_owner.get_address(), &nonce); + let inbox_id = generate_inbox_id(&ffi_inbox_owner.get_address(), &nonce).unwrap(); let path = tmp_path(); @@ -1950,7 +1950,7 @@ mod tests { async fn test_create_client_with_key() { let ffi_inbox_owner = LocalWalletInboxOwner::new(); let nonce = 1; - let inbox_id = generate_inbox_id(&ffi_inbox_owner.get_address(), &nonce); + let inbox_id = generate_inbox_id(&ffi_inbox_owner.get_address(), &nonce).unwrap(); let path = tmp_path(); @@ -2024,7 +2024,7 @@ mod tests { // Setup the initial first client let ffi_inbox_owner = LocalWalletInboxOwner::new(); let nonce = 1; - let inbox_id = generate_inbox_id(&ffi_inbox_owner.get_address(), &nonce); + let inbox_id = generate_inbox_id(&ffi_inbox_owner.get_address(), &nonce).unwrap(); let path = tmp_path(); let key = static_enc_key().to_vec(); @@ -2090,7 +2090,7 @@ mod tests { // Setup the initial first client let ffi_inbox_owner = LocalWalletInboxOwner::new(); let nonce = 1; - let inbox_id = generate_inbox_id(&ffi_inbox_owner.get_address(), &nonce); + let inbox_id = generate_inbox_id(&ffi_inbox_owner.get_address(), &nonce).unwrap(); let path = tmp_path(); let key = static_enc_key().to_vec(); @@ -2176,7 +2176,7 @@ mod tests { async fn test_invalid_external_signature() { let inbox_owner = LocalWalletInboxOwner::new(); let nonce = 1; - let inbox_id = generate_inbox_id(&inbox_owner.get_address(), &nonce); + let inbox_id = generate_inbox_id(&inbox_owner.get_address(), &nonce).unwrap(); let path = tmp_path(); let client = create_client( @@ -2202,9 +2202,9 @@ mod tests { async fn test_can_message() { let amal = LocalWalletInboxOwner::new(); let nonce = 1; - let amal_inbox_id = generate_inbox_id(&amal.get_address(), &nonce); + let amal_inbox_id = generate_inbox_id(&amal.get_address(), &nonce).unwrap(); let bola = LocalWalletInboxOwner::new(); - let bola_inbox_id = generate_inbox_id(&bola.get_address(), &nonce); + let bola_inbox_id = generate_inbox_id(&bola.get_address(), &nonce).unwrap(); let path = tmp_path(); let client_amal = create_client( diff --git a/bindings_node/src/inbox_id.rs b/bindings_node/src/inbox_id.rs index 08875e461..ab4f1388f 100644 --- a/bindings_node/src/inbox_id.rs +++ b/bindings_node/src/inbox_id.rs @@ -30,9 +30,10 @@ pub async fn get_inbox_id_for_address( } #[napi] -pub fn generate_inbox_id(account_address: String) -> String { +pub fn generate_inbox_id(account_address: String) -> Result> { let account_address = account_address.to_lowercase(); // ensure that the nonce is always 1 for now since this will only be used for the // create_client function above, which also has a hard-coded nonce of 1 - xmtp_id_generate_inbox_id(&account_address, &1) + let result = xmtp_id_generate_inbox_id(&account_address, &1).map_err(ErrorWrapper::from)?; + Ok(Some(result)) } diff --git a/bindings_wasm/src/inbox_id.rs b/bindings_wasm/src/inbox_id.rs index 359948266..1aec357c4 100644 --- a/bindings_wasm/src/inbox_id.rs +++ b/bindings_wasm/src/inbox_id.rs @@ -24,9 +24,11 @@ pub async fn get_inbox_id_for_address( } #[wasm_bindgen(js_name = generateInboxId)] -pub fn generate_inbox_id(account_address: String) -> String { +pub fn generate_inbox_id(account_address: String) -> Result { let account_address = account_address.to_lowercase(); // ensure that the nonce is always 1 for now since this will only be used for the // create_client function above, which also has a hard-coded nonce of 1 - xmtp_id_generate_inbox_id(&account_address, &1) + let result = xmtp_id_generate_inbox_id(&account_address, &1) + .map_err(|e| JsError::new(format!("{}", e).as_str()))?; + Ok(result) } diff --git a/examples/cli/cli-client.rs b/examples/cli/cli-client.rs index 9526be1d8..f06259b15 100755 --- a/examples/cli/cli-client.rs +++ b/examples/cli/cli-client.rs @@ -37,7 +37,7 @@ use xmtp_cryptography::{ signature::{RecoverableSignature, SignatureError}, utils::rng, }; -use xmtp_id::associations::generate_inbox_id; +use xmtp_id::associations::{generate_inbox_id, AssociationError}; use xmtp_mls::{ builder::ClientBuilderError, client::ClientError, @@ -141,6 +141,8 @@ enum CliError { StorageError(#[from] StorageError), #[error("generic:{0}")] Generic(String), + #[error(transparent)] + Association(#[from] AssociationError), } impl From for CliError { @@ -506,7 +508,7 @@ where }; let nonce = 0; - let inbox_id = generate_inbox_id(&w.get_address(), &nonce); + let inbox_id = generate_inbox_id(&w.get_address(), &nonce)?; let client = create_client( cli, IdentityStrategy::CreateIfNotFound(inbox_id, w.get_address(), nonce, None), diff --git a/mls_validation_service/src/handlers.rs b/mls_validation_service/src/handlers.rs index 40e7fdd70..cd43ab815 100644 --- a/mls_validation_service/src/handlers.rs +++ b/mls_validation_service/src/handlers.rs @@ -362,7 +362,7 @@ mod tests { let wallet = LocalWallet::new(&mut rand::thread_rng()); let address = format!("0x{}", hex::encode(wallet.address())); - let inbox_id = generate_inbox_id(&address, &0); + let inbox_id = generate_inbox_id(&address, &0).unwrap(); (inbox_id, signing_key) } @@ -413,7 +413,7 @@ mod tests { async fn test_get_association_state() { let account_address = rand_string(); let nonce = rand_u64(); - let inbox_id = generate_inbox_id(&account_address, &nonce); + let inbox_id = generate_inbox_id(&account_address, &nonce).unwrap(); let update = UnverifiedIdentityUpdate::new_test( vec![UnverifiedAction::new_test_create_inbox( &account_address, diff --git a/xmtp_id/src/associations/association_log.rs b/xmtp_id/src/associations/association_log.rs index b7df72f43..6588b3551 100644 --- a/xmtp_id/src/associations/association_log.rs +++ b/xmtp_id/src/associations/association_log.rs @@ -36,6 +36,8 @@ pub enum AssociationError { MissingIdentityUpdate, #[error("Wrong chain id. Initially added with {0} but now signing from {1}")] ChainIdMismatch(u64, u64), + #[error("Invalid account address: Must be 42 hex characters, starting with '0x'.")] + InvalidAccountAddress, } pub trait IdentityAction: Send { @@ -90,11 +92,11 @@ impl IdentityAction for CreateInbox { return Err(AssociationError::LegacySignatureReuse); } - Ok(AssociationState::new( + AssociationState::new( account_address, self.nonce, self.initial_address_signature.chain_id, - )) + ) } fn signatures(&self) -> Vec> { @@ -140,7 +142,7 @@ impl IdentityAction for AddAssociation { && existing_state.inbox_id().ne(&generate_inbox_id( existing_member_identifier.to_string().as_str(), &0, - )) + )?) { return Err(AssociationError::LegacySignatureReuse); } diff --git a/xmtp_id/src/associations/builder.rs b/xmtp_id/src/associations/builder.rs index 1463f7a9f..be6ccc3bf 100644 --- a/xmtp_id/src/associations/builder.rs +++ b/xmtp_id/src/associations/builder.rs @@ -451,7 +451,7 @@ pub(crate) mod tests { let wallet = LocalWallet::new(&mut rand::thread_rng()); let account_address = wallet.get_address(); let nonce = 0; - let inbox_id = generate_inbox_id(&account_address, &nonce); + let inbox_id = generate_inbox_id(&account_address, &nonce).unwrap(); let mut signature_request = SignatureRequestBuilder::new(inbox_id) .create_inbox(account_address.into(), nonce) @@ -474,7 +474,7 @@ pub(crate) mod tests { let account_address = wallet.get_address(); let installation_key_id = installation_key.verifying_key().as_bytes().to_vec(); let nonce = 0; - let inbox_id = generate_inbox_id(&account_address, &nonce); + let inbox_id = generate_inbox_id(&account_address, &nonce).unwrap(); let existing_member_identifier: MemberIdentifier = account_address.into(); let new_member_identifier: MemberIdentifier = installation_key_id.into(); @@ -501,7 +501,7 @@ pub(crate) mod tests { let wallet = LocalWallet::new(&mut rand::thread_rng()); let account_address = wallet.get_address(); let nonce = 0; - let inbox_id = generate_inbox_id(&account_address, &nonce); + let inbox_id = generate_inbox_id(&account_address, &nonce).unwrap(); let existing_member_identifier: MemberIdentifier = account_address.clone().into(); let mut signature_request = SignatureRequestBuilder::new(inbox_id) @@ -524,9 +524,9 @@ pub(crate) mod tests { #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)] #[cfg_attr(not(target_arch = "wasm32"), tokio::test)] async fn attempt_adding_unknown_signer() { - let account_address = "account_address".to_string(); + let account_address = "0x1234567890abcdef1234567890abcdef12345678".to_string(); let nonce = 0; - let inbox_id = generate_inbox_id(&account_address, &nonce); + let inbox_id = generate_inbox_id(&account_address, &nonce).unwrap(); let mut signature_request = SignatureRequestBuilder::new(inbox_id) .create_inbox(account_address.into(), nonce) .build(); diff --git a/xmtp_id/src/associations/hashes.rs b/xmtp_id/src/associations/hashes.rs index a9439b79e..d89b04892 100644 --- a/xmtp_id/src/associations/hashes.rs +++ b/xmtp_id/src/associations/hashes.rs @@ -1,5 +1,8 @@ use sha2::{Digest, Sha256}; +use super::AssociationError; + +/// Helper function to generate a SHA256 hash as a hex string. fn sha256_string(input: String) -> String { let mut hasher = Sha256::new(); hasher.update(input.as_bytes()); @@ -7,6 +10,22 @@ fn sha256_string(input: String) -> String { format!("{:x}", result) } -pub fn generate_inbox_id(account_address: &str, nonce: &u64) -> String { - sha256_string(format!("{}{}", account_address.to_lowercase(), nonce)) +/// Validates that the account address is exactly 42 characters, starts with "0x", +/// and contains only valid hex digits. +fn is_valid_address(account_address: &str) -> bool { + account_address.len() == 42 + && account_address.starts_with("0x") + && account_address[2..].chars().all(|c| c.is_ascii_hexdigit()) +} + +/// Generates an inbox ID if the account address is valid. +pub fn generate_inbox_id(account_address: &str, nonce: &u64) -> Result { + if !is_valid_address(account_address) { + return Err(AssociationError::InvalidAccountAddress); + } + Ok(sha256_string(format!( + "{}{}", + account_address.to_lowercase(), + nonce + ))) } diff --git a/xmtp_id/src/associations/mod.rs b/xmtp_id/src/associations/mod.rs index d248a90fb..ad84ab820 100644 --- a/xmtp_id/src/associations/mod.rs +++ b/xmtp_id/src/associations/mod.rs @@ -126,7 +126,8 @@ pub(crate) mod tests { pub fn new_test_inbox() -> AssociationState { let create_request = CreateInbox::default(); - let inbox_id = generate_inbox_id(&create_request.account_address, &create_request.nonce); + let inbox_id = + generate_inbox_id(&create_request.account_address, &create_request.nonce).unwrap(); let identity_update = IdentityUpdate::new_test(vec![Action::CreateInbox(create_request)], inbox_id); @@ -159,7 +160,8 @@ pub(crate) mod tests { #[test] fn test_create_inbox() { let create_request = CreateInbox::default(); - let inbox_id = generate_inbox_id(&create_request.account_address, &create_request.nonce); + let inbox_id = + generate_inbox_id(&create_request.account_address, &create_request.nonce).unwrap(); let account_address = create_request.account_address.clone(); let identity_update = IdentityUpdate::new_test(vec![Action::CreateInbox(create_request)], inbox_id.clone()); @@ -208,7 +210,7 @@ pub(crate) mod tests { fn create_and_add_together() { let create_action = CreateInbox::default(); let account_address = create_action.account_address.clone(); - let inbox_id = generate_inbox_id(&account_address, &create_action.nonce); + let inbox_id = generate_inbox_id(&account_address, &create_action.nonce).unwrap(); let new_member_identifier: MemberIdentifier = rand_vec().into(); let add_action = AddAssociation { existing_member_signature: VerifiedSignature::new( @@ -254,7 +256,7 @@ pub(crate) mod tests { None, ), }; - let inbox_id = generate_inbox_id(&member_identifier.to_string(), &0); + let inbox_id = generate_inbox_id(&member_identifier.to_string(), &0).unwrap(); let state = get_state(vec![IdentityUpdate::new_test( vec![Action::CreateInbox(create_action)], inbox_id.clone(), @@ -393,7 +395,8 @@ pub(crate) mod tests { #[test] fn reject_if_signer_not_existing_member() { let create_inbox = CreateInbox::default(); - let inbox_id = generate_inbox_id(&create_inbox.account_address, &create_inbox.nonce); + let inbox_id = + generate_inbox_id(&create_inbox.account_address, &create_inbox.nonce).unwrap(); let create_request = Action::CreateInbox(create_inbox); // The default here will create an AddAssociation from a random wallet let update = Action::AddAssociation(AddAssociation { @@ -665,7 +668,7 @@ pub(crate) mod tests { let initial_state = get_state(vec![IdentityUpdate::new_test( vec![Action::CreateInbox(action)], - generate_inbox_id(&signer, &0), + generate_inbox_id(&signer, &0).unwrap(), )]) .expect("initial state should be OK"); diff --git a/xmtp_id/src/associations/serialization.rs b/xmtp_id/src/associations/serialization.rs index cc10155e9..9348e07bd 100644 --- a/xmtp_id/src/associations/serialization.rs +++ b/xmtp_id/src/associations/serialization.rs @@ -583,7 +583,7 @@ pub(crate) mod tests { fn test_round_trip_unverified() { let account_address = rand_string(); let nonce = rand_u64(); - let inbox_id = generate_inbox_id(&account_address, &nonce); + let inbox_id = generate_inbox_id(&account_address, &nonce).unwrap(); let client_timestamp_ns = rand_u64(); let signature_bytes = rand_vec(); diff --git a/xmtp_id/src/associations/state.rs b/xmtp_id/src/associations/state.rs index 776d2b057..97801dacd 100644 --- a/xmtp_id/src/associations/state.rs +++ b/xmtp_id/src/associations/state.rs @@ -5,7 +5,9 @@ use std::collections::{HashMap, HashSet}; -use super::{hashes::generate_inbox_id, member::Member, MemberIdentifier, MemberKind}; +use super::{ + hashes::generate_inbox_id, member::Member, AssociationError, MemberIdentifier, MemberKind, +}; #[derive(Debug, Clone)] pub struct AssociationStateDiff { @@ -179,16 +181,20 @@ impl AssociationState { } } - pub fn new(account_address: String, nonce: u64, chain_id: Option) -> Self { - let inbox_id = generate_inbox_id(&account_address, &nonce); + pub fn new( + account_address: String, + nonce: u64, + chain_id: Option, + ) -> Result { + let inbox_id = generate_inbox_id(&account_address, &nonce)?; let identifier = MemberIdentifier::Address(account_address.clone()); let new_member = Member::new(identifier.clone(), None, None, chain_id); - Self { + Ok(Self { members: HashMap::from_iter([(identifier, new_member)]), seen_signatures: HashSet::new(), recovery_address: account_address.to_lowercase(), inbox_id, - } + }) } } @@ -204,7 +210,7 @@ pub(crate) mod tests { #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)] #[cfg_attr(not(target_arch = "wasm32"), test)] fn can_add_remove() { - let starting_state = AssociationState::new(rand_string(), 0, None); + let starting_state = AssociationState::new(rand_string(), 0, None).unwrap(); let new_entity = Member::default(); let with_add = starting_state.add(new_entity.clone()); assert!(with_add.get(&new_entity.identifier).is_some()); @@ -214,7 +220,7 @@ pub(crate) mod tests { #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)] #[cfg_attr(not(target_arch = "wasm32"), test)] fn can_diff() { - let starting_state = AssociationState::new(rand_string(), 0, None); + let starting_state = AssociationState::new(rand_string(), 0, None).unwrap(); let entity_1 = Member::default(); let entity_2 = Member::default(); let entity_3 = Member::default(); diff --git a/xmtp_id/src/associations/test_utils.rs b/xmtp_id/src/associations/test_utils.rs index c431b4ad3..c76f69042 100644 --- a/xmtp_id/src/associations/test_utils.rs +++ b/xmtp_id/src/associations/test_utils.rs @@ -15,17 +15,19 @@ use ethers::{ signers::{LocalWallet, Signer}, types::Bytes, }; -use rand::{distributions::Alphanumeric, Rng}; +use rand::Rng; use sha2::{Digest, Sha512}; pub fn rand_string() -> String { - let v: String = rand::thread_rng() - .sample_iter(&Alphanumeric) - .take(32) - .map(char::from) + let hex_chars = "0123456789abcdef"; + let v: String = (0..40) + .map(|_| { + let idx = rand::thread_rng().gen_range(0..hex_chars.len()); + hex_chars.chars().nth(idx).unwrap() + }) .collect(); - v.to_lowercase() + format!("0x{}", v) } pub fn rand_u64() -> u64 { diff --git a/xmtp_id/src/associations/unsigned_actions.rs b/xmtp_id/src/associations/unsigned_actions.rs index 1287ad806..a19373f6f 100644 --- a/xmtp_id/src/associations/unsigned_actions.rs +++ b/xmtp_id/src/associations/unsigned_actions.rs @@ -149,16 +149,17 @@ pub(crate) mod tests { #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)] #[cfg_attr(not(target_arch = "wasm32"), test)] fn create_signatures() { - let account_address = "0x123".to_string(); + let account_address = "0x1234567890abcdef1234567890abcdef12345678".to_string(); let client_timestamp_ns: u64 = 12; - let new_member_address = "0x456".to_string(); - let new_recovery_address = "0x789".to_string(); + let new_member_address = "0x4567890abcdef1234567890abcdef12345678123".to_string(); + let new_recovery_address = "0x7890abcdef1234567890abcdef12345678123456".to_string(); let new_installation_id = vec![1, 2, 3]; let create_inbox = UnsignedCreateInbox { nonce: 0, account_address: account_address.clone(), }; - let inbox_id = generate_inbox_id(&create_inbox.account_address, &create_inbox.nonce); + let inbox_id = + generate_inbox_id(&create_inbox.account_address, &create_inbox.nonce).unwrap(); let add_address = UnsignedAddAssociation { new_member_identifier: MemberIdentifier::Address(new_member_address.clone()), @@ -195,21 +196,21 @@ pub(crate) mod tests { let signature_text = identity_update.signature_text(); let expected_text = "XMTP : Authenticate to inbox -Inbox ID: 0b3a92b07ade747bc8d601ac6e173a4f3496f908395496c053b80458a39e1ced +Inbox ID: fcd18d86276d7a99fe522dba9660c420f03c8648785ada7c5daae232a3df77a9 Current time: 1970-01-01T00:00:00Z - Create inbox - (Owner: 0x123) + (Owner: 0x1234567890abcdef1234567890abcdef12345678) - Link address to inbox - (Address: 0x456) + (Address: 0x4567890abcdef1234567890abcdef12345678123) - Grant messaging access to app (ID: 010203) - Unlink address from inbox - (Address: 0x456) + (Address: 0x4567890abcdef1234567890abcdef12345678123) - Revoke messaging access from app (ID: 010203) - Change inbox recovery address - (Address: 0x789) + (Address: 0x7890abcdef1234567890abcdef12345678123456) For more info: https://xmtp.org/signatures"; assert_eq!(signature_text, expected_text) diff --git a/xmtp_id/src/associations/unverified.rs b/xmtp_id/src/associations/unverified.rs index 0dfe9e130..c5cbeaf9b 100644 --- a/xmtp_id/src/associations/unverified.rs +++ b/xmtp_id/src/associations/unverified.rs @@ -411,7 +411,7 @@ mod tests { let account_address = rand_string(); let nonce = 1; let update = UnverifiedIdentityUpdate { - inbox_id: generate_inbox_id(account_address.as_str(), &nonce), + inbox_id: generate_inbox_id(account_address.as_str(), &nonce).unwrap(), client_timestamp_ns: 10, actions: vec![UnverifiedAction::CreateInbox(UnverifiedCreateInbox { unsigned_action: UnsignedCreateInbox { diff --git a/xmtp_mls/src/builder.rs b/xmtp_mls/src/builder.rs index bb60d7082..85d060daf 100644 --- a/xmtp_mls/src/builder.rs +++ b/xmtp_mls/src/builder.rs @@ -342,7 +342,7 @@ pub(crate) mod tests { strategy: { let (legacy_key, legacy_account_address) = generate_random_legacy_key().await; IdentityStrategy::CreateIfNotFound( - generate_inbox_id(&legacy_account_address, &1), + generate_inbox_id(&legacy_account_address, &1).unwrap(), legacy_account_address.clone(), 1, Some(legacy_key), @@ -354,7 +354,7 @@ pub(crate) mod tests { strategy: { let (legacy_key, legacy_account_address) = generate_random_legacy_key().await; IdentityStrategy::CreateIfNotFound( - generate_inbox_id(&legacy_account_address, &1), + generate_inbox_id(&legacy_account_address, &1).unwrap(), legacy_account_address.clone(), 0, Some(legacy_key), @@ -366,7 +366,7 @@ pub(crate) mod tests { strategy: { let (legacy_key, legacy_account_address) = generate_random_legacy_key().await; IdentityStrategy::CreateIfNotFound( - generate_inbox_id(&legacy_account_address, &0), + generate_inbox_id(&legacy_account_address, &0).unwrap(), legacy_account_address.clone(), 0, Some(legacy_key), @@ -379,7 +379,7 @@ pub(crate) mod tests { strategy: { let account_address = generate_local_wallet().get_address(); IdentityStrategy::CreateIfNotFound( - generate_inbox_id(&account_address, &1), + generate_inbox_id(&account_address, &1).unwrap(), account_address.clone(), 0, None, @@ -392,7 +392,7 @@ pub(crate) mod tests { let nonce = 1; let account_address = generate_local_wallet().get_address(); IdentityStrategy::CreateIfNotFound( - generate_inbox_id(&account_address, &nonce), + generate_inbox_id(&account_address, &nonce).unwrap(), account_address.clone(), nonce, None, @@ -405,7 +405,7 @@ pub(crate) mod tests { let nonce = 0; let account_address = generate_local_wallet().get_address(); IdentityStrategy::CreateIfNotFound( - generate_inbox_id(&account_address, &nonce), + generate_inbox_id(&account_address, &nonce).unwrap(), account_address.clone(), nonce, None, @@ -445,7 +445,7 @@ pub(crate) mod tests { async fn test_2nd_time_client_creation() { let (legacy_key, legacy_account_address) = generate_random_legacy_key().await; let identity_strategy = IdentityStrategy::CreateIfNotFound( - generate_inbox_id(&legacy_account_address, &0), + generate_inbox_id(&legacy_account_address, &0).unwrap(), legacy_account_address.clone(), 0, Some(legacy_key.clone()), @@ -478,7 +478,7 @@ pub(crate) mod tests { assert!(client1.installation_public_key() == client2.installation_public_key()); let client3 = ClientBuilder::new(IdentityStrategy::CreateIfNotFound( - generate_inbox_id(&legacy_account_address, &0), + generate_inbox_id(&legacy_account_address, &0).unwrap(), legacy_account_address.to_string(), 0, None, @@ -494,7 +494,7 @@ pub(crate) mod tests { assert!(client1.installation_public_key() == client3.installation_public_key()); let client4 = ClientBuilder::new(IdentityStrategy::CreateIfNotFound( - generate_inbox_id(&legacy_account_address, &0), + generate_inbox_id(&legacy_account_address, &0).unwrap(), legacy_account_address.to_string(), 0, Some(legacy_key), @@ -527,7 +527,7 @@ pub(crate) mod tests { .unwrap(); let nonce = 0; let address = generate_local_wallet().get_address(); - let inbox_id = generate_inbox_id(&address, &nonce); + let inbox_id = generate_inbox_id(&address, &nonce).unwrap(); let address_cloned = address.clone(); let inbox_id_cloned = inbox_id.clone(); @@ -569,7 +569,7 @@ pub(crate) mod tests { .unwrap(); let nonce = 0; let address = generate_local_wallet().get_address(); - let inbox_id = generate_inbox_id(&address, &nonce); + let inbox_id = generate_inbox_id(&address, &nonce).unwrap(); let address_cloned = address.clone(); let inbox_id_cloned = inbox_id.clone(); @@ -609,7 +609,7 @@ pub(crate) mod tests { .unwrap(); let nonce = 0; let address = generate_local_wallet().get_address(); - let inbox_id = generate_inbox_id(&address, &nonce); + let inbox_id = generate_inbox_id(&address, &nonce).unwrap(); let stored: StoredIdentity = (&Identity { inbox_id: inbox_id.clone(), @@ -638,7 +638,7 @@ pub(crate) mod tests { let nonce = 0; let address = generate_local_wallet().get_address(); - let stored_inbox_id = generate_inbox_id(&address, &nonce); + let stored_inbox_id = generate_inbox_id(&address, &nonce).unwrap(); let tmpdb = tmp_path(); let store = EncryptedMessageStore::new( @@ -688,7 +688,7 @@ pub(crate) mod tests { .unwrap(); let nonce = 1; - let inbox_id = generate_inbox_id(&wallet.get_address(), &nonce); + let inbox_id = generate_inbox_id(&wallet.get_address(), &nonce).unwrap(); let client_a = Client::builder(IdentityStrategy::CreateIfNotFound( inbox_id.clone(), wallet.get_address(), @@ -800,7 +800,7 @@ pub(crate) mod tests { let account_id = AccountId::new_evm(anvil_meta.chain_id, account_address.clone()); let identity_strategy = IdentityStrategy::CreateIfNotFound( - generate_inbox_id(&account_address, &0), + generate_inbox_id(&account_address, &0).unwrap(), account_address, 0, None, diff --git a/xmtp_mls/src/identity.rs b/xmtp_mls/src/identity.rs index 7761a094a..b273d3a26 100644 --- a/xmtp_mls/src/identity.rs +++ b/xmtp_mls/src/identity.rs @@ -37,6 +37,7 @@ use thiserror::Error; use tracing::debug; use tracing::info; use xmtp_id::associations::unverified::{UnverifiedInstallationKeySignature, UnverifiedSignature}; +use xmtp_id::associations::AssociationError; use xmtp_id::scw_verifier::SmartContractSignatureVerifier; use xmtp_id::{ associations::{ @@ -187,6 +188,8 @@ pub enum IdentityError { NewIdentity(String), #[error(transparent)] DieselResult(#[from] diesel::result::Error), + #[error(transparent)] + Association(#[from] AssociationError), } impl RetryableError for IdentityError { @@ -291,7 +294,8 @@ impl Identity { )); } // If the inbox_id found on the network does not match the one generated from the address and nonce, we must error - if inbox_id != generate_inbox_id(&address, &nonce) { + let generated_inbox_id = generate_inbox_id(&address, &nonce)?; + if inbox_id != generated_inbox_id { return Err(IdentityError::NewIdentity( "Inbox ID doesn't match nonce & address".to_string(), )); @@ -343,12 +347,12 @@ impl Identity { Ok(identity) } else { - if inbox_id != generate_inbox_id(&address, &nonce) { + let generated_inbox_id = generate_inbox_id(&address, &nonce)?; + if inbox_id != generated_inbox_id { return Err(IdentityError::NewIdentity( "Inbox ID doesn't match nonce & address".to_string(), )); } - let inbox_id = generate_inbox_id(&address, &nonce); let mut builder = SignatureRequestBuilder::new(inbox_id.clone()); builder = builder.create_inbox(member_identifier.clone(), nonce); diff --git a/xmtp_mls/src/identity_updates.rs b/xmtp_mls/src/identity_updates.rs index 76030e20c..7183c31c7 100644 --- a/xmtp_mls/src/identity_updates.rs +++ b/xmtp_mls/src/identity_updates.rs @@ -247,7 +247,7 @@ where maybe_nonce: Option, ) -> Result { let nonce = maybe_nonce.unwrap_or(0); - let inbox_id = generate_inbox_id(&wallet_address, &nonce); + let inbox_id = generate_inbox_id(&wallet_address, &nonce)?; let installation_public_key = self.identity().installation_keys.public(); let member_identifier: MemberIdentifier = wallet_address.to_lowercase().into(); diff --git a/xmtp_mls/src/storage/encrypted_store/association_state.rs b/xmtp_mls/src/storage/encrypted_store/association_state.rs index e4f2c2620..e37f30c37 100644 --- a/xmtp_mls/src/storage/encrypted_store/association_state.rs +++ b/xmtp_mls/src/storage/encrypted_store/association_state.rs @@ -131,7 +131,12 @@ pub(crate) mod tests { #[cfg_attr(not(target_arch = "wasm32"), tokio::test)] async fn test_batch_read() { with_connection(|conn| { - let association_state = AssociationState::new("1234".to_string(), 0, None); + let association_state = AssociationState::new( + "0x1234567890abcdef1234567890abcdef12345678".to_string(), + 0, + None, + ) + .unwrap(); let inbox_id = association_state.inbox_id().clone(); StoredAssociationState::write_to_cache( conn, @@ -141,7 +146,12 @@ pub(crate) mod tests { ) .unwrap(); - let association_state_2 = AssociationState::new("456".to_string(), 2, None); + let association_state_2 = AssociationState::new( + "0x4567890abcdef1234567890abcdef12345678123".to_string(), + 2, + None, + ) + .unwrap(); let inbox_id_2 = association_state_2.inbox_id().clone(); StoredAssociationState::write_to_cache( conn, diff --git a/xmtp_mls/src/utils/test/mod.rs b/xmtp_mls/src/utils/test/mod.rs index 1effb52f0..d86a8177b 100755 --- a/xmtp_mls/src/utils/test/mod.rs +++ b/xmtp_mls/src/utils/test/mod.rs @@ -156,7 +156,7 @@ where A: XmtpApi, { let nonce = 1; - let inbox_id = generate_inbox_id(&owner.get_address(), &nonce); + let inbox_id = generate_inbox_id(&owner.get_address(), &nonce).unwrap(); let client = Client::::builder(IdentityStrategy::CreateIfNotFound( inbox_id, @@ -188,7 +188,7 @@ where V: SmartContractSignatureVerifier, { let nonce = 1; - let inbox_id = generate_inbox_id(&owner.get_address(), &nonce); + let inbox_id = generate_inbox_id(&owner.get_address(), &nonce).unwrap(); let client = Client::::builder(IdentityStrategy::CreateIfNotFound( inbox_id,