Conversation
Introduce a Squid integration: add provider, client, and model files to packages/swapper/src/squid, plus unit and optional live integration tests. Export the new squid module from the swapper package and wire SquidProvider into the apps/api provider list, using a new SQUID_INTEGRATOR_ID env var (added to .env.example). The SquidProvider maps Cosmos chains/assets to Squid identifiers and uses fetchRoute to obtain quotes and transaction data.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly expands the platform's cross-chain swapping capabilities by integrating the Squid routing protocol. It introduces a dedicated Squid provider within the swapper package, enabling the application to fetch quotes and transaction data for various Cosmos-based chains and assets. This integration enhances the system's ability to facilitate seamless asset transfers across different blockchain networks. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new SquidProvider for cross-chain swaps and integrates it into the API. The implementation is well-structured, with clear separation of the client, provider logic, and models. The addition of both unit and integration tests is a great practice. My review focuses on improving robustness, fixing a broken test, and increasing efficiency. Key points include handling missing environment variables more gracefully, correcting a potential bug in API error handling, fixing a broken integration test, and improving test code consistency. Addressing these points will make the new provider more robust and reliable.
| const routeData = quote.route_data as { estimate: { gasCosts: object[]; actions: object[] } }; | ||
| expect(routeData.estimate.gasCosts.length).toBeGreaterThan(0); | ||
| expect(routeData.estimate.actions.length).toBeGreaterThan(0); |
There was a problem hiding this comment.
This test will fail with a TypeError. The get_quote method in SquidProvider currently returns route_data: {}. Therefore, quote.route_data is an empty object, and trying to access routeData.estimate will throw an error.
To fix this, get_quote should be updated to populate route_data with the response from the Squid API. Additionally, the SquidEstimate model in model.ts does not contain gasCosts or actions, so either the model needs to be updated or this assertion is incorrect.
apps/api/src/index.ts
Outdated
| orca: new OrcaWhirlpoolProvider(solanaRpc), | ||
| panora: new PanoraProvider(), | ||
| okx: new OkxProvider(solanaRpc), | ||
| squid: new SquidProvider(process.env.SQUID_INTEGRATOR_ID || ""), |
There was a problem hiding this comment.
Passing an empty string for integratorId when the SQUID_INTEGRATOR_ID environment variable is missing can lead to silent failures at runtime. The provider will be created, but API calls to Squid will likely fail, which can be difficult to debug. It's better to ensure required environment variables are present at application startup and fail fast if they are not.
| if (!response.ok) { | ||
| let detail: string; | ||
| try { | ||
| const errorBody = (await response.json()) as SquidErrorResponse; | ||
| detail = errorBody.errors?.[0]?.message || errorBody.message || response.statusText; | ||
| } catch { | ||
| detail = await response.text(); | ||
| } | ||
| throw new Error(`Squid API error ${response.status}: ${detail}`); | ||
| } | ||
|
|
||
| return (await response.json()) as SquidRouteResponse; | ||
| } |
There was a problem hiding this comment.
This error handling block has a potential bug. The response body can only be read once. If !response.ok is true and await response.json() fails (e.g., the body is not valid JSON), the subsequent await response.text() in the catch block will also fail because the body stream is already consumed. A safer approach is to read the body as text first, then try to parse it.
Additionally, consider throwing a SwapperException (from ../error) for consistency with the rest of the swapper framework, which allows for more structured error handling upstream.
| if (!response.ok) { | |
| let detail: string; | |
| try { | |
| const errorBody = (await response.json()) as SquidErrorResponse; | |
| detail = errorBody.errors?.[0]?.message || errorBody.message || response.statusText; | |
| } catch { | |
| detail = await response.text(); | |
| } | |
| throw new Error(`Squid API error ${response.status}: ${detail}`); | |
| } | |
| return (await response.json()) as SquidRouteResponse; | |
| } | |
| const responseText = await response.text(); | |
| if (!response.ok) { | |
| let detail: string; | |
| try { | |
| const errorBody = JSON.parse(responseText) as SquidErrorResponse; | |
| detail = errorBody.errors?.[0]?.message || errorBody.message || response.statusText; | |
| } catch { | |
| detail = responseText || response.statusText; | |
| } | |
| throw new Error(`Squid API error ${response.status}: ${detail}`); | |
| } | |
| return JSON.parse(responseText) as SquidRouteResponse; | |
| } |
| quote: quoteRequest, | ||
| output_value: route.estimate.toAmount, | ||
| output_min_value: route.estimate.toAmountMin, | ||
| route_data: {}, |
There was a problem hiding this comment.
Returning an empty object for route_data causes the integration test to fail and is inefficient, as the get_quote_data method has to make a second API call to get the transaction data.
By populating route_data with the route object from the API response, you can fix the integration test and optimize get_quote_data to avoid the second network request.
| route_data: {}, | |
| route_data: route, |
| slippage_bps: 100, | ||
| }; | ||
|
|
||
| describe("SquidProvider", () => { |
There was a problem hiding this comment.
Mock cleanup is handled inconsistently within this test suite. Some tests use mockRestore() on a specific spy, while others use jest.restoreAllMocks(). To improve consistency and ensure proper test isolation, it's a good practice to use a lifecycle hook like afterEach for cleanup.
describe("SquidProvider", () => {
afterEach(() => {
jest.restoreAllMocks();
});
// ... your tests without manual restore calls
});
1 participant
Introduce a Squid integration: add provider, client, and model files to packages/swapper/src/squid, plus unit and optional live integration tests. Export the new squid module from the swapper package and wire SquidProvider into the apps/api provider list, using a new SQUID_INTEGRATOR_ID env var (added to .env.example). The SquidProvider maps Cosmos chains/assets to Squid identifiers and uses fetchRoute to obtain quotes and transaction data.