-
Notifications
You must be signed in to change notification settings - Fork 33
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
feat(rfq-relayer): relayer supports active quoting #3198
Changes from all commits
d93e20f
481f043
fdbc865
77c51e8
d10d56d
d2b1701
f06a64b
f6300a1
2646149
ea61286
dcd264a
02bf53c
a83253f
6febf7b
3ce1bd3
460f5ac
9ec49cb
92b49ec
e85ff62
f4ed5b5
0c0b562
1742fe3
0d7a7c4
4828dfc
ab9513d
a2a2079
69ed171
fbccdf8
4b098f9
135f1ac
b4969e9
bf3adaa
82ed6bb
896d52d
0018269
c3f0eb3
50b969e
64389c4
0c07fe4
719361a
26c9174
ccd24b3
9688fa7
b98ca8c
739472d
7d7f6df
6a5590f
56afeff
bdb7539
ab15e5d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -103,7 +103,6 @@ func (c *ServerSuite) TestActiveRFQSingleRelayer() { | |
} | ||
|
||
// Prepare the relayer quote response | ||
originAmount := userRequestAmount.String() | ||
destAmount := new(big.Int).Sub(userRequestAmount, big.NewInt(1000)).String() | ||
quoteResp := &model.WsRFQResponse{ | ||
DestAmount: destAmount, | ||
|
@@ -119,8 +118,7 @@ func (c *ServerSuite) TestActiveRFQSingleRelayer() { | |
// Assert the response | ||
c.Assert().True(userQuoteResp.Success) | ||
c.Assert().Equal("active", userQuoteResp.QuoteType) | ||
c.Assert().Equal(destAmount, *userQuoteResp.Data.DestAmount) | ||
c.Assert().Equal(originAmount, userQuoteResp.Data.OriginAmount) | ||
c.Assert().Equal(destAmount, userQuoteResp.DestAmount) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider refactoring repeated assertions into a helper function The assertion |
||
|
||
// Verify ActiveQuoteRequest insertion | ||
activeQuoteRequests, err := c.database.GetActiveQuoteRequests(c.GetTestContext()) | ||
|
@@ -206,7 +204,6 @@ func (c *ServerSuite) TestActiveRFQMultipleRelayers() { | |
} | ||
|
||
// Prepare the relayer quote responses | ||
originAmount := userRequestAmount.String() | ||
destAmount := "999000" | ||
quoteResp := model.WsRFQResponse{ | ||
DestAmount: destAmount, | ||
|
@@ -234,8 +231,7 @@ func (c *ServerSuite) TestActiveRFQMultipleRelayers() { | |
// Assert the response | ||
c.Assert().True(userQuoteResp.Success) | ||
c.Assert().Equal("active", userQuoteResp.QuoteType) | ||
c.Assert().Equal(destAmount, *userQuoteResp.Data.DestAmount) | ||
c.Assert().Equal(originAmount, userQuoteResp.Data.OriginAmount) | ||
c.Assert().Equal(destAmount, userQuoteResp.DestAmount) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Reduce code duplication by using a helper function This assertion duplicates logic from other tests: c.Assert().Equal(destAmount, userQuoteResp.DestAmount) Refactoring repeated code into a helper function can enhance readability and maintainability. |
||
|
||
// Verify ActiveQuoteRequest insertion | ||
activeQuoteRequests, err := c.database.GetActiveQuoteRequests(c.GetTestContext()) | ||
|
@@ -309,9 +305,8 @@ func (c *ServerSuite) TestActiveRFQFallbackToPassive() { | |
// Assert the response | ||
c.Assert().True(userQuoteResp.Success) | ||
c.Assert().Equal("passive", userQuoteResp.QuoteType) | ||
c.Assert().Equal("998000", *userQuoteResp.Data.DestAmount) // destAmount is quote destAmount minus fixed fee | ||
c.Assert().Equal(userRequestAmount.String(), userQuoteResp.Data.OriginAmount) | ||
c.Assert().Equal(c.relayerWallets[0].Address().Hex(), *userQuoteResp.Data.RelayerAddress) | ||
c.Assert().Equal("998000", userQuoteResp.DestAmount) // destAmount is quote destAmount minus fixed fee | ||
c.Assert().Equal(c.relayerWallets[0].Address().Hex(), userQuoteResp.RelayerAddress) | ||
Comment on lines
+308
to
+309
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Compute expected In the assertions: c.Assert().Equal("998000", userQuoteResp.DestAmount) // destAmount is quote destAmount minus fixed fee
c.Assert().Equal(c.relayerWallets[0].Address().Hex(), userQuoteResp.RelayerAddress) The expected Apply this change: + expectedDestAmount := new(big.Int).Sub(passiveQuotes[0].DestAmount.BigInt(), passiveQuotes[0].FixedFee.BigInt())
// Assert the response
c.Assert().True(userQuoteResp.Success)
c.Assert().Equal("passive", userQuoteResp.QuoteType)
- c.Assert().Equal("998000", userQuoteResp.DestAmount) // destAmount is quote destAmount minus fixed fee
+ c.Assert().Equal(expectedDestAmount.String(), userQuoteResp.DestAmount)
c.Assert().Equal(c.relayerWallets[0].Address().Hex(), userQuoteResp.RelayerAddress) |
||
} | ||
|
||
func (c *ServerSuite) TestActiveRFQPassiveBestQuote() { | ||
|
@@ -389,7 +384,6 @@ func (c *ServerSuite) TestActiveRFQPassiveBestQuote() { | |
// Assert the response | ||
c.Assert().True(userQuoteResp.Success) | ||
c.Assert().Equal("passive", userQuoteResp.QuoteType) | ||
c.Assert().Equal("998900", *userQuoteResp.Data.DestAmount) // destAmount is quote destAmount minus fixed fee | ||
c.Assert().Equal(userRequestAmount.String(), userQuoteResp.Data.OriginAmount) | ||
c.Assert().Equal(c.relayerWallets[0].Address().Hex(), *userQuoteResp.Data.RelayerAddress) | ||
c.Assert().Equal("998900", userQuoteResp.DestAmount) // destAmount is quote destAmount minus fixed fee | ||
c.Assert().Equal(c.relayerWallets[0].Address().Hex(), userQuoteResp.RelayerAddress) | ||
Comment on lines
+387
to
+388
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Calculate expected The assertion uses a hardcoded value: c.Assert().Equal("998900", userQuoteResp.DestAmount) // destAmount is quote destAmount minus fixed fee To enhance maintainability, compute the expected Apply this change: + expectedDestAmount := new(big.Int).Sub(passiveQuotes[0].DestAmount.BigInt(), passiveQuotes[0].FixedFee.BigInt())
// Assert the response
c.Assert().True(userQuoteResp.Success)
c.Assert().Equal("passive", userQuoteResp.QuoteType)
- c.Assert().Equal("998900", userQuoteResp.DestAmount) // destAmount is quote destAmount minus fixed fee
+ c.Assert().Equal(expectedDestAmount.String(), userQuoteResp.DestAmount)
c.Assert().Equal(c.relayerWallets[0].Address().Hex(), userQuoteResp.RelayerAddress) |
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix variable shadowing of
err
to ensure correct error handlingThe use of
:=
inresp, err := client.ReceiveQuoteResponse(collectionCtx, requestID)
shadows theerr
variable declared earlier at line 93. This means that theerr
used in the deferred function may remainnil
, and any error returned byReceiveQuoteResponse
won't be properly logged or passed tometrics.EndSpanWithErr
. To fix this, replace:=
with=
to assign to the existingerr
variable.Apply this diff to fix the issue: