From 5647ac640664f7a746447fc983caf692202a64f1 Mon Sep 17 00:00:00 2001 From: Adolfo Builes Date: Fri, 5 Jun 2020 11:22:56 -0500 Subject: [PATCH 1/3] Use historyQ in path finding action handlers. --- services/horizon/internal/actions_path.go | 10 +- .../horizon/internal/actions_path_test.go | 384 +++++++++++------- .../horizon/internal/paths/mock_finder.go | 30 ++ services/horizon/internal/web.go | 4 +- 4 files changed, 284 insertions(+), 144 deletions(-) create mode 100644 services/horizon/internal/paths/mock_finder.go diff --git a/services/horizon/internal/actions_path.go b/services/horizon/internal/actions_path.go index a769b5db7b..0937289121 100644 --- a/services/horizon/internal/actions_path.go +++ b/services/horizon/internal/actions_path.go @@ -9,7 +9,7 @@ import ( "github.com/stellar/go/amount" "github.com/stellar/go/protocols/horizon" "github.com/stellar/go/services/horizon/internal/actions" - "github.com/stellar/go/services/horizon/internal/db2/core" + "github.com/stellar/go/services/horizon/internal/db2/history" "github.com/stellar/go/services/horizon/internal/ledger" "github.com/stellar/go/services/horizon/internal/paths" hProblem "github.com/stellar/go/services/horizon/internal/render/problem" @@ -30,7 +30,7 @@ type FindPathsHandler struct { setLastLedgerHeader bool maxAssetsParamLength int pathFinder paths.Finder - coreQ *core.Q + historyQ *history.Q } // StrictReceivePathsQuery query struct for paths/strict-send end-point @@ -154,7 +154,7 @@ func (handler FindPathsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request sourceAccount := xdr.MustAddress(sourceAccount) query.SourceAccount = &sourceAccount query.ValidateSourceBalance = true - query.SourceAssets, query.SourceAssetBalances, err = handler.coreQ.AssetsForAddress( + query.SourceAssets, query.SourceAssetBalances, err = handler.historyQ.AssetsForAddress( query.SourceAccount.Address(), ) if err != nil { @@ -212,7 +212,7 @@ type FindFixedPathsHandler struct { maxAssetsParamLength int setLastLedgerHeader bool pathFinder paths.Finder - coreQ *core.Q + historyQ *history.Q } var destinationAssetsOrDestinationAccount = problem.P{ @@ -320,7 +320,7 @@ func (handler FindFixedPathsHandler) ServeHTTP(w http.ResponseWriter, r *http.Re } if destinationAccount != "" { - destinationAssets, _, err = handler.coreQ.AssetsForAddress( + destinationAssets, _, err = handler.historyQ.AssetsForAddress( destinationAccount, ) if err != nil { diff --git a/services/horizon/internal/actions_path_test.go b/services/horizon/internal/actions_path_test.go index 537a3211b4..814207a2a4 100644 --- a/services/horizon/internal/actions_path_test.go +++ b/services/horizon/internal/actions_path_test.go @@ -4,41 +4,42 @@ import ( "fmt" "net/http" "net/url" - "strconv" "strings" "testing" "github.com/go-chi/chi" - "github.com/stellar/go/exp/orderbook" "github.com/stellar/go/protocols/horizon" "github.com/stellar/go/services/horizon/internal/actions" - "github.com/stellar/go/services/horizon/internal/db2" - "github.com/stellar/go/services/horizon/internal/db2/core" + "github.com/stellar/go/services/horizon/internal/db2/history" + "github.com/stellar/go/services/horizon/internal/paths" horizonProblem "github.com/stellar/go/services/horizon/internal/render/problem" "github.com/stellar/go/services/horizon/internal/simplepath" "github.com/stellar/go/services/horizon/internal/test" "github.com/stellar/go/support/render/problem" "github.com/stellar/go/xdr" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" ) -func inMemoryPathFindingClient( +func mockPathFindingClient( tt *test.T, - graph *orderbook.OrderBookGraph, + finder paths.Finder, maxAssetsParamLength int, ) test.RequestHelper { router := chi.NewRouter() findPaths := FindPathsHandler{ - pathFinder: simplepath.NewInMemoryFinder(graph), + pathFinder: finder, maxAssetsParamLength: maxAssetsParamLength, + maxPathLength: 3, setLastLedgerHeader: true, - coreQ: &core.Q{tt.CoreSession()}, + historyQ: &history.Q{tt.HorizonSession()}, } findFixedPaths := FindFixedPathsHandler{ - pathFinder: simplepath.NewInMemoryFinder(graph), + pathFinder: finder, maxAssetsParamLength: maxAssetsParamLength, + maxPathLength: 3, setLastLedgerHeader: true, - coreQ: &core.Q{tt.CoreSession()}, + historyQ: &history.Q{tt.HorizonSession()}, } router.Group(func(r chi.Router) { @@ -51,24 +52,28 @@ func inMemoryPathFindingClient( } func TestPathActionsStillIngesting(t *testing.T) { - tt := test.Start(t).Scenario("paths") + tt := test.Start(t) defer tt.Finish() + test.ResetHorizonDB(t, tt.HorizonDB) + assertions := &Assertions{tt.Assert} - rh := inMemoryPathFindingClient( + finder := paths.MockFinder{} + finder.On("Find", mock.Anything, uint(3)). + Return([]paths.Path{}, uint32(0), simplepath.ErrEmptyInMemoryOrderBook).Times(2) + finder.On("FindFixedPaths", mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return([]paths.Path{}, uint32(0), simplepath.ErrEmptyInMemoryOrderBook).Times(1) + + rh := mockPathFindingClient( tt, - orderbook.NewOrderBookGraph(), - 3, + &finder, + 2, ) var q = make(url.Values) q.Add( - "destination_account", - "GAEDTJ4PPEFVW5XV2S7LUXBEHNQMX5Q2GM562RJGOQG7GVCE5H3HIB4V", - ) - q.Add( - "source_account", - "GARSFJNXJIHO6ULUBK3DBYKVSIZE7SC72S5DYBCHU7DKL22UXKVD7MXP", + "source_assets", + "native", ) q.Add( "destination_asset_issuer", @@ -84,55 +89,125 @@ func TestPathActionsStillIngesting(t *testing.T) { assertions.Problem(w.Body, horizonProblem.StillIngesting) assertions.Equal("", w.Header().Get(actions.LastLedgerHeaderName)) } + + q = make(url.Values) + + q.Add("destination_assets", "native") + q.Add("source_asset_issuer", "GDSBCQO34HWPGUGQSP3QBFEXVTSR2PW46UIGTHVWGWJGQKH3AFNHXHXN") + q.Add("source_asset_type", "credit_alphanum4") + q.Add("source_asset_code", "EUR") + q.Add("source_amount", "10") + + w := rh.Get("/paths/strict-send" + "?" + q.Encode()) + assertions.Equal(horizonProblem.StillIngesting.Status, w.Code) + assertions.Problem(w.Body, horizonProblem.StillIngesting) + assertions.Equal("", w.Header().Get(actions.LastLedgerHeaderName)) + + finder.AssertExpectations(t) } -func loadOffers( - tt *test.T, - orderBookGraph *orderbook.OrderBookGraph, - fromAddress string, - ledger uint32, -) { - coreQ := &core.Q{tt.CoreSession()} - offers := []core.Offer{} - pageQuery := db2.PageQuery{ - Order: db2.OrderAscending, - Limit: 100, +func TestPathActionsStrictReceive(t *testing.T) { + tt := test.Start(t) + defer tt.Finish() + test.ResetHorizonDB(t, tt.HorizonDB) + sourceAssets := []xdr.Asset{ + xdr.MustNewCreditAsset("AAA", "GDSBCQO34HWPGUGQSP3QBFEXVTSR2PW46UIGTHVWGWJGQKH3AFNHXHXN"), + xdr.MustNewCreditAsset("USD", "GDSBCQO34HWPGUGQSP3QBFEXVTSR2PW46UIGTHVWGWJGQKH3AFNHXHXN"), + xdr.MustNewNativeAsset(), } - err := coreQ.OffersByAddress(&offers, fromAddress, pageQuery) - tt.Assert.NoError(err) - for _, offer := range offers { - - orderBookGraph.AddOffer(xdr.OfferEntry{ - SellerId: xdr.MustAddress(offer.SellerID), - OfferId: xdr.Int64(offer.OfferID), - Selling: offer.SellingAsset, - Buying: offer.BuyingAsset, - Amount: offer.Amount, - Price: xdr.Price{N: xdr.Int32(offer.Price * 100), D: 100}, - }) + sourceAccount := "GARSFJNXJIHO6ULUBK3DBYKVSIZE7SC72S5DYBCHU7DKL22UXKVD7MXP" + + q := &history.Q{tt.HorizonSession()} + + account := xdr.AccountEntry{ + AccountId: xdr.MustAddress(sourceAccount), + Balance: 20000, + SeqNum: 223456789, + NumSubEntries: 10, + Flags: 1, + Thresholds: xdr.Thresholds{1, 2, 3, 4}, + Ext: xdr.AccountEntryExt{ + V: 1, + V1: &xdr.AccountEntryV1{ + Liabilities: xdr.Liabilities{ + Buying: 3, + Selling: 4, + }, + }, + }, } - tt.Assert.NoError(orderBookGraph.Apply(ledger)) -} -func TestPathActionsInMemoryFinder(t *testing.T) { - tt := test.Start(t).Scenario("paths") - defer tt.Finish() - orderBookGraph := orderbook.NewOrderBookGraph() + batch := q.NewAccountsBatchInsertBuilder(0) + err := batch.Add(account, 1234) + assert.NoError(t, err) + err = batch.Exec() + assert.NoError(t, err) - coreQ := &core.Q{tt.CoreSession()} - sourceAccount := "GARSFJNXJIHO6ULUBK3DBYKVSIZE7SC72S5DYBCHU7DKL22UXKVD7MXP" - sourceAssets, _, err := coreQ.AssetsForAddress(sourceAccount) - tt.Assert.NoError(err) + assetsByKeys := map[string]xdr.Asset{} + + for _, asset := range sourceAssets { + code := asset.String() + assetsByKeys[code] = asset + if code == "native" { + continue + } + trustline := xdr.TrustLineEntry{ + AccountId: xdr.MustAddress(sourceAccount), + Asset: asset, + Balance: 10000, + Limit: 123456789, + Flags: 0, + Ext: xdr.TrustLineEntryExt{ + V: 1, + V1: &xdr.TrustLineEntryV1{ + Liabilities: xdr.Liabilities{ + Buying: 1, + Selling: 2, + }, + }, + }, + } + + rows, err1 := q.InsertTrustLine(trustline, 1234) + assert.NoError(t, err1) + assert.Equal(t, int64(1), rows) + } + + finder := paths.MockFinder{} + withSourceAssetsBalance := true + + finder.On("Find", mock.Anything, uint(3)).Return([]paths.Path{}, uint32(1234), nil).Run(func(args mock.Arguments) { + query := args.Get(0).(paths.Query) + for _, asset := range query.SourceAssets { + var assetType, code, issuer string + + asset.MustExtract(&assetType, &code, &issuer) + if assetType == "native" { + tt.Assert.NotNil(assetsByKeys["native"]) + } else { + tt.Assert.NotNil(assetsByKeys[code]) + } + + } + tt.Assert.Equal(xdr.MustNewCreditAsset("EUR", "GDSBCQO34HWPGUGQSP3QBFEXVTSR2PW46UIGTHVWGWJGQKH3AFNHXHXN"), query.DestinationAsset) + tt.Assert.Equal(xdr.Int64(100000000), query.DestinationAmount) - inMemoryPathsClient := inMemoryPathFindingClient( + if withSourceAssetsBalance { + tt.Assert.Equal([]xdr.Int64{10000, 10000, 20000}, query.SourceAssetBalances) + tt.Assert.True(query.ValidateSourceBalance) + } else { + tt.Assert.Equal([]xdr.Int64{0, 0, 0}, query.SourceAssetBalances) + tt.Assert.False(query.ValidateSourceBalance) + } + + }).Times(4) + + rh := mockPathFindingClient( tt, - orderBookGraph, + &finder, len(sourceAssets), ) - loadOffers(tt, orderBookGraph, "GA2NC4ZOXMXLVQAQQ5IQKJX47M3PKBQV2N5UV5Z4OXLQJ3CKMBA2O2YL", 1) - loadOffers(tt, orderBookGraph, "GDSBCQO34HWPGUGQSP3QBFEXVTSR2PW46UIGTHVWGWJGQKH3AFNHXHXN", 2) - var withSourceAccount = make(url.Values) withSourceAccount.Add( "destination_account", @@ -158,38 +233,31 @@ func TestPathActionsInMemoryFinder(t *testing.T) { withSourceAssets.Add("source_assets", assetsToURLParam(sourceAssets)) for _, uri := range []string{"/paths", "/paths/strict-receive"} { - w := inMemoryPathsClient.Get(uri + "?" + withSourceAccount.Encode()) + w := rh.Get(uri + "?" + withSourceAccount.Encode()) tt.Assert.Equal(http.StatusOK, w.Code) - inMemorySourceAccountResponse := []horizon.Path{} - tt.UnmarshalPage(w.Body, &inMemorySourceAccountResponse) - tt.Assert.Equal("2", w.Header().Get(actions.LastLedgerHeaderName)) + tt.Assert.Equal("1234", w.Header().Get(actions.LastLedgerHeaderName)) - tt.Assert.True(len(inMemorySourceAccountResponse) > 0) - - w = inMemoryPathsClient.Get(uri + "?" + withSourceAssets.Encode()) + withSourceAssetsBalance = false + w = rh.Get(uri + "?" + withSourceAssets.Encode()) tt.Assert.Equal(http.StatusOK, w.Code) - inMemorySourceAssetsResponse := []horizon.Path{} - tt.UnmarshalPage(w.Body, &inMemorySourceAssetsResponse) - tt.Assert.Equal("2", w.Header().Get(actions.LastLedgerHeaderName)) - - tt.Assert.Equal(inMemorySourceAssetsResponse, inMemorySourceAccountResponse) + tt.Assert.Equal("1234", w.Header().Get(actions.LastLedgerHeaderName)) + withSourceAssetsBalance = true } + + finder.AssertExpectations(t) } func TestPathActionsEmptySourceAcount(t *testing.T) { - tt := test.Start(t).Scenario("paths") + tt := test.Start(t) defer tt.Finish() - orderBookGraph := orderbook.NewOrderBookGraph() + test.ResetHorizonDB(t, tt.HorizonDB) assertions := &Assertions{tt.Assert} - inMemoryPathsClient := inMemoryPathFindingClient( + finder := paths.MockFinder{} + rh := mockPathFindingClient( tt, - orderBookGraph, - 3, + &finder, + 2, ) - - loadOffers(tt, orderBookGraph, "GA2NC4ZOXMXLVQAQQ5IQKJX47M3PKBQV2N5UV5Z4OXLQJ3CKMBA2O2YL", 1) - loadOffers(tt, orderBookGraph, "GDSBCQO34HWPGUGQSP3QBFEXVTSR2PW46UIGTHVWGWJGQKH3AFNHXHXN", 2) - var q = make(url.Values) q.Add( @@ -210,7 +278,7 @@ func TestPathActionsEmptySourceAcount(t *testing.T) { q.Add("destination_amount", "10") for _, uri := range []string{"/paths", "/paths/strict-receive"} { - w := inMemoryPathsClient.Get(uri + "?" + q.Encode()) + w := rh.Get(uri + "?" + q.Encode()) assertions.Equal(http.StatusOK, w.Code) inMemoryResponse := []horizon.Path{} tt.UnmarshalPage(w.Body, &inMemoryResponse) @@ -220,14 +288,15 @@ func TestPathActionsEmptySourceAcount(t *testing.T) { } func TestPathActionsSourceAssetsValidation(t *testing.T) { - tt := test.Start(t).Scenario("paths") + tt := test.Start(t) defer tt.Finish() + test.ResetHorizonDB(t, tt.HorizonDB) assertions := &Assertions{tt.Assert} - orderBookGraph := orderbook.NewOrderBookGraph() - rh := inMemoryPathFindingClient( + finder := paths.MockFinder{} + rh := mockPathFindingClient( tt, - orderBookGraph, - 3, + &finder, + 2, ) missingSourceAccountAndAssets := make(url.Values) @@ -298,16 +367,16 @@ func TestPathActionsSourceAssetsValidation(t *testing.T) { } func TestPathActionsDestinationAssetsValidation(t *testing.T) { - tt := test.Start(t).Scenario("paths") + tt := test.Start(t) defer tt.Finish() + test.ResetHorizonDB(t, tt.HorizonDB) assertions := &Assertions{tt.Assert} - orderBookGraph := orderbook.NewOrderBookGraph() - rh := inMemoryPathFindingClient( + finder := paths.MockFinder{} + rh := mockPathFindingClient( tt, - orderBookGraph, - 3, + &finder, + 2, ) - missingDestinationAccountAndAssets := make(url.Values) missingDestinationAccountAndAssets.Add( "source_asset_issuer", @@ -380,25 +449,97 @@ func TestPathActionsDestinationAssetsValidation(t *testing.T) { } func TestPathActionsStrictSend(t *testing.T) { - tt := test.Start(t).Scenario("paths") + tt := test.Start(t) defer tt.Finish() + test.ResetHorizonDB(t, tt.HorizonDB) assertions := &Assertions{tt.Assert} - orderBookGraph := orderbook.NewOrderBookGraph() + historyQ := &history.Q{tt.HorizonSession()} + destinationAccount := "GARSFJNXJIHO6ULUBK3DBYKVSIZE7SC72S5DYBCHU7DKL22UXKVD7MXP" + destinationAssets := []xdr.Asset{ + xdr.MustNewCreditAsset("AAA", "GDSBCQO34HWPGUGQSP3QBFEXVTSR2PW46UIGTHVWGWJGQKH3AFNHXHXN"), + xdr.MustNewCreditAsset("USD", "GDSBCQO34HWPGUGQSP3QBFEXVTSR2PW46UIGTHVWGWJGQKH3AFNHXHXN"), + xdr.MustNewNativeAsset(), + } - coreQ := &core.Q{tt.CoreSession()} - destinationAccount := "GA2NC4ZOXMXLVQAQQ5IQKJX47M3PKBQV2N5UV5Z4OXLQJ3CKMBA2O2YL" - destinationAssets, _, err := coreQ.AssetsForAddress(destinationAccount) - tt.Assert.NoError(err) + account := xdr.AccountEntry{ + AccountId: xdr.MustAddress(destinationAccount), + Balance: 20000, + SeqNum: 223456789, + NumSubEntries: 10, + Flags: 1, + Thresholds: xdr.Thresholds{1, 2, 3, 4}, + Ext: xdr.AccountEntryExt{ + V: 1, + V1: &xdr.AccountEntryV1{ + Liabilities: xdr.Liabilities{ + Buying: 3, + Selling: 4, + }, + }, + }, + } + + batch := historyQ.NewAccountsBatchInsertBuilder(0) + err := batch.Add(account, 1234) + assert.NoError(t, err) + err = batch.Exec() + assert.NoError(t, err) + + assetsByKeys := map[string]xdr.Asset{} - rh := inMemoryPathFindingClient( + for _, asset := range destinationAssets { + code := asset.String() + assetsByKeys[code] = asset + if code == "native" { + continue + } + trustline := xdr.TrustLineEntry{ + AccountId: xdr.MustAddress(destinationAccount), + Asset: asset, + Balance: 10000, + Limit: 123456789, + Flags: 0, + Ext: xdr.TrustLineEntryExt{ + V: 1, + V1: &xdr.TrustLineEntryV1{ + Liabilities: xdr.Liabilities{ + Buying: 1, + Selling: 2, + }, + }, + }, + } + + rows, err := historyQ.InsertTrustLine(trustline, 1234) + assert.NoError(t, err) + assert.Equal(t, int64(1), rows) + } + + finder := paths.MockFinder{} + // withSourceAssetsBalance := true + sourceAsset := xdr.MustNewCreditAsset("USD", "GDSBCQO34HWPGUGQSP3QBFEXVTSR2PW46UIGTHVWGWJGQKH3AFNHXHXN") + + finder.On("FindFixedPaths", sourceAsset, xdr.Int64(100000000), mock.Anything, uint(3)).Return([]paths.Path{}, uint32(1234), nil).Run(func(args mock.Arguments) { + destinationAssets := args.Get(2).([]xdr.Asset) + for _, asset := range destinationAssets { + var assetType, code, issuer string + + asset.MustExtract(&assetType, &code, &issuer) + if assetType == "native" { + tt.Assert.NotNil(assetsByKeys["native"]) + } else { + tt.Assert.NotNil(assetsByKeys[code]) + } + + } + }).Times(2) + + rh := mockPathFindingClient( tt, - orderBookGraph, + &finder, len(destinationAssets), ) - loadOffers(tt, orderBookGraph, "GA2NC4ZOXMXLVQAQQ5IQKJX47M3PKBQV2N5UV5Z4OXLQJ3CKMBA2O2YL", 1) - loadOffers(tt, orderBookGraph, "GDSBCQO34HWPGUGQSP3QBFEXVTSR2PW46UIGTHVWGWJGQKH3AFNHXHXN", 2) - var q = make(url.Values) q.Add( @@ -415,46 +556,15 @@ func TestPathActionsStrictSend(t *testing.T) { w := rh.Get("/paths/strict-send?" + q.Encode()) assertions.Equal(http.StatusOK, w.Code) - accountResponse := []horizon.Path{} - tt.UnmarshalPage(w.Body, &accountResponse) - assertions.Len(accountResponse, 12) - assertions.Equal("2", w.Header().Get(actions.LastLedgerHeaderName)) - - for i, path := range accountResponse { - assertions.Equal(path.SourceAssetCode, "USD") - assertions.Equal(path.SourceAssetType, "credit_alphanum4") - assertions.Equal(path.SourceAssetIssuer, "GDSBCQO34HWPGUGQSP3QBFEXVTSR2PW46UIGTHVWGWJGQKH3AFNHXHXN") - assertions.Equal(path.SourceAmount, "10.0000000") - - if path.DestinationAssetType == "credit_alphanum4" && path.DestinationAssetCode == "USD" { - assertions.Equal(path.DestinationAssetIssuer, "GDSBCQO34HWPGUGQSP3QBFEXVTSR2PW46UIGTHVWGWJGQKH3AFNHXHXN") - assertions.Equal(path.DestinationAmount, "10.0000000") - assertions.Len(path.Path, 0) - } - - if i > 1 && - accountResponse[i-1].DestinationAssetType == path.DestinationAssetType && - accountResponse[i-1].DestinationAssetCode == path.DestinationAssetCode && - accountResponse[i-1].DestinationAssetIssuer == path.DestinationAssetIssuer { - previous, err := strconv.ParseFloat(accountResponse[i-1].DestinationAmount, 64) - assertions.NoError(err) - - current, err := strconv.ParseFloat(path.DestinationAmount, 64) - assertions.NoError(err) - - assertions.True(previous >= current) - } - } + assertions.Equal("1234", w.Header().Get(actions.LastLedgerHeaderName)) q.Del("destination_account") q.Add("destination_assets", assetsToURLParam(destinationAssets)) w = rh.Get("/paths/strict-send?" + q.Encode()) assertions.Equal(http.StatusOK, w.Code) - assetListResponse := []horizon.Path{} - tt.UnmarshalPage(w.Body, &assetListResponse) - assertions.Len(assetListResponse, 12) - tt.Assert.Equal(accountResponse, assetListResponse) - assertions.Equal("2", w.Header().Get(actions.LastLedgerHeaderName)) + assertions.Equal("1234", w.Header().Get(actions.LastLedgerHeaderName)) + + finder.AssertExpectations(t) } func assetsToURLParam(xdrAssets []xdr.Asset) string { diff --git a/services/horizon/internal/paths/mock_finder.go b/services/horizon/internal/paths/mock_finder.go new file mode 100644 index 0000000000..d98bd86f9c --- /dev/null +++ b/services/horizon/internal/paths/mock_finder.go @@ -0,0 +1,30 @@ +package paths + +import ( + "github.com/stellar/go/xdr" + "github.com/stretchr/testify/mock" +) + +var _ Finder = (*MockFinder)(nil) + +// MockFinder is a mock implementation of the Finder interface +type MockFinder struct { + mock.Mock +} + +func (m *MockFinder) Find(q Query, maxLength uint) ([]Path, uint32, error) { + args := m.Called(q, maxLength) + + return args.Get(0).([]Path), args.Get(1).(uint32), args.Error(2) +} + +func (m *MockFinder) FindFixedPaths( + sourceAsset xdr.Asset, + amountToSpend xdr.Int64, + destinationAssets []xdr.Asset, + maxLength uint, +) ([]Path, uint32, error) { + args := m.Called(sourceAsset, amountToSpend, destinationAssets, maxLength) + + return args.Get(0).([]Path), args.Get(1).(uint32), args.Error(2) +} diff --git a/services/horizon/internal/web.go b/services/horizon/internal/web.go index 191702be02..8d2d6d63e3 100644 --- a/services/horizon/internal/web.go +++ b/services/horizon/internal/web.go @@ -187,14 +187,14 @@ func (w *web) mustInstallActions(config Config, pathFinder paths.Finder, session maxPathLength: config.MaxPathLength, maxAssetsParamLength: maxAssetsForPathFinding, pathFinder: pathFinder, - coreQ: w.coreQ, + historyQ: w.historyQ, } findFixedPaths := FindFixedPathsHandler{ maxPathLength: config.MaxPathLength, setLastLedgerHeader: true, maxAssetsParamLength: maxAssetsForPathFinding, pathFinder: pathFinder, - coreQ: w.coreQ, + historyQ: w.historyQ, } r.Method(http.MethodGet, "/paths", findPaths) From de9cc88de5d3439eb75184cbd5634cd1c966fa09 Mon Sep 17 00:00:00 2001 From: Adolfo Builes Date: Fri, 5 Jun 2020 16:15:02 -0500 Subject: [PATCH 2/3] Require a repeatable read tx for loading account assets. --- .../internal/db2/history/trust_lines.go | 8 +++++ .../internal/db2/history/trust_lines_test.go | 36 +++++++++++++++---- 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/services/horizon/internal/db2/history/trust_lines.go b/services/horizon/internal/db2/history/trust_lines.go index 2880483787..12ee52e5db 100644 --- a/services/horizon/internal/db2/history/trust_lines.go +++ b/services/horizon/internal/db2/history/trust_lines.go @@ -1,6 +1,7 @@ package history import ( + "database/sql" "encoding/base64" sq "github.com/Masterminds/squirrel" @@ -24,6 +25,13 @@ func (trustLine TrustLine) IsAuthorizedToMaintainLiabilities() bool { // AssetsForAddress returns a list of assets and balances for those assets held by // a given address. func (q *Q) AssetsForAddress(addy string) ([]xdr.Asset, []xdr.Int64, error) { + if tx := q.GetTx(); tx == nil { + return nil, nil, errors.New("cannot be called outside of a transaction") + } + if opts := q.GetTxOptions(); opts == nil || !opts.ReadOnly || opts.Isolation != sql.LevelRepeatableRead { + return nil, nil, errors.New("should only be called in a repeatable read transaction") + } + account, err := q.GetAccountByID(addy) if q.NoRows(err) { diff --git a/services/horizon/internal/db2/history/trust_lines_test.go b/services/horizon/internal/db2/history/trust_lines_test.go index 764a6736b6..b283d372e0 100644 --- a/services/horizon/internal/db2/history/trust_lines_test.go +++ b/services/horizon/internal/db2/history/trust_lines_test.go @@ -1,6 +1,7 @@ package history import ( + "database/sql" "testing" "github.com/stellar/go/services/horizon/internal/test" @@ -366,16 +367,27 @@ func TestGetTrustLinesByAccountID(t *testing.T) { } -func TestAssetsForAddress(t *testing.T) { +func TestAssetsForAddressRequiresTransaction(t *testing.T) { tt := test.Start(t) defer tt.Finish() test.ResetHorizonDB(t, tt.HorizonDB) q := &Q{tt.HorizonSession()} - assets, balances, err := q.AssetsForAddress(eurTrustLine.AccountId.Address()) - tt.Assert.NoError(err) - tt.Assert.Empty(assets) - tt.Assert.Empty(balances) + _, _, err := q.AssetsForAddress(eurTrustLine.AccountId.Address()) + assert.EqualError(t, err, "cannot be called outside of a transaction") + + assert.NoError(t, q.Begin()) + defer q.Rollback() + + _, _, err = q.AssetsForAddress(eurTrustLine.AccountId.Address()) + assert.EqualError(t, err, "should only be called in a repeatable read transaction") +} + +func TestAssetsForAddress(t *testing.T) { + tt := test.Start(t) + defer tt.Finish() + test.ResetHorizonDB(t, tt.HorizonDB) + q := &Q{tt.HorizonSession()} ledgerEntries := []xdr.LedgerEntry{ xdr.LedgerEntry{ @@ -387,7 +399,7 @@ func TestAssetsForAddress(t *testing.T) { }, } - err = q.UpsertAccounts(ledgerEntries) + err := q.UpsertAccounts(ledgerEntries) assert.NoError(t, err) _, err = q.InsertTrustLine(eurTrustLine, 1234) @@ -413,6 +425,18 @@ func TestAssetsForAddress(t *testing.T) { _, err = q.InsertTrustLine(brlTrustLine, 1234) tt.Assert.NoError(err) + err = q.BeginTx(&sql.TxOptions{ + Isolation: sql.LevelRepeatableRead, + ReadOnly: true, + }) + assert.NoError(t, err) + defer q.Rollback() + + assets, balances, err := q.AssetsForAddress(usdTrustLine.AccountId.Address()) + tt.Assert.NoError(err) + tt.Assert.Empty(assets) + tt.Assert.Empty(balances) + assets, balances, err = q.AssetsForAddress(account1.AccountId.Address()) tt.Assert.NoError(err) From e769b25b355bc6de12acbb5b4570c8929ed826ed Mon Sep 17 00:00:00 2001 From: Adolfo Builes Date: Fri, 5 Jun 2020 16:43:36 -0500 Subject: [PATCH 3/3] Create repeatable read tx before callling AssetsForAddress --- services/horizon/internal/actions_path.go | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/services/horizon/internal/actions_path.go b/services/horizon/internal/actions_path.go index 0937289121..0b536766bc 100644 --- a/services/horizon/internal/actions_path.go +++ b/services/horizon/internal/actions_path.go @@ -2,6 +2,7 @@ package horizon import ( "context" + "database/sql" "fmt" "net/http" "strings" @@ -154,9 +155,7 @@ func (handler FindPathsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request sourceAccount := xdr.MustAddress(sourceAccount) query.SourceAccount = &sourceAccount query.ValidateSourceBalance = true - query.SourceAssets, query.SourceAssetBalances, err = handler.historyQ.AssetsForAddress( - query.SourceAccount.Address(), - ) + query.SourceAssets, query.SourceAssetBalances, err = assetsForAddress(handler.historyQ, query.SourceAccount.Address()) if err != nil { problem.Render(ctx, w, err) return @@ -320,9 +319,7 @@ func (handler FindFixedPathsHandler) ServeHTTP(w http.ResponseWriter, r *http.Re } if destinationAccount != "" { - destinationAssets, _, err = handler.historyQ.AssetsForAddress( - destinationAccount, - ) + destinationAssets, _, err = assetsForAddress(handler.historyQ, destinationAccount) if err != nil { problem.Render(ctx, w, err) return @@ -359,3 +356,15 @@ func (handler FindFixedPathsHandler) ServeHTTP(w http.ResponseWriter, r *http.Re renderPaths(ctx, records, w) } + +func assetsForAddress(historyQ *history.Q, addy string) ([]xdr.Asset, []xdr.Int64, error) { + err := historyQ.BeginTx(&sql.TxOptions{ + Isolation: sql.LevelRepeatableRead, + ReadOnly: true, + }) + if err != nil { + return nil, nil, err + } + defer historyQ.Rollback() + return historyQ.AssetsForAddress(addy) +}