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

RFQ Quoter: generate quotes in parallel #2847

Merged
merged 6 commits into from
Jul 6, 2024
Merged

RFQ Quoter: generate quotes in parallel #2847

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f293514
Feat: generate quotes in parallel
dwasse Jul 5, 2024
e95d574
[goreleaser]
dwasse Jul 5, 2024
fc0fc60
Cleanup: lint
dwasse Jul 5, 2024
cc9836d
[goreleaser]
dwasse Jul 5, 2024
1546b67
Merge branch 'master' into feat/parallel-quote-gen
dwasse Jul 5, 2024
1c09d00
[goreleaser]
dwasse Jul 5, 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
39 changes: 27 additions & 12 deletions services/rfq/relayer/quoter/quoter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"math/big"
"strconv"
"strings"
"sync"
"sync/atomic"

"github.com/synapsecns/sanguine/contrib/screener-api/client"
Expand Down Expand Up @@ -324,26 +325,40 @@
if err != nil {
return nil, fmt.Errorf("error getting destination RFQ address: %w", err)
}

destTokenID := fmt.Sprintf("%d-%s", chainID, address.Hex())

// generate quotes in parallel
g, gctx := errgroup.WithContext(ctx)
quoteMtx := &sync.Mutex{}
quotes = []model.PutQuoteRequest{}
for keyTokenID, itemTokenIDs := range m.quotableTokens {
for k, itemTokenIDs := range m.quotableTokens {
for _, tokenID := range itemTokenIDs {
//nolint:nestif
if tokenID == destTokenID {
quote, quoteErr := m.generateQuote(ctx, keyTokenID, chainID, address, balance, destRFQAddr)
if quoteErr != nil {
// continue generating quotes even if one fails
span.AddEvent("error generating quote", trace.WithAttributes(
attribute.String("key_token_id", keyTokenID),
attribute.String("error", quoteErr.Error()),
))
continue
}
quotes = append(quotes, *quote)
keyTokenID := k
g.Go(func() error {
quote, quoteErr := m.generateQuote(gctx, keyTokenID, chainID, address, balance, destRFQAddr)
Comment on lines +339 to +340
Copy link
Contributor

Choose a reason for hiding this comment

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

Fix variable scope issue in goroutine.

The variable keyTokenID from the range scope is used in the goroutine, which can lead to incorrect behavior.

-        for _, tokenID := range itemTokenIDs {
-            if tokenID == destTokenID {
-                g.Go(func() error {
-                    quote, quoteErr := m.generateQuote(gctx, keyTokenID, chainID, address, balance, destRFQAddr)
+        for i := range itemTokenIDs {
+            if itemTokenIDs[i] == destTokenID {
+                keyToken := keyTokenID
+                g.Go(func() error {
+                    quote, quoteErr := m.generateQuote(gctx, keyToken, chainID, address, balance, destRFQAddr)
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.

Suggested change
g.Go(func() error {
quote, quoteErr := m.generateQuote(gctx, keyTokenID, chainID, address, balance, destRFQAddr)
for i := range itemTokenIDs {
if itemTokenIDs[i] == destTokenID {
keyToken := keyTokenID
g.Go(func() error {
quote, quoteErr := m.generateQuote(gctx, keyToken, chainID, address, balance, destRFQAddr)
Tools
GitHub Check: Lint (services/rfq)

[failure] 339-339:
Using the variable on range scope keyTokenID in function literal (scopelint)

if quoteErr != nil {
// continue generating quotes even if one fails
span.AddEvent("error generating quote", trace.WithAttributes(
attribute.String("key_token_id", keyTokenID),
Copy link
Contributor

Choose a reason for hiding this comment

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

Fix variable scope issue in goroutine.

The variable keyTokenID from the range scope is used in the goroutine, which can lead to incorrect behavior.

-                        span.AddEvent("error generating quote", trace.WithAttributes(
-                            attribute.String("key_token_id", keyTokenID),
+                        span.AddEvent("error generating quote", trace.WithAttributes(
+                            attribute.String("key_token_id", keyToken),

Committable suggestion was skipped due to low confidence.

Tools
GitHub Check: Lint (services/rfq)

[failure] 343-343:
Using the variable on range scope keyTokenID in function literal (scopelint)

attribute.String("error", quoteErr.Error()),
))
return nil
}

Check warning on line 348 in services/rfq/relayer/quoter/quoter.go

View check run for this annotation

Codecov / codecov/patch

services/rfq/relayer/quoter/quoter.go#L342-L348 

Added lines #L342 - L348 were not covered by tests
quoteMtx.Lock()
defer quoteMtx.Unlock()
quotes = append(quotes, *quote)
return nil
})
}
}
}
err = g.Wait()
if err != nil {
return nil, fmt.Errorf("error generating quotes: %w", err)
}

Check warning on line 360 in services/rfq/relayer/quoter/quoter.go

View check run for this annotation

Codecov / codecov/patch

services/rfq/relayer/quoter/quoter.go#L359-L360 

Added lines #L359 - L360 were not covered by tests

m.currentQuotes = quotes
return quotes, nil
}
Expand Down
Loading