feat(rfq-relayer): add MaxBalance param #2917
Conversation
WalkthroughThe recent changes enhance the quoting functionality within the relayer services by introducing minimum and maximum balance constraints. This ensures that sufficient funds remain after quoting, improving financial management. Key updates include modifications to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Quoter
participant Config
participant Token
User->>Quoter: Request Quote
Quoter->>Config: Get Token Configuration
Config->>Token: Retrieve MinBalance and MaxBalance
Token-->>Config: Return Balances
Quoter->>Quoter: Calculate quotableBalance
Quoter-->>User: Return Quoted Amount
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 Configuration File (
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
Introduced a MinBalance parameter to ensure the relayer maintains a minimum balance, preventing over-quoting.
services/rfq/relayer/quoter/quoter.go: AddedMinBalanceparameter to calculatequotableBalanceand clipquoteAmountif necessary.services/rfq/relayer/quoter/quoter_test.go: Updated tests to includeminBalanceparameter, ensuring correct quote adjustments.services/rfq/relayer/relconfig/config.go: AddedMinBalancefield toTokenConfigstruct for user-configurable minimum balance thresholds.services/rfq/relayer/relconfig/getters.go: AddedGetMinBalancefunction to retrieve and scale the minimum balance for a given chain and address.
4 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings
| quoteAmountFlt, ok := new(big.Float).SetString(tokenCfg.MinBalance) | ||
| if !ok { | ||
| return big.NewInt(defaultMinBalance) |
There was a problem hiding this comment.
Logic: Potential issue: If MinBalance is not a valid string representation of a big.Float, the function defaults to 0 without logging an error.
| if quoteAmountFlt.Cmp(big.NewFloat(0)) <= 0 { | ||
| return big.NewInt(defaultMinBalance) | ||
| } |
There was a problem hiding this comment.
Logic: Potential issue: If MinBalance is less than or equal to 0, the function defaults to 0 without logging an error.
Deploying sanguine-fe with
|
| Latest commit: |
7d00318
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b11c8d3f.sanguine-fe.pages.dev |
| Branch Preview URL: | https://feat-min-balance.sanguine-fe.pages.dev |
![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, codebase verification and nitpick comments (3)
services/rfq/relayer/relconfig/config.go (1)
131-132: Add documentation for the new fieldMinBalance.The new field
MinBalanceshould have a comment explaining its purpose, similar to other fields in the struct.+ // MinBalance is the minimum balance that should be leftover from quoting, in human-readable units. MinBalance string `yaml:"min_balance"`services/rfq/relayer/quoter/quoter_test.go (1)
207-207: Duplicate test case.The test case is a duplicate of the previous one. Consider removing it or modifying it to test a different scenario.
services/rfq/relayer/relconfig/getters.go (1)
301-302: Remove unnecessary constant declaration.The
defaultMinBalanceconstant is declared but not used elsewhere in the code. Consider removing it.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (4)
- services/rfq/relayer/quoter/quoter.go (1 hunks)
- services/rfq/relayer/quoter/quoter_test.go (2 hunks)
- services/rfq/relayer/relconfig/config.go (1 hunks)
- services/rfq/relayer/relconfig/getters.go (1 hunks)
Additional comments not posted (12)
services/rfq/relayer/quoter/quoter_test.go (6)
167-171: Ensure the new parameterminBalanceis correctly integrated.The new parameter
minBalanceis added to thesetQuoteParamsfunction. Ensure that it is correctly integrated into the function and used in the test cases.
186-186: Test case forminBalanceparameter.The test case sets
minBalanceto "0". Ensure that other test cases also cover scenarios with non-zerominBalancevalues.
193-193: Test case forminBalanceparameter.The test case sets
minBalanceto "0". Ensure that other test cases also cover scenarios with non-zerominBalancevalues.
200-200: Test case forminBalanceparameter.The test case sets
minBalanceto "0". Ensure that other test cases also cover scenarios with non-zerominBalancevalues.
214-214: Test case forminBalanceparameter.The test case sets
minBalanceto "0". Ensure that other test cases also cover scenarios with non-zerominBalancevalues.
220-225: Test case forminBalanceparameter.This test case sets
minBalanceto "200". Ensure that the expected amount calculation is correct and covers various scenarios.services/rfq/relayer/relconfig/getters.go (1)
303-334: Ensure correct handling ofMinBalanceretrieval and scaling.The
GetMinBalancemethod retrieves and scales theMinBalancevalue. Ensure that the method correctly handles edge cases, such as invalid or zero values.services/rfq/relayer/quoter/quoter.go (5)
522-522: Ensure the minimum balance is correctly retrieved.The line retrieves the minimum balance from the configuration. Verify that the
GetMinBalancemethod correctly returns the expected value for the given destination and address.
523-524: Calculate quotable balance accurately.The
quotableBalanceis calculated by subtractingminBalancefrombalance. Ensure that bothbalanceandminBalanceare non-negative to avoid unexpected results.
525-526: Correctly compare quote amount with quotable balance.The comparison ensures the
quoteAmountdoes not exceed thequotableBalance. This logic is sound, but ensure thatquoteAmountandquotableBalanceare in the same units.
529-530: Log detailed information for debugging.The logging provides detailed information about the
quoteAmount,balance,quotableBalance, andminBalance. This is useful for debugging and monitoring.
532-532: Clip the quote amount to the quotable balance.The
quoteAmountis clipped to thequotableBalanceif it exceeds it. This ensures that the minimum balance constraint is respected.
There was a problem hiding this comment.
PR Summary
(updates since last review)
The recent changes introduce a MinBalance parameter to ensure the relayer maintains a minimum balance, preventing over-quoting.
services/rfq/relayer/quoter/quoter_test.go: Updated tests to includeminBalanceparameter, ensuring correct quote adjustments.services/rfq/relayer/relconfig/config.go: AddedMinBalancefield toTokenConfigstruct for user-configurable minimum balance thresholds.services/rfq/relayer/relconfig/getters.go: AddedGetMinBalancefunction to retrieve and scale the minimum balance for a given chain and address.
No major changes found since the last review.
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2917 +/- ##
===================================================
+ Coverage 25.29264% 25.32165% +0.02901%
===================================================
Files 780 780
Lines 56811 56817 +6
Branches 82 82
===================================================
+ Hits 14369 14387 +18
+ Misses 40962 40951 -11
+ Partials 1480 1479 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- services/rfq/relayer/quoter/quoter_test.go (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- services/rfq/relayer/quoter/quoter_test.go
dwasse
changed the title
There was a problem hiding this comment.
PR Summary
(updates since last review)
The recent changes introduce a MinBalance parameter to ensure the relayer maintains a minimum balance, preventing over-quoting.
services/rfq/relayer/quoter/export_test.go: Updated method signatures to accommodate the newMinBalanceparameter and refactored methods to use a new input struct.services/rfq/relayer/quoter/quoter.go: IntroducedMinBalanceparameter andQuoteInputstruct, ensuring minimum balance preservation in quoting logic.services/rfq/relayer/quoter/quoter_test.go: Updated test cases to validate the newMinBalanceparameter.services/rfq/relayer/relconfig/config.go: RenamedMinBalancetoMaxBalanceinTokenConfigstruct, reflecting a shift in configuration logic.services/rfq/relayer/relconfig/getters.go: RenamedGetMinBalancetoGetMaxBalanceand updated associated logic and comments.
5 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (5)
- services/rfq/relayer/quoter/export_test.go (1 hunks)
- services/rfq/relayer/quoter/quoter.go (9 hunks)
- services/rfq/relayer/quoter/quoter_test.go (5 hunks)
- services/rfq/relayer/relconfig/config.go (1 hunks)
- services/rfq/relayer/relconfig/getters.go (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- services/rfq/relayer/quoter/quoter_test.go
- services/rfq/relayer/relconfig/config.go
- services/rfq/relayer/relconfig/getters.go
Additional comments not posted (5)
services/rfq/relayer/quoter/export_test.go (2)
17-18: LGTM! But verify the usage of the new struct.The
GetOriginAmountfunction now accepts aQuoteInputstruct. Ensure that the underlying functiongetOriginAmountcorrectly utilizes this new struct.Verification successful
Verified: The
QuoteInputstruct is correctly utilized in thegetOriginAmountfunction.The function
getOriginAmountinservices/rfq/relayer/quoter/quoter.goaccesses multiple attributes from theQuoteInputstruct, confirming its proper usage.
- Location:
services/rfq/relayer/quoter/quoter.goScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the new `QuoteInput` struct in the `getOriginAmount` function. # Test: Search for the function definition and usage of `QuoteInput` struct in `getOriginAmount`. rg --type go -A 10 $'func (m *Manager) getOriginAmount(parentCtx context.Context, input QuoteInput)'Length of output: 101
Script:
#!/bin/bash # Description: Verify the usage of the new `QuoteInput` struct in the `getOriginAmount` function. # Test: Search for the function definition and usage of `QuoteInput` struct in `getOriginAmount`. rg --type go 'func (m *Manager) getOriginAmount' -A 10Length of output: 56
Script:
#!/bin/bash # Description: Verify the usage of the new `QuoteInput` struct in any function containing `getOriginAmount`. # Test: Search for any function definitions containing `getOriginAmount`. rg --type go 'func .*getOriginAmount' -A 10Length of output: 1076
12-14: LGTM! But verify the usage of the new parameter.The
GenerateQuotesfunction now includes an additional parameterinv. Ensure that the underlying functiongenerateQuotescorrectly utilizes this new parameter.services/rfq/relayer/quoter/quoter.go (3)
Line range hint
314-366:
LGTM! But verify the usage of the new parameter.The
generateQuotesfunction now includes an additional parameterinvand uses it to retrieve balances for origin tokens. Ensure that the usage ofinvis correct and consistent throughout the function.Verification successful
LGTM! The
generateQuotesfunction now includes an additional parameterinvand uses it to retrieve balances for origin tokens. The usage of theinvparameter is correct and consistent throughout the function.
- The
invparameter is used to access balances within thegenerateQuotesfunction.- The function is called with the
invparameter in theprepareAndSubmitQuotesfunction.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the new `inv` parameter in the `generateQuotes` function. # Test: Search for the usage of `inv` parameter in the `generateQuotes` function. rg --type go -A 10 'inv' services/rfq/relayer/quoter/quoter.goLength of output: 5095
Line range hint
490-574:
LGTM! But verify the usage of the new struct.The
getOriginAmountfunction now accepts aQuoteInputstruct and uses its fields. Ensure that the usage of theQuoteInputstruct is correct and consistent throughout the function.Verification successful
Verified: The
QuoteInputstruct is correctly defined and used consistently.The
getOriginAmountfunction and other related functions use theQuoteInputstruct correctly. The struct encapsulates all necessary fields and is used consistently across the codebase.
- The
QuoteInputstruct is defined inservices/rfq/relayer/quoter/quoter.go.- It is used in the
generateQuoteandgetOriginAmountfunctions, among others.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the new `QuoteInput` struct in the `getOriginAmount` function. # Test: Search for the usage of `QuoteInput` struct in the `getOriginAmount` function. rg --type go -A 10 'input QuoteInput' services/rfq/relayer/quoter/quoter.goLength of output: 1129
Script:
#!/bin/bash # Description: Verify the definition and usage of the `QuoteInput` struct. # 1. Search for the definition of `QuoteInput`. rg --type go 'type QuoteInput struct' -A 20 # 2. Search for all usages of `QuoteInput` across the codebase. rg --type go 'QuoteInput'Length of output: 2266
Line range hint
404-447:
LGTM! But verify the usage of the new struct.The
generateQuotefunction now accepts aQuoteInputstruct and uses its fields. Ensure that the usage of theQuoteInputstruct is correct and consistent throughout the function.Verification successful
LGTM!
The
generateQuotefunction and related functions use theQuoteInputstruct correctly and consistently.
generateQuotefunction inservices/rfq/relayer/quoter/quoter.gogetOriginAmountfunction inservices/rfq/relayer/quoter/quoter.goGetOriginAmountfunction inservices/rfq/relayer/quoter/export_test.goScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the new `QuoteInput` struct in the `generateQuote` function. # Test: Search for the usage of `QuoteInput` struct in the `generateQuote` function. rg --type go -A 10 'input QuoteInput' services/rfq/relayer/quoter/quoter.goLength of output: 1129
Script:
#!/bin/bash # Description: Verify the usage of the `QuoteInput` struct within the `getOriginAmount` function and any other functions it might be passed to. # Test: Search for the usage of `QuoteInput` struct in the `getOriginAmount` function. rg --type go -A 10 'func (m *Manager) getOriginAmount' services/rfq/relayer/quoter/quoter.go # Test: Search for any other functions that use the `QuoteInput` struct. rg --type go 'input QuoteInput' services/rfq/relayer/quoter/Length of output: 599
There was a problem hiding this comment.
PR Summary
(updates since last review)
The recent changes introduce a MaxBalance parameter to ensure the relayer maintains a maximum balance, preventing over-quoting.
services/rfq/relayer/quoter/quoter.go: Added checks for negative quotable balances and set quote amount to zero in such cases.services/rfq/relayer/relconfig/config.go: RenamedMinBalancetoMaxBalanceinTokenConfigstruct.services/rfq/relayer/relconfig/getters.go: Updated logic to retrieveMaxBalanceand associated comments.services/rfq/relayer/quoter/quoter_test.go: Updated test cases to validate the newMaxBalanceparameter.
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
PR Summary
(updates since last review)
The recent changes introduce a MaxBalance parameter to ensure the relayer maintains a maximum balance, preventing over-quoting.
services/rfq/relayer/quoter/quoter.go: Added checks for negative quotable balances and set quote amount to zero in such cases.services/rfq/relayer/relconfig/config.go: RenamedMinBalancetoMaxBalanceinTokenConfigstruct.services/rfq/relayer/relconfig/getters.go: Updated logic to retrieveMaxBalanceand associated comments.services/rfq/relayer/quoter/quoter_test.go: Updated test cases to validate the newMaxBalanceparameter.
9 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (1)
services/rfq/relayer/quoter/quoter.go (1)
421-421: Error handling: Improve error message clarity.The error message for getting the total fee should be more descriptive.
- return nil, fmt.Errorf("error getting total fee: %w", err) + return nil, fmt.Errorf("error getting total fee for origin chain %d and destination chain %d: %w", input.OriginChainID, input.DestChainID, err)
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (4)
- services/rfq/relayer/quoter/quoter.go (9 hunks)
- services/rfq/relayer/quoter/quoter_test.go (5 hunks)
- services/rfq/relayer/relconfig/config.go (1 hunks)
- services/rfq/relayer/relconfig/getters.go (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- services/rfq/relayer/quoter/quoter_test.go
- services/rfq/relayer/relconfig/config.go
- services/rfq/relayer/relconfig/getters.go
Additional comments not posted (5)
services/rfq/relayer/quoter/quoter.go (5)
585-585: LGTM!The function logic for
deductGasCostis correct.
585-585: LGTM!The function logic for
getDestAmountis correct.
314-314: Function signature update: Verify the usage of the new parameter.The
generateQuotesfunction now includes aninvparameter. Ensure all function calls togenerateQuotesare updated to pass theinvparameter correctly.Verification successful
Function signature update verified: All calls to
generateQuoteshave been updated to include theinvparameter.
services/rfq/relayer/quoter/export_test.goservices/rfq/relayer/quoter/quoter.goScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `generateQuotes` are updated to pass the `inv` parameter. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go -A 5 $'generateQuotes'Length of output: 2109
Line range hint
404-447:
Function signature update: Verify the usage of the new struct.The
generateQuotefunction now accepts aQuoteInputstruct. Ensure all function calls togenerateQuoteare updated to pass theQuoteInputstruct correctly.Verification successful
Function signature update: Verify the usage of the new struct.
The
generateQuotefunction has been correctly updated to use theQuoteInputstruct. All occurrences ofgenerateQuotein the codebase, including thegenerateQuotesfunction, are passing theQuoteInputstruct as expected.
- Verified locations:
services/rfq/relayer/quoter/quoter.go
- Line 404:
func (m *Manager) generateQuote(ctx context.Context, input QuoteInput)Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `generateQuote` are updated to pass the `QuoteInput` struct. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go -A 5 $'generateQuote'Length of output: 3641
Script:
#!/bin/bash # Description: Verify all function calls to `generateQuote` are updated to pass the `QuoteInput` struct. # Test: Search for the function usage and check the parameters passed. Expect: Only occurrences of the new signature with `QuoteInput`. rg --type go 'generateQuote' -A 3Length of output: 2817
Line range hint
490-573:
Function signature update: Verify the usage of the new struct.The
getOriginAmountfunction now accepts aQuoteInputstruct. Ensure all function calls togetOriginAmountare updated to pass theQuoteInputstruct correctly.Verification successful
The script output shows that the
getOriginAmountfunction is being called with theQuoteInputstruct in the following files:
services/rfq/relayer/quoter/quoter.goservices/rfq/relayer/quoter/export_test.goThe function calls in these files are consistent with the updated function signature. Therefore, the usage of the new
QuoteInputstruct is correctly implemented.Function signature update: Verified the usage of the new struct.
- The function
getOriginAmounthas been updated to accept aQuoteInputstruct.- All function calls to
getOriginAmounthave been updated to pass theQuoteInputstruct correctly.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `getOriginAmount` are updated to pass the `QuoteInput` struct. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go -A 5 $'getOriginAmount'Length of output: 2011
| if input.OriginBalance != nil && maxBalance.Cmp(big.NewInt(0)) > 0 { | ||
| quotableBalance := new(big.Int).Sub(maxBalance, input.OriginBalance) |
There was a problem hiding this comment.
Check for negative quotable balance.
Ensure that quotableBalance is not negative before assigning it to quoteAmount.
- if input.OriginBalance != nil && maxBalance.Cmp(big.NewInt(0)) > 0 {
+ if input.OriginBalance != nil && maxBalance.Sign() > 0 {
quotableBalance := new(big.Int).Sub(maxBalance, input.OriginBalance)
if quotableBalance.Cmp(big.NewInt(0)) < 0 {
span.AddEvent("negative quotable balance", trace.WithAttributes(
attribute.String("quotable_balance", quotableBalance.String()),
attribute.String("max_balance", maxBalance.String()),
attribute.String("origin_balance", input.OriginBalance.String()),
))
quoteAmount = big.NewInt(0)
} else if quoteAmount.Cmp(quotableBalance) > 0 {
span.AddEvent("quote amount greater than quotable balance", trace.WithAttributes(
attribute.String("quote_amount", quoteAmount.String()),
attribute.String("quotable_balance", quotableBalance.String()),
attribute.String("max_balance", maxBalance.String()),
attribute.String("origin_balance", input.OriginBalance.String()),
))
quoteAmount = quotableBalance
}
}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 input.OriginBalance != nil && maxBalance.Cmp(big.NewInt(0)) > 0 { | |
| quotableBalance := new(big.Int).Sub(maxBalance, input.OriginBalance) | |
| if input.OriginBalance != nil && maxBalance.Sign() > 0 { | |
| quotableBalance := new(big.Int).Sub(maxBalance, input.OriginBalance) | |
| if quotableBalance.Cmp(big.NewInt(0)) < 0 { | |
| span.AddEvent("negative quotable balance", trace.WithAttributes( | |
| attribute.String("quotable_balance", quotableBalance.String()), | |
| attribute.String("max_balance", maxBalance.String()), | |
| attribute.String("origin_balance", input.OriginBalance.String()), | |
| )) | |
| quoteAmount = big.NewInt(0) | |
| } else if quoteAmount.Cmp(quotableBalance) > 0 { | |
| span.AddEvent("quote amount greater than quotable balance", trace.WithAttributes( | |
| attribute.String("quote_amount", quoteAmount.String()), | |
| attribute.String("quotable_balance", quotableBalance.String()), | |
| attribute.String("max_balance", maxBalance.String()), | |
| attribute.String("origin_balance", input.OriginBalance.String()), | |
| )) | |
| quoteAmount = quotableBalance | |
| } | |
| } |
There was a problem hiding this comment.
PR Summary
(updates since last review)
The recent changes introduce a MaxBalance parameter to ensure the relayer maintains a maximum balance, preventing over-quoting.
services/rfq/relayer/quoter/quoter.go: Enhanced quoting logic to respect both minimum and maximum balance constraints.services/rfq/relayer/quoter/quoter_test.go: Updated test cases to validate the newMaxBalanceparameter.services/rfq/relayer/relconfig/config.go: ModifiedMaxBalancefield inTokenConfigstruct to be a nullable pointer.services/rfq/relayer/relconfig/getters.go: Introduced logic to handle the newMaxBalanceparameter, defaulting tonilif not set.
4 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (4)
- services/rfq/relayer/quoter/quoter.go (9 hunks)
- services/rfq/relayer/quoter/quoter_test.go (5 hunks)
- services/rfq/relayer/relconfig/config.go (1 hunks)
- services/rfq/relayer/relconfig/getters.go (2 hunks)
Files skipped from review as they are similar to previous changes (3)
- services/rfq/relayer/quoter/quoter_test.go
- services/rfq/relayer/relconfig/config.go
- services/rfq/relayer/relconfig/getters.go
Additional comments not posted (8)
services/rfq/relayer/quoter/quoter.go (8)
393-402: Verify the fields of theQuoteInputstruct.The struct consolidates multiple parameters for the
generateQuotefunction. Ensure that all necessary fields are included and properly utilized.Verification successful
Verify the fields of the
QuoteInputstruct.The
QuoteInputstruct consolidates multiple parameters for thegenerateQuotefunction and is used appropriately in the functions where it is passed as an argument. All necessary fields are included and properly utilized.
generateQuotefunctiongetOriginAmountfunctionScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the `QuoteInput` struct to ensure all necessary fields are included and utilized. # Test: Search for the struct usage. Expect: Proper usage in the `generateQuote` function. rg --type go -A 5 $'QuoteInput'Length of output: 3220
314-314: Ensure proper error handling and concurrency management.The function generates quotes in parallel using an errgroup. Ensure that all potential errors are properly handled and that concurrency issues are avoided.
Verification successful
Proper error handling and concurrency management confirmed.
The
generateQuotesfunction properly handles errors and manages concurrency using an errgroup and mutex. Errors during quote generation are logged but do not halt the entire process, ensuring robustness.
- Lines 331-332: Use of
errgroupfor concurrency.- Lines 368-374: Error handling within goroutines.
- Lines 376-378: Mutex for safe access to shared resources.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the error handling and concurrency management in the `generateQuotes` function. # Test: Search for the error handling and concurrency management. Expect: Proper error handling and synchronization. rg --type go -A 10 $'generateQuotes'Length of output: 3120
Script:
#!/bin/bash # Description: Retrieve the full implementation of the `generateQuotes` function from `services/rfq/relayer/quoter/quoter.go` # Use `ast-grep` to extract the full function implementation ast-grep --lang go --pattern $'func (m *Manager) generateQuotes(parentCtx context.Context, chainID int, address common.Address, balance *big.Int, inv map[int]map[common.Address]*big.Int) (quotes []model.PutQuoteRequest, err error) { $$$ }' services/rfq/relayer/quoter/quoter.goLength of output: 6028
490-490: Ensure proper usage of theQuoteInputstruct ingetOriginAmount.The function has been modified to accept the
QuoteInputstruct. Ensure that all fields from the struct are correctly utilized in the function.Verification successful
Ensure proper usage of the
QuoteInputstruct ingetOriginAmount.The function correctly utilizes all fields from the
QuoteInputstruct and includes comprehensive error handling.
- The fields
OriginChainID,DestChainID,DestTokenAddr,OriginTokenAddr,OriginBalance, andDestBalanceare used appropriately.- Error handling is implemented for gas sufficiency checks, quote percentage retrieval, token name retrieval, quote offset application, and gas cost deduction.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the `QuoteInput` struct in the `getOriginAmount` function. # Test: Search for the struct usage within the function. Expect: Proper utilization of all fields. rg --type go -A 10 $'getOriginAmount'Length of output: 2995
Script:
#!/bin/bash # Description: Retrieve the full implementation of the `getOriginAmount` function to verify complete usage of the `QuoteInput` struct and error handling. # Test: Extract the full function implementation. ast-grep --lang go --pattern $'func (m *Manager) getOriginAmount(parentCtx context.Context, input QuoteInput) (quoteAmount *big.Int, err error) { $$$ }'Length of output: 8843
406-406: Ensure proper error handling ingenerateQuote.The function's internal logic has been updated. Ensure that all potential errors are properly handled to avoid unexpected behavior.
Verification successful
Proper error handling is ensured in
generateQuote.The
generateQuotefunction includes comprehensive error handling for all potential errors that may occur during its execution. Each critical operation checks for errors, logs them, and returns them appropriately.
- getOriginAmount: Handles specific
errMinGasExceedsQuoteAmountand other general errors.- GetTokenName: Logs and returns errors.
- GetTotalFee: Logs and returns errors.
- GetRFQAddress: Logs and returns errors.
- getDestAmount: Logs and returns errors.
All errors are properly managed, ensuring robust error handling throughout the function.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the error handling in the `generateQuote` function. # Test: Search for error handling within the function. Expect: Proper error handling for all potential errors. rg --type go -A 10 $'generateQuote'Length of output: 5486
Script:
#!/bin/bash # Description: Extract the full implementation of the `generateQuote` function to verify error handling. # Search for the complete implementation of the `generateQuote` function. ast-grep --lang go --pattern $'func (m *Manager) generateQuote(ctx context.Context, input QuoteInput) (quote *model.PutQuoteRequest, err error) { $$$ }'Length of output: 4037
508-512: Ensure proper error handling ingetOriginAmount.The function's logic has been refined to include checks against both origin and destination balances. Ensure that all potential errors are properly handled.
Verification successful
Error handling in
getOriginAmountis properly implemented.The function includes comprehensive error checks and wraps each error with a descriptive message before returning it. This ensures that all potential errors are properly handled.
services/rfq/relayer/quoter/quoter.go: Lines 490-591Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the error handling in the `getOriginAmount` function. # Test: Search for error handling within the function. Expect: Proper error handling for all potential errors. rg --type go -A 10 $'getOriginAmount'Length of output: 2995
Script:
#!/bin/bash # Description: Retrieve the full implementation of the `getOriginAmount` function for thorough review. # Search for the full implementation of the `getOriginAmount` function in `quoter.go`. ast-grep --lang go --pattern $'func (m *Manager) getOriginAmount(parentCtx context.Context, input QuoteInput) (quoteAmount *big.Int, err error) { $$$ }' --after 100Length of output: 16727
263-263: Verify the usage of the newinvparameter.The new
invparameter is introduced to access balances for origin tokens. Ensure that this parameter is correctly populated and passed from the calling function.
585-585: Ensure correctness of the gas cost deduction logic.The function deducts the gas cost from the quote amount. Ensure that the logic is correct and that all potential errors are properly handled.
Verification successful
Ensure correctness of the gas cost deduction logic.
The function
deductGasCostcorrectly deducts the gas cost from the quote amount. It checks if the address is a gas token, retrieves the minimum gas token balance, and deducts it from the quote amount. Proper error handling is also implemented.
- The function returns the original quote amount if the address is not a gas token.
- It starts a tracing span for monitoring.
- Retrieves the minimum gas token balance and deducts it from the quote amount.
- Handles errors if the minimum gas token balance exceeds the quote amount.
The logic and error handling appear to be correct.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of the gas cost deduction logic in the `deductGasCost` function. # Test: Search for the function implementation. Expect: Correct logic and proper error handling. rg --type go -A 10 $'deductGasCost'Length of output: 1911
Script:
#!/bin/bash # Description: Retrieve the complete implementation of the `deductGasCost` function for full verification. # Search for the function implementation and include more lines for full context. rg --type go -A 50 $'func (m *Manager) deductGasCost(parentCtx context.Context, quoteAmount *big.Int, address common.Address, dest int) (quoteAmountAdj *big.Int, err error)'Length of output: 174
Script:
#!/bin/bash # Description: Retrieve the complete implementation of the `deductGasCost` function for full verification. # Search for the function implementation and include more lines for full context. rg --type go -A 100 $'func (m *Manager) deductGasCost(parentCtx context.Context, quoteAmount *big.Int, address common.Address, dest int) (quoteAmountAdj *big.Int, err error)'Length of output: 175
Script:
#!/bin/bash # Description: Retrieve the complete implementation of the `deductGasCost` function for full verification. # Search for the function implementation using ast-grep for more reliable results. ast-grep --lang go --pattern $'func (m *Manager) deductGasCost(parentCtx context.Context, quoteAmount *big.Int, address common.Address, dest int) (quoteAmountAdj *big.Int, err error) { $$$ }'Length of output: 2560
404-404: Ensure proper usage of theQuoteInputstruct.The function has been refactored to accept a
QuoteInputstruct. Ensure that all fields from the struct are correctly utilized in the function.
There was a problem hiding this comment.
PR Summary
(updates since last review)
The recent changes introduce a MaxBalance parameter to ensure the relayer maintains a maximum balance, preventing over-quoting.
services/rfq/relayer/relconfig/getters.go: AddedGetMaxBalancefunction to retrieve maximum balance for specific tokens.services/rfq/relayer/quoter/quoter.go: Enhanced quoting logic to respect both minimum and maximum balance constraints.services/rfq/relayer/quoter/quoter_test.go: Updated test cases to validate the newMaxBalanceparameter.services/rfq/relayer/relconfig/config.go: ModifiedMaxBalancefield inTokenConfigstruct to be a nullable pointer.
1 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings
| quoteAmountFlt, ok := new(big.Float).SetString(*tokenCfg.MaxBalance) | ||
| if !ok { | ||
| return defaultMaxBalance |
There was a problem hiding this comment.
Logic: Potential issue: If MaxBalance is not a valid string representation of a big.Float, the function defaults to 0 without logging an error.
| if quoteAmountFlt.Cmp(big.NewFloat(0)) <= 0 { | ||
| return defaultMaxBalance | ||
| } |
There was a problem hiding this comment.
Logic: Potential issue: If MaxBalance is less than or equal to 0, the function defaults to 0 without logging an error.
There was a problem hiding this comment.
PR Summary
(updates since last review)
The recent changes introduce a MaxBalance parameter to ensure the relayer maintains a maximum balance, preventing over-quoting.
services/rfq/relayer/quoter/quoter.go: Enhanced quoting logic to respect both minimum and maximum balance constraints.services/rfq/relayer/relconfig/getters.go: AddedGetMaxBalancefunction to retrieve maximum balance for specific tokens.services/rfq/relayer/relconfig/config_test.go: Updated test cases to validate the newMaxBalanceparameter.services/rfq/relayer/quoter/quoter_test.go: Added tests to ensure quoting behavior adheres to the newMaxBalanceconstraints.
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
dwasse
changed the title
There was a problem hiding this comment.
Actionable comments posted: 5
Outside diff range, codebase verification and nitpick comments (4)
services/rfq/relayer/quoter/quoter.go (4)
327-327: Update function signature in the comments.The function signature in the comment should include the new
invparameter.-// Generates quotes for a given chain ID, address, and balance. +// Generates quotes for a given chain ID, address, balance, and inventory.
417-417: Update function signature in the comments.The function signature in the comment should include the new
QuoteInputparameter.-// Calculate the quote amount for this route +// Calculate the quote amount for this route using QuoteInput
503-503: Update function signature in the comments.The function signature in the comment should include the new
QuoteInputparameter.-// getOriginAmount calculates the origin quote amount for a given route. +// getOriginAmount calculates the origin quote amount for a given route using QuoteInput.
589-590: Correct the event message.The event message should be "quote amount greater than destination balance".
- span.AddEvent("quote amount greater than destination balance", trace.WithAttributes( + span.AddEvent("quote amount greater than destination balance", trace.WithAttributes( attribute.String("quote_amount", quoteAmount.String()), attribute.String("balance", input.DestBalance.String()), )) quoteAmount = input.DestBalance }
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (4)
- services/rfq/relayer/quoter/quoter.go (9 hunks)
- services/rfq/relayer/quoter/quoter_test.go (5 hunks)
- services/rfq/relayer/relconfig/config.go (1 hunks)
- services/rfq/relayer/relconfig/getters.go (1 hunks)
Additional comments not posted (8)
services/rfq/relayer/quoter/quoter_test.go (4)
49-50: Verify the new parameter usage.The new parameter
invis introduced but ensure that it is correctly utilized in theGenerateQuotesfunction.
176-182: Verify the new parameter usage.The new parameter
maxBalanceis introduced. Ensure that it is correctly utilized in thesetQuoteParamsfunction.Verification successful
Verify the new parameter usage.
The
maxBalanceparameter is correctly utilized in thesetQuoteParamsfunction. It is assigned to theMaxBalancefield of theoriginTokenCfgconfiguration object.
- File:
services/rfq/relayer/quoter/quoter_test.go- Lines: 176-182
The implementation correctly assigns the
maxBalanceparameter to the configuration, ensuring it is used as intended.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the new parameter `maxBalance` in the `setQuoteParams` function. # Test: Search for the function definition and usage. Expect: Correct usage of the `maxBalance` parameter. rg --type go -A 5 $'func setQuoteParams(quotePct, quoteOffset float64, minQuoteAmount, maxBalance string)'Length of output: 107
Script:
#!/bin/bash # Description: Verify the usage of the new parameter `maxBalance` in the `setQuoteParams` function. # Test 1: Search for the function definition of `setQuoteParams` in the codebase. rg --type go 'func setQuoteParams' # Test 2: Search for the usage of the `setQuoteParams` function to ensure `maxBalance` is correctly utilized. rg --type go 'setQuoteParams'Length of output: 1122
Script:
#!/bin/bash # Description: Verify the implementation of the `setQuoteParams` function to ensure the `maxBalance` parameter is correctly assigned. # Test: Extract the implementation of the `setQuoteParams` function to verify the assignment of `maxBalance`. rg --type go -A 10 'setQuoteParams := func(quotePct, quoteOffset float64, minQuoteAmount, maxBalance string)' services/rfq/relayer/quoter/quoter_test.goLength of output: 154
Script:
#!/bin/bash # Description: Verify the implementation of the `setQuoteParams` function to ensure the `maxBalance` parameter is correctly assigned. # Test: Extract the implementation of the `setQuoteParams` function to verify the assignment of `maxBalance`. rg --type go -A 20 'setQuoteParams := func' services/rfq/relayer/quoter/quoter_test.goLength of output: 1633
25-26: Verify the new parameter usage.The new parameter
invis introduced but ensure that it is correctly utilized in theGenerateQuotesfunction.
176-182: Verify the new struct and parameter usage.The new struct
QuoteInputand the parametermaxBalanceare introduced. Ensure that they are correctly utilized in theGetOriginAmountfunction.Verification successful
Verified the new struct and parameter usage.
The
QuoteInputstruct andmaxBalanceparameter are correctly utilized within thegenerateQuoteandgetOriginAmountfunctions inservices/rfq/relayer/quoter/quoter.go.
QuoteInputstruct is defined and used appropriately.maxBalanceparameter is integrated into the logic as expected.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the new struct `QuoteInput` and the parameter `maxBalance` in the `GetOriginAmount` function. # Test: Search for the function definition and usage. Expect: Correct usage of the `QuoteInput` struct and `maxBalance` parameter. rg --type go -A 5 $'func (m *QuoterManager) GetOriginAmount(ctx context.Context, input quoter.QuoteInput)'Length of output: 107
Script:
#!/bin/bash # Description: Verify the usage of the new struct `QuoteInput` and the parameter `maxBalance` in the `GetOriginAmount` function. # Step 1: Search for the definition of the `QuoteInput` struct. rg --type go 'type QuoteInput struct' # Step 2: Search for the `GetOriginAmount` function definition and its usage. rg --type go -A 10 'func (m *QuoterManager) GetOriginAmount(ctx context.Context, input quoter.QuoteInput)' # Step 3: Search for the usage of the `maxBalance` parameter within the repository. rg --type go 'maxBalance'Length of output: 970
Script:
#!/bin/bash # Description: Verify the usage of the new struct `QuoteInput` within the `GetOriginAmount` function. # Extract the definition and usage of the `GetOriginAmount` function to verify `QuoteInput` usage. rg --type go -A 20 'func (m *QuoterManager) GetOriginAmount(ctx context.Context, input quoter.QuoteInput)' services/rfq/relayer/quoter/quoter.goLength of output: 146
Script:
#!/bin/bash # Description: Verify the usage of the new struct `QuoteInput` and the parameter `maxBalance` in the `GetOriginAmount` function. # Step 1: Broaden the search for the definition of the `GetOriginAmount` function across the entire repository. rg --type go -A 20 'func (m *QuoterManager) GetOriginAmount(ctx context.Context, input quoter.QuoteInput)' # Step 2: Verify the usage of the `QuoteInput` struct within the `GetOriginAmount` function. rg --type go 'QuoteInput' services/rfq/relayer/quoter/quoter.go # Step 3: Verify the usage of the `maxBalance` parameter within the repository. rg --type go 'maxBalance'Length of output: 1284
services/rfq/relayer/relconfig/config.go (1)
136-137: Verify the new field usage.The new field
MaxBalanceis introduced. Ensure that it is correctly utilized in theTokenConfigstruct.Verification successful
Verified the new field usage.
The new field
MaxBalancein theTokenConfigstruct is correctly utilized in the codebase.
services/rfq/relayer/relconfig/config.goservices/rfq/relayer/quoter/quoter_test.goservices/rfq/relayer/quoter/quoter.goservices/rfq/relayer/relconfig/getters.goScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the new field `MaxBalance` in the `TokenConfig` struct. # Test: Search for the field definition and usage. Expect: Correct usage of the `MaxBalance` field. rg --type go -A 5 $'MaxBalance *string `yaml:"max_balance"`'Length of output: 61
Script:
#!/bin/bash # Description: Verify the usage of the new field `MaxBalance` in the `TokenConfig` struct. # Test: Search for any usage of the `MaxBalance` field in the Go files. rg --type go 'MaxBalance'Length of output: 1434
services/rfq/relayer/relconfig/getters.go (1)
393-426: Verify the function logic and error handling.The function
GetMaxBalanceincludes logic to retrieve and process theMaxBalancefield. Ensure that the error handling is robust, especially for the conversion ofMaxBalancefrom a string to abig.Float.Verification successful
Verify the function logic and error handling.
The function
GetMaxBalanceincludes logic to retrieve and process theMaxBalancefield. The usage inquoter.goshows that the function handles cases wheremaxBalanceisniland performs comparisons correctly. The error handling for convertingMaxBalancefrom a string to abig.Floatis also robust.
- File:
services/rfq/relayer/relconfig/getters.go- File:
services/rfq/relayer/quoter/quoter.goScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the logic and error handling in the `GetMaxBalance` function. # Test: Search for the function definition and usage. Expect: Correct logic and error handling. rg --type go -A 5 $'func (c Config) GetMaxBalance(chainID int, addr common.Address) *big.Int'Length of output: 94
Script:
#!/bin/bash # Search for the definition of the GetMaxBalance function in the codebase. rg --type go 'func (c Config) GetMaxBalance(chainID int, addr common.Address) *big.Int' # Search for the usage of the GetMaxBalance function in the codebase. rg --type go 'GetMaxBalance'Length of output: 470
Script:
#!/bin/bash # Extract the implementation of the GetMaxBalance function from getters.go. rg --type go -A 20 'func (c Config) GetMaxBalance(chainID int, addr common.Address) *big.Int' services/rfq/relayer/relconfig/getters.go # Extract the usage of the GetMaxBalance function from quoter.go. rg --type go -A 10 'GetMaxBalance' services/rfq/relayer/quoter/quoter.goLength of output: 820
services/rfq/relayer/quoter/quoter.go (2)
363-367: Check for nil origin balance.Ensure that
originBalanceis not nil before using it in further calculations.- var originBalance *big.Int - originTokens, ok := inv[origin] - if ok { - originBalance = originTokens[originTokenAddr] + originTokens, ok := inv[origin] + if !ok { + span.AddEvent("origin tokens not found", trace.WithAttributes( + attribute.Int("origin", origin), + )) + continue + } + originBalance, ok := originTokens[originTokenAddr] + if !ok { + span.AddEvent("origin token address not found", trace.WithAttributes( + attribute.String("origin_token_addr", originTokenAddr.Hex()), + )) + continue }
446-446: Handle potential errors in getting destination amount.Ensure proper error handling when calling
m.getDestAmount.- destAmount, err := m.getDestAmount(ctx, originAmount, input.DestChainID, destToken) - if err != nil { - logger.Error("Error getting dest amount", "error", err) - return nil, fmt.Errorf("error getting dest amount: %w", err) + destAmount, destErr := m.getDestAmount(ctx, originAmount, input.DestChainID, destToken) + if destErr != nil { + logger.Error("Error getting dest amount", "error", destErr) + return nil, fmt.Errorf("error getting dest amount: %w", destErr) }Likely invalid or redundant comment.
| originAmount, err := m.getOriginAmount(ctx, input) | ||
| // don't quote if gas exceeds quote | ||
| if errors.Is(err, errMinGasExceedsQuoteAmount) { |
There was a problem hiding this comment.
Handle potential errors in getting origin amount.
Ensure proper error handling when calling m.getOriginAmount.
- // don't quote if gas exceeds quote
- if errors.Is(err, errMinGasExceedsQuoteAmount) {
- originAmount = big.NewInt(0)
+ if err != nil {
+ if errors.Is(err, errMinGasExceedsQuoteAmount) {
+ originAmount = big.NewInt(0)
+ } else {
+ logger.Error("Error getting quote amount", "error", err)
+ return nil, err
+ }
}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.
| originAmount, err := m.getOriginAmount(ctx, input) | |
| // don't quote if gas exceeds quote | |
| if errors.Is(err, errMinGasExceedsQuoteAmount) { | |
| originAmount, err := m.getOriginAmount(ctx, input) | |
| // don't quote if gas exceeds quote | |
| if err != nil { | |
| if errors.Is(err, errMinGasExceedsQuoteAmount) { | |
| originAmount = big.NewInt(0) | |
| } else { | |
| logger.Error("Error getting quote amount", "error", err) | |
| return nil, err | |
| } | |
| } |
| destToken, err := m.config.GetTokenName(uint32(input.DestChainID), input.DestTokenAddr.Hex()) | ||
| if err != nil { | ||
| logger.Error("Error getting dest token ID", "error", err) | ||
| return nil, fmt.Errorf("error getting dest token ID: %w", err) | ||
| } | ||
| fee, err := m.feePricer.GetTotalFee(ctx, uint32(origin), uint32(chainID), destToken, true) | ||
| fee, err := m.feePricer.GetTotalFee(ctx, uint32(input.OriginChainID), uint32(input.DestChainID), destToken, true) |
There was a problem hiding this comment.
Handle potential errors in getting token name.
Ensure proper error handling when calling m.config.GetTokenName.
- destToken, err := m.config.GetTokenName(uint32(input.DestChainID), input.DestTokenAddr.Hex())
- if err != nil {
- logger.Error("Error getting dest token ID", "error", err)
- return nil, fmt.Errorf("error getting dest token ID: %w", err)
+ destToken, tokenErr := m.config.GetTokenName(uint32(input.DestChainID), input.DestTokenAddr.Hex())
+ if tokenErr != nil {
+ logger.Error("Error getting dest token ID", "error", tokenErr)
+ return nil, fmt.Errorf("error getting dest token ID: %w", tokenErr)
}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.
| destToken, err := m.config.GetTokenName(uint32(input.DestChainID), input.DestTokenAddr.Hex()) | |
| if err != nil { | |
| logger.Error("Error getting dest token ID", "error", err) | |
| return nil, fmt.Errorf("error getting dest token ID: %w", err) | |
| } | |
| fee, err := m.feePricer.GetTotalFee(ctx, uint32(origin), uint32(chainID), destToken, true) | |
| fee, err := m.feePricer.GetTotalFee(ctx, uint32(input.OriginChainID), uint32(input.DestChainID), destToken, true) | |
| destToken, tokenErr := m.config.GetTokenName(uint32(input.DestChainID), input.DestTokenAddr.Hex()) | |
| if tokenErr != nil { | |
| logger.Error("Error getting dest token ID", "error", tokenErr) | |
| return nil, fmt.Errorf("error getting dest token ID: %w", tokenErr) | |
| } | |
| fee, err := m.feePricer.GetTotalFee(ctx, uint32(input.OriginChainID), uint32(input.DestChainID), destToken, true) |
| } | ||
|
|
||
| // Deduct gas cost from the quote amount, if necessary | ||
| quoteAmount, err = m.deductGasCost(ctx, quoteAmount, address, dest) | ||
| quoteAmount, err = m.deductGasCost(ctx, quoteAmount, input.DestTokenAddr, input.DestChainID) |
There was a problem hiding this comment.
Handle potential errors in deducting gas cost.
Ensure proper error handling when calling m.deductGasCost.
- quoteAmount, err = m.deductGasCost(ctx, quoteAmount, input.DestTokenAddr, input.DestChainID)
- if err != nil {
- return nil, fmt.Errorf("error deducting gas cost: %w", err)
+ quoteAmount, gasErr := m.deductGasCost(ctx, quoteAmount, input.DestTokenAddr, input.DestChainID)
+ if gasErr != nil {
+ return nil, fmt.Errorf("error deducting gas cost: %w", gasErr)
}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.
| quoteAmount, err = m.deductGasCost(ctx, quoteAmount, input.DestTokenAddr, input.DestChainID) | |
| quoteAmount, err = m.deductGasCost(ctx, quoteAmount, input.DestTokenAddr, input.DestChainID) | |
| if err != nil { | |
| return nil, fmt.Errorf("error deducting gas cost: %w", err) | |
| } |
| // Clip the quoteAmount by the max origin balance | ||
| maxBalance := m.config.GetMaxBalance(input.OriginChainID, input.OriginTokenAddr) | ||
| if maxBalance != nil && input.OriginBalance != nil { | ||
| quotableBalance := new(big.Int).Sub(maxBalance, input.OriginBalance) |
There was a problem hiding this comment.
Check for negative quotable balance.
Ensure that quotableBalance is not negative before assigning it to quoteAmount.
- if maxBalance != nil && input.OriginBalance != nil {
- quotableBalance := new(big.Int).Sub(maxBalance, input.OriginBalance)
- if quotableBalance.Cmp(big.NewInt(0)) <= 0 {
+ if maxBalance != nil && input.OriginBalance != nil {
+ quotableBalance := new(big.Int).Sub(maxBalance, input.OriginBalance)
+ if quotableBalance.Cmp(big.NewInt(0)) < 0 {
span.AddEvent("non-positive quotable balance", trace.WithAttributes(
attribute.String("quotable_balance", quotableBalance.String()),
attribute.String("max_balance", maxBalance.String()),
attribute.String("origin_balance", input.OriginBalance.String()),
))
quoteAmount = big.NewInt(0)
} else if quoteAmount.Cmp(quotableBalance) > 0 {
span.AddEvent("quote amount greater than quotable balance", trace.WithAttributes(
attribute.String("quote_amount", quoteAmount.String()),
attribute.String("quotable_balance", quotableBalance.String()),
attribute.String("max_balance", maxBalance.String()),
attribute.String("origin_balance", input.OriginBalance.String()),
))
quoteAmount = quotableBalance
}
}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.
| // Clip the quoteAmount by the max origin balance | |
| maxBalance := m.config.GetMaxBalance(input.OriginChainID, input.OriginTokenAddr) | |
| if maxBalance != nil && input.OriginBalance != nil { | |
| quotableBalance := new(big.Int).Sub(maxBalance, input.OriginBalance) | |
| // Clip the quoteAmount by the max origin balance | |
| maxBalance := m.config.GetMaxBalance(input.OriginChainID, input.OriginTokenAddr) | |
| if maxBalance != nil && input.OriginBalance != nil { | |
| quotableBalance := new(big.Int).Sub(maxBalance, input.OriginBalance) | |
| if quotableBalance.Cmp(big.NewInt(0)) < 0 { | |
| span.AddEvent("non-positive quotable balance", trace.WithAttributes( | |
| attribute.String("quotable_balance", quotableBalance.String()), | |
| attribute.String("max_balance", maxBalance.String()), | |
| attribute.String("origin_balance", input.OriginBalance.String()), | |
| )) | |
| quoteAmount = big.NewInt(0) | |
| } else if quoteAmount.Cmp(quotableBalance) > 0 { | |
| span.AddEvent("quote amount greater than quotable balance", trace.WithAttributes( | |
| attribute.String("quote_amount", quoteAmount.String()), | |
| attribute.String("quotable_balance", quotableBalance.String()), | |
| attribute.String("max_balance", maxBalance.String()), | |
| attribute.String("origin_balance", input.OriginBalance.String()), | |
| )) | |
| quoteAmount = quotableBalance | |
| } | |
| } |
| keyTokenID := k // Parse token info | ||
| originStr := strings.Split(keyTokenID, "-")[0] | ||
| origin, tokenErr := strconv.Atoi(originStr) | ||
| if err != nil { | ||
| span.AddEvent("error converting origin chainID", trace.WithAttributes( | ||
| attribute.String("key_token_id", keyTokenID), | ||
| attribute.String("error", tokenErr.Error()), | ||
| )) | ||
| continue | ||
| } | ||
| originTokenAddr := common.HexToAddress(strings.Split(keyTokenID, "-")[1]) | ||
|
|
||
| var originBalance *big.Int | ||
| originTokens, ok := inv[origin] | ||
| if ok { | ||
| originBalance = originTokens[originTokenAddr] | ||
| } |
There was a problem hiding this comment.
Handle potential errors in parsing token info.
The current implementation parses origin and originTokenAddr without handling potential errors. Ensure proper error handling to avoid unexpected behavior.
- originStr := strings.Split(keyTokenID, "-")[0]
- origin, tokenErr := strconv.Atoi(originStr)
- if err != nil {
+ parts := strings.Split(keyTokenID, "-")
+ if len(parts) != 2 {
+ span.AddEvent("invalid key token ID format", trace.WithAttributes(
+ attribute.String("key_token_id", keyTokenID),
+ ))
+ continue
+ }
+ origin, tokenErr := strconv.Atoi(parts[0])
+ if tokenErr != nil {
span.AddEvent("error converting origin chainID", trace.WithAttributes(
attribute.String("key_token_id", keyTokenID),
attribute.String("error", tokenErr.Error()),
))
continue
}
originTokenAddr := common.HexToAddress(parts[1])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.
| keyTokenID := k // Parse token info | |
| originStr := strings.Split(keyTokenID, "-")[0] | |
| origin, tokenErr := strconv.Atoi(originStr) | |
| if err != nil { | |
| span.AddEvent("error converting origin chainID", trace.WithAttributes( | |
| attribute.String("key_token_id", keyTokenID), | |
| attribute.String("error", tokenErr.Error()), | |
| )) | |
| continue | |
| } | |
| originTokenAddr := common.HexToAddress(strings.Split(keyTokenID, "-")[1]) | |
| var originBalance *big.Int | |
| originTokens, ok := inv[origin] | |
| if ok { | |
| originBalance = originTokens[originTokenAddr] | |
| } | |
| keyTokenID := k // Parse token info | |
| parts := strings.Split(keyTokenID, "-") | |
| if len(parts) != 2 { | |
| span.AddEvent("invalid key token ID format", trace.WithAttributes( | |
| attribute.String("key_token_id", keyTokenID), | |
| )) | |
| continue | |
| } | |
| origin, tokenErr := strconv.Atoi(parts[0]) | |
| if tokenErr != nil { | |
| span.AddEvent("error converting origin chainID", trace.WithAttributes( | |
| attribute.String("key_token_id", keyTokenID), | |
| attribute.String("error", tokenErr.Error()), | |
| )) | |
| continue | |
| } | |
| originTokenAddr := common.HexToAddress(parts[1]) | |
| var originBalance *big.Int | |
| originTokens, ok := inv[origin] | |
| if ok { | |
| originBalance = originTokens[originTokenAddr] | |
| } |
There was a problem hiding this comment.
PR Summary
(updates since last review)
The recent changes introduce a MaxBalance parameter to ensure the relayer maintains a maximum balance, preventing over-quoting. The updates focus on the quoter and relconfig packages, with significant enhancements to quoting logic and test cases.
services/rfq/relayer/quoter/quoter.go: Enhanced quoting logic to respect both minimum and maximum balance constraints.services/rfq/relayer/relconfig/getters.go: AddedGetMaxBalancefunction to retrieve maximum balance for specific tokens.services/rfq/relayer/relconfig/config_test.go: Updated test cases to validate the newMaxBalanceparameter.services/rfq/relayer/quoter/quoter_test.go: Added tests to ensure quoting behavior adheres to the newMaxBalanceconstraints.services/rfq/api/rest/handler.go: IntroducedfilterQuoteAgefunction to filter quotes based on their age, ensuring only recent quotes are returned.
259 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings
| quoteAmountFlt, ok := new(big.Float).SetString(*tokenCfg.MaxBalance) | ||
| if !ok { | ||
| return defaultMaxBalance |
There was a problem hiding this comment.
logic: Potential issue: If MaxBalance is not a valid string representation of a big.Float, the function defaults to 0 without logging an error.
| if quoteAmountFlt.Cmp(big.NewFloat(0)) <= 0 { | ||
| return defaultMaxBalance | ||
| } |
There was a problem hiding this comment.
logic: Potential issue: If MaxBalance is less than or equal to 0, the function defaults to 0 without logging an error.
Summary by CodeRabbit
New Features
Bug Fixes
Tests