refactor: move AcrossApi types into separate file #292
Conversation
WalkthroughThis change refactors the Across bridging provider codebase by extracting several TypeScript interface definitions from the main implementation files into a new dedicated Changes
Suggested reviewers
Poem
✨ Finishing Touches
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:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/bridging/providers/across/types.ts (1)
21-22: Consider cleaning up commented codeThere are commented-out fields for
inputTokenandoutputToken. If these are no longer needed, consider removing them completely rather than keeping them commented.- // inputToken: string - // outputToken: string
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/bridging/providers/across/AcrossApi.test.ts(1 hunks)src/bridging/providers/across/AcrossApi.ts(1 hunks)src/bridging/providers/across/AcrossBridgeProvider.test.ts(3 hunks)src/bridging/providers/across/AcrossBridgeProvider.ts(1 hunks)src/bridging/providers/across/types.ts(1 hunks)src/bridging/providers/across/util.test.ts(1 hunks)src/bridging/providers/across/util.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
src/bridging/providers/across/AcrossApi.ts (1)
Learnt from: anxolin
PR: cowprotocol/cow-sdk#251
File: src/bridging/providers/across/AcrossApi.ts:5-10
Timestamp: 2025-03-21T15:14:59.767Z
Learning: In AcrossApi.ts, interfaces like AvailableRoutesRequest and Route use string types for chain IDs (originChainId, destinationChainId) because they're mapping to an external API contract from Across, which expects chain IDs as strings, rather than using internal enum types like TargetChainId.
src/bridging/providers/across/AcrossBridgeProvider.test.ts (1)
Learnt from: anxolin
PR: cowprotocol/cow-sdk#267
File: src/bridging/BridgingSdk/getCrossChainOrder.ts:44-46
Timestamp: 2025-04-14T20:37:56.543Z
Learning: The decoding of bridge appData (using provider.decodeBridgeHook) in the getCrossChainOrder function was intentionally left as a TODO comment for implementation in a future PR.
src/bridging/providers/across/types.ts (1)
Learnt from: anxolin
PR: cowprotocol/cow-sdk#251
File: src/bridging/providers/across/AcrossApi.ts:5-10
Timestamp: 2025-03-21T15:14:59.767Z
Learning: In AcrossApi.ts, interfaces like AvailableRoutesRequest and Route use string types for chain IDs (originChainId, destinationChainId) because they're mapping to an external API contract from Across, which expects chain IDs as strings, rather than using internal enum types like TargetChainId.
🧬 Code Graph Analysis (2)
src/bridging/providers/across/AcrossBridgeProvider.test.ts (1)
src/chains/types.ts (1)
TargetChainId(31-31)
src/bridging/providers/across/types.ts (1)
src/chains/types.ts (1)
TargetChainId(31-31)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: test
- GitHub Check: Build Package
- GitHub Check: eslint
🔇 Additional comments (17)
src/bridging/providers/across/types.ts (6)
3-8: LGTM: Well-defined interface for route requests!The
AvailableRoutesRequestinterface is properly defined with clear field names for origin and destination chain IDs and tokens.
10-17: LGTM: Consistent with request interfaceThe
Routeinterface appropriately extends the request fields with additional token symbol information.
19-67: Note the type difference between chain ID fieldsI notice that while
AvailableRoutesRequestandRouteusestringfor chain IDs,SuggestedFeesRequestusesTargetChainId. This difference is intentional as the route interfaces map to an external API contract from Across that expects string chain IDs, whereasSuggestedFeesRequestis using internal types.
69-111: LGTM: Well-documented deposit limitsThe
SuggestedFeesLimitsinterface is thoroughly documented with clear explanations for each limit type and detailed comments explaining timing expectations per chain.
113-187: LGTM: Comprehensive response interfaceThe
SuggestedFeesResponseinterface provides a detailed structure for fee breakdowns with appropriate documentation for each field.
189-198: LGTM: Clear percentage fee representationThe
PctFeeinterface clearly defines how percentage fees are represented, with helpful comments explaining the fixed-point format.src/bridging/providers/across/util.test.ts (1)
6-6: LGTM: Correct import path updatedThe import path for
SuggestedFeesResponsehas been correctly updated to use the new types file.src/bridging/providers/across/util.ts (1)
7-7: LGTM: Correct import path updatedThe import path for
SuggestedFeesResponsehas been correctly updated to use the new types file.src/bridging/providers/across/AcrossBridgeProvider.ts (1)
27-33: LGTM: Proper separation of importsThe imports have been correctly organized to separate the API class imports from the type imports.
AcrossApiandAcrossApiOptionsare still imported from the original file, whileSuggestedFeesResponsenow comes from the new types module.src/bridging/providers/across/AcrossApi.test.ts (1)
1-3: Imports correctly updated to reflect the type refactoring.The import statements have been properly adjusted to reflect the type definitions being moved to a dedicated
types.tsfile. TheAcrossApiclass is now imported separately from the typesSuggestedFeesRequestandSuggestedFeesResponse.src/bridging/providers/across/AcrossBridgeProvider.test.ts (3)
4-8: Import statements correctly restructured for better type organization.The imports have been properly reorganized:
- Types like
BridgeQuoteResultandQuoteBridgeRequestare now imported from '../../types'SuggestedFeesResponseis imported from the new dedicated './types' file- Added
latestAppDatafrom '@cowprotocol/app-data/dist/generatedTypes' to support the updated type usage in thedecodeBridgeHooktestThis aligns with the overall refactoring to improve type organization.
81-81: Simplified argument for unsupported chain test.The argument for testing unsupported chains has been simplified from an object with a
targetChainIdproperty to directly using a numeric value with a type assertion. This makes the test more concise while maintaining type safety.
173-175: Updated type casting indecodeBridgeHooktest.The test now properly uses
latestAppData.CoWHookfor type casting instead ofBridgeHook. This change aligns with the note in the retrieved learnings that mentions the implementation ofdecodeBridgeHookwas intentionally left as a TODO for a future PR.src/bridging/providers/across/AcrossApi.ts (4)
1-9: Types correctly extracted to dedicated file.The type definitions have been properly moved from inline declarations to imports from the new './types' module. This includes:
AvailableRoutesRequestPctFeeRouteSuggestedFeesLimitsSuggestedFeesRequestSuggestedFeesResponseThis refactoring improves code organization by separating type declarations from implementation logic, making the codebase more maintainable.
51-71: Implementation ofgetSuggestedFeesremains unchanged.The implementation logic continues to work correctly with the imported types. The method correctly handles the request parameters and makes the API call to retrieve fee suggestions from the Across API.
Note: I see there's a TODO comment (lines 66-67) mentioning a potential discrepancy between documented API parameters and actual implementation. This is unrelated to the current refactoring but might be worth addressing in a future PR.
119-142: Validation function continues to work with imported types.The
isValidSuggestedFeesResponsefunction correctly uses the importedSuggestedFeesResponseandPctFeetypes. The validation logic remains unchanged and will continue to work as expected.
144-187: Helper validation functions properly reference imported types.The helper functions
isValidPctFee,isValidSuggestedFeeLimits,isValidRoutes, andisValidRoutecorrectly use the imported types. These functions remain unchanged in logic and will continue to validate the API responses properly.
Just moved types from
AcrossApi.tsinto a separate file.Also fixed one test.
Summary by CodeRabbit