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

FetchCommunity doesn't get a new community description after adding permissions #4496

Closed
igor-sirotin opened this issue Dec 20, 2023 · 17 comments · Fixed by #4509
Closed

FetchCommunity doesn't get a new community description after adding permissions #4496

igor-sirotin opened this issue Dec 20, 2023 · 17 comments · Fixed by #4509

Comments

@igor-sirotin
Copy link
Collaborator

  1. This bug is now reproducible for @dlipicar and @endulab.
  2. @mprakhov was able to reproduce this with some tests.
  3. I think I'm facing something similar with in Flaky test - TestRequestMultipleCommunities #4495, but without permissions.

Let's gather the information here.

@igor-sirotin
Copy link
Collaborator Author

@endulab @dlipicar @mprakhov, please, leave all the information about your reproduce steps here. With communities ID, fleets and any other useful information.

I will schedule a call for us to discuss this further.

@mprakhov
Copy link
Contributor

mprakhov commented Dec 20, 2023

@igor-sirotin
test for repro

func (s *MessengerStoreNodeRequestSuite) TestRequestUpdateCommunityFromShard() {
	s.createOwner()
	s.createBob()

	s.waitForAvailableStoreNode(s.owner)
	community := s.createCommunity(s.owner)

	expectedShard := &shard.Shard{
		Cluster: shard.MainStatusShardCluster,
		Index:   23,
	}

	shardRequest := &requests.SetCommunityShard{
		CommunityID: community.ID(),
		Shard:       expectedShard,
	}

	_, err := s.owner.SetCommunityShard(shardRequest)
	s.Require().NoError(err)

	s.waitForAvailableStoreNode(s.bob)

	community, err = s.owner.communitiesManager.GetByID(community.ID())
	s.Require().NoError(err)
	s.Require().NotNil(community)
	s.Require().NotNil(community.Shard())

	s.fetchCommunity(s.bob, community.CommunityShard(), community)

	ownerEditRequest := &requests.EditCommunity{
		CommunityID: community.ID(),
		CreateCommunity: requests.CreateCommunity{
			Name:        "changed name",
			Description: "changed description",
			Color:       community.Color(),
			Membership:  community.Permissions().Access,
		},
	}
	_, err = s.owner.EditCommunity(ownerEditRequest)
	s.Require().NoError(err)

	community, err = s.owner.communitiesManager.GetByID(community.ID())

	s.fetchCommunity(s.bob, community.CommunityShard(), community)
}

@dlipicar
Copy link
Contributor

dlipicar commented Dec 20, 2023

My steps to reproduce:

Instance A:

  • Enable community management on testnet
  • Create community
  • Mint Token-Master and Owner tokens
  • Mint Extra Collectible.
  • Airdrop N collectibles to account B

Instance B:

  • Wait until collectibles list is refreshed (toggle testnet mode to force re-check)

N collectibles are added to the list. The client detects these belong to a community. Fetches community from mailserver. Checks CommunityDescription and searches CommunityTokenMetadata for the matching contract address, but that list only contains the T-Master and Owner tokens, not the Extra Collectible.

-o-

If Account B joins the community, then the next collectible refresh does get the Extra Collectible metadata.

@mprakhov
Copy link
Contributor

mprakhov commented Dec 20, 2023

@igor-sirotin fetchCommunity on the first call returns the correct community, on all next fetchCommunity calls - it returns current -1
What I mean:

  1. Edit community name -> test_1. Calling fetchCommunity -> result test_1
  2. Edit community name -> test_2. Calling fetchCommunity -> result test_1
  3. Edit community name -> test_3. Calling fetchCommunity -> result test_2
  4. Edit community name -> test_4. Calling fetchCommunity -> result test_3
  5. Calling fetchCommunity without any changes -> result test_4

@dlipicar
Copy link
Contributor

@igor-sirotin fetchCommunity on the first call returns the correct community, on all next fetchCommunity calls - it returns current -1 What I mean:

  1. Edit community name -> test_1. Calling fetchCommunity -> result test_1
  2. Edit community name -> test_2. Calling fetchCommunity -> result test_1
  3. Edit community name -> test_3. Calling fetchCommunity -> result test_2
  4. Edit community name -> test_4. Calling fetchCommunity -> result test_3

huh interesting... In my tests, I wouldn't get the new token metadata no matter how many times I retried

@mprakhov
Copy link
Contributor

mprakhov commented Dec 20, 2023

@igor-sirotin fetchCommunity on the first call returns the correct community, on all next fetchCommunity calls - it returns current -1 What I mean:

  1. Edit community name -> test_1. Calling fetchCommunity -> result test_1
  2. Edit community name -> test_2. Calling fetchCommunity -> result test_1
  3. Edit community name -> test_3. Calling fetchCommunity -> result test_2
  4. Edit community name -> test_4. Calling fetchCommunity -> result test_3

huh interesting... In my tests, I wouldn't get the new token metadata no matter how many times I retried

@dlipicar I think your case is a little bit more complex, as you deploy OnwerToken, so your response can be put into runOwnerVerificationLoop.
I would check if your response was not put into HandleCommunityDescriptionMessage->queue. If it is there - you'll need additional setup of your test SetValidateInterval

P.S. But I'm not sure that's the case

@endulab
Copy link
Contributor

endulab commented Dec 20, 2023

I have got 2 scenarios and the results are really strange:

Scenario 1.

  1. Create normal community.
  2. Add "A" as a member.
  3. Use "A" account in CLI bridge. Fetch community in CLI bridge - I see "A" as a member. I can send, can receive messages.
  4. Control node adds permission. Because "A" has enough ETH it is still a member.
  5. Fetch community in CLI bridge - I see "A" as a member.
  6. I can send, but I can’t receive (encryption error).

Scenario 2.

  1. Create normal community.
  2. Add permissions.
  3. Use account "A" in UI. Join community successfully (because "A" has enough ETH).
  4. Use "A" account in CLI bridge. Fetch community in CLI bridge - there is no "A" as a member. I can't send and I can't receive.

Additionally when control node changes community name and I fetch community in CLI bridge the name is always old.

@igor-sirotin
Copy link
Collaborator Author

Here're the action points we got from the call:

  • Write a test to check that existing community filter is not removed by FetchCommunity
  • Investigate why after changing community name FetchCommunity returns previous name (starting from second call)
    Maybe add Clock check to the code.
  • Write a helper test to get information about a community from real store nodes.
    • Given:
      • CommunityID
      • Shard
      • list of store nodes (maybe based on fleet)
    • The test should print:
      • all fetched envelopes for this content topic
      • all community descriptions with Name and Clock

Also, we should think later of how should FetchCommunity behave in case a verification loop is started.
Should it maybe wait for the verification loop to end? Or return some special kind of information?

@igor-sirotin
Copy link
Collaborator Author

Investigate why after changing community name FetchCommunity returns previous name (starting from second call)
Maybe add Clock check to the code.

It seems that it's a test-only problem. We request the data from the store node too fast, faster then it's being published to the store node by the owner.

I'll keep an eye on this, meanwhile go to the next step.

@igor-sirotin
Copy link
Collaborator Author

Implemented a test for filters. Everything's ok there.
#4500

@igor-sirotin
Copy link
Collaborator Author

Confirming that the "previous community name test" is failing because the envelope is too slow.

Note that local community description is not newer than existing happenes earlier than the store node receives the new envelope.

It's strange that this doesn't happen when creating the community. But I guess that's a question of time. We should add some code to wait for the store node to receive the envelopes.

2023-12-20T23:41:43.589Z	INFO	owner-waku-node	wakuv2/waku.go:1045	publishing message via relay	{"envelopeHash": "0x237550142739ba0b2ccd9236952eddd418708e872b88d492da3150829063c116", "pubsubTopic": "/waku/2/default-waku/proto", "contentTopic": "/waku/1/0xd4f16ee1/rfc26", "timestamp": 1703115703589889000}
2023-12-20T23:41:43.589Z	DEBUG	owner-messenger	pushnotificationclient/client.go:729	handling message sent
2023-12-20T23:41:43.589Z	DEBUG	owner-messenger	protocol/messenger_communities.go:304	published org
2023-12-20T23:41:43.589Z	DEBUG	bob-messenger.StoreNodeRequestManager	protocol/messenger_store_node_request_manager.go:357	local community description is not newer than existing	{"requestID": {"requestType":0,"dataID":"0x02c4f8c5fe2d7d55bb09ea0323e418a6bb53659493c7a3c00e785ec6a133ccb5ef"}, "envelopesCount": 4, "existingClock": 1703115703527, "minimumDataClock": 1703115703527}
2023-12-20T23:41:43.589Z	DEBUG	owner-messenger.communityKeyDistributor	protocol/communities_key_distributor.go:30	distributing keys	{"communityID": "0x02c4f8c5fe2d7d55bb09ea0323e418a6bb53659493c7a3c00e785ec6a133ccb5ef"}
2023-12-20T23:41:43.589Z	DEBUG	owner-waku-node	wakuv2/waku.go:1369	received new envelope	{"envelopeHash": "0x237550142739ba0b2ccd9236952eddd418708e872b88d492da3150829063c116", "contentTopic": "/waku/1/0xd4f16ee1/rfc26", "timestamp": 1703115703589889000}
2023-12-20T23:41:43.589Z	DEBUG	owner-waku-node	wakuv2/waku.go:1412	w envelope already cached	{"envelopeHash": "0x237550142739ba0b2ccd9236952eddd418708e872b88d492da3150829063c116"}
2023-12-20T23:41:43.589Z	DEBUG	owner-waku-node	wakuv2/waku.go:1425	posting event	{"envelopeHash": "0x237550142739ba0b2ccd9236952eddd418708e872b88d492da3150829063c116"}
2023-12-20T23:41:43.590Z	DEBUG	owner-waku-node	wakuv2/waku.go:1475	filters did match	{"envelopeHash": "0x237550142739ba0b2ccd9236952eddd418708e872b88d492da3150829063c116", "pubsubTopic": "/waku/2/default-waku/proto", "contentTopic": "/waku/1/0xd4f16ee1/rfc26", "timestamp": 1703115703589889000}
2023-12-20T23:41:43.590Z	DEBUG	owner-waku-node	wakuv2/waku.go:1475	filters did match	{"envelopeHash": "0x237550142739ba0b2ccd9236952eddd418708e872b88d492da3150829063c116", "pubsubTopic": "/waku/2/default-waku/proto", "contentTopic": "/waku/1/0xd4f16ee1/rfc26", "timestamp": 1703115703589889000}
2023-12-20T23:41:43.590Z	DEBUG	store-node-waku	wakuv2/waku.go:1369	received new envelope	{"envelopeHash": "0x237550142739ba0b2ccd9236952eddd418708e872b88d492da3150829063c116", "contentTopic": "/waku/1/0xd4f16ee1/rfc26", "timestamp": 1703115703589889000}
2023-12-20T23:41:43.590Z	DEBUG	store-node-waku	wakuv2/waku.go:1415	cached w envelope	{"envelopeHash": "0x237550142739ba0b2ccd9236952eddd418708e872b88d492da3150829063c116"}
2023-12-20T23:41:43.590Z	DEBUG	store-node-waku	wakuv2/waku.go:1425	posting event	{"envelopeHash": "0x237550142739ba0b2ccd9236952eddd418708e872b88d492da3150829063c116"}
2023-12-20T23:41:43.590Z	DEBUG	store-node-waku	wakuv2/waku.go:1470	filters did not match	{"envelopeHash": "0x237550142739ba0b2ccd9236952eddd418708e872b88d492da3150829063c116", "pubsubTopic": "/waku/2/default-waku/proto", "contentTopic": "/waku/1/0xd4f16ee1/rfc26", "timestamp": 1703115703589889000}

@endulab
Copy link
Contributor

endulab commented Dec 21, 2023

@igor-sirotin @mprakhov I found some buggy behaviour. I used MessengerStoreNodeRequestSuite::TestRequestBigCommunity on status.prod fleet.
Scenario:

  1. Create community - test fetches community with correct name, description
  2. Update community name - test fetches community with correct name, description
  3. Add a member to community
  4. Update community name - test fetches community with wrong name, description

@endulab
Copy link
Contributor

endulab commented Dec 21, 2023

test1:
change name - OK
change name - not OK
change name - not OK
test2:
add member - OK
change name - OK
change name - not OK
change name - not OK
test3:
member 1 added - OK
member 2 added - OK
add permissions - NOT OK

@endulab
Copy link
Contributor

endulab commented Dec 21, 2023

Another test - I created a new community and changed the name 4 times.
Community was published correctly every time with different timestamp and name:
Timestamps:
1
1,703,161,368,539
1,703,161,387,791
1,703,161,401,939
1,703,161,414,919

Then I ran FetchCommunity test and displayed timestamp in HandleCommunityDescriptionMessage.
Received messages with timestamp:
1st round:

  • 1703161387791
  • 1703161401939
  • 1
    2nd round (after some time):
  • 1703161401939
  • 1
  • 1703161368539
  • 1703161387791

Indeed, fetched community had a name changed by a message with timestamp 1,703,161,401,939.

@igor-sirotin
Copy link
Collaborator Author

So far it seems that waku envelopes are fetched in the wrong order.
Working on this, will keep updated.

@igor-sirotin
Copy link
Collaborator Author

Ok, so there's a problem indeed with nim-waku implementation: waku-org/nwaku#2317
Meanwhile I will push a fix to pull all envelopes. This will be slower, but will fetch the latest community description.

@igor-sirotin
Copy link
Collaborator Author

Should be fixed with latest status-go version.
@dlipicar @endulab please test and open a new issue if bugs are still reproduced.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants