Identity E2E integration #755
Conversation
| // get current number of users in group | ||
| let member_count = self.members()?.len(); | ||
| if member_count + inbox_id_map.len() > MAX_GROUP_SIZE as usize { | ||
| return Err(GroupError::UserLimitExceeded); | ||
| } | ||
|
|
||
| if inbox_id_map.len() != account_addresses.len() { |
There was a problem hiding this comment.
The way I am doing this validation is not very good. I have to jump through some hoops to get the actual list of missing addresses.
| @@ -490,6 +501,13 @@ impl MlsGroup { | |||
| .get_membership_update_intent(client, &provider, inbox_ids, vec![]) | |||
| .await?; | |||
|
|
|||
| // TODO:nm this isn't the best test for whether the request is valid | |||
There was a problem hiding this comment.
Will fix this to do a more thorough check. This is the wrong way to do it.
| @@ -1144,6 +1144,8 @@ mod tests { | |||
| } | |||
|
|
|||
| #[tokio::test(flavor = "multi_thread", worker_threads = 5)] | |||
| // This one is flaky for me. Passes reliably locally and fails on CI | |||
There was a problem hiding this comment.
yeah this is the one that fails because of the timeout (found during the refactor). It passes locally since CI is just slower. using async_barrier for sync points instead of tokio::sleep should fix it.
not sure if a ticket yet but should be, will make it if not
There was a problem hiding this comment.
I don't think this one has a ticket. You mind making one
| .add_ecdsa_signature( | ||
| inbox_owner | ||
| .sign(signature_request.signature_text().await.unwrap()) | ||
| .unwrap(), | ||
| ) | ||
| .await | ||
| .unwrap(); |
There was a problem hiding this comment.
In this case, does platform SDK need to prompt user to sign the signature text with their wallet App, and pass in signature instead of inbox_owner signing it there?
There was a problem hiding this comment.
Yes, that's right.
Hopefully in most cases we can just use the legacy keys and so there would be no signature_request
There was a problem hiding this comment.
How about new users, do they have to sign signature_request?
There was a problem hiding this comment.
At launch, the platform SDKs are going to create a V2 identity before creating the V3 SDK. So even new users will have a legacy key.
Once we completely kill V2, new users will have to sign a signature_request. But they won't have the 2 V2 signatures, so it's actually one less signature than today.
tl;dr
Follow-ups
There are a few changes I had to make to get everything passing that need some follow-up.
There are also many places that need better test coverage.