Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mutable Metadata group name Part 3 #677

Merged
merged 9 commits into from
Apr 23, 2024

Conversation

cameronvoell
Copy link
Contributor

@cameronvoell cameronvoell commented Apr 19, 2024

This PR adds the Validation that group names obey the update_group_name_policy set upon creating a group.

See #548 for more info.

Previous PR's:

PR's related to this change:

Before merging this to main:

  • Update openmls reference in Cargo.toml to commit on xmtp/openmls main branch
  • Update dev/gen_protos.sh to point to xmtp-proto repo main branch

@cameronvoell cameronvoell requested a review from a team as a code owner April 19, 2024 18:41
@cameronvoell
Copy link
Contributor Author

cameronvoell commented Apr 19, 2024

I just noticed in the MLS spec that Commits are considered invalid if they have more than one Group Context Extensions Proposal (https://www.rfc-editor.org/rfc/rfc9420.html#section-12.2-2)

I'm going to take a closer look at what the conditions are that might lead to us having multiple proposals in a commit, and make sure we handle multiple GCE Proposals in the queue properly.

@@ -294,29 +552,78 @@ impl PolicySet {
})
}

// In case group creator is on future version of libxmtp, we can validate
// metadata policies on new unknown fields
fn evaluate_metadata_policy(&self, change: &MetadataChange, actor: &CommitParticipant) -> bool {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function enables users to validate metadata updates for metadata fields that might not exist in their version of libxmtp

@cameronvoell
Copy link
Contributor Author

cameronvoell commented Apr 22, 2024

@neekolas I think this latest commit d9c26d5 incorporating the proto update to use mappings will achieve the functionality of allowing group members to validate metadata updates for fields that do not exist in their version of libxmtp (see function here #677 (comment)).

Since permission policies are still only in immutable data, this would only apply to being invited to a new group by someone on a future version of libxmtp. But if we move mutable permission policies to mutable metadata, this will could enable adding new metadata fields to existing groups.

I'll spend some time today looking for spots to make the code cleaner, more intuitive, but any feedback is welcome 🙏

Copy link
Contributor

@neekolas neekolas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking great

@cameronvoell cameronvoell force-pushed the cv/mutable-metadata-part-3-fixed-commits branch from 5c71e20 to 8baeabf Compare April 22, 2024 23:25
@cameronvoell cameronvoell merged commit 9951e45 into main Apr 23, 2024
6 checks passed
@cameronvoell cameronvoell deleted the cv/mutable-metadata-part-3-fixed-commits branch April 23, 2024 01:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants