Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add DISABLE-TX-SUB configuration parameter #4979

Merged
merged 32 commits into from
Aug 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
ef65a93
Add DisableTxSub param
aditya1702 Jul 25, 2023
79562fb
Add tests
aditya1702 Jul 25, 2023
fdcdc65
Add test for ApplyFlags
aditya1702 Jul 26, 2023
a4c3640
Revert "Add tests"
aditya1702 Jul 26, 2023
88d50c7
Make changes - 1
aditya1702 Jul 26, 2023
b1d4c77
Merge branch 'master' into add-tx-disabled
aditya1702 Aug 1, 2023
0c72438
Update parameters_test.go
aditya1702 Aug 1, 2023
45bb55a
Add check for INGEST=false and refactor test for help output
aditya1702 Aug 2, 2023
bcca4f9
Update integration.go
aditya1702 Aug 2, 2023
4610cee
Check if integration tests have been enabled
aditya1702 Aug 2, 2023
8153de2
Remove INGEST=false condition for tx-submission
aditya1702 Aug 3, 2023
d3fa9e7
Merge branch 'master' into add-tx-disabled
aditya1702 Aug 9, 2023
9251635
Merge branch 'master' into add-tx-disabled
aditya1702 Aug 10, 2023
9103d40
Merge branch 'master' into add-tx-disabled
aditya1702 Aug 11, 2023
fe8984c
Update flags.go
aditya1702 Aug 14, 2023
1392018
Merge branch 'master' into add-tx-disabled
aditya1702 Aug 14, 2023
7304870
Merge branch 'master' into add-tx-disabled
aditya1702 Aug 15, 2023
43864dc
Make changes - 1
aditya1702 Aug 15, 2023
b0f03e0
Merge branch 'master' into add-tx-disabled
aditya1702 Aug 21, 2023
e31b120
Make changes - 2
aditya1702 Aug 21, 2023
43b8b74
Merge branch 'master' into add-tx-disabled
aditya1702 Aug 22, 2023
b165589
Merge branch 'add-tx-disabled' of https://github.com/aditya1702/go in…
aditya1702 Aug 22, 2023
a4ca08f
Make changes - 3
aditya1702 Aug 22, 2023
12ad799
Change type to String
aditya1702 Aug 22, 2023
7439eba
Make changes - 4
aditya1702 Aug 23, 2023
5377630
Merge branch 'master' into add-tx-disabled
aditya1702 Aug 25, 2023
3cc8813
Add tests for different configurations of DISABLE_TX_SUB
aditya1702 Aug 28, 2023
7d3b717
Update parameters_test.go
aditya1702 Aug 28, 2023
4c9125f
Add integration tests checking transaction submission
aditya1702 Aug 28, 2023
6e8cafc
Update txsub_test.go
aditya1702 Aug 28, 2023
8235160
Update parameters_test.go
aditya1702 Aug 28, 2023
b94e9cf
Update parameters_test.go
aditya1702 Aug 29, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions services/horizon/internal/actions/submit_transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ type NetworkSubmitter interface {
type SubmitTransactionHandler struct {
Submitter NetworkSubmitter
NetworkPassphrase string
DisableTxSub bool
CoreStateGetter
}

Expand Down Expand Up @@ -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
Expand Down
46 changes: 46 additions & 0 deletions services/horizon/internal/actions/submit_transaction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,3 +153,49 @@
_, 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{
aditya1702 marked this conversation as resolved.
Show resolved Hide resolved
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(

Check failure on line 186 in services/horizon/internal/actions/submit_transaction_test.go

View workflow job for this annotation

GitHub Actions / golangci

should rewrite http.NewRequestWithContext or add (*Request).WithContext (noctx)
"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)
}
35 changes: 18 additions & 17 deletions services/horizon/internal/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -538,7 +540,6 @@ func (a *App) init() error {
},
cache: newHealthCache(healthCacheTTL),
},
EnableIngestionFiltering: a.config.EnableIngestionFiltering,
}

if a.primaryHistoryQ != nil {
Expand Down
2 changes: 2 additions & 0 deletions services/horizon/internal/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
38 changes: 25 additions & 13 deletions services/horizon/internal/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,11 @@
// 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)

Check failure on line 55 in services/horizon/internal/flags.go

View workflow job for this annotation

GitHub Actions / golangci

line is 144 characters (lll)
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
Expand Down Expand Up @@ -146,6 +149,15 @@
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.",
aditya1702 marked this conversation as resolved.
Show resolved Hide resolved
ConfigKey: &config.DisableTxSub,
Hidden: false,
},
&support.ConfigOption{
Name: captiveCoreConfigAppendPathName,
OptType: types.String,
Expand Down Expand Up @@ -211,9 +223,9 @@
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 {
Expand Down Expand Up @@ -251,7 +263,7 @@
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
}
Expand Down Expand Up @@ -388,7 +400,7 @@
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
Expand Down Expand Up @@ -820,8 +832,6 @@
config.Ingest = true
}

config.EnableIngestionFiltering = true
aditya1702 marked this conversation as resolved.
Show resolved Hide resolved

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
Expand Down Expand Up @@ -849,11 +859,12 @@
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)
}
}

Expand All @@ -863,7 +874,7 @@
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)
}
}

Expand All @@ -878,7 +889,8 @@
}

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
Expand Down
2 changes: 2 additions & 0 deletions services/horizon/internal/httpx/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ type RouterConfig struct {
FriendbotURL *url.URL
HealthCheck http.Handler
EnableIngestionFiltering bool
DisableTxSub bool
}

type Router struct {
Expand Down Expand Up @@ -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,
aditya1702 marked this conversation as resolved.
Show resolved Hide resolved
CoreStateGetter: config.CoreGetter,
}})

Expand Down
2 changes: 1 addition & 1 deletion services/horizon/internal/integration/clawback_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
56 changes: 54 additions & 2 deletions services/horizon/internal/integration/parameters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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: "",
aditya1702 marked this conversation as resolved.
Show resolved Hide resolved
})
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()
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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")
}
Expand Down
Loading
Loading