Skip to content

Commit e1bbb34

Browse files
authored
Pause groups when min libxmtp version exceeds current (#1708)
* pause groups when min libxmtp version exceeds current version * fix spelling error * cleanup * groups from welcome are paused if they have min version higher than current * verify groups on different versions are not paused when committing until someone set min group version * fix import * more logs when skipping sync on paused group * adds paused_for_version to bindings * fix bindings, make paused_for_version synchronous * fix after merge with main * fmt fix * remove unused import * fixed metadata permissions * lint warning * tests showing expected errors when trying to send a message in a group while paused * fix default policy check since update min version defaults to super admin only * add version_info to wasm trait. make ordering consistent * wasm version_info scoped client one more spot * remove extra char --------- Co-authored-by: cameronvoell <cameronvoell@users.noreply.github.com>
1 parent 9bb54ef commit e1bbb34

File tree

19 files changed

+959
-73
lines changed

19 files changed

+959
-73
lines changed

bindings_ffi/src/mls.rs

+5
Original file line numberDiff line numberDiff line change
@@ -2102,6 +2102,11 @@ impl FfiConversation {
21022102
self.inner.is_active(&provider).map_err(Into::into)
21032103
}
21042104

2105+
pub fn paused_for_version(&self) -> Result<Option<String>, GenericError> {
2106+
let provider = self.inner.mls_provider()?;
2107+
self.inner.paused_for_version(&provider).map_err(Into::into)
2108+
}
2109+
21052110
pub fn consent_state(&self) -> Result<FfiConsentState, GenericError> {
21062111
self.inner
21072112
.consent_state()

bindings_node/src/conversation.rs

+15
Original file line numberDiff line numberDiff line change
@@ -601,6 +601,21 @@ impl Conversation {
601601
)
602602
}
603603

604+
#[napi]
605+
pub fn paused_for_version(&self) -> napi::Result<Option<String>> {
606+
let group = MlsGroup::new(
607+
self.inner_client.clone(),
608+
self.group_id.clone(),
609+
self.created_at_ns,
610+
);
611+
612+
Ok(
613+
group
614+
.paused_for_version(&group.mls_provider().map_err(ErrorWrapper::from)?)
615+
.map_err(ErrorWrapper::from)?,
616+
)
617+
}
618+
604619
#[napi]
605620
pub fn added_by_inbox_id(&self) -> Result<String> {
606621
let group = MlsGroup::new(

bindings_wasm/src/conversation.rs

+13
Original file line numberDiff line numberDiff line change
@@ -548,6 +548,19 @@ impl Conversation {
548548
.map_err(|e| JsError::new(&format!("{e}")))
549549
}
550550

551+
#[wasm_bindgen(js_name = pausedForVersion)]
552+
pub fn paused_for_version(&self) -> Result<Option<String>, JsError> {
553+
let group = self.to_mls_group();
554+
555+
group
556+
.paused_for_version(
557+
&group
558+
.mls_provider()
559+
.map_err(|e| JsError::new(&format!("{e}")))?,
560+
)
561+
.map_err(|e| JsError::new(&format!("{e}")))
562+
}
563+
551564
#[wasm_bindgen(js_name = addedByInboxId)]
552565
pub fn added_by_inbox_id(&self) -> Result<String, JsError> {
553566
let group = self.to_mls_group();
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
ALTER TABLE groups
2+
DROP COLUMN paused_for_version;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
ALTER TABLE groups
2+
ADD COLUMN paused_for_version TEXT DEFAULT NULL;

xmtp_mls/src/builder.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ pub struct ClientBuilder<ApiClient, V> {
5757
}
5858

5959
impl Client<(), ()> {
60-
/// Ge tthe builder for this [`Client`]
60+
/// Get the builder for this [`Client`]
6161
pub fn builder(strategy: IdentityStrategy) -> ClientBuilder<(), ()> {
6262
ClientBuilder::<(), ()>::new(strategy)
6363
}

xmtp_mls/src/client.rs

+20
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use crate::groups::device_sync::WorkerHandle;
33
use crate::groups::group_mutable_metadata::MessageDisappearingSettings;
44
use crate::groups::{ConversationListItem, DMMetadataOptions};
55
use crate::storage::consent_record::ConsentType;
6+
use crate::utils::VersionInfo;
67
use crate::GroupCommitLock;
78
use crate::{
89
groups::{
@@ -141,6 +142,7 @@ pub struct Client<ApiClient, V = RemoteSignatureVerifier<ApiClient>> {
141142
pub(crate) local_events: broadcast::Sender<LocalEvents>,
142143
/// The method of verifying smart contract wallet signatures for this Client
143144
pub(crate) scw_verifier: Arc<V>,
145+
pub(crate) version_info: Arc<VersionInfo>,
144146

145147
#[cfg(any(test, feature = "test-utils"))]
146148
pub(crate) sync_worker_handle: Arc<parking_lot::Mutex<Option<Arc<WorkerHandle>>>>,
@@ -155,6 +157,7 @@ impl<ApiClient, V> Clone for Client<ApiClient, V> {
155157
history_sync_url: self.history_sync_url.clone(),
156158
local_events: self.local_events.clone(),
157159
scw_verifier: self.scw_verifier.clone(),
160+
version_info: self.version_info.clone(),
158161

159162
#[cfg(any(test, feature = "test-utils"))]
160163
sync_worker_handle: self.sync_worker_handle.clone(),
@@ -217,6 +220,18 @@ impl XmtpMlsLocalContext {
217220
}
218221
}
219222

223+
impl<ApiClient, V> Client<ApiClient, V>
224+
where
225+
ApiClient: XmtpApi,
226+
V: SmartContractSignatureVerifier,
227+
{
228+
// Test only function to update the version of the client
229+
#[cfg(test)]
230+
pub fn test_update_version(&mut self, version: &str) {
231+
Arc::make_mut(&mut self.version_info).test_update_version(version);
232+
}
233+
}
234+
220235
impl<ApiClient, V> Client<ApiClient, V>
221236
where
222237
ApiClient: XmtpApi,
@@ -252,12 +267,17 @@ where
252267
#[cfg(any(test, feature = "test-utils"))]
253268
sync_worker_handle: Arc::new(parking_lot::Mutex::default()),
254269
scw_verifier: scw_verifier.into(),
270+
version_info: Arc::new(VersionInfo::default()),
255271
}
256272
}
257273

258274
pub fn scw_verifier(&self) -> &Arc<V> {
259275
&self.scw_verifier
260276
}
277+
278+
pub fn version_info(&self) -> &Arc<VersionInfo> {
279+
&self.version_info
280+
}
261281
}
262282

263283
impl<ApiClient, V> Client<ApiClient, V>

xmtp_mls/src/groups/device_sync/backup/export_stream/group_save.rs

+1
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ impl TryFrom<GroupSave> for StoredGroup {
6464
last_message_ns: value.last_message_ns,
6565
message_disappear_from_ns: value.message_disappear_from_ns,
6666
message_disappear_in_ns: value.message_disappear_in_ns,
67+
paused_for_version: None, // TODO: Add this to the backup
6768
})
6869
}
6970
}

xmtp_mls/src/groups/group_mutable_metadata.rs

+3
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ pub enum MetadataField {
4646
GroupImageUrlSquare,
4747
MessageDisappearFromNS,
4848
MessageDisappearInNS,
49+
MinimumSupportedProtocolVersion,
4950
}
5051

5152
impl MetadataField {
@@ -57,6 +58,7 @@ impl MetadataField {
5758
MetadataField::GroupImageUrlSquare => "group_image_url_square",
5859
MetadataField::MessageDisappearFromNS => "message_disappear_from_ns",
5960
MetadataField::MessageDisappearInNS => "message_disappear_in_ns",
61+
MetadataField::MinimumSupportedProtocolVersion => "minimum_supported_protocol_version",
6062
}
6163
}
6264
}
@@ -205,6 +207,7 @@ impl GroupMutableMetadata {
205207
MetadataField::GroupImageUrlSquare,
206208
MetadataField::MessageDisappearFromNS,
207209
MetadataField::MessageDisappearInNS,
210+
MetadataField::MinimumSupportedProtocolVersion,
208211
]
209212
}
210213

xmtp_mls/src/groups/group_permissions.rs

+87-27
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ use super::{
3131
};
3232
use crate::configuration::{GROUP_PERMISSIONS_EXTENSION_ID, SUPER_ADMIN_METADATA_PREFIX};
3333
use crate::groups::group_mutable_metadata::MetadataField;
34-
use crate::groups::group_mutable_metadata::MetadataField::MessageDisappearInNS;
3534

3635
/// Errors that can occur when working with GroupMutablePermissions.
3736
#[derive(Debug, Error)]
@@ -228,10 +227,22 @@ impl MetadataPolicies {
228227
pub fn default_map(policies: MetadataPolicies) -> HashMap<String, MetadataPolicies> {
229228
let mut map: HashMap<String, MetadataPolicies> = HashMap::new();
230229
for field in GroupMutableMetadata::supported_fields() {
231-
if field == MessageDisappearInNS {
232-
map.insert(field.to_string(), MetadataPolicies::allow_if_actor_admin());
233-
} else {
234-
map.insert(field.to_string(), policies.clone());
230+
match field {
231+
MetadataField::MessageDisappearInNS => {
232+
map.insert(field.to_string(), MetadataPolicies::allow_if_actor_admin());
233+
}
234+
MetadataField::MessageDisappearFromNS => {
235+
map.insert(field.to_string(), MetadataPolicies::allow_if_actor_admin());
236+
}
237+
MetadataField::MinimumSupportedProtocolVersion => {
238+
map.insert(
239+
field.to_string(),
240+
MetadataPolicies::allow_if_actor_super_admin(),
241+
);
242+
}
243+
_ => {
244+
map.insert(field.to_string(), policies.clone());
245+
}
235246
}
236247
}
237248
map
@@ -960,27 +971,37 @@ impl PolicySet {
960971

961972
// Verify that update metadata policy was not violated
962973
let metadata_changes_valid = self.evaluate_metadata_policy(
963-
commit.metadata_changes.metadata_field_changes.iter(),
974+
commit
975+
.metadata_validation_info
976+
.metadata_field_changes
977+
.iter(),
964978
&self.update_metadata_policy,
965979
&commit.actor,
966980
);
967981

968982
// Verify that add admin policy was not violated
969-
let added_admins_valid = commit.metadata_changes.admins_added.is_empty()
983+
let added_admins_valid = commit.metadata_validation_info.admins_added.is_empty()
970984
|| self.add_admin_policy.evaluate(&commit.actor);
971985

972986
// Verify that remove admin policy was not violated
973-
let removed_admins_valid = commit.metadata_changes.admins_removed.is_empty()
987+
let removed_admins_valid = commit.metadata_validation_info.admins_removed.is_empty()
974988
|| self.remove_admin_policy.evaluate(&commit.actor);
975989

976990
// Verify that super admin add policy was not violated
977-
let super_admin_add_valid =
978-
commit.metadata_changes.super_admins_added.is_empty() || commit.actor.is_super_admin;
991+
let super_admin_add_valid = commit
992+
.metadata_validation_info
993+
.super_admins_added
994+
.is_empty()
995+
|| commit.actor.is_super_admin;
979996

980997
// Verify that super admin remove policy was not violated
981998
// You can never remove the last super admin
982-
let super_admin_remove_valid = commit.metadata_changes.super_admins_removed.is_empty()
983-
|| (commit.actor.is_super_admin && commit.metadata_changes.num_super_admins > 0);
999+
let super_admin_remove_valid = commit
1000+
.metadata_validation_info
1001+
.super_admins_removed
1002+
.is_empty()
1003+
|| (commit.actor.is_super_admin
1004+
&& commit.metadata_validation_info.num_super_admins > 0);
9841005

9851006
// Permissions can only be changed by the super admin
9861007
let permissions_changes_valid = !commit.permissions_changed || commit.actor.is_super_admin;
@@ -1158,9 +1179,14 @@ pub fn is_policy_default(policy: &PolicySet) -> Result<bool, PolicyError> {
11581179
name: field_name.to_string(),
11591180
},
11601181
)?;
1161-
if field_name == MessageDisappearInNS.as_str() {
1182+
if field_name == MetadataField::MessageDisappearInNS.as_str()
1183+
|| field_name == MetadataField::MessageDisappearFromNS.as_str()
1184+
{
11621185
metadata_policies_equal = metadata_policies_equal
11631186
&& metadata_policy.eq(&MetadataPolicies::allow_if_actor_admin());
1187+
} else if field_name == MetadataField::MinimumSupportedProtocolVersion.as_str() {
1188+
metadata_policies_equal = metadata_policies_equal
1189+
&& metadata_policy.eq(&MetadataPolicies::allow_if_actor_super_admin());
11641190
} else {
11651191
metadata_policies_equal =
11661192
metadata_policies_equal && metadata_policy.eq(&MetadataPolicies::allow());
@@ -1188,8 +1214,13 @@ pub fn is_policy_admin_only(policy: &PolicySet) -> Result<bool, PolicyError> {
11881214
name: field_name.to_string(),
11891215
},
11901216
)?;
1191-
metadata_policies_equal = metadata_policies_equal
1192-
&& metadata_policy.eq(&MetadataPolicies::allow_if_actor_admin());
1217+
if field_name == MetadataField::MinimumSupportedProtocolVersion.as_str() {
1218+
metadata_policies_equal = metadata_policies_equal
1219+
&& metadata_policy.eq(&MetadataPolicies::allow_if_actor_super_admin());
1220+
} else {
1221+
metadata_policies_equal = metadata_policies_equal
1222+
&& metadata_policy.eq(&MetadataPolicies::allow_if_actor_admin());
1223+
}
11931224
}
11941225
Ok(metadata_policies_equal
11951226
&& policy.add_member_policy == MembershipPolicies::allow_if_actor_admin()
@@ -1205,12 +1236,26 @@ pub fn is_policy_admin_only(policy: &PolicySet) -> Result<bool, PolicyError> {
12051236
pub(crate) fn default_policy() -> PolicySet {
12061237
let mut metadata_policies_map: HashMap<String, MetadataPolicies> = HashMap::new();
12071238
for field in GroupMutableMetadata::supported_fields() {
1208-
metadata_policies_map.insert(field.to_string(), MetadataPolicies::allow());
1239+
match field {
1240+
MetadataField::MessageDisappearInNS => {
1241+
metadata_policies_map
1242+
.insert(field.to_string(), MetadataPolicies::allow_if_actor_admin());
1243+
}
1244+
MetadataField::MessageDisappearFromNS => {
1245+
metadata_policies_map
1246+
.insert(field.to_string(), MetadataPolicies::allow_if_actor_admin());
1247+
}
1248+
MetadataField::MinimumSupportedProtocolVersion => {
1249+
metadata_policies_map.insert(
1250+
field.to_string(),
1251+
MetadataPolicies::allow_if_actor_super_admin(),
1252+
);
1253+
}
1254+
_ => {
1255+
metadata_policies_map.insert(field.to_string(), MetadataPolicies::allow());
1256+
}
1257+
}
12091258
}
1210-
metadata_policies_map.insert(
1211-
MessageDisappearInNS.to_string(),
1212-
MetadataPolicies::allow_if_actor_admin(),
1213-
);
12141259

12151260
PolicySet::new(
12161261
MembershipPolicies::allow(),
@@ -1228,12 +1273,27 @@ pub(crate) fn default_policy() -> PolicySet {
12281273
pub(crate) fn policy_admin_only() -> PolicySet {
12291274
let mut metadata_policies_map: HashMap<String, MetadataPolicies> = HashMap::new();
12301275
for field in GroupMutableMetadata::supported_fields() {
1231-
metadata_policies_map.insert(field.to_string(), MetadataPolicies::allow_if_actor_admin());
1276+
match field {
1277+
MetadataField::MessageDisappearInNS => {
1278+
metadata_policies_map
1279+
.insert(field.to_string(), MetadataPolicies::allow_if_actor_admin());
1280+
}
1281+
MetadataField::MessageDisappearFromNS => {
1282+
metadata_policies_map
1283+
.insert(field.to_string(), MetadataPolicies::allow_if_actor_admin());
1284+
}
1285+
MetadataField::MinimumSupportedProtocolVersion => {
1286+
metadata_policies_map.insert(
1287+
field.to_string(),
1288+
MetadataPolicies::allow_if_actor_super_admin(),
1289+
);
1290+
}
1291+
_ => {
1292+
metadata_policies_map
1293+
.insert(field.to_string(), MetadataPolicies::allow_if_actor_admin());
1294+
}
1295+
}
12321296
}
1233-
metadata_policies_map.insert(
1234-
MetadataField::MessageDisappearInNS.to_string(),
1235-
MetadataPolicies::allow_if_actor_admin(),
1236-
);
12371297

12381298
PolicySet::new(
12391299
MembershipPolicies::allow_if_actor_admin(),
@@ -1297,7 +1357,7 @@ pub(crate) mod tests {
12971357

12981358
use crate::groups::{
12991359
group_metadata::DmMembers, group_mutable_metadata::MetadataField,
1300-
validated_commit::MutableMetadataChanges,
1360+
validated_commit::MutableMetadataValidationInfo,
13011361
};
13021362
use xmtp_common::{rand_string, rand_vec};
13031363

@@ -1388,7 +1448,7 @@ pub(crate) mod tests {
13881448
removed_inboxes: member_removed
13891449
.map(build_membership_change)
13901450
.unwrap_or_default(),
1391-
metadata_changes: MutableMetadataChanges {
1451+
metadata_validation_info: MutableMetadataValidationInfo {
13921452
metadata_field_changes: field_changes,
13931453
..Default::default()
13941454
},

xmtp_mls/src/groups/intents.rs

+7
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,13 @@ impl UpdateMetadataIntentData {
250250
field_value: in_ns.to_string(),
251251
}
252252
}
253+
254+
pub fn new_update_group_min_version_to_match_self(min_version: String) -> Self {
255+
Self {
256+
field_name: MetadataField::MinimumSupportedProtocolVersion.to_string(),
257+
field_value: min_version,
258+
}
259+
}
253260
}
254261

255262
impl From<UpdateMetadataIntentData> for Vec<u8> {

0 commit comments

Comments
 (0)