RFQ API: add bulk quotes endpoint #2846
Conversation
WalkthroughThe changes introduce a new bulk quote upsertion feature in the RFQ system. An enhanced API now allows relayers to submit multiple quotes in a single request. Adopting this new functionality, the RFQ service has expanded its client, handler, and database interfaces, updated endpoint documentation, altered setup configurations, and introduced new request models. Changes
Sequence Diagram(s)sequenceDiagram
participant Relayer
participant API
participant Handler
participant DB
Relayer->>API: PUT /bulk_quotes
API->>Handler: HandleBulkQuotes
Handler->>DB: UpsertQuotes
DB-->>Handler: Success/Error
Handler-->>API: Status Response
API-->>Relayer: HTTP Response
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 (
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
- Added
PutBulkQuotesmethod toAuthenticatedClientinservices/rfq/api/client/client.go - Introduced
TestPutAndGetBulkQuotesinservices/rfq/api/client/client_test.go - Added
UpsertQuotesmethod toAPIDBWriterinservices/rfq/api/db/api_db.go - Implemented
UpsertQuotesinservices/rfq/api/db/sql/base/store.go - Created
PutBulkQuotesRequeststruct inservices/rfq/api/model/request.go - Added
ModifyBulkQuotesendpoint inservices/rfq/api/rest/handler.go - Defined
/bulk_quotesroute and updatedAuthMiddlewareinservices/rfq/api/rest/server.go - Introduced
submitBulkQuotesmethod inservices/rfq/relayer/quoter/quoter.go - Added
SubmitSingleQuotesconfiguration option inservices/rfq/relayer/relconfig/config.go
9 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Outside diff range and nitpick comments (2)
services/rfq/api/db/sql/base/store.go (1)
68-79: Ensure comprehensive error handling.The method correctly uses
gorm'sOnConflictclause to handle conflicts. However, consider logging the error details for better diagnostics.if dbTx.Error != nil { log.Printf("UpsertQuotes error: %v", dbTx.Error) return fmt.Errorf("could not update quotes: %w", dbTx.Error) }services/rfq/api/rest/handler.go (1)
Line range hint
121-147: Refactor error messages to follow style guidelines.Error messages should not be capitalized.
if err != nil { return nil, fmt.Errorf("Invalid DestAmount") } if err != nil { return nil, fmt.Errorf("Invalid MaxOriginAmount") } if err != nil { return nil, fmt.Errorf("Invalid FixedFee") }if err != nil { return nil, fmt.Errorf("invalid DestAmount") } if err != nil { return nil, fmt.Errorf("invalid MaxOriginAmount") } if err != nil { return nil, fmt.Errorf("invalid FixedFee") }Tools
GitHub Check: Lint (services/rfq)
[failure] 105-105:
G601: Implicit memory aliasing in for loop. (gosec)
[failure] 124-124:
ST1005: error strings should not be capitalized (stylecheck)
[failure] 128-128:
ST1005: error strings should not be capitalized (stylecheck)
[failure] 132-132:
ST1005: error strings should not be capitalized (stylecheck)
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (9)
- services/rfq/api/client/client.go (2 hunks)
- services/rfq/api/client/client_test.go (1 hunks)
- services/rfq/api/db/api_db.go (1 hunks)
- services/rfq/api/db/sql/base/store.go (2 hunks)
- services/rfq/api/model/request.go (1 hunks)
- services/rfq/api/rest/handler.go (3 hunks)
- services/rfq/api/rest/server.go (4 hunks)
- services/rfq/relayer/quoter/quoter.go (2 hunks)
- services/rfq/relayer/relconfig/config.go (1 hunks)
Files skipped from review due to trivial changes (2)
- services/rfq/api/model/request.go
- services/rfq/relayer/relconfig/config.go
Additional context used
GitHub Check: Lint (services/rfq)
services/rfq/api/rest/handler.go
[failure] 105-105:
G601: Implicit memory aliasing in for loop. (gosec)
[failure] 124-124:
ST1005: error strings should not be capitalized (stylecheck)
[failure] 128-128:
ST1005: error strings should not be capitalized (stylecheck)
[failure] 132-132:
ST1005: error strings should not be capitalized (stylecheck)
Additional comments not posted (6)
services/rfq/api/db/api_db.go (1)
53-54: LGTM!The
UpsertQuotesmethod is correctly added to theAPIDBWriterinterface.services/rfq/api/rest/handler.go (1)
58-69: LGTM!The modifications to the
ModifyQuotefunction are correct and improve code readability by using theparseDBQuotefunction.services/rfq/api/client/client.go (1)
29-29: Interface update looks good.The addition of
PutBulkQuotesto theAuthenticatedClientinterface aligns with the new functionality for handling multiple quotes.services/rfq/api/rest/server.go (2)
145-146: Constant definition looks good.The addition of
BulkQuotesRoutealigns with the new functionality for handling bulk quote requests.
Line range hint
196-247: Ensure proper handling of destination chain IDs.The updates in
AuthMiddlewareto handle multiple destination chain IDs for bulk quotes look good. However, ensure that the destination chain IDs are correctly processed and validated.services/rfq/relayer/quoter/quoter.go (1)
273-297: Configuration-based submission looks good.The updates in
prepareAndSubmitQuotesto usesubmitBulkQuotesbased on theSubmitSingleQuotesconfiguration flag are correctly implemented. Ensure that the configuration flag is properly set and validated.
| // PutBulkQuotes puts multiple new quotes in the RFQ quoting API. | ||
| func (c *clientImpl) PutBulkQuotes(ctx context.Context, q *model.PutBulkQuotesRequest) error { | ||
| res, err := c.rClient.R(). | ||
| SetContext(ctx). | ||
| SetBody(q). | ||
| Put(rest.BulkQuotesRoute) | ||
|
|
||
| // TODO: Figure out if there's anything to do with the response, right now it's result: Status Code 200 OK | ||
| _ = res | ||
|
|
||
| return err | ||
| } |
There was a problem hiding this comment.
Ensure proper error handling for the response.
The function PutBulkQuotes is correctly implemented to handle multiple quotes. However, it would be beneficial to handle the response to check for errors or log relevant information.
- // TODO: Figure out if there's anything to do with the response, right now it's result: Status Code 200 OK
- _ = res
+ if res.IsError() {
+ return fmt.Errorf("error from server: %s", res.Status())
+ }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.
| // PutBulkQuotes puts multiple new quotes in the RFQ quoting API. | |
| func (c *clientImpl) PutBulkQuotes(ctx context.Context, q *model.PutBulkQuotesRequest) error { | |
| res, err := c.rClient.R(). | |
| SetContext(ctx). | |
| SetBody(q). | |
| Put(rest.BulkQuotesRoute) | |
| // TODO: Figure out if there's anything to do with the response, right now it's result: Status Code 200 OK | |
| _ = res | |
| return err | |
| } | |
| // PutBulkQuotes puts multiple new quotes in the RFQ quoting API. | |
| func (c *clientImpl) PutBulkQuotes(ctx context.Context, q *model.PutBulkQuotesRequest) error { | |
| res, err := c.rClient.R(). | |
| SetContext(ctx). | |
| SetBody(q). | |
| Put(rest.BulkQuotesRoute) | |
| // TODO: Figure out if there's anything to do with the response, right now it's result: Status Code 200 OK | |
| if res.IsError() { | |
| return fmt.Errorf("error from server: %s", res.Status()) | |
| } | |
| return err | |
| } |
| func (m *Manager) submitBulkQuotes(ctx context.Context, quotes []model.PutQuoteRequest) error { | ||
| quoteCtx, quoteCancel := context.WithTimeout(ctx, m.config.GetQuoteSubmissionTimeout()) | ||
| defer quoteCancel() | ||
|
|
||
| req := model.PutBulkQuotesRequest{ | ||
| Quotes: quotes, | ||
| } | ||
| err := m.rfqClient.PutBulkQuotes(quoteCtx, &req) | ||
| if err != nil { | ||
| return fmt.Errorf("error submitting bulk quotes: %w", err) | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Ensure proper error handling for the response.
The function submitBulkQuotes is correctly implemented to handle bulk quote submission. However, it would be beneficial to handle the response to check for errors or log relevant information.
- if err != nil {
- return fmt.Errorf("error submitting bulk quotes: %w", err)
- }
+ if err != nil || res.IsError() {
+ return fmt.Errorf("error from server: %s, %w", res.Status(), err)
+ }Committable suggestion was skipped due to low confidence.
| func (c *ClientSuite) TestPutAndGetBulkQuotes() { | ||
| req := model.PutBulkQuotesRequest{ | ||
| Quotes: []model.PutQuoteRequest{ | ||
| { | ||
| OriginChainID: 1, | ||
| OriginTokenAddr: "0xOriginTokenAddr", | ||
| DestChainID: 42161, | ||
| DestTokenAddr: "0xDestTokenAddr", | ||
| DestAmount: "100", | ||
| MaxOriginAmount: "200", | ||
| FixedFee: "10", | ||
| }, | ||
| { | ||
| OriginChainID: 42161, | ||
| OriginTokenAddr: "0xOriginTokenAddr", | ||
| DestChainID: 1, | ||
| DestTokenAddr: "0xDestTokenAddr", | ||
| DestAmount: "100", | ||
| MaxOriginAmount: "200", | ||
| FixedFee: "10", | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| err := c.client.PutBulkQuotes(c.GetTestContext(), &req) | ||
| c.Require().NoError(err) | ||
|
|
||
| quotes, err := c.client.GetAllQuotes(c.GetTestContext()) | ||
| c.Require().NoError(err) | ||
|
|
||
| expectedResp := []model.GetQuoteResponse{ | ||
| { | ||
| OriginChainID: 1, | ||
| OriginTokenAddr: "0xOriginTokenAddr", | ||
| DestChainID: 42161, | ||
| DestTokenAddr: "0xDestTokenAddr", | ||
| DestAmount: "100", | ||
| MaxOriginAmount: "200", | ||
| FixedFee: "10", | ||
| RelayerAddr: c.testWallet.Address().String(), | ||
| UpdatedAt: quotes[0].UpdatedAt, | ||
| }, | ||
| { | ||
| OriginChainID: 42161, | ||
| OriginTokenAddr: "0xOriginTokenAddr", | ||
| DestChainID: 1, | ||
| DestTokenAddr: "0xDestTokenAddr", | ||
| DestAmount: "100", | ||
| MaxOriginAmount: "200", | ||
| FixedFee: "10", | ||
| RelayerAddr: c.testWallet.Address().String(), | ||
| UpdatedAt: quotes[0].UpdatedAt, | ||
| }, | ||
| } | ||
| c.Len(quotes, 2) | ||
| c.Equal(expectedResp[0], *quotes[0]) | ||
| c.Equal(expectedResp[1], *quotes[1]) | ||
| } |
There was a problem hiding this comment.
Address implicit memory aliasing.
The loop in TestPutAndGetBulkQuotes contains implicit memory aliasing. To avoid this, create a new variable inside the loop.
for _, quoteReq := range putRequest.Quotes {
dbQuote, err := parseDBQuote("eReq, relayerAddr)
if err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid quote request"})
return
}
dbQuotes = append(dbQuotes, dbQuote)
}for i := range putRequest.Quotes {
quoteReq := putRequest.Quotes[i]
dbQuote, err := parseDBQuote("eReq, relayerAddr)
if err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid quote request"})
return
}
dbQuotes = append(dbQuotes, dbQuote)
}
services/rfq/api/rest/handler.go
Outdated
| // ModifyBulkQuotes upserts multiple quotes | ||
| // | ||
| // PUT /bulk_quotes | ||
| // @dev Protected Method: Authentication is handled through middleware in server.go. | ||
| // nolint: cyclop | ||
| // @Summary Upsert quotes | ||
| // @Schemes | ||
| // @Description upsert bulk quotes from relayer. | ||
| // @Param request body model.PutBulkQuotesRequest true "query params" | ||
| // @Tags quotes | ||
| // @Accept json | ||
| // @Produce json | ||
| // @Success 200 | ||
| // @Router /bulk_quotes [put]. | ||
| func (h *Handler) ModifyBulkQuotes(c *gin.Context) { | ||
| // Retrieve the request from context | ||
| req, exists := c.Get("putRequest") | ||
| if !exists { | ||
| c.JSON(http.StatusBadRequest, gin.H{"error": "Request not found"}) | ||
| return | ||
| } | ||
| relayerAddr, exists := c.Get("relayerAddr") | ||
| if !exists { | ||
| c.JSON(http.StatusBadRequest, gin.H{"error": "No relayer address recovered from signature"}) | ||
| return | ||
| } | ||
| putRequest, ok := req.(*model.PutBulkQuotesRequest) | ||
| if !ok { | ||
| c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid request type"}) | ||
| return | ||
| } | ||
|
|
||
| dbQuotes := []*db.Quote{} | ||
| for _, quoteReq := range putRequest.Quotes { | ||
| dbQuote, err := parseDBQuote("eReq, relayerAddr) | ||
| if err != nil { | ||
| c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid quote request"}) | ||
| return | ||
| } | ||
| dbQuotes = append(dbQuotes, dbQuote) | ||
| } | ||
|
|
||
| err := h.db.UpsertQuotes(c, dbQuotes) | ||
| if err != nil { | ||
| c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid FixedFee"}) | ||
| c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) | ||
| return | ||
| } | ||
| c.Status(http.StatusOK) | ||
| } |
There was a problem hiding this comment.
Address implicit memory aliasing.
The loop in ModifyBulkQuotes contains implicit memory aliasing. To avoid this, create a new variable inside the loop.
for _, quoteReq := range putRequest.Quotes {
dbQuote, err := parseDBQuote("eReq, relayerAddr)
if err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid quote request"})
return
}
dbQuotes = append(dbQuotes, dbQuote)
}for i := range putRequest.Quotes {
quoteReq := putRequest.Quotes[i]
dbQuote, err := parseDBQuote("eReq, relayerAddr)
if err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid quote request"})
return
}
dbQuotes = append(dbQuotes, dbQuote)
}Tools
GitHub Check: Lint (services/rfq)
[failure] 105-105:
G601: Implicit memory aliasing in for loop. (gosec)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2846 +/- ##
===================================================
- Coverage 25.91293% 25.90211% -0.01082%
===================================================
Files 709 709
Lines 52441 52405 -36
Branches 80 80
===================================================
- Hits 13589 13574 -15
+ Misses 37465 37445 -20
+ Partials 1387 1386 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Deploying sanguine-fe with
|
| Latest commit: |
c2c7b6f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://8b09a3a8.sanguine-fe.pages.dev |
| Branch Preview URL: | https://feat-bulk-quote.sanguine-fe.pages.dev |
There was a problem hiding this comment.
PR Summary
(updates since last review)
- Added new endpoint for bulk quote upserts in
services/rfq/api/rest/handler.go - Refactored quote submission logic to support single and bulk submissions
- Modified
parseDBQuotefunction to accept a value instead of a pointer for better request handling
1 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 (1)
- services/rfq/api/rest/handler.go (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- services/rfq/api/rest/handler.go
There was a problem hiding this comment.
PR Summary
(updates since last review)
- Introduced
submitBulkQuotesmethod inservices/rfq/relayer/quoter/quoter.gofor handling multiple quotes in a single request. - Added configuration option to toggle between single and bulk quote submissions in
services/rfq/relayer/quoter/quoter.go.
No major changes found since last review.
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)
- Added
/bulk_quotesendpoint inservices/rfq/api/docs/docs.gofor upserting multiple quotes. - Introduced
model.PutBulkQuotesRequestschema inservices/rfq/api/docs/swagger.json. - Updated
services/rfq/api/docs/swagger.yamlto include new bulk quote functionality.
3 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/api/docs/docs.go (2 hunks)
- services/rfq/api/docs/swagger.json (2 hunks)
- services/rfq/api/docs/swagger.yaml (2 hunks)
- services/rfq/relayer/quoter/quoter.go (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- services/rfq/relayer/quoter/quoter.go
Additional comments not posted (6)
services/rfq/api/docs/swagger.yaml (2)
46-52: Model Definition ApprovedThe
PutBulkQuotesRequestmodel is correctly defined, including an array ofPutQuoteRequestobjects.
97-116: Endpoint Definition ApprovedThe
/bulk_quotesendpoint is correctly defined with a PUT method, including the request body schema and expected responses.services/rfq/api/docs/swagger.json (2)
38-68: Endpoint Definition ApprovedThe
/bulk_quotesendpoint is correctly defined with a PUT method, including the request body schema and expected responses.
207-217: Model Definition ApprovedThe
PutBulkQuotesRequestmodel is correctly defined, including an array ofPutQuoteRequestobjects.services/rfq/api/docs/docs.go (2)
49-79: Endpoint Definition ApprovedThe
/bulk_quotesendpoint is correctly defined with a PUT method, including the request body schema and expected responses.
218-228: Model Definition ApprovedThe
PutBulkQuotesRequestmodel is correctly defined, including an array ofPutQuoteRequestobjects.
There was a problem hiding this comment.
PR Summary
(updates since last review)
- Added
/bulk_quotesendpoint indocs/bridge/docs/rfq/API/upsert-quotes.api.mdxfor bulk quote upsertion. - Updated request and response documentation for the new endpoint.
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
Bundle ReportChanges will increase total bundle size by 13.18MB ⬆️
|
There was a problem hiding this comment.
PR Summary
(updates since last review)
- Moved test database initialization from
SetupSuitetoSetupTestin/services/rfq/api/client/suite_test.go - Moved metrics handler initialization from
SetupSuitetoSetupTestin/services/rfq/api/client/suite_test.go
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (3)
docs/bridge/docs/rfq/API/upsert-quotes.api.mdx (3)
2-4: Improve Description for ClarityThe current description is concise but could be improved for clarity.
- description: "upsert bulk quotes from relayer." + description: "Upsert multiple quotes in bulk from the relayer."
28-28: Add Missing PunctuationA punctuation mark might be missing here.
- as={"h1"} + as={"h1"},Tools
LanguageTool
[uncategorized] ~28-~28: A punctuation mark might be missing here.
Context: ... "@theme/TabItem"; <Heading as={"h1"} className={"openapi__heading"} childre...(AI_EN_LECTOR_MISSING_PUNCTUATION)
45-45: Add Missing PunctuationA punctuation mark might be missing here.
- id={"request"} + id={"request"},Tools
LanguageTool
[uncategorized] ~45-~45: A punctuation mark might be missing here.
Context: ...sert bulk quotes from relayer. <Heading id={"request"} as={"h2"} className={...(AI_EN_LECTOR_MISSING_PUNCTUATION)
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- docs/bridge/docs/rfq/API/upsert-quotes.api.mdx (1 hunks)
- services/rfq/api/client/suite_test.go (2 hunks)
Additional context used
LanguageTool
docs/bridge/docs/rfq/API/upsert-quotes.api.mdx
[uncategorized] ~28-~28: A punctuation mark might be missing here.
Context: ... "@theme/TabItem"; <Heading as={"h1"} className={"openapi__heading"} childre...(AI_EN_LECTOR_MISSING_PUNCTUATION)
[uncategorized] ~45-~45: A punctuation mark might be missing here.
Context: ...sert bulk quotes from relayer. <Heading id={"request"} as={"h2"} className={...(AI_EN_LECTOR_MISSING_PUNCTUATION)
Additional comments not posted (1)
services/rfq/api/client/suite_test.go (1)
61-62: Correct Initialization ofmetricsHandlerThe initialization of
metricsHandleris correct.
| dbType, err := dbcommon.DBTypeFromString("sqlite") | ||
| c.Require().NoError(err) | ||
| // TODO use temp file / in memory sqlite3 to not create in directory files | ||
| testDB, err := sql.Connect(c.GetSuiteContext(), dbType, filet.TmpDir(c.T(), ""), metricsHandler) | ||
| c.Require().NoError(err) | ||
| c.database = testDB |
There was a problem hiding this comment.
Correct Initialization of testDB
The initialization of testDB is correct. However, the TODO comment should be addressed.
- // TODO use temp file / in memory sqlite3 to not create in directory filesConsider using a temporary file or in-memory sqlite3 to avoid creating files in the directory.
Would you like me to address this TODO comment and update the code accordingly?
<!-- This is an auto-generated comment by CodeRabbit -->
Summary by CodeRabbit
/bulk_quotesfor upserting bulk quotes from a relayer.9fbfc16: docs preview link
f026953: docs preview link