fix trade aggregation ordering (#216)

services/horizon/internal/actions_trade_test.go

@@ -245,3 +245,40 @@ func TestTradeActions_IndexRegressions(t *testing.T) {
 
 	ht.Assert.Equal(404, w.Code) //This used to be 200 with length 0
 }
+
+// TestTradeActions_AggregationOrdering checks that open/close aggregation
+// fields are correct for multiple trades that occur in the same ledger
+// https://github.com/stellar/go/issues/215
+func TestTradeActions_AggregationOrdering(t *testing.T) {
+
+	ht := StartHTTPTest(t, "base")
+	defer ht.Finish()
+
+	seller := GetTestAccount()
+	buyer := GetTestAccount()
+	ass1 := GetTestAsset("usd")
+	ass2 := GetTestAsset("euro")
+
+	dbQ := &Q{ht.HorizonSession()}
+	IngestTestTrade(dbQ, ass1, ass2, seller, buyer, 1, 3, 0, 3)
+	IngestTestTrade(dbQ, ass1, ass2, seller, buyer, 1, 1, 0, 1)
+	IngestTestTrade(dbQ, ass1, ass2, seller, buyer, 1, 2, 0, 2)
+
+	q := make(url.Values)
+	setAssetQuery(&q, "base_", ass1)
+	setAssetQuery(&q, "counter_", ass2)
+
+	q.Add("start_time", "0")
+	q.Add("end_time", "10")
+	q.Add("order", "asc")
+	q.Add("resolution", "10")
+
+	var records []resource.TradeAggregation
+	w := ht.GetWithParams("/trade_aggregations", q)
+	if ht.Assert.Equal(200, w.Code) {
+		ht.Assert.PageOf(1, w.Body)
+		ht.UnmarshalPage(w.Body, &records)
+		ht.Assert.Equal("1.0000000", records[0].Open)
+		ht.Assert.Equal("3.0000000", records[0].Close)
+	}
+}

services/horizon/internal/db2/history/trade_aggregation.go

@@ -77,6 +77,9 @@ func (q *TradeAggregationsQ) GetSql() sq.SelectBuilder {
 		bucketSql = bucketSql.Where(sq.Lt{"ledger_closed_at": q.endTime.ToTime()})
 	}
 
+	//ensure open/close order for cases when multiple trades occur in the same ledger
+	bucketSql = bucketSql.OrderBy("history_operation_id ", "\"order\"")
+
 	return sq.Select(
 		"timestamp",
 		"count(*) as count",
@@ -104,6 +107,8 @@ func formatBucketTimestampSelect(resolution int64) string {
 func bucketTrades(resolution int64) sq.SelectBuilder {
 	return sq.Select(
 		formatBucketTimestampSelect(resolution),
+		"history_operation_id",
+		"\"order\"",
 		"base_asset_id",
 		"base_amount",
 		"counter_asset_id",
@@ -117,6 +122,8 @@ func bucketTrades(resolution int64) sq.SelectBuilder {
 func reverseBucketTrades(resolution int64) sq.SelectBuilder {
 	return sq.Select(
 		formatBucketTimestampSelect(resolution),
+		"history_operation_id",
+		"\"order\"",
 		"counter_asset_id as base_asset_id",
 		"counter_amount as base_amount",
 		"base_asset_id as counter_asset_id",

services/horizon/internal/test/trades/main.go

@@ -8,25 +8,25 @@ import (
 	"github.com/stellar/go/xdr"
 )
 
-//getTestAsset generates an issuer on the fly and creates a CreditAlphanum4 Asset with given code
-func getTestAsset(code string) xdr.Asset {
+//GetTestAsset generates an issuer on the fly and creates a CreditAlphanum4 Asset with given code
+func GetTestAsset(code string) xdr.Asset {
 	var codeBytes [4]byte
 	copy(codeBytes[:], []byte(code))
-	ca4 := xdr.AssetAlphaNum4{Issuer: getTestAccount(), AssetCode: codeBytes}
+	ca4 := xdr.AssetAlphaNum4{Issuer: GetTestAccount(), AssetCode: codeBytes}
 	return xdr.Asset{Type: xdr.AssetTypeAssetTypeCreditAlphanum4, AlphaNum4: &ca4, AlphaNum12: nil}
 }
 
 //Get generates and returns an account on the fly
-func getTestAccount() xdr.AccountId {
+func GetTestAccount() xdr.AccountId {
 	var key xdr.Uint256
 	kp, _ := keypair.Random()
 	copy(key[:], kp.Address())
 	acc, _ := xdr.NewAccountId(xdr.PublicKeyTypePublicKeyTypeEd25519, key)
 	return acc
 }
 
-//ingestTestTrade mock ingests a trade
-func ingestTestTrade(
+//IngestTestTrade mock ingests a trade
+func IngestTestTrade(
 	q *Q,
 	assetSold xdr.Asset,
 	assetBought xdr.Asset,
@@ -55,13 +55,13 @@ func PopulateTestTrades(
 	delta int64,
 	opStart int64) (ass1 xdr.Asset, ass2 xdr.Asset, err error) {
 
-	acc1 := getTestAccount()
-	acc2 := getTestAccount()
-	ass1 = getTestAsset("usd")
-	ass2 = getTestAsset("euro")
+	acc1 := GetTestAccount()
+	acc2 := GetTestAccount()
+	ass1 = GetTestAsset("usd")
+	ass2 = GetTestAsset("euro")
 	for i := 1; i <= numOfTrades; i++ {
 		timestamp := time.MillisFromInt64(startTs + (delta * int64(i-1)))
-		err = ingestTestTrade(
+		err = IngestTestTrade(
 			q, ass1, ass2, acc1, acc2, int64(i*100), int64(i*100)*int64(i), timestamp, opStart+int64(i))
 		//tt.Assert.NoError(err)
 		if err != nil {
@@ -70,3 +70,4 @@ func PopulateTestTrades(
 	}
 	return
 }
+