xmtp / libxmtp

MIT License
33 stars 13 forks source link

Identity Release Bug - members() func returning zero for non group creator #769

Closed cameronvoell closed 1 month ago

cameronvoell commented 1 month ago

Describe the bug

See PR for test case reproduction: https://github.com/xmtp/libxmtp/pull/768/files#diff-c29f56a38916c7410eff8091df1a2e43487ffe20646d96827e846e475f4608d3R926

Expected behavior

No response

Steps to reproduce the bug

    // Test members function from non group creator
    #[tokio::test(flavor = "multi_thread", worker_threads = 1)]
    async fn test_members_func_from_non_creator() {
        let amal = ClientBuilder::new_test_client(&generate_local_wallet()).await;
        let bola = ClientBuilder::new_test_client(&generate_local_wallet()).await;

        let amal_group = amal.create_group(None).unwrap();
        amal_group
            .add_members_by_inbox_id(&amal, vec![bola.inbox_id()])
            .await
            .unwrap();

        // Get bola's version of the same group
        let bola_groups = bola.sync_welcomes().await.unwrap();
        let bola_group = bola_groups.first().unwrap();

        // Call sync for both
        amal_group.sync(&amal).await.unwrap();
        bola_group.sync(&bola).await.unwrap();

        // Verify bola can see the group name
        let bola_group_name = bola_group.group_name().unwrap();
        assert_eq!(bola_group_name, "New Group");

        // Check if both clients can see the members correctly
        let amal_members = amal_group.members().unwrap();
        let bola_members = bola_group.members().unwrap();

        assert_eq!(amal_members.len(), 2);
        assert_eq!(bola_members.len(), 2); // failing here, see len == 0
    }
neekolas commented 1 month ago

Odd. I can take a look into this.

neekolas commented 1 month ago

OK. I at least know what's going on. There are two TODOs in the integration work that both play into this issue

  1. When you join a group from a welcome, we don't currently go and download all the identity updates from the network for all the group members and make sure that the association state is correct.
  2. When calling group.members(), we only check our local cache for the association state to fill in each group member. Once we solve (1) that cache will be populated. Currently it only gets populated the next time you create or read a commit.

Once the first one is fixed this issue should resolve itself in most real cases. And once the second one is fixed this situation will result in an error.

cameronvoell commented 1 month ago

@neekolas Verified that this was fixed by #781 🎉

Have a new PR with the test that caught this issue here => https://github.com/xmtp/libxmtp/pull/794