-
Notifications
You must be signed in to change notification settings - Fork 61
feat(gateway): list orders's output tokens #711
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: master
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds multi-output support to Onramp orders by introducing outputTokenAddresses and outputTokenAmounts in response types and a new getOutputs() method on OnrampOrder. Existing single-output getters are annotated as deprecated. Client mapping adds getOutputs() to OnrampOrder objects by resolving token addresses to Token instances and pairing with amounts. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant Client as Gateway Client
participant Order as OnrampOrder
participant Registry as ADDRESS_LOOKUP
App->>Client: getOnrampOrders()
Client-->>App: Array<OnrampOrder> (includes getOutputs)
App->>Order: getOutputs()
Order->>Registry: Resolve outputTokenAddresses by chainId
Registry-->>Order: Token objects (per address)
Order-->>App: Array<[Token?, amount?>]
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 3
🧹 Nitpick comments (1)
sdk/src/gateway/types.ts (1)
364-365
: Clarify getOutputs contract to always return an arrayEnsure the documentation clearly communicates that getOutputs always returns an array (possibly empty). This aligns with the recommended consumer ergonomics and the implementation fix suggested in client.ts.
Apply this diff:
- /** @description get all received tokens and the amounts */ + /** @description Returns all received tokens paired with their amounts. Returns [] when none. */ getOutputs(): [Token, string][];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
sdk/src/gateway/client.ts
(1 hunks)sdk/src/gateway/types.ts
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
getOutputs() { | ||
return order.outputTokenAddresses?.map((address, i) => { | ||
const token = ADDRESS_LOOKUP[this.chainId][address]; | ||
const amount = order.outputTokenAmounts?.[i]; | ||
return [token, amount]; | ||
}); | ||
}, |
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.
💡 Verification agent
🧩 Analysis chain
Fix getOutputs: wrong this context, return type mismatch, and undefined return
Issues:
- Uses this.chainId inside a plain object method; this will not be the GatewayApiClient. Use captured chainId.
- May return undefined when outputTokenAddresses is falsy; OnrampOrder.getOutputs() is typed to return [Token, string][].
- Returns [Token | undefined, string | undefined][]; types require non-optional Token and string.
Apply this diff to make getOutputs robust and type-safe:
- getOutputs() {
- return order.outputTokenAddresses?.map((address, i) => {
- const token = ADDRESS_LOOKUP[this.chainId][address];
- const amount = order.outputTokenAmounts?.[i];
- return [token, amount];
- });
- },
+ getOutputs() {
+ const addresses = order.outputTokenAddresses ?? [];
+ const amounts = order.outputTokenAmounts ?? [];
+ return addresses
+ .map((address, i) => {
+ const token = ADDRESS_LOOKUP[chainId][address];
+ const amount = amounts[i];
+ if (!token || amount === undefined) return undefined;
+ return [token, amount] as [Token, string];
+ })
+ .filter((x): x is [Token, string] => Boolean(x));
+ },
- Confirm the Gateway API guarantees that in multi-output scenarios outputTokenAddress (singular) remains populated. If not, this line nearby can throw at runtime due to the non-null assertion:
outputToken: ADDRESS_LOOKUP[chainId][order.outputTokenAddress!],
If the guarantee doesn’t hold, update it to:
outputToken: order.outputTokenAddress ? ADDRESS_LOOKUP[chainId][order.outputTokenAddress] : undefined,
🏁 Script executed:
#!/bin/bash
# Show context around the getOutputs method
rg -n -C3 'getOutputs\(\)' sdk/src/gateway/client.ts
# Show where singular outputTokenAddress is used nearby
rg -n -C5 'outputTokenAddress' sdk/src/gateway/client.ts
Length of output: 4735
#!/bin/bash
# Find the definition of OnrampOrder
rg -n "OnrampOrder" -g "sdk/src/**/*.ts" -C3
🏁 Script executed:
#!/bin/bash
# Search for OnrampOrder definitions and usages in the SDK
rg -n -C3 "OnrampOrder" -g "sdk/src/**/*.ts"
Length of output: 6794
🏁 Script executed:
#!/bin/bash
# Show full OnrampOrder type definition
rg -n -C5 "export type OnrampOrder" sdk/src/gateway/types.ts
Length of output: 422
🏁 Script executed:
#!/bin/bash
# Inspect singular and plural output token fields in the OnrampOrderResponse type
rg -n -C5 '\boutputTokenAddress\b' sdk/src/gateway/types.ts
rg -n -C5 '\boutputTokenAddresses\b' sdk/src/gateway/types.ts
rg -n -C5 '\boutputTokenAmounts\b' sdk/src/gateway/types.ts
Length of output: 2063
Fix getOutputs and singular outputToken lookup
The current implementation uses this.chainId
, doesn’t guard optional arrays, and can return undefined
or mismatched tuples, while order.outputTokenAddress
is optional but asserted non-null.
Attention needed in sdk/src/gateway/client.ts
:
- Line 830: guard
order.outputTokenAddress
before lookup - Lines 841–847: use captured
chainId
, default to empty arrays, filter out invalid entries, and cast to[Token, string]
Apply this diff:
@@ sdk/src/gateway/client.ts
- baseToken: ADDRESS_LOOKUP[chainId][order.baseTokenAddress],
- outputToken: ADDRESS_LOOKUP[chainId][order.outputTokenAddress!],
+ baseToken: ADDRESS_LOOKUP[chainId][order.baseTokenAddress],
+ outputToken: order.outputTokenAddress
+ ? ADDRESS_LOOKUP[chainId][order.outputTokenAddress]
+ : undefined,
@@ sdk/src/gateway/client.ts
- getOutputs() {
- return order.outputTokenAddresses?.map((address, i) => {
- const token = ADDRESS_LOOKUP[this.chainId][address];
- const amount = order.outputTokenAmounts?.[i];
- return [token, amount];
- });
- },
+ getOutputs() {
+ const addresses = order.outputTokenAddresses ?? [];
+ const amounts = order.outputTokenAmounts ?? [];
+ return addresses
+ .map((address, i) => {
+ const token = ADDRESS_LOOKUP[chainId][address];
+ const amount = amounts[i];
+ if (!token || amount === undefined) return undefined;
+ return [token, amount] as [Token, string];
+ })
+ .filter((x): x is [Token, string] => Boolean(x));
+ },
These changes ensure the correct chainId
is used, optional data is handled safely, and the return type matches [Token, string][]
.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
getOutputs() { | |
return order.outputTokenAddresses?.map((address, i) => { | |
const token = ADDRESS_LOOKUP[this.chainId][address]; | |
const amount = order.outputTokenAmounts?.[i]; | |
return [token, amount]; | |
}); | |
}, | |
// Around line 830 | |
baseToken: ADDRESS_LOOKUP[chainId][order.baseTokenAddress], | |
outputToken: order.outputTokenAddress | |
? ADDRESS_LOOKUP[chainId][order.outputTokenAddress] | |
: undefined, | |
// Around lines 841–847 | |
getOutputs() { | |
const addresses = order.outputTokenAddresses ?? []; | |
const amounts = order.outputTokenAmounts ?? []; | |
return addresses | |
.map((address, i) => { | |
const token = ADDRESS_LOOKUP[chainId][address]; | |
const amount = amounts[i]; | |
if (!token || amount === undefined) return undefined; | |
return [token, amount] as [Token, string]; | |
}) | |
.filter((x): x is [Token, string] => Boolean(x)); | |
}, |
🤖 Prompt for AI Agents
In sdk/src/gateway/client.ts around lines 830 and 841–847, the code uses
this.chainId directly, doesn't guard optional singular outputTokenAddress, and
maps optional arrays without defaults which can produce undefined or mismatched
tuples; change to capture const chainId = this.chainId for lookup,
check/order.outputTokenAddress exists before using it for single-token lookup,
use const outputs = order.outputTokenAddresses ?? [] and const amounts =
order.outputTokenAmounts ?? [], map over outputs using the captured chainId to
resolve ADDRESS_LOOKUP[chainId][address], pair with amounts[i], filter out
entries where token or amount is falsy, and return the result cast as [Token,
string][] so the return type and values are safe and consistent.
outputTokenAddresses?: EvmAddress[]; | ||
/** @description The output amount (from strategies) */ | ||
outputTokenAmounts?: string[]; | ||
/** @description The tx hash on the EVM chain */ |
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.
💡 Verification agent
🧩 Analysis chain
Fix type reference and incorrect JSDoc for multi-output fields
- EvmAddress is not defined/imported in this file; use Address (already imported from viem).
- The JSDoc on Line 313 incorrectly describes tx hash; it should describe output token addresses.
Apply this diff:
- /** @description The tx hash on the EVM chain */
- outputTokenAddresses?: EvmAddress[];
+ /** @description Output token contract addresses (from strategies). Matches outputTokenAmounts by index. */
+ outputTokenAddresses?: Address[];
/** @description The output amount (from strategies) */
outputTokenAmounts?: string[];
Run to confirm EvmAddress isn’t defined anywhere:
🏁 Script executed:
#!/bin/bash
rg -n -C2 '\bEvmAddress\b'
Length of output: 394
🏁 Script executed:
#!/bin/bash
rg -n "import.*\bAddress\b" -C2 sdk/src/gateway/types.ts
Length of output: 208
Fix undefined EvmAddress and correct JSDoc for outputTokenAddresses
- Replace the undefined
EvmAddress
type with the importedAddress
fromviem
. - Update JSDoc to accurately describe
outputTokenAddresses
.
--- a/sdk/src/gateway/types.ts
+++ b/sdk/src/gateway/types.ts
@@ lines 311-316
- /** @description The tx hash on the EVM chain */
- outputTokenAddresses?: EvmAddress[];
+ /** @description Output token contract addresses (from strategies). Matches outputTokenAmounts by index. */
+ outputTokenAddresses?: Address[];
/** @description The output amount (from strategies) */
outputTokenAmounts?: string[];
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
outputTokenAddresses?: EvmAddress[]; | |
/** @description The output amount (from strategies) */ | |
outputTokenAmounts?: string[]; | |
/** @description The tx hash on the EVM chain */ | |
/** @description Output token contract addresses (from strategies). Matches outputTokenAmounts by index. */ | |
outputTokenAddresses?: Address[]; | |
/** @description The output amount (from strategies) */ | |
outputTokenAmounts?: string[]; | |
/** @description The tx hash on the EVM chain */ |
🤖 Prompt for AI Agents
In sdk/src/gateway/types.ts around lines 313 to 316, the type EvmAddress is
undefined and the JSDoc for outputTokenAddresses is inaccurate; replace
EvmAddress with the imported Address type from 'viem' (add an import if not
present) and update the JSDoc to accurately describe that outputTokenAddresses
is an array of token contract addresses (Address[]) returned by strategies;
ensure the type is used for both outputTokenAddresses and any related
documentation/comments.
/** @deprecated use getOutputTokenAmounts instead */ | ||
getTokenAddress(): string | undefined; | ||
/** @description Get the actual token received */ | ||
/** @deprecated use getOutputTokenAmounts instead */ | ||
getToken(): Token | undefined; | ||
/** @description Get the actual amount received of the token */ | ||
/** @deprecated use getOutputTokenAmounts instead */ | ||
getTokenAmount(): string | number | undefined; |
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.
Deprecated guidance points to a non-existent API; use getOutputs()
The deprecation notes recommend getOutputTokenAmounts, which doesn’t exist. The intended replacement is getOutputs().
Apply this diff:
- /** @deprecated use getOutputTokenAmounts instead */
+ /** @deprecated use getOutputs() instead */
getTokenAddress(): string | undefined;
/** @description Get the actual token received */
- /** @deprecated use getOutputTokenAmounts instead */
+ /** @deprecated use getOutputs() instead */
getToken(): Token | undefined;
/** @description Get the actual amount received of the token */
- /** @deprecated use getOutputTokenAmounts instead */
+ /** @deprecated use getOutputs() instead */
getTokenAmount(): string | number | undefined;
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
/** @deprecated use getOutputTokenAmounts instead */ | |
getTokenAddress(): string | undefined; | |
/** @description Get the actual token received */ | |
/** @deprecated use getOutputTokenAmounts instead */ | |
getToken(): Token | undefined; | |
/** @description Get the actual amount received of the token */ | |
/** @deprecated use getOutputTokenAmounts instead */ | |
getTokenAmount(): string | number | undefined; | |
/** @deprecated use getOutputs() instead */ | |
getTokenAddress(): string | undefined; | |
/** @description Get the actual token received */ | |
/** @deprecated use getOutputs() instead */ | |
getToken(): Token | undefined; | |
/** @description Get the actual amount received of the token */ | |
/** @deprecated use getOutputs() instead */ | |
getTokenAmount(): string | number | undefined; |
🤖 Prompt for AI Agents
In sdk/src/gateway/types.ts around lines 355 to 362, the deprecation JSDoc
refers to a non-existent getOutputTokenAmounts API; update the deprecation notes
to point to the correct replacement getOutputs(). Edit the three deprecated
method comments (getTokenAddress, getToken, getTokenAmount) so their
"@deprecated" lines say "use getOutputs() instead" and optionally update any
accompanying "@description" text to match the new API naming.
Description
We want to be able to list multiple output tokens from an order instead of just one. The existing functions will continue to work but if there are multiple output tokens on an order, it will only return one token. Hence, it's recommended to use the new
getOutputs
methodSummary by CodeRabbit
New Features
Documentation