-
Notifications
You must be signed in to change notification settings - Fork 499
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
Add offer prices to trades and trade aggregations #302
Conversation
d901d90
to
82c17ed
Compare
965f735
to
75f71ce
Compare
@@ -120,11 +135,15 @@ func TestTradeActions_Aggregation(t *testing.T) { | |||
|
|||
dbQ := &Q{ht.HorizonSession()} | |||
ass1, ass2, err := PopulateTestTrades(dbQ, start, numOfTrades, minute, 0) | |||
ht.Require.NoError(err) | |||
if !ht.Assert.NoError(err) { |
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.
If a failed assertion should abort a test run then use Require.NoError
instead of assert.
|
||
//add other trades as noise, to ensure asset filtering is working | ||
_, _, err = PopulateTestTrades(dbQ, start, numOfTrades, minute, numOfTrades) | ||
ht.Require.NoError(err) | ||
if !ht.Assert.NoError(err) { |
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.
ht.Require.NoError(err)
if orderPreserved { | ||
baseAccountId, baseAmount, counterAccountId, counterAmount = | ||
sellerAccountId, trade.AmountSold, buyerAccountId, trade.AmountBought | ||
} else { | ||
baseAccountId, baseAmount, counterAccountId, counterAmount = | ||
buyerAccountId, trade.AmountBought, sellerAccountId, trade.AmountSold | ||
baseAccountId, baseAmount, counterAccountId, counterAmount, price = |
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.
I think a 5 clause compound assignment is over doing it. Let's expand this into separate statements for readability's sake.
xdr/db.go
Outdated
arr := pq.Int64Array{} | ||
err := arr.Scan(src) | ||
if err != nil || len(arr)!=2 { | ||
return err |
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.
Err will be nil here if len(arr)!=2
Please expand into two separate statements:
if err != nil {
return err
}
if len(arr) != 2 {
return errors.New("bzzzt")
}
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.
👍
/trades
query and response endpoint/trade_aggregations
to use trade offer pricescloses #224