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
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
19 changes: 10 additions & 9 deletions contrib/opbot/botmd/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@
Handler: func(ctx *slacker.CommandContext) {
type Status struct {
relayer string
*relapi.GetQuoteRequestStatusResponse
*relapi.GetQuoteRequestResponse

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

View check run for this annotation

Codecov / codecov/patch

contrib/opbot/botmd/commands.go#L155

Added line #L155 was not covered by tests
}

var statuses []Status
Expand All @@ -175,26 +175,26 @@
client := relapi.NewRelayerClient(b.handler, relayer)
go func() {
defer wg.Done()
res, err := client.GetQuoteRequestStatusByTxHash(ctx.Context(), tx)
res, err := client.GetQuoteRequestByTxHash(ctx.Context(), tx)

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

View check run for this annotation

Codecov / codecov/patch

contrib/opbot/botmd/commands.go#L178

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

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

View check run for this annotation

Codecov / codecov/patch

contrib/opbot/botmd/commands.go#L185

Added line #L185 was not covered by tests
}()

go func() {
defer wg.Done()
res, err := client.GetQuoteRequestStatusByTxID(ctx.Context(), tx)
res, err := client.GetQuoteRequestByTXID(ctx.Context(), tx)

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

View check run for this annotation

Codecov / codecov/patch

contrib/opbot/botmd/commands.go#L190

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

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

View check run for this annotation

Codecov / codecov/patch

contrib/opbot/botmd/commands.go#L197

Added line #L197 was not covered by tests
Comment on lines +178 to +197
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

}()
}
wg.Wait()
Expand Down Expand Up @@ -234,7 +234,7 @@
},
{
Type: slack.MarkdownType,
Text: fmt.Sprintf("*Estimated Tx Age*: %s", getTxAge(ctx.Context(), client, status.GetQuoteRequestStatusResponse)),
Text: fmt.Sprintf("*Estimated Tx Age*: %s", getTxAge(ctx.Context(), client, status.GetQuoteRequestResponse)),

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

View check run for this annotation

Codecov / codecov/patch

contrib/opbot/botmd/commands.go#L237

Added line #L237 was not covered by tests
},
}

Expand All @@ -257,7 +257,8 @@
if err != nil {
log.Println(err)
}
}}
},
}
}

// nolint: gocognit, cyclop.
Expand Down Expand Up @@ -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.

// 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 {
Expand Down Expand Up @@ -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)

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

View check run for this annotation

Codecov / codecov/patch

contrib/opbot/botmd/commands.go#L394

Added line #L394 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.

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

if err == nil {
// override tx with txid
tx = txRequest.TxID
Expand Down
2 changes: 1 addition & 1 deletion contrib/opbot/botmd/commands_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func TestStripLinks(t *testing.T) {
func TestTxAge(t *testing.T) {
notExpected := "unknown time ago" // should be a definite time

status := &relapi.GetQuoteRequestStatusResponse{
status := &relapi.GetQuoteRequestResponse{
OriginTxHash: "0x954264d120f5f3cf50edc39ebaf88ea9dc647d9d6843b7a120ed3677e23d7890",
OriginChainID: 421611,
}
Expand Down
2 changes: 1 addition & 1 deletion contrib/opbot/botmd/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ func StripLinks(input string) string {
return stripLinks(input)
}

func GetTxAge(ctx context.Context, client client.EVM, res *relapi.GetQuoteRequestStatusResponse) string {
func GetTxAge(ctx context.Context, client client.EVM, res *relapi.GetQuoteRequestResponse) string {
return getTxAge(ctx, client, res)
}
12 changes: 4 additions & 8 deletions services/rfq/relayer/relapi/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,6 @@ import (
// RelayerClient is the interface for the relayer client.
type RelayerClient interface {
Health(ctx context.Context) (ok bool, err error)
// Deprecated: use GetQuoteRequestByTxHash
GetQuoteRequestStatusByTxHash(ctx context.Context, hash string) (*GetQuoteRequestStatusResponse, error)
// Deprecated: use GetQuoteRequestStatusByTxID
GetQuoteRequestStatusByTxID(ctx context.Context, hash string) (*GetQuoteRequestStatusResponse, error)
RetryTransaction(ctx context.Context, txhash string) (*GetTxRetryResponse, error)
Withdraw(ctx context.Context, req *WithdrawRequest) (*WithdrawResponse, error)
GetTxHashByNonce(ctx context.Context, req *GetTxByNonceRequest) (*TxHashByNonceResponse, error)
Expand Down Expand Up @@ -58,8 +54,8 @@ func (r *relayerClient) Health(ctx context.Context) (ok bool, err error) {
return ok, nil
}

func (r *relayerClient) GetQuoteRequestStatusByTxHash(ctx context.Context, hash string) (*GetQuoteRequestStatusResponse, error) {
var res GetQuoteRequestStatusResponse
func (r *relayerClient) GetQuoteRequestStatusByTxHash(ctx context.Context, hash string) (*GetQuoteRequestResponse, error) {
var res GetQuoteRequestResponse

resp, err := r.client.R().SetContext(ctx).
SetQueryParam("hash", hash).
Expand All @@ -75,8 +71,8 @@ func (r *relayerClient) GetQuoteRequestStatusByTxHash(ctx context.Context, hash
return &res, nil
}

func (r *relayerClient) GetQuoteRequestStatusByTxID(ctx context.Context, txid string) (*GetQuoteRequestStatusResponse, error) {
var res GetQuoteRequestStatusResponse
func (r *relayerClient) GetQuoteRequestStatusByTxID(ctx context.Context, txid string) (*GetQuoteRequestResponse, error) {
var res GetQuoteRequestResponse

resp, err := r.client.R().SetContext(ctx).
SetQueryParam("id", txid).
Expand Down
4 changes: 2 additions & 2 deletions services/rfq/relayer/relapi/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func (c *RelayerClientSuite) TestGetQuoteRequestStatusByTxHash() {
err := c.underlying.database.StoreQuoteRequest(c.GetTestContext(), testReq)
c.Require().NoError(err)

resp, err := c.Client.GetQuoteRequestStatusByTxHash(c.GetTestContext(), testReq.OriginTxHash.String())
resp, err := c.Client.GetQuoteRequestByTxHash(c.GetTestContext(), testReq.OriginTxHash.String())
c.Require().NoError(err)

c.Equal(resp.Status, testReq.Status.String())
Expand All @@ -40,7 +40,7 @@ func (c *RelayerClientSuite) TestGetQuoteRequestStatusByTxID() {
err := c.underlying.database.StoreQuoteRequest(c.GetTestContext(), testReq)
c.Require().NoError(err)

resp, err := c.Client.GetQuoteRequestStatusByTxID(c.GetTestContext(), hexutil.Encode(testReq.TransactionID[:]))
resp, err := c.Client.GetQuoteRequestByTXID(c.GetTestContext(), hexutil.Encode(testReq.TransactionID[:]))
c.Require().NoError(err)

c.Equal(resp.Status, testReq.Status.String())
Expand Down
67 changes: 8 additions & 59 deletions services/rfq/relayer/relapi/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,65 +51,6 @@ func (h *Handler) GetHealth(c *gin.Context) {

const unspecifiedTxHash = "Must specify 'hash' (corresponding to origin tx)"

// GetQuoteRequestStatusByTxHash gets the status of a quote request, given an origin tx hash.
func (h *Handler) GetQuoteRequestStatusByTxHash(c *gin.Context) {
txHashStr := c.Query("hash")
if txHashStr == "" {
c.JSON(http.StatusBadRequest, gin.H{"error": unspecifiedTxHash})
return
}

txHash := common.HexToHash(txHashStr)
quoteRequest, err := h.db.GetQuoteRequestByOriginTxHash(c, txHash)
if err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
return
}

resp := GetQuoteRequestStatusResponse{
Status: quoteRequest.Status.String(),
TxID: hexutil.Encode(quoteRequest.TransactionID[:]),
OriginTxHash: quoteRequest.OriginTxHash.String(),
OriginChainID: quoteRequest.Transaction.OriginChainId,
DestChainID: quoteRequest.Transaction.DestChainId,
DestTxHash: quoteRequest.DestTxHash.String(),
}
c.JSON(http.StatusOK, resp)
}

// GetQuoteRequestStatusByTxID gets the status of a quote request, given a tx id.
func (h *Handler) GetQuoteRequestStatusByTxID(c *gin.Context) {
txIDStr := c.Query("id")
if txIDStr == "" {
c.JSON(http.StatusBadRequest, gin.H{"error": "Must specify 'id'"})
return
}

txIDBytes, err := hexutil.Decode(txIDStr)
if err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid txID"})
return
}
var txID [32]byte
copy(txID[:], txIDBytes)

quoteRequest, err := h.db.GetQuoteRequestByID(c, txID)
if err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
return
}

resp := GetQuoteRequestStatusResponse{
Status: quoteRequest.Status.String(),
TxID: hexutil.Encode(quoteRequest.TransactionID[:]),
OriginTxHash: quoteRequest.OriginTxHash.String(),
OriginChainID: quoteRequest.Transaction.OriginChainId,
DestChainID: quoteRequest.Transaction.DestChainId,
DestTxHash: quoteRequest.DestTxHash.String(),
}
c.JSON(http.StatusOK, resp)
}

// GetTxRetry retries a transaction based on tx hash.
func (h *Handler) GetTxRetry(c *gin.Context) {
txHashStr := c.Query("hash")
Expand Down Expand Up @@ -171,7 +112,11 @@ func (h *Handler) GetQuoteRequestByTxID(c *gin.Context) {
}

resp := GetQuoteRequestResponse{
Status: quoteRequest.Status.String(),
TxID: hexutil.Encode(quoteRequest.TransactionID[:]),
QuoteRequestRaw: common.Bytes2Hex(quoteRequest.RawRequest),
OriginTxHash: quoteRequest.OriginTxHash.String(),
DestTxHash: quoteRequest.DestTxHash.String(),
OriginChainID: quoteRequest.Transaction.OriginChainId,
DestChainID: quoteRequest.Transaction.DestChainId,
OriginToken: quoteRequest.Transaction.OriginToken.Hex(),
Expand All @@ -196,7 +141,11 @@ func (h *Handler) GetQuoteRequestByTxHash(c *gin.Context) {
}

resp := GetQuoteRequestResponse{
Status: quoteRequest.Status.String(),
TxID: hexutil.Encode(quoteRequest.TransactionID[:]),
QuoteRequestRaw: common.Bytes2Hex(quoteRequest.RawRequest),
OriginTxHash: quoteRequest.OriginTxHash.String(),
DestTxHash: quoteRequest.DestTxHash.String(),
OriginChainID: quoteRequest.Transaction.OriginChainId,
DestChainID: quoteRequest.Transaction.DestChainId,
OriginToken: quoteRequest.Transaction.OriginToken.Hex(),
Expand Down
14 changes: 4 additions & 10 deletions services/rfq/relayer/relapi/model.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,5 @@
package relapi

// GetQuoteRequestStatusResponse contains the schema for a GET /quote response.
type GetQuoteRequestStatusResponse struct {
Status string `json:"status"`
TxID string `json:"tx_id"`
OriginTxHash string `json:"origin_tx_hash"`
OriginChainID uint32 `json:"origin_chain_id"`
DestTxHash string `json:"dest_tx_hash"`
DestChainID uint32 `json:"dest_chain_id"`
}

// GetTxRetryResponse contains the schema for a PUT /tx/retry response.
type GetTxRetryResponse struct {
TxID string `json:"tx_id"`
Expand All @@ -27,7 +17,11 @@ type PutRelayAckResponse struct {

// GetQuoteRequestResponse is the response to a get quote request.
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"`
Expand Down
4 changes: 0 additions & 4 deletions services/rfq/relayer/relapi/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,6 @@ func (r *RelayerAPIServer) Run(ctx context.Context) error {

// Assign GET routes
engine.GET(getHealthRoute, h.GetHealth)
// TODO: replace with non-status specific endpoints
engine.GET(getQuoteStatusByTxHashRoute, h.GetQuoteRequestStatusByTxHash)
// TODO: replace with non-status specific endpoints
engine.GET(getQuoteStatusByTxIDRoute, h.GetQuoteRequestStatusByTxID)
engine.GET(getRetryRoute, h.GetTxRetry)
engine.GET(getRequestByTxID, h.GetQuoteRequestByTxID)
engine.GET(getRequestByTxHash, h.GetQuoteRequestByTxHash)
Expand Down
8 changes: 4 additions & 4 deletions services/rfq/relayer/relapi/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,10 @@ func (c *RelayerServerSuite) TestGetQuoteRequestByTxHash() {
c.Equal(http.StatusOK, resp.StatusCode)

// Compare to expected result
var result relapi.GetQuoteRequestStatusResponse
var result relapi.GetQuoteRequestResponse
err = json.NewDecoder(resp.Body).Decode(&result)
c.Require().NoError(err)
expectedResult := relapi.GetQuoteRequestStatusResponse{
expectedResult := relapi.GetQuoteRequestResponse{
Status: quoteRequest.Status.String(),
TxID: hexutil.Encode(quoteRequest.TransactionID[:]),
OriginTxHash: quoteRequest.OriginTxHash.String(),
Expand Down Expand Up @@ -102,10 +102,10 @@ func (c *RelayerServerSuite) TestGetQuoteRequestByTxID() {
c.Equal(http.StatusOK, resp.StatusCode)

// Compare to expected result
var result relapi.GetQuoteRequestStatusResponse
var result relapi.GetQuoteRequestResponse
err = json.NewDecoder(resp.Body).Decode(&result)
c.Require().NoError(err)
expectedResult := relapi.GetQuoteRequestStatusResponse{
expectedResult := relapi.GetQuoteRequestResponse{
Status: quoteRequest.Status.String(),
TxID: hexutil.Encode(quoteRequest.TransactionID[:]),
OriginTxHash: quoteRequest.OriginTxHash.String(),
Expand Down
Loading