services/horizon: Return 503 error in POST /transactions if Stellar-C…

…ore 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

@@ -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

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,
 	)

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,

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)

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

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
+}

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

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

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