Accept EcdsaCompact signatures for legacy keys (#508)

Changes include:
- Update example app to use v2 legacy signing flow
- Update error messages in libxmtp to be more descriptive, as well as add more logging
- Prefix libxmtp logs with `[libxmtp]` to make filtering easier
- Accept `EcdsaCompact` signatures for legacy keys in addition to the existing `WalletEcdsaCompact`

I'm not sure if there is an interop bug in Android somewhere, but it's better for Rust to just be permissive with the signatures here, and we can loop back if there is truly an issue in Android.

_____
My notes on `EcdsaCompact` for posterity:

it looks like JS will throw if WalletEcdsaCompact is not set: https://github.com/xmtp/xmtp-js/blob/main/src/crypto/PublicKey.ts#L145. However, they map ecdsaCompact from the legacy key to walletEcdsaCompact when translating from PrivateKeyBundleV1 to a PrivateKeyBundleV2 : https://github.com/xmtp/xmtp-js/blob/main/src/crypto/PublicKey.ts#L204

I think the bug in Android is the translation step from V1 to V2, which is copying the old signature as-is: https://github.com/xmtp/xmtp-android/blob/edd7eb27dd30a557be6ef81f0a4221c5ebfbb259/library/src/main/java/org/xmtp/android/library/messages/SignedPrivateKey.kt#L17

I'm not sure whether this breaks anything in practice, because I'm not sure whether Android uploads the resulting V2 bundle to the network afterwards, or just consumes it locally on the client.
____

Also for the V2 bundles, it seems we're leaving it up to the integrator to implement SigningKey: https://github.com/xmtp/xmtp-android?tab=readme-ov-file#create-a-client

The SigningKey interface basically leaves it up to them whether they set EcdsaCompact or WalletEcdsaCompact, for example this is a SigningKey implementation: https://github.com/xmtp/xmtp-android/blob/edd7eb27dd30a557be6ef81f0a4221c5ebfbb259/library/src/main/java/org/xmtp/android/library/messages/PrivateKey.kt#L88

I'm not sure whether or not the SDK gracefully handles that case

Author: richardhuaaa

bindings_ffi/examples/MainActivity.kt

@@ -5,19 +5,24 @@ import android.util.Log
 import android.widget.TextView
 import androidx.appcompat.app.AppCompatActivity
 import com.example.xmtpv3_example.R.id.selftest_output
+import java.io.File
+import java.nio.charset.StandardCharsets
+import java.security.SecureRandom
 import kotlinx.coroutines.runBlocking
 import org.bouncycastle.util.encoders.Hex.toHexString
 import org.web3j.crypto.Credentials
 import org.web3j.crypto.ECKeyPair
 import org.web3j.crypto.Sign
+import org.xmtp.android.library.Client
+import org.xmtp.android.library.ClientOptions
+import org.xmtp.android.library.XMTPEnvironment
+import org.xmtp.android.library.messages.PrivateKeyBuilder
+import org.xmtp.android.library.messages.toV2
 import uniffi.xmtpv3.FfiConversationCallback
 import uniffi.xmtpv3.FfiGroup
 import uniffi.xmtpv3.FfiInboxOwner
 import uniffi.xmtpv3.FfiLogger
 import uniffi.xmtpv3.LegacyIdentitySource
-import java.io.File
-import java.nio.charset.StandardCharsets
-import java.security.SecureRandom
 
 const val EMULATOR_LOCALHOST_ADDRESS = "http://10.0.2.2:5556"
 const val DEV_NETWORK_ADDRESS = "https://dev.xmtp.network:5556"
@@ -40,9 +45,15 @@ class AndroidFfiLogger : FfiLogger {
     }
 }
 
-class ConversationCallback: FfiConversationCallback {
+class ConversationCallback : FfiConversationCallback {
     override fun onConversation(conversation: FfiGroup) {
-        Log.i("App", "INFO - Conversation callback with ID: " + toHexString(conversation.id()) + ", members: " + conversation.listMembers())
+        Log.i(
+                "App",
+                "INFO - Conversation callback with ID: " +
+                        toHexString(conversation.id()) +
+                        ", members: " +
+                        conversation.listMembers()
+        )
     }
 }
 
@@ -54,44 +65,61 @@ class MainActivity : AppCompatActivity() {
         setContentView(R.layout.activity_main)
 
         val textView: TextView = findViewById<TextView>(selftest_output)
-        val privateKey: ByteArray = SecureRandom().generateSeed(32)
-        val credentials: Credentials = Credentials.create(ECKeyPair.create(privateKey))
-        val inboxOwner = Web3jInboxOwner(credentials)
         val dbDir: File = File(this.filesDir.absolutePath, "xmtp_db")
+        try {
+            dbDir.deleteRecursively()
+        } catch (e: Exception) {}
         dbDir.mkdir()
         val dbPath: String = dbDir.absolutePath + "/android_example.db3"
         val dbEncryptionKey = SecureRandom().generateSeed(32)
         Log.i(
-            "App",
-            "INFO -\naccountAddress: " + inboxOwner.getAddress() + "\nprivateKey: " + privateKey.asList() + "\nDB path: " + dbPath + "\nDB encryption key: " + dbEncryptionKey
+                "App",
+                "INFO -\nDB path: " +
+                        dbPath +
+                        "\nDB encryption key: " +
+                        dbEncryptionKey
         )
 
         runBlocking {
             try {
-                val client = uniffi.xmtpv3.createClient(
-                    AndroidFfiLogger(),
-                    EMULATOR_LOCALHOST_ADDRESS,
-                    false,
-                    dbPath,
-                    dbEncryptionKey,
-                    inboxOwner.getAddress(),
-                    LegacyIdentitySource.NONE,
-                    null,
-                )
-                var walletSignature: ByteArray? = null;
-                val textToSign = client.textToSign();
+                val key = PrivateKeyBuilder()
+                val client =
+                        uniffi.xmtpv3.createClient(
+                                AndroidFfiLogger(),
+                                EMULATOR_LOCALHOST_ADDRESS,
+                                false,
+                                dbPath,
+                                dbEncryptionKey,
+                                key.address,
+                                LegacyIdentitySource.KEY_GENERATOR,
+                                getV2SerializedSignedPrivateKey(key),
+                        )
+                var walletSignature: ByteArray? = null
+                val textToSign = client.textToSign()
                 if (textToSign != null) {
-                    walletSignature = inboxOwner.sign(textToSign)
+                    walletSignature = key.sign(textToSign).toByteArray()
                 }
                 client.registerIdentity(walletSignature);
                 textView.text = "Libxmtp version\n" + uniffi.xmtpv3.getVersionInfo() + "\n\nClient constructed, wallet address: " + client.accountAddress()
                 Log.i("App", "Setting up conversation streaming")
-                client.conversations().stream(ConversationCallback());
+                client.conversations().stream(ConversationCallback())
             } catch (e: Exception) {
                 textView.text = "Failed to construct client: " + e.message
             }
         }
+    }
 
-        dbDir.deleteRecursively()
+    fun getV2SerializedSignedPrivateKey(key: PrivateKeyBuilder): ByteArray {
+        val options =
+            ClientOptions(
+                api =
+                ClientOptions.Api(
+                    env = XMTPEnvironment.LOCAL,
+                    isSecure = false,
+                ),
+                appContext = this@MainActivity
+            )
+        val client = Client().create(account = key, options = options)
+        return client.privateKeyBundleV1.toV2().identityKey.toByteArray();
     }
 }

bindings_ffi/jniLibs/arm64-v8a/libuniffi_xmtpv3.so

@@ -20,7 +20,7 @@ impl log::Log for RustLogger {
             self.logger.lock().expect("Logger mutex is poisoned!").log(
                 record.level() as u32,
                 record.level().to_string(),
-                record.args().to_string(),
+                format!("[libxmtp] {}", record.args().to_string()),
             );
         }
     }

bindings_ffi/src/logger.rs

@@ -616,6 +616,41 @@ mod tests {
         assert!(!client.account_address().is_empty());
     }
 
+    #[tokio::test]
+    async fn test_legacy_identity() {
+        let legacy_address = "0x419cb1fa5635b0c6df47c9dc5765c8f1f4dff78e";
+        let legacy_signed_private_key_proto = vec![
+            8, 128, 154, 196, 133, 220, 244, 197, 216, 23, 18, 34, 10, 32, 214, 70, 104, 202, 68,
+            204, 25, 202, 197, 141, 239, 159, 145, 249, 55, 242, 147, 126, 3, 124, 159, 207, 96,
+            135, 134, 122, 60, 90, 82, 171, 131, 162, 26, 153, 1, 10, 79, 8, 128, 154, 196, 133,
+            220, 244, 197, 216, 23, 26, 67, 10, 65, 4, 232, 32, 50, 73, 113, 99, 115, 168, 104,
+            229, 206, 24, 217, 132, 223, 217, 91, 63, 137, 136, 50, 89, 82, 186, 179, 150, 7, 127,
+            140, 10, 165, 117, 233, 117, 196, 134, 227, 143, 125, 210, 187, 77, 195, 169, 162, 116,
+            34, 20, 196, 145, 40, 164, 246, 139, 197, 154, 233, 190, 148, 35, 131, 240, 106, 103,
+            18, 70, 18, 68, 10, 64, 90, 24, 36, 99, 130, 246, 134, 57, 60, 34, 142, 165, 221, 123,
+            63, 27, 138, 242, 195, 175, 212, 146, 181, 152, 89, 48, 8, 70, 104, 94, 163, 0, 25,
+            196, 228, 190, 49, 108, 141, 60, 174, 150, 177, 115, 229, 138, 92, 105, 170, 226, 204,
+            249, 206, 12, 37, 145, 3, 35, 226, 15, 49, 20, 102, 60, 16, 1,
+        ];
+
+        let client = create_client(
+            Box::new(MockLogger {}),
+            xmtp_api_grpc::LOCALHOST_ADDRESS.to_string(),
+            false,
+            Some(tmp_path()),
+            None,
+            legacy_address.to_string(),
+            LegacyIdentitySource::KeyGenerator,
+            Some(legacy_signed_private_key_proto),
+        )
+        .await
+        .unwrap();
+
+        assert!(client.text_to_sign().is_none());
+        client.register_identity(None).await.unwrap();
+        assert_eq!(client.account_address(), legacy_address);
+    }
+
     #[tokio::test(flavor = "multi_thread", worker_threads = 1)]
     async fn test_create_client_with_storage() {
         let ffi_inbox_owner = LocalWalletInboxOwner::new();

bindings_ffi/src/mls.rs

@@ -9,7 +9,7 @@ android {
 
     defaultConfig {
         applicationId "com.example.xmtpv3_example"
-        minSdk 21
+        minSdk 23
         targetSdk 33
         versionCode 1
         versionName "1.0"
@@ -37,6 +37,7 @@ dependencies {
     implementation 'androidx.constraintlayout:constraintlayout:2.1.4'
     implementation 'androidx.core:core-ktx:1.7.0'
     implementation 'com.google.android.material:material:1.8.0'
+    implementation "org.xmtp:android:0.7.10"
     implementation "net.java.dev.jna:jna:5.13.0@aar"
     implementation 'org.web3j:crypto:5.0.0'
     testImplementation 'junit:junit:4.13.2'

examples/android/xmtpv3_example/app/build.gradle

@@ -1,6 +1,6 @@
 // Top-level build file where you can add configuration options common to all sub-projects/modules.
 plugins {
-    id 'com.android.application' version '7.3.1' apply false
-    id 'com.android.library' version '7.3.1' apply false
+    id 'com.android.application' version '8.0.0-rc01' apply false
+    id 'com.android.library' version '8.0.0-rc01' apply false
     id 'org.jetbrains.kotlin.android' version '1.7.20' apply false
 }

examples/android/xmtpv3_example/build.gradle

@@ -20,4 +20,6 @@ kotlin.code.style=official
 # Enables namespacing of each library's R class so that its R class includes only the
 # resources declared in the library itself and none from the library's dependencies,
 # thereby reducing the size of the R class for that library
-android.nonTransitiveRClass=true
+android.nonTransitiveRClass=true
+android.defaults.buildfeatures.buildconfig=true
+android.nonFinalResIds=false

examples/android/xmtpv3_example/gradle.properties

@@ -1,6 +1,6 @@
 #Wed Mar 29 17:13:20 PDT 2023
 distributionBase=GRADLE_USER_HOME
-distributionUrl=https\://services.gradle.org/distributions/gradle-7.4-bin.zip
+distributionUrl=https\://services.gradle.org/distributions/gradle-8.0-bin.zip
 distributionPath=wrapper/dists
 zipStorePath=wrapper/dists
 zipStoreBase=GRADLE_USER_HOME

examples/android/xmtpv3_example/gradle/wrapper/gradle-wrapper.properties

@@ -3,6 +3,7 @@ use std::println as debug;
 
 #[cfg(not(test))]
 use log::debug;
+use log::info;
 use thiserror::Error;
 
 use xmtp_proto::api_client::XmtpMlsClient;
@@ -38,7 +39,7 @@ pub enum ClientBuilderError {
     // AssociationFailed(#[from] AssociationError),
     // #[error("Error Initializing Store")]
     // StoreInitialization(#[from] SE),
-    #[error("Error Initalizing Identity")]
+    #[error("Error initializing identity: {0}")]
     IdentityInitialization(#[from] IdentityError),
 
     #[error("Storage Error")]
@@ -82,6 +83,7 @@ impl IdentityStrategy {
         api_client: &ApiClientWrapper<ApiClient>,
         store: &EncryptedMessageStore,
     ) -> Result<Identity, ClientBuilderError> {
+        info!("Initializing identity");
         let conn = store.conn()?;
         let provider = XmtpOpenMlsProvider::new(&conn);
         let identity_option: Option<Identity> = provider
@@ -117,6 +119,7 @@ impl IdentityStrategy {
         account_address: String,
         legacy_identity: LegacyIdentity,
     ) -> Result<Identity, ClientBuilderError> {
+        info!("Creating identity");
         let identity = match legacy_identity {
             // This is a fresh install, and at most one v2 signature (enable_identity)
             // has been requested so far, so it's fine to request another one (grant_messaging_access).

xmtp_mls/src/builder.rs

@@ -42,7 +42,9 @@ impl LegacyCreateIdentityAssociation {
             LegacySignedPrivateKeyProto::decode(legacy_signed_private_key.as_slice())?;
         let signed_private_key::Union::Secp256k1(secp256k1) = legacy_signed_private_key_proto
             .union
-            .ok_or(AssociationError::MalformedLegacyKey)?;
+            .ok_or(AssociationError::MalformedLegacyKey(
+                "Missing secp256k1.union field".to_string(),
+            ))?;
         let legacy_private_key = secp256k1.bytes;
         let (mut delegating_signature, recovery_id) = k256_helper::sign_sha256(
             &legacy_private_key,      // secret_key
@@ -51,9 +53,9 @@ impl LegacyCreateIdentityAssociation {
         .map_err(AssociationError::LegacySignature)?;
         delegating_signature.push(recovery_id); // TODO: normalize recovery ID if necessary
 
-        let legacy_signed_public_key_proto = legacy_signed_private_key_proto
-            .public_key
-            .ok_or(AssociationError::MalformedLegacyKey)?;
+        let legacy_signed_public_key_proto = legacy_signed_private_key_proto.public_key.ok_or(
+            AssociationError::MalformedLegacyKey("Missing public_key field".to_string()),
+        )?;
         Self::new_validated(
             installation_public_key,
             delegating_signature,

xmtp_mls/src/credential/legacy_create_identity_association.rs

@@ -25,10 +25,10 @@ use self::legacy_create_identity_association::LegacyCreateIdentityAssociation;
 pub enum AssociationError {
     #[error("bad signature")]
     BadSignature(#[from] SignatureError),
-    #[error("decode error")]
+    #[error("decode error: {0}")]
     DecodeError(#[from] DecodeError),
-    #[error("legacy key")]
-    MalformedLegacyKey,
+    #[error("legacy key: {0}")]
+    MalformedLegacyKey(String),
     #[error("legacy signature: {0}")]
     LegacySignature(String),
     #[error("Association text mismatch")]

xmtp_mls/src/credential/mod.rs

@@ -1,3 +1,4 @@
+use log::info;
 use prost::Message;
 
 use xmtp_cryptography::signature::RecoverableSignature;
@@ -61,19 +62,35 @@ impl TryFrom<LegacySignedPublicKeyProto> for ValidatedLegacySignedPublicKey {
 
     fn try_from(proto: LegacySignedPublicKeyProto) -> Result<Self, AssociationError> {
         let serialized_key_data = proto.key_bytes;
-        let Union::WalletEcdsaCompact(wallet_ecdsa_compact) = proto
+        let union = proto
             .signature
-            .ok_or(AssociationError::MalformedLegacyKey)?
+            .ok_or(AssociationError::MalformedLegacyKey(
+                "Missing signature field".to_string(),
+            ))?
             .union
-            .ok_or(AssociationError::MalformedLegacyKey)?
-        else {
-            return Err(AssociationError::MalformedLegacyKey);
+            .ok_or(AssociationError::MalformedLegacyKey(
+                "Missing signature.union field".to_string(),
+            ))?;
+        let wallet_signature = match union {
+            Union::WalletEcdsaCompact(wallet_ecdsa_compact) => {
+                info!("Reading WalletEcdsaCompact from legacy key");
+                let mut wallet_signature = wallet_ecdsa_compact.bytes.clone();
+                wallet_signature.push(wallet_ecdsa_compact.recovery as u8); // TODO: normalize recovery ID if necessary
+                if wallet_signature.len() != 65 {
+                    return Err(AssociationError::MalformedAssociation);
+                }
+                wallet_signature
+            }
+            Union::EcdsaCompact(ecdsa_compact) => {
+                info!("Reading EcdsaCompact from legacy key");
+                let mut signature = ecdsa_compact.bytes.clone();
+                signature.push(ecdsa_compact.recovery as u8); // TODO: normalize recovery ID if necessary
+                if signature.len() != 65 {
+                    return Err(AssociationError::MalformedAssociation);
+                }
+                signature
+            }
         };
-        let mut wallet_signature = wallet_ecdsa_compact.bytes.clone();
-        wallet_signature.push(wallet_ecdsa_compact.recovery as u8); // TODO: normalize recovery ID if necessary
-        if wallet_signature.len() != 65 {
-            return Err(AssociationError::MalformedAssociation);
-        }
         let wallet_signature = RecoverableSignature::Eip191Signature(wallet_signature);
         let account_address =
             wallet_signature.recover_address(&Self::text(&serialized_key_data))?;