Skip to content
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.

Sign up for GitHub

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(rfq-relayer): respond with failure if quote amount is zero [SLT-459] #3391

Merged
merged 5 commits into from
Nov 20, 2024
Merged

fix(rfq-relayer): respond with failure if quote amount is zero [SLT-459] #3391

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
42c0bd0
Feat: return success=false if dest amount is zero
dwasse Nov 13, 2024
cda587c
Feat: add getQuoteResponse helper
dwasse Nov 13, 2024
a27259b
Feat: add test for the no quotes case
dwasse Nov 13, 2024
38dda7f
Cleanup: explicit checks for quote type
dwasse Nov 18, 2024
dea6371
Cleanup: comparisons
dwasse Nov 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions services/rfq/api/rest/rfq_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,15 @@ func (c *ServerSuite) TestActiveRFQFallbackToPassive() {
c.Assert().Equal("passive", userQuoteResp.QuoteType)
c.Assert().Equal("998000", userQuoteResp.DestAmount) // destAmount is quote destAmount minus fixed fee
c.Assert().Equal(c.relayerWallets[0].Address().Hex(), userQuoteResp.RelayerAddress)

// Submit the user quote request with a large origin amount, expect no quotes will be found
userQuoteReq.Data.OriginAmountExact = big.NewInt(1e18).String()
userQuoteResp, err = userClient.PutRFQRequest(c.GetTestContext(), userQuoteReq)
c.Require().NoError(err)

// Assert the response
c.Assert().False(userQuoteResp.Success)
c.Assert().Equal("no quotes found", userQuoteResp.Reason)
}

func (c *ServerSuite) TestActiveRFQPassiveBestQuote() {
Expand Down
33 changes: 25 additions & 8 deletions services/rfq/api/rest/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package rest
import (
"context"
"encoding/json"
"math/big"

"fmt"
"net/http"
Expand Down Expand Up @@ -547,20 +548,35 @@ func (r *QuoterAPIServer) PutRFQRequest(c *gin.Context) {
span.SetAttributes(attribute.String("passive_quote_dest_amount", *passiveQuote.DestAmount))
}
quote := getBestQuote(activeQuote, passiveQuote)
var quoteType string
if quote == activeQuote {
quoteType = quoteTypeActive
} else if quote == passiveQuote {
quoteType = quoteTypePassive
}

// build and return the response
resp := getQuoteResponse(ctx, quote, quoteType)
c.JSON(http.StatusOK, resp)
}

func getQuoteResponse(ctx context.Context, quote *model.QuoteData, quoteType string) (resp model.PutRFQResponse) {
span := trace.SpanFromContext(ctx)

// construct the response
var resp model.PutRFQResponse
if quote == nil {
destAmount := big.NewInt(0)
if quote != nil && quote.DestAmount != nil {
amt, ok := destAmount.SetString(*quote.DestAmount, 10)
if ok {
destAmount = amt
}
}
Comment on lines +568 to +572
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add error handling for SetString operation

The SetString operation's success flag is not properly handled. If parsing fails, the code continues with a zero amount, which could lead to false negatives.

Apply this fix:

 if quote != nil && quote.DestAmount != nil {
-    amt, ok := destAmount.SetString(*quote.DestAmount, 10)
-    if ok {
-        destAmount = amt
-    }
+    if amt, ok := destAmount.SetString(*quote.DestAmount, 10); !ok {
+        span.AddEvent("failed to parse destination amount")
+        return model.PutRFQResponse{
+            Success: false,
+            Reason:  "invalid destination amount",
+        }
+    }
 }

Committable suggestion skipped: line range outside the PR's diff.

if destAmount.Sign() <= 0 {
span.AddEvent("no quotes found")
resp = model.PutRFQResponse{
Success: false,
Reason: "no quotes found",
}
} else {
quoteType := quoteTypeActive
if activeQuote == nil {
quoteType = quoteTypePassive
}
span.SetAttributes(
attribute.String("quote_type", quoteType),
attribute.String("quote_dest_amount", *quote.DestAmount),
Expand All @@ -573,7 +589,8 @@ func (r *QuoterAPIServer) PutRFQRequest(c *gin.Context) {
RelayerAddress: *quote.RelayerAddress,
}
}
c.JSON(http.StatusOK, resp)

return resp
}

func (r *QuoterAPIServer) recordLatestQuoteAge(ctx context.Context, observer metric.Observer) (err error) {
Expand Down
Loading