From 9e69521ed24044eed8e45ba3710055785f74d2c7 Mon Sep 17 00:00:00 2001 From: tamirms Date: Tue, 14 Sep 2021 18:12:33 +0100 Subject: [PATCH] Fix trade prices (#3915) --- protocols/horizon/main.go | 113 ++++++++++++------ .../horizon/internal/actions_trade_test.go | 6 +- .../internal/db2/history/trade_aggregation.go | 33 ++--- .../internal/integration/protocol18_test.go | 4 +- .../horizon/internal/resourceadapter/trade.go | 6 +- .../resourceadapter/trade_aggregation.go | 30 +++-- .../internal/scraper/trade_scraper_test.go | 18 +-- 7 files changed, 122 insertions(+), 88 deletions(-) diff --git a/protocols/horizon/main.go b/protocols/horizon/main.go index 763eec56d9..30de65f4cd 100644 --- a/protocols/horizon/main.go +++ b/protocols/horizon/main.go @@ -5,6 +5,7 @@ package horizon import ( "encoding/base64" "encoding/json" + "math/big" "strconv" "time" @@ -297,7 +298,7 @@ func (p Path) PagingToken() string { return "" } -// Price represents a price +// Price represents a price for an offer type Price base.Price // PriceLevel represents an aggregation of offers that share a given price @@ -356,6 +357,44 @@ type Signer struct { Sponsor string `json:"sponsor,omitempty"` } +// TradePrice represents a price for a trade +type TradePrice struct { + N int64 `json:"n,string"` + D int64 `json:"d,string"` +} + +// String returns a string representation of the trade price +func (p TradePrice) String() string { + return big.NewRat(p.N, p.D).FloatString(7) +} + +// UnmarshalJSON implements a custom unmarshaler for TradePrice +// which can handle a numerator and denominator fields which can be a string or int +func (p *TradePrice) UnmarshalJSON(data []byte) error { + v := struct { + N json.Number `json:"n"` + D json.Number `json:"d"` + }{} + err := json.Unmarshal(data, &v) + if err != nil { + return err + } + + if v.N != "" { + p.N, err = v.N.Int64() + if err != nil { + return err + } + } + if v.D != "" { + p.D, err = v.D.Int64() + if err != nil { + return err + } + } + return nil +} + // Trade represents a horizon digested trade type Trade struct { Links struct { @@ -365,28 +404,28 @@ type Trade struct { Operation hal.Link `json:"operation"` } `json:"_links"` - ID string `json:"id"` - PT string `json:"paging_token"` - LedgerCloseTime time.Time `json:"ledger_close_time"` - OfferID string `json:"offer_id,omitempty"` - TradeType string `json:"trade_type"` - LiquidityPoolFeeBP uint32 `json:"liquidity_pool_fee_bp,omitempty"` - BaseLiquidityPoolID string `json:"base_liquidity_pool_id,omitempty"` - BaseOfferID string `json:"base_offer_id,omitempty"` - BaseAccount string `json:"base_account,omitempty"` - BaseAmount string `json:"base_amount"` - BaseAssetType string `json:"base_asset_type"` - BaseAssetCode string `json:"base_asset_code,omitempty"` - BaseAssetIssuer string `json:"base_asset_issuer,omitempty"` - CounterLiquidityPoolID string `json:"counter_liquidity_pool_id,omitempty"` - CounterOfferID string `json:"counter_offer_id,omitempty"` - CounterAccount string `json:"counter_account"` - CounterAmount string `json:"counter_amount"` - CounterAssetType string `json:"counter_asset_type"` - CounterAssetCode string `json:"counter_asset_code,omitempty"` - CounterAssetIssuer string `json:"counter_asset_issuer,omitempty"` - BaseIsSeller bool `json:"base_is_seller"` - Price *Price `json:"price"` + ID string `json:"id"` + PT string `json:"paging_token"` + LedgerCloseTime time.Time `json:"ledger_close_time"` + OfferID string `json:"offer_id,omitempty"` + TradeType string `json:"trade_type"` + LiquidityPoolFeeBP uint32 `json:"liquidity_pool_fee_bp,omitempty"` + BaseLiquidityPoolID string `json:"base_liquidity_pool_id,omitempty"` + BaseOfferID string `json:"base_offer_id,omitempty"` + BaseAccount string `json:"base_account,omitempty"` + BaseAmount string `json:"base_amount"` + BaseAssetType string `json:"base_asset_type"` + BaseAssetCode string `json:"base_asset_code,omitempty"` + BaseAssetIssuer string `json:"base_asset_issuer,omitempty"` + CounterLiquidityPoolID string `json:"counter_liquidity_pool_id,omitempty"` + CounterOfferID string `json:"counter_offer_id,omitempty"` + CounterAccount string `json:"counter_account"` + CounterAmount string `json:"counter_amount"` + CounterAssetType string `json:"counter_asset_type"` + CounterAssetCode string `json:"counter_asset_code,omitempty"` + CounterAssetIssuer string `json:"counter_asset_issuer,omitempty"` + BaseIsSeller bool `json:"base_is_seller"` + Price TradePrice `json:"price,omitempty"` } // PagingToken implementation for hal.Pageable @@ -421,19 +460,19 @@ type TradeEffect struct { // TradeAggregation represents trade data aggregation over a period of time type TradeAggregation struct { - Timestamp int64 `json:"timestamp,string"` - TradeCount int64 `json:"trade_count,string"` - BaseVolume string `json:"base_volume"` - CounterVolume string `json:"counter_volume"` - Average string `json:"avg"` - High string `json:"high"` - HighR xdr.Price `json:"high_r"` - Low string `json:"low"` - LowR xdr.Price `json:"low_r"` - Open string `json:"open"` - OpenR xdr.Price `json:"open_r"` - Close string `json:"close"` - CloseR xdr.Price `json:"close_r"` + Timestamp int64 `json:"timestamp,string"` + TradeCount int64 `json:"trade_count,string"` + BaseVolume string `json:"base_volume"` + CounterVolume string `json:"counter_volume"` + Average string `json:"avg"` + High string `json:"high"` + HighR TradePrice `json:"high_r"` + Low string `json:"low"` + LowR TradePrice `json:"low_r"` + Open string `json:"open"` + OpenR TradePrice `json:"open_r"` + Close string `json:"close"` + CloseR TradePrice `json:"close_r"` } // PagingToken implementation for hal.Pageable. Not actually used @@ -524,7 +563,7 @@ func (t Transaction) MarshalJSON() ([]byte, error) { } // UnmarshalJSON implements a custom unmarshaler for Transaction -// which can handle a max_fee field which can be a string of int +// which can handle a max_fee field which can be a string or int func (t *Transaction) UnmarshalJSON(data []byte) error { type Alias Transaction // we define Alias to avoid infinite recursion when calling UnmarshalJSON() v := &struct { diff --git a/services/horizon/internal/actions_trade_test.go b/services/horizon/internal/actions_trade_test.go index c07fd11859..f56d6ff53a 100644 --- a/services/horizon/internal/actions_trade_test.go +++ b/services/horizon/internal/actions_trade_test.go @@ -311,8 +311,8 @@ func assertResponseTradeEqualsDBTrade(ht *HTTPT, row history.Trade, record horiz ht.Assert.Equal(uint32(row.LiquidityPoolFee.Int64), record.LiquidityPoolFeeBP) ht.Assert.Equal(row.PagingToken(), record.PagingToken()) ht.Assert.Equal(row.LedgerCloseTime.Unix(), record.LedgerCloseTime.Unix()) - ht.Assert.Equal(int32(row.PriceN.Int64), record.Price.N) - ht.Assert.Equal(int32(row.PriceD.Int64), record.Price.D) + ht.Assert.Equal(row.PriceN.Int64, record.Price.N) + ht.Assert.Equal(row.PriceD.Int64, record.Price.D) if row.BaseLiquidityPoolID.Valid || row.CounterLiquidityPoolID.Valid { ht.Assert.Equal(history.LiquidityPoolTrades, record.TradeType) @@ -338,7 +338,7 @@ func unsetAssetQuery(q *url.Values, prefix string) { } //testPrice ensures that the price float string is equal to the rational price -func testPrice(t *HTTPT, priceStr string, priceR xdr.Price) { +func testPrice(t *HTTPT, priceStr string, priceR horizon.TradePrice) { price, err := strconv.ParseFloat(priceStr, 64) if t.Assert.NoError(err) { t.Assert.Equal(price, float64(priceR.N)/float64(priceR.D)) diff --git a/services/horizon/internal/db2/history/trade_aggregation.go b/services/horizon/internal/db2/history/trade_aggregation.go index 0ad7335f70..20efbcd688 100644 --- a/services/horizon/internal/db2/history/trade_aggregation.go +++ b/services/horizon/internal/db2/history/trade_aggregation.go @@ -11,7 +11,6 @@ import ( "github.com/stellar/go/services/horizon/internal/toid" "github.com/stellar/go/support/errors" strtime "github.com/stellar/go/support/time" - "github.com/stellar/go/xdr" ) // AllowedResolutions is the set of trade aggregation time windows allowed to be used as the @@ -36,30 +35,14 @@ type TradeAggregation struct { BaseVolume string `db:"base_volume"` CounterVolume string `db:"counter_volume"` Average float64 `db:"avg"` - HighN int32 `db:"high_n"` - HighD int32 `db:"high_d"` - LowN int32 `db:"low_n"` - LowD int32 `db:"low_d"` - OpenN int32 `db:"open_n"` - OpenD int32 `db:"open_d"` - CloseN int32 `db:"close_n"` - CloseD int32 `db:"close_d"` -} - -func (t TradeAggregation) High() xdr.Price { - return xdr.Price{N: xdr.Int32(t.HighN), D: xdr.Int32(t.HighD)} -} - -func (t TradeAggregation) Low() xdr.Price { - return xdr.Price{N: xdr.Int32(t.LowN), D: xdr.Int32(t.LowD)} -} - -func (t TradeAggregation) Open() xdr.Price { - return xdr.Price{N: xdr.Int32(t.OpenN), D: xdr.Int32(t.OpenD)} -} - -func (t TradeAggregation) Close() xdr.Price { - return xdr.Price{N: xdr.Int32(t.CloseN), D: xdr.Int32(t.CloseD)} + HighN int64 `db:"high_n"` + HighD int64 `db:"high_d"` + LowN int64 `db:"low_n"` + LowD int64 `db:"low_d"` + OpenN int64 `db:"open_n"` + OpenD int64 `db:"open_d"` + CloseN int64 `db:"close_n"` + CloseD int64 `db:"close_d"` } // TradeAggregationsQ is a helper struct to aid in configuring queries to diff --git a/services/horizon/internal/integration/protocol18_test.go b/services/horizon/internal/integration/protocol18_test.go index ba96c63745..2c83ffff2a 100644 --- a/services/horizon/internal/integration/protocol18_test.go +++ b/services/horizon/internal/integration/protocol18_test.go @@ -442,8 +442,8 @@ func TestLiquidityPoolHappyPath(t *testing.T) { tt.Equal("1.0353642", trade1.CounterAmount) tt.Equal("native", trade1.CounterAssetType) - tt.Equal(int32(10353642), trade1.Price.N) - tt.Equal(int32(20000000), trade1.Price.D) + tt.Equal(int64(10353642), trade1.Price.N) + tt.Equal(int64(20000000), trade1.Price.D) } func TestLiquidityPoolRevoke(t *testing.T) { diff --git a/services/horizon/internal/resourceadapter/trade.go b/services/horizon/internal/resourceadapter/trade.go index 5a392c7506..69466fecb7 100644 --- a/services/horizon/internal/resourceadapter/trade.go +++ b/services/horizon/internal/resourceadapter/trade.go @@ -67,9 +67,9 @@ func PopulateTrade( } if row.HasPrice() { - dest.Price = &protocol.Price{ - N: int32(row.PriceN.Int64), - D: int32(row.PriceD.Int64), + dest.Price = protocol.TradePrice{ + N: row.PriceN.Int64, + D: row.PriceD.Int64, } } diff --git a/services/horizon/internal/resourceadapter/trade_aggregation.go b/services/horizon/internal/resourceadapter/trade_aggregation.go index 8d7c476023..3cafe7fd0d 100644 --- a/services/horizon/internal/resourceadapter/trade_aggregation.go +++ b/services/horizon/internal/resourceadapter/trade_aggregation.go @@ -9,7 +9,7 @@ import ( "github.com/stellar/go/services/horizon/internal/db2/history" ) -// Populate fills out the details of a trade using a row from the history_trades +// PopulateTradeAggregation fills out the details of a trade aggregation using a row from the trade aggregations // table. func PopulateTradeAggregation( ctx context.Context, @@ -28,13 +28,25 @@ func PopulateTradeAggregation( return err } dest.Average = price.StringFromFloat64(row.Average) - dest.High = row.High().String() - dest.HighR = row.High() - dest.Low = row.Low().String() - dest.LowR = row.Low() - dest.Open = row.Open().String() - dest.OpenR = row.Open() - dest.Close = row.Close().String() - dest.CloseR = row.Close() + dest.HighR = protocol.TradePrice{ + N: row.HighN, + D: row.HighD, + } + dest.High = dest.HighR.String() + dest.LowR = protocol.TradePrice{ + N: row.LowN, + D: row.LowD, + } + dest.Low = dest.LowR.String() + dest.OpenR = protocol.TradePrice{ + N: row.OpenN, + D: row.OpenD, + } + dest.Open = dest.OpenR.String() + dest.CloseR = protocol.TradePrice{ + N: row.CloseN, + D: row.CloseD, + } + dest.Close = dest.CloseR.String() return nil } diff --git a/services/ticker/internal/scraper/trade_scraper_test.go b/services/ticker/internal/scraper/trade_scraper_test.go index 2ba1557058..935424923e 100644 --- a/services/ticker/internal/scraper/trade_scraper_test.go +++ b/services/ticker/internal/scraper/trade_scraper_test.go @@ -23,10 +23,10 @@ func TestReverseAssets(t *testing.T) { baseIsSeller := true - n := int32(10) - d := int32(50) + n := int64(10) + d := int64(50) - price := hProtocol.Price{ + price := hProtocol.TradePrice{ N: n, D: d, } @@ -43,7 +43,7 @@ func TestReverseAssets(t *testing.T) { CounterAssetType: counterAssetType, CounterAssetIssuer: counterAssetIssuer, BaseIsSeller: baseIsSeller, - Price: &price, + Price: price, } fmt.Println(trade1) @@ -92,10 +92,10 @@ func TestNormalizeTradeAssets(t *testing.T) { baseAssetType := "BASEASSETTYPE" baseAssetIssuer := "BASEASSETISSUER" - n := int32(10) - d := int32(50) + n := int64(10) + d := int64(50) - price := hProtocol.Price{ + price := hProtocol.TradePrice{ N: n, D: d, } @@ -107,7 +107,7 @@ func TestNormalizeTradeAssets(t *testing.T) { BaseAssetType: baseAssetType, BaseAssetIssuer: baseAssetIssuer, CounterAssetType: "native", - Price: &price, + Price: price, } NormalizeTradeAssets(&trade1) @@ -134,7 +134,7 @@ func TestNormalizeTradeAssets(t *testing.T) { CounterAssetCode: "AAA", CounterAssetType: counterAssetType, CounterAssetIssuer: counterAssetIssuer, - Price: &price, + Price: price, } NormalizeTradeAssets(&trade2) assert.Equal(t, baseAmount, trade2.CounterAmount)