search

status-im / status-desktop

Status Desktop client made in Nim & QML
https://status.app
Mozilla Public License 2.0
273 stars 76 forks source link

Channel appears open when on the wrong network #14885

Open anastasiyaig opened 1 month ago

anastasiyaig commented 1 month ago

Bug Report

It was discussed here https://github.com/status-im/status-desktop/pull/14871#issuecomment-2125364984

I created permission to have some dumb token on test net (sepolia), but i can bypass that permission with a community member with mainnet enabled. also, i encountered a problem, when a community member joins community having only 1 address, there is a button "Share all addresses and join" which is okay. Which in fact seems to be sharing all the future addresses if i generate them later. So for example I joined a community with status address. This address does not have community token needed to join a specific channel. Then i generate an address in wallet and i get the needed token airdropped to that address. I would expect i still should not be able to see the channel as i did not share yet the additional wallet address with that community. In fact, i was able to post to the channel even the permission requirement is not matching

https://github.com/status-im/status-desktop/assets/82375995/9b4d3241-2cb1-438d-89f6-f1b256cd0f66

Also, for that member that is able to post when he should not be able to -> newly created address is unchecked in Shared addresses modal, which is correct and expected

Screenshot 2024-05-23 at 09 14 02

Then when i check the address and click reveal button, i am no longer available to post to that channel (expected behavior from the very beginning) If i again uncheck the address in Shared addresses, the app still behaves correctly (no possibility to view / post)

As @jrainville said it's a bug on the control node that gives access to the channel from the very beginning. When i do re-evaluation after, then everything works fine

Steps to reproduce

  1. create community
  2. create a channel
  3. for that channel create a permission with a token minted on test net (view and post permission)
  4. join that community with a member with only status address shared with the community and being on mainnet
  5. as a member , generate new address in the wallet after you joined to the community
  6. as community owner, airdrop the needed testnet token to the secondary address of the member
  7. as a member of that community , try to post to that channel when being on mainnet -> you are able to view and post, which is a bug
  8. as a member, click Edit shared addresses -> note that secondary address is unselected (correct behavior)
  9. as a member, select the address and click Reveal button
  10. check that you are not able to view and post anymore, because you are on mainnet (correct behavior)
  11. as a member, click Edit shared addresses and uncheck the secondary address
  12. check that you are still not able to view and post (correct behavior)

Expected behavior

Control node should not give access to the channel

Actual behavior

Can view and post to a channel when keeping needed token on secondary address, which was not yet revealed to community and having wrong chain enabled

Additional Information

anastasiyaig commented 1 month ago

@jrainville @osmaczko i tracked the issue we discussed in https://github.com/status-im/status-desktop/pull/14871#issuecomment-2125364984 here, since it is not related to that PR

anastasiyaig commented 1 month ago

just reproduced that again. I added a permission for a channel to keep a certain token and the channel was open for a member who does not have this token . As this member, i added one more address to the wallet, and then finally the channel become locked. Then i clicked through the app (opened wallet) and went back to community and the channel is available again

https://github.com/status-im/status-desktop/assets/82375995/73058338-0230-45c1-ae1d-02929e5daba5

jrainville commented 1 month ago

I think those are side effects of the changes we made to no longer check the permissions on chain for the members.

As we can see in the latest video you shared, the user is still a member of the channel. That is most likely because the re-evaluation didn't happen yet.

So the fact that the user can still see the channel is actually expected at that point. The bug is more about the fact that the user "lost access" to the channel when changing the address. I think because we still use the same model for the edit shared address and the real view is why we lost it. When you switched to the wallet and back to the channel, it re-asked if it had access and it saw it was part of the member list so it showed.

So the fix would be to separate the models, which is a refactor @alexjba is working on.

So I think we should put this issue in 2.30.

There is also the issue of "bypassing" the permissions by changing the network. This is a fake hack. Yes, it let's the user see the channel, but since they will still not be part of the member list, they won't receive messages or be able to send.

Here, the fix would be to not let the app show the channel even if we think we have the right tokens (in this case because of wrong network).

However, it might be harder in this case, because we need a new error message about the network.

So all in all, I think we should put it in 2.30, as most of those are either edge cases or too hard to fix for the release.