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

feat(rfq-api): active api schema changes #3248

Closed
wants to merge 3 commits into from
+18 −13
Closed

feat(rfq-api): active api schema changes #3248

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
55d0dfd
Feat: deduct fixed fee from dest amount
dwasse Oct 3, 2024
ac59d5e
Fix: correct quote type
dwasse Oct 3, 2024
994cd52
[goreleaser]
dwasse Oct 7, 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
7 changes: 0 additions & 7 deletions services/rfq/api/model/request.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,6 @@ type PutRFQRequest struct {
Data QuoteData `json:"data"`
}

// QuoteRequest represents a request for a quote.
type QuoteRequest struct {
RequestID string `json:"request_id"`
Data QuoteData `json:"data"`
CreatedAt time.Time `json:"created_at"`
}

// QuoteData represents the data within a quote request.
type QuoteData struct {
OriginChainID int `json:"origin_chain_id"`
Expand Down
6 changes: 3 additions & 3 deletions services/rfq/api/rest/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -557,9 +557,9 @@ func (r *QuoterAPIServer) PutRFQRequest(c *gin.Context) {
Reason: "no quotes found",
}
} else {
quoteType := quoteTypeActive
if activeQuote == nil {
quoteType = quoteTypePassive
quoteType := quoteTypePassive
if quote.QuoteID == activeQuote.QuoteID {
quoteType = quoteTypeActive
}
span.SetAttributes(
attribute.String("quote_type", quoteType),
Expand Down
18 changes: 15 additions & 3 deletions services/rfq/relayer/quoter/quoter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,13 +340,25 @@ func (m *Manager) generateActiveRFQ(ctx context.Context, msg *model.ActiveRFQMes
if err != nil {
return nil, fmt.Errorf("error generating quote: %w", err)
}
span.SetAttributes(attribute.String("dest_amount", rawQuote.DestAmount))
fixedFee, ok := new(big.Int).SetString(rawQuote.FixedFee, 10)
if !ok {
return nil, fmt.Errorf("error parsing fixed fee: %s", rawQuote.FixedFee)
}
rawDestAmount, ok := new(big.Int).SetString(rawQuote.DestAmount, 10)
if !ok {
return nil, fmt.Errorf("error parsing dest amount: %s", rawQuote.DestAmount)
}
destAmountNet := new(big.Int).Sub(rawDestAmount, fixedFee)
span.SetAttributes(
attribute.String("dest_amount", rawQuote.DestAmount),
attribute.String("fixed_fee", rawQuote.FixedFee),
attribute.String("dest_amount_net", destAmountNet.String()),
)

rfqResp := model.WsRFQResponse{
RequestID: rfqRequest.RequestID,
DestAmount: rawQuote.DestAmount,
DestAmount: destAmountNet.String(),
Comment on lines +343 to +360
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve error handling and consider using decimal arithmetic for financial calculations.

The changes introduce logic to calculate the net destination amount after deducting a fixed fee. While the implementation is generally correct, there are a few areas that could be improved:

  1. Error handling: The code uses if !ok checks for parsing strings to big.Int. Consider using more descriptive error messages or custom errors for better debugging.

  2. Precision concerns: Financial calculations often require high precision. Consider using a decimal arithmetic library like shopspring/decimal for more accurate calculations, especially when dealing with fees and amounts.

  3. Negative amount check: There's no check to ensure that the destAmountNet doesn't become negative after subtracting the fixed fee. This could lead to unexpected behavior.

Here's a suggested refactoring using the shopspring/decimal library:

import (
    "github.com/shopspring/decimal"
    // ... other imports
)

// ... inside generateActiveRFQ method
fixedFee, err := decimal.NewFromString(rawQuote.FixedFee)
if err != nil {
    return nil, fmt.Errorf("error parsing fixed fee: %w", err)
}

rawDestAmount, err := decimal.NewFromString(rawQuote.DestAmount)
if err != nil {
    return nil, fmt.Errorf("error parsing dest amount: %w", err)
}

destAmountNet := rawDestAmount.Sub(fixedFee)
if destAmountNet.IsNegative() {
    return nil, fmt.Errorf("net destination amount is negative: %s", destAmountNet)
}

span.SetAttributes(
    attribute.String("dest_amount", rawQuote.DestAmount),
    attribute.String("fixed_fee", rawQuote.FixedFee),
    attribute.String("dest_amount_net", destAmountNet.String()),
)

rfqResp := model.WsRFQResponse{
    RequestID:  rfqRequest.RequestID,
    DestAmount: destAmountNet.String(),
}

This refactoring improves precision, error handling, and adds a check for negative net amounts.

}
span.SetAttributes(attribute.String("dest_amount", rawQuote.DestAmount))
respBytes, err := json.Marshal(rfqResp)
if err != nil {
return nil, fmt.Errorf("error serializing response: %w", err)
Expand Down
Loading