RFQ relayer continues generating quotes if one fails #2264
Conversation
|
Warning Rate Limit Exceeded@trajan0x has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 26 minutes and 9 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. WalkthroughThe recent updates introduce enhanced handling for gas balances in the inventory management system and refine the quote generation process for bridgeable tokens. By ensuring gas balance retrieval and streamlining quote generation with a new dedicated function, these changes aim to improve efficiency and accuracy in operations. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2264 +/- ##
===================================================
+ Coverage 47.84597% 47.86207% +0.01610%
===================================================
Files 360 360
Lines 26903 26942 +39
Branches 83 83
===================================================
+ Hits 12872 12895 +23
- Misses 12691 12708 +17
+ Partials 1340 1339 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Deploying with
|
| Latest commit: |
0dca5ef
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://cdd32546.sanguine-fe.pages.dev |
| Branch Preview URL: | https://fix-gas-balance.sanguine-fe.pages.dev |
![coderabbitai[bot]](https://github.com/avatars/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 4
Configuration used: .coderabbit.yaml
Files selected for processing (2)
- services/rfq/relayer/inventory/manager.go (1 hunks)
- services/rfq/relayer/quoter/quoter.go (1 hunks)
Check Runs (10)
codecov/patch completed (8)
- services/rfq/relayer/inventory/manager.go: 87-90: Added lines #L87 - L90 were not covered by tests
- services/rfq/relayer/quoter/quoter.go: 291-295: Added lines #L291 - L295 were not covered by tests
- services/rfq/relayer/quoter/quoter.go: 309-310: Added lines #L309 - L310 were not covered by tests
- services/rfq/relayer/quoter/quoter.go: 319-320: Added lines #L319 - L320 were not covered by tests
- services/rfq/relayer/quoter/quoter.go: 325-326: Added lines #L325 - L326 were not covered by tests
- services/rfq/relayer/quoter/quoter.go: 329-330: Added lines #L329 - L330 were not covered by tests
- services/rfq/relayer/quoter/quoter.go: 333-334: Added lines #L333 - L334 were not covered by tests
- services/rfq/relayer/quoter/quoter.go: 339-340: Added lines #L339 - L340 were not covered by tests
Go Generate (Module Changes) (services/stiprelayer) completed (6)
Go Coverage (1.21.x, ubuntu-latest, services/stiprelayer) completed (4)
Lint (services/rfq) completed (2)
Get PR Metadata completed (2)
Build (1.21.x, ubuntu-latest) completed (2)
Change Detection completed (13)
general-linters completed (1)
label-statuses completed (3)
pr-labeler completed (2)
Additional comments: 1
services/rfq/relayer/quoter/quoter.go (1)
- 268-297: The introduction of the
generateQuotefunction withingenerateQuotesis a significant improvement in handling individual quote generation failures. This change allows the system to continue generating quotes for other tokens even if one fails, which aligns with the PR's objectives. However, it's noted that the error handling within this loop logs the error but does not stop the execution or report it in a way that could be monitored or alerted on. Consider implementing a mechanism to track these failures for operational monitoring and alerting.Consider enhancing error handling by introducing error tracking or alerting mechanisms to monitor individual quote generation failures. This could involve incrementing a metric counter for failures or logging errors to a centralized system where they can be tracked and alerted on.
| gasBalance, ok := i.gasBalances[chainID] | ||
| if !ok || gasBalance == nil { | ||
| return nil, fmt.Errorf("could not get gas balance for chain %d", chainID) | ||
| } |
There was a problem hiding this comment.
The added logic for handling gas balances when the gas token is not found in the inventory tokens map is a good improvement. It ensures that gas balances are properly managed, addressing a potential gap in the system's inventory management capabilities. However, it appears these changes are not covered by tests.
Ensuring test coverage for this new logic is crucial for validating its correctness and reliability. Consider adding unit tests that cover scenarios where the gas token is not found in the inventory tokens map, as well as cases where the gas balance is successfully retrieved or when it fails.
| // continue generating quotes even if one fails | ||
| span.AddEvent("error generating quote", trace.WithAttributes( | ||
| attribute.String("key_token_id", keyTokenID), | ||
| )) | ||
| continue |
There was a problem hiding this comment.
The loop within generateQuotes that continues generating quotes even if one fails is a crucial improvement. However, the added lines were not covered by tests according to the static analysis hints. Ensuring comprehensive test coverage for this new logic is essential for validating its correctness and reliability.
Would you like assistance in writing tests to cover the new logic introduced in the generateQuotes function? This could help ensure the robustness and reliability of the changes.
| return nil, fmt.Errorf("error converting origin chainID: %w", err) | ||
| } |
There was a problem hiding this comment.
Several error handling blocks within the generateQuote function are not covered by tests, as indicated by static analysis hints. It's crucial to have tests that cover these scenarios to ensure the function behaves as expected under various error conditions.
I can help create tests that cover the error handling scenarios within the generateQuote function. Ensuring these paths are tested will improve the overall reliability of the quote generation process.
Also applies to: 319-320, 325-326, 329-330, 333-334, 339-340
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Description
Fixes issue where an error generating an individual quote causes all quotes to fail.
Summary by CodeRabbit