-
-
Notifications
You must be signed in to change notification settings - Fork 268
feat(ramps): Reorganize RampsController initialization #7707
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
base: main
Are you sure you want to change the base?
Conversation
…geolocation and countries, add countries to state, and create hydrateStore() for providers/tokens
|
cursor review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
| if (country.states && country.states.length > 0) { | ||
| const hasSupportedState = country.states.some( | ||
| (state) => state.supported !== false, | ||
| (state) => state.supported?.buy ?? state.supported?.sell, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nullish coalescing incorrectly filters sell-only supported states
High Severity
The state filtering logic uses ?? (nullish coalescing) instead of || (logical OR) to check for buy/sell support. When a state has supported: { buy: false, sell: true }, the expression state.supported?.buy ?? state.supported?.sell evaluates to false because false is not nullish. This incorrectly filters out states that only support sell operations, making sell functionality unavailable in those regions. The country-level check correctly uses || on line 622-623, but the state check is inconsistent.
Explanation
Reorganizes RampsController initialization to separate critical startup data from region-specific data fetching. This change improves app startup performance by deferring non-essential data fetching until the wallet home page loads.
Additionally, there are interface updates as the API now returns countries with
supported: { buy: true, sell: true }.Changes
init()method: Now only fetches geolocation and countries (24h TTL) on engine instantiationhydrateState()method: New method that fetches providers and tokens for the user's region, called on wallet home page loadcountries: Country[]to controller state with 24 hour TTL cachinggetCountries(): Removedactionparameter - API now returns buy/sell support info in a single responsesetUserRegion(): Refactored to use countries from state instead of fetching themSupportedActionstype: New type{ buy: boolean; sell: boolean }for region support infoCountry.supported/State.supported: Changed frombooleantoSupportedActionsobjectBenefits
hydrateState()will be called on wallet page loadsetUserRegion()could fail if sell countries were fetched after initReferences
https://consensyssoftware.atlassian.net/browse/TRAM-3230
Checklist
Note
Scope
init()to fetch onlygetGeolocationandgetCountries(saved tostate.countrieswith TTL caching); defers region-specific data.hydrateState()to asynchronously fetchtokensandprovidersfor the currentuserRegion.API/Type updates (BREAKING)
Country.supportedandState.supportedchanged frombooleantoSupportedActions({ buy; sell }).getCountries()no longer accepts anactionparameter; service/controller updated accordingly.setUserRegion()now resolves strictly fromstate.countriesand triggers token/provider fetches; cleans dependent state on region change.Other
countries: Country[]to controller state (persisted, exposed to UI).updateUserRegionand associated trigger; introduceshydrateStateand updated trigger methods.Written by Cursor Bugbot for commit 74a0e27. This will update automatically on new commits. Configure here.