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.

Sign up for GitHub

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

refactor(relapi): deprecate GetQuoteRequestStatusResponse model #3045

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
79d6930
deprecate qrstatus model
golangisfun123 Aug 21, 2024
9336647
add remaining fields
golangisfun123 Aug 23, 2024
74f6146
Merge branch 'master' of github.com:synapsecns/sanguine into deprecat…
golangisfun123 Aug 26, 2024
838fe2e
fix test [goreleaser]
golangisfun123 Aug 26, 2024
2758b5f
fix test [goreleaser]
golangisfun123 Aug 26, 2024
819e357
Merge branch 'master' of github.com:synapsecns/sanguine into deprecat…
golangisfun123 Aug 27, 2024
fbcfba3
[goreleaser]
golangisfun123 Aug 27, 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
19 changes: 10 additions & 9 deletions contrib/opbot/botmd/commands.go
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
42 changes: 24 additions & 18 deletions services/rfq/relayer/relapi/server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func (c *RelayerServerSuite) TestGetQuoteRequestByTxHash() {

// Fetch the quote request by tx hash
client := &http.Client{}
req, err := http.NewRequestWithContext(c.GetTestContext(), http.MethodGet, fmt.Sprintf("http://localhost:%d/status?hash=%s", c.port, quoteRequest.OriginTxHash), nil)
req, err := http.NewRequestWithContext(c.GetTestContext(), http.MethodGet, fmt.Sprintf("http://localhost:%d/request/by_tx_hash?hash=%s", c.port, quoteRequest.OriginTxHash), nil)
c.Require().NoError(err)
resp, err := client.Do(req)
c.Require().NoError(err)
Expand All @@ -63,16 +63,19 @@ 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{
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(),
expectedResult := relapi.GetQuoteRequestResponse{
Status: quoteRequest.Status.String(),
TxID: hexutil.Encode(quoteRequest.TransactionID[:]),
QuoteRequestRaw: result.QuoteRequestRaw,
OriginTxHash: quoteRequest.OriginTxHash.String(),
DestTxHash: quoteRequest.DestTxHash.String(),
OriginChainID: quoteRequest.Transaction.OriginChainId,
DestChainID: quoteRequest.Transaction.DestChainId,
OriginToken: quoteRequest.Transaction.OriginToken.String(),
DestToken: quoteRequest.Transaction.DestToken.String(),
}
c.Equal(expectedResult, result)
c.GetTestContext().Done()
Expand All @@ -89,7 +92,7 @@ func (c *RelayerServerSuite) TestGetQuoteRequestByTxID() {
// Fetch the quote request by tx hash
client := &http.Client{}
txIDStr := hexutil.Encode(quoteRequest.TransactionID[:])
req, err := http.NewRequestWithContext(c.GetTestContext(), http.MethodGet, fmt.Sprintf("http://localhost:%d/status/by_tx_id?id=%s", c.port, txIDStr), nil)
req, err := http.NewRequestWithContext(c.GetTestContext(), http.MethodGet, fmt.Sprintf("http://localhost:%d/request/by_tx_id?id=%s", c.port, txIDStr), nil)
c.Require().NoError(err)
resp, err := client.Do(req)
c.Require().NoError(err)
Expand All @@ -102,16 +105,19 @@ 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{
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(),
expectedResult := relapi.GetQuoteRequestResponse{
Status: quoteRequest.Status.String(),
TxID: hexutil.Encode(quoteRequest.TransactionID[:]),
QuoteRequestRaw: result.QuoteRequestRaw,
OriginTxHash: quoteRequest.OriginTxHash.String(),
DestTxHash: quoteRequest.DestTxHash.String(),
OriginChainID: quoteRequest.Transaction.OriginChainId,
DestChainID: quoteRequest.Transaction.DestChainId,
OriginToken: quoteRequest.Transaction.OriginToken.String(),
DestToken: quoteRequest.Transaction.DestToken.String(),
}
c.Equal(expectedResult, result)
c.GetTestContext().Done()
Expand Down
Loading