Add DISABLE-TX-SUB configuration parameter (#4979)

* Add DisableTxSub param

* Add tests

* Add test for ApplyFlags

* Revert "Add tests"

This reverts commit 79562fb.

* Make changes - 1

* Update parameters_test.go

* Add check for INGEST=false and refactor test for help output

* Update integration.go

* Check if integration tests have been enabled

* Remove INGEST=false condition for tx-submission

* Update flags.go

* Make changes - 1

* Make changes - 2

* Make changes - 3

* Change type to String

This makes it easy for us to check if the flag has been provided by the user or not.

* Make changes - 4

* Add tests for different configurations of DISABLE_TX_SUB

* Update parameters_test.go

* Add integration tests checking transaction submission

* Update txsub_test.go

* Update parameters_test.go

* Update parameters_test.go

services/horizon/internal/actions/submit_transaction.go

@@ -24,6 +24,7 @@ type NetworkSubmitter interface {
 type SubmitTransactionHandler struct {
 	Submitter         NetworkSubmitter
 	NetworkPassphrase string
+	DisableTxSub      bool
 	CoreStateGetter
 }
 
@@ -128,6 +129,17 @@ func (handler SubmitTransactionHandler) GetResource(w HeaderWriter, r *http.Requ
 		return nil, err
 	}
 
+	if handler.DisableTxSub {
+		return nil, &problem.P{
+			Type:   "transaction_submission_disabled",
+			Title:  "Transaction Submission Disabled",
+			Status: http.StatusMethodNotAllowed,
+			Detail: "Transaction submission has been disabled for Horizon. " +
+				"To enable it again, remove env variable DISABLE_TX_SUB.",
+			Extras: map[string]interface{}{},
+		}
+	}
+
 	raw, err := getString(r, "tx")
 	if err != nil {
 		return nil, err

services/horizon/internal/actions/submit_transaction_test.go

@@ -153,3 +153,49 @@ func TestClientDisconnectSubmission(t *testing.T) {
 	_, err = handler.GetResource(w, request)
 	assert.Equal(t, hProblem.ClientDisconnected, err)
 }
+
+func TestDisableTxSubFlagSubmission(t *testing.T) {
+	mockSubmitChannel := make(chan txsub.Result)
+
+	mock := &coreStateGetterMock{}
+	mock.On("GetCoreState").Return(corestate.State{
+		Synced: true,
+	})
+
+	mockSubmitter := &networkSubmitterMock{}
+	mockSubmitter.On("Submit").Return(mockSubmitChannel)
+
+	handler := SubmitTransactionHandler{
+		Submitter:         mockSubmitter,
+		NetworkPassphrase: network.PublicNetworkPassphrase,
+		DisableTxSub:      true,
+		CoreStateGetter:   mock,
+	}
+
+	form := url.Values{}
+
+	var p = &problem.P{
+		Type:   "transaction_submission_disabled",
+		Title:  "Transaction Submission Disabled",
+		Status: http.StatusMethodNotAllowed,
+		Detail: "Transaction submission has been disabled for Horizon. " +
+			"To enable it again, remove env variable DISABLE_TX_SUB.",
+		Extras: map[string]interface{}{},
+	}
+
+	request, err := http.NewRequest(
+		"POST",
+		"https://horizon.stellar.org/transactions",
+		strings.NewReader(form.Encode()),
+	)
+
+	require.NoError(t, err)
+	request.Header.Add("Content-Type", "application/x-www-form-urlencoded")
+	ctx, cancel := context.WithCancel(request.Context())
+	cancel()
+	request = request.WithContext(ctx)
+
+	w := httptest.NewRecorder()
+	_, err = handler.GetResource(w, request)
+	assert.Equal(t, p, err)
+}

services/horizon/internal/app.go

@@ -513,22 +513,24 @@ func (a *App) init() error {
 	initTxSubMetrics(a)
 
 	routerConfig := httpx.RouterConfig{
-		DBSession:               a.historyQ.SessionInterface,
-		TxSubmitter:             a.submitter,
-		RateQuota:               a.config.RateQuota,
-		BehindCloudflare:        a.config.BehindCloudflare,
-		BehindAWSLoadBalancer:   a.config.BehindAWSLoadBalancer,
-		SSEUpdateFrequency:      a.config.SSEUpdateFrequency,
-		StaleThreshold:          a.config.StaleThreshold,
-		ConnectionTimeout:       a.config.ConnectionTimeout,
-		NetworkPassphrase:       a.config.NetworkPassphrase,
-		MaxPathLength:           a.config.MaxPathLength,
-		MaxAssetsPerPathRequest: a.config.MaxAssetsPerPathRequest,
-		PathFinder:              a.paths,
-		PrometheusRegistry:      a.prometheusRegistry,
-		CoreGetter:              a,
-		HorizonVersion:          a.horizonVersion,
-		FriendbotURL:            a.config.FriendbotURL,
+		DBSession:                a.historyQ.SessionInterface,
+		TxSubmitter:              a.submitter,
+		RateQuota:                a.config.RateQuota,
+		BehindCloudflare:         a.config.BehindCloudflare,
+		BehindAWSLoadBalancer:    a.config.BehindAWSLoadBalancer,
+		SSEUpdateFrequency:       a.config.SSEUpdateFrequency,
+		StaleThreshold:           a.config.StaleThreshold,
+		ConnectionTimeout:        a.config.ConnectionTimeout,
+		NetworkPassphrase:        a.config.NetworkPassphrase,
+		MaxPathLength:            a.config.MaxPathLength,
+		MaxAssetsPerPathRequest:  a.config.MaxAssetsPerPathRequest,
+		PathFinder:               a.paths,
+		PrometheusRegistry:       a.prometheusRegistry,
+		CoreGetter:               a,
+		HorizonVersion:           a.horizonVersion,
+		FriendbotURL:             a.config.FriendbotURL,
+		EnableIngestionFiltering: a.config.EnableIngestionFiltering,
+		DisableTxSub:             a.config.DisableTxSub,
 		HealthCheck: healthCheck{
 			session: a.historyQ.SessionInterface,
 			ctx:     a.ctx,
@@ -538,7 +540,6 @@ func (a *App) init() error {
 			},
 			cache: newHealthCache(healthCacheTTL),
 		},
-		EnableIngestionFiltering: a.config.EnableIngestionFiltering,
 	}
 
 	if a.primaryHistoryQ != nil {

services/horizon/internal/config.go

@@ -115,4 +115,6 @@ type Config struct {
 	RoundingSlippageFilter int
 	// Stellar network: 'testnet' or 'pubnet'
 	Network string
+	// DisableTxSub disables transaction submission functionality for Horizon.
+	DisableTxSub bool
 }

services/horizon/internal/flags.go

@@ -51,8 +51,11 @@ const (
 	// HistoryArchiveURLsFlagName is the command line flag for specifying the history archive URLs
 	HistoryArchiveURLsFlagName = "history-archive-urls"
 	// NetworkFlagName is the command line flag for specifying the "network"
-	NetworkFlagName              = "network"
-	EnableIngestionFilteringFlag = "exp-enable-ingestion-filtering"
+	NetworkFlagName = "network"
+	// EnableIngestionFilteringFlagName is the command line flag for enabling the experimental ingestion filtering feature (now enabled by default)
+	EnableIngestionFilteringFlagName = "exp-enable-ingestion-filtering"
+	// DisableTxSubFlagName is the command line flag for disabling transaction submission feature of Horizon
+	DisableTxSubFlagName = "disable-tx-sub"
 
 	captiveCoreMigrationHint = "If you are migrating from Horizon 1.x.y, start with the Migration Guide here: https://developers.stellar.org/docs/run-api-server/migrating/"
 	// StellarPubnet is a constant representing the Stellar public network
@@ -146,6 +149,15 @@ func Flags() (*Config, support.ConfigOptions) {
 			Usage:       "path to stellar core binary, look for the stellar-core binary in $PATH by default.",
 			ConfigKey:   &config.CaptiveCoreBinaryPath,
 		},
+		&support.ConfigOption{
+			Name:        DisableTxSubFlagName,
+			OptType:     types.Bool,
+			FlagDefault: false,
+			Required:    false,
+			Usage:       "disables the transaction submission functionality of Horizon.",
+			ConfigKey:   &config.DisableTxSub,
+			Hidden:      false,
+		},
 		&support.ConfigOption{
 			Name:        captiveCoreConfigAppendPathName,
 			OptType:     types.String,
@@ -211,9 +223,9 @@ func Flags() (*Config, support.ConfigOptions) {
 			ConfigKey:   &config.EnableCaptiveCoreIngestion,
 		},
 		&support.ConfigOption{
-			Name:        EnableIngestionFilteringFlag,
-			OptType:     types.Bool,
-			FlagDefault: true,
+			Name:        EnableIngestionFilteringFlagName,
+			OptType:     types.String,
+			FlagDefault: "",
 			Required:    false,
 			ConfigKey:   &config.EnableIngestionFiltering,
 			CustomSetValue: func(opt *support.ConfigOption) error {
@@ -251,7 +263,7 @@ func Flags() (*Config, support.ConfigOptions) {
 				if existingValue == "" || existingValue == "." {
 					cwd, err := os.Getwd()
 					if err != nil {
-						return fmt.Errorf("Unable to determine the current directory: %s", err)
+						return fmt.Errorf("unable to determine the current directory: %s", err)
 					}
 					existingValue = cwd
 				}
@@ -388,7 +400,7 @@ func Flags() (*Config, support.ConfigOptions) {
 			CustomSetValue: func(co *support.ConfigOption) error {
 				ll, err := logrus.ParseLevel(viper.GetString(co.Name))
 				if err != nil {
-					return fmt.Errorf("Could not parse log-level: %v", viper.GetString(co.Name))
+					return fmt.Errorf("could not parse log-level: %v", viper.GetString(co.Name))
 				}
 				*(co.ConfigKey.(*logrus.Level)) = ll
 				return nil
@@ -820,8 +832,6 @@ func ApplyFlags(config *Config, flags support.ConfigOptions, options ApplyOption
 		config.Ingest = true
 	}
 
-	config.EnableIngestionFiltering = true
-
 	if config.Ingest {
 		// Migrations should be checked as early as possible. Apply and check
 		// only on ingesting instances which are required to have write-access
@@ -849,11 +859,12 @@ func ApplyFlags(config *Config, flags support.ConfigOptions, options ApplyOption
 			if viper.GetString(CaptiveCoreConfigPathName) != "" {
 				captiveCoreConfigFlag = CaptiveCoreConfigPathName
 			}
-			return fmt.Errorf("Invalid config: one or more captive core params passed (--%s or --%s) but --ingest not set. "+captiveCoreMigrationHint,
+			return fmt.Errorf("invalid config: one or more captive core params passed (--%s or --%s) but --ingest not set"+captiveCoreMigrationHint,
 				StellarCoreBinaryPathName, captiveCoreConfigFlag)
 		}
 		if config.StellarCoreDatabaseURL != "" {
-			return fmt.Errorf("Invalid config: --%s passed but --ingest not set. ", StellarCoreDBURLFlagName)
+			return fmt.Errorf("invalid config: --%s passed but --ingest not set"+
+				"", StellarCoreDBURLFlagName)
 		}
 	}
 
@@ -863,7 +874,7 @@ func ApplyFlags(config *Config, flags support.ConfigOptions, options ApplyOption
 		if err == nil {
 			log.DefaultLogger.SetOutput(logFile)
 		} else {
-			return fmt.Errorf("Failed to open file to log: %s", err)
+			return fmt.Errorf("failed to open file to log: %s", err)
 		}
 	}
 
@@ -878,7 +889,8 @@ func ApplyFlags(config *Config, flags support.ConfigOptions, options ApplyOption
 	}
 
 	if config.BehindCloudflare && config.BehindAWSLoadBalancer {
-		return fmt.Errorf("Invalid config: Only one option of --behind-cloudflare and --behind-aws-load-balancer is allowed. If Horizon is behind both, use --behind-cloudflare only.")
+		return fmt.Errorf("invalid config: Only one option of --behind-cloudflare and --behind-aws-load-balancer is allowed." +
+			" If Horizon is behind both, use --behind-cloudflare only")
 	}
 
 	return nil

services/horizon/internal/httpx/router.go

@@ -48,6 +48,7 @@ type RouterConfig struct {
 	FriendbotURL             *url.URL
 	HealthCheck              http.Handler
 	EnableIngestionFiltering bool
+	DisableTxSub             bool
 }
 
 type Router struct {
@@ -319,6 +320,7 @@ func (r *Router) addRoutes(config *RouterConfig, rateLimiter *throttled.HTTPRate
 	r.Method(http.MethodPost, "/transactions", ObjectActionHandler{actions.SubmitTransactionHandler{
 		Submitter:         config.TxSubmitter,
 		NetworkPassphrase: config.NetworkPassphrase,
+		DisableTxSub:      config.DisableTxSub,
 		CoreStateGetter:   config.CoreGetter,
 	}})
 

services/horizon/internal/integration/clawback_test.go

@@ -23,7 +23,7 @@ func TestHappyClawbackAccount(t *testing.T) {
 
 	asset, fromKey, _ := setupClawbackAccountTest(tt, itest, master)
 
-	// Clawback all of the asset
+	// Clawback all the asset
 	submissionResp := itest.MustSubmitOperations(itest.MasterAccount(), master, &txnbuild.Clawback{
 		From:   fromKey.Address(),
 		Amount: "10",

services/horizon/internal/integration/parameters_test.go

@@ -397,6 +397,59 @@ func TestIngestionFilteringAlwaysDefaultingToTrue(t *testing.T) {
 	})
 }
 
+func TestDisableTxSub(t *testing.T) {
+	t.Run("require stellar-core-url when both DISABLE_TX_SUB=false and INGEST=false", func(t *testing.T) {
+		localParams := integration.MergeMaps(networkParamArgs, map[string]string{
+			horizon.NetworkFlagName:          "testnet",
+			horizon.IngestFlagName:           "false",
+			horizon.DisableTxSubFlagName:     "false",
+			horizon.StellarCoreDBURLFlagName: "",
+		})
+		testConfig := integration.GetTestConfig()
+		testConfig.HorizonIngestParameters = localParams
+		testConfig.SkipCoreContainerCreation = true
+		test := integration.NewTest(t, *testConfig)
+		err := test.StartHorizon()
+		assert.ErrorContains(t, err, "cannot initialize Horizon: flag --stellar-core-url cannot be empty")
+		test.Shutdown()
+	})
+	t.Run("horizon starts successfully when DISABLE_TX_SUB=false, INGEST=false and stellar-core-url is provided", func(t *testing.T) {
+		// TODO: Remove explicit mention of stellar-core-db-url once this issue is done: https://github.com/stellar/go/issues/4855
+		localParams := integration.MergeMaps(networkParamArgs, map[string]string{
+			horizon.NetworkFlagName:          "testnet",
+			horizon.IngestFlagName:           "false",
+			horizon.DisableTxSubFlagName:     "false",
+			horizon.StellarCoreDBURLFlagName: "",
+			horizon.StellarCoreURLFlagName:   "http://localhost:11626",
+		})
+		testConfig := integration.GetTestConfig()
+		testConfig.HorizonIngestParameters = localParams
+		testConfig.SkipCoreContainerCreation = true
+		test := integration.NewTest(t, *testConfig)
+		err := test.StartHorizon()
+		assert.NoError(t, err)
+		test.Shutdown()
+	})
+	t.Run("horizon starts successfully when DISABLE_TX_SUB=true and INGEST=true", func(t *testing.T) {
+		//localParams := integration.MergeMaps(networkParamArgs, map[string]string{
+		//	//horizon.NetworkFlagName:           "testnet",
+		//	horizon.IngestFlagName:            "true",
+		//	horizon.DisableTxSubFlagName:      "true",
+		//	horizon.StellarCoreBinaryPathName: "/usr/bin/stellar-core",
+		//})
+		testConfig := integration.GetTestConfig()
+		testConfig.HorizonIngestParameters = map[string]string{
+			"disable-tx-sub": "true",
+			"ingest":         "true",
+		}
+		test := integration.NewTest(t, *testConfig)
+		err := test.StartHorizon()
+		assert.NoError(t, err)
+		test.WaitForHorizon()
+		test.Shutdown()
+	})
+}
+
 func TestDeprecatedOutputForIngestionFilteringFlag(t *testing.T) {
 	originalStderr := os.Stderr
 	r, w, _ := os.Pipe()
@@ -433,7 +486,7 @@ func TestDeprecatedOutputForIngestionFilteringFlag(t *testing.T) {
 		"the same no-filtering result. Remove usage of this flag in all cases.")
 }
 
-func TestHelpOutputForNoIngestionFilteringFlag(t *testing.T) {
+func TestHelpOutput(t *testing.T) {
 	config, flags := horizon.Flags()
 
 	horizonCmd := &cobra.Command{
@@ -461,7 +514,6 @@ func TestHelpOutputForNoIngestionFilteringFlag(t *testing.T) {
 	if err := horizonCmd.Execute(); err != nil {
 		fmt.Println(err)
 	}
-
 	output := writer.(*bytes.Buffer).String()
 	assert.NotContains(t, output, "--exp-enable-ingestion-filtering")
 }