feat(synapse-interface): SDK bridgequote chainID validation #2803
Conversation
abtestingalpha
requested review from
bigboydiamonds,
aureliusbtc,
ChiTimesChi and
trajan0x
as code owners
|
Warning Rate limit exceeded@abtestingalpha has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 36 minutes and 36 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe recent updates primarily enhance the Synapse module's functionality by integrating Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
- Added
originChainIdanddestChainIdtoBridgeQuoteinsynapseModuleSet.ts - Updated
BridgeQuotetype with chain IDs intypes.ts - Enhanced test cases for chain ID validation in
sdk.test.ts - Introduced chain ID validation in
BridgeTransactionButton.tsx - Included chain IDs in
EMPTY_BRIDGE_QUOTEobjects inbridge.ts - Updated
package.jsonto use local SDK changes - Added debugging logs and chain IDs in
index.tsx - Imported
BridgeQuoteandTokentypes inreducer.ts - Enhanced
BridgeQuotetype with chain IDs intypes/index.tsx
9 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (5)
packages/synapse-interface/components/StateManagedBridge/BridgeTransactionButton.tsx (1)
Line range hint
66-66: Consider Using Optional ChainingThe static analysis tool suggests using optional chaining for safer property access. This is generally a good practice to prevent runtime errors due to
nullorundefinedvalues, especially in a dynamic language like JavaScript.- bridgeQuote.originChainId + bridgeQuote?.originChainIdpackages/synapse-interface/utils/types/index.tsx (2)
Line range hint
319-319: Simplify Conditional ExpressionThe use of a ternary operator here is unnecessary. Simplifying this by directly assigning the boolean value can improve code readability.
- const isMetaVar = Boolean(swapDepositAddresses || forceMeta) + const isMetaVar = !!(swapDepositAddresses || forceMeta)
Line range hint
341-347: Remove Redundant Else ClauseThe else clause is unnecessary as the previous branches include return statements. Removing it can simplify the control flow and improve readability.
- else { - return obj - } + return objpackages/synapse-interface/pages/state-managed-bridge/index.tsx (2)
Line range hint
230-230: Simplify boolean expression.Consider simplifying the boolean expression to enhance code readability and reduce complexity.
- const isUnsupported = AcceptedChainId[fromChainId] ? false : true + const isUnsupported = !AcceptedChainId[fromChainId]
Line range hint
578-578: Add missingkeyproperty in iterable elements.React elements in a list require a
keyproperty to maintain their identity across re-renders, which improves performance and prevents UI issues.- <>{instance.MaintenanceCountdownProgressBar}</> + <div key={instance.id}>{instance.MaintenanceCountdownProgressBar}</div>
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (9)
- packages/sdk-router/src/module/synapseModuleSet.ts (1 hunks)
- packages/sdk-router/src/module/types.ts (1 hunks)
- packages/sdk-router/src/sdk.test.ts (13 hunks)
- packages/synapse-interface/components/StateManagedBridge/BridgeTransactionButton.tsx (2 hunks)
- packages/synapse-interface/constants/bridge.ts (2 hunks)
- packages/synapse-interface/package.json (1 hunks)
- packages/synapse-interface/pages/state-managed-bridge/index.tsx (3 hunks)
- packages/synapse-interface/slices/bridge/reducer.ts (1 hunks)
- packages/synapse-interface/utils/types/index.tsx (1 hunks)
Files skipped from review due to trivial changes (2)
- packages/sdk-router/src/sdk.test.ts
- packages/synapse-interface/package.json
Additional context used
Biome
packages/synapse-interface/components/StateManagedBridge/BridgeTransactionButton.tsx
[error] 66-66: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/synapse-interface/utils/types/index.tsx
[error] 319-319: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
[error] 341-347: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
packages/synapse-interface/pages/state-managed-bridge/index.tsx
[error] 230-230: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
[error] 578-578: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
Additional comments not posted (8)
packages/synapse-interface/constants/bridge.ts (1)
20-21: Proper Initialization of Chain IDs in ConstantsThe addition of
originChainIdanddestChainIdset tonullin bothEMPTY_BRIDGE_QUOTEandEMPTY_BRIDGE_QUOTE_ZEROconstants is a correct implementation to support the new functionality of tracking chain IDs. This change ensures that these objects are ready to be used with the new data structure requirements.Also applies to: 38-39
packages/sdk-router/src/module/types.ts (1)
67-68: UpdatedBridgeQuoteType with Chain IDsThe addition of
originChainIdanddestChainIdfields to theBridgeQuotetype is correctly implemented. This update is essential for tracking the chain IDs in bridge transactions, aligning with the PR's objectives.packages/synapse-interface/components/StateManagedBridge/BridgeTransactionButton.tsx (1)
79-84: Addition ofchainSelectionsMatchBridgeQuoteFunctionThe new function
chainSelectionsMatchBridgeQuotecorrectly checks the match between user-selected chain IDs and those in the bridge quote. This is a necessary addition for ensuring the UI's consistency with the backend state.packages/sdk-router/src/module/synapseModuleSet.ts (1)
218-219: UpdatedfinalizeBridgeRouteMethod with Chain IDsThe inclusion of
originChainIdanddestChainIdin thefinalizeBridgeRoutemethod is correctly implemented. This enhancement aligns with the PR's objectives to track chain IDs more thoroughly across the application.packages/synapse-interface/utils/types/index.tsx (1)
86-87: UpdatedBridgeQuoteType in Utility TypesThe addition of
originChainIdanddestChainIdto theBridgeQuotetype in the utility types is correctly implemented, ensuring consistency across different parts of the application.packages/synapse-interface/slices/bridge/reducer.ts (1)
5-5: Use of TypeScripttypeimports is appropriate.The modification to use
typeimports forBridgeQuoteandTokenis a good practice in TypeScript to avoid runtime import overhead.packages/synapse-interface/pages/state-managed-bridge/index.tsx (2)
167-167: Logging addition is beneficial for debugging.Adding a console log for
allBridgeQuoteshelps in tracking the fetch and filter operations on bridge quotes, which is useful for debugging and ensuring data integrity.
283-284: Enhancement by adding chain IDs to bridge quotes.The inclusion of
originChainIdanddestChainIdin the bridge quote enriches the transaction data, providing critical information for cross-chain operations and improving traceability.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2803 +/- ##
===================================================
+ Coverage 25.60341% 25.71943% +0.11601%
===================================================
Files 758 700 -58
Lines 54399 51673 -2726
Branches 80 80
===================================================
- Hits 13928 13290 -638
+ Misses 39037 37019 -2018
+ Partials 1434 1364 -70
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Deploying sanguine with
|
| Latest commit: |
89ed125
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://15c21c5e.sanguine.pages.dev |
| Branch Preview URL: | https://fe-bridgequote-chain-ids.sanguine.pages.dev |
There was a problem hiding this comment.
PR Summary
(updates since last review)
- Removed async thunks
fetchAndStoreBridgeQuoteandfetchAndStoreBridgeQuotesfromhooks.ts - Removed extra reducers handling
fetchAndStoreBridgeQuotesinreducer.ts - Refactored
fetchBridgeQuotes.tsx, removingfetchBridgeQuote,fetchBridgeQuotes, andBridgeQuoteRequest - Deleted
useAlternateBridgeQuotes.tscustom hook - Added
destinationTokenanddestinationChainIdtoBridgeQuoteResponseintypes/index.tsx
5 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (2)
packages/synapse-interface/utils/types/index.tsx (2)
Line range hint
324-324: Simplify Boolean ExpressionThe use of a ternary operator here is unnecessary and can be simplified.
- this.isEthSwap = swapEthAddresses ? true : false + this.isEthSwap = Boolean(swapEthAddresses)
Line range hint
346-352: Remove Redundant Else ClauseThe else clause following an early return is redundant and can be omitted for cleaner code.
if (condition) { return value; } - else { - other code - } + other code
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (4)
- packages/synapse-interface/slices/bridge/hooks.ts (2 hunks)
- packages/synapse-interface/slices/bridge/reducer.ts (3 hunks)
- packages/synapse-interface/utils/actions/fetchBridgeQuotes.tsx (1 hunks)
- packages/synapse-interface/utils/types/index.tsx (1 hunks)
Files skipped from review due to trivial changes (1)
- packages/synapse-interface/slices/bridge/hooks.ts
Files skipped from review as they are similar to previous changes (1)
- packages/synapse-interface/slices/bridge/reducer.ts
Additional context used
Biome
packages/synapse-interface/utils/types/index.tsx
[error] 324-324: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
[error] 346-352: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
Additional comments not posted (1)
packages/synapse-interface/utils/types/index.tsx (1)
86-88: Update toBridgeQuotetype: Addition of Chain IDsThe addition of
originChainIdanddestChainIdto theBridgeQuotetype is consistent with the PR's objective to enhance bridge routing and quote management by including chain IDs.
Deploying sanguine-fe with
|
| Latest commit: |
6f571ab
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://44f58db6.sanguine-fe.pages.dev |
| Branch Preview URL: | https://fe-bridgequote-chain-ids.sanguine-fe.pages.dev |
There was a problem hiding this comment.
PR Summary
(updates since last review)
- Enhanced
BridgeQuotedetails withoriginChainIdanddestChainId - Improved chain ID comparison for button state and label accuracy
- Updated
@synapsecns/sdk-routerdependency to version 0.7.0 - Added Scroll chain support in
widgetpackage - Refactored
fetchBridgeQuotefunctionality and state management hooks
18 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
PR Summary
(updates since last review)
- Enhanced
BridgeQuotedetails to includeoriginChainIdanddestChainId - Improved chain ID comparison for button states and labels
- Updated package dependency for
@synapsecns/sdk-router - Restructured
fetchBridgeQuotefunctionality, removing unused imports - Adjusted type declarations in import statements for better clarity
2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (2)
packages/synapse-interface/pages/state-managed-bridge/index.tsx (2)
Line range hint
228-228: Simplify boolean assignment forisUnsupported.The current implementation uses a ternary operator unnecessarily. You can simplify this by directly assigning the result of the condition.
- const isUnsupported = AcceptedChainId[fromChainId] ? false : true + const isUnsupported = !AcceptedChainId[fromChainId]
Line range hint
576-576: Add akeyproperty to the mappedMaintenanceCountdownProgressBarcomponents.React requires a unique
keyprop on elements in a list for efficient updates. This is missing in the current implementation.- <>{instance.MaintenanceCountdownProgressBar}</> + <div key={instance.id}>{instance.MaintenanceCountdownProgressBar}</div>
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- packages/synapse-interface/pages/state-managed-bridge/index.tsx (2 hunks)
Additional context used
Biome
packages/synapse-interface/pages/state-managed-bridge/index.tsx
[error] 228-228: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
[error] 576-576: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
Additional comments not posted (1)
packages/synapse-interface/pages/state-managed-bridge/index.tsx (1)
204-205: Ensure proper integration oforiginChainIdanddestChainIdin bridge quote updates.The addition of
originChainIdanddestChainIdto the bridge quote structure is crucial for the new validation feature. It's important to ensure these fields are correctly used and integrated throughout the component's logic.Also applies to: 281-282
There was a problem hiding this comment.
PR Summary
(updates since last review)
- Added
originChainIdanddestChainIdto bridge quotes for detailed route info - Enhanced chain ID validation affecting button states in
BridgeTransactionButton - Improved logging for bridge quotes
- Updated dependencies and imports
- Refactored functions for fetching and processing bridge quotes, removed unused imports/interfaces
2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
PR Summary
(updates since last review)
- Added
originChainIdanddestChainIdto bridge quotes for detailed route info - Button states now influenced by chain ID comparisons
- Enhanced logging for bridge quotes
- Refactored functions for fetching and processing bridge quotes
- Updated dependencies and imports for consistency
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
abtestingalpha
force-pushed
the
fe/bridgequote-chain-ids
branch
from
89ed125 to
6f571ab
Compare
There was a problem hiding this comment.
PR Summary
(updates since last review)
- Added
originChainIdanddestChainIdproperties to bridge quotes for detailed route info - Introduced chain ID comparison function affecting button states in
BridgeTransactionButton - Enhanced logging for bridge quotes
- Updated dependencies and imports
- Restructured functions for fetching and processing bridge quotes, removed unused imports/interfaces
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
Summary by CodeRabbit
New Features
originChainIdanddestChainIdto bridge quotes for more detailed route information.Bug Fixes
Chores
Refactor
7c5975c: synapse-interface preview link
f23f66a: synapse-interface preview link
cc071d5: synapse-interface preview link
4b8644d: synapse-interface preview link
94bd2da: synapse-interface preview link
54993fa: synapse-interface preview link