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

Fe/move pending bridge transactions #1546

Merged
merged 11 commits into from
Nov 8, 2023

Conversation

bigboydiamonds
Copy link
Collaborator

@bigboydiamonds bigboydiamonds commented Nov 8, 2023

Moving pendingBridgeTransactions state and related actions/reducers from bridge to transactions slice

Remove bridge slice from persistedReducers, only track application and transactions in local storage

Upgrade redux persist version to reset client side storage to prevent errors

Summary by CodeRabbit

  • Refactor
    • Reorganized bridge transactions functionality to improve code structure and efficiency.
  • New Features
    • Introduced new actions and interfaces for managing pending bridge transactions.
  • Bug Fixes
    • Updated the persistConfig version to reset cache and fix data structure errors.

7203a92a48330af13bc5c8a4114451029a8ec212: synapse-interface preview link
f560cb2788268c92168f9568f12ee3b9248ea22e: synapse-interface preview link
c667e2f5d51853ab1987efcf5d980b79255aec54: synapse-interface preview link

Copy link
Contributor

coderabbitai bot commented Nov 8, 2023

Walkthrough

The changes involve a significant refactoring of the codebase related to managing pending bridge transactions and the reorganization of code within the StateManagedBridge component. Import statements, state management, and Redux actions have been modified to streamline the handling of pending bridge transactions. Additionally, the Redux store configuration and reducer have been updated to accommodate these changes.

Changes

Files Change Summaries
Activity.tsx,
MostRecentTransaction.tsx,
PendingTransaction.tsx,
Portfolio.tsx
Import statements and state references related to pending bridge transactions have been modified.
index.tsx Removal of imports and functions related to fetching token balances, bridge transaction actions, and references to pending bridge transactions. Code reorganization within the StateManagedBridge component.
actions.ts,
reducer.ts,
updater.tsx
Introduction of new interfaces and actions related to pending bridge transactions, removal of unused imports, and adjustments to state management related to pending bridge transactions.
reducer.ts Update of persistConfig version and addition of bridge to the appReducer combineReducers.

Poem

🐇 Hopping through the code, with a refactor in sight,
Moving transactions from bridge to right.
🍂 As the leaves fall, we tidy our nest,
Ensuring our codebase is simply the best.
🌕 Under the November moon so bright,
We celebrate our code taking flight.
🎉 On this day in history, X-ray was found,
Like our bugs, now nowhere to be found.
🌍 From all cultures, east to west,
We code inclusively, doing our best.


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • If you reply to a review comment from CodeRabbit, the bot will automatically respond.
  • To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
  • Note: Review comments are made on code diffs or files, not on the PR overview.
  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

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 a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai help to get help.
  • @coderabbitai resolve to resolve all the CodeRabbit review comments.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

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.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between c1cd08d and d28b9f2.
Files selected for processing (11)
  • packages/synapse-interface/components/Portfolio/Activity.tsx (3 hunks)
  • packages/synapse-interface/components/Portfolio/Transaction/MostRecentTransaction.tsx (3 hunks)
  • packages/synapse-interface/components/Portfolio/Transaction/PendingTransaction.tsx (1 hunks)
  • packages/synapse-interface/pages/state-managed-bridge/index.tsx (2 hunks)
  • packages/synapse-interface/slices/bridge/actions.ts (1 hunks)
  • packages/synapse-interface/slices/bridge/reducer.ts (4 hunks)
  • packages/synapse-interface/slices/portfolio/updater.tsx (1 hunks)
  • packages/synapse-interface/slices/transactions/actions.ts (1 hunks)
  • packages/synapse-interface/slices/transactions/reducer.ts (6 hunks)
  • packages/synapse-interface/slices/transactions/updater.tsx (4 hunks)
  • packages/synapse-interface/store/reducer.ts (1 hunks)
Additional comments: 31
packages/synapse-interface/slices/bridge/actions.ts (1)
  • 1-12: The removal of PendingBridgeTransaction interface and related actions is consistent with the pull request summary. The addition of resetBridgeInputs and resetFetchedBridgeQuotes actions seems to be new functionality. Ensure that these actions are dispatched appropriately in the application.
packages/synapse-interface/slices/transactions/actions.ts (2)
  • 1-18: The PendingBridgeTransaction interface has been added. Ensure that all properties are correctly used and updated in the application.

  • 20-33: New actions related to PendingBridgeTransaction have been added. Make sure these actions are dispatched and handled correctly in the corresponding reducer.

packages/synapse-interface/slices/portfolio/updater.tsx (1)
  • 1-18: The changes in the imports and the use of useTransactionsState instead of useBridgeState are consistent with the pull request summary. Ensure that all instances of useBridgeState have been replaced with useTransactionsState throughout the codebase.
packages/synapse-interface/components/Portfolio/Transaction/MostRecentTransaction.tsx (3)
  • 1-11: The import statements have been updated to reflect the changes in the state management. The useBridgeState hook has been replaced with useTransactionsState and the pendingBridgeTransactions is now being fetched from TransactionsState instead of BridgeState.

  • 16-21: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [16-28]

The useTransactionsState hook is now being used to fetch the pendingBridgeTransactions state. This is a significant change as it moves the responsibility of managing the pendingBridgeTransactions state from the bridge slice to the transactions slice.

  • 27-27: Ensure that all references to pendingBridgeTransactions throughout the codebase have been updated to fetch from TransactionsState instead of BridgeState.
packages/synapse-interface/components/Portfolio/Transaction/PendingTransaction.tsx (1)
  • 5-11: The import statements have been updated to reflect the new location of the removePendingBridgeTransaction and updatePendingBridgeTransaction functions. Ensure that these functions are correctly implemented in the new location and that all references to these functions in the codebase have been updated.
packages/synapse-interface/components/Portfolio/Activity.tsx (3)
  • 9-15: The import statements have been updated to reflect the new location of PendingBridgeTransaction. This is a good change as it keeps the codebase consistent with the new structure.

  • 25-29: The useTransactionsState hook is now being used to fetch pendingBridgeTransactions. This is a good change as it aligns with the new structure of the Redux store.

  • 326-333: The PendingTransactionAwaitingIndexing component now retrieves pendingBridgeTransactions from useTransactionsState instead of useBridgeState. This is a good change as it aligns with the new structure of the Redux store.

packages/synapse-interface/slices/transactions/updater.tsx (5)
  • 1-3: Imports are well organized and only necessary packages are imported.

  • 15-37: The import statements are well organized and only necessary packages are imported.

  • 39-40: The queryHistoricalTime and queryPendingTime assignments are correct and follow best practices.

  • 50-54: The destructuring of variables from useTransactionsState() is correct and follows best practices.

  • 168-174: The matchingTransactionHashes filter logic is correct and follows best practices.

packages/synapse-interface/store/reducer.ts (3)
  • 20-23: The bridge slice is removed from persistedReducers. Ensure that this change is intentional and that the bridge slice does not need to be persisted in local storage.

  • 28-32: The persistConfig version is updated from 0 to 1. This will reset the client-side storage and prevent errors due to outdated data structures. Make sure that this is communicated to users, as it may result in loss of local data.

  • 34-38: The bridge slice is added to the appReducer. Ensure that all necessary actions and reducers related to the bridge slice are properly handled in the appReducer.

packages/synapse-interface/pages/state-managed-bridge/index.tsx (3)
  • 56-74: The imports related to fetching token balances and allowances, and bridge transaction actions have been removed. Ensure that these functionalities are handled elsewhere in the codebase.

  • 75-76: The StateManagedBridge component has been reorganized. Ensure that the new organization of the code does not affect the functionality of the component.

  • 97-103: The useBridgeState hook has been replaced with useTransactionsState to fetch pendingBridgeTransactions. Ensure that the useTransactionsState hook is defined and correctly fetches the pendingBridgeTransactions.

packages/synapse-interface/slices/bridge/reducer.ts (4)
  • 13-19: The removal of these imports indicates that the corresponding actions are no longer being used in this file. Ensure that these actions are not required elsewhere in the codebase or that they have been appropriately replaced.

  • 42-45: The BridgeState interface no longer includes pendingBridgeTransactions. Ensure that this change does not break any functionality that relies on this property.

  • 79-82: The initialState no longer includes pendingBridgeTransactions. This change should be safe as long as all references to pendingBridgeTransactions in the codebase have been updated accordingly.

  • 474-479: The resetBridgeInputs action no longer resets pendingBridgeTransactions. Ensure that this change does not break any functionality that relies on this action resetting pendingBridgeTransactions.

packages/synapse-interface/slices/transactions/reducer.ts (5)
  • 1-11: The import statements are well organized and clear. The new actions related to PendingBridgeTransaction are imported correctly.

  • 31-34: The TransactionsState interface is updated to include pendingBridgeTransactions which is an array of PendingBridgeTransaction. This is a good way to track the state of pending bridge transactions.

  • 43-49: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [37-47]

The initialState of the TransactionsState is correctly initialized with pendingBridgeTransactions as an empty array.

  • 52-105: The extraReducers section of the transactionsSlice is updated to handle the new actions related to PendingBridgeTransaction. The logic in these handlers seems correct and they should correctly update the state of pendingBridgeTransactions.

  • 226-232: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [226-243]

The resetTransactionsState action correctly resets pendingBridgeTransactions to its initial state. This is a good practice to ensure the state is correctly reset when needed.

Copy link

cloudflare-workers-and-pages bot commented Nov 8, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 923c829
Status: ✅  Deploy successful!
Preview URL: https://50253dda.sanguine.pages.dev
Branch Preview URL: https://fe-move-pending-bridge-trans.sanguine.pages.dev

View logs

Copy link

codecov bot commented Nov 8, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (32f7496) 51.23324% compared to head (923c829) 51.23324%.
Report is 3 commits behind head on master.

Additional details and impacted files
@@              Coverage Diff              @@
##              master       #1546   +/-   ##
=============================================
  Coverage   51.23324%   51.23324%           
=============================================
  Files            362         362           
  Lines          24691       24691           
  Branches         283         283           
=============================================
  Hits           12650       12650           
  Misses         10813       10813           
  Partials        1228        1228           
Flag Coverage Δ
packages 91.00610% <ø> (ø)

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.

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.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between d28b9f2 and 7105c3a.
Files selected for processing (1)
  • packages/synapse-interface/slices/transactions/actions.ts (1 hunks)
Additional comments: 5
packages/synapse-interface/slices/transactions/actions.ts (5)
  • 6-18: The PendingBridgeTransaction interface is well defined and covers all necessary fields for a pending bridge transaction. Ensure that the types used (Chain, Token) are correctly defined and imported.

  • 20-23: The addPendingBridgeTransaction action is correctly defined and uses the PendingBridgeTransaction interface as its payload.

  • 24-29: The updatePendingBridgeTransaction action is correctly defined. Ensure that all fields in the payload are correctly used in the reducer.

  • 30-32: The removePendingBridgeTransaction action is correctly defined and uses the transaction id as its payload.

  • 33-35: The updatePendingBridgeTransactions action is correctly defined and uses an array of PendingBridgeTransaction as its payload. Ensure that this action is correctly handled in the reducer.

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.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 7105c3a and 923c829.
Files selected for processing (1)
  • packages/synapse-interface/components/Portfolio/Portfolio.tsx (1 hunks)
Files skipped from review due to trivial changes (1)
  • packages/synapse-interface/components/Portfolio/Portfolio.tsx

@bigboydiamonds bigboydiamonds merged commit 925a11a into master Nov 8, 2023
43 checks passed
@bigboydiamonds bigboydiamonds deleted the fe/move-pending-bridge-transactions branch November 8, 2023 19:02
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