-
Notifications
You must be signed in to change notification settings - Fork 35
feat(checkout): Migrate to v2 sanction API and add token info #2670
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
View your CI Pipeline Execution ↗ for commit d689bb3
☁️ Nx Cloud last updated this comment at |
const requestPayload: SanctionsCheckV2RequestItem[] = addresses.map((address) => { | ||
const item: SanctionsCheckV2RequestItem = { address }; | ||
|
||
// Add token and amount data if available |
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.
In what scenario would the token and amount data not be available?
Should we still send through a request to the sanctions service without this information?
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.
yea the request should still be sent even when token and amount data are not available,
In what scenario would the token and amount data not be available?
like the sale context (where token/amount are determined later in the flow), early-stage widget initialization (before user selects token/amount), and generic address validation (where we just want to check if an address is sanctioned regardless of specific transaction details) etc
}; | ||
|
||
// Add token and amount data | ||
const tokenInfo = tokenData.find((t) => t.address.toLowerCase() === address.toLowerCase()); |
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.
Looks like if tokenInfo can't be found, we'll send
token_addr: '',
amount: '0',
to the API.. what will happen? Will this cause an error for the user?
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.
the token_addr and amount are not used in determining the risk level actually so there should not be any impact, but let me update the logic here to only call api when token info's valid
export const fetchRiskAssessment = async ( | ||
addresses: string[], | ||
config: CheckoutConfiguration, | ||
tokenData: Array<{ address: string; tokenAddr: string; amount: string }>, |
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.
Is address
here the same as the address is addresses
- can we remove one of them so that the caller doesn't have the provide the same info twice?
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.
yep will update
checkout.config, | ||
[{ | ||
address: toAddress, | ||
tokenAddr: selectedToken?.address || '', |
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.
Should we be checking that a token has been selected before calling this - rather than going ahead with the empty string?
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.
yep good callout, updated
packages/checkout/widgets-lib/src/widgets/add-tokens/views/AddTokens.tsx
Outdated
Show resolved
Hide resolved
packages/checkout/widgets-lib/src/widgets/add-tokens/views/AddTokens.tsx
Outdated
Show resolved
Hide resolved
packages/checkout/widgets-lib/src/widgets/add-tokens/views/AddTokens.tsx
Outdated
Show resolved
Hide resolved
packages/checkout/widgets-lib/src/widgets/bridge/components/BridgeForm.tsx
Outdated
Show resolved
Hide resolved
packages/checkout/widgets-lib/src/widgets/bridge/components/BridgeForm.tsx
Outdated
Show resolved
Hide resolved
packages/checkout/widgets-lib/src/widgets/sale/context/SaleContextProvider.tsx
Outdated
Show resolved
Hide resolved
…Tokens.tsx Co-authored-by: Keith Broughton <[email protected]>
…Tokens.tsx Co-authored-by: Keith Broughton <[email protected]>
…Tokens.tsx Co-authored-by: Keith Broughton <[email protected]>
…idgeForm.tsx Co-authored-by: Keith Broughton <[email protected]>
…idgeForm.tsx Co-authored-by: Keith Broughton <[email protected]>
…textProvider.tsx Co-authored-by: Keith Broughton <[email protected]>
Hi👋, please ensure the PR title follows the below standards:
type(scope): message
. For example:feat(passport): my new feature
!
after thetype(scope)
, for examplefeat(passport)!: my new breaking feature
Summary
What Changed
Key Updates (under checkout)
Backward Compatibility: yes -> All existing code continues to work
Detail and impact of the change
Added
Changed
Deprecated
Removed
Fixed
Security
Anything else worth calling out?