-
Notifications
You must be signed in to change notification settings - Fork 33
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
fix(synapse-interface): RFQ input vs bridgeQuote validation #2742
Conversation
WalkthroughThe Changes
Poem
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 ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2742 +/- ##
=============================================
Coverage 26.06700% 26.06700%
=============================================
Files 692 692
Lines 50539 50539
Branches 80 80
=============================================
Hits 13174 13174
Misses 36023 36023
Partials 1342 1342
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Deploying sanguine with
|
Latest commit: |
b6461b6
|
Status: | ✅ Deploy successful! |
Preview URL: | https://bdce60c4.sanguine.pages.dev |
Branch Preview URL: | https://fe-bridge-quote-validation.sanguine.pages.dev |
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
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- packages/synapse-interface/components/StateManagedBridge/BridgeTransactionButton.tsx (2 hunks)
Additional context used
Biome
packages/synapse-interface/components/StateManagedBridge/BridgeTransactionButton.tsx
[error] 71-71: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
Additional comments not posted (3)
packages/synapse-interface/components/StateManagedBridge/BridgeTransactionButton.tsx (3)
73-75
: The calculation offromValueBigInt
usinguseMemo
is correctly implemented to optimize performance by recalculating only whenfromValue
orfromTokenDecimals
changes.
77-82
: The logic to determine if the bridge quote amount is greater than the input for RFQ is correctly encapsulated within auseMemo
hook, optimizing the component's performance.
90-90
: Ensure that the conditions for disabling the button and setting button properties are comprehensive and accurately reflect the intended logic, especially with the newbridgeQuoteAmountGreaterThanInputForRfq
condition.Also applies to: 124-128
Verification successful
To verify the review comment, we need to check the following:
- The definition and calculation of
bridgeQuoteAmountGreaterThanInputForRfq
.- How
bridgeQuoteAmountGreaterThanInputForRfq
is used to determine the button's state (e.g., disabled state and properties).Let's start by finding where
bridgeQuoteAmountGreaterThanInputForRfq
is defined and how it is calculated.
Next, we need to locate how
bridgeQuoteAmountGreaterThanInputForRfq
is used within the component to determine the button's state and properties.
To verify the conditions for disabling the button and setting its properties, we need to extract and analyze the surrounding code where
bridgeQuoteAmountGreaterThanInputForRfq
is used.
The variable
bridgeQuoteAmountGreaterThanInputForRfq
is used in the conditions for disabling the button and setting its properties. The conditions appear to be comprehensive and accurately reflect the intended logic.
- The button is disabled if
bridgeQuoteAmountGreaterThanInputForRfq
is true.- The button's properties are set to display "Invalid bridge quote" when
bridgeQuoteAmountGreaterThanInputForRfq
is true.These conditions ensure that the button's state is managed correctly based on the new logic introduced.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the button's disabled state and properties are set correctly across different application states. # Test: Check if the button's state changes appropriately under different conditions. echo "Manual testing needed to verify button states under various conditions."Length of output: 152
Script:
#!/bin/bash # Find the definition and calculation of `bridgeQuoteAmountGreaterThanInputForRfq` in the specified file. ast-grep --lang tsx --pattern 'const bridgeQuoteAmountGreaterThanInputForRfq = $_' packages/synapse-interface/components/StateManagedBridge/BridgeTransactionButton.tsxLength of output: 936
Script:
#!/bin/bash # Search for the usage of `bridgeQuoteAmountGreaterThanInputForRfq` within the specified file. rg 'bridgeQuoteAmountGreaterThanInputForRfq' packages/synapse-interface/components/StateManagedBridge/BridgeTransactionButton.tsxLength of output: 298
Script:
#!/bin/bash # Extract and display the surrounding code where `bridgeQuoteAmountGreaterThanInputForRfq` is used. rg -A 5 -B 5 'bridgeQuoteAmountGreaterThanInputForRfq' packages/synapse-interface/components/StateManagedBridge/BridgeTransactionButton.tsxLength of output: 1174
Deploying sanguine-fe with
|
Latest commit: |
b6461b6
|
Status: | ✅ Deploy successful! |
Preview URL: | https://5a62c1a1.sanguine-fe.pages.dev |
Branch Preview URL: | https://fe-bridge-quote-validation.sanguine-fe.pages.dev |
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
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- packages/synapse-interface/components/StateManagedBridge/BridgeTransactionButton.tsx (4 hunks)
Additional context used
Biome
packages/synapse-interface/components/StateManagedBridge/BridgeTransactionButton.tsx
[error] 66-66: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
Additional comments not posted (2)
packages/synapse-interface/components/StateManagedBridge/BridgeTransactionButton.tsx (2)
68-70
: Good use ofuseMemo
for optimizing performance when calculatingfromValueBigInt
.
72-77
: Correct implementation ofuseMemo
for thebridgeQuoteAmountGreaterThanInputForRfq
calculation. This logic is crucial for the new validation feature.
const fromTokenDecimals: number | undefined = | ||
fromToken && fromToken?.decimals[fromChainId] | ||
|
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.
Use optional chaining for safer and cleaner code.
- const fromTokenDecimals: number | undefined = fromToken && fromToken?.decimals[fromChainId]
+ const fromTokenDecimals: number | undefined = fromToken?.decimals[fromChainId]
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 fromTokenDecimals: number | undefined = | |
fromToken && fromToken?.decimals[fromChainId] | |
const fromTokenDecimals: number | undefined = fromToken?.decimals[fromChainId] |
Tools
Biome
[error] 66-66: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
Summary by CodeRabbit
BridgeTransactionButton
to include new calculations for token decimals and values.950938f: synapse-interface preview link
b07ed8c: synapse-interface preview link