-
Notifications
You must be signed in to change notification settings - Fork 275
Sam/infinite ramp plugin #5715
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: develop
Are you sure you want to change the base?
Sam/infinite ramp plugin #5715
Conversation
2311b66
to
9ebe8d3
Compare
4a14ce2
to
00f5b8c
Compare
6e2b3b7
to
5cf5c68
Compare
5cf5c68
to
53d9054
Compare
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.
First round of review. I need to take a closer look at the Infinite API and some other related things.
getAuthState: false, // This is always local, no API call | ||
saveCustomerId: false, // This is always local, no API call | ||
isAuthenticated: false // This is always local, no API call | ||
} |
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.
I disagree with Cursor on this one. This is critical for development since we don't want to use real funds for testing.
import type { NavigationBase } from '../../../../types/routerTypes' | ||
|
||
export interface NavigationFlow { | ||
navigate: (route: string, params?: any) => void |
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.
I don't like the any
. Since we can go to scenes like rampPending
, it seems like we are dealing with the buy / sell tab navigator, so we should type this using those route names / params.
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.
There's a high cog-load to the navigation types, so I'll just make it typed to NavigationBase
} | ||
|
||
resolve({ confirmed: true, transfer }) | ||
} |
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 makes a valid point - the funds could come from many utxo's, not necessarily our latest receive address (indeed, this is usually a fresh address for utxo coins). If Infinite.dev is just using this as a refund address, then it's fine. If not, we might need a rethink.
try { | ||
const record = await makeRecord(info) | ||
const json = wasVaultPersonalRecord(record) | ||
await vaultDisklet.setText(`${info.type}/${record.uuid}.json`, json) |
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.
So we are saving data in edgeVault/{randomHex}.json
. Ok, I guess this is fine.
They don't really need to be UUID's - we could do 128 bits of base58 (about 21 characters). This would simplify creating these things, and since we don't really parse them or read into them, it wouldn't affect anybody.
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.
Found a few more things. Now the review is complete.
/** | ||
* Exit error class for gracefully exiting workflows | ||
*/ | ||
export class Exit extends Error { |
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.
This should probably be called ExitError
. It's not really an error as such, because we expect it, but calling it ExitError
makes it clear that we want to catch it.
if ( | ||
kycStatus.kycStatus === 'not_started' || | ||
kycStatus.kycStatus === 'incomplete' | ||
) { | ||
// Fall through to show KYC form |
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.
This logic threw me for a loop. Would it make more sense:
if (
kycStatus.kycStatus !== 'not_started' &&
kycStatus.kycStatus !== 'incomplete'
) {
// For all other statuses (under_review, awaiting_ubo, etc.), show pending scene
await showKycPendingScene(...
}
} | ||
let privateKey: Uint8Array | ||
// Try to load from storage (stored as hex string) | ||
const itemIds = await account.dataStore.listItemIds(pluginId) |
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.
Just do const key: string | undefined = account.dataStore.getItem(pluginId, INFINITE_PRIVATE_KEY).catch(() => {})
. It will either return the key if we have it, or undefined. Saves a round-trip over the bridge and a bunch of expensive disk access.
): Promise<void> => { | ||
const itemIds = await account.dataStore.listItemIds(pluginId) | ||
if (itemIds.includes(INFINITE_PRIVATE_KEY) === true) { | ||
await account.dataStore.deleteItem(pluginId, INFINITE_PRIVATE_KEY) |
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.
Just call account.dataStore.deleteItem(pluginId, INFINITE_PRIVATE_KEY)
directly. Deleting missing items is a no-op - it's not an error, so there's no need to guard the call.
let currenciesCache: CacheEntry<InfiniteCurrenciesResponse> | null = null | ||
let normalizedCurrenciesCache: CacheEntry<NormalizedCurrenciesMap> | null = |
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.
These two caches contain the same data, just in two formats. Could they be merged somehow?
let currenciesCache: CacheEntry<{
raw: InfiniteCurrenciesResponse
normalized: NormalizedCurrenciesMap
}> | null = null
...
const getCurrenciesWithCache =
...
const currenciesData = await infiniteApi.getCurrencies()
const data = {
raw: currenciesData,
normalized: normalizeCurrencies(currenciesData)
}
currenciesCache = { data, timestamp: Date.now() }
return data
}) | ||
|
||
// Get fresh quote before confirmation using existing params | ||
const freshQuote = await infiniteApi.createQuote(quoteParams) |
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.
So we're re-quoting in case the KYC took a long time, and the old quote expired?
const hexToBytes = (hex: string): Uint8Array => { | ||
if (hex.startsWith('0x')) hex = hex.slice(2) | ||
const bytes = new Uint8Array(hex.length / 2) | ||
for (let i = 0; i < hex.length; i += 2) { | ||
bytes[i / 2] = parseInt(hex.substring(i, i + 2), 16) | ||
} | ||
return bytes | ||
} | ||
|
||
const bytesToHex = (bytes: Uint8Array): string => { | ||
return Array.from(bytes) | ||
.map(b => b.toString(16).padStart(2, '0')) | ||
.join('') | ||
} |
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.
These should be base16.parse
/ base16.stringify
from rfc4648
const shouldIncludeAuth = | ||
options?.includeAuth ?? (authState.token != null && !isTokenExpired()) | ||
|
||
if (shouldIncludeAuth && authState.token != null && !isTokenExpired()) { |
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.
This way should be equivalent, and simpler:
const hasAuth = authState.token != null && !isTokenExpired()
if (options?.includeAuth !== false && hasAuth) {
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
noneRequirements
If you have made any visual changes to the GUI. Make sure you have:
https://app.asana.com/0/0/1211067783183644(original task; first attempt)Note
Introduces the Infinite ramp plugin with end-to-end auth/KYC/TOS/bank/transfer flows, new ramp scenes, deep-link handling, plugin loading and env inits, plus supporting utilities, tests, and UI tweaks.
infinite
with API/types, auth challenge/sign (secp256k1), quotes/transfers, KYC/TOS status, bank accounts, currencies/countries, and normalized currency mapping.authenticateWorkflow
,kycWorkflow
(pending polling),tosWorkflow
,bankAccountWorkflow
,confirmationWorkflow
, nav flow helper.ramp
link parsing/handling and deeplink registration in webview utilities; updatedDeepLinkingActions
and types.ENV.RAMP_PLUGIN_INITS.infinite
; load only plugins with init present; register inallRampPlugins
.RampKycFormScene
,RampBankFormScene
,RampBankRoutingDetailsScene
,RampConfirmationScene
,RampPendingScene
; wired into Buy/Sell stacks; dev test entries.edgeVault
(personal/address/bank records) with comprehensive unit tests and sample records.CurrencyInfoHelpers
: addgetContractAddress
.cleanFetch
: typed URL resources, options; addasSearchParams
.useBackEvent
hook.FilledTextInput
: minLength, improved messages;Paragraph
fit text;GuiFormField
/GuiFormRow
enhancements.AirshipInstance.showToastSpinner
accepts function.string_close_cap
; confirm buy/sell titles.testMatch
; add@noble/{curves,hashes}
deps.Written by Cursor Bugbot for commit 85e99f7. This will update automatically on new commits. Configure here.