From eedcd1af30b91be61f15a52c3acdff39174454c1 Mon Sep 17 00:00:00 2001 From: Bartek Nowotarski Date: Wed, 2 Jun 2021 16:35:58 +0200 Subject: [PATCH 1/5] services/horizon: Return 503 error in POST /transactions if Stellar-Core not synced --- services/horizon/internal/actions/helpers.go | 4 +++ services/horizon/internal/actions/root.go | 22 +++++-------- .../internal/actions/submit_transaction.go | 6 ++++ services/horizon/internal/app.go | 31 +++---------------- services/horizon/internal/httpx/router.go | 13 ++++---- services/horizon/internal/init.go | 2 +- .../internal/render/problem/problem.go | 4 +-- 7 files changed, 33 insertions(+), 49 deletions(-) diff --git a/services/horizon/internal/actions/helpers.go b/services/horizon/internal/actions/helpers.go index 49986c07c3..59d5e8410d 100644 --- a/services/horizon/internal/actions/helpers.go +++ b/services/horizon/internal/actions/helpers.go @@ -345,6 +345,10 @@ func getAsset(r *http.Request, prefix string) (xdr.Asset, error) { func getURLParam(r *http.Request, key string) (string, bool) { rctx := chi.RouteContext(r.Context()) + if rctx == nil { + return "", false + } + // Return immediately if keys does not match Values // This can happen when a named param is not specified. // This is a bug in chi: https://github.com/go-chi/chi/issues/426 diff --git a/services/horizon/internal/actions/root.go b/services/horizon/internal/actions/root.go index 054be6d51f..89857f1906 100644 --- a/services/horizon/internal/actions/root.go +++ b/services/horizon/internal/actions/root.go @@ -5,24 +5,18 @@ import ( "net/url" "github.com/stellar/go/protocols/horizon" + "github.com/stellar/go/services/horizon/internal/corestate" "github.com/stellar/go/services/horizon/internal/ledger" "github.com/stellar/go/services/horizon/internal/resourceadapter" ) -type CoreSettings struct { - Synced bool - CurrentProtocolVersion int32 - CoreSupportedProtocolVersion int32 - CoreVersion string -} - -type CoreSettingsGetter interface { - GetCoreSettings() CoreSettings +type CoreStateGetter interface { + GetCoreState() corestate.State } type GetRootHandler struct { LedgerState *ledger.State - CoreSettingsGetter + CoreStateGetter NetworkPassphrase string FriendbotURL *url.URL HorizonVersion string @@ -37,16 +31,16 @@ func (handler GetRootHandler) GetResource(w HeaderWriter, r *http.Request) (inte "strictReceivePaths": StrictReceivePathsQuery{}.URITemplate(), "strictSendPaths": FindFixedPathsQuery{}.URITemplate(), } - coreSettings := handler.GetCoreSettings() + coreState := handler.GetCoreState() resourceadapter.PopulateRoot( r.Context(), &res, handler.LedgerState.CurrentStatus(), handler.HorizonVersion, - coreSettings.CoreVersion, + coreState.CoreVersion, handler.NetworkPassphrase, - coreSettings.CurrentProtocolVersion, - coreSettings.CoreSupportedProtocolVersion, + coreState.CurrentProtocolVersion, + coreState.CoreSupportedProtocolVersion, handler.FriendbotURL, templates, ) diff --git a/services/horizon/internal/actions/submit_transaction.go b/services/horizon/internal/actions/submit_transaction.go index 11055d829d..13dae57a29 100644 --- a/services/horizon/internal/actions/submit_transaction.go +++ b/services/horizon/internal/actions/submit_transaction.go @@ -19,6 +19,7 @@ import ( type SubmitTransactionHandler struct { Submitter *txsub.System NetworkPassphrase string + CoreStateGetter } type envelopeInfo struct { @@ -136,6 +137,11 @@ func (handler SubmitTransactionHandler) GetResource(w HeaderWriter, r *http.Requ } } + coreState := handler.GetCoreState() + if !coreState.Synced { + return nil, hProblem.StaleHistory + } + submission := handler.Submitter.Submit( r.Context(), info.raw, diff --git a/services/horizon/internal/app.go b/services/horizon/internal/app.go index 617107ee3f..5f2ee91087 100644 --- a/services/horizon/internal/app.go +++ b/services/horizon/internal/app.go @@ -13,8 +13,7 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/stellar/go/clients/stellarcore" - proto "github.com/stellar/go/protocols/stellarcore" - "github.com/stellar/go/services/horizon/internal/actions" + "github.com/stellar/go/services/horizon/internal/corestate" "github.com/stellar/go/services/horizon/internal/db2/history" "github.com/stellar/go/services/horizon/internal/httpx" "github.com/stellar/go/services/horizon/internal/ingest" @@ -30,26 +29,6 @@ import ( "github.com/stellar/go/support/log" ) -type coreSettingsStore struct { - sync.RWMutex - actions.CoreSettings -} - -func (c *coreSettingsStore) set(resp *proto.InfoResponse) { - c.Lock() - defer c.Unlock() - c.Synced = resp.IsSynced() - c.CoreVersion = resp.Info.Build - c.CurrentProtocolVersion = int32(resp.Info.Ledger.Version) - c.CoreSupportedProtocolVersion = int32(resp.Info.ProtocolVersion) -} - -func (c *coreSettingsStore) get() actions.CoreSettings { - c.RLock() - defer c.RUnlock() - return c.CoreSettings -} - // App represents the root of the state of a horizon instance. type App struct { done chan struct{} @@ -60,7 +39,7 @@ type App struct { ctx context.Context cancel func() horizonVersion string - coreSettings coreSettingsStore + coreState corestate.Store orderBookStream *ingest.OrderBookStream submitter *txsub.System paths paths.Finder @@ -80,8 +59,8 @@ type App struct { coreSynced prometheus.GaugeFunc } -func (a *App) GetCoreSettings() actions.CoreSettings { - return a.coreSettings.get() +func (a *App) GetCoreState() corestate.State { + return a.coreState.Get() } const tickerMaxFrequency = 1 * time.Second @@ -390,7 +369,7 @@ func (a *App) UpdateStellarCoreInfo(ctx context.Context) { os.Exit(1) } - a.coreSettings.set(resp) + a.coreState.Set(resp) } // DeleteUnretainedHistory forwards to the app's reaper. See diff --git a/services/horizon/internal/httpx/router.go b/services/horizon/internal/httpx/router.go index 27dd893b84..62d889487a 100644 --- a/services/horizon/internal/httpx/router.go +++ b/services/horizon/internal/httpx/router.go @@ -43,7 +43,7 @@ type RouterConfig struct { MaxPathLength uint PathFinder paths.Finder PrometheusRegistry *prometheus.Registry - CoreGetter actions.CoreSettingsGetter + CoreGetter actions.CoreStateGetter HorizonVersion string FriendbotURL *url.URL HealthCheck http.Handler @@ -124,11 +124,11 @@ func (r *Router) addRoutes(config *RouterConfig, rateLimiter *throttled.HTTPRate r.Method(http.MethodGet, "/health", config.HealthCheck) r.Method(http.MethodGet, "/", ObjectActionHandler{Action: actions.GetRootHandler{ - LedgerState: ledgerState, - CoreSettingsGetter: config.CoreGetter, - NetworkPassphrase: config.NetworkPassphrase, - FriendbotURL: config.FriendbotURL, - HorizonVersion: config.HorizonVersion, + LedgerState: ledgerState, + CoreStateGetter: config.CoreGetter, + NetworkPassphrase: config.NetworkPassphrase, + FriendbotURL: config.FriendbotURL, + HorizonVersion: config.HorizonVersion, }}) streamHandler := sse.StreamHandler{ @@ -292,6 +292,7 @@ func (r *Router) addRoutes(config *RouterConfig, rateLimiter *throttled.HTTPRate r.Method(http.MethodPost, "/transactions", ObjectActionHandler{actions.SubmitTransactionHandler{ Submitter: config.TxSubmitter, NetworkPassphrase: config.NetworkPassphrase, + CoreStateGetter: config.CoreGetter, }}) // Network state related endpoints diff --git a/services/horizon/internal/init.go b/services/horizon/internal/init.go index b3b7fe1d44..65cccd5d33 100644 --- a/services/horizon/internal/init.go +++ b/services/horizon/internal/init.go @@ -208,7 +208,7 @@ func initDbMetrics(app *App) { Help: "determines if Stellar-Core defined by --stellar-core-url is synced with the network", }, func() float64 { - if app.coreSettings.Synced { + if app.coreState.Synced { return 1 } else { return 0 diff --git a/services/horizon/internal/render/problem/problem.go b/services/horizon/internal/render/problem/problem.go index f1b23b676e..046c3b6604 100644 --- a/services/horizon/internal/render/problem/problem.go +++ b/services/horizon/internal/render/problem/problem.go @@ -100,8 +100,8 @@ var ( Status: http.StatusServiceUnavailable, Detail: "This horizon instance is configured to reject client requests " + "when it can determine that the history database is lagging too far " + - "behind the connected instance of stellar-core or read replica. Please " + - "try again later.", + "behind the connected instance of Stellar-Core or read replica. It's " + + "also possible that Stellar-Core is out of sync. Please try again later.", } // StillIngesting is a well-known problem type. Use it as a shortcut From 5de5b3b81b726918f837eeea92d82eb8e3d5da38 Mon Sep 17 00:00:00 2001 From: Bartek Nowotarski Date: Wed, 2 Jun 2021 17:12:36 +0200 Subject: [PATCH 2/5] Add new module --- services/horizon/internal/corestate/main.go | 34 +++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 services/horizon/internal/corestate/main.go diff --git a/services/horizon/internal/corestate/main.go b/services/horizon/internal/corestate/main.go new file mode 100644 index 0000000000..d19b98b7a1 --- /dev/null +++ b/services/horizon/internal/corestate/main.go @@ -0,0 +1,34 @@ +package corestate + +import ( + "sync" + + "github.com/stellar/go/protocols/stellarcore" +) + +type State struct { + Synced bool + CurrentProtocolVersion int32 + CoreSupportedProtocolVersion int32 + CoreVersion string +} + +type Store struct { + sync.RWMutex + State +} + +func (c *Store) Set(resp *stellarcore.InfoResponse) { + c.Lock() + defer c.Unlock() + c.Synced = resp.IsSynced() + c.CoreVersion = resp.Info.Build + c.CurrentProtocolVersion = int32(resp.Info.Ledger.Version) + c.CoreSupportedProtocolVersion = int32(resp.Info.ProtocolVersion) +} + +func (c *Store) Get() State { + c.RLock() + defer c.RUnlock() + return c.State +} From cc76c68bf6603ef1fb6b0e255938eb17eb664bf9 Mon Sep 17 00:00:00 2001 From: Bartek Nowotarski Date: Wed, 2 Jun 2021 17:19:21 +0200 Subject: [PATCH 3/5] Fix test --- .../internal/actions_transaction_test.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/services/horizon/internal/actions_transaction_test.go b/services/horizon/internal/actions_transaction_test.go index caefe2846c..3a7f815a1e 100644 --- a/services/horizon/internal/actions_transaction_test.go +++ b/services/horizon/internal/actions_transaction_test.go @@ -248,6 +248,9 @@ func TestTransactionActions_Post(t *testing.T) { ht := StartHTTPTest(t, "base") defer ht.Finish() + // Pass Synced check + ht.App.coreState.Synced = true + tx := xdr.TransactionEnvelope{ Type: xdr.EnvelopeTypeEnvelopeTypeTxV0, V0: &xdr.TransactionV0Envelope{ @@ -289,6 +292,9 @@ func TestTransactionActions_PostSuccessful(t *testing.T) { ht := StartHTTPTest(t, "failed_transactions") defer ht.Finish() + // Pass Synced check + ht.App.coreState.Synced = true + destAID := xdr.MustAddress("GBXGQJWVLWOYHFLVTKWV5FGHA3LNYY2JQKM7OAJAUEQFU6LPCSEFVXON") tx2 := xdr.TransactionEnvelope{ Type: xdr.EnvelopeTypeEnvelopeTypeTxV0, @@ -340,6 +346,9 @@ func TestTransactionActions_PostFailed(t *testing.T) { ht := StartHTTPTest(t, "failed_transactions") defer ht.Finish() + // Pass Synced check + ht.App.coreState.Synced = true + // aa168f12124b7c196c0adaee7c73a64d37f99428cacb59a91ff389626845e7cf form := url.Values{"tx": []string{"AAAAAG5oJtVdnYOVdZqtXpTHBtbcY0mCmfcBIKEgWnlvFIhaAAAAZAAAAAIAAAACAAAAAAAAAAAAAAABAAAAAAAAAAEAAAAAO2C/AO45YBD3tHVFO1R3A0MekP8JR6nN1A9eWidyItUAAAABVVNEAAAAAACuo3ot45qCPExpQ/3oHN+z17Ryis1lfMFYmQWgruS+TAAAAAB3NZQAAAAAAAAAAAFvFIhaAAAAQKcGS9OsVnVHCVIH04C9ZKzzKYBRdCmy+Jwmzld7QcALOxZUcAgkuGfoSdvXpH38mNvrqQiaMsSNmTJWYRzHvgo="}} @@ -352,6 +361,10 @@ func TestTransactionActions_PostFailed(t *testing.T) { func TestPostFeeBumpTransaction(t *testing.T) { ht := StartHTTPTestWithoutScenario(t) defer ht.Finish() + + // Pass Synced check + ht.App.coreState.Synced = true + test.ResetHorizonDB(t, ht.HorizonDB) q := &history.Q{ht.HorizonSession()} fixture := history.FeeBumpScenario(ht.T, q, true) @@ -387,6 +400,10 @@ func TestPostFeeBumpTransaction(t *testing.T) { func TestPostFailedFeeBumpTransaction(t *testing.T) { ht := StartHTTPTestWithoutScenario(t) defer ht.Finish() + + // Pass Synced check + ht.App.coreState.Synced = true + test.ResetHorizonDB(t, ht.HorizonDB) q := &history.Q{ht.HorizonSession()} fixture := history.FeeBumpScenario(ht.T, q, false) From 4dc19c65f4a47eec98d1f7a13942e19f991f66af Mon Sep 17 00:00:00 2001 From: Bartek Nowotarski Date: Fri, 4 Jun 2021 13:50:11 +0200 Subject: [PATCH 4/5] Update corestate.Store to not expose State fields --- services/horizon/internal/corestate/main.go | 12 ++++++------ services/horizon/internal/init.go | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/services/horizon/internal/corestate/main.go b/services/horizon/internal/corestate/main.go index d19b98b7a1..56bfd72b54 100644 --- a/services/horizon/internal/corestate/main.go +++ b/services/horizon/internal/corestate/main.go @@ -15,20 +15,20 @@ type State struct { type Store struct { sync.RWMutex - State + state State } func (c *Store) Set(resp *stellarcore.InfoResponse) { c.Lock() defer c.Unlock() - c.Synced = resp.IsSynced() - c.CoreVersion = resp.Info.Build - c.CurrentProtocolVersion = int32(resp.Info.Ledger.Version) - c.CoreSupportedProtocolVersion = int32(resp.Info.ProtocolVersion) + c.state.Synced = resp.IsSynced() + c.state.CoreVersion = resp.Info.Build + c.state.CurrentProtocolVersion = int32(resp.Info.Ledger.Version) + c.state.CoreSupportedProtocolVersion = int32(resp.Info.ProtocolVersion) } func (c *Store) Get() State { c.RLock() defer c.RUnlock() - return c.State + return c.state } diff --git a/services/horizon/internal/init.go b/services/horizon/internal/init.go index 65cccd5d33..d3aa042592 100644 --- a/services/horizon/internal/init.go +++ b/services/horizon/internal/init.go @@ -208,7 +208,7 @@ func initDbMetrics(app *App) { Help: "determines if Stellar-Core defined by --stellar-core-url is synced with the network", }, func() float64 { - if app.coreState.Synced { + if app.coreState.Get().Synced { return 1 } else { return 0 From 6147fb81ba449842bb1ed5b9b74f405fe8b62a88 Mon Sep 17 00:00:00 2001 From: Bartek Nowotarski Date: Fri, 4 Jun 2021 14:01:51 +0200 Subject: [PATCH 5/5] Fix tests --- services/horizon/internal/actions_transaction_test.go | 11 ++++++----- services/horizon/internal/corestate/main.go | 6 ++++++ 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/services/horizon/internal/actions_transaction_test.go b/services/horizon/internal/actions_transaction_test.go index 3a7f815a1e..7c6abebfe2 100644 --- a/services/horizon/internal/actions_transaction_test.go +++ b/services/horizon/internal/actions_transaction_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stellar/go/protocols/horizon" + "github.com/stellar/go/services/horizon/internal/corestate" "github.com/stellar/go/services/horizon/internal/db2/history" "github.com/stellar/go/services/horizon/internal/ingest" "github.com/stellar/go/services/horizon/internal/test" @@ -249,7 +250,7 @@ func TestTransactionActions_Post(t *testing.T) { defer ht.Finish() // Pass Synced check - ht.App.coreState.Synced = true + ht.App.coreState.SetState(corestate.State{Synced: true}) tx := xdr.TransactionEnvelope{ Type: xdr.EnvelopeTypeEnvelopeTypeTxV0, @@ -293,7 +294,7 @@ func TestTransactionActions_PostSuccessful(t *testing.T) { defer ht.Finish() // Pass Synced check - ht.App.coreState.Synced = true + ht.App.coreState.SetState(corestate.State{Synced: true}) destAID := xdr.MustAddress("GBXGQJWVLWOYHFLVTKWV5FGHA3LNYY2JQKM7OAJAUEQFU6LPCSEFVXON") tx2 := xdr.TransactionEnvelope{ @@ -347,7 +348,7 @@ func TestTransactionActions_PostFailed(t *testing.T) { defer ht.Finish() // Pass Synced check - ht.App.coreState.Synced = true + ht.App.coreState.SetState(corestate.State{Synced: true}) // aa168f12124b7c196c0adaee7c73a64d37f99428cacb59a91ff389626845e7cf form := url.Values{"tx": []string{"AAAAAG5oJtVdnYOVdZqtXpTHBtbcY0mCmfcBIKEgWnlvFIhaAAAAZAAAAAIAAAACAAAAAAAAAAAAAAABAAAAAAAAAAEAAAAAO2C/AO45YBD3tHVFO1R3A0MekP8JR6nN1A9eWidyItUAAAABVVNEAAAAAACuo3ot45qCPExpQ/3oHN+z17Ryis1lfMFYmQWgruS+TAAAAAB3NZQAAAAAAAAAAAFvFIhaAAAAQKcGS9OsVnVHCVIH04C9ZKzzKYBRdCmy+Jwmzld7QcALOxZUcAgkuGfoSdvXpH38mNvrqQiaMsSNmTJWYRzHvgo="}} @@ -363,7 +364,7 @@ func TestPostFeeBumpTransaction(t *testing.T) { defer ht.Finish() // Pass Synced check - ht.App.coreState.Synced = true + ht.App.coreState.SetState(corestate.State{Synced: true}) test.ResetHorizonDB(t, ht.HorizonDB) q := &history.Q{ht.HorizonSession()} @@ -402,7 +403,7 @@ func TestPostFailedFeeBumpTransaction(t *testing.T) { defer ht.Finish() // Pass Synced check - ht.App.coreState.Synced = true + ht.App.coreState.SetState(corestate.State{Synced: true}) test.ResetHorizonDB(t, ht.HorizonDB) q := &history.Q{ht.HorizonSession()} diff --git a/services/horizon/internal/corestate/main.go b/services/horizon/internal/corestate/main.go index 56bfd72b54..cd9fd463a7 100644 --- a/services/horizon/internal/corestate/main.go +++ b/services/horizon/internal/corestate/main.go @@ -27,6 +27,12 @@ func (c *Store) Set(resp *stellarcore.InfoResponse) { c.state.CoreSupportedProtocolVersion = int32(resp.Info.ProtocolVersion) } +func (c *Store) SetState(state State) { + c.Lock() + defer c.Unlock() + c.state = state +} + func (c *Store) Get() State { c.RLock() defer c.RUnlock()