feat(synapse-interface): confirm new price [SLT-150] #3084
Conversation
bigboydiamonds
requested review from
abtestingalpha,
aureliusbtc,
trajan0x and
ChiTimesChi
as code owners
|
Warning Rate limit exceeded@bigboydiamonds has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 12 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe pull request introduces new components and hooks for managing bridge quote states, enhancing user interactions within the bridge transaction process. Key updates include the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BridgeTransactionButton
participant BridgeQuoteResetTimer
participant useConfirmNewBridgePrice
User->>BridgeTransactionButton: Interacts with button
BridgeTransactionButton->>useConfirmNewBridgePrice: Check for price change
useConfirmNewBridgePrice->>BridgeTransactionButton: Return confirmation state
BridgeTransactionButton->>BridgeQuoteResetTimer: Render countdown if quote is valid
BridgeQuoteResetTimer-->>User: Show countdown
Possibly related PRs
Suggested labels
Suggested reviewers
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
|
github-actions
bot
added
M-synapse-interface
Sol
Typescript
M-vscode-settings
size/s
labels
Deploying sanguine-fe with
|
| Latest commit: |
907d0e1
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://7e44b30b.sanguine-fe.pages.dev |
| Branch Preview URL: | https://fe-confirm-new-price.sanguine-fe.pages.dev |
Deploying sanguine with
|
| Latest commit: |
907d0e1
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://618e58ed.sanguine.pages.dev |
| Branch Preview URL: | https://fe-confirm-new-price.sanguine.pages.dev |
Bundle ReportChanges will decrease total bundle size by 25.54MB (-71.21%) ⬇️. This is within the configured threshold ✅ Detailed changes
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3084 +/- ##
====================================================
+ Coverage 38.22938% 90.96267% +52.73329%
====================================================
Files 423 54 -369
Lines 24353 1018 -23335
Branches 119 82 -37
====================================================
- Hits 9310 926 -8384
+ Misses 14301 91 -14210
+ Partials 742 1 -741
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
* request user confirm after first accepted * Update button message to reflect when quote updating * change button text ordering for insufficient balance * rm log
…sanguine into fe/confirm-new-price
* animate-quote-refresh * pair stale quote refresh with each quote * update progress circle animation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
packages/synapse-interface/components/StateManagedBridge/hooks/useConfirmNewBridgePrice.ts (1)
112-122: Add type annotations for improved type safety.The
calculateOutputRelativeDifferencefunction is properly handling edge cases and calculating the relative difference between two bridge quote outputs. However, it could be improved by adding type annotations for the arguments and return value to improve type safety and readability.Consider adding type annotations like this:
const calculateOutputRelativeDifference = ( quoteA?: BridgeQuote, quoteB?: BridgeQuote ): number | null => { // ... }
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- packages/synapse-interface/components/StateManagedBridge/hooks/useConfirmNewBridgePrice.ts (1 hunks)
Additional comments not posted (1)
packages/synapse-interface/components/StateManagedBridge/hooks/useConfirmNewBridgePrice.ts (1)
8-110: LGTM!The
useConfirmNewBridgePricehook is well-structured and follows a clear logic flow. It properly manages state and handles edge cases, while also optimizing performance by memoizing values and cleaning up state when necessary. The naming of variables and functions is descriptive and easy to understand.
bigboydiamonds
force-pushed
the
fe/confirm-new-price
branch
from
abc7b21 to
1c5414f
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (2)
packages/synapse-interface/components/StateManagedBridge/BridgeQuoteResetTimer.tsx (2)
35-55: Consider extracting SVG dimensions and other constants.The
AnimatedLoadingCirclecomponent is correctly implemented and the SVG animation is properly defined.As a minor refactor, consider extracting the SVG dimensions and other constants into separate variables for better readability and maintainability.
+const LOADING_CIRCLE_DIMENSIONS = { + width: 24, + height: 24, + viewBox: '-12 -12 24 24', + radius: 8, + pathLength: 1, + strokeDashArray: 0.05, + strokeOpacity: 0.5, + animationDuration: '2.5s', +}; const AnimatedLoadingCircle = () => { return ( <svg - width="24" - height="24" - viewBox="-12 -12 24 24" + width={LOADING_CIRCLE_DIMENSIONS.width} + height={LOADING_CIRCLE_DIMENSIONS.height} + viewBox={LOADING_CIRCLE_DIMENSIONS.viewBox} stroke="currentcolor" fill="none" className="absolute block -rotate-90" > - <circle r="8" pathLength="1" stroke-dashArray="0.05" stroke-opacity=".5"> + <circle + r={LOADING_CIRCLE_DIMENSIONS.radius} + pathLength={LOADING_CIRCLE_DIMENSIONS.pathLength} + stroke-dashArray={LOADING_CIRCLE_DIMENSIONS.strokeDashArray} + stroke-opacity={LOADING_CIRCLE_DIMENSIONS.strokeOpacity} + > <animate attributeName="stroke-dashoffset" to="-1" - dur="2.5s" + dur={LOADING_CIRCLE_DIMENSIONS.animationDuration} repeatCount="indefinite" /> </circle> </svg> ) }
57-113: Consider extracting SVG dimensions and other constants.The
AnimatedProgressCirclecomponent is correctly implemented and the SVG animation is properly defined. The use ofuseStateanduseEffecthooks to trigger a re-render when theanimateKeyprop changes is a good approach. TheconvertMsToSecondsutility function is correctly used to convert thedurationprop from milliseconds to seconds.As a minor refactor, consider extracting the SVG dimensions and other constants into separate variables for better readability and maintainability.
+const PROGRESS_CIRCLE_DIMENSIONS = { + width: 24, + height: 24, + viewBox: '-12 -12 24 24', + radius: 8, + pathLength: 1, + strokeOpacity: { + initial: 0.25, + final: 0.5, + }, + strokeDashArray: { + initial: 1, + final: 0.05, + }, + animationDuration: '2.5s', +}; const AnimatedProgressCircle = ({ animateKey, duration, }: { animateKey: string duration: number }) => { const [animationKey, setAnimationKey] = useState(0) useEffect(() => { setAnimationKey((prevKey) => prevKey + 1) }, [animateKey]) return ( <svg key={animationKey} - width="24" - height="24" - viewBox="-12 -12 24 24" + width={PROGRESS_CIRCLE_DIMENSIONS.width} + height={PROGRESS_CIRCLE_DIMENSIONS.height} + viewBox={PROGRESS_CIRCLE_DIMENSIONS.viewBox} stroke="currentcolor" fill="none" className="absolute block -rotate-90" > - <circle r="8" pathLength="1" stroke-opacity=".25"> + <circle + r={PROGRESS_CIRCLE_DIMENSIONS.radius} + pathLength={PROGRESS_CIRCLE_DIMENSIONS.pathLength} + stroke-opacity={PROGRESS_CIRCLE_DIMENSIONS.strokeOpacity.initial} + > <animate attributeName="stroke-dashoffset" to="-1" - dur="2.5s" + dur={PROGRESS_CIRCLE_DIMENSIONS.animationDuration} repeatCount="indefinite" /> <set attributeName="stroke-dasharray" - to="0.05" + to={PROGRESS_CIRCLE_DIMENSIONS.strokeDashArray.final} begin={`${convertMsToSeconds(duration)}s`} /> <set attributeName="stroke-opacity" - to="0.5" + to={PROGRESS_CIRCLE_DIMENSIONS.strokeOpacity.final} begin={`${convertMsToSeconds(duration)}s`} /> </circle> - <circle r="8" stroke-dasharray="1" pathLength="1"> + <circle + r={PROGRESS_CIRCLE_DIMENSIONS.radius} + stroke-dasharray={PROGRESS_CIRCLE_DIMENSIONS.strokeDashArray.initial} + pathLength={PROGRESS_CIRCLE_DIMENSIONS.pathLength} + > <animate attributeName="stroke-dashoffset" values="2; 1" dur={`${convertMsToSeconds(duration)}s`} fill="freeze" /> <animate attributeName="stroke-opacity" values="0; 1" dur={`${convertMsToSeconds(duration)}s`} /> </circle> </svg> ) }
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- packages/synapse-interface/components/StateManagedBridge/BridgeQuoteResetTimer.tsx (1 hunks)
- packages/synapse-interface/pages/state-managed-bridge/index.tsx (5 hunks)
Additional comments not posted (8)
packages/synapse-interface/components/StateManagedBridge/BridgeQuoteResetTimer.tsx (1)
6-33: LGTM!The
BridgeQuoteResetTimercomponent is well-implemented and aligns with the PR objectives. The use ofuseMemois a good optimization to avoid unnecessary re-renders. The component is properly typed and the props are well-defined.packages/synapse-interface/pages/state-managed-bridge/index.tsx (7)
68-70: LGTM!The new imports are relevant to the changes made in the file.
223-230: LGTM!The
isUpdaterEnabledconstant is a well-defined flag that determines if the quote updater should be enabled based on various conditions. The logic is correct and covers all the necessary conditions.
437-439: LGTM!The new error handling case for user-rejected transactions is a reasonable approach. If the user rejects the transaction, the code will fetch and set a new bridge quote by calling the
getAndSetBridgeQuotefunction, which ensures that the quote is updated accordingly.
484-484: LGTM!The
isQuoteStaleprop is passed to theOutputContainercomponent, which is consistent with the introduction of theisQuoteStaleconstant earlier in the code. The component likely uses this prop to conditionally render UI elements or display warnings based on the staleness of the quote.
489-506: LGTM!The new layout structure for the bridge transaction button and the bridge quote reset timer looks good. The
BridgeTransactionButtoncomponent receives several new props that likely control its behavior and appearance based on various conditions, while theBridgeQuoteResetTimercomponent is positioned absolutely to the right of the button and receives relevant props. The layout and prop usage seem reasonable and consistent with the rest of the code.
231-236: Verify the usage ofisQuoteStale.The
isQuoteStaleconstant is assigned the return value of theuseStaleQuoteUpdaterhook, which determines if the bridge quote is stale based on the provided arguments. Please ensure thatisQuoteStaleis used correctly to conditionally render UI elements or trigger actions based on the staleness of the quote.Run the following script to verify the usage of
isQuoteStale:Verification successful
Usage of
isQuoteStaleis verified and correct.The
isQuoteStaleconstant is used appropriately in the codebase to conditionally render UI elements and apply styles based on the staleness of the quote. It is passed as a prop toOutputContainer, affects class names, and is part of conditional logic inBridgeTransactionButton.
index.tsx: Passed as a prop toOutputContainer.OutputContainer.tsx: Used to determineinputClassName.BridgeTransactionButton.tsx: Used in a conditional expression.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `isQuoteStale` constant. # Test: Search for the usage of `isQuoteStale`. Expect: At least one occurrence. rg --type tsx $'isQuoteStale'Length of output: 56
Script:
#!/bin/bash # Description: Verify the usage of `isQuoteStale` constant. # Test: Search for the usage of `isQuoteStale` in the codebase. rg 'isQuoteStale'Length of output: 982
102-103: Verify the usage ofhasValidQuoteandhasSufficientBalance.The destructured constants
hasValidQuoteandhasSufficientBalancefrom theuseBridgeValidationshook are not used in the provided code segment. Please ensure that they are used correctly in other parts of the file.Run the following script to verify the usage of the constants:
Verification successful
Usage of
hasValidQuoteandhasSufficientBalanceis verified.The constants
hasValidQuoteandhasSufficientBalanceare actively used in logical conditions throughout the file, confirming their correct usage. No issues were found.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `hasValidQuote` and `hasSufficientBalance` constants. # Test: Search for the usage of `hasValidQuote`. Expect: At least one occurrence. rg --type tsx $'hasValidQuote' # Test: Search for the usage of `hasSufficientBalance`. Expect: At least one occurrence. rg --type tsx $'hasSufficientBalance'Length of output: 122
Script:
#!/bin/bash # Description: Verify the usage of `hasValidQuote` and `hasSufficientBalance` constants in .tsx files. # Find all .tsx files and search for the usage of `hasValidQuote`. fd --extension tsx --exec rg 'hasValidQuote' {} # Find all .tsx files and search for the usage of `hasSufficientBalance`. fd --extension tsx --exec rg 'hasSufficientBalance' {}Length of output: 807
There was a problem hiding this comment.
Actionable comments posted: 5
Outside diff range and nitpick comments (3)
packages/synapse-interface/messages/ar.json (1)
61-61: LGTM! Consider adding a trailing comma for consistency.The new translation for "Confirm new quote" is correctly added and appears accurate. However, for consistency with the JSON structure in the rest of the file, consider adding a trailing comma at the end of the line.
Here's the suggested change:
- "Confirm new quote": "تأكيد العرض الجديد" + "Confirm new quote": "تأكيد العرض الجديد",packages/synapse-interface/messages/en-US.json (1)
61-61: LGTM! Consider enhancing the message for clarity.The new localization string "Confirm new quote" has been correctly added to the "Bridge" section, aligning with the PR objective of implementing user confirmation for updated bridge quotes.
To enhance user understanding, consider expanding the message slightly:
- "Confirm new quote": "Confirm new quote" + "Confirm new quote": "Confirm new bridge quote"This minor change explicitly mentions that it's a bridge quote, which could improve clarity for users.
packages/synapse-interface/messages/tr.json (1)
61-61: LGTM! Consider adding a trailing comma for consistency.The new translation "Confirm new quote": "Yeni teklifi onayla" is correctly added and accurately translated. It's appropriately placed within the "Bridge" section.
For consistency with the rest of the file, consider adding a trailing comma at the end of the line:
- "Confirm new quote": "Yeni teklifi onayla" + "Confirm new quote": "Yeni teklifi onayla",
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (9)
- packages/synapse-interface/components/StateManagedBridge/BridgeTransactionButton.tsx (5 hunks)
- packages/synapse-interface/messages/ar.json (1 hunks)
- packages/synapse-interface/messages/en-US.json (1 hunks)
- packages/synapse-interface/messages/es.json (1 hunks)
- packages/synapse-interface/messages/fr.json (1 hunks)
- packages/synapse-interface/messages/jp.json (1 hunks)
- packages/synapse-interface/messages/tr.json (1 hunks)
- packages/synapse-interface/messages/zh-CN.json (1 hunks)
- packages/synapse-interface/pages/state-managed-bridge/index.tsx (5 hunks)
Additional comments not posted (14)
packages/synapse-interface/messages/zh-CN.json (2)
61-61: LGTM: New localization entry aligns with PR objectivesThe addition of "Confirm new quote": "确认新报价" is well-placed and correctly formatted. This new entry supports the PR objective of implementing user confirmation for updated bridge quotes.
Line range hint
1-1024: Overall assessment: Localization update is appropriate and focusedThe change to this Chinese (zh-CN) localization file is minimal and precisely targets the new feature for confirming updated bridge quotes. The addition maintains consistency with the existing file structure and doesn't introduce any unintended modifications to other translations.
packages/synapse-interface/messages/jp.json (1)
60-61: LGTM! New translation added correctly.The changes look good. The existing translation for the "No route found" message is preserved, and a new translation for "Confirm new quote" has been added. This aligns with the PR objectives of improving user signaling for updated bridge quotes.
To ensure consistency across all language files, let's check if this new key exists in other language files:
Verification successful
Verified: All language files include the "Confirm new quote" translation.
The "Confirm new quote" key has been successfully added to all language JSON files, ensuring consistent translations across the Synapse interface.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if "Confirm new quote" exists in all language files for file in $(fd -e json . packages/synapse-interface/messages); do echo "Checking $file:" jq '."Bridge"."Confirm new quote"' "$file" echo "---" doneLength of output: 2165
packages/synapse-interface/messages/ar.json (1)
Line range hint
1-361: Overall, the Arabic localization file is well-maintained and consistent.The addition of the new translation for "Confirm new quote" is appropriate and fits well within the existing structure. Remember to maintain consistency in formatting (such as trailing commas) throughout the file to ensure easy maintenance and reduce the likelihood of parsing errors.
Keep up the good work in providing comprehensive localization for the Synapse interface!
packages/synapse-interface/messages/tr.json (1)
Line range hint
1-561: Overall, the localization file looks good with the new addition.The Turkish localization file has been updated with a new translation for "Confirm new quote". The addition is appropriate and consistent with the existing content. No other changes or issues were observed in the file. The overall structure and comprehensive set of translations remain intact.
packages/synapse-interface/messages/es.json (1)
61-61: New translation added correctlyThe new translation "Confirm new quote": "Confirmar nueva cotización" has been added appropriately to the Spanish localization file. This addition aligns well with the PR objectives, specifically the feature requiring user confirmation for minor changes in bridge quotes.
packages/synapse-interface/messages/fr.json (1)
61-61: LGTM: New translation added correctlyThe new translation for "Confirm new quote" has been added correctly to the French localization file. The translation "Confirmer la nouvelle offre" is accurate and consistent with the style of other translations in the file.
However, to ensure completeness, please verify if there are any related strings (e.g., for canceling or rejecting a new quote) that might also need translation. If so, consider adding them in this PR as well.
Verification successful
LGTM: New translation added correctly
The new translation for "Confirm new quote" has been added correctly to the French localization file. The translation "Confirmer la nouvelle offre" is accurate and consistent with the style of other translations in the file.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for related strings in the English localization file rg --type json "new quote" packages/synapse-interface/messages/en.jsonLength of output: 156
Script:
#!/bin/bash # Check if en.json exists in the expected directory if [ -f packages/synapse-interface/messages/en.json ]; then echo "en.json exists." else echo "en.json does not exist. Searching for alternative English localization files..." # Search for any English localization files in the repository fd --extension json --glob '*en*.json' fiLength of output: 1825
Script:
#!/bin/bash # Search for related strings in the en-US.json localization file rg --type json "new quote" packages/synapse-interface/messages/en-US.jsonLength of output: 120
packages/synapse-interface/pages/state-managed-bridge/index.tsx (7)
68-71: Imports added correctly for new dependenciesThe new imports are appropriate and necessary for the added functionality.
75-75: DestructuredisConnectedfromuseAccountappropriatelyObtaining
isConnectedfromuseAccountis necessary for connection status checks.
102-102: Destructured validation states fromuseBridgeValidationsThe variables
hasValidQuoteandhasSufficientBalanceare correctly obtained for validation purposes.
223-230: Logical conditions forisUpdaterEnabledare appropriateThe conditions to determine if the quote updater should be enabled are correctly implemented.
231-236: InitializedisQuoteStaleusinguseStaleQuoteUpdatercorrectlyThe
isQuoteStalestate is properly set up usinguseStaleQuoteUpdaterwith the appropriate dependencies.
484-484: PassedisQuoteStaleprop toOutputContainerThe
isQuoteStaleprop is correctly passed toOutputContainerto handle stale quote display.
490-497: AddedisQuoteStaleprop toBridgeTransactionButtonPassing
isQuoteStaletoBridgeTransactionButtonallows the button to respond appropriately when the quote is stale.
| } else if (isApproved && hasValidQuote && isPendingConfirmChange) { | ||
| buttonProperties = { | ||
| label: t('Confirm new quote'), | ||
| onClick: () => onUserAcceptChange(), | ||
| className: | ||
| '!outline !outline-1 !outline-synapsePurple !outline-offset-[-1px] !from-bgLight !to-bgLight', | ||
| } |
There was a problem hiding this comment.
Avoid redundant condition by reusing existing logic
The condition starting at line 190 checks for isApproved && hasValidQuote && isPendingConfirmChange, which may overlap with earlier conditions. To simplify and prevent duplication, consider combining this logic with the adjustment made to isButtonDisabled.
| @@ -73,6 +77,7 @@ | |||
| isBridgeQuoteAmountGreaterThanInputForRfq || | |||
| (isConnected && !hasValidQuote) || | |||
| (isConnected && !hasSufficientBalance) || | |||
| (isConnected && isQuoteStale) || | |||
There was a problem hiding this comment.
Adjust 'isButtonDisabled' logic to enable user confirmation
Including (isConnected && isQuoteStale) in isButtonDisabled disables the button when the quote is stale, even when user confirmation is required. Modify the condition to exclude cases where isPendingConfirmChange is true, allowing the user to confirm the new quote.
Apply this diff to adjust the disabling logic:
const isButtonDisabled =
isBridgePaused ||
isTyping ||
isLoading ||
isWalletPending ||
!hasValidInput ||
!doesBridgeStateMatchQuote ||
isBridgeQuoteAmountGreaterThanInputForRfq ||
(isConnected && !hasValidQuote) ||
(isConnected && !hasSufficientBalance) ||
- (isConnected && isQuoteStale) ||
+ (isConnected && isQuoteStale && !isPendingConfirmChange) ||
(destinationAddress && !isAddress(destinationAddress))Committable suggestion was skipped due to low confidence.
| } else if (isLoading && hasValidQuote) { | ||
| buttonProperties = { | ||
| label: isPendingConfirmChange | ||
| ? t('Confirm new quote') | ||
| : t('Bridge {symbol}', { symbol: fromToken?.symbol }), | ||
| pendingLabel: t('Bridge {symbol}', { symbol: fromToken?.symbol }), | ||
| onClick: null, | ||
| className: ` | ||
| ${ | ||
| isPendingConfirmChange | ||
| ? '!outline !outline-1 !outline-synapsePurple !outline-offset-[-1px] !from-bgLight !to-bgLight' | ||
| : '!bg-gradient-to-r !from-fuchsia-500 !to-purple-500 dark:!to-purple-600' | ||
| } | ||
| !opacity-100`, | ||
| } |
There was a problem hiding this comment.
Ensure 'Confirm new quote' button is actionable when user confirmation is required
When isPendingConfirmChange is true, the button displays "Confirm new quote" but onClick is set to null, preventing the user from confirming the new quote. To allow users to proceed, the onClick handler should invoke onUserAcceptChange.
Apply this diff to set onClick appropriately:
buttonProperties = {
label: isPendingConfirmChange
? t('Confirm new quote')
: t('Bridge {symbol}', { symbol: fromToken?.symbol }),
pendingLabel: t('Bridge {symbol}', { symbol: fromToken?.symbol }),
- onClick: null,
+ onClick: isPendingConfirmChange ? () => onUserAcceptChange() : null,
className: `
${
isPendingConfirmChange
? '!outline !outline-1 !outline-synapsePurple !outline-offset-[-1px] !from-bgLight !to-bgLight'
: '!bg-gradient-to-r !from-fuchsia-500 !to-purple-500 dark:!to-purple-600'
}
!opacity-100`,
}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.
| } else if (isLoading && hasValidQuote) { | |
| buttonProperties = { | |
| label: isPendingConfirmChange | |
| ? t('Confirm new quote') | |
| : t('Bridge {symbol}', { symbol: fromToken?.symbol }), | |
| pendingLabel: t('Bridge {symbol}', { symbol: fromToken?.symbol }), | |
| onClick: null, | |
| className: ` | |
| ${ | |
| isPendingConfirmChange | |
| ? '!outline !outline-1 !outline-synapsePurple !outline-offset-[-1px] !from-bgLight !to-bgLight' | |
| : '!bg-gradient-to-r !from-fuchsia-500 !to-purple-500 dark:!to-purple-600' | |
| } | |
| !opacity-100`, | |
| } | |
| } else if (isLoading && hasValidQuote) { | |
| buttonProperties = { | |
| label: isPendingConfirmChange | |
| ? t('Confirm new quote') | |
| : t('Bridge {symbol}', { symbol: fromToken?.symbol }), | |
| pendingLabel: t('Bridge {symbol}', { symbol: fromToken?.symbol }), | |
| onClick: isPendingConfirmChange ? () => onUserAcceptChange() : null, | |
| className: ` | |
| ${ | |
| isPendingConfirmChange | |
| ? '!outline !outline-1 !outline-synapsePurple !outline-offset-[-1px] !from-bgLight !to-bgLight' | |
| : '!bg-gradient-to-r !from-fuchsia-500 !to-purple-500 dark:!to-purple-600' | |
| } | |
| !opacity-100`, | |
| } |
| <div className="absolute flex items-center !right-10 pointer-events-none"> | ||
| <BridgeQuoteResetTimer | ||
| bridgeQuote={bridgeQuote} | ||
| isLoading={isLoading} | ||
| isActive={isUpdaterEnabled} | ||
| duration={quoteTimeout} | ||
| /> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Potential layout issues with absolute positioning
Using absolute positioning for the BridgeQuoteResetTimer component may cause layout issues on different screen sizes or devices. Consider reviewing the UI to ensure the timer is properly positioned and does not overlap with other elements.
| if (isTransactionUserRejectedError) { | ||
| getAndSetBridgeQuote() | ||
| } |
There was a problem hiding this comment.
Fix incorrect condition in error handling
The condition if (isTransactionUserRejectedError) is incorrect. It should be a function call to check if the error is a user-rejected transaction error. The correct usage would be if (isTransactionUserRejectedError(error)).
Apply this diff to fix the condition:
-if (isTransactionUserRejectedError) {
+if (isTransactionUserRejectedError(error)) {
getAndSetBridgeQuote()
}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.
| if (isTransactionUserRejectedError) { | |
| getAndSetBridgeQuote() | |
| } | |
| if (isTransactionUserRejectedError(error)) { | |
| getAndSetBridgeQuote() | |
| } |
packages/synapse-interface/components/StateManagedBridge/hooks/useConfirmNewBridgePrice.ts
Outdated
Show resolved
Hide resolved
| import { BridgeQuote } from '@/utils/types' | ||
|
|
||
| export const useConfirmNewBridgePrice = () => { | ||
| const triggerQuoteRef = useRef<any>(null) |
There was a problem hiding this comment.
Specify explicit type for 'triggerQuoteRef' to enhance type safety
Currently, triggerQuoteRef is typed as any, which negates the benefits of TypeScript's type checking. By specifying a more precise type, you can improve type safety and reduce potential runtime errors.
Update the declaration to use the appropriate type:
-const triggerQuoteRef = useRef<any>(null)
+const triggerQuoteRef = useRef<BridgeQuote | null>(null)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.
| const triggerQuoteRef = useRef<any>(null) | |
| const triggerQuoteRef = useRef<BridgeQuote | null>(null) |
| if (!currentQuote?.outputAmountString || !previousQuote?.outputAmountString) { | ||
| return null | ||
| } | ||
|
|
||
| const currentOutput = parseFloat(currentQuote.outputAmountString) | ||
| const previousOutput = parseFloat(currentQuote.outputAmountString) | ||
|
|
||
| return previousOutput - currentOutput / previousOutput |
There was a problem hiding this comment.
Handle potential division by zero in 'calculateOutputRelativeDifference'
If previousOutput is zero, the calculation (currentOutput - previousOutput) / previousOutput will result in a division by zero error. This can cause a runtime exception and break the application flow. Consider adding a check to prevent division by zero and handle this edge case appropriately.
Apply the following changes to handle the zero previousOutput case:
const currentOutput = parseFloat(currentQuote.outputAmountString)
const previousOutput = parseFloat(previousQuote.outputAmountString)
+if (previousOutput === 0) {
+ return null // or handle accordingly based on the application's requirements
+}
return (currentOutput - previousOutput) / previousOutputCommittable 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.
| if (!currentQuote?.outputAmountString || !previousQuote?.outputAmountString) { | |
| return null | |
| } | |
| const currentOutput = parseFloat(currentQuote.outputAmountString) | |
| const previousOutput = parseFloat(currentQuote.outputAmountString) | |
| return previousOutput - currentOutput / previousOutput | |
| if (!currentQuote?.outputAmountString || !previousQuote?.outputAmountString) { | |
| return null | |
| } | |
| const currentOutput = parseFloat(currentQuote.outputAmountString) | |
| const previousOutput = parseFloat(previousQuote.outputAmountString) | |
| if (previousOutput === 0) { | |
| return null // or handle accordingly based on the application's requirements | |
| } | |
| return (currentOutput - previousOutput) / previousOutput |
| const outputAmountDiffMoreThanThreshold = validQuotes | ||
| ? calculateOutputRelativeDifference( | ||
| bridgeQuote, | ||
| triggerQuoteRef.current ?? previousBridgeQuote | ||
| ) > bpsThreshold | ||
| : false | ||
|
|
There was a problem hiding this comment.
Ensure safe comparison when 'calculateOutputRelativeDifference' may return null
The function calculateOutputRelativeDifference may return null if outputAmountString is missing or if there's a division by zero. Comparing null directly with a number using the > operator can lead to unintended behavior. It's important to check for null before performing the comparison to ensure the application logic is sound.
Apply the following changes to handle the null case safely:
const diff = calculateOutputRelativeDifference(
bridgeQuote,
triggerQuoteRef.current ?? previousBridgeQuote
)
const outputAmountDiffMoreThanThreshold = validQuotes
- ? diff > bpsThreshold
+ ? diff !== null && diff > bpsThreshold
: falseCommittable suggestion was skipped due to low confidence.
* update bl * remove global solidity extension settings * use monorepo support in global workspace only * - use Solidity extension for formatting *.sol files - use `forge fmt` as formatter in Solidity extension * REST API Improvements [SLT-179] (#3133) * fix swaptxinfo function * Updates test coverage command * migrating to using token addresses instead of symbols * fix linting errors * fixing swaptxinfocontroller * new tests and new functionality --------- Co-authored-by: abtestingalpha <[email protected]> * Publish - @synapsecns/[email protected] - @synapsecns/[email protected] * fix harmony proxy (#3149) Co-authored-by: Trajan0x <[email protected]> * merging rfq indexer into monorepo [SLT-164] [SLT-176] (#3136) * merging rfq indexer into monorepo * nuke .env * fix commands * fix package name * test coverage script * rough pass at docs and some linting and fixes yarn * Upgrades wagmi & rainbowkit * indxer * Adds invisible but used packages * +recent-invalid-fills [SLT-188] * Moves wagmi to root * new endpoints and clean up linting --------- Co-authored-by: Trajan0x <[email protected]> Co-authored-by: abtestingalpha <[email protected]> Co-authored-by: parodime <[email protected]> * Publish - @synapsecns/[email protected] - @synapsecns/[email protected] - @synapsecns/[email protected] * Adds /destinationTokens route [SLT-204] (#3151) * Adds /destinationTokens route * ZeroAddress & NativeGasAddress * Adds test for native gas tokens * Checksums incoming token address params * Publish - @synapsecns/[email protected] * boba pause (#3150) * boba pause * only boba to txns * Publish - @synapsecns/[email protected] * fix(synapse-interface): Reorders validation to check existence first (#3156) * Reorders validation to check existence first * Removes duplicates * Publish - @synapsecns/[email protected] * Fix boba pause (#3158) * Publish - @synapsecns/[email protected] * update bl * feat(rest-api): Adds Swagger for api docs [SLT-205] (#3159) * Adds Swagger for api docs * Replace prepended verb Get routes with nouns * Adds dev flag for swagger serverUrl * Publish - @synapsecns/[email protected] - @synapsecns/[email protected] - @synapsecns/[email protected] - @synapsecns/[email protected] * Pulls version from package json (#3160) * Publish - @synapsecns/[email protected] * Require vs import due to file location (#3161) * Require vs import due to file location * Publish - @synapsecns/[email protected] * Prevent caching of api docs (#3162) * Publish - @synapsecns/[email protected] * feat(contracts-rfq): relay/prove/claim with different address [SLT-130] (#3138) * init. solidity ^. FbV2 relay/prove/claim overloads * +IFastBridgeV2, explicit address0 cast, func scope & inheritdoc fixes * pragma lock, contract relabel * feat: start scoping V2 tests * test: override relayer role scenarios, no longer enforced by V2 * test: finish the parity test * test: the management methods * test: dst chain scenarios * test: bridge * test: prove * test: claim * test: dispute * test: refund * test: bridge reverts * remove redundant extend. rearrange inherit list * revert 0.8.20 in favor of user (non-ws) setting --------- Co-authored-by: ChiTimesChi <[email protected]> * Publish - [email protected] * fix(promexporter): make spans better (#3164) * move the errors * [goreleaser] * fix v to w * changing native token address standard [SLT-210] (#3157) * changing native token address standard * fixing tests * normalizeNativeTokenAddress middleware, additional tests --------- Co-authored-by: abtestingalpha <[email protected]> * Publish - @synapsecns/[email protected] * Refactoring rfq-indexer API and adding swagger docs [SLT-228] (#3167) * refactoring and adding swagger * remove testing scripts * fix typos and consistency with 404 errors * Publish - @synapsecns/[email protected] * fix read mes (#3168) * Publish - @synapsecns/[email protected] - [email protected] - @synapsecns/[email protected] * fix(opbot): use submitter get tx status [SLT-158] (#3134) * use experimental logger to debug * fix lint * [goreleaser] * use submitter instead of client * [goreleaser] * [goreleaser] * fix(synapse-interface): Additional checks on screen [SLT-166] (#3152) * Additional checks on screen * Adds checks on chain/token changes * Publish - @synapsecns/[email protected] * feat(synapse-interface): confirm new price [SLT-150] (#3084) * add bridge quote history middleware * request user confirm changes when quoted price updates * add conditions for displaying confirm change state * track initial quote initializing confirm change state * specify output delta threshold * callback functions to handle initialize/accept/reset confirm changes flow * quote countdown timer animation to signal refresh * implement automatic refresh intervals * mouse move to refresh automatic intervals * add i8n translations for button text --------- Co-authored-by: abtestingalpha <[email protected]> * Publish - @synapsecns/[email protected] * fix: formatted bridge fee amount (#3165) * Publish - @synapsecns/[email protected] * fix(contracts-rfq): CI workflows [SLT-245] (#3178) * fix: license, files * fix: package name * build: update solhint to latest * build: remove prettier dependencies * fix: solhint workflows * build: update solhint in other packages as well * chore: solhint rules, exceptions * fix: silence linter warnings in tests * chore: forge fmt * add variable to test linter CI * Revert "add variable to test linter CI" This reverts commit 0629309. * Publish - @synapsecns/[email protected] - @synapsecns/[email protected] - @synapsecns/[email protected] * feat(api): bridge limits [SLT-165] (#3179) * adds `/bridgeLimits` route, controller * fetch best sdk quote for min/max origin amounts * add tests * implement middleware to normalize addresses * adds swagger doc * Publish - @synapsecns/[email protected] * fix(contracts-rfq): limit the amount of solhint warnings [SLT-245] (#3182) * ci: limit the amount of solhint warnings * refactor: move the errors into the separate interface * refactor: errors imports in tests * Publish - @synapsecns/[email protected] * ci: Solidity gas diff [SLT-259] (#3181) * ci: run tests w/o coverage first for better visibility * test: malform the test to check the adjusted workflow * Revert "test: malform the test to check the adjusted workflow" This reverts commit e7db6e1. * ci: add gas-diff workflow * try changing the contract to trigger gas diffs * retrigger the workflow * ci: provide the correct report path * ci: run on pull requests only * ci: save gas reports in monorepo root * Revert "ci: run on pull requests only" This reverts commit 0a01d60. * Revert "try changing the contract to trigger gas diffs" This reverts commit 91bc03e. * refactor: wrap if statement * refactor: exclude `solidity-devops` package in a more generic way * ci: run tests w/o coverage for `solidity-devops`, add comments * add generic comment to trigger `solidity-devops` workflows * Revert "add generic comment to trigger `solidity-devops` workflows" This reverts commit cc35a43. * Publish - @synapsecns/[email protected] * fix(contracts-core): set very high gas limit for intensive tests [SLT-259] (#3186) * fix: set very high gas limit for intensive tests * ci: speed up solidity coverage * Publish - @synapsecns/[email protected] * feat(rest-api): Adds validateRouteExists validation [SLT-260] (#3180) * Adds validateRouteExists validation * Remove timeouts for 400s * Publish - @synapsecns/[email protected] * add duplicate command warning (#3174) Co-authored-by: Trajan0x <[email protected]> * reduce solhint warnings on FbV2 (#3189) * reduce solhint warnings on FbV2 * fix whitespace * Publish - @synapsecns/[email protected] * ci: solidity gas diff options [SLT-267] (#3193) * ci: ignore test files in gas diff report * add some changes to the test files * ci: define some options for gas-diff * try changing the contract to trigger gas diffs * Revert "try changing the contract to trigger gas diffs" This reverts commit 4504e3c. * Revert "add some changes to the test files" This reverts commit 7e7d6cb. * prove w/ tx id [SLT-181] (#3169) * prove w/ tx id SLT-181 * +proveOther tests, forge fmt * fmt * fmt * Publish - @synapsecns/[email protected] * fix(sdk-router): disable ARB airdrop tests (#3195) * Publish - @synapsecns/[email protected] - @synapsecns/[email protected] - @synapsecns/[email protected] - @synapsecns/[email protected] * Fixing issue for wallet integration [SLT-270] (#3194) * slight modification to graphql call * fixing explorer frontend as well * Publish - @synapsecns/[email protected] - @synapsecns/[email protected] * store relayer on relay [SLT-182] (#3170) * store relayer on relay [SLT-182] * +tests, zeroAddr check, fmt * Publish - @synapsecns/[email protected] * Adjust text to trigger build (#3199) * Publish - @synapsecns/[email protected] * feat(synapse-interface): refund RFQ transaction [SLT-272] (#3197) * Txn transaction refund tracking * Update store to support tracking * Query FastBridge contract for `bridgeStatuses` to find refund status * Track bridge transaction `bridgeQuote.routerAddress` in store * Fetch FastBridge contract address when only provided router address * add translations --------- Co-authored-by: aureliusbtc <[email protected]> Co-authored-by: ChiTimesChi <[email protected]> Co-authored-by: abtestingalpha <[email protected]> Co-authored-by: Defi-Moses <[email protected]> Co-authored-by: trajan0x <[email protected]> Co-authored-by: Trajan0x <[email protected]> Co-authored-by: parodime <[email protected]> Co-authored-by: abtestingalpha <[email protected]> Co-authored-by: abtestingalpha <[email protected]> Co-authored-by: parodime <[email protected]> Co-authored-by: vro <[email protected]> Co-authored-by: ChiTimesChi <[email protected]> Co-authored-by: bigboydiamonds <[email protected]> Co-authored-by: bigboydiamonds <[email protected]>
Description
This PR adds functionality around improving signaling to a User when a bridge quote has updated.
This PR adds the following functionalities:
bridge()18c5172: synapse-interface preview link
02bf5bb: synapse-interface preview link
8e99687: synapse-interface preview link
313cd3a: synapse-interface preview link
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
3856d30: synapse-interface preview link
f1b31a4: synapse-interface preview link
291fa53: synapse-interface preview link
8809695: synapse-interface preview link
432f9ca: synapse-interface preview link
c0857f2: synapse-interface preview link
c39247a: synapse-interface preview link
b9451d1: synapse-interface preview link
ccd21cd: synapse-interface preview link
b0beafc: synapse-interface preview link
fe86222: synapse-interface preview link
5ba611f: synapse-interface preview link
95a4052: synapse-interface preview link
fa3cf49: synapse-interface preview link
c668e67: synapse-interface preview link
2ea8146: synapse-interface preview link
7904e51: synapse-interface preview link
93d78f6: synapse-interface preview link
10be0da: synapse-interface preview link
f599af8: synapse-interface preview link
634665e: synapse-interface preview link
7c69b80: synapse-interface preview link
ac66722: synapse-interface preview link
ab4f112: synapse-interface preview link
5814e8b: synapse-interface preview link
c432216: synapse-interface preview link
38b6b14: synapse-interface preview link
0637ec7: synapse-interface preview link