From 1a5d0ebe33fda7280bb1c47d63cea01adbbc39dd Mon Sep 17 00:00:00 2001 From: Nicholas Molnar <65710+neekolas@users.noreply.github.com> Date: Mon, 19 Aug 2024 15:03:13 -0700 Subject: [PATCH 01/10] Use latest MLS --- Cargo.lock | 59 ++++++---------- Cargo.toml | 22 +++--- bindings_ffi/Cargo.lock | 67 +++++++----------- bindings_node/Cargo.lock | 59 ++++++---------- xmtp_mls/src/client.rs | 4 +- xmtp_mls/src/groups/mod.rs | 2 +- xmtp_mls/src/groups/sync.rs | 8 ++- xmtp_mls/src/storage/errors.rs | 10 +-- xmtp_mls/src/storage/sql_key_store.rs | 98 ++++++--------------------- 9 files changed, 107 insertions(+), 222 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f2c3a428a..1a18b2acf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2236,7 +2236,7 @@ dependencies = [ "hpke-rs-crypto", "log", "serde", - "tls_codec 0.4.1", + "tls_codec", "zeroize", ] @@ -3246,9 +3246,10 @@ dependencies = [ [[package]] name = "openmls" -version = "0.5.0" -source = "git+https://github.com/xmtp/openmls?rev=9cb3207#9cb3207b077fcf6bc327408dfcf3df6237aec49c" +version = "0.6.0-pre.2" +source = "git+https://github.com/xmtp/openmls?rev=cf42738018d093434c955a1b50a9de34cc12b8c5#cf42738018d093434c955a1b50a9de34cc12b8c5" dependencies = [ + "backtrace", "itertools 0.10.5", "log", "once_cell", @@ -3262,27 +3263,27 @@ dependencies = [ "serde", "serde_json", "thiserror", - "tls_codec 0.4.2-pre.1", + "tls_codec", "wasm-bindgen-test", ] [[package]] name = "openmls_basic_credential" -version = "0.2.0" -source = "git+https://github.com/xmtp/openmls?rev=9cb3207#9cb3207b077fcf6bc327408dfcf3df6237aec49c" +version = "0.3.0-pre.1" +source = "git+https://github.com/xmtp/openmls?rev=cf42738018d093434c955a1b50a9de34cc12b8c5#cf42738018d093434c955a1b50a9de34cc12b8c5" dependencies = [ "ed25519-dalek", "openmls_traits", "p256", "rand", "serde", - "tls_codec 0.4.2-pre.1", + "tls_codec", ] [[package]] name = "openmls_memory_storage" -version = "0.2.0" -source = "git+https://github.com/xmtp/openmls?rev=9cb3207#9cb3207b077fcf6bc327408dfcf3df6237aec49c" +version = "0.3.0-pre.2" +source = "git+https://github.com/xmtp/openmls?rev=cf42738018d093434c955a1b50a9de34cc12b8c5#cf42738018d093434c955a1b50a9de34cc12b8c5" dependencies = [ "hex", "log", @@ -3294,8 +3295,8 @@ dependencies = [ [[package]] name = "openmls_rust_crypto" -version = "0.2.0" -source = "git+https://github.com/xmtp/openmls?rev=9cb3207#9cb3207b077fcf6bc327408dfcf3df6237aec49c" +version = "0.3.0-pre.1" +source = "git+https://github.com/xmtp/openmls?rev=cf42738018d093434c955a1b50a9de34cc12b8c5#cf42738018d093434c955a1b50a9de34cc12b8c5" dependencies = [ "aes-gcm", "chacha20poly1305", @@ -3313,13 +3314,13 @@ dependencies = [ "serde", "sha2 0.10.8", "thiserror", - "tls_codec 0.4.2-pre.1", + "tls_codec", ] [[package]] name = "openmls_test" -version = "0.1.0" -source = "git+https://github.com/xmtp/openmls?rev=9cb3207#9cb3207b077fcf6bc327408dfcf3df6237aec49c" +version = "0.1.0-pre.1" +source = "git+https://github.com/xmtp/openmls?rev=cf42738018d093434c955a1b50a9de34cc12b8c5#cf42738018d093434c955a1b50a9de34cc12b8c5" dependencies = [ "ansi_term", "openmls_rust_crypto", @@ -3333,11 +3334,11 @@ dependencies = [ [[package]] name = "openmls_traits" -version = "0.2.0" -source = "git+https://github.com/xmtp/openmls?rev=9cb3207#9cb3207b077fcf6bc327408dfcf3df6237aec49c" +version = "0.3.0-pre.2" +source = "git+https://github.com/xmtp/openmls?rev=cf42738018d093434c955a1b50a9de34cc12b8c5#cf42738018d093434c955a1b50a9de34cc12b8c5" dependencies = [ "serde", - "tls_codec 0.4.2-pre.1", + "tls_codec", ] [[package]] @@ -5193,17 +5194,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b5e78c9c330f8c85b2bae7c8368f2739157db9991235123aa1b15ef9502bfb6a" dependencies = [ "serde", - "tls_codec_derive 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)", - "zeroize", -] - -[[package]] -name = "tls_codec" -version = "0.4.2-pre.1" -source = "git+https://github.com/rustcrypto/formats#6456b48674e0646b4628adb075c82a290d3160b9" -dependencies = [ - "serde", - "tls_codec_derive 0.4.1 (git+https://github.com/rustcrypto/formats)", + "tls_codec_derive", "zeroize", ] @@ -5218,16 +5209,6 @@ dependencies = [ "syn 2.0.72", ] -[[package]] -name = "tls_codec_derive" -version = "0.4.1" -source = "git+https://github.com/rustcrypto/formats#6456b48674e0646b4628adb075c82a290d3160b9" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.72", -] - [[package]] name = "tokio" version = "1.39.2" @@ -6423,7 +6404,7 @@ dependencies = [ "smart-default", "tempfile", "thiserror", - "tls_codec 0.4.1", + "tls_codec", "tokio", "tokio-stream", "toml", diff --git a/Cargo.toml b/Cargo.toml index 9ad24599a..fba795ca2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,6 +26,7 @@ resolver = "2" [workspace.dependencies] anyhow = "1.0" +async-stream = "0.3" async-trait = "0.1.77" chrono = "0.4.38" ctor = "0.2" @@ -37,10 +38,14 @@ futures = "0.3.30" futures-core = "0.3.30" hex = "0.4.3" log = { version = "0.4" } -openmls = { git = "https://github.com/xmtp/openmls", rev = "9cb3207", default-features = false } -openmls_basic_credential = { git = "https://github.com/xmtp/openmls", rev = "9cb3207" } -openmls_rust_crypto = { git = "https://github.com/xmtp/openmls", rev = "9cb3207" } -openmls_traits = { git = "https://github.com/xmtp/openmls", rev = "9cb3207" } +openmls = { git = "https://github.com/xmtp/openmls", rev = "cf42738018d093434c955a1b50a9de34cc12b8c5", default-features = false } +openmls_basic_credential = { git = "https://github.com/xmtp/openmls", rev = "cf42738018d093434c955a1b50a9de34cc12b8c5" } +openmls_rust_crypto = { git = "https://github.com/xmtp/openmls", rev = "cf42738018d093434c955a1b50a9de34cc12b8c5" } +openmls_traits = { git = "https://github.com/xmtp/openmls", rev = "cf42738018d093434c955a1b50a9de34cc12b8c5" } +pbjson = "0.6.0" +pbjson-types = "0.6.0" +prost = "^0.12" +prost-types = "^0.12" rand = "0.8.5" regex = "1.10.4" rustc-hex = "2.1.0" @@ -48,17 +53,12 @@ serde = "1.0" serde_json = "1.0" sha2 = "0.10.8" thiserror = "1.0" -tls_codec = "0.4.0" +tls_codec = "0.4.1" tokio = { version = "1.35.1", default-features = false } -async-stream = "0.3" +tonic = "^0.11" tracing = { version = "0.1" } tracing-subscriber = "0.3" url = "2.5.0" -tonic = "^0.11" -prost = "^0.12" -prost-types = "^0.12" -pbjson = "0.6.0" -pbjson-types = "0.6.0" # Internal Crate Dependencies xmtp_cryptography = { path = "xmtp_cryptography" } diff --git a/bindings_ffi/Cargo.lock b/bindings_ffi/Cargo.lock index 04299c077..8d174507a 100644 --- a/bindings_ffi/Cargo.lock +++ b/bindings_ffi/Cargo.lock @@ -2049,7 +2049,7 @@ dependencies = [ "hpke-rs-crypto", "log", "serde", - "tls_codec 0.4.0", + "tls_codec", "zeroize", ] @@ -2841,9 +2841,10 @@ dependencies = [ [[package]] name = "openmls" -version = "0.5.0" -source = "git+https://github.com/xmtp/openmls?rev=9cb3207#9cb3207b077fcf6bc327408dfcf3df6237aec49c" +version = "0.6.0-pre.2" +source = "git+https://github.com/xmtp/openmls?rev=cf42738018d093434c955a1b50a9de34cc12b8c5#cf42738018d093434c955a1b50a9de34cc12b8c5" dependencies = [ + "backtrace", "itertools 0.10.5", "log", "once_cell", @@ -2857,27 +2858,27 @@ dependencies = [ "serde", "serde_json", "thiserror", - "tls_codec 0.4.2-pre.1", + "tls_codec", "wasm-bindgen-test", ] [[package]] name = "openmls_basic_credential" -version = "0.2.0" -source = "git+https://github.com/xmtp/openmls?rev=9cb3207#9cb3207b077fcf6bc327408dfcf3df6237aec49c" +version = "0.3.0-pre.1" +source = "git+https://github.com/xmtp/openmls?rev=cf42738018d093434c955a1b50a9de34cc12b8c5#cf42738018d093434c955a1b50a9de34cc12b8c5" dependencies = [ "ed25519-dalek", "openmls_traits", "p256", "rand", "serde", - "tls_codec 0.4.2-pre.1", + "tls_codec", ] [[package]] name = "openmls_memory_storage" -version = "0.2.0" -source = "git+https://github.com/xmtp/openmls?rev=9cb3207#9cb3207b077fcf6bc327408dfcf3df6237aec49c" +version = "0.3.0-pre.2" +source = "git+https://github.com/xmtp/openmls?rev=cf42738018d093434c955a1b50a9de34cc12b8c5#cf42738018d093434c955a1b50a9de34cc12b8c5" dependencies = [ "hex", "log", @@ -2889,8 +2890,8 @@ dependencies = [ [[package]] name = "openmls_rust_crypto" -version = "0.2.0" -source = "git+https://github.com/xmtp/openmls?rev=9cb3207#9cb3207b077fcf6bc327408dfcf3df6237aec49c" +version = "0.3.0-pre.1" +source = "git+https://github.com/xmtp/openmls?rev=cf42738018d093434c955a1b50a9de34cc12b8c5#cf42738018d093434c955a1b50a9de34cc12b8c5" dependencies = [ "aes-gcm", "chacha20poly1305", @@ -2908,13 +2909,13 @@ dependencies = [ "serde", "sha2", "thiserror", - "tls_codec 0.4.2-pre.1", + "tls_codec", ] [[package]] name = "openmls_test" -version = "0.1.0" -source = "git+https://github.com/xmtp/openmls?rev=9cb3207#9cb3207b077fcf6bc327408dfcf3df6237aec49c" +version = "0.1.0-pre.1" +source = "git+https://github.com/xmtp/openmls?rev=cf42738018d093434c955a1b50a9de34cc12b8c5#cf42738018d093434c955a1b50a9de34cc12b8c5" dependencies = [ "ansi_term", "openmls_rust_crypto", @@ -2928,11 +2929,11 @@ dependencies = [ [[package]] name = "openmls_traits" -version = "0.2.0" -source = "git+https://github.com/xmtp/openmls?rev=9cb3207#9cb3207b077fcf6bc327408dfcf3df6237aec49c" +version = "0.3.0-pre.2" +source = "git+https://github.com/xmtp/openmls?rev=cf42738018d093434c955a1b50a9de34cc12b8c5#cf42738018d093434c955a1b50a9de34cc12b8c5" dependencies = [ "serde", - "tls_codec 0.4.2-pre.1", + "tls_codec", ] [[package]] @@ -4681,40 +4682,20 @@ checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20" [[package]] name = "tls_codec" -version = "0.4.0" +version = "0.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d38a1d5fcfa859f0ec2b5e111dc903890bd7dac7f34713232bf9aa4fd7cad7b2" +checksum = "b5e78c9c330f8c85b2bae7c8368f2739157db9991235123aa1b15ef9502bfb6a" dependencies = [ "serde", - "tls_codec_derive 0.4.0", - "zeroize", -] - -[[package]] -name = "tls_codec" -version = "0.4.2-pre.1" -source = "git+https://github.com/rustcrypto/formats#ef837bce43803eebc99922c6552b1cb9b2a01a03" -dependencies = [ - "serde", - "tls_codec_derive 0.4.1", + "tls_codec_derive", "zeroize", ] -[[package]] -name = "tls_codec_derive" -version = "0.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d8e00e3e7a54e0f1c8834ce72ed49c8487fbd3f801d8cfe1a0ad0640382f8e15" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.48", -] - [[package]] name = "tls_codec_derive" version = "0.4.1" -source = "git+https://github.com/rustcrypto/formats#ef837bce43803eebc99922c6552b1cb9b2a01a03" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8d9ef545650e79f30233c0003bcc2504d7efac6dad25fca40744de773fe2049c" dependencies = [ "proc-macro2", "quote", @@ -5855,7 +5836,7 @@ dependencies = [ "sha2", "smart-default", "thiserror", - "tls_codec 0.4.0", + "tls_codec", "tokio", "tokio-stream", "toml 0.8.8", diff --git a/bindings_node/Cargo.lock b/bindings_node/Cargo.lock index caa5d48cc..292ff858a 100644 --- a/bindings_node/Cargo.lock +++ b/bindings_node/Cargo.lock @@ -1869,7 +1869,7 @@ dependencies = [ "hpke-rs-crypto", "log", "serde", - "tls_codec 0.4.1", + "tls_codec", "zeroize", ] @@ -2675,9 +2675,10 @@ dependencies = [ [[package]] name = "openmls" -version = "0.5.0" -source = "git+https://github.com/xmtp/openmls?rev=9cb3207#9cb3207b077fcf6bc327408dfcf3df6237aec49c" +version = "0.6.0-pre.2" +source = "git+https://github.com/xmtp/openmls?rev=cf42738018d093434c955a1b50a9de34cc12b8c5#cf42738018d093434c955a1b50a9de34cc12b8c5" dependencies = [ + "backtrace", "itertools 0.10.5", "log", "once_cell", @@ -2691,27 +2692,27 @@ dependencies = [ "serde", "serde_json", "thiserror", - "tls_codec 0.4.2-pre.1", + "tls_codec", "wasm-bindgen-test", ] [[package]] name = "openmls_basic_credential" -version = "0.2.0" -source = "git+https://github.com/xmtp/openmls?rev=9cb3207#9cb3207b077fcf6bc327408dfcf3df6237aec49c" +version = "0.3.0-pre.1" +source = "git+https://github.com/xmtp/openmls?rev=cf42738018d093434c955a1b50a9de34cc12b8c5#cf42738018d093434c955a1b50a9de34cc12b8c5" dependencies = [ "ed25519-dalek", "openmls_traits", "p256", "rand", "serde", - "tls_codec 0.4.2-pre.1", + "tls_codec", ] [[package]] name = "openmls_memory_storage" -version = "0.2.0" -source = "git+https://github.com/xmtp/openmls?rev=9cb3207#9cb3207b077fcf6bc327408dfcf3df6237aec49c" +version = "0.3.0-pre.2" +source = "git+https://github.com/xmtp/openmls?rev=cf42738018d093434c955a1b50a9de34cc12b8c5#cf42738018d093434c955a1b50a9de34cc12b8c5" dependencies = [ "hex", "log", @@ -2723,8 +2724,8 @@ dependencies = [ [[package]] name = "openmls_rust_crypto" -version = "0.2.0" -source = "git+https://github.com/xmtp/openmls?rev=9cb3207#9cb3207b077fcf6bc327408dfcf3df6237aec49c" +version = "0.3.0-pre.1" +source = "git+https://github.com/xmtp/openmls?rev=cf42738018d093434c955a1b50a9de34cc12b8c5#cf42738018d093434c955a1b50a9de34cc12b8c5" dependencies = [ "aes-gcm", "chacha20poly1305", @@ -2742,13 +2743,13 @@ dependencies = [ "serde", "sha2", "thiserror", - "tls_codec 0.4.2-pre.1", + "tls_codec", ] [[package]] name = "openmls_test" -version = "0.1.0" -source = "git+https://github.com/xmtp/openmls?rev=9cb3207#9cb3207b077fcf6bc327408dfcf3df6237aec49c" +version = "0.1.0-pre.1" +source = "git+https://github.com/xmtp/openmls?rev=cf42738018d093434c955a1b50a9de34cc12b8c5#cf42738018d093434c955a1b50a9de34cc12b8c5" dependencies = [ "ansi_term", "openmls_rust_crypto", @@ -2762,11 +2763,11 @@ dependencies = [ [[package]] name = "openmls_traits" -version = "0.2.0" -source = "git+https://github.com/xmtp/openmls?rev=9cb3207#9cb3207b077fcf6bc327408dfcf3df6237aec49c" +version = "0.3.0-pre.2" +source = "git+https://github.com/xmtp/openmls?rev=cf42738018d093434c955a1b50a9de34cc12b8c5#cf42738018d093434c955a1b50a9de34cc12b8c5" dependencies = [ "serde", - "tls_codec 0.4.2-pre.1", + "tls_codec", ] [[package]] @@ -4359,17 +4360,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b5e78c9c330f8c85b2bae7c8368f2739157db9991235123aa1b15ef9502bfb6a" dependencies = [ "serde", - "tls_codec_derive 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)", - "zeroize", -] - -[[package]] -name = "tls_codec" -version = "0.4.2-pre.1" -source = "git+https://github.com/rustcrypto/formats#cf967322b60b14a0db800c9c2652afdb6964453c" -dependencies = [ - "serde", - "tls_codec_derive 0.4.1 (git+https://github.com/rustcrypto/formats)", + "tls_codec_derive", "zeroize", ] @@ -4384,16 +4375,6 @@ dependencies = [ "syn 2.0.64", ] -[[package]] -name = "tls_codec_derive" -version = "0.4.1" -source = "git+https://github.com/rustcrypto/formats#cf967322b60b14a0db800c9c2652afdb6964453c" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.64", -] - [[package]] name = "tokio" version = "1.37.0" @@ -5312,7 +5293,7 @@ dependencies = [ "sha2", "smart-default", "thiserror", - "tls_codec 0.4.1", + "tls_codec", "tokio", "tokio-stream", "toml", diff --git a/xmtp_mls/src/client.rs b/xmtp_mls/src/client.rs index 6157a25dc..3f6fd4375 100644 --- a/xmtp_mls/src/client.rs +++ b/xmtp_mls/src/client.rs @@ -129,9 +129,7 @@ pub enum MessageProcessingError { #[error(transparent)] Identity(#[from] IdentityError), #[error("openmls process message error: {0}")] - OpenMlsProcessMessage( - #[from] openmls::prelude::ProcessMessageError, - ), + OpenMlsProcessMessage(#[from] openmls::prelude::ProcessMessageError), #[error("merge pending commit: {0}")] MergePendingCommit( #[from] openmls::group::MergePendingCommitError, diff --git a/xmtp_mls/src/groups/mod.rs b/xmtp_mls/src/groups/mod.rs index 0ec267a7e..c3b35d8f2 100644 --- a/xmtp_mls/src/groups/mod.rs +++ b/xmtp_mls/src/groups/mod.rs @@ -107,7 +107,7 @@ pub enum GroupError { #[error("intent error: {0}")] Intent(#[from] IntentError), #[error("create message: {0}")] - CreateMessage(#[from] openmls::prelude::CreateMessageError), + CreateMessage(#[from] openmls::prelude::CreateMessageError), #[error("TLS Codec error: {0}")] TlsError(#[from] TlsCodecError), #[error("SequenceId not found in local db")] diff --git a/xmtp_mls/src/groups/sync.rs b/xmtp_mls/src/groups/sync.rs index a9e5c5839..459602a41 100644 --- a/xmtp_mls/src/groups/sync.rs +++ b/xmtp_mls/src/groups/sync.rs @@ -53,6 +53,7 @@ use openmls::{ ProcessedMessage, ProcessedMessageContent, Sender, }, prelude_test::KeyPackage, + treesync::LeafNodeParameters, }; use openmls_basic_credential::SignatureKeyPair; use openmls_traits::OpenMlsProvider; @@ -907,8 +908,11 @@ impl MlsGroup { Ok(Some((msg_bytes, None))) } IntentKind::KeyUpdate => { - let (commit, _, _) = openmls_group - .self_update(&provider, &self.context.identity.installation_keys)?; + let (commit, _, _) = openmls_group.self_update( + &provider, + &self.context.identity.installation_keys, + LeafNodeParameters::default(), + )?; Ok(Some((commit.tls_serialize_detached()?, None))) } diff --git a/xmtp_mls/src/storage/errors.rs b/xmtp_mls/src/storage/errors.rs index 6319cc9f1..43bd7efec 100644 --- a/xmtp_mls/src/storage/errors.rs +++ b/xmtp_mls/src/storage/errors.rs @@ -89,7 +89,6 @@ impl RetryableError for openmls::group::CreateCommitError bool { match self { Self::KeyStoreError(storage) => retryable!(storage), - Self::KeyPackageGenerationError(generation) => retryable!(generation), _ => false, } } @@ -134,12 +133,9 @@ impl RetryableError } } -impl RetryableError for openmls::prelude::MlsGroupStateError { +impl RetryableError for openmls::prelude::MlsGroupStateError { fn is_retryable(&self) -> bool { - match self { - Self::StorageError(storage) => retryable!(storage), - _ => false, - } + false } } @@ -206,7 +202,7 @@ impl RetryableError for openmls::group::MergePendingCommitError { +impl RetryableError for openmls::prelude::ProcessMessageError { fn is_retryable(&self) -> bool { match self { Self::GroupStateError(err) => retryable!(err), diff --git a/xmtp_mls/src/storage/sql_key_store.rs b/xmtp_mls/src/storage/sql_key_store.rs index f5d12ad37..9c19da8e5 100644 --- a/xmtp_mls/src/storage/sql_key_store.rs +++ b/xmtp_mls/src/storage/sql_key_store.rs @@ -416,7 +416,7 @@ impl StorageProvider for SqlKeyStore { .collect::, _>>() } - fn treesync< + fn tree< GroupId: traits::GroupId, TreeSync: traits::TreeSync, >( @@ -733,31 +733,6 @@ impl StorageProvider for SqlKeyStore { self.delete::(OWN_LEAF_NODE_INDEX_LABEL, &key) } - fn use_ratchet_tree_extension>( - &self, - group_id: &GroupId, - ) -> Result, Self::Error> { - let key = build_key::(USE_RATCHET_TREE_LABEL, group_id)?; - self.read(USE_RATCHET_TREE_LABEL, &key) - } - - fn set_use_ratchet_tree_extension>( - &self, - group_id: &GroupId, - value: bool, - ) -> Result<(), Self::Error> { - let key = build_key::(USE_RATCHET_TREE_LABEL, group_id)?; - self.write::(USE_RATCHET_TREE_LABEL, &key, &bincode::serialize(&value)?) - } - - fn delete_use_ratchet_tree_extension>( - &self, - group_id: &GroupId, - ) -> Result<(), Self::Error> { - let key = build_key::(USE_RATCHET_TREE_LABEL, group_id)?; - self.delete::(USE_RATCHET_TREE_LABEL, &key) - } - fn group_epoch_secrets< GroupId: traits::GroupId, GroupEpochSecrets: traits::GroupEpochSecrets, @@ -938,37 +913,6 @@ impl StorageProvider for SqlKeyStore { self.append::(OWN_LEAF_NODES_LABEL, &key, &value) } - fn aad>( - &self, - group_id: &GroupId, - ) -> Result, Self::Error> { - let key = build_key::(AAD_LABEL, group_id)?; - match self.read::>(AAD_LABEL, &key) { - Ok(Some(value)) => Ok(value), - Ok(None) => Ok(Vec::new()), - Err(e) => Err(e), - } - } - - fn write_aad>( - &self, - group_id: &GroupId, - aad: &[u8], - ) -> Result<(), Self::Error> { - let key = build_key::(AAD_LABEL, group_id)?; - let value = bincode::serialize(&aad)?; - - self.write::(AAD_LABEL, &key, &value) - } - - fn delete_aad>( - &self, - group_id: &GroupId, - ) -> Result<(), Self::Error> { - let key = build_key::(AAD_LABEL, group_id)?; - self.delete::(AAD_LABEL, &key) - } - fn delete_own_leaf_nodes>( &self, group_id: &GroupId, @@ -1130,31 +1074,31 @@ mod tests { .is_none()); } - #[test] - fn list_write_remove() { - let db_path = tmp_path(); - let store = EncryptedMessageStore::new( - StorageOption::Persistent(db_path), - EncryptedMessageStore::generate_enc_key(), - ) - .unwrap(); - let conn = store.conn().unwrap(); - let key_store = SqlKeyStore::new(conn.clone()); - let provider = XmtpOpenMlsProvider::new(conn); - let group_id = GroupId::random(provider.rand()); + // #[test] + // fn list_write_remove() { + // let db_path = tmp_path(); + // let store = EncryptedMessageStore::new( + // StorageOption::Persistent(db_path), + // EncryptedMessageStore::generate_enc_key(), + // ) + // .unwrap(); + // let conn = store.conn().unwrap(); + // let key_store = SqlKeyStore::new(conn.clone()); + // let provider = XmtpOpenMlsProvider::new(conn); + // let group_id = GroupId::random(provider.rand()); - assert!(key_store.aad::(&group_id).unwrap().is_empty()); + // assert!(key_store.aad::(&group_id).unwrap().is_empty()); - key_store - .write_aad::(&group_id, "test".as_bytes()) - .unwrap(); + // key_store + // .write_aad::(&group_id, "test".as_bytes()) + // .unwrap(); - assert!(!key_store.aad::(&group_id).unwrap().is_empty()); + // assert!(!key_store.aad::(&group_id).unwrap().is_empty()); - key_store.delete_aad::(&group_id).unwrap(); + // key_store.delete_aad::(&group_id).unwrap(); - assert!(key_store.aad::(&group_id).unwrap().is_empty()); - } + // assert!(key_store.aad::(&group_id).unwrap().is_empty()); + // } #[derive(Serialize, Deserialize, PartialEq, Eq, Debug, Clone)] struct Proposal(Vec); From 1f893cf72b2996d6d05a257f4db57790e06e8827 Mon Sep 17 00:00:00 2001 From: Nicholas Molnar <65710+neekolas@users.noreply.github.com> Date: Mon, 19 Aug 2024 15:13:13 -0700 Subject: [PATCH 02/10] Remove unused labels --- xmtp_mls/src/storage/sql_key_store.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/xmtp_mls/src/storage/sql_key_store.rs b/xmtp_mls/src/storage/sql_key_store.rs index 9c19da8e5..3dce75d2f 100644 --- a/xmtp_mls/src/storage/sql_key_store.rs +++ b/xmtp_mls/src/storage/sql_key_store.rs @@ -273,12 +273,10 @@ const CONFIRMATION_TAG_LABEL: &[u8] = b"ConfirmationTag"; const OWN_LEAF_NODE_INDEX_LABEL: &[u8] = b"OwnLeafNodeIndex"; const EPOCH_SECRETS_LABEL: &[u8] = b"EpochSecrets"; const MESSAGE_SECRETS_LABEL: &[u8] = b"MessageSecrets"; -const USE_RATCHET_TREE_LABEL: &[u8] = b"UseRatchetTree"; // related to MlsGroup const JOIN_CONFIG_LABEL: &[u8] = b"MlsGroupJoinConfig"; const OWN_LEAF_NODES_LABEL: &[u8] = b"OwnLeafNodes"; -const AAD_LABEL: &[u8] = b"AAD"; const GROUP_STATE_LABEL: &[u8] = b"GroupState"; const QUEUED_PROPOSAL_LABEL: &[u8] = b"QueuedProposal"; const PROPOSAL_QUEUE_REFS_LABEL: &[u8] = b"ProposalQueueRefs"; From f5d9ec1a268e59933af650c7223572e163c83415 Mon Sep 17 00:00:00 2001 From: Nicholas Molnar <65710+neekolas@users.noreply.github.com> Date: Mon, 19 Aug 2024 15:20:31 -0700 Subject: [PATCH 03/10] Update retryable error checking --- xmtp_mls/src/storage/errors.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/xmtp_mls/src/storage/errors.rs b/xmtp_mls/src/storage/errors.rs index 43bd7efec..2df4cd869 100644 --- a/xmtp_mls/src/storage/errors.rs +++ b/xmtp_mls/src/storage/errors.rs @@ -89,6 +89,16 @@ impl RetryableError for openmls::group::CreateCommitError bool { match self { Self::KeyStoreError(storage) => retryable!(storage), + Self::LeafNodeUpdateError(leaf_node_update) => retryable!(leaf_node_update), + _ => false, + } + } +} + +impl RetryableError for openmls::treesync::LeafNodeUpdateError { + fn is_retryable(&self) -> bool { + match self { + Self::StorageError(storage) => retryable!(storage), _ => false, } } @@ -145,7 +155,6 @@ impl RetryableError fn is_retryable(&self) -> bool { match self { Self::CreateCommitError(create_commit) => retryable!(create_commit), - Self::MlsGroupStateError(group_state) => retryable!(group_state), Self::StorageError(storage) => retryable!(storage), _ => false, } From 52101de679d8f4cda8cda1f97f24972af565db8b Mon Sep 17 00:00:00 2001 From: Nicholas Molnar <65710+neekolas@users.noreply.github.com> Date: Mon, 19 Aug 2024 15:23:45 -0700 Subject: [PATCH 04/10] Use correct error type --- xmtp_mls/src/storage/errors.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xmtp_mls/src/storage/errors.rs b/xmtp_mls/src/storage/errors.rs index 2df4cd869..bd4fbfd7f 100644 --- a/xmtp_mls/src/storage/errors.rs +++ b/xmtp_mls/src/storage/errors.rs @@ -98,7 +98,7 @@ impl RetryableError for openmls::group::CreateCommitError { fn is_retryable(&self) -> bool { match self { - Self::StorageError(storage) => retryable!(storage), + Self::Storage(storage) => retryable!(storage), _ => false, } } From fa09077fe12c5cced1a05ece61ac96a5bad27960 Mon Sep 17 00:00:00 2001 From: Nicholas Molnar <65710+neekolas@users.noreply.github.com> Date: Mon, 19 Aug 2024 15:53:58 -0700 Subject: [PATCH 05/10] Set MAX_PAST_EPOCHS consistently --- xmtp_mls/src/configuration.rs | 2 ++ xmtp_mls/src/groups/mod.rs | 6 +++--- xmtp_mls/src/groups/sync.rs | 4 ++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/xmtp_mls/src/configuration.rs b/xmtp_mls/src/configuration.rs index e1dcf1fd3..bd297ed2b 100644 --- a/xmtp_mls/src/configuration.rs +++ b/xmtp_mls/src/configuration.rs @@ -18,6 +18,8 @@ pub const UPDATE_INSTALLATIONS_INTERVAL_NS: i64 = NANOSECONDS_IN_HOUR / 2; // 30 pub const MAX_GROUP_SIZE: u16 = 400; +pub const MAX_PAST_EPOCHS: usize = 3; + /// the max amount of data that can be sent in one gRPC call /// we leave 5 * 1024 * 1024 as extra buffer room pub const GRPC_DATA_LIMIT: usize = 45 * 1024 * 1024; diff --git a/xmtp_mls/src/groups/mod.rs b/xmtp_mls/src/groups/mod.rs index c3b35d8f2..f00f3b7cd 100644 --- a/xmtp_mls/src/groups/mod.rs +++ b/xmtp_mls/src/groups/mod.rs @@ -72,7 +72,7 @@ use crate::{ client::{deserialize_welcome, ClientError, MessageProcessingError, XmtpMlsLocalContext}, configuration::{ CIPHERSUITE, GROUP_MEMBERSHIP_EXTENSION_ID, GROUP_PERMISSIONS_EXTENSION_ID, MAX_GROUP_SIZE, - MUTABLE_METADATA_EXTENSION_ID, + MAX_PAST_EPOCHS, MUTABLE_METADATA_EXTENSION_ID, }, hpke::{decrypt_welcome, HpkeError}, identity::{parse_credential, Identity, IdentityError}, @@ -1164,7 +1164,7 @@ fn build_group_config( .capabilities(capabilities) .ciphersuite(CIPHERSUITE) .wire_format_policy(WireFormatPolicy::default()) - .max_past_epochs(3) // Trying with 3 max past epochs for now + .max_past_epochs(MAX_PAST_EPOCHS) // Trying with 3 max past epochs for now .use_ratchet_tree_extension(true) .build()) } @@ -1211,7 +1211,7 @@ async fn validate_initial_group_membership( fn build_group_join_config() -> MlsGroupJoinConfig { MlsGroupJoinConfig::builder() .wire_format_policy(WireFormatPolicy::default()) - .max_past_epochs(3) // Trying with 3 max past epochs for now + .max_past_epochs(MAX_PAST_EPOCHS) // Trying with 3 max past epochs for now .use_ratchet_tree_extension(true) .build() } diff --git a/xmtp_mls/src/groups/sync.rs b/xmtp_mls/src/groups/sync.rs index 459602a41..cf05295d0 100644 --- a/xmtp_mls/src/groups/sync.rs +++ b/xmtp_mls/src/groups/sync.rs @@ -17,7 +17,7 @@ use crate::{ client::MessageProcessingError, codecs::{group_updated::GroupUpdatedCodec, ContentCodec}, configuration::{ - DELIMITER, GRPC_DATA_LIMIT, MAX_GROUP_SIZE, MAX_INTENT_PUBLISH_ATTEMPTS, + DELIMITER, GRPC_DATA_LIMIT, MAX_GROUP_SIZE, MAX_INTENT_PUBLISH_ATTEMPTS, MAX_PAST_EPOCHS, UPDATE_INSTALLATIONS_INTERVAL_NS, }, groups::{ @@ -345,7 +345,7 @@ impl MlsGroup { intent.id, group_epoch, message_epoch, - 1, // max_past_epochs, TODO: expose from OpenMLS MlsGroup + MAX_PAST_EPOCHS, ) { conn.set_group_intent_to_publish(intent.id)?; return Ok(()); From ca626940605a2af77263d5a8ad2dd862c8dfc0b9 Mon Sep 17 00:00:00 2001 From: Nicholas Molnar <65710+neekolas@users.noreply.github.com> Date: Mon, 19 Aug 2024 15:59:20 -0700 Subject: [PATCH 06/10] Add logging for old epoch messages --- xmtp_mls/src/groups/sync.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/xmtp_mls/src/groups/sync.rs b/xmtp_mls/src/groups/sync.rs index cf05295d0..3e4c6396a 100644 --- a/xmtp_mls/src/groups/sync.rs +++ b/xmtp_mls/src/groups/sync.rs @@ -347,6 +347,7 @@ impl MlsGroup { message_epoch, MAX_PAST_EPOCHS, ) { + log::warn!("[{}] Message was sent in an old epoch. Attempting to re-send. Group epoch: {group_epoch}. Message epoch: {message_epoch}", self.context.inbox_id()); conn.set_group_intent_to_publish(intent.id)?; return Ok(()); } From 07f8e165c6e8c1e116d0a91b851d9ba70098b133 Mon Sep 17 00:00:00 2001 From: Nicholas Molnar <65710+neekolas@users.noreply.github.com> Date: Mon, 19 Aug 2024 16:15:39 -0700 Subject: [PATCH 07/10] Simplify past epoch test --- bindings_ffi/src/mls.rs | 50 +++++++++++++++-------------------------- 1 file changed, 18 insertions(+), 32 deletions(-) diff --git a/bindings_ffi/src/mls.rs b/bindings_ffi/src/mls.rs index d674acd65..a20f1c7a5 100644 --- a/bindings_ffi/src/mls.rs +++ b/bindings_ffi/src/mls.rs @@ -2417,82 +2417,68 @@ mod tests { async fn test_can_send_messages_when_epochs_behind() { let alix = new_test_client().await; let bo = new_test_client().await; - let caro = new_test_client().await; let alix_group = alix .conversations() .create_group( - vec![caro.account_address.clone()], + vec![bo.account_address.clone()], FfiCreateGroupOptions::default(), ) .await .unwrap(); - caro.conversations().sync().await.unwrap(); + bo.conversations().sync().await.unwrap(); - let caro_group = caro.group(alix_group.id()).unwrap(); + let bo_group = bo.group(alix_group.id()).unwrap(); + // Move forward 4 epochs alix_group - .send("alix message 1".as_bytes().to_vec()) - .await - .unwrap(); - caro_group - .send("caro message 1".as_bytes().to_vec()) + .update_group_description("change 1".to_string()) .await .unwrap(); alix_group - .add_members(vec![bo.account_address.clone()]) + .update_group_description("change 2".to_string()) .await .unwrap(); alix_group - .send("alix message 2".as_bytes().to_vec()) + .update_group_description("change 3".to_string()) .await .unwrap(); - caro_group - .send("caro message 2".as_bytes().to_vec()) + alix_group + .update_group_description("change 4".to_string()) .await .unwrap(); - bo.conversations().sync().await.unwrap(); - let bo_group = bo.group(alix_group.id()).unwrap(); bo_group .send("bo message 1".as_bytes().to_vec()) .await .unwrap(); - bo_group.sync().await.unwrap(); alix_group.sync().await.unwrap(); - caro_group.sync().await.unwrap(); + bo_group.sync().await.unwrap(); let alix_messages = alix_group .find_messages(FfiListMessagesOptions::default()) .unwrap(); - let caro_messages = caro_group - .find_messages(FfiListMessagesOptions::default()) - .unwrap(); let bo_messages = bo_group .find_messages(FfiListMessagesOptions::default()) .unwrap(); - let caro_message_2_found_bo = bo_messages + let alix_can_see_bo_message = alix_messages .iter() - .any(|message| message.content == "caro message 2".as_bytes()); + .any(|message| message.content == "bo message 1".as_bytes()); assert!( - caro_message_2_found_bo, - "\"caro message 2\" not found in bo_messages" + alix_can_see_bo_message, + "\"bo message 1\" not found in alix's messages" ); - let caro_message_2_found = alix_messages + let bo_can_see_bo_message = bo_messages .iter() - .any(|message| message.content == "caro message 2".as_bytes()); + .any(|message| message.content == "bo message 1".as_bytes()); assert!( - caro_message_2_found, - "\"caro message 2\" not found in alix_messages" + bo_can_see_bo_message, + "\"bo message 1\" not found in bo's messages" ); - - assert_eq!(alix_messages.len(), 7); - assert_eq!(caro_messages.len(), 6); - assert_eq!(bo_messages.len(), 3); } #[tokio::test(flavor = "multi_thread", worker_threads = 5)] From 7c81320edaed5566e353ae2e32f053ea7b22d899 Mon Sep 17 00:00:00 2001 From: Nicholas Molnar <65710+neekolas@users.noreply.github.com> Date: Mon, 19 Aug 2024 18:07:00 -0700 Subject: [PATCH 08/10] Make more errors retryable --- xmtp_mls/src/storage/errors.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/xmtp_mls/src/storage/errors.rs b/xmtp_mls/src/storage/errors.rs index bd4fbfd7f..a3141db63 100644 --- a/xmtp_mls/src/storage/errors.rs +++ b/xmtp_mls/src/storage/errors.rs @@ -63,11 +63,12 @@ impl RetryableError for diesel::result::Error { impl RetryableError for StorageError { fn is_retryable(&self) -> bool { match self { - Self::DieselConnect(connection) => { - matches!(connection, diesel::ConnectionError::BadConnection(_)) - } + Self::DieselConnect(_) => true, Self::DieselResult(result) => retryable!(result), Self::Pool(_) => true, + Self::Lock(_) => true, + Self::PoolNeedsConnection => true, + Self::SqlCipherNotLoaded => true, _ => false, } } From 5e07e06763a0f6bc342e61fc04708ecbd86ac60c Mon Sep 17 00:00:00 2001 From: Nicholas Molnar <65710+neekolas@users.noreply.github.com> Date: Tue, 20 Aug 2024 08:37:32 -0700 Subject: [PATCH 09/10] Remove test --- xmtp_mls/src/storage/sql_key_store.rs | 31 ++++----------------------- 1 file changed, 4 insertions(+), 27 deletions(-) diff --git a/xmtp_mls/src/storage/sql_key_store.rs b/xmtp_mls/src/storage/sql_key_store.rs index 3dce75d2f..ec18964b0 100644 --- a/xmtp_mls/src/storage/sql_key_store.rs +++ b/xmtp_mls/src/storage/sql_key_store.rs @@ -1021,7 +1021,10 @@ mod tests { use openmls::group::GroupId; use openmls_basic_credential::{SignatureKeyPair, StorageId}; use openmls_traits::{ - storage::{traits, Entity, Key, StorageProvider, CURRENT_VERSION}, + storage::{ + traits::{self}, + Entity, Key, StorageProvider, CURRENT_VERSION, + }, OpenMlsProvider, }; use serde::{Deserialize, Serialize}; @@ -1072,32 +1075,6 @@ mod tests { .is_none()); } - // #[test] - // fn list_write_remove() { - // let db_path = tmp_path(); - // let store = EncryptedMessageStore::new( - // StorageOption::Persistent(db_path), - // EncryptedMessageStore::generate_enc_key(), - // ) - // .unwrap(); - // let conn = store.conn().unwrap(); - // let key_store = SqlKeyStore::new(conn.clone()); - // let provider = XmtpOpenMlsProvider::new(conn); - // let group_id = GroupId::random(provider.rand()); - - // assert!(key_store.aad::(&group_id).unwrap().is_empty()); - - // key_store - // .write_aad::(&group_id, "test".as_bytes()) - // .unwrap(); - - // assert!(!key_store.aad::(&group_id).unwrap().is_empty()); - - // key_store.delete_aad::(&group_id).unwrap(); - - // assert!(key_store.aad::(&group_id).unwrap().is_empty()); - // } - #[derive(Serialize, Deserialize, PartialEq, Eq, Debug, Clone)] struct Proposal(Vec); impl traits::QueuedProposal for Proposal {} From 9043a7da8d022cefd3691fd1d6e2ccf1d1bfdba8 Mon Sep 17 00:00:00 2001 From: Nicholas Molnar <65710+neekolas@users.noreply.github.com> Date: Tue, 20 Aug 2024 08:48:55 -0700 Subject: [PATCH 10/10] Address comments --- xmtp_mls/src/groups/mod.rs | 4 ++-- xmtp_mls/src/groups/sync.rs | 1 - xmtp_mls/src/storage/errors.rs | 1 - 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/xmtp_mls/src/groups/mod.rs b/xmtp_mls/src/groups/mod.rs index f00f3b7cd..16f24b636 100644 --- a/xmtp_mls/src/groups/mod.rs +++ b/xmtp_mls/src/groups/mod.rs @@ -1164,7 +1164,7 @@ fn build_group_config( .capabilities(capabilities) .ciphersuite(CIPHERSUITE) .wire_format_policy(WireFormatPolicy::default()) - .max_past_epochs(MAX_PAST_EPOCHS) // Trying with 3 max past epochs for now + .max_past_epochs(MAX_PAST_EPOCHS) .use_ratchet_tree_extension(true) .build()) } @@ -1211,7 +1211,7 @@ async fn validate_initial_group_membership( fn build_group_join_config() -> MlsGroupJoinConfig { MlsGroupJoinConfig::builder() .wire_format_policy(WireFormatPolicy::default()) - .max_past_epochs(MAX_PAST_EPOCHS) // Trying with 3 max past epochs for now + .max_past_epochs(MAX_PAST_EPOCHS) .use_ratchet_tree_extension(true) .build() } diff --git a/xmtp_mls/src/groups/sync.rs b/xmtp_mls/src/groups/sync.rs index 3e4c6396a..cf05295d0 100644 --- a/xmtp_mls/src/groups/sync.rs +++ b/xmtp_mls/src/groups/sync.rs @@ -347,7 +347,6 @@ impl MlsGroup { message_epoch, MAX_PAST_EPOCHS, ) { - log::warn!("[{}] Message was sent in an old epoch. Attempting to re-send. Group epoch: {group_epoch}. Message epoch: {message_epoch}", self.context.inbox_id()); conn.set_group_intent_to_publish(intent.id)?; return Ok(()); } diff --git a/xmtp_mls/src/storage/errors.rs b/xmtp_mls/src/storage/errors.rs index a3141db63..c73d81b12 100644 --- a/xmtp_mls/src/storage/errors.rs +++ b/xmtp_mls/src/storage/errors.rs @@ -67,7 +67,6 @@ impl RetryableError for StorageError { Self::DieselResult(result) => retryable!(result), Self::Pool(_) => true, Self::Lock(_) => true, - Self::PoolNeedsConnection => true, Self::SqlCipherNotLoaded => true, _ => false, }