fix(ramps-controller): refresh countries catalog on app startup#9261
fix(ramps-controller): refresh countries catalog on app startup#9261amitabh94 wants to merge 11 commits into
Conversation
Stop persisting the countries catalog and force-refetch via init() so preset amounts stay in sync after app restarts. TRAM-3682 Co-authored-by: Cursor <cursoragent@cursor.com>
TRAM-3682 Co-authored-by: Cursor <cursoragent@cursor.com>
dae992f to
82c35e8
Compare
saustrie-consensys
left a comment
There was a problem hiding this comment.
I left an important question before I can approve
| if (forceRefresh || !hasCountries) { | ||
| await this.getCountries(options); | ||
| } | ||
| await this.getCountries({ ...options, forceRefresh: true }); |
There was a problem hiding this comment.
@amitabh94 @wenfix Why not simply use forceRefresh? We created that option for a reason
There was a problem hiding this comment.
I believe this PR uses forceRefresh only.
There was a problem hiding this comment.
@saustrie-consensys just to make sure I follow: do you mean keep reading options.forceRefresh instead of hardcoding it (the old forceRefresh || !hasCountries shape), or that the caller should drive the refresh via init({ forceRefresh }) and #runInit shouldn't force it at all? Happy to go either way, with persist: false the cold start already refetches regardless, so the hardcoded forceRefresh: true was redundant either way.
There was a problem hiding this comment.
This is a small endpoint and not doing any major work so I am even okay to always refresh it on controller init i.e. everytime the app starts or user closes and reopens the app.
| }, | ||
| countries: { | ||
| persist: true, | ||
| persist: false, |
There was a problem hiding this comment.
I think this should be true as we want to honor the users selection of region from settings.
There was a problem hiding this comment.
I believe we can keep this change to remove the persistence on the countries endpoint as before because it is just to get the country catalog.My bad as I got confused between countries data and user region which are separate.
|
@metamaskbot publish-preview |
a1f454c to
1132ac3
Compare
|
@metamaskbot publish-preview |
|
@metamaskbot publish-preview |
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
|
@metamaskbot publish-preview |
|
@metamaskbot publish-preview |
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
Explanation
Stale persisted countries catalog data caused preset amounts (e.g. $100 / $250 / $500) to stay wrong after app restarts until the user changed region, even when the API had updated values.
This change is controller-only in
@metamask/ramps-controller— no MetaMask Mobile or portfolio changes.persist: false) so restarts do not reuse stale catalog data.init(), always refetch countries viagetCountries()(including whenuserRegionis already set).getCountries(), re-syncuserRegionpreset amounts from the fresh catalog via#syncUserRegionFromCountriesCatalog.Consumers need a new
@metamask/ramps-controllerrelease; mobile can pick it up via the usual Core dependency bump.References
Checklist
Made with Cursor