Community members reevaluation fixes

[igor-sirotin]

Tip

Review by commits

1. Load community from the database inside ReevaluateMembers, instead of passing as argument.
   To ensure that the community is loaded and saved while m.communityLock is locked.

2. Made ReevaluateCommunityMembersPermissions a private func.
   To ensure that it's only called inside the members reevaluation loop.
   Where it was called directly, we now call ScheduleMembersReevaluation.

3. Created a public StartMembersReevaluationLoop and made ReevaluateMembersPeriodically a private func.
   To ensure ReevaluateMembersPeriodically is started as a separate goroutine and make the code simpler.

   StartMembersReevaluationLoop has a boolean argument reevaluateOnStart.
   When true, members reevaluation is forced once on the loop start, which is same as current behaviour.

4. Prevent publishing older community description in publishOrgAndDistributeEncryptionKeys

Required for #5076

[status-im-auto]

Jenkins Builds

Click to see older builds (17)

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
❌	7aedfc1	#1	2024-05-06 19:37:05	~1 min	ios	📄log
✖️	7aedfc1	#1	2024-05-06 19:37:10	~1 min	tests	📄log
❌	7aedfc1	#1	2024-05-06 19:37:15	~1 min	android	📄log
❌	7aedfc1	#1	2024-05-06 19:37:19	~1 min	linux	📄log
✖️	899ae54	#2	2024-05-06 19:45:54	~2 min	tests	📄log
✔️	899ae54	#2	2024-05-06 19:46:11	~3 min	linux	📦zip
✔️	899ae54	#2	2024-05-06 19:47:01	~3 min	ios	📦zip
✔️	899ae54	#2	2024-05-06 19:47:56	~4 min	android	📦aar
✔️	b289711	#3	2024-05-06 20:21:48	~1 min	android	📦aar
✔️	b289711	#3	2024-05-06 20:22:20	~2 min	linux	📦zip
✔️	b289711	#3	2024-05-06 20:22:51	~2 min	ios	📦zip
✖️	b289711	#3	2024-05-06 20:55:37	~35 min	tests	📄log
✖️	b289711	#4	2024-05-06 21:35:47	~35 min	tests	📄log
✔️	9d3f5c0	#4	2024-05-07 13:31:49	~2 min	linux	📦zip
✔️	9d3f5c0	#4	2024-05-07 13:31:58	~2 min	android	📦aar
✔️	9d3f5c0	#4	2024-05-07 13:33:21	~4 min	ios	📦zip
✖️	9d3f5c0	#5	2024-05-07 14:04:22	~34 min	tests	📄log

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
✔️	077a049	#5	2024-05-07 14:21:30	~4 min	linux	📦zip
✔️	077a049	#5	2024-05-07 14:21:42	~4 min	ios	📦zip
✖️	077a049	#6	2024-05-07 14:22:34	~5 min	tests	📄log
✔️	077a049	#5	2024-05-07 14:22:46	~5 min	android	📦aar
✔️	e0df615	#6	2024-05-07 14:26:35	~1 min	android	📦aar
✔️	e0df615	#6	2024-05-07 14:27:08	~2 min	linux	📦zip
✔️	e0df615	#6	2024-05-07 14:28:18	~3 min	ios	📦zip
✔️	e0df615	#7	2024-05-07 15:06:07	~41 min	tests	📄log

[igor-sirotin]

Looks like I fixed my test and broke a few others 😄

[igor-sirotin]

I made the condition a bit more lenient: allow publishing community with same clock as last sent:

diff --git a/protocol/messenger_communities.go b/protocol/messenger_communities.go
index d803e0b4d..bc3ba7caf 100644
--- a/protocol/messenger_communities.go
+++ b/protocol/messenger_communities.go
@@ -291,7 +288,7 @@ func (m *Messenger) handleCommunitiesSubscription(c chan *communities.Subscripti
        publishOrgAndDistributeEncryptionKeys := func(community *communities.Community) {
                recentlyPublishedOrg := recentlyPublishedOrgs[community.IDString()]
 
-               if recentlyPublishedOrg != nil && community.Clock() <= recentlyPublishedOrg.Clock() {
+               if recentlyPublishedOrg != nil && community.Clock() < recentlyPublishedOrg.Clock() {
                        return
                }

This fixes (at least makes same as before) MessengerCommunitiesTokenPermissionsSuite tests.

I'm not sure if it's the right thing to do.
Obviously, we don't need to publish same community description again. But we do this on other events as well, when the community description clock is not increased. I guess it's because on sending we attach more community info (like keys) to the message.

[jrainville]

LGTM, though I'm not an expert on that part of the code

[osmaczko]

Thanks @igor-sirotin for fixes and cleanup 👍

I'm not sure if it's the right thing to do.
Obviously, we don't need to publish same community description again. But we do this on other events as well, when the community description clock is not increased.

Your guess seems correct. We should not publish same community twice.

I guess it's because on sending we attach more community info (like keys) to the message.

That should not be the case. Keys should be attached already with first publish. I created issue to investigate that deeper: #5149