GoodDollar / GoodDAPP

GoodDollar.org Wallet is the simplest access point to Claim your daily G$. It Is based on web3 and React native web.
good-dapp.vercel.app
MIT License
108 stars 55 forks source link

Fix: loginMethod not set properly when signing in/up #4270

Closed L03TJ3 closed 5 months ago

L03TJ3 commented 6 months ago

Description

When a user signs in we attempt to set the loginMethod. Due to a racing condition between sync from remote / persisting local set userProperties it consistently ends up overwriting the locally updated values with the previous data from the remote database (see screenshot for flow)

The second syncFromRemote does not make sense to me, because we request props from remote before we have finished updating based on our current local props. This means we will always end up with the old user properties in this second syncFromRemoteFlow.

Flow in console image

Fix:

About # (link your issue here)

4265

How Has This Been Tested?

Locally running the app and changing the logMethod set. After fix it consistently keeps whatever value is set during login

Checklist:

vercel[bot] commented 6 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
goodwallet ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 20, 2024 2:23pm
2 Ignored Deployments | Name | Status | Preview | Comments | Updated (UTC) | | :--- | :----- | :------ | :------- | :------ | | **gooddollar-delta** | ⬜️ Ignored ([Inspect](https://vercel.com/gooddollarteam/gooddollar-delta/4Xx3jyCqRBk8ub4EwDEB6cwiwEaG)) | [Visit Preview](https://gooddollar-delta-git-4265-loginmethod-missing-gooddollarteam.vercel.app) | | May 20, 2024 2:23pm | | **goodid** | ⬜️ Ignored ([Inspect](https://vercel.com/gooddollarteam/goodid/5eD5ZZaFSMTJ7rQGb5Pa2xiVKxGT)) | [Visit Preview](https://goodid-git-4265-loginmethod-missing-gooddollarteam.vercel.app) | | May 20, 2024 2:23pm |
L03TJ3 commented 6 months ago

@sirpy requesting review because I am not sure what the intended behavior is for the second syncFromRemote

L03TJ3 commented 6 months ago

It does solve the question. @sirpy

The answer is: We fetch old props while updating the remote with new props. Second persist overwrites the new remote props with old props, resulting in nothing changed The settings are read from remote on start-up.

By the initial syncFromRemote called when there are no local props set yet.

L03TJ3 commented 6 months ago

@sirpy you were right, the first fix it did not solve the question.

Revised fix with using mutex lock.

It was tested with multiple devices logged in and changing properties.