-
Notifications
You must be signed in to change notification settings - Fork 32
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.
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
[SLT-315] refactor(opbot): use gql api #3260
Changes from 4 commits
dadc913
dccd638
c1d21e4
cd2187a
8885dd5
9dc780d
2ef3bbf
6cf146a
451a2d7
6c02517
a932e29
543fa15
50c4ecc
a4cd9aa
3e90715
a40d79d
7c12249
8724af9
9bd0038
a7d5072
4f1f04c
1dbcd96
6b4c78e
51d46c5
d0867c5
7679fde
fce526b
146d28d
111d862
54b1391
0dd607d
7835278
6c7c8f2
56bcab9
5e3f0e2
2e85f89
2829c0f
88f9165
643c0dc
1193c9c
a4997e6
634144a
bc29e4d
72fa23b
69e03b2
f6c9020
1fbfbfa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -11,7 +11,6 @@ | |||||||||||||||
"regexp" | ||||||||||||||||
"sort" | ||||||||||||||||
"strings" | ||||||||||||||||
"sync" | ||||||||||||||||
"time" | ||||||||||||||||
|
||||||||||||||||
"github.com/dustin/go-humanize" | ||||||||||||||||
|
@@ -24,7 +23,6 @@ | |||||||||||||||
"github.com/synapsecns/sanguine/contrib/opbot/signoz" | ||||||||||||||||
"github.com/synapsecns/sanguine/core/retry" | ||||||||||||||||
"github.com/synapsecns/sanguine/ethergo/chaindata" | ||||||||||||||||
"github.com/synapsecns/sanguine/ethergo/client" | ||||||||||||||||
"github.com/synapsecns/sanguine/ethergo/submitter" | ||||||||||||||||
rfqClient "github.com/synapsecns/sanguine/services/rfq/api/client" | ||||||||||||||||
"github.com/synapsecns/sanguine/services/rfq/contracts/fastbridge" | ||||||||||||||||
|
@@ -164,57 +162,12 @@ | |||||||||||||||
"rfq 0x30f96b45ba689c809f7e936c140609eb31c99b182bef54fccf49778716a7e1ca", | ||||||||||||||||
}, | ||||||||||||||||
Handler: func(ctx *slacker.CommandContext) { | ||||||||||||||||
type Status struct { | ||||||||||||||||
relayer string | ||||||||||||||||
*relapi.GetQuoteRequestResponse | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
var statuses []Status | ||||||||||||||||
var sliceMux sync.Mutex | ||||||||||||||||
|
||||||||||||||||
if len(b.cfg.RelayerURLS) == 0 { | ||||||||||||||||
_, err := ctx.Response().Reply("no relayer urls configured") | ||||||||||||||||
if err != nil { | ||||||||||||||||
log.Println(err) | ||||||||||||||||
} | ||||||||||||||||
return | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
tx := stripLinks(ctx.Request().Param("tx")) | ||||||||||||||||
|
||||||||||||||||
var wg sync.WaitGroup | ||||||||||||||||
// 2 routines per relayer, one for tx hashh one for tx id | ||||||||||||||||
wg.Add(len(b.cfg.RelayerURLS) * 2) | ||||||||||||||||
for _, relayer := range b.cfg.RelayerURLS { | ||||||||||||||||
client := relapi.NewRelayerClient(b.handler, relayer) | ||||||||||||||||
go func() { | ||||||||||||||||
defer wg.Done() | ||||||||||||||||
res, err := client.GetQuoteRequestByTxHash(ctx.Context(), tx) | ||||||||||||||||
if err != nil { | ||||||||||||||||
log.Printf("error fetching quote request status by tx hash: %v\n", err) | ||||||||||||||||
return | ||||||||||||||||
} | ||||||||||||||||
sliceMux.Lock() | ||||||||||||||||
defer sliceMux.Unlock() | ||||||||||||||||
statuses = append(statuses, Status{relayer: relayer, GetQuoteRequestResponse: res}) | ||||||||||||||||
}() | ||||||||||||||||
|
||||||||||||||||
go func() { | ||||||||||||||||
defer wg.Done() | ||||||||||||||||
res, err := client.GetQuoteRequestByTXID(ctx.Context(), tx) | ||||||||||||||||
if err != nil { | ||||||||||||||||
log.Printf("error fetching quote request status by tx id: %v\n", err) | ||||||||||||||||
return | ||||||||||||||||
} | ||||||||||||||||
sliceMux.Lock() | ||||||||||||||||
defer sliceMux.Unlock() | ||||||||||||||||
statuses = append(statuses, Status{relayer: relayer, GetQuoteRequestResponse: res}) | ||||||||||||||||
}() | ||||||||||||||||
} | ||||||||||||||||
wg.Wait() | ||||||||||||||||
|
||||||||||||||||
if len(statuses) == 0 { | ||||||||||||||||
_, err := ctx.Response().Reply("no quote request found") | ||||||||||||||||
res, status, err := b.rfqClient.GetRFQByTxID(ctx.Context(), tx) | ||||||||||||||||
if err != nil { | ||||||||||||||||
b.logger.Errorf(ctx.Context(), "error fetching quote request: %v", err) | ||||||||||||||||
_, err := ctx.Response().Reply(fmt.Sprintf("error fetching quote request %s", err.Error())) | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add unit tests for error handling in Lines 167-170 introduce new error handling when fetching the quote request using 🧰 Tools🪛 GitHub Check: codecov/patch
|
||||||||||||||||
if err != nil { | ||||||||||||||||
log.Println(err) | ||||||||||||||||
} | ||||||||||||||||
|
@@ -223,51 +176,44 @@ | |||||||||||||||
|
||||||||||||||||
var slackBlocks []slack.Block | ||||||||||||||||
|
||||||||||||||||
for _, status := range statuses { | ||||||||||||||||
client, err := b.rpcClient.GetChainClient(ctx.Context(), int(status.OriginChainID)) | ||||||||||||||||
if err != nil { | ||||||||||||||||
log.Printf("error getting chain client: %v\n", err) | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
objects := []*slack.TextBlockObject{ | ||||||||||||||||
{ | ||||||||||||||||
Type: slack.MarkdownType, | ||||||||||||||||
Text: fmt.Sprintf("*Relayer*: %s", status.relayer), | ||||||||||||||||
}, | ||||||||||||||||
{ | ||||||||||||||||
Type: slack.MarkdownType, | ||||||||||||||||
Text: fmt.Sprintf("*Status*: %s", status.Status), | ||||||||||||||||
}, | ||||||||||||||||
{ | ||||||||||||||||
Type: slack.MarkdownType, | ||||||||||||||||
Text: fmt.Sprintf("*TxID*: %s", toExplorerSlackLink(status.TxID)), | ||||||||||||||||
}, | ||||||||||||||||
{ | ||||||||||||||||
Type: slack.MarkdownType, | ||||||||||||||||
Text: fmt.Sprintf("*OriginTxHash*: %s", toTXSlackLink(status.OriginTxHash, status.OriginChainID)), | ||||||||||||||||
}, | ||||||||||||||||
{ | ||||||||||||||||
Type: slack.MarkdownType, | ||||||||||||||||
Text: fmt.Sprintf("*Estimated Tx Age*: %s", getTxAge(ctx.Context(), client, status.GetQuoteRequestResponse)), | ||||||||||||||||
}, | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
if status.DestTxHash == (common.Hash{}).String() { | ||||||||||||||||
objects = append(objects, &slack.TextBlockObject{ | ||||||||||||||||
Type: slack.MarkdownType, | ||||||||||||||||
Text: "*DestTxHash*: not available", | ||||||||||||||||
}) | ||||||||||||||||
} else { | ||||||||||||||||
objects = append(objects, &slack.TextBlockObject{ | ||||||||||||||||
Type: slack.MarkdownType, | ||||||||||||||||
Text: fmt.Sprintf("*DestTxHash*: %s", toTXSlackLink(status.DestTxHash, status.DestChainID)), | ||||||||||||||||
}) | ||||||||||||||||
} | ||||||||||||||||
objects := []*slack.TextBlockObject{ | ||||||||||||||||
{ | ||||||||||||||||
Type: slack.MarkdownType, | ||||||||||||||||
Text: fmt.Sprintf("*Relayer*: %s", res.BridgeRelay.Relayer), | ||||||||||||||||
}, | ||||||||||||||||
{ | ||||||||||||||||
Type: slack.MarkdownType, | ||||||||||||||||
Text: fmt.Sprintf("*Status*: %s", status), | ||||||||||||||||
}, | ||||||||||||||||
{ | ||||||||||||||||
Type: slack.MarkdownType, | ||||||||||||||||
Text: fmt.Sprintf("*TxID*: %s", toExplorerSlackLink(res.Bridge.TransactionID)), | ||||||||||||||||
}, | ||||||||||||||||
{ | ||||||||||||||||
Type: slack.MarkdownType, | ||||||||||||||||
Text: fmt.Sprintf("*OriginTxHash*: %s", toTXSlackLink(res.BridgeRequest.TransactionHash, uint32(res.Bridge.OriginChainID))), | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Potential integer overflow when converting chain ID The conversion of Consider implementing a safe conversion function: func safeUint32(v int) uint32 {
if v < 0 || v > math.MaxUint32 {
// Handle error or return a default value
return 0
}
return uint32(v)
} Then use it in the function: - Text: fmt.Sprintf("*OriginTxHash*: %s", toTXSlackLink(res.BridgeRequest.TransactionHash, uint32(res.Bridge.OriginChainID))),
+ Text: fmt.Sprintf("*OriginTxHash*: %s", toTXSlackLink(res.BridgeRequest.TransactionHash, safeUint32(res.Bridge.OriginChainID))), |
||||||||||||||||
}, | ||||||||||||||||
{ | ||||||||||||||||
Type: slack.MarkdownType, | ||||||||||||||||
Text: fmt.Sprintf("*Estimated Tx Age*: %s", getTxAge(res.BridgeRelay.BlockTimestamp)), | ||||||||||||||||
}, | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
slackBlocks = append(slackBlocks, slack.NewSectionBlock(nil, objects, nil)) | ||||||||||||||||
if status == "Requested" { | ||||||||||||||||
objects = append(objects, &slack.TextBlockObject{ | ||||||||||||||||
Type: slack.MarkdownType, | ||||||||||||||||
Text: "*DestTxHash*: not available", | ||||||||||||||||
}) | ||||||||||||||||
} else { | ||||||||||||||||
objects = append(objects, &slack.TextBlockObject{ | ||||||||||||||||
Type: slack.MarkdownType, | ||||||||||||||||
Text: fmt.Sprintf("*DestTxHash*: %s", toTXSlackLink(res.BridgeRelay.TransactionHash, uint32(res.Bridge.DestChainID))), | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Potential integer overflow when converting At line 210, Ensure that
🧰 Tools🪛 GitHub Check: Lint (contrib/opbot)
|
||||||||||||||||
}) | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
_, err := ctx.Response().ReplyBlocks(slackBlocks, slacker.WithUnfurlLinks(false)) | ||||||||||||||||
slackBlocks = append(slackBlocks, slack.NewSectionBlock(nil, objects, nil)) | ||||||||||||||||
|
||||||||||||||||
_, err = ctx.Response().ReplyBlocks(slackBlocks, slacker.WithUnfurlLinks(false)) | ||||||||||||||||
Comment on lines
+216
to
+218
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle errors when sending Slack message blocks When sending Slack message blocks (lines 216-218), if an error occurs, it is logged but not communicated to the user. Consider notifying the user when message delivery fails to enhance error transparency and user experience. 🧰 Tools🪛 GitHub Check: codecov/patch
|
||||||||||||||||
if err != nil { | ||||||||||||||||
log.Println(err) | ||||||||||||||||
} | ||||||||||||||||
|
@@ -292,16 +238,7 @@ | |||||||||||||||
return | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
var rawRequest *relapi.GetQuoteRequestResponse | ||||||||||||||||
var err error | ||||||||||||||||
var relClient relapi.RelayerClient | ||||||||||||||||
for _, relayer := range b.cfg.RelayerURLS { | ||||||||||||||||
relClient = relapi.NewRelayerClient(b.handler, relayer) | ||||||||||||||||
rawRequest, err = getQuoteRequest(ctx.Context(), relClient, tx) | ||||||||||||||||
if err == nil { | ||||||||||||||||
break | ||||||||||||||||
} | ||||||||||||||||
} | ||||||||||||||||
rawRequest, err := b.rfqClient.GetRFQByTxHash(ctx.Context(), tx) | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add unit tests for fetching quote request in Line 243 adds a call to 🧰 Tools🪛 GitHub Check: codecov/patch
|
||||||||||||||||
if err != nil { | ||||||||||||||||
b.logger.Errorf(ctx.Context(), "error fetching quote request: %v", err) | ||||||||||||||||
_, err := ctx.Response().Reply("error fetching quote request") | ||||||||||||||||
|
@@ -364,12 +301,14 @@ | |||||||||||||||
) | ||||||||||||||||
if err != nil { | ||||||||||||||||
b.logger.Errorf(ctx.Context(), "error fetching quote request: %v", err) | ||||||||||||||||
_, err := ctx.Response().Reply(fmt.Sprintf("error fetching explorer link to refund, but nonce is %d", nonce)) | ||||||||||||||||
log.Printf("error fetching quote request: %v\n", err) | ||||||||||||||||
_, err := ctx.Response().Reply(fmt.Sprintf("refund submitted with nonce %d", nonce)) | ||||||||||||||||
if err != nil { | ||||||||||||||||
log.Println(err) | ||||||||||||||||
} | ||||||||||||||||
Comment on lines
+309
to
+312
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Improve error handling after refund submission The current implementation sends a success message even when an error occurs during the status check. This could mislead users about the state of their refund. Consider modifying the error handling to provide more accurate feedback: if err != nil {
b.logger.Errorf(ctx.Context(), "error fetching quote request: %v", err)
- _, err := ctx.Response().Reply(fmt.Sprintf("refund submitted with nonce %d", nonce))
+ _, err := ctx.Response().Reply(fmt.Sprintf("Refund submitted with nonce %d, but there was an error confirming the status. Please check later.", nonce))
if err != nil {
log.Println(err)
}
return
}
🧰 Tools🪛 GitHub Check: codecov/patch
|
||||||||||||||||
return | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
_, err = ctx.Response().Reply(fmt.Sprintf("refund submitted: %s", toExplorerSlackLink(status.TxHash().String()))) | ||||||||||||||||
_, err = ctx.Response().Reply(fmt.Sprintf("refund submitted: %s", toTXSlackLink(status.TxHash().String(), rawRequest.OriginChainID))) | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Handle errors when sending refund transaction link Similar to the previous point, if there's an error when sending the refund transaction link, consider handling it appropriately to enhance user experience and error transparency. 🧰 Tools🪛 GitHub Check: codecov/patch
|
||||||||||||||||
if err != nil { | ||||||||||||||||
log.Println(err) | ||||||||||||||||
} | ||||||||||||||||
|
@@ -405,24 +344,14 @@ | |||||||||||||||
return fastBridgeHandle, nil | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
func getTxAge(ctx context.Context, client client.EVM, res *relapi.GetQuoteRequestResponse) string { | ||||||||||||||||
// TODO: add CreatedAt field to GetQuoteRequestStatusResponse so we don't need to make network calls? | ||||||||||||||||
receipt, err := client.TransactionReceipt(ctx, common.HexToHash(res.OriginTxHash)) | ||||||||||||||||
if err != nil { | ||||||||||||||||
return "unknown time ago" | ||||||||||||||||
} | ||||||||||||||||
txBlock, err := client.HeaderByHash(ctx, receipt.BlockHash) | ||||||||||||||||
if err != nil { | ||||||||||||||||
return "unknown time ago" | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
return humanize.Time(time.Unix(int64(txBlock.Time), 0)) | ||||||||||||||||
func getTxAge(timestamp int64) string { | ||||||||||||||||
return humanize.Time(time.Unix(timestamp, 0)) | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
func toExplorerSlackLink(ogHash string) string { | ||||||||||||||||
rfqHash := strings.ToUpper(ogHash) | ||||||||||||||||
// cut off 0x | ||||||||||||||||
if strings.HasPrefix(rfqHash, "0x") { | ||||||||||||||||
if strings.HasPrefix(rfqHash, "0X") { | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure consistent hash prefix handling in The current code checks for the "0X" prefix but does not handle the lowercase "0x" prefix. Consider updating the condition to handle both cases to prevent inconsistent behavior with different hash formats. Apply this diff to address the issue: if strings.HasPrefix(rfqHash, "0X") || strings.HasPrefix(rfqHash, "0x") {
rfqHash = rfqHash[2:]
}
rfqHash = strings.ToLower(rfqHash)
🧰 Tools🪛 GitHub Check: codecov/patch
|
||||||||||||||||
rfqHash = strings.ToLower(rfqHash[2:]) | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
Comment on lines
+357
to
360
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Improve hash prefix handling in toExplorerSlackLink The current implementation only handles the uppercase "0X" prefix. To ensure consistent behavior with different hash formats, consider updating the condition to handle both uppercase and lowercase prefixes. Apply this diff to address the issue: - if strings.HasPrefix(rfqHash, "0X") {
+ if strings.HasPrefix(rfqHash, "0X") || strings.HasPrefix(rfqHash, "0x") {
rfqHash = strings.ToLower(rfqHash[2:])
} This change will ensure that both "0X" and "0x" prefixes are handled consistently. 📝 Committable suggestion
Suggested change
🧰 Tools🪛 GitHub Check: codecov/patch
|
||||||||||||||||
|
@@ -444,16 +373,3 @@ | |||||||||||||||
linkRegex := regexp.MustCompile(`<https?://[^|>]+\|([^>]+)>`) | ||||||||||||||||
return linkRegex.ReplaceAllString(input, "$1") | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
func getQuoteRequest(ctx context.Context, client relapi.RelayerClient, tx string) (qr *relapi.GetQuoteRequestResponse, err error) { | ||||||||||||||||
if qr, err = client.GetQuoteRequestByTxHash(ctx, tx); err == nil { | ||||||||||||||||
return qr, nil | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
// look up quote request | ||||||||||||||||
if qr, err = client.GetQuoteRequestByTXID(ctx, tx); err == nil { | ||||||||||||||||
return qr, nil | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
return nil, fmt.Errorf("error fetching quote request: %w", err) | ||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,16 +1,5 @@ | ||
package botmd | ||
|
||
import ( | ||
"context" | ||
|
||
"github.com/synapsecns/sanguine/ethergo/client" | ||
"github.com/synapsecns/sanguine/services/rfq/relayer/relapi" | ||
) | ||
|
||
func StripLinks(input string) string { | ||
return stripLinks(input) | ||
} | ||
|
||
func GetTxAge(ctx context.Context, client client.EVM, res *relapi.GetQuoteRequestResponse) string { | ||
return getTxAge(ctx, client, res) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,6 +38,8 @@ type Config struct { | |
ScreenerURL string `yaml:"screener_url"` | ||
// Database is the database config. | ||
Database DatabaseConfig `yaml:"database"` | ||
// RFQIndexerAPIURL is the URL of the RFQ indexer API. | ||
RFQIndexerAPIURL string `yaml:"rfq_indexer_api_url"` | ||
Comment on lines
+41
to
+42
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider enhancing the new The addition of the
Here's a suggested improvement for the field declaration: + // RFQIndexerAPIURL is the URL of the RFQ indexer API.
RFQIndexerAPIURL string `yaml:"rfq_indexer_api_url"` Consider updating the func (c *Config) Validate() error {
// ... existing validations ...
if c.RFQIndexerAPIURL == "" {
return errors.New("rfq_indexer_api_url is required")
}
return nil
} |
||
} | ||
|
||
// DatabaseConfig represents the configuration for the database. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Issue Found: Missing
RFQIndexerAPIURL
inconfig.Config
The
RFQIndexerAPIURL
field is missing in theconfig.Config
struct, which is required for the proper initialization ofrfqClient
.contrib/opbot/config/config.go
🔗 Analysis chain
LGTM: Initialization of rfqClient and improved formatting
The initialization of the
rfqClient
in theNewBot
function is correct and aligns with the new field added to theBot
struct. The reformatting of the struct initialization improves readability.Please ensure that the
RFQIndexerAPIURL
andRelayerURLS
fields have been added to theconfig.Config
struct. Run the following script to verify:🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 240