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.

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

Open
wants to merge 32 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
dadc913
use gql api
golangisfun123 Oct 9, 2024
dccd638
remove old test
golangisfun123 Oct 9, 2024
c1d21e4
[goreleaser]
golangisfun123 Oct 10, 2024
cd2187a
fix lint[goreleaser]
golangisfun123 Oct 10, 2024
8885dd5
[goreleaser]
golangisfun123 Oct 10, 2024
9dc780d
fix lint [goreleaser]
golangisfun123 Oct 10, 2024
2ef3bbf
explorer UI updated
Defi-Moses Oct 10, 2024
6cf146a
rfq-indexer update
Defi-Moses Oct 10, 2024
451a2d7
explorer backend update
Defi-Moses Oct 10, 2024
6c02517
[goreleaser] trigger explorer version bump
Defi-Moses Oct 10, 2024
a932e29
rfq indexer with the right contracts
Defi-Moses Oct 10, 2024
543fa15
fix logic
golangisfun123 Oct 10, 2024
50c4ecc
[goreleaser]
golangisfun123 Oct 10, 2024
a4cd9aa
fix error msg
golangisfun123 Oct 10, 2024
3e90715
[goreleaser] adding catch
Defi-Moses Oct 10, 2024
a40d79d
Merge branch 'master' of github.com:synapsecns/sanguine into opbot-mi…
golangisfun123 Oct 11, 2024
7c12249
remove function
golangisfun123 Oct 11, 2024
8724af9
Merge branch 'master' into feat/explorer-w
Defi-Moses Oct 12, 2024
9bd0038
response error fixes and wld decimals
Defi-Moses Oct 14, 2024
a7d5072
Merge branch 'master' into feat/explorer-w
Defi-Moses Oct 14, 2024
4f1f04c
origin tx hash
golangisfun123 Oct 14, 2024
1dbcd96
adding address
Defi-Moses Oct 15, 2024
6b4c78e
feat(rfq-indexer): add `request` column to `BridgeRequested` for refu…
golangisfun123 Oct 16, 2024
51d46c5
Merge branch 'master' of github.com:synapsecns/sanguine into opbot-mi…
golangisfun123 Oct 17, 2024
d0867c5
idr
golangisfun123 Oct 17, 2024
7679fde
Merge branch 'feat/explorer-w' of github.com:synapsecns/sanguine into…
golangisfun123 Oct 17, 2024
fce526b
merge
golangisfun123 Oct 17, 2024
146d28d
refund from rfq indexer
golangisfun123 Oct 17, 2024
111d862
add request field
golangisfun123 Oct 17, 2024
54b1391
yarn lcok
golangisfun123 Oct 17, 2024
0dd607d
Revert "yarn lcok"
golangisfun123 Oct 17, 2024
7835278
Revert "add request field"
golangisfun123 Oct 17, 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
11 changes: 7 additions & 4 deletions contrib/opbot/botmd/botmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

"github.com/slack-io/slacker"
"github.com/synapsecns/sanguine/contrib/opbot/config"
"github.com/synapsecns/sanguine/contrib/opbot/internal"
"github.com/synapsecns/sanguine/contrib/opbot/signoz"
screenerClient "github.com/synapsecns/sanguine/contrib/screener-api/client"
"github.com/synapsecns/sanguine/core/dbcommon"
Expand All @@ -29,6 +30,7 @@
signozClient *signoz.Client
signozEnabled bool
rpcClient omnirpcClient.RPCClient
rfqClient internal.RFQClient
signer signer.Signer
submitter submitter.TransactionSubmitter
screener screenerClient.ScreenerClient
Expand All @@ -42,10 +44,11 @@
sugaredLogger := otelzap.New(experimentalLogger.MakeZapLogger()).Sugar()

bot := Bot{
handler: handler,
cfg: cfg,
server: server,
logger: experimentalLogger.MakeWrappedSugaredLogger(sugaredLogger),
handler: handler,
cfg: cfg,
server: server,
logger: experimentalLogger.MakeWrappedSugaredLogger(sugaredLogger),
rfqClient: internal.NewRFQClient(handler, cfg.RFQIndexerAPIURL, cfg.RelayerURLS),

Check warning on line 51 in contrib/opbot/botmd/botmd.go

View check run for this annotation

Codecov / codecov/patch

contrib/opbot/botmd/botmd.go#L47-L51

Added lines #L47 - L51 were not covered by tests
Comment on lines +47 to +51
Copy link
Contributor

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 in config.Config

The RFQIndexerAPIURL field is missing in the config.Config struct, which is required for the proper initialization of rfqClient.

  • File: contrib/opbot/config/config.go
🔗 Analysis chain

LGTM: Initialization of rfqClient and improved formatting

The initialization of the rfqClient in the NewBot function is correct and aligns with the new field added to the Bot struct. The reformatting of the struct initialization improves readability.

Please ensure that the RFQIndexerAPIURL and RelayerURLS fields have been added to the config.Config struct. Run the following script to verify:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the presence of new config fields

# Test: Check for RFQIndexerAPIURL and RelayerURLS in the config struct
rg --type go 'type\s+Config\s+struct' -A 20 | rg 'RFQIndexerAPIURL|RelayerURLS'

Length of output: 240

}

// you should be able to run opbot even without signoz.
Expand Down
213 changes: 65 additions & 148 deletions contrib/opbot/botmd/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
"regexp"
"sort"
"strings"
"sync"
"time"

"github.com/dustin/go-humanize"
Expand All @@ -21,14 +20,13 @@
"github.com/hako/durafmt"
"github.com/slack-go/slack"
"github.com/slack-io/slacker"
"github.com/synapsecns/sanguine/contrib/opbot/internal"
"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"
"github.com/synapsecns/sanguine/services/rfq/relayer/relapi"
)

func (b *Bot) requiresSignoz(definition *slacker.CommandDefinition) *slacker.CommandDefinition {
Expand Down Expand Up @@ -159,62 +157,17 @@
func (b *Bot) rfqLookupCommand() *slacker.CommandDefinition {
return &slacker.CommandDefinition{
Command: "rfq <tx>",
Description: "find a rfq transaction by either tx hash or txid on all configured relayers",
Description: "find a rfq transaction by either tx hash or txid from the rfq-indexer api",

Check warning on line 160 in contrib/opbot/botmd/commands.go

View check run for this annotation

Codecov / codecov/patch

contrib/opbot/botmd/commands.go#L160

Added line #L160 was not covered by tests
Examples: []string{
"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.GetRFQ(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()))

Check warning on line 170 in contrib/opbot/botmd/commands.go

View check run for this annotation

Codecov / codecov/patch

contrib/opbot/botmd/commands.go#L167-L170

Added lines #L167 - L170 were not covered by tests
if err != nil {
log.Println(err)
}
Expand All @@ -223,51 +176,46 @@

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,
//nolint: gosec
Text: fmt.Sprintf("*OriginTxHash*: %s", toTXSlackLink(res.BridgeRequest.TransactionHash, uint32(res.Bridge.OriginChainID))),
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

Potential integer overflow when converting int to uint32

At line 194, res.Bridge.OriginChainID is being converted from int to uint32 using uint32(res.Bridge.OriginChainID). This conversion can lead to an integer overflow if res.Bridge.OriginChainID exceeds the maximum value of a uint32 (4,294,967,295). Such an overflow could result in incorrect chain IDs and unexpected behavior.

Consider verifying that res.Bridge.OriginChainID always falls within the uint32 range. If there is a possibility of higher values, you might:

  • Use a larger unsigned integer type like uint64.
  • Modify the toTXSlackLink function to accept an int or int64 to avoid unnecessary and potentially unsafe type conversions.
🧰 Tools
🪛 GitHub Check: Lint (contrib/opbot)

[failure] 194-194:
G115: integer overflow conversion int -> uint32 (gosec)

},
{
Type: slack.MarkdownType,
Text: fmt.Sprintf("*Estimated Tx Age*: %s", humanize.Time(time.Unix(res.BridgeRelay.BlockTimestamp, 0))),
},
}

Check warning on line 201 in contrib/opbot/botmd/commands.go

View check run for this annotation

Codecov / codecov/patch

contrib/opbot/botmd/commands.go#L179-L201

Added lines #L179 - L201 were not covered by tests
Comment on lines +179 to +201
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

Improved Slack message construction with potential overflow risk

The new approach to constructing Slack messages using TextBlockObjects improves readability and maintainability. However, there's a potential integer overflow risk when converting res.Bridge.OriginChainID to uint32 on line 195.

Consider using a safer conversion method or ensuring that the chain ID is within the valid range for uint32:

- Text: fmt.Sprintf("*OriginTxHash*: %s", toTXSlackLink(res.BridgeRequest.TransactionHash, uint32(res.Bridge.OriginChainID))),
+ Text: fmt.Sprintf("*OriginTxHash*: %s", toTXSlackLink(res.BridgeRequest.TransactionHash, safeCastToUint32(res.Bridge.OriginChainID))),

You'll need to implement a safeCastToUint32 function that handles potential overflows.

Committable suggestion was skipped due to low confidence.


slackBlocks = append(slackBlocks, slack.NewSectionBlock(nil, objects, nil))
if status == "Requested" {
objects = append(objects, &slack.TextBlockObject{
Type: slack.MarkdownType,
Text: "*DestTxHash*: not available",
})
} else {
//nolint: gosec
objects = append(objects, &slack.TextBlockObject{
Type: slack.MarkdownType,
Text: fmt.Sprintf("*DestTxHash*: %s", toTXSlackLink(res.BridgeRelay.TransactionHash, uint32(res.Bridge.DestChainID))),
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

Potential integer overflow when converting int to uint32

At line 210, res.Bridge.DestChainID is being converted from int to uint32 using uint32(res.Bridge.DestChainID). Similar to the issue at line 194, this could cause an integer overflow if res.Bridge.DestChainID exceeds the uint32 maximum value, leading to incorrect chain IDs.

Ensure that res.Bridge.DestChainID is within the uint32 range. If not, consider:

  • Switching to a larger unsigned integer type like uint64.
  • Adjusting the toTXSlackLink function to accept an int or int64 to safely handle larger chain ID values without risking overflow.
🧰 Tools
🪛 GitHub Check: Lint (contrib/opbot)

[failure] 210-210:
G115: integer overflow conversion int -> uint32 (gosec)

})

Check warning on line 213 in contrib/opbot/botmd/commands.go

View check run for this annotation

Codecov / codecov/patch

contrib/opbot/botmd/commands.go#L203-L213

Added lines #L203 - L213 were not covered by tests
Comment on lines +203 to +213
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 unit tests for conditional handling of DestTxHash

Lines 203-213 introduce conditional logic to handle the presence or absence of DestTxHash based on the status. This conditional logic is not currently covered by unit tests. Consider adding tests to verify that the correct message is displayed for each possible status.

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 203-213: contrib/opbot/botmd/commands.go#L203-L213
Added lines #L203 - L213 were not covered by tests

}

_, err := ctx.Response().ReplyBlocks(slackBlocks, slacker.WithUnfurlLinks(false))
slackBlocks = append(slackBlocks, slack.NewSectionBlock(nil, objects, nil))

_, err = ctx.Response().ReplyBlocks(slackBlocks, slacker.WithUnfurlLinks(false))

Check warning on line 218 in contrib/opbot/botmd/commands.go

View check run for this annotation

Codecov / codecov/patch

contrib/opbot/botmd/commands.go#L216-L218

Added lines #L216 - L218 were not covered by tests
Comment on lines +216 to +218
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

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

[warning] 216-218: contrib/opbot/botmd/commands.go#L216-L218
Added lines #L216 - L218 were not covered by tests

Comment on lines +203 to +218
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

Status-based message customization with DRY opportunity

The conditional logic for handling different RFQ statuses improves the user experience by providing relevant information. However, there are two areas for improvement:

  1. DRY Principle: The code for adding TextBlockObjects is repeated. Consider refactoring to a helper function:
func addTextBlockObject(objects []*slack.TextBlockObject, text string) []*slack.TextBlockObject {
    return append(objects, &slack.TextBlockObject{
        Type: slack.MarkdownType,
        Text: text,
    })
}
  1. Integer Overflow: There's another potential integer overflow when converting res.Bridge.DestChainID to uint32 on line 212. Apply the same safe casting solution as suggested for the OriginChainID.

if err != nil {
log.Println(err)
}
Expand All @@ -292,16 +240,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.GetRFQ(ctx.Context(), tx)

Check warning on line 243 in contrib/opbot/botmd/commands.go

View check run for this annotation

Codecov / codecov/patch

contrib/opbot/botmd/commands.go#L243

Added line #L243 was not covered by tests
if err != nil {
b.logger.Errorf(ctx.Context(), "error fetching quote request: %v", err)
_, err := ctx.Response().Reply("error fetching quote request")
Expand All @@ -320,7 +259,7 @@
return
}

isScreened, err := b.screener.ScreenAddress(ctx.Context(), rawRequest.Sender)
isScreened, err := b.screener.ScreenAddress(ctx.Context(), rawRequest.Bridge.Sender)

Check warning on line 262 in contrib/opbot/botmd/commands.go

View check run for this annotation

Codecov / codecov/patch

contrib/opbot/botmd/commands.go#L262

Added line #L262 was not covered by tests
if err != nil {
_, err := ctx.Response().Reply("error screening address")
if err != nil {
Expand All @@ -336,13 +275,16 @@
return
}

nonce, err := b.submitter.SubmitTransaction(ctx.Context(), big.NewInt(int64(rawRequest.OriginChainID)), func(transactor *bind.TransactOpts) (tx *types.Transaction, err error) {
tx, err = fastBridgeContract.Refund(transactor, common.Hex2Bytes(rawRequest.QuoteRequestRaw))
if err != nil {
return nil, fmt.Errorf("error submitting refund: %w", err)
}
return tx, nil
})
nonce, err := b.submitter.SubmitTransaction(
ctx.Context(),
big.NewInt(int64(rawRequest.Bridge.OriginChainID)),
func(transactor *bind.TransactOpts) (tx *types.Transaction, err error) {
tx, err = fastBridgeContract.Refund(transactor, common.Hex2Bytes(rawRequest.BridgeRequest.Request))
if err != nil {
return nil, fmt.Errorf("error submitting refund: %w", err)
}
return tx, nil

Check warning on line 286 in contrib/opbot/botmd/commands.go

View check run for this annotation

Codecov / codecov/patch

contrib/opbot/botmd/commands.go#L278-L286

Added lines #L278 - L286 were not covered by tests
})
if err != nil {
log.Printf("error submitting refund: %v\n", err)
return
Expand All @@ -352,7 +294,7 @@
err = retry.WithBackoff(
ctx.Context(),
func(ctx context.Context) error {
status, err = b.submitter.GetSubmissionStatus(ctx, big.NewInt(int64(rawRequest.OriginChainID)), nonce)
status, err = b.submitter.GetSubmissionStatus(ctx, big.NewInt(int64(rawRequest.Bridge.OriginChainID)), nonce)

Check warning on line 297 in contrib/opbot/botmd/commands.go

View check run for this annotation

Codecov / codecov/patch

contrib/opbot/botmd/commands.go#L297

Added line #L297 was not covered by tests
if err != nil || !status.HasTx() {
b.logger.Errorf(ctx, "error fetching quote request: %v", err)
return fmt.Errorf("error fetching quote request: %w", err)
Expand All @@ -364,20 +306,22 @@
)
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)
}

Check warning on line 312 in contrib/opbot/botmd/commands.go

View check run for this annotation

Codecov / codecov/patch

contrib/opbot/botmd/commands.go#L309-L312

Added lines #L309 - L312 were not covered by tests
Comment on lines +309 to +312
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 when sending refund confirmation

If an error occurs when replying to the refund submission, the code currently logs the error but does not inform the user. Consider adding error handling or a retry mechanism to ensure the user is aware of any issues.

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 306-309: contrib/opbot/botmd/commands.go#L306-L309
Added lines #L306 - L309 were not covered by tests

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(), uint32(rawRequest.Bridge.OriginChainID))))

Check failure on line 316 in contrib/opbot/botmd/commands.go

View workflow job for this annotation

GitHub Actions / Lint (contrib/opbot)

G115: integer overflow conversion int -> uint32 (gosec)

Check warning on line 316 in contrib/opbot/botmd/commands.go

View check run for this annotation

Codecov / codecov/patch

contrib/opbot/botmd/commands.go#L316

Added line #L316 was not covered by tests
if err != nil {
log.Println(err)
}
},
}
}

func (b *Bot) makeFastBridge(ctx context.Context, req *relapi.GetQuoteRequestResponse) (*fastbridge.FastBridge, error) {
func (b *Bot) makeFastBridge(ctx context.Context, req *internal.GetRFQByTxIDResponse) (*fastbridge.FastBridge, error) {

Check warning on line 324 in contrib/opbot/botmd/commands.go

View check run for this annotation

Codecov / codecov/patch

contrib/opbot/botmd/commands.go#L324

Added line #L324 was not covered by tests
client, err := rfqClient.NewUnauthenticatedClient(b.handler, b.cfg.RFQApiURL)
if err != nil {
return nil, fmt.Errorf("error creating rfq client: %w", err)
Expand All @@ -388,12 +332,12 @@
return nil, fmt.Errorf("error fetching rfq contracts: %w", err)
}

chainClient, err := b.rpcClient.GetChainClient(ctx, int(req.OriginChainID))
chainClient, err := b.rpcClient.GetChainClient(ctx, int(req.Bridge.OriginChainID))

Check failure on line 335 in contrib/opbot/botmd/commands.go

View workflow job for this annotation

GitHub Actions / Lint (contrib/opbot)

unnecessary conversion (unconvert)

Check warning on line 335 in contrib/opbot/botmd/commands.go

View check run for this annotation

Codecov / codecov/patch

contrib/opbot/botmd/commands.go#L335

Added line #L335 was not covered by tests
if err != nil {
return nil, fmt.Errorf("error getting chain client: %w", err)
}

contractAddress, ok := contracts.Contracts[req.OriginChainID]
contractAddress, ok := contracts.Contracts[uint32(req.Bridge.OriginChainID)]

Check failure on line 340 in contrib/opbot/botmd/commands.go

View workflow job for this annotation

GitHub Actions / Lint (contrib/opbot)

G115: integer overflow conversion int -> uint32 (gosec)

Check warning on line 340 in contrib/opbot/botmd/commands.go

View check run for this annotation

Codecov / codecov/patch

contrib/opbot/botmd/commands.go#L340

Added line #L340 was not covered by tests
if !ok {
return nil, errors.New("contract address not found")
}
Expand All @@ -405,24 +349,10 @@
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 toExplorerSlackLink(ogHash string) string {
rfqHash := strings.ToUpper(ogHash)
// cut off 0x
if strings.HasPrefix(rfqHash, "0x") {
if strings.HasPrefix(rfqHash, "0X") {

Check warning on line 355 in contrib/opbot/botmd/commands.go

View check run for this annotation

Codecov / codecov/patch

contrib/opbot/botmd/commands.go#L355

Added line #L355 was not covered by tests
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

Ensure consistent hash prefix handling in toExplorerSlackLink

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)

Committable suggestion was skipped due to low confidence.

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 356-356: contrib/opbot/botmd/commands.go#L356
Added line #L356 was not covered by tests

rfqHash = strings.ToLower(rfqHash[2:])
}

Comment on lines +355 to 358
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

Suggestion: 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

‼️ 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
if strings.HasPrefix(rfqHash, "0X") {
rfqHash = strings.ToLower(rfqHash[2:])
}
if strings.HasPrefix(rfqHash, "0X") || strings.HasPrefix(rfqHash, "0x") {
rfqHash = strings.ToLower(rfqHash[2:])
}

Expand All @@ -444,16 +374,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)
}
25 changes: 0 additions & 25 deletions contrib/opbot/botmd/commands_test.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
package botmd_test

import (
"context"
"testing"

"github.com/synapsecns/sanguine/contrib/opbot/botmd"
"github.com/synapsecns/sanguine/core/metrics"
omnirpcClient "github.com/synapsecns/sanguine/services/omnirpc/client"
"github.com/synapsecns/sanguine/services/rfq/relayer/relapi"
)

func TestStripLinks(t *testing.T) {
Expand All @@ -18,24 +14,3 @@ func TestStripLinks(t *testing.T) {
t.Errorf("StripLinks(%s) = %s; want %s", testLink, got, expected)
}
}

func TestTxAge(t *testing.T) {
notExpected := "unknown time ago" // should be a definite time

status := &relapi.GetQuoteRequestResponse{
OriginTxHash: "0x954264d120f5f3cf50edc39ebaf88ea9dc647d9d6843b7a120ed3677e23d7890",
OriginChainID: 421611,
}

ctx := context.Background()

client := omnirpcClient.NewOmnirpcClient("https://arb1.arbitrum.io/rpc", metrics.Get())
cc, err := client.GetChainClient(ctx, int(status.OriginChainID))
if err != nil {
t.Fatalf("GetChainClient() failed: %v", err)
}

if got := botmd.GetTxAge(context.Background(), cc, status); got == notExpected {
t.Errorf("TxAge(%s) = %s; want not %s", status.OriginTxHash, got, notExpected)
}
}
Loading
Loading