status-desktop icon indicating copy to clipboard operation
status-desktop copied to clipboard

Owner loses the Token page sometimes

Open jrainville opened this issue 11 months ago • 4 comments

Bug Report

Description

This was experienced by Iuri when we were testing creating tokens for the community.

He went to the Tokens pages in the Admin panel and he could no longer see his minted tokens. It said he needed to mint the Owner token first. Obviously that was already done.

Steps to reproduce

Unsure. The Owner token was already minted. We minted a new collectible. It got created. We went back to the Tokens page to create another one and it wasn't possible.

This might be related to https://github.com/status-im/status-desktop/issues/13651 s we also saw the screen switching unpredictably a lot.

We saw when that was happening (the switching) that a lot of logs about checking pending transactions were happening. It ended up calming down.

Expected behavior

The list of minted tokens are there and we can mint a new one

Actual behavior

Ownr is stuck because it says he needs to mint the owner token even if it's already minted

Additional Information

  • Status desktop version: 2.27 RC6

jrainville avatar Mar 07 '24 16:03 jrainville

AFAIK, the navigation of this Mint token page is driven by changes to the community token model. If this model is empty, we will see the "welcome" page (i.e. the initial page where the user can mint the owner and tmaster token). Once this model is updated and some elements are in the model, the view automatically changes and shows the list of community tokens. That being said, this issue can be:

  • A UI thing, because the UI doesn't listen correctly to model changes.
  • A backend thing, because the backend does not update the model correctly.

noeliaSD avatar Mar 13 '24 13:03 noeliaSD

This might be related to https://github.com/status-im/status-desktop/issues/13651 s we also saw the screen switching unpredictably a lot.

It has the same root cause. When a community is edited we create a new section item for it and update everything but the tokens model. The tokens model update is an async operation triggered when the community is updated. This means that first we'll have a community without tokens model, and later we'll add the tokens. As a result anything related to the Minted tokens page gets reset. The tokens page will show the placeholder used when there's no token minted and any previous navigation in that page is lost.

IMO there are 4 flows to improve here, but not all are necessary to fix this bug:

  1. The entire section is updated without any delta (including the id). This would potentially fire lots of bindings. A simple approach would be to do a comparison on all section model roles and fire dataChanged only for what was changed.
  2. The tokens model is temporarily reset until we have the tokens from the backend. One quick fix would be to preserve the tokens model from the section when handling community edit events. The tokens model would be updated only by the async operation.
  3. Break the getCommunityTokensDetailsTaskArg into more granular updates. We bundle several rpc calls into this: getRemainingSupply, getPendingTransactions, getCommunityTokenBurnState, getRemoteDestructedAddresses. If any of these fail (E.g. No more infura calls available), we'll loose the access to community tokens page even if we already have all the tokens data. It's more or less a refactoring task.
  4. The tokens model needs a more granular update (E.g If the remaining supply on a single token changed, we need to update only the supply in the model). A quick fix would be to use the same approach proposed for the section model.

alexjba avatar Apr 30 '24 11:04 alexjba

@jrainville I guess all the suggested points to be addressed could be splitted in different tasks for your team! I'll change the label to backend-team and unassign @alexjba !

Thanks @alexjba for your deep investigation!

noeliaSD avatar May 07 '24 04:05 noeliaSD

I moved this to 2.30 since we won't have time to fix it for 2.29. Thanks for the analysis @alexjba

jrainville avatar May 07 '24 13:05 jrainville

@friofry I think your fix fixes this one too right (it's the same bug)

jrainville avatar Jun 21 '24 19:06 jrainville

yeah, probably https://github.com/status-im/status-desktop/pull/15301 fixes this issue

friofry avatar Jun 21 '24 19:06 friofry

I close this for now since #15301 should have fixed it. Let's reopen if it wasn't

jrainville avatar Jun 28 '24 16:06 jrainville