Add functionality to hide ingestion enabled flag from --help output

services/horizon/cmd/ingest.go

@@ -121,7 +121,7 @@ var ingestVerifyRangeCmd = &cobra.Command{
 		}
 
 		if ingestVerifyState && !mngr.IsCheckpoint(ingestVerifyTo) {
-			return fmt.Errorf("`--to` must be a checkpoint ledger when `--verify-state` is set.")
+			return fmt.Errorf("`--to` must be a checkpoint ledger when `--verify-state` is set")
 		}
 
 		ingestConfig := ingest.Config{
@@ -276,7 +276,7 @@ var ingestTriggerStateRebuildCmd = &cobra.Command{
 			return fmt.Errorf("cannot open Horizon DB: %v", err)
 		}
 
-		historyQ := &history.Q{horizonSession}
+		historyQ := &history.Q{SessionInterface: horizonSession}
 		if err := historyQ.UpdateIngestVersion(ctx, 0); err != nil {
 			return fmt.Errorf("cannot trigger state rebuild: %v", err)
 		}
@@ -300,7 +300,7 @@ var ingestInitGenesisStateCmd = &cobra.Command{
 			return fmt.Errorf("cannot open Horizon DB: %v", err)
 		}
 
-		historyQ := &history.Q{horizonSession}
+		historyQ := &history.Q{SessionInterface: horizonSession}
 
 		lastIngestedLedger, err := historyQ.GetLastLedgerIngestNonBlocking(ctx)
 		if err != nil {
@@ -372,7 +372,7 @@ var ingestBuildStateCmd = &cobra.Command{
 			return fmt.Errorf("cannot open Horizon DB: %v", err)
 		}
 
-		historyQ := &history.Q{horizonSession}
+		historyQ := &history.Q{SessionInterface: horizonSession}
 
 		lastIngestedLedger, err := historyQ.GetLastLedgerIngestNonBlocking(context.Background())
 		if err != nil {

services/horizon/cmd/root.go

@@ -36,7 +36,7 @@ func (e ErrUsage) Error() string {
 	return e.cmd.UsageString()
 }
 
-// Indicates we want to exit with a specific error code without printing an error.
+// ErrExitCode Indicates we want to exit with a specific error code without printing an error.
 type ErrExitCode int
 
 func (e ErrExitCode) Error() string {

services/horizon/internal/flags.go

@@ -47,7 +47,8 @@ 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"
+	NetworkFlagName              = "network"
+	EnableIngestionFilteringFlag = "exp-enable-ingestion-filtering"
 
 	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
@@ -206,12 +207,28 @@ func Flags() (*Config, support.ConfigOptions) {
 			ConfigKey:   &config.EnableCaptiveCoreIngestion,
 		},
 		&support.ConfigOption{
-			Name:        "exp-enable-ingestion-filtering",
+			Name:        EnableIngestionFilteringFlag,
 			OptType:     types.Bool,
-			FlagDefault: false,
+			FlagDefault: true,
 			Required:    false,
-			Usage:       "causes Horizon to enable the experimental Ingestion Filtering and the ingestion admin HTTP endpoint at /ingestion/filter",
 			ConfigKey:   &config.EnableIngestionFiltering,
+			CustomSetValue: func(opt *support.ConfigOption) error {
+
+				// Always enable ingestion filtering by default.
+				config.EnableIngestionFiltering = true
+
+				if val := viper.GetString(opt.Name); val != "" {
+					stdLog.Printf(
+						"DEPRECATED - No ingestion filter rules are defined by default, which equates to no filtering " +
+							"of historical data. If you have never added filter rules to this deployment, then nothing further needed. " +
+							"If you have defined ingestion filter rules prior but disabled filtering overall by setting this flag " +
+							"disabled with --exp-enable-ingestion-filtering=false, then you should now delete the filter rules using " +
+							"the Horizon Admin API to achieve the same no-filtering result. Remove usage of this flag in all cases.",
+					)
+				}
+				return nil
+			},
+			Hidden: true,
 		},
 		&support.ConfigOption{
 			Name:           "captive-core-http-port",
@@ -799,6 +816,8 @@ 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

services/horizon/internal/integration/ingestion_filtering_test.go

@@ -13,13 +13,12 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestFilteringAccountWhiteList(t *testing.T) {
+func TestFilteringWithNoFilters(t *testing.T) {
 	tt := assert.New(t)
 	const adminPort uint16 = 6000
 	itest := integration.NewTest(t, integration.Config{
 		HorizonIngestParameters: map[string]string{
-			"admin-port":                     strconv.Itoa(int(adminPort)),
-			"exp-enable-ingestion-filtering": "true",
+			"admin-port": strconv.Itoa(int(adminPort)),
 		},
 	})
 
@@ -28,33 +27,64 @@ func TestFilteringAccountWhiteList(t *testing.T) {
 	whitelistedAccountKey := fullKeys[0]
 	nonWhitelistedAccount := accounts[1]
 	nonWhitelistedAccountKey := fullKeys[1]
-	enabled := true
 
 	// all assets are allowed by default because the asset filter config is empty.
 	defaultAllowedAsset := txnbuild.CreditAsset{Code: "PTS", Issuer: itest.Master().Address()}
 	itest.MustEstablishTrustline(whitelistedAccountKey, whitelistedAccount, defaultAllowedAsset)
 	itest.MustEstablishTrustline(nonWhitelistedAccountKey, nonWhitelistedAccount, defaultAllowedAsset)
 
-	// assert that by system default, filters with no rules yet, allow all first
+	// assert that by system default, filters with no rules yet, allow all first.
 	txResp := itest.MustSubmitOperations(itest.MasterAccount(), itest.Master(),
 		&txnbuild.Payment{
 			Destination: nonWhitelistedAccount.GetAccountID(),
 			Amount:      "10",
 			Asset:       defaultAllowedAsset,
 		},
 	)
-
 	txResp, err := itest.Client().TransactionDetail(txResp.Hash)
 	tt.NoError(err)
 
+	// Make sure that when using a whitelisted account, that transaction is also allowed.
+	txResp = itest.MustSubmitOperations(itest.MasterAccount(), itest.Master(),
+		&txnbuild.Payment{
+			Destination: whitelistedAccount.GetAccountID(),
+			Amount:      "10",
+			Asset:       defaultAllowedAsset,
+		},
+	)
+	_, err = itest.Client().TransactionDetail(txResp.Hash)
+	tt.NoError(err)
+}
+
+func TestFilteringAccountWhiteList(t *testing.T) {
+	tt := assert.New(t)
+	const adminPort uint16 = 6000
+	itest := integration.NewTest(t, integration.Config{
+		HorizonIngestParameters: map[string]string{
+			"admin-port": strconv.Itoa(int(adminPort)),
+		},
+	})
+
+	fullKeys, accounts := itest.CreateAccounts(2, "10000")
+	whitelistedAccount := accounts[0]
+	whitelistedAccountKey := fullKeys[0]
+	nonWhitelistedAccount := accounts[1]
+	nonWhitelistedAccountKey := fullKeys[1]
+	enabled := true
+
+	// all assets are allowed by default because the asset filter config is empty.
+	defaultAllowedAsset := txnbuild.CreditAsset{Code: "PTS", Issuer: itest.Master().Address()}
+	itest.MustEstablishTrustline(whitelistedAccountKey, whitelistedAccount, defaultAllowedAsset)
+	itest.MustEstablishTrustline(nonWhitelistedAccountKey, nonWhitelistedAccount, defaultAllowedAsset)
+
 	// Setup a whitelisted account rule, force refresh of filter configs to be quick
 	filters.SetFilterConfigCheckIntervalSeconds(1)
 
 	expectedAccountFilter := hProtocol.AccountFilterConfig{
 		Whitelist: []string{whitelistedAccount.GetAccountID()},
 		Enabled:   &enabled,
 	}
-	err = itest.AdminClient().SetIngestionAccountFilter(expectedAccountFilter)
+	err := itest.AdminClient().SetIngestionAccountFilter(expectedAccountFilter)
 	tt.NoError(err)
 
 	accountFilter, err := itest.AdminClient().GetIngestionAccountFilter()
@@ -67,7 +97,7 @@ func TestFilteringAccountWhiteList(t *testing.T) {
 	time.Sleep(time.Duration(filters.GetFilterConfigCheckIntervalSeconds()) * time.Second)
 
 	// Make sure that when using a non-whitelisted account, the transaction is not stored
-	txResp = itest.MustSubmitOperations(itest.MasterAccount(), itest.Master(),
+	txResp := itest.MustSubmitOperations(itest.MasterAccount(), itest.Master(),
 		&txnbuild.Payment{
 			Destination: nonWhitelistedAccount.GetAccountID(),
 			Amount:      "10",
@@ -94,8 +124,7 @@ func TestFilteringAssetWhiteList(t *testing.T) {
 	const adminPort uint16 = 6000
 	itest := integration.NewTest(t, integration.Config{
 		HorizonIngestParameters: map[string]string{
-			"admin-port":                     strconv.Itoa(int(adminPort)),
-			"exp-enable-ingestion-filtering": "true",
+			"admin-port": strconv.Itoa(int(adminPort)),
 		},
 	})
 
@@ -110,18 +139,6 @@ func TestFilteringAssetWhiteList(t *testing.T) {
 	itest.MustEstablishTrustline(defaultAllowedAccountKey, defaultAllowedAccount, nonWhitelistedAsset)
 	enabled := true
 
-	// assert that by system default, filters with no rules yet, allow all first
-	txResp := itest.MustSubmitOperations(itest.MasterAccount(), itest.Master(),
-		&txnbuild.Payment{
-			Destination: defaultAllowedAccount.GetAccountID(),
-			Amount:      "10",
-			Asset:       nonWhitelistedAsset,
-		},
-	)
-
-	_, err := itest.Client().TransactionDetail(txResp.Hash)
-	tt.NoError(err)
-
 	// Setup a whitelisted asset rule, force refresh of filters to be quick
 	filters.SetFilterConfigCheckIntervalSeconds(1)
 
@@ -144,7 +161,7 @@ func TestFilteringAssetWhiteList(t *testing.T) {
 	time.Sleep(time.Duration(filters.GetFilterConfigCheckIntervalSeconds()) * time.Second)
 
 	// Make sure that when using a non-whitelisted asset, the transaction is not stored
-	txResp = itest.MustSubmitOperations(itest.MasterAccount(), itest.Master(),
+	txResp := itest.MustSubmitOperations(itest.MasterAccount(), itest.Master(),
 		&txnbuild.Payment{
 			Destination: defaultAllowedAccount.GetAccountID(),
 			Amount:      "10",

services/horizon/internal/integration/parameters_test.go

@@ -2,6 +2,10 @@
 package integration
 
 import (
+	"bytes"
+	"fmt"
+	"github.com/spf13/cobra"
+	"io"
 	"io/ioutil"
 	"os"
 	"os/exec"
@@ -212,6 +216,54 @@
 	})
 }
 
+func TestIngestionFilteringAlwaysDefaultingToTrue(t *testing.T) {
+	t.Run("ingestion filtering flag set to default value", func(t *testing.T) {
+		test := NewParameterTest(t, map[string]string{})
+		err := test.StartHorizon()
+		assert.NoError(t, err)
+		assert.Equal(t, test.HorizonIngest().Config().EnableIngestionFiltering, true)
+	})
+	t.Run("ingestion filtering flag set to false", func(t *testing.T) {
+		test := NewParameterTest(t, map[string]string{"exp-enable-ingestion-filtering": "false"})
+		err := test.StartHorizon()
+		assert.NoError(t, err)
+		assert.Equal(t, test.HorizonIngest().Config().EnableIngestionFiltering, true)
+	})
+}
+
+func TestHelpOutputForNoIngestionFilteringFlag(t *testing.T) {
+	config, flags := horizon.Flags()
+
+	horizonCmd := &cobra.Command{
+		Use:           "horizon",
+		Short:         "client-facing api server for the Stellar network",
+		SilenceErrors: true,
+		SilenceUsage:  true,
+		Long:          "Client-facing API server for the Stellar network. It acts as the interface between Stellar Core and applications that want to access the Stellar network. It allows you to submit transactions to the network, check the status of accounts, subscribe to event streams and more.",
+		RunE: func(cmd *cobra.Command, args []string) error {
+			_, err := horizon.NewAppFromFlags(config, flags)
+			if err != nil {
+				return err
+			}
+			return nil
+		},
+	}
+
+	var writer io.Writer = &bytes.Buffer{}
+	horizonCmd.SetOutput(writer)
+
+	horizonCmd.SetArgs([]string{"-h"})
+	if err := flags.Init(horizonCmd); err != nil {
+		fmt.Println(err)
+	}
+	if err := horizonCmd.Execute(); err != nil {
+		fmt.Println(err)
+	}
+
+	output := writer.(*bytes.Buffer).String()
+	assert.NotContains(t, output, "--exp-enable-ingestion-filtering")
+}
+
 // Pattern taken from testify issue:
 // https://github.com/stretchr/testify/issues/858#issuecomment-600491003
 //
@@ -223,8 +275,8 @@
 	suite.Suite
 }
 
-func (t *FatalTestCase) Exits(subprocess func()) {
-	testName := t.T().Name()
+func (suite *FatalTestCase) Exits(subprocess func()) {
+	testName := suite.T().Name()
 	if os.Getenv("ASSERT_EXISTS_"+testName) == "1" {
 		subprocess()
 		return
@@ -234,12 +286,12 @@
 	cmd.Env = append(os.Environ(), "ASSERT_EXISTS_"+testName+"=1")
 	err := cmd.Run()
 
-	t.T().Log("Result:", err)
+	suite.T().Log("Result:", err)
 	if e, ok := err.(*exec.ExitError); ok && !e.Success() {
 		return
 	}
 
-	t.Fail("expecting unsuccessful exit, got", err)
+	suite.Fail("expecting unsuccessful exit, got", err)
 }
 
 // validateNoBucketDirPath ensures the Stellar Core auto-generated configuration

services/horizon/internal/test/integration/integration.go

@@ -98,21 +98,6 @@ type Test struct {
 	passPhrase    string
 }
 
-func NewTestForRemoteHorizon(t *testing.T, horizonURL string, passPhrase string, masterKey *keypair.Full) *Test {
-	adminClient, err := sdk.NewAdminClient(0, "", 0)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	return &Test{
-		t:                  t,
-		horizonClient:      &sdk.Client{HorizonURL: horizonURL},
-		horizonAdminClient: adminClient,
-		masterKey:          masterKey,
-		passPhrase:         passPhrase,
-	}
-}
-
 // NewTest starts a new environment for integration test at a given
 // protocol version and blocks until Horizon starts ingesting.
 //

support/config/config_option.go

@@ -23,10 +23,10 @@ type ConfigOptions []*ConfigOption
 // Init calls Init on each ConfigOption passing on the cobra.Command.
 func (cos ConfigOptions) Init(cmd *cobra.Command) error {
 	for _, co := range cos {
-		err := co.Init(cmd)
-		if err != nil {
+		if err := co.Init(cmd); err != nil {
 			return err
 		}
+		co.SetHidden(cmd)
 	}
 	return nil
 }
@@ -69,6 +69,7 @@ type ConfigOption struct {
 	CustomSetValue func(*ConfigOption) error // Optional function for custom validation/transformation
 	ConfigKey      interface{}               // Pointer to the final key in the linked Config struct
 	flag           *pflag.Flag               // The persistent flag that the config option is attached to
+	Hidden         bool                      // A flag which indicates whether to hide the flag from --help output
 }
 
 // Init handles initialisation steps, including configuring and binding the env variable name.
@@ -82,6 +83,13 @@ func (co *ConfigOption) Init(cmd *cobra.Command) error {
 	return co.setFlag(cmd)
 }
 
+// SetHidden Hides the flag from --help output
+func (co *ConfigOption) SetHidden(cmd *cobra.Command) {
+	if co.Hidden {
+		co.flag.Hidden = true
+	}
+}
+
 // Bind binds the config option to viper.
 func (co *ConfigOption) Bind() {
 	viper.BindPFlag(co.Name, co.flag)