-
Notifications
You must be signed in to change notification settings - Fork 32
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 uuid #2900
Conversation
Deploying sanguine with Cloudflare Pages
|
WalkthroughThe changes involve enhancements to various components, primarily focusing on time management and bridge transaction handling. Key updates include the introduction of new utility functions for time calculations, modifications to existing components to utilize these functions, and the addition of identifiers to bridge quotes. These adjustments aim to improve the interface and data handling within the application, ensuring effective tracking and user feedback during transactions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BridgeTransactionButton
participant StateManagedBridge
participant fetchBridgeQuote
User->>BridgeTransactionButton: Clicks button
BridgeTransactionButton->>StateManagedBridge: Trigger transaction
StateManagedBridge->>fetchBridgeQuote: Fetch bridge quote with id
fetchBridgeQuote-->>StateManagedBridge: Return quote
StateManagedBridge-->>BridgeTransactionButton: Update button state
BridgeTransactionButton-->>User: Show updated state
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
Deploying sanguine-fe with Cloudflare Pages
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
packages/synapse-interface/utils/hooks/useStaleQuoteUpdater.ts (1)
18-18
: Comment clarity improvement.The comment clarifying the default value of
staleTimeout
is helpful. Ensure that this comment is consistent across the codebase.Ensure that similar comments are added wherever
staleTimeout
is used.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (14)
- packages/synapse-interface/components/StateManagedBridge/BridgeTransactionButton.tsx (1 hunks)
- packages/synapse-interface/components/_Transaction/components/AnimatedProgressBar.tsx (2 hunks)
- packages/synapse-interface/constants/bridge.ts (1 hunks)
- packages/synapse-interface/package.json (1 hunks)
- packages/synapse-interface/pages/state-managed-bridge/index.tsx (12 hunks)
- packages/synapse-interface/slices/bridgeQuote/thunks.ts (2 hunks)
- packages/synapse-interface/utils/convertUuidToUnix.ts (1 hunks)
- packages/synapse-interface/utils/hooks/useApplicationListener.ts (3 hunks)
- packages/synapse-interface/utils/hooks/useIntervalTimer.ts (2 hunks)
- packages/synapse-interface/utils/hooks/useStaleQuoteUpdater.ts (2 hunks)
- packages/synapse-interface/utils/hooks/useTransactionListener.ts (1 hunks)
- packages/synapse-interface/utils/time.ts (1 hunks)
- packages/synapse-interface/utils/txErrorHandler.tsx (1 hunks)
- packages/synapse-interface/utils/types/index.tsx (1 hunks)
Files skipped from review due to trivial changes (2)
- packages/synapse-interface/utils/time.ts
- packages/synapse-interface/utils/txErrorHandler.tsx
Additional comments not posted (21)
packages/synapse-interface/utils/convertUuidToUnix.ts (1)
1-1
: Verify the import.Ensure that
UUID
is correctly imported fromuuidv7
. If not, update the import statement accordingly.packages/synapse-interface/utils/hooks/useIntervalTimer.ts (2)
3-3
: Verify the import and implementation ofgetUnixTimeMinutesFromNow
.Ensure that
getUnixTimeMinutesFromNow
is correctly implemented and imported from@/utils/time
.
17-17
: Ensure the new function behaves as expected.Verify that
getUnixTimeMinutesFromNow
behaves as expected and does not introduce any bugs in the timer functionality.Also applies to: 24-24
packages/synapse-interface/constants/bridge.ts (1)
7-7
: Verify the usage of theid
property.Ensure that the
id
property is used consistently throughout the codebase and does not introduce any bugs.packages/synapse-interface/utils/hooks/useStaleQuoteUpdater.ts (3)
22-22
: LGTM!The logic to derive
quoteTime
fromquote.id
usingconvertUuidToUnix
is correctly implemented.The code changes are approved.
24-24
: LGTM!The
isValidQuote
logic is correctly updated to use the newquoteTime
.The code changes are approved.
7-7
: Ensure correctness of the new utility function.The
convertUuidToUnix
function is newly introduced. Ensure that this function correctly converts the UUID to a Unix timestamp.Run the following script to verify the implementation of
convertUuidToUnix
:Verification successful
The
convertUuidToUnix
function is correctly implemented.The function accurately extracts a timestamp from a UUID by parsing its bytes and converting them to a Unix timestamp. This logic is consistent with typical practices for handling UUIDs.
- Location:
packages/synapse-interface/utils/convertUuidToUnix.ts
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of `convertUuidToUnix`. # Test: Search for the function implementation. Expect: Correct conversion logic. rg --type python -A 5 $'def convertUuidToUnix'Length of output: 78
Script:
#!/bin/bash # Description: Verify the implementation of `convertUuidToUnix`. # Test: Search for the function implementation across the codebase. rg 'function convertUuidToUnix' -A 5Length of output: 38
Script:
#!/bin/bash # Description: Search for any occurrence of `convertUuidToUnix` in the codebase. # Search for `convertUuidToUnix` to find its usage or definition. rg 'convertUuidToUnix' # Search for the import statement to trace its origin. rg 'import .*convertUuidToUnix'Length of output: 927
Script:
#!/bin/bash # Description: Extract the implementation of `convertUuidToUnix` to verify its logic. # Search for the implementation in `convertUuidToUnix.ts`. rg 'export const convertUuidToUnix' -A 10Length of output: 947
packages/synapse-interface/utils/hooks/useApplicationListener.ts (3)
30-30
: LGTM!The logic to derive
sevenDaysAgo
usinggetUnixTimeMinutesBeforeNow
is correctly implemented.The code changes are approved.
45-47
: LGTM!The logic to update
lastConnectedTime
usinggetUnixTimeMinutesBeforeNow
is correctly implemented.The code changes are approved.
13-13
: Ensure correctness of the new utility function.The
getUnixTimeMinutesBeforeNow
function is newly introduced. Ensure that this function correctly calculates the Unix time.Run the following script to verify the implementation of
getUnixTimeMinutesBeforeNow
:Verification successful
The
getUnixTimeMinutesBeforeNow
function is correctly implemented.The function accurately calculates the Unix time by subtracting the specified minutes from the current time in seconds. This implementation is consistent with the expected behavior. No issues were found with this change.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of `getUnixTimeMinutesBeforeNow`. # Test: Search for the function implementation. Expect: Correct calculation logic. rg --type python -A 5 $'def getUnixTimeMinutesBeforeNow'Length of output: 88
Script:
#!/bin/bash # Description: Verify the implementation of `getUnixTimeMinutesBeforeNow`. # Test: Search for the function implementation in JavaScript or TypeScript. Expect: Correct calculation logic. ast-grep --lang javascript --pattern 'function getUnixTimeMinutesBeforeNow($$$) { $$$ }'Length of output: 90
Script:
#!/bin/bash # Description: Search for any occurrence of `getUnixTimeMinutesBeforeNow` in JavaScript and TypeScript files. rg 'getUnixTimeMinutesBeforeNow' --type js --type ts -A 5Length of output: 4978
packages/synapse-interface/utils/hooks/useTransactionListener.ts (2)
20-20
: LGTM!The logic to derive
queryHistoricalTime
usinggetUnixTimeMinutesBeforeNow
is correctly implemented.The code changes are approved.
18-18
: Ensure correctness of the new utility function.The
getUnixTimeMinutesBeforeNow
function is newly introduced. Ensure that this function correctly calculates the Unix time.Run the following script to verify the implementation of
getUnixTimeMinutesBeforeNow
:packages/synapse-interface/package.json (1)
74-74
: LGTM!The addition of the
uuidv7
dependency is appropriate and enhances the project's capabilities.The code changes are approved.
packages/synapse-interface/components/_Transaction/components/AnimatedProgressBar.tsx (1)
2-2
: LGTM! But verify the implementation ofgetUnixTimeMinutesBeforeNow
.The replacement of
getTimeMinutesBeforeNow
withgetUnixTimeMinutesBeforeNow
is appropriate. Ensure that the new function is correctly implemented and imported.The code changes are approved.
Run the following script to verify the implementation of
getUnixTimeMinutesBeforeNow
:Also applies to: 22-22
Verification successful
Function
getUnixTimeMinutesBeforeNow
is correctly implemented and used.The function
getUnixTimeMinutesBeforeNow
is defined inpackages/synapse-interface/utils/time.ts
and is correctly imported and used across the codebase, including the file under review. The implementation appears to be consistent with its intended purpose.
- Location of definition:
packages/synapse-interface/utils/time.ts
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of `getUnixTimeMinutesBeforeNow`. # Test: Search for the function implementation. Expect: The function should be defined and correctly implemented. rg --type ts $'getUnixTimeMinutesBeforeNow'Length of output: 1260
packages/synapse-interface/components/StateManagedBridge/BridgeTransactionButton.tsx (1)
100-100
: LGTM!The addition of the
pendingLabel
property improves the user interface by providing clearer communication of the button's state during asynchronous operations.The code changes are approved.
packages/synapse-interface/slices/bridgeQuote/thunks.ts (1)
Line range hint
109-121
: LGTM!The function is correctly implemented and the addition of
id
enhances the tracking of bridge quotes.The code changes are approved.
packages/synapse-interface/utils/types/index.tsx (1)
73-73
: LGTM!The addition of
id
enhances the tracking of bridge quotes and the type is correctly defined.The code changes are approved.
packages/synapse-interface/pages/state-managed-bridge/index.tsx (4)
57-63
: LGTM!The imports are correctly added and improve the functionality of the component.
The code changes are approved.
81-81
: LGTM!The constant
quoteTimeout
is correctly defined and likely represents a timeout duration for bridge quote operations.The code changes are approved.
141-141
: LGTM!The change improves the accuracy of timestamp calculations.
The code changes are approved.
231-241
: LGTM!The changes enhance the robustness and error handling of the function.
The code changes are approved.
Also applies to: 399-399
) | ||
|
||
if (timeDifference > quoteTimeout && !isLoading) { | ||
await getAndSetBridgeQuote() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't think we should do this here after executeBridge
has been called, which implies user has agreed to quote at t_0
. If the quote changes at t_1
, then they won't know the quote's changed since it won't be obvious to them.
Example: User sees RFQ quote from chainA to chainB for 100 USDC -> 99 USDC (t_0
). They click bridge but don't sign just yet. RFQ ChainB inventory adjusts. Some seconds pass. getAndSetBridgeQuote()
is called. Now the new quote is 100 USDC -> 80 USDC on CCTP/SynapseBridge (t_1
), but there's no clear indicator to the user that anything's changed (since they're expecting is to sign the txn in their wallet). Issue is mainly the user agreed to quote at t_0
but not t_1
even though they're signing for it.
There was a problem hiding this 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
Files selected for processing (1)
- packages/synapse-interface/pages/state-managed-bridge/index.tsx (12 hunks)
Additional comments not posted (8)
packages/synapse-interface/pages/state-managed-bridge/index.tsx (8)
57-63
: LGTM!The import statements for the new utility functions are correctly added.
72-72
: LGTM!Declaring
dispatch
at the beginning of the component improves readability and ensures it is available throughout the component's lifecycle.
81-81
: LGTM!The
quoteTimeout
constant is correctly introduced and likely represents a timeout duration for bridge quote operations.
141-141
: LGTM!The
currentTimestamp
is now correctly calculated usinggetUnixTimeMinutesFromNow
, reflecting a shift in how timestamps are calculated for bridge transactions.
205-206
: LGTM!Passing
quoteTimeout
as an argument touseStaleQuoteUpdater
ensures that the timeout duration is correctly applied.
231-231
: LGTM!The
currentTimestamp
is now correctly calculated usinggetUnixTimeMinutesFromNow
, reflecting a shift in how timestamps are calculated for bridge transactions.
243-244
: LGTM!Including
address
andbridgeQuote.id
in thesegmentAnalyticsEvent
call ensures that the analytics event captures the necessary information for tracking the bridge transaction.
390-390
: LGTM!Using
console.error
instead ofconsole.log
enhances error visibility and debugging.
There was a problem hiding this 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
Files selected for processing (2)
- packages/synapse-interface/package.json (1 hunks)
- packages/synapse-interface/pages/state-managed-bridge/index.tsx (12 hunks)
Files skipped from review due to trivial changes (1)
- packages/synapse-interface/package.json
Files skipped from review as they are similar to previous changes (1)
- packages/synapse-interface/pages/state-managed-bridge/index.tsx
Require #2896 to be merged in first
null
c4a392e: synapse-interface preview link
7eb45d3: synapse-interface preview link
dd77bd4: synapse-interface preview link
6212b70: synapse-interface preview link
ec9ad93: synapse-interface preview link
Summary by CodeRabbit
New Features
pendingLabel
for the button in the transaction process to enhance user feedback during loading.id
property to theEMPTY_BRIDGE_QUOTE
for better tracking of bridge quotes.Improvements
console.log
toconsole.error
for better visibility.BridgeQuote
type to include anid
for enhanced data handling.c69c4ff: synapse-interface preview link
3f782e6: synapse-interface preview link
db4d6c0: synapse-interface preview link
bcdc933: synapse-interface preview link
28fd8da: synapse-interface preview link