SDK: add RFQ #1695
SDK: add RFQ #1695
Conversation
WalkthroughThe updates introduce a new Changes
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 (
|
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #1695 +/- ##
===================================================
+ Coverage 51.86177% 52.05070% +0.18892%
===================================================
Files 360 366 +6
Lines 24654 24772 +118
Branches 285 295 +10
===================================================
+ Hits 12786 12894 +108
- Misses 10630 10636 +6
- Partials 1238 1242 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
ChiTimesChi
force-pushed
the
sdk/add-rfq
branch
from
fd82861 to
7740694
Compare
|
This PR was rebased to separate the changes to the bindings in #1706 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- packages/sdk-router/src/constants/medianTime.ts (1 hunks)
- packages/synapse-interface/components/Portfolio/Transaction/PendingTransaction.tsx (2 hunks)
Files skipped from review due to trivial changes (1)
- packages/sdk-router/src/constants/medianTime.ts
Files skipped from review as they are similar to previous changes (1)
- packages/synapse-interface/components/Portfolio/Transaction/PendingTransaction.tsx
This reverts commit 8fae090.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- packages/sdk-router/src/rfq/fastBridgeSet.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/sdk-router/src/rfq/fastBridgeSet.ts
…ency * Revised the logic for estimating transaction completion time in _Transaction.tsx. Now, the system starts checking for transaction completion a few minutes before the estimated completion time. * Reduced the interval for checking the current time in _Transactions.tsx from 30 seconds to 5 seconds.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (13)
- packages/synapse-interface/components/Portfolio/Activity.tsx (2 hunks)
- packages/synapse-interface/components/Portfolio/PortfolioTabManager.tsx (2 hunks)
- packages/synapse-interface/components/_Transaction/_Transaction.tsx (1 hunks)
- packages/synapse-interface/components/_Transaction/_Transactions.tsx (1 hunks)
- packages/synapse-interface/components/_Transaction/helpers/constants.ts (1 hunks)
- packages/synapse-interface/components/_Transaction/helpers/getExplorerAddressLink.ts (1 hunks)
- packages/synapse-interface/components/_Transaction/helpers/getTxBlockExplorerLink.ts (1 hunks)
- packages/synapse-interface/components/_Transaction/helpers/useBridgeTxStatus.tsx (1 hunks)
- packages/synapse-interface/pages/_app.tsx (2 hunks)
- packages/synapse-interface/slices/_transactions/hooks.ts (1 hunks)
- packages/synapse-interface/slices/_transactions/reducer.ts (1 hunks)
- packages/synapse-interface/slices/_transactions/updater.tsx (1 hunks)
- packages/synapse-interface/store/reducer.ts (2 hunks)
Additional comments: 12
packages/synapse-interface/slices/_transactions/hooks.ts (1)
- 4-6: The
use_TransactionsStatehook is a simple selector hook for the_transactionsslice of the Redux state. It looks correct and follows best practices for Redux hooks.packages/synapse-interface/components/_Transaction/helpers/getExplorerAddressLink.ts (1)
- 3-14: The
getExplorerAddressLinkfunction correctly constructs a URL to a block explorer for a given chain ID and address. It also handles the case where the chain ID or address is missing by logging an error and returningnullvalues. This is a good practice for error handling.packages/synapse-interface/components/_Transaction/helpers/getTxBlockExplorerLink.ts (1)
- 3-14: The
getTxBlockExplorerLinkfunction is similar togetExplorerAddressLinkbut for transaction hashes. It also correctly handles missing chain IDs or transaction hashes by logging an error and returningnullvalues.packages/synapse-interface/components/_Transaction/helpers/constants.ts (2)
1-21: The
ExplorerLinksconstant provides a mapping of chain IDs to their respective block explorer URLs. It's important to ensure that these URLs are kept up-to-date and that the list includes all supported chains.23-43: The
ExplorerNamesconstant provides a mapping of chain IDs to the human-readable names of their respective block explorers. This is useful for UI purposes where the name of the block explorer is displayed to the user.packages/synapse-interface/store/reducer.ts (1)
- 24-24: The
_transactionsreducer has been added to thepersistedReducersobject, which means that the_transactionsstate will be persisted across sessions. This is a significant change and should be verified to ensure that only the necessary parts of the_transactionsstate are persisted and that sensitive information is not stored in local storage.Verification successful
The review of the
_transactionsreducer file reveals the following:
- The
_TransactionDetailsinterface includes fields such asoriginChain,originToken,destinationChain,destinationToken,originValue,originTxHash,bridgeModuleName,estimatedTime,timestamp,kappa, andisComplete.- The
_TransactionsStateinterface is a mapping from transaction hashes to_TransactionDetails.- The
initialStatefor the reducer is an empty object.- The reducer includes actions to add, remove, update, and complete transactions, as well as to clear all transactions.
The data stored in the
_transactionsreducer appears to be related to transaction details and does not include any sensitive information such as private keys or personal user data. The persistence of this data across sessions would be typical for a transaction history feature in a financial application, allowing users to see past transactions after restarting the application.Based on the contents of the reducer file, the PR comment regarding the persistence of the
_transactionsstate seems to be addressed appropriately, as the persisted data is relevant to the application's functionality and does not include sensitive information.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that only necessary parts of the _transactions state are persisted. rg --type javascript "persistedReducers"Length of output: 75
Script:
#!/bin/bash # Verify that only necessary parts of the _transactions state are persisted. rg "persistedReducers" --type jsLength of output: 32
Script:
#!/bin/bash # Search for "persistedReducers" in all files to verify the _transactions state persistence. rg "persistedReducers"Length of output: 251
Script:
#!/bin/bash # Output the lines around the definition of `persistedReducers` to understand its structure and contents. rg "const persistedReducers =" -A 10 -B 2 packages/synapse-interface/store/reducer.tsLength of output: 438
Script:
#!/bin/bash # Search for the definition of the `_transactions` reducer to understand what data it contains. rg "_transactions" -A 10 -B 2 packages/synapse-interface/store/reducer.tsLength of output: 1016
Script:
#!/bin/bash # Find the `_transactions` reducer file and output its contents. fd "_transactions" --exec rg --type jsLength of output: 38
Script:
#!/bin/bash # Find the file path of the `_transactions` reducer. fd "_transactions\.js$"Length of output: 25
Script:
#!/bin/bash # Find any file that contains `_transactions` in its name. fd "_transactions"Length of output: 136
Script:
#!/bin/bash # List the contents of the _transactions directory to find the reducer file. ls packages/synapse-interface/slices/_transactions/Length of output: 85
Script:
#!/bin/bash # Display the contents of the _transactions reducer file. cat packages/synapse-interface/slices/_transactions/reducer.tsLength of output: 2413
packages/synapse-interface/components/_Transaction/_Transactions.tsx (1)
- 9-60: The
_Transactionscomponent is responsible for rendering a list of transaction components. It uses several hooks to get the necessary data and correctly updates the current time every 5 seconds to trigger re-checks of transaction statuses. The component also handles the case where there are no transactions to display. Ensure that the interval is cleared correctly on component unmount to prevent memory leaks.packages/synapse-interface/components/Portfolio/PortfolioTabManager.tsx (1)
- 1-15: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [2-40]
The
PortfolioTabManagercomponent now includes the_Transactionscomponent, passing theconnectedAddressprop obtained from theuseAccounthook. This change integrates the new_Transactionscomponent into the portfolio tab manager, allowing users to view their transactions within the portfolio UI.packages/synapse-interface/slices/_transactions/reducer.ts (1)
- 26-102: The
transactionsSlicereducer has been defined with actions to add, remove, update, and complete transactions. The reducer's actions are well-defined and handle the necessary updates to the state. Ensure that the reducer's actions are covered by unit tests to verify their behavior.packages/synapse-interface/pages/_app.tsx (1)
- 56-56: The
_TransactionsUpdatercomponent has been added to theUpdatersfunction, which is responsible for running various updaters that sync the application state with external data. This change ensures that the_transactionsstate is kept up-to-date.packages/synapse-interface/components/_Transaction/_Transaction.tsx (1)
- 58-208: The
_Transactioncomponent is responsible for rendering individual transaction details and status. It uses several hooks to get the necessary data and correctly updates the transaction status. The component also provides a dropdown menu with options related to the transaction, such as viewing on a block explorer or contacting support. Ensure that thehandleClearTransactionfunction is tested to confirm that it correctly removes transactions from the state.packages/synapse-interface/components/Portfolio/Activity.tsx (1)
- 232-238: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [235-279]
The conditional rendering of the "Pending" section within the
Activitycomponent has been commented out. This change affects the logic for displaying pending transactions. If this is intentional and the pending transactions are now handled elsewhere, this change is acceptable. However, if this is a temporary change for testing or development purposes, it should be noted, and the original code should be restored before merging.
| export default function Updater() { | ||
| const dispatch = useAppDispatch() | ||
| const { pendingBridgeTransactions } = useTransactionsState() | ||
| const transactions = use_TransactionsState() | ||
|
|
||
| /** Add transaction if not in _transactions store */ | ||
| useEffect(() => { | ||
| if (checkTransactionsExist(pendingBridgeTransactions)) { | ||
| pendingBridgeTransactions.forEach((tx: PendingBridgeTransaction) => { | ||
| if (!transactions[tx.transactionHash]) { | ||
| dispatch( | ||
| addTransaction({ | ||
| originTxHash: tx.transactionHash, | ||
| originValue: tx.originValue, | ||
| originChain: tx.originChain, | ||
| originToken: tx.originToken, | ||
| destinationChain: tx.destinationChain, | ||
| destinationToken: tx.destinationToken, | ||
| bridgeModuleName: tx.bridgeModuleName, | ||
| estimatedTime: tx.estimatedTime, | ||
| timestamp: tx.id, | ||
| }) | ||
| ) | ||
| } | ||
| }) | ||
| } | ||
| }, [pendingBridgeTransactions]) | ||
|
|
||
| return null |
There was a problem hiding this comment.
The checkTransactionsExist function checks if the transactions parameter is not null or undefined, is an array, and has a length greater than zero. It does not, however, check for uniqueness of transactions within the array. This could potentially lead to duplicates if the same transaction is added multiple times to the array that is passed to this function.
Analysis chain
The Updater component is using a React effect to add transactions to the _transactions slice if they are not already present. It correctly checks for the existence of transactions before dispatching the addTransaction action. However, it's important to ensure that the checkTransactionsExist utility function is correctly identifying unique transactions to prevent duplicates.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify that checkTransactionsExist correctly identifies unique transactions.
ast-grep --lang javascript --pattern $'function checkTransactionsExist($_) {
$$$
}'
Length of output: 86
Script:
#!/bin/bash
# Search for any function or method that includes 'checkTransactionsExist' to verify its implementation.
rg "checkTransactionsExist"
Length of output: 4478
Script:
#!/bin/bash
# Display the contents of the checkTransactionsExist.ts file to verify the implementation of the function.
cat packages/synapse-interface/utils/checkTransactionsExist.ts
Length of output: 280
| export const useBridgeTxStatus = ({ | ||
| originChainId, | ||
| destinationChainId, | ||
| originTxHash, | ||
| bridgeModuleName, | ||
| kappa, | ||
| checkStatus = false, | ||
| currentTime, | ||
| }: UseBridgeTxStatusProps) => { | ||
| const { synapseSDK } = useSynapseContext() | ||
| const [isComplete, setIsComplete] = useState<boolean>(false) | ||
| const [fetchedKappa, setFetchedKappa] = useState<string>(null) | ||
|
|
||
| const getKappa = async (): Promise<string> => { | ||
| if (!synapseSDK) return null | ||
| if (!bridgeModuleName || !originChainId || !originTxHash) return null | ||
| return await synapseSDK.getSynapseTxId( | ||
| originChainId, | ||
| bridgeModuleName, | ||
| originTxHash | ||
| ) | ||
| } | ||
|
|
||
| const getBridgeTxStatus = async ( | ||
| destinationChainId: number, | ||
| bridgeModuleName: string, | ||
| kappa: string | ||
| ) => { | ||
| if (!synapseSDK) return null | ||
| if (!destinationChainId || !bridgeModuleName || !kappa) return null | ||
| return await synapseSDK.getBridgeTxStatus( | ||
| destinationChainId, | ||
| bridgeModuleName, | ||
| kappa | ||
| ) | ||
| } | ||
|
|
||
| useEffect(() => { | ||
| if (!checkStatus) return | ||
| if (isComplete) return | ||
| ;(async () => { | ||
| let _kappa | ||
|
|
||
| if (!kappa) { | ||
| console.log('fetching kappa') | ||
| _kappa = await getKappa() | ||
| setFetchedKappa(_kappa) | ||
| } else { | ||
| _kappa = kappa | ||
| } | ||
|
|
||
| console.log('fetching tx status') | ||
| const txStatus = await getBridgeTxStatus( | ||
| destinationChainId, | ||
| bridgeModuleName, | ||
| _kappa | ||
| ) | ||
|
|
||
| if (txStatus !== null) { | ||
| setIsComplete(txStatus) | ||
| } | ||
| })() | ||
| }, [currentTime, checkStatus]) | ||
|
|
||
| return [isComplete, fetchedKappa] |
There was a problem hiding this comment.
The useBridgeTxStatus hook is used to fetch the status of a bridge transaction and the associated kappa value. It correctly handles the case where the transaction is already complete or if the necessary parameters are not provided. However, the hook relies on the currentTime prop to trigger updates, which could lead to unnecessary re-renders or fetches. Consider optimizing this by using more specific dependencies for the useEffect hook.
- }, [currentTime, checkStatus])
+ }, [checkStatus, isTxComplete, kappa])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.
| export const useBridgeTxStatus = ({ | |
| originChainId, | |
| destinationChainId, | |
| originTxHash, | |
| bridgeModuleName, | |
| kappa, | |
| checkStatus = false, | |
| currentTime, | |
| }: UseBridgeTxStatusProps) => { | |
| const { synapseSDK } = useSynapseContext() | |
| const [isComplete, setIsComplete] = useState<boolean>(false) | |
| const [fetchedKappa, setFetchedKappa] = useState<string>(null) | |
| const getKappa = async (): Promise<string> => { | |
| if (!synapseSDK) return null | |
| if (!bridgeModuleName || !originChainId || !originTxHash) return null | |
| return await synapseSDK.getSynapseTxId( | |
| originChainId, | |
| bridgeModuleName, | |
| originTxHash | |
| ) | |
| } | |
| const getBridgeTxStatus = async ( | |
| destinationChainId: number, | |
| bridgeModuleName: string, | |
| kappa: string | |
| ) => { | |
| if (!synapseSDK) return null | |
| if (!destinationChainId || !bridgeModuleName || !kappa) return null | |
| return await synapseSDK.getBridgeTxStatus( | |
| destinationChainId, | |
| bridgeModuleName, | |
| kappa | |
| ) | |
| } | |
| useEffect(() => { | |
| if (!checkStatus) return | |
| if (isComplete) return | |
| ;(async () => { | |
| let _kappa | |
| if (!kappa) { | |
| console.log('fetching kappa') | |
| _kappa = await getKappa() | |
| setFetchedKappa(_kappa) | |
| } else { | |
| _kappa = kappa | |
| } | |
| console.log('fetching tx status') | |
| const txStatus = await getBridgeTxStatus( | |
| destinationChainId, | |
| bridgeModuleName, | |
| _kappa | |
| ) | |
| if (txStatus !== null) { | |
| setIsComplete(txStatus) | |
| } | |
| })() | |
| }, [currentTime, checkStatus]) | |
| return [isComplete, fetchedKappa] | |
| useEffect(() => { | |
| if (!checkStatus) return | |
| if (isComplete) return | |
| ;(async () => { | |
| let _kappa | |
| if (!kappa) { | |
| console.log('fetching kappa') | |
| _kappa = await getKappa() | |
| setFetchedKappa(_kappa) | |
| } else { | |
| _kappa = kappa | |
| } | |
| console.log('fetching tx status') | |
| const txStatus = await getBridgeTxStatus( | |
| destinationChainId, | |
| bridgeModuleName, | |
| _kappa | |
| ) | |
| if (txStatus !== null) { | |
| setIsComplete(txStatus) | |
| } | |
| })() | |
| }, [checkStatus, isTxComplete, kappa]) |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- packages/synapse-interface/components/_Transaction/helpers/useBridgeTxStatus.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/synapse-interface/components/_Transaction/helpers/useBridgeTxStatus.tsx
* Sorting wip * Show by latest * Merging sorting-j * Hides debugging related kappa and time * debug why new type isnt recognized * Convert transactions object to list * Render transactions in list * Check tx is stored before adding into _transaction * Check tx has been confirmed before adding to _transaction store * Sorting by descending timestamp * Allow clear tx functionality to persist * back to optimism mainnet rpc * Prevent refiring tx sdk queries if already complete * Mark tx as complete once kappa and isTxComplete avail * Check if store marked Tx complete before firing tx status hook, render persisted tx status * `useBridgeTxStatus` hook initialized to set isComplete to true if kappa exists, save call * Revert prev commit, require check on tx status beyond kappa avail * Utilize `checkStatus` as guardrail check in `useBridgeTxStatus` * removes unused prop * removes another unused prop * Hide debugging meta * Hide debugging meta * limits to 5 txns * align time --------- Co-authored-by: bigboydiamonds <57741810+bigboydiamonds@users.noreply.github.com>
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (5)
- packages/synapse-interface/components/_Transaction/_Transaction.tsx (1 hunks)
- packages/synapse-interface/components/_Transaction/_Transactions.tsx (1 hunks)
- packages/synapse-interface/components/_Transaction/helpers/useBridgeTxStatus.tsx (1 hunks)
- packages/synapse-interface/slices/_transactions/reducer.ts (1 hunks)
- packages/synapse-interface/slices/_transactions/updater.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (5)
- packages/synapse-interface/components/_Transaction/_Transaction.tsx
- packages/synapse-interface/components/_Transaction/_Transactions.tsx
- packages/synapse-interface/components/_Transaction/helpers/useBridgeTxStatus.tsx
- packages/synapse-interface/slices/_transactions/reducer.ts
- packages/synapse-interface/slices/_transactions/updater.tsx
This reverts commit 32dcaf6.
ChiTimesChi
force-pushed
the
sdk/add-rfq
branch
from
95d88e5 to
139c512
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- yarn.lock
Files selected for processing (1)
- packages/synapse-interface/slices/_transactions/reducer.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/synapse-interface/slices/_transactions/reducer.ts
Description
Adds support for basic RFQ implementation, without additional swaps on origin chain.
TODO items
FastBridgecontractbridgeQuote()Optional gas rebateFastBridge:Get the list of supported token pairs between two chainsGet the quote for a specific token pair and amountSummary by CodeRabbit
New Features
Enhancements
fastBridgeSetfunctionality.Bug Fixes
Documentation
UI/UX Improvements
Testing
Refactor
dcc652b01dfd213c6070a5dadf96f5e6829b8c1f: synapse-interface preview link
888f6b9418e954f1b056193ce414b0fdd4ea2cd8: synapse-interface preview link
5e2bfc1584a29f241da4d57904e97cbed0298e1e: synapse-interface preview link
01a3cd8ec7e01f94157a22f2c3d0693202a78edb: synapse-interface preview link
09dd857c720aa2fecf36ae7292cb3b2d6f229f86: synapse-interface preview link
493dcae4e1cee85aa3008ff08690f1496e0332da: synapse-interface preview link
eefbf499b0c420c2e0cc0a78da91047347d19852: synapse-interface preview link
38207219484062b3e06c14841189df981f7d2182: synapse-interface preview link
1ad091b8bb19cf47485b1c7079361a3b978f140e: synapse-interface preview link
6137733e2b3b3a650d4c5c9e81660a27a4b94874: synapse-interface preview link
0546a9674d28a83c82046fb00bf1127bf4559f6e: synapse-interface preview link
98ebd1ab9d032684b5aa2a2fc533a2310afdaed8: synapse-interface preview link
994a2d028f5ed98a2a5a8e61297c26c6924cb172: synapse-interface preview link
6f496eaaede3c5ab2db76f71ad8f885f68dec652: synapse-interface preview link
041ef39d54491e2129f2aaba8d4d5e520d5ba911: synapse-interface preview link
a5fc1382b5832e3d2d38e9329c69659cee1fe83c: synapse-interface preview link
f2a68072e4a071d958f197ed6187e93aee39a58a: synapse-interface preview link
82b3f8af84631ace0ccab6a777aa4a8f549b2def: synapse-interface preview link
99b918ee0f071cd48d01245be3df187e3573b411: synapse-interface preview link
8eb4a6e4a23d838837eb6b49bc92f04cd4fb736a: synapse-interface preview link
d7fcf1409f3fa0ea98e4d7952bb88d4ce9206034: synapse-interface preview link
eb34f5531535dbb80520073946dfd659e3f16596: synapse-interface preview link
c6b49c6e2ac5ad55f2728dec12b5e661008f9c46: synapse-interface preview link