From 0c60e5ef586bd22ed22b11bb8d1117a3483d393e Mon Sep 17 00:00:00 2001 From: Bartek Nowotarski Date: Fri, 4 Jun 2021 16:41:38 +0200 Subject: [PATCH] services/horizon: Return 503 error in POST /transactions if Stellar-Core not synced (#3653) This commit changes `POST /transactions` behaviour to return `503 Service Unavailable` (`stale_history`) instead of `504 Gateway Timeout` (`timeout`) response if connected Stellar-Core is not synced. `timeout` error is confusing when the connected Stellar-Core is not synced because it means that the transaction has been broadcasted but there is no result yet. What actually happens is that the transaction is not broadcasted (and Horizon cannot ingest it) when Stellar-Core is not in synced. The new response is returned as soon as Horizon determines that the Stellar-Core status has changed. --- services/horizon/internal/actions/helpers.go | 4 ++ services/horizon/internal/actions/root.go | 22 ++++------ .../internal/actions/submit_transaction.go | 6 +++ .../internal/actions_transaction_test.go | 18 +++++++++ services/horizon/internal/app.go | 31 +++----------- services/horizon/internal/corestate/main.go | 40 +++++++++++++++++++ services/horizon/internal/httpx/router.go | 13 +++--- services/horizon/internal/init.go | 2 +- .../internal/render/problem/problem.go | 4 +- 9 files changed, 91 insertions(+), 49 deletions(-) create mode 100644 services/horizon/internal/corestate/main.go 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/actions_transaction_test.go b/services/horizon/internal/actions_transaction_test.go index caefe2846c..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" @@ -248,6 +249,9 @@ func TestTransactionActions_Post(t *testing.T) { ht := StartHTTPTest(t, "base") defer ht.Finish() + // Pass Synced check + ht.App.coreState.SetState(corestate.State{Synced: true}) + tx := xdr.TransactionEnvelope{ Type: xdr.EnvelopeTypeEnvelopeTypeTxV0, V0: &xdr.TransactionV0Envelope{ @@ -289,6 +293,9 @@ func TestTransactionActions_PostSuccessful(t *testing.T) { ht := StartHTTPTest(t, "failed_transactions") defer ht.Finish() + // Pass Synced check + ht.App.coreState.SetState(corestate.State{Synced: true}) + destAID := xdr.MustAddress("GBXGQJWVLWOYHFLVTKWV5FGHA3LNYY2JQKM7OAJAUEQFU6LPCSEFVXON") tx2 := xdr.TransactionEnvelope{ Type: xdr.EnvelopeTypeEnvelopeTypeTxV0, @@ -340,6 +347,9 @@ func TestTransactionActions_PostFailed(t *testing.T) { ht := StartHTTPTest(t, "failed_transactions") defer ht.Finish() + // Pass Synced check + ht.App.coreState.SetState(corestate.State{Synced: true}) + // aa168f12124b7c196c0adaee7c73a64d37f99428cacb59a91ff389626845e7cf form := url.Values{"tx": []string{"AAAAAG5oJtVdnYOVdZqtXpTHBtbcY0mCmfcBIKEgWnlvFIhaAAAAZAAAAAIAAAACAAAAAAAAAAAAAAABAAAAAAAAAAEAAAAAO2C/AO45YBD3tHVFO1R3A0MekP8JR6nN1A9eWidyItUAAAABVVNEAAAAAACuo3ot45qCPExpQ/3oHN+z17Ryis1lfMFYmQWgruS+TAAAAAB3NZQAAAAAAAAAAAFvFIhaAAAAQKcGS9OsVnVHCVIH04C9ZKzzKYBRdCmy+Jwmzld7QcALOxZUcAgkuGfoSdvXpH38mNvrqQiaMsSNmTJWYRzHvgo="}} @@ -352,6 +362,10 @@ func TestTransactionActions_PostFailed(t *testing.T) { func TestPostFeeBumpTransaction(t *testing.T) { ht := StartHTTPTestWithoutScenario(t) defer ht.Finish() + + // Pass Synced check + ht.App.coreState.SetState(corestate.State{Synced: true}) + test.ResetHorizonDB(t, ht.HorizonDB) q := &history.Q{ht.HorizonSession()} fixture := history.FeeBumpScenario(ht.T, q, true) @@ -387,6 +401,10 @@ func TestPostFeeBumpTransaction(t *testing.T) { func TestPostFailedFeeBumpTransaction(t *testing.T) { ht := StartHTTPTestWithoutScenario(t) defer ht.Finish() + + // Pass Synced check + ht.App.coreState.SetState(corestate.State{Synced: true}) + test.ResetHorizonDB(t, ht.HorizonDB) q := &history.Q{ht.HorizonSession()} fixture := history.FeeBumpScenario(ht.T, q, false) 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/corestate/main.go b/services/horizon/internal/corestate/main.go new file mode 100644 index 0000000000..cd9fd463a7 --- /dev/null +++ b/services/horizon/internal/corestate/main.go @@ -0,0 +1,40 @@ +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 State +} + +func (c *Store) Set(resp *stellarcore.InfoResponse) { + c.Lock() + defer c.Unlock() + 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) SetState(state State) { + c.Lock() + defer c.Unlock() + c.state = state +} + +func (c *Store) Get() State { + c.RLock() + defer c.RUnlock() + return c.state +} 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..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.coreSettings.Synced { + if app.coreState.Get().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