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

refactor(relapi): deprecate GetQuoteRequestStatusResponse model #3045

Merged
merged 7 commits into from
Aug 28, 2024

Conversation

golangisfun123
Copy link
Collaborator

@golangisfun123 golangisfun123 commented Aug 21, 2024

simply integrated the remaining fields into the existing GetQuoteRequestResponse model for clearer code

Summary by CodeRabbit

  • New Features

    • Enhanced quote request responses with additional details including QuoteRequestRaw, OriginTxHash, and DestTxHash.
  • Bug Fixes

    • Updated command handling for quote requests, improving accuracy in data retrieval.
  • Tests

    • Modified test cases to align with the revised response types for quote requests, ensuring they reflect the latest API specifications.
  • Chores

    • Deprecated older methods related to quote request status, streamlining the API and removing unnecessary endpoints.

Copy link
Contributor

coderabbitai bot commented Aug 21, 2024

Walkthrough

The recent changes streamline the interaction between the bot and the relayer client by replacing the GetQuoteRequestStatusResponse type with the more comprehensive GetQuoteRequestResponse. This shift enhances the data structure, allowing for richer information transfer regarding quote requests. Various functions and tests throughout the codebase are updated to reflect this new structure, ensuring consistency and improving overall functionality.

Changes

Files Change Summary
contrib/opbot/botmd/.../commands.go, contrib/opbot/botmd/.../commands_test.go, contrib/opbot/botmd/.../export_test.go Updated GetQuoteRequestStatusResponse to GetQuoteRequestResponse, altering function signatures and internal structures accordingly.
services/rfq/relayer/relapi/.../client.go, services/rfq/relayer/relapi/.../client_test.go Deprecated status retrieval methods, replacing them with new methods that utilize the updated response type.
services/rfq/relayer/relapi/.../handler.go Modified response methods to return the new GetQuoteRequestResponse, enhancing the data returned.
services/rfq/relayer/relapi/.../model.go Removed GetQuoteRequestStatusResponse, integrating its fields into GetQuoteRequestResponse.
services/rfq/relayer/relapi/.../server_test.go Updated test functions to utilize GetQuoteRequestResponse instead of the deprecated status response type.

Sequence Diagram(s)

sequenceDiagram
    participant C as Client
    participant B as Bot
    participant R as RelayerClient
    participant S as Server

    C->>B: Send Quote Request
    B->>R: GetQuoteRequestByTxHash()
    R->>S: Fetch Quote Request
    S->>R: Return GetQuoteRequestResponse
    R->>B: Return GetQuoteRequestResponse
    B->>C: Provide Quote Information
Loading

🐇 "In the meadow where bunnies hop,
New responses make our coding pop!
From status to quote, we’ve made the shift,
With smoother flows, our spirits lift.
Hops and jumps, we celebrate,
With each change, we elevate!" 🐇


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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added go Pull requests that update Go code size/s labels Aug 21, 2024
Copy link

cloudflare-workers-and-pages bot commented Aug 21, 2024

Deploying sanguine-fe with  Cloudflare Pages  Cloudflare Pages

Latest commit: fbcfba3
Status: ✅  Deploy successful!
Preview URL: https://31ee7f9d.sanguine-fe.pages.dev
Branch Preview URL: https://deprecate-getqrstatus.sanguine-fe.pages.dev

View logs

Copy link

codecov bot commented Aug 21, 2024

Codecov Report

Attention: Patch coverage is 56.25000% with 7 lines in your changes missing coverage. Please review.

Project coverage is 23.12661%. Comparing base (bb13b4b) to head (fbcfba3).
Report is 1 commits behind head on master.

Files Patch % Lines
contrib/opbot/botmd/commands.go 12.50000% 7 Missing ⚠️
Additional details and impacted files
@@                 Coverage Diff                 @@
##              master       #3045         +/-   ##
===================================================
- Coverage   26.25742%   23.12661%   -3.13082%     
===================================================
  Files             97         193         +96     
  Lines           3877       11610       +7733     
  Branches          82          82                 
===================================================
+ Hits            1018        2685       +1667     
- Misses          2851        8683       +5832     
- Partials           8         242        +234     
Flag Coverage Δ
opbot 0.50378% <12.50000%> (ø)
promexporter 6.92368% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between f789485 and 79d6930.

Files selected for processing (8)
  • contrib/opbot/botmd/commands.go (6 hunks)
  • contrib/opbot/botmd/commands_test.go (1 hunks)
  • contrib/opbot/botmd/export_test.go (1 hunks)
  • services/rfq/relayer/relapi/client.go (3 hunks)
  • services/rfq/relayer/relapi/client_test.go (2 hunks)
  • services/rfq/relayer/relapi/handler.go (2 hunks)
  • services/rfq/relayer/relapi/model.go (2 hunks)
  • services/rfq/relayer/relapi/server_test.go (2 hunks)
Additional context used
GitHub Check: codecov/patch
contrib/opbot/botmd/commands.go

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


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


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


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


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


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


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

Additional comments not posted (14)
contrib/opbot/botmd/export_test.go (1)

14-14: Verify the logic of GetTxAge with the new parameter type.

The parameter type for res has been updated to *relapi.GetQuoteRequestResponse. Ensure that the getTxAge function correctly handles the new structure and fields.

Run the following script to verify the function's logic:

Verification successful

The logic of getTxAge correctly handles the new parameter type.

The getTxAge function uses the OriginTxHash field from the GetQuoteRequestResponse struct to retrieve transaction details, which is appropriate given the struct's definition. No issues were found with the logic concerning the new parameter type.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the logic of `getTxAge` with the new parameter type.

# Test: Search for the `getTxAge` function definition. Expect: Proper handling of `GetQuoteRequestResponse`.
ast-grep --lang go --pattern $'func getTxAge(ctx context.Context, client client.EVM, res *relapi.GetQuoteRequestResponse) string {
  $$$
}'

Length of output: 1112


Script:

#!/bin/bash
# Search for the definition of the GetQuoteRequestResponse struct to verify its fields.
ast-grep --lang go --pattern 'type GetQuoteRequestResponse struct {
  $$$
}'

Length of output: 964

services/rfq/relayer/relapi/model.go (1)

20-24: Verify the completeness and correctness of GetQuoteRequestResponse.

The GetQuoteRequestResponse struct now includes fields from the removed GetQuoteRequestStatusResponse. Ensure all necessary fields are present and correctly defined.

Run the following script to verify the struct's completeness:

Verification successful

Verification Successful: GetQuoteRequestResponse Struct is Complete and Correct

The GetQuoteRequestResponse struct contains all necessary fields, including those from the removed GetQuoteRequestStatusResponse. The fields are correctly defined as per the requirements.

  • Status
  • TxID
  • QuoteRequestRaw
  • OriginTxHash
  • DestTxHash
  • OriginChainID
  • DestChainID
  • OriginToken
  • DestToken
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the completeness and correctness of `GetQuoteRequestResponse`.

# Test: Search for the `GetQuoteRequestResponse` struct definition. Expect: All necessary fields are present.
ast-grep --lang go --pattern $'type GetQuoteRequestResponse struct {
  Status          string `json:"status"`
  TxID            string `json:"tx_id"`
  QuoteRequestRaw string `json:"quote_request"`
  OriginTxHash    string `json:"origin_tx_hash"`
  DestTxHash      string `json:"dest_tx_hash"`
  OriginChainID   uint32 `json:"origin_chain_id"`
  DestChainID     uint32 `json:"dest_chain_id"`
  OriginToken     string `json:"origin_token"`
  DestToken       string `json:"dest_token"`
}'

Length of output: 1373

contrib/opbot/botmd/commands_test.go (1)

25-27: Verify the test logic in TestTxAge with the updated response type.

The status variable now uses relapi.GetQuoteRequestResponse. Ensure the test logic correctly validates the new response format.

Run the following script to verify the test logic:

Verification successful

Test logic in TestTxAge is correctly validating the new response format.

The TestTxAge function properly uses relapi.GetQuoteRequestResponse to verify that the transaction age is calculated correctly and is not "unknown time ago". The test logic aligns with the expected behavior.

  • File: contrib/opbot/botmd/commands_test.go
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the test logic in `TestTxAge` with the updated response type.

# Test: Search for the `TestTxAge` function definition. Expect: Proper validation of `GetQuoteRequestResponse`.
ast-grep --lang go --pattern $'func TestTxAge(t *testing.T) {
  $$$
  status := &relapi.GetQuoteRequestResponse{
    OriginTxHash:  "0x954264d120f5f3cf50edc39ebaf88ea9dc647d9d6843b7a120ed3677e23d7890",
    OriginChainID: 421611,
  }
  $$$
}'

Length of output: 242


Script:

#!/bin/bash
# Search for the `TestTxAge` function definition in the codebase.
rg 'func TestTxAge' -A 20

Length of output: 1444

services/rfq/relayer/relapi/client.go (2)

Line range hint 74-87: LGTM! But verify the method usage in the codebase.

The return type change is consistent with the PR objectives.

Ensure that all calls to GetQuoteRequestStatusByTxID are updated to handle the new return type.

Run the following script to verify the method usage:


Line range hint 57-70: LGTM! But verify the method usage in the codebase.

The return type change is consistent with the PR objectives.

Ensure that all calls to GetQuoteRequestStatusByTxHash are updated to handle the new return type.

Run the following script to verify the method usage:

Verification successful

Method usage is correctly updated with the new return type.

The handler and test functions have been appropriately updated to handle the GetQuoteRequestResponse return type.

  • services/rfq/relayer/relapi/handler.go: Constructs a GetQuoteRequestResponse object.
  • services/rfq/relayer/relapi/client_test.go: Tests the fields of GetQuoteRequestResponse.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all method calls to `GetQuoteRequestStatusByTxHash` match the new return type.

# Test: Search for the method usage. Expect: Only occurrences of the new return type.
rg --type go -A 5 $'GetQuoteRequestStatusByTxHash'

Length of output: 2266


Script:

#!/bin/bash
# Description: Verify the handling of the new return type in the handler and test files.

# Check the handler function for handling of the new return type.
rg --type go -A 10 'func (h \*Handler) GetQuoteRequestStatusByTxHash' services/rfq/relayer/relapi/handler.go

# Check the test function for handling of the new return type.
rg --type go -A 10 'func (c \*RelayerClientSuite) TestGetQuoteRequestStatusByTxHash' services/rfq/relayer/relapi/client_test.go

Length of output: 240


Script:

#!/bin/bash
# Description: Broadly search for the function definitions to ensure they handle the new return type.

# Search for the handler function definition in the handler file.
rg --type go 'func .*GetQuoteRequestStatusByTxHash' services/rfq/relayer/relapi/handler.go -A 20

# Search for the test function definition in the test file.
rg --type go 'func .*TestGetQuoteRequestStatusByTxHash' services/rfq/relayer/relapi/client_test.go -A 20

Length of output: 1926

services/rfq/relayer/relapi/client_test.go (2)

29-29: Test method name updated successfully.

The test method name change aligns with the updated client method name.


43-43: Test method name updated successfully.

The test method name change aligns with the updated client method name.

services/rfq/relayer/relapi/server_test.go (2)

66-69: Response type updated successfully.

The response type change aligns with the updated response model in the client.


105-108: Response type updated successfully.

The response type change aligns with the updated response model in the client.

contrib/opbot/botmd/commands.go (3)

155-155: LGTM! The struct update aligns with the deprecation goal.

The Status struct now uses GetQuoteRequestResponse, which aligns with the PR's objective to deprecate GetQuoteRequestStatusResponse.

Tools
GitHub Check: codecov/patch

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


178-197: LGTM! Function calls updated to match new response type.

The function calls now use GetQuoteRequestByTxHash and GetQuoteRequestByTXID, reflecting the updated response type.

Tools
GitHub Check: codecov/patch

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


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


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


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


237-237: LGTM! Function signature updated to reflect new model.

The getTxAge function now uses GetQuoteRequestResponse, aligning with the updated response model.

Tools
GitHub Check: codecov/patch

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

services/rfq/relayer/relapi/handler.go (2)

69-78: LGTM! Response structure updated to use GetQuoteRequestResponse.

The response structure in GetQuoteRequestStatusByTxHash now uses GetQuoteRequestResponse, with additional fields for richer data.


105-114: LGTM! Response structure updated to use GetQuoteRequestResponse.

The response structure in GetQuoteRequestStatusByTxID now uses GetQuoteRequestResponse, with additional fields for richer data.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between f789485 and 79d6930.

Files selected for processing (8)
  • contrib/opbot/botmd/commands.go (6 hunks)
  • contrib/opbot/botmd/commands_test.go (1 hunks)
  • contrib/opbot/botmd/export_test.go (1 hunks)
  • services/rfq/relayer/relapi/client.go (3 hunks)
  • services/rfq/relayer/relapi/client_test.go (2 hunks)
  • services/rfq/relayer/relapi/handler.go (2 hunks)
  • services/rfq/relayer/relapi/model.go (2 hunks)
  • services/rfq/relayer/relapi/server_test.go (2 hunks)
Additional context used
GitHub Check: codecov/patch
contrib/opbot/botmd/commands.go

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


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


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


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


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


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


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

Additional comments not posted (12)
contrib/opbot/botmd/export_test.go (1)

14-14: LGTM! But verify the function usage in the codebase.

The change to use *relapi.GetQuoteRequestResponse is consistent with the PR's objective.

Ensure that all function calls to GetTxAge are updated to use the new response type.

Run the following script to verify the function usage:

Verification successful

Function usage is consistent with the changes.

The GetTxAge function is correctly using the new response type *relapi.GetQuoteRequestResponse in the codebase.

  • The status variable in commands_test.go is of the correct type.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `GetTxAge` use the new response type.

# Test: Search for the function usage. Expect: Only occurrences of the new response type.
rg --type go -A 5 $'GetTxAge'

Length of output: 589


Script:

#!/bin/bash
# Description: Check the type of the `status` variable in `commands_test.go`.

# Search for the declaration of `status` to verify its type.
rg --type go 'status\s*:' contrib/opbot/botmd/commands_test.go -A 2

Length of output: 225

services/rfq/relayer/relapi/model.go (1)

20-24: Consolidation of response fields approved.

The integration of fields from GetQuoteRequestStatusResponse into GetQuoteRequestResponse simplifies the response structure.

Ensure that all references to GetQuoteRequestStatusResponse are updated to use the new response structure.

Run the following script to verify the struct usage:

Verification successful

Reference to GetQuoteRequestStatusResponse found in a comment.

The GetQuoteRequestStatusResponse struct is not actively used in the codebase. However, there is a comment in contrib/opbot/botmd/commands.go that references it. Consider updating or removing this comment for clarity.

  • contrib/opbot/botmd/commands.go: Comment referencing GetQuoteRequestStatusResponse.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all references to `GetQuoteRequestStatusResponse` are updated.

# Test: Search for the struct usage. Expect: No occurrences of the old struct.
rg --type go $'GetQuoteRequestStatusResponse'

Length of output: 179

contrib/opbot/botmd/commands_test.go (1)

25-27: Test update approved.

The test now uses relapi.GetQuoteRequestResponse, consistent with the PR's objective.

Ensure that all tests are updated to use the new response type and verify the test coverage.

Run the following script to verify the test coverage:

Verification successful

All tests are using the new response type.

The search confirmed that no test files are using GetQuoteRequestStatusResponse. The update to relapi.GetQuoteRequestResponse in tests is consistent across the codebase.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all tests use the new response type.

# Test: Search for the old response type in tests. Expect: No occurrences of the old type.
rg --type go -A 5 $'GetQuoteRequestStatusResponse'

Length of output: 537

services/rfq/relayer/relapi/client.go (2)

Line range hint 74-85: Verify the usage of GetQuoteRequestResponse.

Ensure that the new response type GetQuoteRequestResponse is correctly populated and used throughout the codebase.


Line range hint 57-68: Verify the usage of GetQuoteRequestResponse.

Ensure that the new response type GetQuoteRequestResponse is correctly populated and used throughout the codebase.

Run the following script to verify the usage of GetQuoteRequestResponse:

Verification successful

Consistent Usage of GetQuoteRequestResponse Verified

The GetQuoteRequestResponse type is consistently used across the codebase, including in handler functions, client methods, and test files. This indicates that the transition to using this new response type has been implemented correctly. No further action is needed.

  • Files where GetQuoteRequestResponse is used:
    • services/rfq/relayer/relapi/handler.go
    • services/rfq/relayer/relapi/server_test.go
    • services/rfq/relayer/relapi/client.go
    • services/rfq/relayer/relapi/model.go
    • contrib/opbot/botmd/commands.go
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 occurrences of `GetQuoteRequestResponse`. Expect: Consistent usage across the codebase.
rg --type go 'GetQuoteRequestResponse'

Length of output: 3165

services/rfq/relayer/relapi/client_test.go (2)

43-43: Verify the test logic for GetQuoteRequestByTXID.

Ensure that the test correctly validates the functionality of the updated method GetQuoteRequestByTXID.


29-29: Verify the test logic for GetQuoteRequestByTxHash.

Ensure that the test correctly validates the functionality of the updated method GetQuoteRequestByTxHash.

Run the following script to verify the test logic:

Verification successful

The test logic for GetQuoteRequestByTxHash is correctly implemented.

The test file client_test.go includes comprehensive checks to validate the functionality of the GetQuoteRequestByTxHash method, ensuring that it behaves as expected. The test verifies the response status, transaction ID, destination transaction hash, and the length of the raw quote request. No further action is needed.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the test logic for `GetQuoteRequestByTxHash`.

# Test: Search for the test function and ensure it correctly validates the updated method.
ast-grep --lang go --pattern $'func (c *RelayerClientSuite) TestGetQuoteRequestByTxHash() {
  $$$
}'

Length of output: 101


Script:

#!/bin/bash
# Description: Search for any mention of `GetQuoteRequestByTxHash` in the test file to determine if the function exists or if there are related tests.
rg 'GetQuoteRequestByTxHash' services/rfq/relayer/relapi/client_test.go -A 5

Length of output: 574

services/rfq/relayer/relapi/server_test.go (2)

105-108: Verify the test logic for GetQuoteRequestResponse.

Ensure that the test correctly validates the functionality and structure of the updated response type GetQuoteRequestResponse.


66-69: Verify the test logic for GetQuoteRequestResponse.

Ensure that the test correctly validates the functionality and structure of the updated response type GetQuoteRequestResponse.

Run the following script to verify the test logic:

Verification successful

Test logic for GetQuoteRequestResponse is correctly implemented.

The test function TestGetQuoteRequestByTxHash successfully validates the structure and content of the GetQuoteRequestResponse by comparing the decoded response with an expected result constructed from the quoteRequest object. No issues found with the test logic.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the test logic for `GetQuoteRequestResponse`.

# Test: Search for the test function and ensure it correctly validates the updated response type.
ast-grep --lang go --pattern $'func (c *RelayerServerSuite) TestGetQuoteRequestByTxHash() {
  $$$
}'

Length of output: 3138

contrib/opbot/botmd/commands.go (1)

155-155: Verify the usage of the Status struct.

Ensure that the Status struct's new definition is correctly used throughout the codebase.

Run the following script to verify the usage of the Status struct:

Verification successful

The Status struct is correctly used in the codebase.

The Status struct is defined and utilized as expected in contrib/opbot/botmd/commands.go. No issues were found with its usage.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of the `Status` struct.

# Test: Search for the struct usage. Expect: Only occurrences of the new definition.
rg --type go -A 5 $'type Status struct {'

Length of output: 146


Script:

#!/bin/bash
# Description: Verify the usage of the `Status` struct.

# Test: Search for the struct definition and its usage.
rg --type go 'type Status struct' -A 5

Length of output: 342

Tools
GitHub Check: codecov/patch

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

services/rfq/relayer/relapi/handler.go (2)

105-114: Verify the correctness of the new response fields.

Ensure that QuoteRequestRaw, OriginToken, and DestToken are correctly populated in the response.

Run the following script to check the correctness of the new fields:

Verification successful

The new response fields are correctly populated.

The fields QuoteRequestRaw, OriginToken, and DestToken are consistently populated using appropriate methods to convert data to hexadecimal strings. This ensures their correctness in the response.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the correctness of the new response fields.

# Test: Check for the usage of `QuoteRequestRaw`, `OriginToken`, and `DestToken`.
rg --type go -A 5 $'QuoteRequestRaw:'

Length of output: 2289


69-78: Verify the correctness of the new response fields.

Ensure that QuoteRequestRaw, OriginToken, and DestToken are correctly populated in the response.

Run the following script to check the correctness of the new fields:

Comment on lines +178 to +197
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, GetQuoteRequestStatusResponse: res})
statuses = append(statuses, Status{relayer: relayer, GetQuoteRequestResponse: res})
}()

go func() {
defer wg.Done()
res, err := client.GetQuoteRequestStatusByTxID(ctx.Context(), tx)
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, GetQuoteRequestStatusResponse: res})
statuses = append(statuses, Status{relayer: relayer, GetQuoteRequestResponse: res})
Copy link
Contributor

Choose a reason for hiding this comment

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

Add tests for the new relayer client calls.

The calls to GetQuoteRequestByTxHash and GetQuoteRequestByTXID are not covered by tests.

Would you like me to generate unit tests for these calls or open a GitHub issue to track this task?

Tools
GitHub Check: codecov/patch

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


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


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


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

@@ -348,7 +349,7 @@
return fastBridgeHandle, nil
}

func getTxAge(ctx context.Context, client client.EVM, res *relapi.GetQuoteRequestStatusResponse) string {
func getTxAge(ctx context.Context, client client.EVM, res *relapi.GetQuoteRequestResponse) string {
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider optimizing the getTxAge function.

The TODO comment suggests adding a CreatedAt field to avoid network calls. This could improve performance.

If feasible, add a CreatedAt field to GetQuoteRequestResponse and use it to calculate the transaction age.

@@ -390,7 +391,7 @@

func getQuoteRequest(ctx context.Context, client relapi.RelayerClient, tx string) (*relapi.GetQuoteRequestResponse, error) {
// at this point tx can be a txid or a has, we try both
txRequest, err := client.GetQuoteRequestStatusByTxHash(ctx, tx)
txRequest, err := client.GetQuoteRequestByTxHash(ctx, tx)
Copy link
Contributor

Choose a reason for hiding this comment

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

Add tests for the getQuoteRequest function.

The initial call to GetQuoteRequestByTxHash is not covered by tests.

Would you like me to generate unit tests for this function or open a GitHub issue to track this task?

Tools
GitHub Check: codecov/patch

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

Copy link
Contributor

@trajan0x trajan0x left a comment

Choose a reason for hiding this comment

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

Can we delete the status routes as well?

getQuoteStatusByTxHashRoute = "/status"
getQuoteStatusByTxIDRoute = "/status/by_tx_id"

If concern si brekaing backwards compatibility LGTM then delete in a future pr

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 79d6930 and 9336647.

Files selected for processing (2)
  • services/rfq/relayer/relapi/handler.go (3 hunks)
  • services/rfq/relayer/relapi/server.go (1 hunks)
Files skipped from review due to trivial changes (1)
  • services/rfq/relayer/relapi/server.go
Additional comments not posted (2)
services/rfq/relayer/relapi/handler.go (2)

115-119: LGTM! Enhanced response structure.

The added fields QuoteRequestRaw, OriginTxHash, and DestTxHash provide more comprehensive details about the quote request.


144-148: LGTM! Enhanced response structure.

The added fields QuoteRequestRaw, OriginTxHash, and DestTxHash provide more comprehensive details about the quote request.

trajan0x added a commit that referenced this pull request Aug 25, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 9336647 and 838fe2e.

Files selected for processing (1)
  • services/rfq/relayer/relapi/server_test.go (4 hunks)
Files skipped from review as they are similar to previous changes (1)
  • services/rfq/relayer/relapi/server_test.go

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 838fe2e and 2758b5f.

Files selected for processing (2)
  • services/rfq/relayer/relapi/client.go (2 hunks)
  • services/rfq/relayer/relapi/server.go (2 hunks)
Files skipped from review as they are similar to previous changes (2)
  • services/rfq/relayer/relapi/client.go
  • services/rfq/relayer/relapi/server.go

@trajan0x trajan0x merged commit eccbfd1 into master Aug 28, 2024
35 of 36 checks passed
@trajan0x trajan0x deleted the deprecate-getqrstatus branch August 28, 2024 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go Pull requests that update Go code size/s
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants