feat(opbot): add screener to opbot #3076
Conversation
golangisfun123
requested review from
dwasse,
trajan0x and
aureliusbtc
as code owners
WalkthroughThe changes introduce a new dependency on a screener client, enhancing the bot's functionality to interact with a screener service. Modifications include improved error handling in the refund process and streamlined quote request retrieval. Additionally, a new configuration option for the screener URL is added, and the API response structure is updated to include sender information in quote requests. These changes collectively expand the capabilities and robustness of the bot and related services. Changes
Sequence Diagram(s)sequenceDiagram
participant Bot
participant ScreenerClient
participant QuoteRequest
Bot->>ScreenerClient: Screen Address
alt Address Valid
ScreenerClient-->>Bot: Address Validated
Bot->>QuoteRequest: Process Refund
else Address Invalid
ScreenerClient-->>Bot: Address Not Valid
Bot-->>User: Refund Not Allowed
end
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3076 +/- ##
===================================================
- Coverage 23.09876% 23.06899% -0.02977%
===================================================
Files 193 193
Lines 11624 11639 +15
Branches 82 82
===================================================
Hits 2685 2685
- Misses 8697 8712 +15
Partials 242 242
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (7)
- contrib/opbot/botmd/botmd.go (3 hunks)
- contrib/opbot/botmd/commands.go (2 hunks)
- contrib/opbot/cmd/commands.go (1 hunks)
- contrib/opbot/config/config.go (1 hunks)
- contrib/opbot/go.mod (2 hunks)
- services/rfq/relayer/relapi/handler.go (2 hunks)
- services/rfq/relayer/relapi/model.go (1 hunks)
Files skipped from review due to trivial changes (1)
- contrib/opbot/cmd/commands.go
Additional context used
GitHub Check: codecov/patch
contrib/opbot/botmd/botmd.go
[warning] 94-99: contrib/opbot/botmd/botmd.go#L94-L99
Added lines #L94 - L99 were not covered by testscontrib/opbot/botmd/commands.go
[warning] 302-308: contrib/opbot/botmd/commands.go#L302-L308
Added lines #L302 - L308 were not covered by tests
[warning] 311-316: contrib/opbot/botmd/commands.go#L311-L316
Added lines #L311 - L316 were not covered by tests
[warning] 410-412: contrib/opbot/botmd/commands.go#L410-L412
Added lines #L410 - L412 were not covered by tests
[warning] 416-417: contrib/opbot/botmd/commands.go#L416-L417
Added lines #L416 - L417 were not covered by tests
[warning] 420-420: contrib/opbot/botmd/commands.go#L420
Added line #L420 was not covered by tests
Additional comments not posted (7)
services/rfq/relayer/relapi/model.go (1)
20-20: LGTM! Verify impact on consuming services and documentation.The addition of the
Senderfield enhances the response structure by providing information about the sender of the quote request. Ensure that all consuming services and documentation are updated accordingly.Run the following script to verify the usage of
GetQuoteRequestResponsein the codebase:Verification successful
Integration of
SenderField Verified. Ensure Documentation and Consuming Services are Updated.The
Senderfield has been successfully integrated into theGetQuoteRequestResponsestruct and is being utilized in the codebase. Please verify that all relevant documentation and consuming services are updated to handle this new field appropriately.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `GetQuoteRequestResponse` in the codebase. # Test: Search for the struct usage. Expect: Occurrences of the new field. rg --type go -A 5 $'GetQuoteRequestResponse'Length of output: 8568
contrib/opbot/config/config.go (1)
36-37: LGTM! Verify impact on configuration files and documentation.The addition of the
ScreenerURLfield enhances the configuration capabilities by allowing it to include a URL for a screener. Ensure that all configuration files and documentation are updated accordingly.Run the following script to verify the usage of
ScreenerURLin the codebase:contrib/opbot/botmd/botmd.go (1)
32-32: LGTM! Verify impact on bot's initialization and usage.The addition of the
screenerfield enhances the bot's functionality by allowing it to utilize the screener client for its operations. Ensure that all initialization and usage of the bot are updated accordingly.Run the following script to verify the usage of
screenerin the codebase:Verification successful
Screener Field Integration Verified
The
screenerfield of typescreenerClient.ScreenerClientis properly integrated into the codebase. It is initialized and used incontrib/opbot/botmd/botmd.goand utilized incontrib/opbot/botmd/commands.gofor address screening. Ensure that all related initialization and usage are updated accordingly.
contrib/opbot/botmd/botmd.go: Initialization and usage ofscreener.contrib/opbot/botmd/commands.go: Usage ofscreenerfor address screening.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `screener` in the codebase. # Test: Search for the field usage. Expect: Occurrences of the new field. rg --type go -A 5 $'screener'Length of output: 37044
services/rfq/relayer/relapi/handler.go (2)
115-115: LGTM!The function is correctly implemented and the new field
Senderis properly added to the response object.The code changes are approved.
145-145: LGTM!The function is correctly implemented and the new field
Senderis properly added to the response object.The code changes are approved.
contrib/opbot/go.mod (2)
25-25: LGTM!The addition of the new dependency is correctly implemented.
The code changes are approved.
298-298: LGTM!The update of the replace directive is correctly implemented.
The code changes are approved.
| screenerClient, err := screenerClient.NewClient(b.handler, b.cfg.ScreenerURL) | ||
| if err != nil { | ||
| return fmt.Errorf("could not create screener client: %w", err) | ||
| } | ||
| b.screener = screenerClient | ||
|
|
There was a problem hiding this comment.
LGTM! Add tests to cover the new lines.
The new lines in the Start method enhance the bot's functionality by creating a screenerClient and assigning it to the screener field. However, these lines are not covered by tests, which may impact the reliability of the new functionality.
Do you want me to generate the unit testing code or open a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 94-99: contrib/opbot/botmd/botmd.go#L94-L99
Added lines #L94 - L99 were not covered by tests
|
|
||
| canRefund, err := b.screener.ScreenAddress(ctx.Context(), rawRequest.Sender) | ||
| if err != nil { | ||
| _, err := ctx.Response().Reply("error screening address") | ||
| if err != nil { | ||
| log.Println(err) | ||
| } | ||
| return | ||
| } | ||
|
|
||
| if !canRefund { | ||
| _, err := ctx.Response().Reply("address cannot be refunded") | ||
| if err != nil { | ||
| log.Println(err) | ||
| } | ||
| return | ||
| } |
There was a problem hiding this comment.
LGTM! But add tests for the new logic.
The function is correctly implemented and the new logic enhances the robustness of the refund process. However, test coverage is missing for the new lines.
The code changes are approved.
Do you want me to generate the unit testing code or open a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 302-308: contrib/opbot/botmd/commands.go#L302-L308
Added lines #L302 - L308 were not covered by tests
[warning] 311-316: contrib/opbot/botmd/commands.go#L311-L316
Added lines #L311 - L316 were not covered by tests
| 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 | ||
| qr, err := client.GetQuoteRequestByTXID(ctx, tx) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("error fetching quote request: %w", err) | ||
| if qr, err = client.GetQuoteRequestByTXID(ctx, tx); err == nil { | ||
| return qr, nil | ||
| } | ||
|
|
||
| return qr, nil | ||
| return nil, fmt.Errorf("error fetching quote request: %w", err) |
There was a problem hiding this comment.
LGTM! But add tests for the new logic.
The function is correctly implemented and the error handling has been improved. However, test coverage is missing for the new lines.
The code changes are approved.
Do you want me to generate the unit testing code or open a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 410-412: contrib/opbot/botmd/commands.go#L410-L412
Added lines #L410 - L412 were not covered by tests
[warning] 416-417: contrib/opbot/botmd/commands.go#L416-L417
Added lines #L416 - L417 were not covered by tests
[warning] 420-420: contrib/opbot/botmd/commands.go#L420
Added line #L420 was not covered by tests
Deploying sanguine-fe with
|
| Latest commit: |
e0923f8
|
| Status: | ⚡️ Build in progress... |
Description
A clear and concise description of the features you're adding in this pull request.
Additional context
Add any other context about the problem you're solving.
Metadata
Summary by CodeRabbit
New Features
Bug Fixes
Chores