add minimal rfq relayer query interface #2772
Conversation
|
Warning Rate limit exceeded@trajan0x has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 51 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThis update introduces a variety of significant enhancements and new functionalities across multiple files. The core changes include new functionalities for handling RFQ transactions, additional fields and methods to support blockchain-related operations, and the introduction of a new configuration parameter. Dependency updates and the addition of a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Bot
participant RelayerService
User->>Bot: Execute rfqLookupCommand with TxHash/ID
Bot->>RelayerService: GetQuoteRequestStatusByTxHash/ID
RelayerService-->>Bot: Quote Request Status (with OriginChainID & DestChainID)
Bot-->>User: Display Status with formatted Slack links
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
trajan0x
force-pushed
the
feat/opbot-rfq-interface
branch
from
66d4fec
to
93a3c92
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2772 +/- ##
===================================================
+ Coverage 25.36523% 25.37643% +0.01119%
===================================================
Files 755 715 -40
Lines 54007 52533 -1474
Branches 80 80
===================================================
- Hits 13699 13331 -368
+ Misses 38895 37849 -1046
+ Partials 1413 1353 -60
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Deploying sanguine-fe with
|
| Latest commit: |
c1a5473
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://1cdba6cb.sanguine-fe.pages.dev |
| Branch Preview URL: | https://feat-opbot-rfq-interface.sanguine-fe.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (1)
contrib/opbot/go.sumis excluded by!**/*.sum
Files selected for processing (8)
- contrib/opbot/botmd/botmd.go (1 hunks)
- contrib/opbot/botmd/commands.go (3 hunks)
- contrib/opbot/config/config.go (1 hunks)
- contrib/opbot/go.mod (8 hunks)
- services/rfq/go.mod (5 hunks)
- services/rfq/relayer/relapi/client.go (1 hunks)
- services/rfq/relayer/relapi/client_test.go (1 hunks)
- services/rfq/relayer/relapi/suite_test.go (1 hunks)
Files skipped from review due to trivial changes (1)
- services/rfq/go.mod
Additional context used
GitHub Check: codecov/patch
contrib/opbot/botmd/botmd.go
[warning] 30-30: contrib/opbot/botmd/botmd.go#L30
Added line #L30 was not covered by tests
[warning] 35-38: contrib/opbot/botmd/botmd.go#L35-L38
Added lines #L35 - L38 were not covered by testsservices/rfq/relayer/relapi/client.go
[warning] 42-43: services/rfq/relayer/relapi/client.go#L42-L43
Added lines #L42 - L43 were not covered by tests
[warning] 45-46: services/rfq/relayer/relapi/client.go#L45-L46
Added lines #L45 - L46 were not covered by tests
[warning] 61-62: services/rfq/relayer/relapi/client.go#L61-L62
Added lines #L61 - L62 were not covered by tests
[warning] 64-65: services/rfq/relayer/relapi/client.go#L64-L65
Added lines #L64 - L65 were not covered by tests
[warning] 78-79: services/rfq/relayer/relapi/client.go#L78-L79
Added lines #L78 - L79 were not covered by tests
[warning] 81-82: services/rfq/relayer/relapi/client.go#L81-L82
Added lines #L81 - L82 were not covered by tests
[warning] 94-95: services/rfq/relayer/relapi/client.go#L94-L95
Added lines #L94 - L95 were not covered by tests
[warning] 97-98: services/rfq/relayer/relapi/client.go#L97-L98
Added lines #L97 - L98 were not covered by testscontrib/opbot/botmd/commands.go
[warning] 78-78: contrib/opbot/botmd/commands.go#L78
Added line #L78 was not covered by tests
[warning] 115-136: contrib/opbot/botmd/commands.go#L115-L136
Added lines #L115 - L136 were not covered by tests
[warning] 139-155: contrib/opbot/botmd/commands.go#L139-L155
Added lines #L139 - L155 were not covered by tests
[warning] 158-167: contrib/opbot/botmd/commands.go#L158-L167
Added lines #L158 - L167 were not covered by tests
[warning] 170-177: contrib/opbot/botmd/commands.go#L170-L177
Added lines #L170 - L177 were not covered by tests
[warning] 180-204: contrib/opbot/botmd/commands.go#L180-L204
Added lines #L180 - L204 were not covered by tests
[warning] 206-209: contrib/opbot/botmd/commands.go#L206-L209
Added lines #L206 - L209 were not covered by tests
GitHub Check: Lint (services/rfq)
services/rfq/relayer/relapi/client.go
[failure] 25-25:
Comment should end in a period (godot)
[failure] 102-102:
unnecessary trailing newline (whitespace)
[failure] 42-42:
error returned from external package is unwrapped: sig: func (*github.com/go-resty/resty/v2.Request).Get(url string) (*github.com/go-resty/resty/v2.Response, error) (wrapcheck)
GitHub Check: Lint (contrib/opbot)
contrib/opbot/botmd/commands.go
[failure] 211-211:
unnecessary trailing newline (whitespace)
Additional comments not posted (5)
contrib/opbot/config/config.go (1)
18-19: Approved the addition of theRelayerURLSfield.The addition of
RelayerURLSextends the configuration capabilities to include multiple relayer URLs, which aligns with the PR's objective to enhance RFQ functionalities. It would be beneficial to add documentation comments explaining its usage and purpose.services/rfq/relayer/relapi/client_test.go (1)
8-51: Test methods are well-implemented and comprehensive.The test methods in this file are well-structured and provide thorough coverage for the functionalities of the
RelayerClient. They appropriately handle error checks and response validations, which is crucial for maintaining code reliability.services/rfq/relayer/relapi/client.go (1)
13-19: TheRelayerClientinterface is well-defined and aligns with project requirements.The methods defined in the
RelayerClientinterface are essential for the functionality of the relayer. This interface will help ensure that implementations adhere to a consistent specification, which is crucial for maintainability and scalability.services/rfq/relayer/relapi/suite_test.go (1)
198-227: Approve Addition ofRelayerClientSuite.The addition of
RelayerClientSuiteenhances the test framework by providing specialized setup and teardown procedures for testing the relayer client functionality. This is a positive change for maintaining robustness in testing.contrib/opbot/go.mod (1)
23-23: Verify New Dependencies and Path Changes.The added dependencies in the
go.modfile are significant and should be verified for compatibility and necessary licensing. The change in the path for theservices/rfqandservices/cctp-relayermodules to relative paths should also be confirmed to ensure they resolve correctly in the build environment.Also applies to: 31-90, 277-279
| log.Println(err) | ||
| } | ||
|
|
||
| }} |
There was a problem hiding this comment.
Remove Unnecessary Trailing Newline.
There is an unnecessary trailing newline at the end of the file.
- 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.
| }} | |
| }} |
Tools
GitHub Check: Lint (contrib/opbot)
[failure] 211-211:
unnecessary trailing newline (whitespace)
| @@ -27,11 +27,17 @@ | |||
|
|
|||
| bot.signozClient = signoz.NewClientFromUser(handler, cfg.SignozBaseURL, cfg.SignozEmail, cfg.SignozPassword) | |||
|
|
|||
| server.AddCommand(bot.traceCommand()) | |||
| bot.addCommands(bot.traceCommand(), bot.rfqLookupCommand()) | |||
There was a problem hiding this comment.
Add test coverage for the new commands in NewBot.
The addition of new commands in the NewBot method is crucial for the bot's functionality. However, these changes are not covered by tests, which is a significant oversight given the importance of this functionality.
Tools
GitHub Check: codecov/patch
[warning] 30-30: contrib/opbot/botmd/botmd.go#L30
Added line #L30 was not covered by tests
| func (b *Bot) addCommands(commands ...*slacker.CommandDefinition) { | ||
| for _, command := range commands { | ||
| b.server.AddCommand(command) | ||
| } |
There was a problem hiding this comment.
Ensure test coverage for the addCommands method.
The addCommands method improves code modularity by enabling the addition of multiple commands. However, the new lines introduced in this method are also not covered by tests, which could potentially lead to untested paths in production.
Tools
GitHub Check: codecov/patch
[warning] 35-38: contrib/opbot/botmd/botmd.go#L35-L38
Added lines #L35 - L38 were not covered by tests
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range and nitpick comments (1)
contrib/opbot/botmd/commands.go (1)
243-243: Clarify TODO comment intoTXSlackLink.The TODO comment in
toTXSlackLinkmentions controlling unfurl, but it's unclear what needs to be controlled or changed. Provide more details or a clearer explanation in the TODO comment to help future developers understand what needs to be addressed.Tools
GitHub Check: codecov/patch
[warning] 243-243: contrib/opbot/botmd/commands.go#L243
Added line #L243 was not covered by tests
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (7)
- contrib/opbot/botmd/commands.go (3 hunks)
- contrib/opbot/go.mod (9 hunks)
- ethergo/chaindata/chaindata.go (1 hunks)
- services/rfq/relayer/relapi/client.go (1 hunks)
- services/rfq/relayer/relapi/handler.go (2 hunks)
- services/rfq/relayer/relapi/model.go (1 hunks)
- services/rfq/relayer/relapi/server_test.go (1 hunks)
Files not reviewed due to errors (4)
- services/rfq/relayer/relapi/client.go (no review received)
- services/rfq/relayer/relapi/handler.go (no review received)
- ethergo/chaindata/chaindata.go (no review received)
- services/rfq/relayer/relapi/server_test.go (no review received)
Files skipped from review as they are similar to previous changes (1)
- contrib/opbot/go.mod
Additional context used
GitHub Check: codecov/patch
services/rfq/relayer/relapi/client.go
[warning] 44-44: services/rfq/relayer/relapi/client.go#L44
Added line #L44 was not covered by tests
[warning] 46-47: services/rfq/relayer/relapi/client.go#L46-L47
Added lines #L46 - L47 were not covered by tests
[warning] 62-63: services/rfq/relayer/relapi/client.go#L62-L63
Added lines #L62 - L63 were not covered by tests
[warning] 65-66: services/rfq/relayer/relapi/client.go#L65-L66
Added lines #L65 - L66 were not covered by tests
[warning] 79-80: services/rfq/relayer/relapi/client.go#L79-L80
Added lines #L79 - L80 were not covered by tests
[warning] 82-83: services/rfq/relayer/relapi/client.go#L82-L83
Added lines #L82 - L83 were not covered by tests
[warning] 95-96: services/rfq/relayer/relapi/client.go#L95-L96
Added lines #L95 - L96 were not covered by tests
[warning] 98-99: services/rfq/relayer/relapi/client.go#L98-L99
Added lines #L98 - L99 were not covered by testsethergo/chaindata/chaindata.go
[warning] 167-174: ethergo/chaindata/chaindata.go#L167-L174
Added lines #L167 - L174 were not covered by tests
[warning] 177-177: ethergo/chaindata/chaindata.go#L177
Added line #L177 was not covered by tests
[warning] 181-185: ethergo/chaindata/chaindata.go#L181-L185
Added lines #L181 - L185 were not covered by tests
[warning] 187-187: ethergo/chaindata/chaindata.go#L187
Added line #L187 was not covered by tests
[warning] 191-196: ethergo/chaindata/chaindata.go#L191-L196
Added lines #L191 - L196 were not covered by testscontrib/opbot/botmd/commands.go
[warning] 80-80: contrib/opbot/botmd/commands.go#L80
Added line #L80 was not covered by tests
[warning] 117-138: contrib/opbot/botmd/commands.go#L117-L138
Added lines #L117 - L138 were not covered by tests
[warning] 141-157: contrib/opbot/botmd/commands.go#L141-L157
Added lines #L141 - L157 were not covered by tests
[warning] 160-169: contrib/opbot/botmd/commands.go#L160-L169
Added lines #L160 - L169 were not covered by tests
[warning] 172-179: contrib/opbot/botmd/commands.go#L172-L179
Added lines #L172 - L179 were not covered by tests
[warning] 182-213: contrib/opbot/botmd/commands.go#L182-L213
Added lines #L182 - L213 were not covered by tests
[warning] 215-215: contrib/opbot/botmd/commands.go#L215
Added line #L215 was not covered by tests
[warning] 218-221: contrib/opbot/botmd/commands.go#L218-L221
Added lines #L218 - L221 were not covered by tests
[warning] 225-230: contrib/opbot/botmd/commands.go#L225-L230
Added lines #L225 - L230 were not covered by tests
[warning] 232-232: contrib/opbot/botmd/commands.go#L232
Added line #L232 was not covered by tests
[warning] 236-240: contrib/opbot/botmd/commands.go#L236-L240
Added lines #L236 - L240 were not covered by tests
[warning] 243-243: contrib/opbot/botmd/commands.go#L243
Added line #L243 was not covered by tests
GitHub Check: Lint (ethergo)
ethergo/chaindata/chaindata.go
[failure] 11-11:
struct fieldChainIdshould beChainID(golint)
[failure] 18-18:
exported varChainMetadataListshould have comment or be unexported (golint)
[failure] 167-167:
func parameterchainIdshould bechainID(golint)
[failure] 181-181:
func parameterchainIdshould bechainID(golint)
Additional comments not posted (1)
services/rfq/relayer/relapi/model.go (1)
5-10: Enhanced response structure with chain IDs.The addition of
OriginChainIDandDestChainIDto theGetQuoteRequestStatusResponsestruct is a valuable enhancement, providing clearer insights into the transaction's origin and destination chains.
| @@ -73,7 +77,7 @@ | |||
| return | |||
| } | |||
|
|
|||
| slackBlocks := []slack.Block{slack.NewHeaderBlock(slack.NewTextBlockObject(slack.PlainTextType, "Traces", false, false))} | |||
| slackBlocks := []slack.Block{slack.NewHeaderBlock(slack.NewTextBlockObject(slack.PlainTextType, fmt.Sprintf("Traces for %s", tags), false, false))} | |||
There was a problem hiding this comment.
Ensure Test Coverage for the Modified traceCommand Function.
The modified line 80, which formats the Slack message header, was not covered by tests according to the static analysis report. Consider adding unit tests to cover this functionality.
Tools
GitHub Check: codecov/patch
[warning] 80-80: contrib/opbot/botmd/commands.go#L80
Added line #L80 was not covered by tests
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
- Added RFQ transaction lookup functionality to the bot.
- Introduced chain metadata capabilities including conversion of chain IDs to names and explorer links.
- Expanded configuration with the new
RelayerURLSfield. - Added fields
OriginChainIDandDestChainIDto RFQ response structures. - Integrated new dependencies to support added features.
13 file(s) reviewed, 9 comment(s)
| sliceMux.Lock() | ||
| defer sliceMux.Unlock() | ||
| statuses = append(statuses, Status{relayer: relayer, GetQuoteRequestStatusResponse: res}) |
There was a problem hiding this comment.
Consider using a more efficient way to handle concurrent writes to the statuses slice.
| }} | ||
| } | ||
|
|
||
| func toExplorerSlackLink(ogHash string) string { |
There was a problem hiding this comment.
Typo in function name: 'toExplorerSlackLink' should be 'toExplorerSlackLink'.
| return fmt.Sprintf("<https://anon.to/?https://explorer.synapseprotocol.com/tx/%s|%s>", rfqHash, ogHash) | ||
| } | ||
|
|
||
| // produce a salck link if the explorer exists. |
There was a problem hiding this comment.
Typo in comment: 'salck' should be 'slack'.
| { | ||
| ChainID: 43114, | ||
| ChainName: "avalanche", | ||
| Explorer: "https://snowtrace.io/", |
There was a problem hiding this comment.
Remove trailing slash for consistency.
| Explorer: "https://snowtrace.io/", | |
| Explorer: "https://snowtrace.io", |
| if chainMetadata.ChainID == chainID { | ||
| // upper the first letter | ||
| if isUpper { | ||
| return strings.ToUpper(chainMetadata.ChainName) |
There was a problem hiding this comment.
Consider using strings.Title for better readability.
| return strings.ToUpper(chainMetadata.ChainName) | |
| return strings.Title(chainMetadata.ChainName) |
|
|
||
| // Health checks if the relayer is healthy. | ||
| func (r *relayerClient) Health(ctx context.Context) (ok bool, err error) { | ||
| resp, err := r.client.R().SetContext(ctx).Get(getHealthRoute) |
There was a problem hiding this comment.
Consider defining 'getHealthRoute' as a constant or variable.
| resp, err := r.client.R().SetContext(ctx). | ||
| SetQueryParam("hash", hash). | ||
| SetResult(&res). | ||
| Get(getQuoteStatusByTxHashRoute) |
There was a problem hiding this comment.
Consider defining 'getQuoteStatusByTxHashRoute' as a constant or variable.
| resp, err := r.client.R().SetContext(ctx). | ||
| SetQueryParam("id", txid). | ||
| SetResult(&res). | ||
| Get(getQuoteStatusByTxIDRoute) |
There was a problem hiding this comment.
Consider defining 'getQuoteStatusByTxIDRoute' as a constant or variable.
| resp, err := r.client.R().SetContext(ctx). | ||
| SetQueryParam("hash", txhash). | ||
| SetResult(&res). | ||
| Get(getRetryRoute) |
There was a problem hiding this comment.
Consider defining 'getRetryRoute' as a constant or variable.
There was a problem hiding this comment.
PR Summary
(updates since last review)
- Added RFQ transaction lookup functionality to the bot
- Introduced chain metadata capabilities (chain IDs to names, explorer links)
- Consolidated command addition logic for improved modularity
- Expanded configuration with
RelayerURLSfield - Added
OriginChainIDandDestChainIDfields to RFQ response structures
1 file(s) reviewed, no comment(s)
Description
adds a miminal rfq interface to opbot
Summary by CodeRabbit
New Features
Enhancements
RelayerURLSfield.OriginChainIDandDestChainIDto RFQ response structures, offering more detailed transaction information.Dependencies