Skip to content
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

feat(synapse-interface): bridge quote state validations #3019

Merged
merged 15 commits into from
Aug 26, 2024

Conversation

abtestingalpha
Copy link
Collaborator

@abtestingalpha abtestingalpha commented Aug 13, 2024

Summary by CodeRabbit

  • New Features

    • Introduced a new hook for managing bridge quote state, enhancing component data retrieval.
    • Added a thunk for fetching bridge quotes, improving cross-chain token transfer functionality.
    • Updated BridgeQuote structure to include a requestId for better request tracking.
  • Bug Fixes

    • Improved logic for button state management in bridge transactions.
    • Enhanced error handling and toast notifications for fetching bridge quotes.
  • Refactor

    • Streamlined state management by consolidating bridge quote handling into a dedicated slice.
    • Enhanced clarity of data access paths across various components.
    • Simplified approval checks and removed redundant state variables.
  • Chores

    • Removed deprecated constants and hooks for improved maintainability.

9df61cd: synapse-interface preview link
fef82f9: synapse-interface preview link
2154755: synapse-interface preview link
34c910b: synapse-interface preview link
450c42e: synapse-interface preview link
30c9c8f: synapse-interface preview link
f561eb0: synapse-interface preview link
b85c19a: synapse-interface preview link
7d6d0b4: synapse-interface preview link
dfff7bb: synapse-interface preview link
63abed7: synapse-interface preview link
8840c68: synapse-interface preview link
140adef: synapse-interface preview link
26ea9c9: synapse-interface preview link
88aff75: synapse-interface preview link
664ce93: synapse-interface preview link

@abtestingalpha abtestingalpha marked this pull request as draft August 13, 2024 23:19
@abtestingalpha abtestingalpha changed the title Fe/bridge quote state [wip] Fe/bridge quote state Aug 13, 2024
Copy link
Contributor

coderabbitai bot commented Aug 13, 2024

Walkthrough

The changes enhance the management of bridge quotes within the Synapse interface by introducing a dedicated state slice for bridgeQuote. This refactoring streamlines how components access quote data and loading states, replacing previous hooks and state management patterns. The updates promote improved modularity and clarity across the application, facilitating better handling of cross-chain token transfers.

Changes

Files Change Summary
packages/synapse-interface/components/StateManagedBridge/BridgeTransactionButton.tsx
OutputContainer.tsx
Introduced useBridgeValidations and useBridgeQuoteState for better validation and state management.
packages/synapse-interface/constants/bridge.ts Added requestId to EMPTY_BRIDGE_QUOTE, removed EMPTY_BRIDGE_QUOTE_ZERO, simplifying the structure.
packages/synapse-interface/pages/state-managed-bridge/index.tsx Updated bridge quote handling to use fetchBridgeQuote, removing direct state manipulation for better clarity.
packages/synapse-interface/slices/bridge/reducer.ts Removed bridgeQuote and isLoading properties from BridgeState, streamlining state management.
packages/synapse-interface/slices/bridgeQuote/reducer.ts Defined a new Redux slice for bridge quotes with loading and error handling.
packages/synapse-interface/slices/bridgeQuote/thunks.ts Created fetchBridgeQuote thunk for asynchronous bridge quote fetching.
packages/synapse-interface/utils/types/index.tsx Added requestId to the BridgeQuote type for improved tracking.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant UI
    participant Store
    participant Thunk
    participant API

    User->>UI: Request bridge quote
    UI->>Store: Dispatch fetchBridgeQuote
    Store->>Thunk: Invoke fetchBridgeQuote
    Thunk->>API: Fetch available bridge quotes
    API-->>Thunk: Return quotes
    Thunk->>Store: Update bridgeQuote state
    Store-->>UI: Provide updated bridge quote
    UI-->>User: Display bridge quote
Loading

🐰 "In the garden, bright and neat,
A bridge of quotes, oh so sweet!
With each new hop, we fetch with cheer,
Tracking requests, we hold them near.
A simpler path, a clearer way,
Let's celebrate this fine new day!" 🌼


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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

Outside diff range, codebase verification and nitpick comments (3)
packages/synapse-interface/slices/bridgeQuote/thunks.ts (1)

63-66: Clarify error message for no active quotes.

The error message could be more informative by including details about the possible reasons for no active quotes.

const msg = `No active bridge route found for ${debouncedFromValue} ${fromToken?.symbol} from ${CHAINS_BY_ID[fromChainId]?.name} to ${toToken?.symbol} on ${CHAINS_BY_ID[toChainId]?.name}. This could be due to paused modules or unsupported routes.`
packages/synapse-interface/slices/bridge/reducer.ts (1)

1-3: References to bridgeQuote and isLoading Still Exist in the Codebase

The properties bridgeQuote and isLoading are still referenced in multiple parts of the codebase, particularly within the bridgeQuote slice and related components. This indicates that their removal from BridgeState could potentially break functionality if not properly handled.

  • Files with References:
    • packages/widget/src/state/slices/bridgeQuote/reducer.ts
    • packages/widget/src/components/Widget.tsx
    • packages/synapse-interface/pages/state-managed-bridge/index.tsx

Please ensure that all logic dependent on these properties is updated accordingly to prevent any issues.

Analysis chain

Removal of bridgeQuote and isLoading from BridgeState.

The removal of bridgeQuote and isLoading properties from BridgeState simplifies the state structure. Ensure that any logic dependent on these properties is updated elsewhere in the application.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the removal of `bridgeQuote` and `isLoading` does not affect other parts of the application.

# Test: Search for usage of removed properties. Expect: No remaining references.
rg --type js --type ts 'bridgeQuote|isLoading'

Length of output: 27908


Script:

#!/bin/bash
# Verify the removal of `bridgeQuote` and `isLoading` from `BridgeState` in the specific file.
rg --type ts 'bridgeQuote|isLoading' packages/synapse-interface/slices/bridge/reducer.ts -A 5 -B 5

Length of output: 100


Script:

#!/bin/bash
# Search for the definition of `bridgeQuote` and `isLoading` in the codebase to determine their current status.
rg --type ts 'bridgeQuote|isLoading' -A 5 -B 5

Length of output: 160186

packages/synapse-interface/pages/state-managed-bridge/index.tsx (1)

Line range hint 154-208: Ensure comprehensive error handling in getAndSetBridgeQuote.

The function handles success and error states, but ensure that all potential error cases are covered. Consider logging errors or providing more detailed error feedback to the user.

- console.log(err)
+ console.error('Error fetching bridge quote:', err)

Consider using console.error for better error logging.

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 751857d and e577fe3.

Files selected for processing (13)
  • packages/synapse-interface/components/StateManagedBridge/BridgeExchangeRateInfo.tsx (5 hunks)
  • packages/synapse-interface/components/StateManagedBridge/BridgeTransactionButton.tsx (4 hunks)
  • packages/synapse-interface/components/StateManagedBridge/OutputContainer.tsx (1 hunks)
  • packages/synapse-interface/constants/bridge.ts (1 hunks)
  • packages/synapse-interface/pages/state-managed-bridge/index.tsx (9 hunks)
  • packages/synapse-interface/slices/bridge/reducer.ts (7 hunks)
  • packages/synapse-interface/slices/bridgeQuote/hooks.ts (1 hunks)
  • packages/synapse-interface/slices/bridgeQuote/reducer.ts (1 hunks)
  • packages/synapse-interface/slices/bridgeQuote/thunks.ts (1 hunks)
  • packages/synapse-interface/store/reducer.ts (2 hunks)
  • packages/synapse-interface/store/store.ts (3 hunks)
  • packages/synapse-interface/utils/hooks/useBridgeListener.ts (1 hunks)
  • packages/synapse-interface/utils/types/index.tsx (1 hunks)
Additional context used
GitHub Check: lint
packages/synapse-interface/slices/bridgeQuote/reducer.ts

[failure] 44-44:
'action' is defined but never used. Allowed unused args must match /^_/u

Biome
packages/synapse-interface/slices/bridgeQuote/thunks.ts

[error] 139-139: 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)

Additional comments not posted (19)
packages/synapse-interface/slices/bridgeQuote/hooks.ts (1)

1-6: LGTM!

The useBridgeQuoteState hook is correctly implemented to select the bridgeQuote state from the Redux store. This follows best practices for state management in React using Redux.

packages/synapse-interface/constants/bridge.ts (2)

22-22: Enhancement: Added requestId to EMPTY_BRIDGE_QUOTE.

The addition of requestId improves traceability for bridge quotes, aligning with the refactoring goals.


Line range hint 1-22:
Verify the removal of EMPTY_BRIDGE_QUOTE_ZERO.

Ensure that the removal of EMPTY_BRIDGE_QUOTE_ZERO does not affect other parts of the codebase where it might have been used.

Verification successful

Removal of EMPTY_BRIDGE_QUOTE_ZERO Verified

The constant EMPTY_BRIDGE_QUOTE_ZERO has been removed from the codebase and is not used elsewhere, ensuring no impact on other parts of the code.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that `EMPTY_BRIDGE_QUOTE_ZERO` is not used elsewhere in the codebase.

# Test: Search for occurrences of `EMPTY_BRIDGE_QUOTE_ZERO`. Expect: No results.
rg --type js --type ts 'EMPTY_BRIDGE_QUOTE_ZERO'

Length of output: 48


Script:

#!/bin/bash
# Description: Verify that the definition of `EMPTY_BRIDGE_QUOTE_ZERO` has been removed from the codebase.

# Test: Search for the definition of `EMPTY_BRIDGE_QUOTE_ZERO`. Expect: No results.
rg --type js --type ts 'const EMPTY_BRIDGE_QUOTE_ZERO' -A 5

Length of output: 61

packages/synapse-interface/utils/hooks/useBridgeListener.ts (2)

11-11: Refactor: Updated setIsLoading import location.

The change in the import location of setIsLoading aligns with the refactor to manage loading states within the bridgeQuote slice, enhancing modularity.


Line range hint 1-11:
Verify the new import location for setIsLoading.

Ensure that the setIsLoading function is correctly defined and exported from the new location @/slices/bridgeQuote/reducer.

Verification successful

The import location for setIsLoading is correct.

The setIsLoading function is correctly defined and exported from @/slices/bridgeQuote/reducer.

  • Verified export in packages/synapse-interface/slices/bridgeQuote/reducer.ts.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that `setIsLoading` is defined and exported from `@/slices/bridgeQuote/reducer`.

# Test: Search for the definition and export of `setIsLoading`. Expect: Correct definition and export.
rg --type js --type ts 'export.*setIsLoading' --context 5

Length of output: 681

packages/synapse-interface/store/reducer.ts (1)

20-20: Ensure persistence if necessary.

The bridgeQuote reducer is integrated into the appReducer. If the bridgeQuote state needs to persist across sessions, consider adding it to the persistedReducers or whitelist in the persistConfig.

packages/synapse-interface/components/StateManagedBridge/OutputContainer.tsx (1)

15-19: Verify the functionality of useBridgeQuoteState.

The switch to useBridgeQuoteState should maintain or improve the component's functionality. Verify that this hook provides the correct bridgeQuote and isLoading states.

packages/synapse-interface/store/store.ts (2)

Line range hint 48-64:
Refactoring improves clarity and maintainability.

The changes streamline access to bridgeQuote and improve the naming of bridgeQuoteState. This enhances readability and aligns with best practices for state management.


Line range hint 57-79:
Event data construction correctly reflects refactored state structure.

The changes ensure that the event data uses the updated bridgeQuote access pattern, maintaining the intended functionality.

packages/synapse-interface/components/StateManagedBridge/BridgeExchangeRateInfo.tsx (4)

59-63: Improved data encapsulation in Slippage component.

The use of useBridgeQuoteState for accessing exchangeRate enhances data encapsulation and maintains the existing logic for calculating slippage.


82-82: Enhanced data management in Router component.

The refactoring to use useBridgeQuoteState for accessing bridgeModuleName improves data management while preserving the component's functionality.


92-93: Promoted separation of concerns in TimeEstimate component.

The use of useBridgeQuoteState for accessing bridgeQuote enhances the separation of concerns and maintains the existing logic for time estimates.


131-134: Improved data encapsulation in GasDropLabel component.

The refactoring to use useBridgeQuoteState for accessing gasDropAmount enhances data encapsulation while preserving the component's functionality.

packages/synapse-interface/components/StateManagedBridge/BridgeTransactionButton.tsx (2)

49-50: Simplified state management with useBridgeQuoteState.

The use of useBridgeQuoteState for managing isLoading and bridgeQuote simplifies state management and maintains the existing button logic.


3-3: Streamlined button control flow.

The removal of checks for EMPTY_BRIDGE_QUOTE_ZERO simplifies the control flow and maintains the intended button functionality.

packages/synapse-interface/utils/types/index.tsx (1)

88-88: Addition of requestId to BridgeQuote.

The addition of requestId to the BridgeQuote type is a positive change that enhances tracking capabilities. Ensure that all relevant parts of the codebase are updated to utilize this new property effectively.

packages/synapse-interface/slices/bridge/reducer.ts (1)

28-30: Ensure all reducers are updated accordingly.

The removal of setIsLoading, setBridgeQuote, and resetBridgeQuote reducers indicates a shift in state management. Verify that the functionality these reducers provided is handled appropriately elsewhere.

packages/synapse-interface/pages/state-managed-bridge/index.tsx (2)

366-366: Verify transaction state reset logic.

Ensure that resetting the bridge quote and destination address state after transaction execution is the intended behavior. Consider potential race conditions or state inconsistencies.

Verification successful

State reset logic after transaction execution is consistent.

The resetBridgeQuote action is used in several places within the StateManagedBridge component to manage state consistency after transactions. This appears to be the intended behavior to handle state resets effectively.

  • Instances of resetBridgeQuote are found in packages/synapse-interface/pages/state-managed-bridge/index.tsx.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the state reset logic after transaction execution.

# Test: Search for the usage of `resetBridgeQuote` after transactions.
rg --type tsx -A 5 $'resetBridgeQuote'

Length of output: 65


Script:

#!/bin/bash
# Description: Verify the state reset logic after transaction execution.

# Test: Search for the usage of `resetBridgeQuote` after transactions in TypeScript and TSX files.
rg --type-add 'tsx:*.{ts,tsx}' --type tsx -A 5 'resetBridgeQuote'

Length of output: 3708


65-67: Verify the usage of new imports.

The new imports useBridgeQuoteState, resetBridgeQuote, and fetchBridgeQuote should be verified for correct usage within the component.

Verification successful

Imports are correctly used in the codebase.

The imported hooks and actions useBridgeQuoteState, resetBridgeQuote, and fetchBridgeQuote are correctly utilized across various components, ensuring proper state management and functionality. No issues were found with their usage.

  • useBridgeQuoteState is used in StateManagedBridge, BridgeExchangeRateInfo, BridgeTransactionButton, and OutputContainer.
  • resetBridgeQuote is used in StateManagedBridge.
  • fetchBridgeQuote is used in StateManagedBridge and Widget.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of newly imported hooks and actions.

# Test: Search for the usage of `useBridgeQuoteState`.
rg --type tsx $'useBridgeQuoteState'

# Test: Search for the usage of `resetBridgeQuote`.
rg --type tsx $'resetBridgeQuote'

# Test: Search for the usage of `fetchBridgeQuote`.
rg --type tsx $'fetchBridgeQuote'

Length of output: 185


Script:

#!/bin/bash
# Description: Verify the usage of newly imported hooks and actions.

# Test: Search for the usage of `useBridgeQuoteState` in .tsx files.
rg 'useBridgeQuoteState' --glob '*.tsx' -A 5

# Test: Search for the usage of `resetBridgeQuote` in .tsx files.
rg 'resetBridgeQuote' --glob '*.tsx' -A 5

# Test: Search for the usage of `fetchBridgeQuote` in .tsx files.
rg 'fetchBridgeQuote' --glob '*.tsx' -A 5

Length of output: 14388

export interface BridgeQuoteState {
bridgeQuote: BridgeQuote
isLoading: boolean
error: any
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using a more specific error type.

The error property is typed as any. Consider using a more specific type to improve type safety and error handling.

state.isLoading = false
}
)
.addCase(fetchBridgeQuote.rejected, (state, action) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove unused parameter action.

The action parameter in the fetchBridgeQuote.rejected case is defined but not used. Consider prefixing it with an underscore to indicate it's intentionally unused or removing it.

- .addCase(fetchBridgeQuote.rejected, (state, action) => {
+ .addCase(fetchBridgeQuote.rejected, (state) => {
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.

Suggested change
.addCase(fetchBridgeQuote.rejected, (state, action) => {
.addCase(fetchBridgeQuote.rejected, (state) => {
Tools
GitHub Check: lint

[failure] 44-44:
'action' is defined but never used. Allowed unused args must match /^_/u

Comment on lines 125 to 126
dispatch(resetBridgeQuote())
dispatch(setIsLoading(false))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider improving state reset logic.

The current logic resets the bridge quote and loading state when conditions are not met. Ensure this behavior is intended and consider encapsulating this logic in a custom hook for better reusability and readability.

- dispatch(resetBridgeQuote())
- dispatch(setIsLoading(false))
+ useResetBridgeQuoteAndLoading()

Consider creating a custom hook useResetBridgeQuoteAndLoading to encapsulate this logic.

Committable suggestion was skipped due to low confidence.

Copy link

cloudflare-workers-and-pages bot commented Aug 13, 2024

Deploying sanguine-fe with  Cloudflare Pages  Cloudflare Pages

Latest commit: e90f6a1
Status: ✅  Deploy successful!
Preview URL: https://e79023b7.sanguine-fe.pages.dev
Branch Preview URL: https://fe-bridgequote-state.sanguine-fe.pages.dev

View logs

Copy link

cloudflare-workers-and-pages bot commented Aug 13, 2024

Deploying sanguine with  Cloudflare Pages  Cloudflare Pages

Latest commit: e90f6a1
Status: ✅  Deploy successful!
Preview URL: https://7ac5c72c.sanguine.pages.dev
Branch Preview URL: https://fe-bridgequote-state.sanguine.pages.dev

View logs

Copy link

codecov bot commented Aug 13, 2024

Bundle Report

Changes will increase total bundle size by 710.22kB ⬆️

Bundle name Size Change
sdk-router-@synapsecns/sdk-router-cjs 526.89kB 409.71kB ⬆️
synapse-interface-edge-server-array-push 83 bytes 0 bytes
synapse-interface-client-array-push 7.41MB 50.88kB ⬇️
sdk-router-@synapsecns/sdk-router-esm 254.83kB 0 bytes
synapse-interface-server-cjs 1.33MB 203.06kB ⬇️
widget-esm-cjs 278.2kB 278.2kB ⬆️
widget-cjs-esm 276.24kB 276.24kB ⬆️

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

This pull request refactors the bridge quote functionality in the Synapse interface, introducing a separate state management for bridge quotes and removing the teaser page components.

  • Introduced new bridgeQuote slice (packages/synapse-interface/slices/bridgeQuote/) for dedicated bridge quote state management
  • Implemented useBridgeQuoteState hook (packages/synapse-interface/slices/bridgeQuote/hooks.ts) for easier access to bridge quote state
  • Removed teaser page components (packages/synapse-interface/pages/teaser/*)
  • Updated BridgeQuote type with requestId field (packages/synapse-interface/utils/types/index.tsx)
  • Refactored useBridgeListener hook (packages/synapse-interface/utils/hooks/useBridgeListener.ts) to handle separate debounce effects for primary and alternative quotes

17 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings

Comment on lines 44 to 49
.addCase(fetchBridgeQuote.rejected, (state, action) => {
// state.error = action.payload
state.bridgeQuote = EMPTY_BRIDGE_QUOTE
// state.status = FetchState.INVALID
state.isLoading = false
})
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Error handling is incomplete. The error state is not being updated when the fetchBridgeQuote action is rejected

return rejectWithValue(msg)
}

const toValueBigInt = BigInt(maxAmountOut.toString()) ?? 0n
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: The nullish coalescing operator (??) is unnecessary here as BigInt() will always return a value

)) /
BigInt(originQuery.minAmountOut)

const isUnsupported = AcceptedChainId[fromChainId] ? false : true
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: This can be simplified to 'const isUnsupported = !AcceptedChainId[fromChainId]'

Copy link

codecov bot commented Aug 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 31.40645%. Comparing base (013d4b9) to head (e90f6a1).
Report is 3 commits behind head on master.

Additional details and impacted files
@@                 Coverage Diff                 @@
##              master       #3019         +/-   ##
===================================================
+ Coverage   30.31061%   31.40645%   +1.09583%     
===================================================
  Files            439         419         -20     
  Lines          28396       27125       -1271     
  Branches          82          82                 
===================================================
- Hits            8607        8519         -88     
+ Misses         18978       17798       -1180     
+ Partials         811         808          -3     
Flag Coverage Δ
packages 90.55118% <ø> (ø)
promexporter ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment on lines 7 to 12
export const useIsBridgeApproved = (
fromToken: Token | null,
fromChainId: number,
bridgeQuote: BridgeQuote | null,
debouncedFromValue: string
) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we have fromToken, fromChainId, bridgeQuote, debouncedFromValue injected into this hook via useBridgeState() hook?

That way we can re-use this hook without having to import useBridgeState() into components

@abtestingalpha abtestingalpha force-pushed the fe/bridgeQuote-state branch 2 times, most recently from 71170e5 to 9db21b1 Compare August 15, 2024 15:49
abtestingalpha and others added 4 commits August 16, 2024 15:10
* useBridgeSelections

* useBridgeValidations

* fix: stale output, infinite loader

* fix: track when input less than fees

* Remove finally

* apply validation + selection hook to input/output containes

* use bridge state for bridge write actions

* Fix chain comparison

* [wip] feat(synapse-interface): track quoted input (#3011)

* feat: compare input amount vs tracked quoted input

* Compare stringified bridge quote v bridge state for validation

* feat: track quoted tokens <> bridge quote (#3018)

* replace validation in callback with button check

* memoize bridge selection comparisons

* segment tracking state <> quote mismatch error

* simplify segment

* switch order

* Add back doesBridgeStateMatchQuote

* add back additional bridge quote fields

* remove unused bridge selection vars

* fix: conditions for active button for connect wallet

* fix: condition for showing amount must be greater than fee

* hasValidFromSelections
* Checks if user is typing

* Optional param

* remove local input state

* optional setIsTyping callback

* Adds swap behavior

---------

Co-authored-by: abtestingalpha <[email protected]>
@abtestingalpha abtestingalpha changed the title [wip] Fe/bridge quote state feat(synapse-interface): bridge quote state validations Aug 26, 2024
@abtestingalpha abtestingalpha marked this pull request as ready for review August 26, 2024 15:08
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Outside diff range, codebase verification and nitpick comments (2)
packages/synapse-interface/utils/hooks/useIsBridgeApproved.ts (2)

8-29: Consider destructuring fromToken properties for better readability.

Destructuring fromToken properties can improve readability and make the code more concise.

- if (fromToken && fromToken.addresses[fromChainId] === zeroAddress) {
+ const { addresses, decimals } = fromToken || {}
+ if (fromToken && addresses[fromChainId] === zeroAddress) {

8-29: Add a return type to the hook.

Adding a return type to the hook can improve type safety and clarity.

- export const useIsBridgeApproved = () => {
+ export const useIsBridgeApproved: () => boolean = () => {
Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between e577fe3 and 814bbe5.

Files selected for processing (21)
  • packages/synapse-interface/components/Maintenance/Maintenance.tsx (1 hunks)
  • packages/synapse-interface/components/StateManagedBridge/BridgeExchangeRateInfo.tsx (6 hunks)
  • packages/synapse-interface/components/StateManagedBridge/BridgeTransactionButton.tsx (4 hunks)
  • packages/synapse-interface/components/StateManagedBridge/FromChainSelector.tsx (1 hunks)
  • packages/synapse-interface/components/StateManagedBridge/FromTokenSelector.tsx (1 hunks)
  • packages/synapse-interface/components/StateManagedBridge/InputContainer.tsx (6 hunks)
  • packages/synapse-interface/components/StateManagedBridge/OutputContainer.tsx (2 hunks)
  • packages/synapse-interface/components/StateManagedBridge/hooks/useBridgeSelections.ts (1 hunks)
  • packages/synapse-interface/components/StateManagedBridge/hooks/useBridgeValidations.ts (1 hunks)
  • packages/synapse-interface/components/StateManagedSwap/SwapInputContainer.tsx (3 hunks)
  • packages/synapse-interface/components/StateManagedSwap/SwapTransactionButton.tsx (2 hunks)
  • packages/synapse-interface/components/ui/AmountInput.tsx (4 hunks)
  • packages/synapse-interface/constants/bridge.ts (2 hunks)
  • packages/synapse-interface/contexts/BackgroundListenerProvider.tsx (2 hunks)
  • packages/synapse-interface/pages/state-managed-bridge/index.tsx (12 hunks)
  • packages/synapse-interface/pages/swap/index.tsx (3 hunks)
  • packages/synapse-interface/slices/bridge/reducer.ts (7 hunks)
  • packages/synapse-interface/slices/bridgeQuote/reducer.ts (1 hunks)
  • packages/synapse-interface/slices/bridgeQuote/thunks.ts (1 hunks)
  • packages/synapse-interface/utils/hooks/useIsBridgeApproved.ts (1 hunks)
  • packages/synapse-interface/utils/types/index.tsx (2 hunks)
Additional context used
Biome
packages/synapse-interface/slices/bridgeQuote/thunks.ts

[error] 140-140: 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)

Additional comments not posted (56)
packages/synapse-interface/components/StateManagedBridge/FromTokenSelector.tsx (2)

1-6: LGTM!

The imports are correctly organized and necessary for the component's functionality.

The code changes are approved.


7-23: Ensure correctness of hooks and components.

The function is correctly implemented. However, verify that the hooks (useBridgeState, useWalletState, useFromTokenListArray) and components (TokenSelector) are correctly defined and used.

Run the following script to verify the hooks and components:

Verification successful

Hooks and Component Verified Successfully

The hooks (useBridgeState, useWalletState, useFromTokenListArray) and the component (TokenSelector) used in the FromTokenSelector component are correctly defined and utilized within the codebase. No issues were found in their implementation or usage.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the hooks and components used in FromTokenSelector.

# Test: Search for the definitions of the hooks and components. Expect: Correct definitions.
rg --type tsx -A 5 $'useBridgeState'
rg --type tsx -A 5 $'useWalletState'
rg --type tsx -A 5 $'useFromTokenListArray'
rg --type tsx -A 5 $'TokenSelector'

Length of output: 261


Script:

#!/bin/bash
# Description: Verify the hooks and components used in FromTokenSelector.

# Test: Search for the definitions of the hooks and components. Expect: Correct definitions.
rg -A 5 'useBridgeState'
rg -A 5 'useWalletState'
rg -A 5 'useFromTokenListArray'
rg -A 5 'TokenSelector'

Length of output: 84400

packages/synapse-interface/components/StateManagedBridge/FromChainSelector.tsx (2)

1-6: LGTM!

The imports are correctly organized and necessary for the component's functionality.

The code changes are approved.


8-24: Ensure correctness of hooks and components.

The function is correctly implemented. However, verify that the hooks (useBridgeState, useWalletState, useFromChainListArray) and components (ChainSelector) are correctly defined and used.

Run the following script to verify the hooks and components:

Verification successful

Hooks and Component Verified Successfully

The hooks (useBridgeState, useWalletState, useFromChainListArray) and the component (ChainSelector) used in the FromChainSelector component are correctly defined and used. Their definitions align with their intended functionality within the component. No issues were found.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the hooks and components used in FromChainSelector.

# Test: Search for the definitions of the hooks and components. Expect: Correct definitions.
rg --type tsx -A 5 $'useBridgeState'
rg --type tsx -A 5 $'useWalletState'
rg --type tsx -A 5 $'useFromChainListArray'
rg --type tsx -A 5 $'ChainSelector'

Length of output: 261


Script:

#!/bin/bash
# Description: Verify the hooks and components used in FromChainSelector.

# Test: Search for the definitions of the hooks and components without specifying a file type.
rg -A 5 'useBridgeState'
rg -A 5 'useWalletState'
rg -A 5 'useFromChainListArray'
rg -A 5 'ChainSelector'

Length of output: 81465

packages/synapse-interface/components/StateManagedBridge/hooks/useBridgeSelections.ts (2)

1-5: LGTM!

The imports are correctly organized and necessary for the hook's functionality.

The code changes are approved.


6-30: Ensure correctness of hooks and utility functions.

The function is correctly implemented. However, verify that the hooks (useBridgeState, usePortfolioBalances) and utility functions (stringToBigInt) are correctly defined and used.

Run the following script to verify the hooks and utility functions:

Verification successful

Hooks and Utility Function Verified Successfully

The hooks useBridgeState and usePortfolioBalances, along with the utility function stringToBigInt, are correctly defined and used in the codebase. They are integrated into the application logic as expected. No issues were found with their definitions or usage.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the hooks and utility functions used in useBridgeSelections.

# Test: Search for the definitions of the hooks and utility functions. Expect: Correct definitions.
rg --type tsx -A 5 $'useBridgeState'
rg --type tsx -A 5 $'usePortfolioBalances'
rg --type tsx -A 5 $'stringToBigInt'

Length of output: 197


Script:

#!/bin/bash
# Description: Verify the hooks and utility functions used in useBridgeSelections.

# Test: Search for the definitions of the hooks and utility functions. Expect: Correct definitions.
rg 'useBridgeState' --glob '*.{ts,tsx}' -A 5
rg 'usePortfolioBalances' --glob '*.{ts,tsx}' -A 5
rg 'stringToBigInt' --glob '*.{ts,tsx}' -A 5

Length of output: 91670

packages/synapse-interface/contexts/BackgroundListenerProvider.tsx (1)

Line range hint 1-21: Verify the impact of removing the useBridgeListener hook.

The removal of the useBridgeListener hook may impact how the application handles bridge-related data or events. Ensure that the functionality provided by useBridgeListener is no longer needed or has been moved elsewhere.

packages/synapse-interface/slices/bridgeQuote/reducer.ts (3)

1-45: Consider using a more specific error type.

The error property is typed as any. Consider using a more specific type to improve type safety and error handling.


40-44: Remove unused parameter action.

The action parameter in the fetchBridgeQuote.rejected case is defined but not used. Consider prefixing it with an underscore to indicate it's intentionally unused or removing it.

- .addCase(fetchBridgeQuote.rejected, (state, action) => {
+ .addCase(fetchBridgeQuote.rejected, (state) => {

40-44: Update the error state when the fetchBridgeQuote action is rejected.

The error state is not being updated when the fetchBridgeQuote action is rejected. Consider updating the error state to handle errors properly.

- .addCase(fetchBridgeQuote.rejected, (state) => {
+ .addCase(fetchBridgeQuote.rejected, (state, action: PayloadAction<Error>) => {
+   state.error = action.payload
packages/synapse-interface/constants/bridge.ts (4)

7-7: LGTM!

The addition of the inputAmountForQuote property enhances the structure of the quote object to accommodate additional data related to the input amount.

The code changes are approved.


8-8: LGTM!

The addition of the originTokenForQuote property enhances the structure of the quote object to accommodate additional data related to the origin token.

The code changes are approved.


9-9: LGTM!

The addition of the destTokenForQuote property enhances the structure of the quote object to accommodate additional data related to the destination token.

The code changes are approved.


25-25: LGTM!

The addition of the requestId property is likely intended to track individual quote requests, enhancing the functionality of the system that utilizes these constants.

The code changes are approved.

packages/synapse-interface/components/ui/AmountInput.tsx (6)

1-2: LGTM!

The imports of useState, useEffect, useCallback, and debounce are necessary for the new functionality related to user input handling and state management.

The code changes are approved.


12-12: LGTM!

The addition of the setIsTyping property to the AmountInputTypes interface allows parent components to be notified when the user is typing, enhancing the component's responsiveness to user input.

The code changes are approved.


21-21: LGTM!

The addition of the setIsTyping parameter to the AmountInput function allows the component to manage the typing state effectively.

The code changes are approved.


23-26: LGTM!

The creation of the debouncedSetIsTyping function using useCallback and debounce helps to reduce the frequency of state updates by delaying the call to setIsTyping for 600 milliseconds after the user stops typing.

The code changes are approved.


28-32: LGTM!

The addition of the handleInputChange function effectively manages input changes by setting isTyping to true immediately and then calling the debounced function to set it back to false after the delay. It also retains the original behavior of invoking handleFromValueChange when the input changes.

The code changes are approved.


54-54: LGTM!

The update of the onChange prop of the input field to use handleInputChange instead of handleFromValueChange ensures that the new typing logic is executed when the input changes.

The code changes are approved.

packages/synapse-interface/components/StateManagedBridge/OutputContainer.tsx (3)

2-2: LGTM!

The import of the useMemo hook from React is necessary for the new logic that optimizes performance by memoizing the output based on dependencies.

The code changes are approved.


16-16: LGTM!

The replacement of the useBridgeState hook with the useBridgeQuoteState hook reflects an enhancement in how bridge-related state is managed and accessed, potentially allowing for more precise data retrieval and state management.

The code changes are approved.


25-33: LGTM!

The addition of the showValue logic using the useMemo hook provides a clearer and more structured approach to rendering the value. It also optimizes performance by memoizing the output based on the dependencies.

The code changes are approved.

packages/synapse-interface/components/StateManagedSwap/SwapTransactionButton.tsx (2)

13-13: LGTM!

The addition of the isTyping prop is well-integrated and enhances the functionality of the SwapTransactionButton component.

The code changes are approved.


63-64: LGTM!

The usage of the isTyping prop in the isButtonDisabled conditional logic is appropriate and enhances the user experience.

The code changes are approved.

packages/synapse-interface/components/StateManagedBridge/hooks/useBridgeValidations.ts (1)

1-108: LGTM!

The useBridgeValidations hook is well-structured and effectively consolidates various validation checks, improving the readability and maintainability of the code.

The code changes are approved.

packages/synapse-interface/components/StateManagedBridge/BridgeTransactionButton.tsx (2)

20-20: LGTM!

The addition of the isTyping prop is well-integrated and enhances the functionality of the BridgeTransactionButton component.

The code changes are approved.


65-65: LGTM!

The usage of the isTyping prop in the isButtonDisabled conditional logic is appropriate and enhances the user experience.

The code changes are approved.

packages/synapse-interface/components/StateManagedBridge/BridgeExchangeRateInfo.tsx (5)

59-66: LGTM!

The changes improve performance by using debounced values and streamline state management by using useBridgeQuoteState.

The code changes are approved.


82-82: LGTM!

The changes streamline state management by using useBridgeQuoteState.

The code changes are approved.


92-93: LGTM!

The changes streamline state management by using useBridgeQuoteState.

The code changes are approved.


131-134: LGTM!

The changes streamline state management by using useBridgeQuoteState.

The code changes are approved.


172-174: LGTM!

The changes improve readability and maintainability by using a more generalized naming convention.

The code changes are approved.

packages/synapse-interface/components/StateManagedBridge/InputContainer.tsx (1)

Line range hint 1-119: LGTM!

The changes improve performance by reducing the frequency of state updates and dispatches to the Redux store. The error handling in handleFromValueChange ensures that only valid inputs are processed.

The code changes are approved.

packages/synapse-interface/components/Maintenance/Maintenance.tsx (1)

29-29: LGTM!

The change expands the scope of the interface for external use without affecting the existing properties or logic.

The code changes are approved.

packages/synapse-interface/components/StateManagedSwap/SwapInputContainer.tsx (5)

35-37: LGTM!

The InputContainerProps interface is correctly defined and improves type safety.

The code changes are approved.


39-41: LGTM!

The SwapInputContainer component is correctly updated to use the InputContainerProps interface and the setIsTyping prop.

The code changes are approved.


87-91: LGTM!

The debouncedUpdateSwapFromValue function is correctly implemented and improves performance by reducing the number of updates triggered during rapid input changes.

The code changes are approved.


93-97: LGTM!

The useEffect hook is correctly implemented to ensure proper cleanup of the debounced function, preventing potential memory leaks.

The code changes are approved.


105-105: LGTM!

The handleFromValueChange event handler is correctly updated to use the debounced function, and the setIsTyping prop is correctly passed to the AmountInput component.

The code changes are approved.

Also applies to: 156-156

packages/synapse-interface/slices/bridgeQuote/thunks.ts (2)

1-13: LGTM!

The import statements are correctly added and necessary for the implementation of the thunk.

The code changes are approved.


14-41: LGTM!

The fetchBridgeQuote thunk is correctly declared using createAsyncThunk.

The code changes are approved.

packages/synapse-interface/utils/types/index.tsx (1)

73-75: LGTM!

The new properties are correctly added to the BridgeQuote type, providing additional context related to the quote request.

The code changes are approved.

Also applies to: 91-91

packages/synapse-interface/pages/swap/index.tsx (3)

57-58: LGTM!

The addition of the isTyping state variable is correctly implemented.

The code changes are approved.


383-383: LGTM! But verify the SwapTransactionButton implementation.

The change allows SwapTransactionButton to adjust its behavior based on the user's typing status. Ensure that the SwapTransactionButton component handles this prop correctly.

The code changes are approved.

Run the following script to verify the SwapTransactionButton implementation:

Verification successful

The SwapTransactionButton component correctly handles the isTyping prop.

The isTyping prop is used in a conditional expression that likely controls the button's behavior, such as disabling it when the user is typing. This ensures the component responds appropriately to the user's input status.

  • SwapTransactionButton.tsx: Properly integrates isTyping into its logic.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the `SwapTransactionButton` implementation.

# Test: Search for the `SwapTransactionButton` component definition. Expect: Handle `isTyping` prop correctly.
rg --type tsx -A 10 $'SwapTransactionButton'

Length of output: 71


Script:

#!/bin/bash
# Description: Verify the `SwapTransactionButton` implementation by searching for its definition and usage.

# Search for the `SwapTransactionButton` component definition to verify handling of `isTyping` prop.
rg 'SwapTransactionButton' -A 10

Length of output: 3418


Script:

#!/bin/bash
# Description: Verify how the `isTyping` prop is used within the `SwapTransactionButton` component.

# Search for the usage of `isTyping` within the `SwapTransactionButton.tsx` file.
rg 'isTyping' packages/synapse-interface/components/StateManagedSwap/SwapTransactionButton.tsx -A 5 -B 5

Length of output: 727


359-359: LGTM! But verify the SwapInputContainer implementation.

The change allows SwapInputContainer to communicate user input activity back to the StateManagedSwap component. Ensure that the SwapInputContainer component handles this prop correctly.

The code changes are approved.

Run the following script to verify the SwapInputContainer implementation:

Verification successful

setIsTyping Prop is Implemented Correctly in SwapInputContainer

The setIsTyping prop is effectively used within the SwapInputContainer component to manage user input activity. It is passed to the AmountInput component, where it is utilized with a debounce function to efficiently track typing state. This implementation ensures that the component handles user input activity as intended.

  • SwapInputContainer correctly passes setIsTyping to AmountInput.
  • AmountInput uses a debounce function to manage typing state.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the `SwapInputContainer` implementation.

# Test: Search for the `SwapInputContainer` component definition. Expect: Handle `setIsTyping` prop correctly.
rg --type tsx -A 10 $'SwapInputContainer'

Length of output: 68


Script:

#!/bin/bash
# Description: Verify the `SwapInputContainer` implementation.

# Test: Search for the `SwapInputContainer` component definition in .tsx files.
fd --extension tsx --exec rg 'SwapInputContainer' -A 10

Length of output: 1640


Script:

#!/bin/bash
# Description: Verify the usage of `setIsTyping` within the `SwapInputContainer` component.

# Test: Search for the usage of `setIsTyping` within the `SwapInputContainer` component.
fd --extension tsx --exec rg 'setIsTyping' -A 5

Length of output: 2475

packages/synapse-interface/slices/bridge/reducer.ts (5)

4-4: LGTM!

The import statement for Token type is correctly added.

The code changes are approved.


Line range hint 89-90: LGTM! But verify the useBridgeQuoteState implementation.

The change simplifies state management by using a dedicated hook for bridgeQuote and isLoading. Ensure that the useBridgeQuoteState hook is correctly implemented.

The code changes are approved.

Run the following script to verify the useBridgeQuoteState implementation:

Verification successful

Verification of useBridgeQuoteState Implementation

The useBridgeQuoteState hook is correctly implemented in the codebase. It is defined in packages/synapse-interface/slices/bridgeQuote/hooks.ts and is used in multiple components, indicating its integration into the application logic.

  • Definition:

    • packages/synapse-interface/slices/bridgeQuote/hooks.ts: The hook is defined to return the bridgeQuote state using useAppSelector.
  • Usage:

    • It is used in several components and hooks, such as useIsBridgeApproved, StateManagedBridge, OutputContainer, BridgeExchangeRateInfo, and BridgeTransactionButton, confirming its role in managing the bridge quote state across the application.

The implementation appears to be consistent with its intended purpose.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the `useBridgeQuoteState` implementation.

# Test: Search for the `useBridgeQuoteState` hook definition. Expect: Correctly implemented.
rg --type tsx -A 10 $'useBridgeQuoteState'

Length of output: 69


Script:

#!/bin/bash
# Description: Verify the `useBridgeQuoteState` implementation without specifying file type.

# Test: Search for the `useBridgeQuoteState` hook definition. Expect: Correctly implemented.
rg -A 10 'useBridgeQuoteState'

Length of output: 20216


Line range hint 126-126: LGTM! But verify the resetBridgeQuote implementation.

The change ensures that the bridge quote is reset when conditions are not met. Ensure that the resetBridgeQuote action is correctly implemented.

The code changes are approved.

Run the following script to verify the resetBridgeQuote implementation:

Verification successful

The resetBridgeQuote implementation is correct.

The resetBridgeQuote action is properly defined in bridgeQuote/reducer.ts and is used throughout the codebase to reset the bridgeQuote state to its initial value. This confirms that the implementation is correct and functions as intended.

  • Definition: packages/synapse-interface/slices/bridgeQuote/reducer.ts
  • Usage: packages/synapse-interface/pages/state-managed-bridge/index.tsx
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the `resetBridgeQuote` implementation.

# Test: Search for the `resetBridgeQuote` action definition. Expect: Correctly implemented.
rg --type tsx -A 10 $'resetBridgeQuote'

Length of output: 66


Script:

#!/bin/bash
# Description: Verify the `resetBridgeQuote` implementation.

# Test: Search for the `resetBridgeQuote` action definition in TypeScript files.
rg --type-add 'typescript:*.ts' --type-add 'typescript:*.tsx' --type typescript -A 10 $'resetBridgeQuote'

Length of output: 6573


Line range hint 133-153: LGTM! But verify the fetchBridgeQuote implementation.

The change encapsulates the logic for fetching bridge quotes in a thunk, improving clarity and maintainability. Ensure that the fetchBridgeQuote thunk is correctly implemented.

The code changes are approved.

Run the following script to verify the fetchBridgeQuote implementation:

Verification successful

Verification Successful: fetchBridgeQuote Implementation is Correct

The fetchBridgeQuote thunk is correctly implemented using createAsyncThunk, and its usage across the codebase, including handling of pending, fulfilled, and rejected states, confirms its proper functionality.

  • Definition: Found in packages/synapse-interface/slices/bridgeQuote/thunks.ts.
  • Usage: Utilized in components like Widget.tsx and index.tsx.
  • State Management: Properly manages asynchronous states in the reducer.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the `fetchBridgeQuote` implementation.

# Test: Search for the `fetchBridgeQuote` thunk definition. Expect: Correctly implemented.
rg --type tsx -A 10 $'fetchBridgeQuote'

Length of output: 66


Script:

#!/bin/bash
# Description: Verify the `fetchBridgeQuote` implementation.

# Test: Search for the `fetchBridgeQuote` thunk definition. Expect: Correctly implemented.
rg --type ts -A 10 $'fetchBridgeQuote'

Length of output: 12643


Line range hint 91-92: LGTM! But verify the useIsBridgeApproved implementation.

The change simplifies state management by using a dedicated hook for isApproved. Ensure that the useIsBridgeApproved hook is correctly implemented.

The code changes are approved.

Run the following script to verify the useIsBridgeApproved implementation:

packages/synapse-interface/pages/state-managed-bridge/index.tsx (5)

27-27: LGTM!

The import statement for CHAINS_BY_ID is correctly added.

The code changes are approved.


78-79: LGTM!

The addition of the isTyping state variable is correctly implemented.

The code changes are approved.


65-65: LGTM! But verify the resetBridgeQuote implementation.

The change ensures that the bridge quote is reset when conditions are not met. Ensure that the resetBridgeQuote action is correctly implemented.

The code changes are approved.

Run the following script to verify the resetBridgeQuote implementation:

Verification successful

The resetBridgeQuote action is correctly implemented and used.

The resetBridgeQuote action resets the bridgeQuote state to its initial state, ensuring that the state is cleared when necessary. This implementation is consistent with the intended functionality.

  • File: packages/synapse-interface/slices/bridgeQuote/reducer.ts
  • Usage: packages/synapse-interface/pages/state-managed-bridge/index.tsx
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the `resetBridgeQuote` implementation.

# Test: Search for the `resetBridgeQuote` action definition. Expect: Correctly implemented.
rg --type tsx -A 10 $'resetBridgeQuote'

Length of output: 66


Script:

#!/bin/bash
# Description: Verify the `resetBridgeQuote` implementation.

# Test: Search for the `resetBridgeQuote` action definition. Expect: Correctly implemented.
rg --type ts -A 10 $'resetBridgeQuote'

Length of output: 6506


426-426: LGTM! But verify the InputContainer implementation.

The change allows InputContainer to communicate user input activity back to the StateManagedBridge component. Ensure that the InputContainer component handles this prop correctly.

The code changes are approved.

Run the following script to verify the InputContainer implementation:

Verification successful

Implementation of InputContainer is correct.

The InputContainer component is correctly set up to handle the setIsTyping prop, ensuring it communicates user input activity back to the parent component as intended.

  • Location: packages/synapse-interface/components/StateManagedBridge/InputContainer.tsx
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the `InputContainer` implementation.

# Test: Search for the `InputContainer` component definition. Expect: Handle `setIsTyping` prop correctly.
rg --type tsx -A 10 $'InputContainer'

Length of output: 64


Script:

#!/bin/bash
# Description: Verify the `InputContainer` implementation by searching for its definition in .tsx files.

# Test: Search for the `InputContainer` component definition. Expect: Handle `setIsTyping` prop correctly.
rg --glob '*.tsx' -A 10 'InputContainer'

Length of output: 9850


63-64: LGTM! But verify the useBridgeQuoteState implementation.

The change simplifies state management by using a dedicated hook for bridgeQuote and isLoading. Ensure that the useBridgeQuoteState hook is correctly implemented.

The code changes are approved.

Run the following script to verify the useBridgeQuoteState implementation:

Verification successful

The useBridgeQuoteState hook is correctly implemented and integrated.

The hook is defined in packages/synapse-interface/slices/bridgeQuote/hooks.ts and is widely used across the codebase, indicating its proper implementation and integration. No issues were found with its usage or definition.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the `useBridgeQuoteState` implementation.

# Test: Search for the `useBridgeQuoteState` hook definition. Expect: Correctly implemented.
rg --type tsx -A 10 $'useBridgeQuoteState'

Length of output: 69


Script:

#!/bin/bash
# Description: Verify the `useBridgeQuoteState` implementation using ast-grep.

# Test: Search for the `useBridgeQuoteState` hook definition.
ast-grep --lang typescript --pattern 'function useBridgeQuoteState($$$) { $$$ }'

Length of output: 82


Script:

#!/bin/bash
# Description: Search for the `useBridgeQuoteState` definition or import in the codebase.

# Test: Search for the `useBridgeQuoteState` term to find its definition or import.
rg 'useBridgeQuoteState'

Length of output: 2420

Comment on lines +42 to +51
const allQuotes = await synapseSDK.allBridgeQuotes(
fromChainId,
toChainId,
fromToken.addresses[fromChainId],
toToken.addresses[toChainId],
stringToBigInt(debouncedFromValue, fromToken?.decimals[fromChainId]),
{
originUserAddress: address,
}
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure proper error handling for synapseSDK.allBridgeQuotes.

The call to synapseSDK.allBridgeQuotes should be wrapped in a try-catch block to handle potential errors gracefully.

try {
  const allQuotes = await synapseSDK.allBridgeQuotes(
    fromChainId,
    toChainId,
    fromToken.addresses[fromChainId],
    toToken.addresses[toChainId],
    stringToBigInt(debouncedFromValue, fromToken?.decimals[fromChainId]),
    {
      originUserAddress: address,
    }
  )
} catch (error) {
  return rejectWithValue(`Failed to fetch bridge quotes: ${error.message}`)
}

Comment on lines +53 to +59
const pausedBridgeModules = new Set(
pausedModulesList
.filter((module) =>
module.chainId ? module.chainId === fromChainId : true
)
.flatMap(getBridgeModuleNames)
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optimize filtering of paused bridge modules.

Consider using Set operations to improve performance when checking for paused bridge modules.

const pausedBridgeModules = new Set(
  pausedModulesList
    .filter(module => !module.chainId || module.chainId === fromChainId)
    .flatMap(getBridgeModuleNames)
)

Comment on lines +121 to +124
if (!(originQuery && maxAmountOut && destQuery && feeAmount)) {
const msg = `No route found for bridging ${debouncedFromValue} ${fromToken?.symbol} on ${CHAINS_BY_ID[fromChainId]?.name} to ${toToken?.symbol} on ${CHAINS_BY_ID[toChainId]?.name}`
return rejectWithValue(msg)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate error message handling.

The error message for no route found is duplicated. Consider refactoring to avoid repetition.

const noRouteMessage = `No route found for bridging ${debouncedFromValue} ${fromToken?.symbol} on ${CHAINS_BY_ID[fromChainId]?.name} to ${toToken?.symbol} on ${CHAINS_BY_ID[toChainId]?.name}`
if (!(originQuery && maxAmountOut && destQuery && feeAmount)) {
  return rejectWithValue(noRouteMessage)
}

)) /
BigInt(originQuery.minAmountOut)

const isUnsupported = AcceptedChainId[fromChainId] ? false : true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplify boolean expression.

The boolean expression for isUnsupported can be simplified by directly using the negation.

const isUnsupported = !AcceptedChainId[fromChainId]
Tools
Biome

[error] 140-140: 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)

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 814bbe5 and a0ec95e.

Files selected for processing (2)
  • packages/synapse-interface/components/StateManagedBridge/BridgeTransactionButton.tsx (4 hunks)
  • packages/synapse-interface/components/StateManagedBridge/hooks/useBridgeValidations.ts (1 hunks)
Files skipped from review as they are similar to previous changes (2)
  • packages/synapse-interface/components/StateManagedBridge/BridgeTransactionButton.tsx
  • packages/synapse-interface/components/StateManagedBridge/hooks/useBridgeValidations.ts

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Outside diff range, codebase verification and nitpick comments (2)
packages/synapse-interface/pages/state-managed-bridge/index.tsx (2)

27-27: Consider removing unused imports.

The CHAINS_BY_ID import is not used in this file. Removing unused imports can help keep the code clean and maintainable.


79-79: Consider adding a comment to explain the purpose of isTyping state.

Adding a comment can help other developers understand why this state is needed.

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between a0ec95e and e90f6a1.

Files selected for processing (1)
  • packages/synapse-interface/pages/state-managed-bridge/index.tsx (11 hunks)
Additional comments not posted (9)
packages/synapse-interface/pages/state-managed-bridge/index.tsx (9)

64-67: Good use of custom hooks for state management.

The introduction of useBridgeQuoteState, resetBridgeQuote, and fetchBridgeQuote improves the modularity and readability of the code.


90-91: Good use of the new useBridgeQuoteState hook.

The integration of useBridgeQuoteState consolidates the handling of bridge quotes and loading states, improving the clarity and maintainability of the code.


92-92: Good use of the useIsBridgeApproved hook.

The use of useIsBridgeApproved simplifies the approval logic and enhances the component's responsiveness to state changes.


127-127: Ensure the reset logic is intended.

The current logic resets the bridge quote when conditions are not met. Ensure this behavior is intended and consider encapsulating this logic in a custom hook for better reusability and readability.


143-144: Good use of async actions and error handling.

The use of fetchBridgeQuote for fetching bridge quotes and handling errors with toast notifications improves the clarity and maintainability of the code.

Also applies to: 160-161, 166-167


192-192: Ensure the reset logic is intended.

The current logic resets the bridge quote when an error occurs. Ensure this behavior is intended and consider encapsulating this logic in a custom hook for better reusability and readability.


202-202: Good use of the useStaleQuoteUpdater hook.

The use of useStaleQuoteUpdater ensures that the bridge quote is updated when necessary, improving the reliability of the component.


336-339: Ensure the reset logic is intended.

The current logic resets the bridge quote and updates the debounced value after a successful transaction. Ensure this behavior is intended and consider encapsulating this logic in a custom hook for better reusability and readability.


Line range hint 428-444: Good use of component composition.

The use of components like InputContainer, SwitchButton, OutputContainer, Warning, BridgeExchangeRateInfo, ConfirmDestinationAddressWarning, and BridgeTransactionButton improves the modularity and readability of the code.

Comment on lines +96 to 97
const { showSettingsSlideOver } = useSelector(
(state: RootState) => state.bridgeDisplay
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using a custom hook for showSettingsSlideOver state.

Encapsulating this logic in a custom hook can improve reusability and readability.

// will have to handle deadlineMinutes here at later time, gets passed as optional last arg in .bridgeQuote()

/* clear stored bridge quote before requesting new bridge quote */
dispatch(setBridgeQuote(EMPTY_BRIDGE_QUOTE_ZERO))
dispatch(resetBridgeQuote())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider improving state reset logic.

The current logic resets the bridge quote before requesting a new bridge quote. Ensure this behavior is intended and consider encapsulating this logic in a custom hook for better reusability and readability.

@abtestingalpha abtestingalpha merged commit 649b3f5 into master Aug 26, 2024
38 checks passed
@abtestingalpha abtestingalpha deleted the fe/bridgeQuote-state branch August 26, 2024 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants