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

horizon: Revert transient protocol18 trades changes #4153

Merged
merged 1 commit into from
Jan 3, 2022
Merged

horizon: Revert transient protocol18 trades changes #4153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
133 changes: 0 additions & 133 deletions services/horizon/internal/actions/protocol17_resourceadapters.go
View file
Open in desktop

This file was deleted.

102 changes: 11 additions & 91 deletions services/horizon/internal/actions/trade.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,64 +20,6 @@ import (
"github.com/stellar/go/xdr"
)

// TradeP17 represents a horizon digested trade.
// This can be removed in release after P18 upgrade.
type TradeP17 struct {
Links struct {
Self hal.Link `json:"self"`
Base hal.Link `json:"base"`
Counter hal.Link `json:"counter"`
Operation hal.Link `json:"operation"`
} `json:"_links"`

ID string `json:"id"`
PT string `json:"paging_token"`
LedgerCloseTime gTime.Time `json:"ledger_close_time"`
OfferID string `json:"offer_id"`
BaseOfferID string `json:"base_offer_id"`
BaseAccount string `json:"base_account"`
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"`
CounterOfferID string `json:"counter_offer_id"`
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 *horizon.Price `json:"price"`
}

// PagingToken implementation for hal.Pageable
func (res TradeP17) PagingToken() string {
return res.PT
}

// TradeAggregationP17 represents trade data aggregation over a period of time
// This can be removed in release after P18 upgrade.
type TradeAggregationP17 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"`
}

// PagingToken implementation for hal.Pageable. Not actually used
func (res TradeAggregationP17) PagingToken() string {
return strconv.FormatInt(res.Timestamp, 10)
}

// TradeAssetsQueryParams represents the base and counter assets on trade related end-points.
type TradeAssetsQueryParams struct {
BaseAssetType string `schema:"base_asset_type" valid:"assetType,optional"`
Expand Down Expand Up @@ -266,19 +208,10 @@ func (handler GetTradesHandler) GetResourcePage(w HeaderWriter, r *http.Request)
}

var response []hal.Pageable
protocolVersion := handler.GetCoreState().CurrentProtocolVersion
for _, record := range records {
if protocolVersion >= 18 {
var res horizon.Trade
resourceadapter.PopulateTrade(ctx, &res, record)
response = append(response, res)
} else {
// This can be removed in release after P18 upgrade and is here
// to remove breaking change before protocol upgrade.
var res TradeP17
PopulateTradeP17(ctx, &res, record)
response = append(response, res)
}
var res horizon.Trade
resourceadapter.PopulateTrade(ctx, &res, record)
response = append(response, res)
}

return response, nil
Expand Down Expand Up @@ -373,22 +306,13 @@ func (handler GetTradeAggregationsHandler) GetResource(w HeaderWriter, r *http.R
return nil, err
}
var aggregations []hal.Pageable
protocolVersion := handler.GetCoreState().CurrentProtocolVersion
for _, record := range records {
if protocolVersion >= 18 {
var res horizon.TradeAggregation
err = resourceadapter.PopulateTradeAggregation(ctx, &res, record)
if err != nil {
return nil, err
}
aggregations = append(aggregations, res)
} else {
// This can be removed in release after P18 upgrade and is here
// to remove breaking change before protocol upgrade.
var res TradeAggregationP17
PopulateTradeAggregationP17(ctx, &res, record)
aggregations = append(aggregations, res)
var res horizon.TradeAggregation
err = resourceadapter.PopulateTradeAggregation(ctx, &res, record)
if err != nil {
return nil, err
}
aggregations = append(aggregations, res)
}

return handler.buildPage(r, aggregations)
Expand Down Expand Up @@ -514,15 +438,11 @@ func (handler GetTradeAggregationsHandler) buildPage(r *http.Request, records []
} else {
lastRecord := records[len(records)-1]

var timestamp int64
switch lastRecord := lastRecord.(type) {
case horizon.TradeAggregation:
timestamp = lastRecord.Timestamp
case TradeAggregationP17:
timestamp = lastRecord.Timestamp
default:
lastRecordTA, ok := lastRecord.(horizon.TradeAggregation)
if !ok {
panic(fmt.Sprintf("Unknown type: %T", lastRecord))
}
timestamp := lastRecordTA.Timestamp

if page.Order == "asc" {
newStartTime := timestamp + int64(qp.ResolutionFilter)
Expand Down
107 changes: 0 additions & 107 deletions services/horizon/internal/integration/protocol18_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,8 @@
package integration

import (
"context"
"fmt"
"io/ioutil"
"net/http"
"strconv"
"strings"
"testing"
"time"

"github.com/stretchr/testify/assert"

Expand Down Expand Up @@ -765,104 +759,3 @@ func TestLiquidityPoolFailedDepositAndWithdraw(t *testing.T) {

tt.Equal("0.0000010", withdrawal.Shares)
}

// TestProtocol18TradesPriceBreakingChanges checks if the breaking changes in
// `/trades` and `/trade_aggegations` (`price.n` and `price.d` change to string)
// activate after the protocol upgrade.
func TestProtocol18TradesPriceBreakingChanges(t *testing.T) {
tt := assert.New(t)
// Start at Protocol 17!
config := integration.Config{ProtocolVersion: 17}
itest := integration.NewTest(t, config)
master := itest.Master()

keys, accounts := itest.CreateAccounts(1, "1000")
tradeKeys, tradeAccount := keys[0], accounts[0]

itest.MustSubmitMultiSigOperations(tradeAccount, []*keypair.Full{tradeKeys, master},
&txnbuild.ChangeTrust{
Line: txnbuild.ChangeTrustAssetWrapper{
Asset: txnbuild.CreditAsset{
Code: "USD",
Issuer: master.Address(),
},
},
Limit: txnbuild.MaxTrustlineLimit,
},
&txnbuild.ManageBuyOffer{
Selling: txnbuild.NativeAsset{},
Buying: txnbuild.CreditAsset{
Code: "USD",
Issuer: master.Address(),
},
Amount: "500",
Price: "1",
},
&txnbuild.ManageBuyOffer{
SourceAccount: master.Address(),
Selling: txnbuild.CreditAsset{
Code: "USD",
Issuer: master.Address(),
},
Buying: txnbuild.NativeAsset{},
Amount: "500",
Price: "1",
},
)

body, err := getHorizonResponseBody(itest, "/trades")
tt.NoError(err)
tt.Contains(body, `"offer_id": "1",`) // Contains non-empty offer_id
tt.Contains(body, `"n": 1,`) // Number
tt.Contains(body, `"d": 1`) // Number

body, err = getHorizonResponseBody(itest,
"/trade_aggregations?"+
"base_asset_type=credit_alphanum4&"+
"base_asset_code=USD&"+
"base_asset_issuer="+master.Address()+"&"+
"counter_asset_type=native&"+
"start_time="+strconv.FormatInt(time.Now().Unix()-3600, 10)+"000&"+
"end_time="+strconv.FormatInt(time.Now().Unix()+60, 10)+"000&"+
"resolution=60000&order=desc&limit=200")
tt.NoError(err)
tt.Contains(body, `"N": 1,`) // Number
tt.Contains(body, `"D": 1`) // Number

// Now upgrade to protocol 18 and update core info manually to check
// responses format
itest.UpgradeProtocol(18)
itest.Horizon().UpdateStellarCoreInfo(context.Background())

body, err = getHorizonResponseBody(itest, "/trades")
tt.NoError(err)
tt.NotContains(body, `"offer_id"`) // No offer_id
tt.Contains(body, `"n": "1",`) // String
tt.Contains(body, `"d": "1"`) // String

body, err = getHorizonResponseBody(itest,
"/trade_aggregations?"+
"base_asset_type=credit_alphanum4&"+
"base_asset_code=USD&"+
"base_asset_issuer="+master.Address()+"&"+
"counter_asset_type=native&"+
"start_time="+strconv.FormatInt(time.Now().Unix()-3600, 10)+"000&"+
"end_time="+strconv.FormatInt(time.Now().Unix()+60, 10)+"000&"+
"resolution=60000&order=desc&limit=200")
tt.NoError(err)
tt.Contains(body, `"n": "1",`) // String
tt.Contains(body, `"n": "1"`) // String
tt.NotContains(body, `"N": 1,`) // Not Number
tt.NotContains(body, `"D": 1`) // Not Number
}

func getHorizonResponseBody(itest *integration.Test, query string) (string, error) {
query = strings.TrimLeft(query, "/")
resp, err := http.Get(fmt.Sprintf("%s%s", itest.Client().HorizonURL, query))
if err != nil {
return "", err
}
body, err := ioutil.ReadAll(resp.Body)
resp.Body.Close()
return string(body), err
}