From edff7315405ee5db019bc2acb59642ce0d1d056a Mon Sep 17 00:00:00 2001 From: bdchatham Date: Tue, 11 Aug 2026 08:53:43 -0700 Subject: [PATCH 01/12] fix(p2p): make the inbound accept rate configurable and raise its default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The router paces its accept loop with a rate limiter whose limit has never been reachable from config: getRouterConfig/createRouter sets MaxDialRate from p2p dial-interval but leaves MaxAcceptRate unset, so every node falls through to the compiled-in default of rate.Every(time.Second) — one inbound connection per second. That is too low for a public-facing node. The kernel keeps completing handshakes into the listen backlog while the loop drains it at 1/s, so once the backlog is more than a few entries deep an arriving peer waits longer than handshake-timeout and never completes. The node stops acquiring inbound peers while continuing to serve its established ones, so it reports healthy. The 1/s value was not chosen for inbound behaviour: #2539 introduced the limiter at 2/s, and #2799 — whose subject and body are about making *dialing* less aggressive — halved it to 1/s while exposing only the dial knob to config. Add a p2p accept-interval key mirroring dial-interval, wire it through to MaxAcceptRate, and default it to 10ms. Concurrency is already bounded separately by MaxConcurrentAccepts (set from MaxConnections), and per-source abuse by MaxIncomingConnectionAttempts, so the global rate limiter is a backstop rather than the binding constraint. rate.Every returns rate.Inf for a non-positive interval, so accept-interval = 0 disables the limiter. Co-Authored-By: Claude Opus 5 --- sei-tendermint/config/config.go | 10 +++++++++ sei-tendermint/config/config_test.go | 23 ++++++++++++++++++++ sei-tendermint/internal/p2p/routeroptions.go | 2 ++ sei-tendermint/node/setup.go | 1 + 4 files changed, 36 insertions(+) diff --git a/sei-tendermint/config/config.go b/sei-tendermint/config/config.go index 0802169e83..2fc2b70166 100644 --- a/sei-tendermint/config/config.go +++ b/sei-tendermint/config/config.go @@ -724,6 +724,12 @@ type P2PConfig struct { // How often node should dial a new peer. DialInterval time.Duration `mapstructure:"dial-interval"` + // How often node should accept a new inbound connection. This paces the + // accept loop only; the number of connections being accepted concurrently is + // bounded separately by MaxConnections, and per-source abuse is bounded by + // MaxIncomingConnectionAttempts. A value <= 0 means unlimited. + AcceptInterval time.Duration `mapstructure:"accept-interval"` + // Testing params. // Force dial to fail TestDialFail bool `mapstructure:"test-dial-fail"` @@ -754,6 +760,7 @@ func DefaultP2PConfig() *P2PConfig { HandshakeTimeout: 10 * time.Second, DialTimeout: 3 * time.Second, DialInterval: 10 * time.Second, + AcceptInterval: 10 * time.Millisecond, TestDialFail: false, QueueType: "simple-priority", } @@ -774,6 +781,9 @@ func (cfg *P2PConfig) ValidateBasic() error { if cfg.RecvRate < 0 { return errors.New("recv-rate can't be negative") } + if cfg.AcceptInterval < 0 { + return errors.New("accept-interval can't be negative") + } return nil } diff --git a/sei-tendermint/config/config_test.go b/sei-tendermint/config/config_test.go index 05c9aadec6..e97c124116 100644 --- a/sei-tendermint/config/config_test.go +++ b/sei-tendermint/config/config_test.go @@ -11,6 +11,7 @@ import ( "github.com/sei-protocol/sei-chain/sei-tendermint/types" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/time/rate" ) func TestDefaultConfig(t *testing.T) { @@ -230,6 +231,7 @@ func TestP2PConfigValidateBasic(t *testing.T) { "MaxPacketMsgPayloadSize", "SendRate", "RecvRate", + "AcceptInterval", } for _, fieldName := range fieldsToTest { @@ -239,6 +241,27 @@ func TestP2PConfigValidateBasic(t *testing.T) { } } +// The accept loop paces itself off AcceptInterval. A default that admits only a +// handful of connections per second cannot drain the kernel accept backlog on a +// public node: peers queue behind it, time out mid-handshake, and the node stops +// acquiring inbound peers while still reporting healthy. Pin the default so that +// regression has to be deliberate rather than incidental. +func TestP2PConfigAcceptInterval(t *testing.T) { + cfg := DefaultP2PConfig() + require.NoError(t, cfg.ValidateBasic()) + + limit := rate.Every(cfg.AcceptInterval) + require.Greater(t, float64(limit), 50.0, + "default accept rate %v/s is too low to drain the accept backlog", float64(limit)) + require.NotEqual(t, rate.Inf, limit, "default accept rate should be bounded, not unlimited") + + // A non-positive interval is the documented escape hatch for disabling the + // limiter outright, and must stay valid rather than becoming a zero rate. + cfg.AcceptInterval = 0 + require.NoError(t, cfg.ValidateBasic()) + require.Equal(t, rate.Inf, rate.Every(cfg.AcceptInterval)) +} + // --- WalFile legacy fallback tests --- func TestWalFile_NewDefault_NoLegacy(t *testing.T) { diff --git a/sei-tendermint/internal/p2p/routeroptions.go b/sei-tendermint/internal/p2p/routeroptions.go index 2403f42f5e..81082224ab 100644 --- a/sei-tendermint/internal/p2p/routeroptions.go +++ b/sei-tendermint/internal/p2p/routeroptions.go @@ -69,6 +69,8 @@ type RouterOptions struct { MaxDialRate utils.Option[rate.Limit] // MaxAcceptRate limits the rate at which router is accepting TCP connections. Defaults to 1/s. + // Node setup always sets this from the p2p accept-interval config key, so the default + // applies only to embedders that construct RouterOptions directly. MaxAcceptRate utils.Option[rate.Limit] // ResolveTimeout is the timeout for resolving NodeAddress URLs. diff --git a/sei-tendermint/node/setup.go b/sei-tendermint/node/setup.go index 9671843322..30154fdf3c 100644 --- a/sei-tendermint/node/setup.go +++ b/sei-tendermint/node/setup.go @@ -500,6 +500,7 @@ func createRouter( Endpoint: ep, MaxIncomingConnectionAttempts: utils.Some(cfg.P2P.MaxIncomingConnectionAttempts), MaxDialRate: utils.Some(rate.Every(cfg.P2P.DialInterval)), + MaxAcceptRate: utils.Some(rate.Every(cfg.P2P.AcceptInterval)), HandshakeTimeout: utils.Some(cfg.P2P.HandshakeTimeout), DialTimeout: utils.Some(cfg.P2P.DialTimeout), PexOnHandshake: cfg.P2P.PexReactor, From cb0338f39b9ce72d0ff6946bf3fafcafe3edfc95 Mon Sep 17 00:00:00 2001 From: bdchatham Date: Tue, 11 Aug 2026 09:09:44 -0700 Subject: [PATCH 02/12] test(config): pin accept-interval under the hidden-knob convention accept-interval follows dial-interval in staying out of the generated template, which leaves "deliberately not rendered" and "not readable at all" indistinguishable from the test suite's point of view. The statesync knobs already have both halves of this convention; the p2p pacing knobs had neither. Add the matching pair: * checkConfig asserts dial-interval and accept-interval are absent from the rendered template, alongside the existing hiddenStateSyncElems block. * p2p_compat_test.go mirrors statesync_compat_test.go: one test proves both keys still parse out of an existing config.toml, one proves a template- shaped file without them still yields the defaults. The second case guards both directions. A zeroed AcceptInterval means rate.Every(0) == rate.Inf and disables accept pacing outright, while an oversized one throttles the accept loop below the rate at which peers arrive; neither is visible in the rendered config. Verified both guards discriminate: typoing the mapstructure tag fails the parse test with "expected: 20ms, actual: 10ms" (the operator's value silently ignored), and adding accept-interval to the template fails TestEnsureRoot with "config file was not expected to contain accept-interval". Co-Authored-By: Claude Opus 5 --- sei-tendermint/config/p2p_compat_test.go | 75 ++++++++++++++++++++++++ sei-tendermint/config/toml_test.go | 13 ++++ 2 files changed, 88 insertions(+) create mode 100644 sei-tendermint/config/p2p_compat_test.go diff --git a/sei-tendermint/config/p2p_compat_test.go b/sei-tendermint/config/p2p_compat_test.go new file mode 100644 index 0000000000..2500b6f273 --- /dev/null +++ b/sei-tendermint/config/p2p_compat_test.go @@ -0,0 +1,75 @@ +package config_test + +import ( + "os" + "path/filepath" + "testing" + "time" + + "github.com/spf13/viper" + "github.com/stretchr/testify/require" + + "github.com/sei-protocol/sei-chain/sei-tendermint/cmd/tendermint/commands" + tmconfig "github.com/sei-protocol/sei-chain/sei-tendermint/config" +) + +// This test (and TestFreshP2PConfigKeepsDefaultPacing) mutate the global viper +// singleton via commands.ParseConfig, so they must not run in parallel with +// other tests in this package. + +// The p2p pacing knobs are deliberately absent from the generated template +// (see checkConfig in toml_test.go), so nothing else proves an operator can +// actually set them. Without this, "not in the template" and "not readable" +// are indistinguishable. +func TestHiddenP2PKnobsStillParseFromExistingConfig(t *testing.T) { + viper.Reset() + t.Cleanup(viper.Reset) + + configPath := filepath.Join(t.TempDir(), "config.toml") + err := os.WriteFile(configPath, []byte(` +[p2p] +laddr = "tcp://0.0.0.0:26656" +dial-interval = "5s" +accept-interval = "20ms" +`), 0600) + require.NoError(t, err) + + viper.SetConfigFile(configPath) + require.NoError(t, viper.ReadInConfig()) + + cfg, err := commands.ParseConfig(tmconfig.DefaultConfig()) + require.NoError(t, err) + require.Equal(t, 5*time.Second, cfg.P2P.DialInterval) + require.Equal(t, 20*time.Millisecond, cfg.P2P.AcceptInterval) + require.NoError(t, cfg.P2P.ValidateBasic()) +} + +// TestFreshP2PConfigKeepsDefaultPacing mirrors the freshly-rendered template +// (no pacing knobs in the file) and verifies ParseConfig still produces the +// defaults. Both directions matter: a zeroed AcceptInterval means +// rate.Every(0) == rate.Inf, i.e. no accept pacing at all, while a value large +// enough to matter throttles the accept loop below the rate at which peers +// arrive. Neither is visible in the rendered config, so pin it here. +func TestFreshP2PConfigKeepsDefaultPacing(t *testing.T) { + viper.Reset() + t.Cleanup(viper.Reset) + + configPath := filepath.Join(t.TempDir(), "config.toml") + err := os.WriteFile(configPath, []byte(` +[p2p] +laddr = "tcp://0.0.0.0:26656" +`), 0600) + require.NoError(t, err) + + viper.SetConfigFile(configPath) + require.NoError(t, viper.ReadInConfig()) + + cfg, err := commands.ParseConfig(tmconfig.DefaultConfig()) + require.NoError(t, err) + + defaults := tmconfig.DefaultP2PConfig() + require.Equal(t, defaults.AcceptInterval, cfg.P2P.AcceptInterval) + require.Equal(t, defaults.DialInterval, cfg.P2P.DialInterval) + require.NotZero(t, cfg.P2P.AcceptInterval, "a zero accept-interval disables accept pacing entirely") + require.NoError(t, cfg.P2P.ValidateBasic()) +} diff --git a/sei-tendermint/config/toml_test.go b/sei-tendermint/config/toml_test.go index de4f059440..4c827fb014 100644 --- a/sei-tendermint/config/toml_test.go +++ b/sei-tendermint/config/toml_test.go @@ -97,6 +97,19 @@ func checkConfig(t *testing.T, configFile string) { t.Errorf("config file was not expected to contain %s", e) } } + + // The p2p pacing knobs are likewise expert-only and stay out of the + // generated template, while still being parsed from existing config files. + // See TestHiddenP2PKnobsStillParseFromExistingConfig. + var hiddenP2PElems = []string{ + "dial-interval", + "accept-interval", + } + for _, e := range hiddenP2PElems { + if configContainsKey(configFile, e) { + t.Errorf("config file was not expected to contain %s", e) + } + } } func configContainsKey(configFile string, key string) bool { From c704f4d723dcaa49ae1aaf25af4e2caff9d3c49e Mon Sep 17 00:00:00 2001 From: bdchatham Date: Tue, 11 Aug 2026 09:19:42 -0700 Subject: [PATCH 03/12] address review: render accept-interval, pin the default exactly, test the wiring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Doc comment (2 nits): * MaxConnections was the wrong knob. Concurrency is bounded by RouterOptions.MaxConcurrentAccepts, which setup derives from max-connections minus the outbound reservation; name that, since the derivation is the load-bearing part of why a 100/s global rate is safe. * "A value <= 0 means unlimited" overstated what is reachable — ValidateBasic rejects negatives, so 0 is the only value that gets there. Render accept-interval in the generated template. The sibling dial-interval is unrendered and I had matched it, but the reviewer is right that the argument cuts the other way here: this PR exists because an unreachable, undocumented accept rate went unnoticed on mainnet listeners, and the template is where an operator looks. Matching the sibling would perpetuate the condition being fixed. dial-interval stays hidden and keeps its parse-side coverage. Pin the default exactly. require.Greater(limit, 50.0) pinned a band, so 10ms -> 19ms was invisible. Assert the exact value, and keep the band, whose failure message carries the reason the value was chosen. Test the wiring itself. This was the reviewer's sharpest point: the PR's own diagnosis is that a 1/s production rate survived because every RouterOptions construction site except node/setup.go substitutes rate.Inf, yet the fix left the cfg.P2P.AcceptInterval -> MaxAcceptRate mapping unasserted. Extract the budget/pacing derivation into p2pRouterOptions and assert it directly: dropping the field fails with "expected: 100, actual: -1" against a sentinel, rather than silently falling back to the package default. Co-Authored-By: Claude Opus 5 --- sei-tendermint/config/config.go | 8 +++-- sei-tendermint/config/config_test.go | 5 +++ sei-tendermint/config/toml.go | 8 +++++ sei-tendermint/config/toml_test.go | 12 +++++-- sei-tendermint/node/setup.go | 53 ++++++++++++++++------------ sei-tendermint/node/setup_test.go | 44 +++++++++++++++++++++++ 6 files changed, 102 insertions(+), 28 deletions(-) diff --git a/sei-tendermint/config/config.go b/sei-tendermint/config/config.go index 2fc2b70166..5ab5d4e622 100644 --- a/sei-tendermint/config/config.go +++ b/sei-tendermint/config/config.go @@ -725,9 +725,11 @@ type P2PConfig struct { DialInterval time.Duration `mapstructure:"dial-interval"` // How often node should accept a new inbound connection. This paces the - // accept loop only; the number of connections being accepted concurrently is - // bounded separately by MaxConnections, and per-source abuse is bounded by - // MaxIncomingConnectionAttempts. A value <= 0 means unlimited. + // accept loop only. The number of connections being handshaked concurrently + // is bounded separately by RouterOptions.MaxConcurrentAccepts, which node + // setup derives from max-connections minus the outbound reservation, and the + // per-source attempt rate by max-incoming-connection-attempts. A value of 0 + // disables the limiter. AcceptInterval time.Duration `mapstructure:"accept-interval"` // Testing params. diff --git a/sei-tendermint/config/config_test.go b/sei-tendermint/config/config_test.go index e97c124116..f4fc9b86ed 100644 --- a/sei-tendermint/config/config_test.go +++ b/sei-tendermint/config/config_test.go @@ -250,6 +250,11 @@ func TestP2PConfigAcceptInterval(t *testing.T) { cfg := DefaultP2PConfig() require.NoError(t, cfg.ValidateBasic()) + // Exact value, so a change to the default shows up in the diff rather than + // sliding anywhere inside the band below. + require.Equal(t, 10*time.Millisecond, cfg.AcceptInterval) + + // The band and its message carry the reason the exact value was chosen. limit := rate.Every(cfg.AcceptInterval) require.Greater(t, float64(limit), 50.0, "default accept rate %v/s is too low to drain the accept backlog", float64(limit)) diff --git a/sei-tendermint/config/toml.go b/sei-tendermint/config/toml.go index 620d4608c8..33134737f2 100644 --- a/sei-tendermint/config/toml.go +++ b/sei-tendermint/config/toml.go @@ -346,6 +346,14 @@ allow-duplicate-ip = {{ .P2P.AllowDuplicateIP }} handshake-timeout = "{{ .P2P.HandshakeTimeout }}" dial-timeout = "{{ .P2P.DialTimeout }}" +# How often the node accepts a new inbound connection. This paces the accept +# loop only: concurrent handshakes are capped at max-connections minus the +# outbound reservation, and the per-source attempt rate by +# max-incoming-connection-attempts. Set too high a value and the kernel accept +# backlog outpaces the loop, so arriving peers wait past handshake-timeout and +# the node stops acquiring inbound peers. A value of 0 disables the limiter. +accept-interval = "{{ .P2P.AcceptInterval }}" + # Time to wait before flushing messages out on the connection # TODO: Remove once MConnConnection is removed. flush-throttle-timeout = "{{ .P2P.FlushThrottleTimeout }}" diff --git a/sei-tendermint/config/toml_test.go b/sei-tendermint/config/toml_test.go index 4c827fb014..c4868a6067 100644 --- a/sei-tendermint/config/toml_test.go +++ b/sei-tendermint/config/toml_test.go @@ -98,12 +98,18 @@ func checkConfig(t *testing.T, configFile string) { } } - // The p2p pacing knobs are likewise expert-only and stay out of the - // generated template, while still being parsed from existing config files. + // accept-interval is rendered deliberately: an accept rate too low to drain + // the kernel backlog silently stops a node acquiring inbound peers, and the + // template is where an operator looks. Keep it discoverable. + if !configContainsKey(configFile, "accept-interval") { + t.Errorf("config file was expected to contain accept-interval but did not") + } + + // dial-interval remains an expert-only knob, left out of the generated + // template while still being parsed from existing config files. // See TestHiddenP2PKnobsStillParseFromExistingConfig. var hiddenP2PElems = []string{ "dial-interval", - "accept-interval", } for _, e := range hiddenP2PElems { if configContainsKey(configFile, e) { diff --git a/sei-tendermint/node/setup.go b/sei-tendermint/node/setup.go index 30154fdf3c..df2b3f77a1 100644 --- a/sei-tendermint/node/setup.go +++ b/sei-tendermint/node/setup.go @@ -454,27 +454,12 @@ func buildFullnodeGigaConfig( }, nil } -func createRouter( - nodeInfoProducer func() *types.NodeInfo, - nodeKey types.NodeKey, - validatorKey utils.Option[atypes.SecretKey], - cfg *config.Config, - app utils.Option[*proxy.Proxy], - genDoc *types.GenesisDoc, - dbProvider config.DBProvider, -) (*p2p.Router, closer, utils.Option[atypes.BlockDB], error) { - closer := func() error { return nil } - noneDB := utils.None[atypes.BlockDB]() - gigaBlockDB := noneDB - ep, err := p2p.ResolveEndpoint(nodeKey.ID().AddressString(cfg.P2P.ListenAddress)) - if err != nil { - return nil, closer, noneDB, err - } - var privatePeerIDs []types.NodeID - for _, id := range tmstrings.SplitAndTrimEmpty(cfg.P2P.PrivatePeerIDs, ",", " ") { - privatePeerIDs = append(privatePeerIDs, types.NodeID(id)) - } - +// p2pRouterOptions derives the router's connection budget and pacing from the +// p2p config. Split out of createRouter so the derivation is testable on its +// own: any RouterOptions field left unset here silently falls back to a package +// default rather than failing, which is how the accept rate stayed pinned at +// its 1/s default while max-connections appeared to govern it. +func p2pRouterOptions(cfg *config.Config, ep p2p.Endpoint, privatePeerIDs []types.NodeID) *p2p.RouterOptions { // MaxConnections defaults to 64 maxConns := 64 if cfg.P2P.MaxConnections > 0 { @@ -496,7 +481,7 @@ func createRouter( connection.SendRate = cfg.P2P.SendRate connection.RecvRate = cfg.P2P.RecvRate connection.MaxPacketMsgPayloadSize = cfg.P2P.MaxPacketMsgPayloadSize - options := &p2p.RouterOptions{ + return &p2p.RouterOptions{ Endpoint: ep, MaxIncomingConnectionAttempts: utils.Some(cfg.P2P.MaxIncomingConnectionAttempts), MaxDialRate: utils.Some(rate.Every(cfg.P2P.DialInterval)), @@ -510,6 +495,30 @@ func createRouter( MaxConcurrentAccepts: utils.Some(maxInbound), Connection: connection, } +} + +func createRouter( + nodeInfoProducer func() *types.NodeInfo, + nodeKey types.NodeKey, + validatorKey utils.Option[atypes.SecretKey], + cfg *config.Config, + app utils.Option[*proxy.Proxy], + genDoc *types.GenesisDoc, + dbProvider config.DBProvider, +) (*p2p.Router, closer, utils.Option[atypes.BlockDB], error) { + closer := func() error { return nil } + noneDB := utils.None[atypes.BlockDB]() + gigaBlockDB := noneDB + ep, err := p2p.ResolveEndpoint(nodeKey.ID().AddressString(cfg.P2P.ListenAddress)) + if err != nil { + return nil, closer, noneDB, err + } + var privatePeerIDs []types.NodeID + for _, id := range tmstrings.SplitAndTrimEmpty(cfg.P2P.PrivatePeerIDs, ",", " ") { + privatePeerIDs = append(privatePeerIDs, types.NodeID(id)) + } + + options := p2pRouterOptions(cfg, ep, privatePeerIDs) if addr := cfg.P2P.ExternalAddress; addr != "" { nodeAddr, err := p2p.ParseNodeAddress(nodeKey.ID().AddressString(addr)) if err != nil { diff --git a/sei-tendermint/node/setup_test.go b/sei-tendermint/node/setup_test.go index 62668485a0..ee6fd065ae 100644 --- a/sei-tendermint/node/setup_test.go +++ b/sei-tendermint/node/setup_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/time/rate" "github.com/sei-protocol/sei-chain/sei-tendermint/abci/example/kvstore" atypes "github.com/sei-protocol/sei-chain/sei-tendermint/autobahn/types" @@ -345,3 +346,46 @@ func TestPreparePersistentStateDir_EmptyStringIsNone(t *testing.T) { _, ok := cfg.PersistentStateDir.Get() require.False(t, ok, "Some(\"\") must be cleared to None for in-memory mode") } + +// This PR's own diagnosis is that a 1/s production accept rate survived because +// every RouterOptions construction site except this one substitutes rate.Inf, +// so nothing exercised the real wiring. Assert the derivation directly: a field +// dropped here, or wired to the wrong config key, falls back to a package +// default silently rather than failing. +func TestP2PRouterOptions_PacingAndBudgetWiring(t *testing.T) { + ep, err := p2p.ResolveEndpoint("tcp://" + string(types.NodeID("0000000000000000000000000000000000000000")) + "@127.0.0.1:26656") + require.NoError(t, err) + + t.Run("defaults reach the router", func(t *testing.T) { + cfg := config.DefaultConfig() + opts := p2pRouterOptions(cfg, ep, nil) + + // Sentinel differs from every plausible real value, so .Or() returning it + // means the field was never set. + const unset = rate.Limit(-1) + require.Equal(t, rate.Every(cfg.P2P.AcceptInterval), opts.MaxAcceptRate.Or(unset)) + require.Equal(t, rate.Every(cfg.P2P.DialInterval), opts.MaxDialRate.Or(unset)) + + // The package fallback is 1 accept/s; setup must override it. + require.NotEqual(t, rate.Every(time.Second), opts.MaxAcceptRate.Or(unset), + "accept rate fell through to the package default") + }) + + t.Run("operator value flows through", func(t *testing.T) { + cfg := config.DefaultConfig() + cfg.P2P.AcceptInterval = 250 * time.Millisecond + opts := p2pRouterOptions(cfg, ep, nil) + require.Equal(t, rate.Every(250*time.Millisecond), opts.MaxAcceptRate.Or(rate.Limit(-1))) + }) + + t.Run("concurrent accepts track the inbound pool, not max-connections", func(t *testing.T) { + cfg := config.DefaultConfig() + cfg.P2P.MaxConnections = 100 + opts := p2pRouterOptions(cfg, ep, nil) + + // 100 total minus the 20 outbound reservation. + require.Equal(t, 80, opts.MaxConcurrentAccepts.Or(-1)) + require.Equal(t, 80, opts.MaxInbound.Or(-1)) + require.Equal(t, 20, opts.MaxOutbound.Or(-1)) + }) +} From 0ccc3ffdfc1e290f67a48bd36cf51f36e604da51 Mon Sep 17 00:00:00 2001 From: bdchatham Date: Tue, 11 Aug 2026 09:37:41 -0700 Subject: [PATCH 04/12] address review round 2: fix the default at the choke point, tighten the tests Structural, and the one that matters. The reviewer cited AGENTS.md "Guard at the choke point, never at each caller" against the previous round's fix: wiring MaxAcceptRate in node setup corrects one caller, while the hazard is the package default every embedder falls through to. Raise the default in maxAcceptRate() itself, from rate.Every(time.Second) to rate.Every(10*time.Millisecond), so no construction path can inherit an accept rate too low to drain the backlog. The config key and the wiring stay; they now tune a safe default rather than rescue a dangerous one. Test quality: * config_test.go's band (> 50/s, != Inf) was unreachable once the exact 10ms pin above it passed, so it read as coverage while never able to fire. Dropped it and moved the "why 10ms" into the doc comment, per AGENTS.md "the step name carries the what, the doc comment carries the why". * setup_test.go's budget case set MaxConnections = 100, which is already the default, so it could not distinguish "derives from max-connections" from "hardcoded to the default". Now table-driven over 50 (-> 30/30/20) and 30 (-> 15/15/15), the latter reaching the min(20, (maxConns+1)/2) branch nothing else covers. Verified it discriminates: wiring MaxConcurrentAccepts to maxConns instead of maxInbound fails with "expected: 30, actual: 50". * Dropped my own "fell through to the package default" assertion, which the default change made unreachable for the same reason as the band above. * Removed a no-op string(types.NodeID("0000...")) round trip. Docs and symmetry: * The template comment said "set too high a value", which reads backwards for an interval. Now states the direction explicitly. * ValidateBasic rejected a negative accept-interval but not dial-interval, which also becomes rate.Inf via rate.Every. Added the sibling check. Co-Authored-By: Claude Opus 5 --- sei-tendermint/config/config.go | 3 ++ sei-tendermint/config/config_test.go | 23 +++++------- sei-tendermint/config/toml.go | 7 ++-- sei-tendermint/internal/p2p/routeroptions.go | 10 +++--- sei-tendermint/node/setup_test.go | 37 ++++++++++++-------- 5 files changed, 43 insertions(+), 37 deletions(-) diff --git a/sei-tendermint/config/config.go b/sei-tendermint/config/config.go index 5ab5d4e622..cdc4eee933 100644 --- a/sei-tendermint/config/config.go +++ b/sei-tendermint/config/config.go @@ -783,6 +783,9 @@ func (cfg *P2PConfig) ValidateBasic() error { if cfg.RecvRate < 0 { return errors.New("recv-rate can't be negative") } + if cfg.DialInterval < 0 { + return errors.New("dial-interval can't be negative") + } if cfg.AcceptInterval < 0 { return errors.New("accept-interval can't be negative") } diff --git a/sei-tendermint/config/config_test.go b/sei-tendermint/config/config_test.go index f4fc9b86ed..649d9859d6 100644 --- a/sei-tendermint/config/config_test.go +++ b/sei-tendermint/config/config_test.go @@ -241,27 +241,20 @@ func TestP2PConfigValidateBasic(t *testing.T) { } } -// The accept loop paces itself off AcceptInterval. A default that admits only a -// handful of connections per second cannot drain the kernel accept backlog on a -// public node: peers queue behind it, time out mid-handshake, and the node stops -// acquiring inbound peers while still reporting healthy. Pin the default so that -// regression has to be deliberate rather than incidental. +// The accept loop paces itself off AcceptInterval. 10ms (100/s) sits well above +// the rate at which peers arrive on a public listener; a default admitting only a +// handful per second cannot drain the kernel accept backlog, so peers queue behind +// it, time out mid-handshake, and the node stops acquiring inbound peers while +// still reporting healthy. Pin the value exactly, so changing it is deliberate and +// visible in the diff. func TestP2PConfigAcceptInterval(t *testing.T) { cfg := DefaultP2PConfig() require.NoError(t, cfg.ValidateBasic()) - // Exact value, so a change to the default shows up in the diff rather than - // sliding anywhere inside the band below. require.Equal(t, 10*time.Millisecond, cfg.AcceptInterval) - // The band and its message carry the reason the exact value was chosen. - limit := rate.Every(cfg.AcceptInterval) - require.Greater(t, float64(limit), 50.0, - "default accept rate %v/s is too low to drain the accept backlog", float64(limit)) - require.NotEqual(t, rate.Inf, limit, "default accept rate should be bounded, not unlimited") - - // A non-positive interval is the documented escape hatch for disabling the - // limiter outright, and must stay valid rather than becoming a zero rate. + // A zero interval is the documented escape hatch for disabling the limiter + // outright, and must stay valid rather than becoming a zero rate. cfg.AcceptInterval = 0 require.NoError(t, cfg.ValidateBasic()) require.Equal(t, rate.Inf, rate.Every(cfg.AcceptInterval)) diff --git a/sei-tendermint/config/toml.go b/sei-tendermint/config/toml.go index 33134737f2..869daa4dbd 100644 --- a/sei-tendermint/config/toml.go +++ b/sei-tendermint/config/toml.go @@ -349,9 +349,10 @@ dial-timeout = "{{ .P2P.DialTimeout }}" # How often the node accepts a new inbound connection. This paces the accept # loop only: concurrent handshakes are capped at max-connections minus the # outbound reservation, and the per-source attempt rate by -# max-incoming-connection-attempts. Set too high a value and the kernel accept -# backlog outpaces the loop, so arriving peers wait past handshake-timeout and -# the node stops acquiring inbound peers. A value of 0 disables the limiter. +# max-incoming-connection-attempts. A larger interval paces the loop more slowly; +# if the kernel accept backlog outpaces it, arriving peers wait past +# handshake-timeout and the node silently stops acquiring inbound peers. +# A value of 0 disables the limiter. accept-interval = "{{ .P2P.AcceptInterval }}" # Time to wait before flushing messages out on the connection diff --git a/sei-tendermint/internal/p2p/routeroptions.go b/sei-tendermint/internal/p2p/routeroptions.go index 81082224ab..1e08de50e4 100644 --- a/sei-tendermint/internal/p2p/routeroptions.go +++ b/sei-tendermint/internal/p2p/routeroptions.go @@ -68,9 +68,11 @@ type RouterOptions struct { // MaxDialRate limits the rate at which router is dialing peers. Defaults to 0.1/s. MaxDialRate utils.Option[rate.Limit] - // MaxAcceptRate limits the rate at which router is accepting TCP connections. Defaults to 1/s. - // Node setup always sets this from the p2p accept-interval config key, so the default - // applies only to embedders that construct RouterOptions directly. + // MaxAcceptRate limits the rate at which router is accepting TCP connections. Defaults to 100/s. + // Node setup sets this from the p2p accept-interval config key; the default covers + // embedders that construct RouterOptions directly. Keep it high enough to drain the + // kernel accept backlog: a rate below the arrival rate leaves peers queued past + // handshake-timeout, so the node stops acquiring inbound peers while looking healthy. MaxAcceptRate utils.Option[rate.Limit] // ResolveTimeout is the timeout for resolving NodeAddress URLs. @@ -167,7 +169,7 @@ func (o *RouterOptions) maxDialRate() rate.Limit { } func (o *RouterOptions) maxAcceptRate() rate.Limit { - return o.MaxAcceptRate.Or(rate.Every(time.Second)) + return o.MaxAcceptRate.Or(rate.Every(10 * time.Millisecond)) } func (o *RouterOptions) incomingConnectionWindow() time.Duration { diff --git a/sei-tendermint/node/setup_test.go b/sei-tendermint/node/setup_test.go index ee6fd065ae..5f6be9af1f 100644 --- a/sei-tendermint/node/setup_test.go +++ b/sei-tendermint/node/setup_test.go @@ -2,6 +2,7 @@ package node import ( "encoding/json" + "fmt" "net/url" "os" "path/filepath" @@ -353,7 +354,7 @@ func TestPreparePersistentStateDir_EmptyStringIsNone(t *testing.T) { // dropped here, or wired to the wrong config key, falls back to a package // default silently rather than failing. func TestP2PRouterOptions_PacingAndBudgetWiring(t *testing.T) { - ep, err := p2p.ResolveEndpoint("tcp://" + string(types.NodeID("0000000000000000000000000000000000000000")) + "@127.0.0.1:26656") + ep, err := p2p.ResolveEndpoint("tcp://0000000000000000000000000000000000000000@127.0.0.1:26656") require.NoError(t, err) t.Run("defaults reach the router", func(t *testing.T) { @@ -365,10 +366,6 @@ func TestP2PRouterOptions_PacingAndBudgetWiring(t *testing.T) { const unset = rate.Limit(-1) require.Equal(t, rate.Every(cfg.P2P.AcceptInterval), opts.MaxAcceptRate.Or(unset)) require.Equal(t, rate.Every(cfg.P2P.DialInterval), opts.MaxDialRate.Or(unset)) - - // The package fallback is 1 accept/s; setup must override it. - require.NotEqual(t, rate.Every(time.Second), opts.MaxAcceptRate.Or(unset), - "accept rate fell through to the package default") }) t.Run("operator value flows through", func(t *testing.T) { @@ -378,14 +375,24 @@ func TestP2PRouterOptions_PacingAndBudgetWiring(t *testing.T) { require.Equal(t, rate.Every(250*time.Millisecond), opts.MaxAcceptRate.Or(rate.Limit(-1))) }) - t.Run("concurrent accepts track the inbound pool, not max-connections", func(t *testing.T) { - cfg := config.DefaultConfig() - cfg.P2P.MaxConnections = 100 - opts := p2pRouterOptions(cfg, ep, nil) - - // 100 total minus the 20 outbound reservation. - require.Equal(t, 80, opts.MaxConcurrentAccepts.Or(-1)) - require.Equal(t, 80, opts.MaxInbound.Or(-1)) - require.Equal(t, 20, opts.MaxOutbound.Or(-1)) - }) + // Non-default totals, so the assertions track the derivation rather than + // restating DefaultP2PConfig. 50 exercises the flat 20-outbound reservation; + // 30 exercises the min(20, (maxConns+1)/2) branch, which nothing else reaches. + for _, tc := range []struct { + maxConns, wantInbound, wantOutbound int + }{ + {maxConns: 50, wantInbound: 30, wantOutbound: 20}, + {maxConns: 30, wantInbound: 15, wantOutbound: 15}, + } { + t.Run(fmt.Sprintf("budget derives from max-connections=%d", tc.maxConns), func(t *testing.T) { + cfg := config.DefaultConfig() + cfg.P2P.MaxConnections = uint(tc.maxConns) + opts := p2pRouterOptions(cfg, ep, nil) + + require.Equal(t, tc.wantInbound, opts.MaxInbound.Or(-1)) + require.Equal(t, tc.wantOutbound, opts.MaxOutbound.Or(-1)) + // MaxConcurrentAccepts tracks the inbound pool, not max-connections. + require.Equal(t, tc.wantInbound, opts.MaxConcurrentAccepts.Or(-1)) + }) + } } From b816df210682535a00e66d28a352b4db77940201 Mon Sep 17 00:00:00 2001 From: bdchatham Date: Tue, 11 Aug 2026 09:54:52 -0700 Subject: [PATCH 05/12] address review round 3: correct the connTracker rationale, close the stale framing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The doc comment's claim about max-incoming-connection-attempts was wrong, and it had propagated into the operator-facing template and the PR description. connTracker.AddConn (internal/p2p/conn_tracker.go:33) compares against a count that RemoveConn decrements, so `max` is 100 *concurrent* connections per source IP, not an attempt rate; the IncomingConnectionWindow check applies only when that count is currently zero, making it a re-connect delay rather than a rate. That changes the argument, not just the wording: 100 concurrent per IP exceeds the default inbound pool of 80, so this limiter does not bound a single source below the pool and cannot be the backstop the comment implied. Both the config doc comment and the template now say what it actually does. Also: * dial-interval's new negative check had no coverage — the previous round added the guard and claimed the test. Added "DialInterval" to fieldsToTest; verified removing the guard now fails TestP2PConfigValidateBasic. * p2p_compat_test.go still described both pacing knobs as hidden, which the previous round falsified by rendering accept-interval. Renamed TestHiddenP2PKnobsStillParseFromExistingConfig -> TestP2PPacingKnobsParseFromExistingConfig and narrowed the comment to dial-interval, the only key still absent from the template. * Reframed TestFreshP2PConfigKeepsDefaultPacing as TestP2PConfigPredatingPacingKnobsKeepsDefaults. It never mirrored a fresh template; what it actually pins is a config.toml written before these keys existed — the case every already-deployed node is in, since seid does not rewrite an existing config.toml. Co-Authored-By: Claude Opus 5 --- sei-tendermint/config/config.go | 8 ++++--- sei-tendermint/config/config_test.go | 1 + sei-tendermint/config/p2p_compat_test.go | 30 ++++++++++++------------ sei-tendermint/config/toml.go | 5 ++-- sei-tendermint/config/toml_test.go | 2 +- 5 files changed, 25 insertions(+), 21 deletions(-) diff --git a/sei-tendermint/config/config.go b/sei-tendermint/config/config.go index cdc4eee933..d005d797bb 100644 --- a/sei-tendermint/config/config.go +++ b/sei-tendermint/config/config.go @@ -727,9 +727,11 @@ type P2PConfig struct { // How often node should accept a new inbound connection. This paces the // accept loop only. The number of connections being handshaked concurrently // is bounded separately by RouterOptions.MaxConcurrentAccepts, which node - // setup derives from max-connections minus the outbound reservation, and the - // per-source attempt rate by max-incoming-connection-attempts. A value of 0 - // disables the limiter. + // setup derives from max-connections minus the outbound reservation. + // max-incoming-connection-attempts caps concurrent connections per source IP + // (with a re-connect delay applied only once a source drops back to zero); + // note its default exceeds the inbound pool, so it does not bound a single + // source below that pool. A value of 0 disables the limiter. AcceptInterval time.Duration `mapstructure:"accept-interval"` // Testing params. diff --git a/sei-tendermint/config/config_test.go b/sei-tendermint/config/config_test.go index 649d9859d6..08d8601119 100644 --- a/sei-tendermint/config/config_test.go +++ b/sei-tendermint/config/config_test.go @@ -231,6 +231,7 @@ func TestP2PConfigValidateBasic(t *testing.T) { "MaxPacketMsgPayloadSize", "SendRate", "RecvRate", + "DialInterval", "AcceptInterval", } diff --git a/sei-tendermint/config/p2p_compat_test.go b/sei-tendermint/config/p2p_compat_test.go index 2500b6f273..e09610fd40 100644 --- a/sei-tendermint/config/p2p_compat_test.go +++ b/sei-tendermint/config/p2p_compat_test.go @@ -13,15 +13,15 @@ import ( tmconfig "github.com/sei-protocol/sei-chain/sei-tendermint/config" ) -// This test (and TestFreshP2PConfigKeepsDefaultPacing) mutate the global viper -// singleton via commands.ParseConfig, so they must not run in parallel with -// other tests in this package. +// This test (and TestP2PConfigPredatingPacingKnobsKeepsDefaults) mutate the +// global viper singleton via commands.ParseConfig, so they must not run in +// parallel with other tests in this package. -// The p2p pacing knobs are deliberately absent from the generated template -// (see checkConfig in toml_test.go), so nothing else proves an operator can -// actually set them. Without this, "not in the template" and "not readable" -// are indistinguishable. -func TestHiddenP2PKnobsStillParseFromExistingConfig(t *testing.T) { +// accept-interval is rendered in the template, but dial-interval is not (see +// checkConfig in toml_test.go), so for that key "absent from the template" and +// "not readable at all" would otherwise be indistinguishable. Cover both, since +// an operator sets them the same way. +func TestP2PPacingKnobsParseFromExistingConfig(t *testing.T) { viper.Reset() t.Cleanup(viper.Reset) @@ -44,13 +44,13 @@ accept-interval = "20ms" require.NoError(t, cfg.P2P.ValidateBasic()) } -// TestFreshP2PConfigKeepsDefaultPacing mirrors the freshly-rendered template -// (no pacing knobs in the file) and verifies ParseConfig still produces the -// defaults. Both directions matter: a zeroed AcceptInterval means -// rate.Every(0) == rate.Inf, i.e. no accept pacing at all, while a value large -// enough to matter throttles the accept loop below the rate at which peers -// arrive. Neither is visible in the rendered config, so pin it here. -func TestFreshP2PConfigKeepsDefaultPacing(t *testing.T) { +// TestP2PConfigPredatingPacingKnobsKeepsDefaults mirrors a config.toml written +// before these keys existed — the case every already-deployed node is in, since +// seid does not rewrite an existing config.toml — and verifies ParseConfig still +// produces the defaults. The failure it guards is silent: a zeroed AcceptInterval +// means rate.Every(0) == rate.Inf, disabling accept pacing entirely, and an +// absent key must not land there. +func TestP2PConfigPredatingPacingKnobsKeepsDefaults(t *testing.T) { viper.Reset() t.Cleanup(viper.Reset) diff --git a/sei-tendermint/config/toml.go b/sei-tendermint/config/toml.go index 869daa4dbd..80fe0a2a13 100644 --- a/sei-tendermint/config/toml.go +++ b/sei-tendermint/config/toml.go @@ -348,8 +348,9 @@ dial-timeout = "{{ .P2P.DialTimeout }}" # How often the node accepts a new inbound connection. This paces the accept # loop only: concurrent handshakes are capped at max-connections minus the -# outbound reservation, and the per-source attempt rate by -# max-incoming-connection-attempts. A larger interval paces the loop more slowly; +# outbound reservation. max-incoming-connection-attempts caps concurrent +# connections per source IP, and its default exceeds that inbound pool, so it +# does not bound a single source below it. A larger interval paces the loop more slowly; # if the kernel accept backlog outpaces it, arriving peers wait past # handshake-timeout and the node silently stops acquiring inbound peers. # A value of 0 disables the limiter. diff --git a/sei-tendermint/config/toml_test.go b/sei-tendermint/config/toml_test.go index c4868a6067..5eecfa737f 100644 --- a/sei-tendermint/config/toml_test.go +++ b/sei-tendermint/config/toml_test.go @@ -107,7 +107,7 @@ func checkConfig(t *testing.T, configFile string) { // dial-interval remains an expert-only knob, left out of the generated // template while still being parsed from existing config files. - // See TestHiddenP2PKnobsStillParseFromExistingConfig. + // See TestP2PPacingKnobsParseFromExistingConfig. var hiddenP2PElems = []string{ "dial-interval", } From be9ad9057673b95603c2d69899f3a35b3a2d5944 Mon Sep 17 00:00:00 2001 From: bdchatham Date: Tue, 11 Aug 2026 11:48:05 -0700 Subject: [PATCH 06/12] address review round 4: route [p2p] through Config.ValidateBasic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P2PConfig.ValidateBasic has never run in production. Config.ValidateBasic (config.go:146) routes BaseConfig, RPC, Mempool, StateSync, Consensus, Instrumentation and SelfRemediation, but not P2P — and both production entry points go through it, commands.ParseConfig and seid's interceptConfigs. Before this commit the only callers of P2PConfig.ValidateBasic in the whole tree were the tests added by this PR, which is what made the coverage look real. So the negative-interval guards the previous round added were dead where it counts: accept-interval = "-1s" started successfully and rate.Every turned it into rate.Inf, disabling the accept limiter outright. The same was true of the pre-existing send-rate, recv-rate, flush-throttle-timeout and max-packet-msg-payload-size checks, for their whole life. Route the section, and assert the routing rather than the section's own checks — the latter passed the entire time nothing called them, which is the failure mode worth guarding. Verified: removing the new routing line fails TestConfigValidateBasicRoutesP2P. Confirmed no config in the tree relied on the gap; config, cmd, internal/p2p and the root module all build and pass. This is the AGENTS.md choke-point rule again, one level up from where the last round applied it: a guard that every path must pass through, rather than a section that happens to own its checks. Also cross-referenced the two copies of the 10ms accept default. config cannot import internal/p2p, so they are deliberate copies; each doc comment now names the other, since only the config-side one is pinned by a test. Co-Authored-By: Claude Opus 5 --- sei-tendermint/config/config.go | 4 ++++ sei-tendermint/config/config_test.go | 14 ++++++++++++++ sei-tendermint/internal/p2p/routeroptions.go | 4 +++- 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/sei-tendermint/config/config.go b/sei-tendermint/config/config.go index d005d797bb..81a252985d 100644 --- a/sei-tendermint/config/config.go +++ b/sei-tendermint/config/config.go @@ -150,6 +150,9 @@ func (cfg *Config) ValidateBasic() error { if err := cfg.RPC.ValidateBasic(); err != nil { return fmt.Errorf("error in [rpc] section: %w", err) } + if err := cfg.P2P.ValidateBasic(); err != nil { + return fmt.Errorf("error in [p2p] section: %w", err) + } if err := cfg.Mempool.ValidateBasic(); err != nil { return fmt.Errorf("error in [mempool] section: %w", err) } @@ -728,6 +731,7 @@ type P2PConfig struct { // accept loop only. The number of connections being handshaked concurrently // is bounded separately by RouterOptions.MaxConcurrentAccepts, which node // setup derives from max-connections minus the outbound reservation. + // Kept in sync with the p2p router's own MaxAcceptRate default. // max-incoming-connection-attempts caps concurrent connections per source IP // (with a re-connect delay applied only once a source drops back to zero); // note its default exceeds the inbound pool, so it does not bound a single diff --git a/sei-tendermint/config/config_test.go b/sei-tendermint/config/config_test.go index 08d8601119..2ba67d8379 100644 --- a/sei-tendermint/config/config_test.go +++ b/sei-tendermint/config/config_test.go @@ -39,6 +39,20 @@ func TestConfigValidateBasic(t *testing.T) { assert.Error(t, cfg.ValidateBasic()) } +// P2PConfig.ValidateBasic was unreachable from production: Config.ValidateBasic +// routed every other section but not [p2p], so its checks — including the +// pre-existing send-rate/recv-rate ones — never ran outside tests, and a negative +// accept-interval reached rate.Every as rate.Inf, silently disabling the accept +// limiter. Assert the section is routed, not merely that its own checks work; the +// latter passed the whole time nothing called them. +func TestConfigValidateBasicRoutesP2P(t *testing.T) { + cfg := DefaultConfig() + require.NoError(t, cfg.ValidateBasic()) + + cfg.P2P.AcceptInterval = -1 + require.Error(t, cfg.ValidateBasic()) +} + func TestTLSConfiguration(t *testing.T) { cfg := DefaultConfig() cfg.SetRoot("/home/user") diff --git a/sei-tendermint/internal/p2p/routeroptions.go b/sei-tendermint/internal/p2p/routeroptions.go index 1e08de50e4..79fffe0ca8 100644 --- a/sei-tendermint/internal/p2p/routeroptions.go +++ b/sei-tendermint/internal/p2p/routeroptions.go @@ -68,7 +68,9 @@ type RouterOptions struct { // MaxDialRate limits the rate at which router is dialing peers. Defaults to 0.1/s. MaxDialRate utils.Option[rate.Limit] - // MaxAcceptRate limits the rate at which router is accepting TCP connections. Defaults to 100/s. + // MaxAcceptRate limits the rate at which router is accepting TCP connections. Defaults to 100/s, + // kept in sync with config.DefaultP2PConfig().AcceptInterval (config cannot import this + // package, so the two values are deliberate copies rather than a shared constant). // Node setup sets this from the p2p accept-interval config key; the default covers // embedders that construct RouterOptions directly. Keep it high enough to drain the // kernel accept backlog: a rate below the arrival rate leaves peers queued past From e56a4791c8e6e01d02eb2a7d8e9f41fd62d5e28e Mon Sep 17 00:00:00 2001 From: bdchatham Date: Wed, 12 Aug 2026 08:41:17 -0700 Subject: [PATCH 07/12] address review round 5: collapse the duplicated accept default to one constant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous round left the 10ms default in two places and a comment asking a future editor to keep them equal. Reviewer is right that a comment is a convention, not an invariant. The suggested fix was a drift test in node/setup_test.go, on the reasoning that config does not import internal/p2p. It does — config/autobahn.go:10 — so the dependency already runs config -> p2p, and the stronger fix is available: export DefaultAcceptInterval from internal/p2p and have DefaultP2PConfig() set accept-interval from it. One value with two references cannot drift, so no test is needed to detect drift that can no longer happen. (A drift test was also not reachable as suggested: maxAcceptRate is unexported, so neither node nor an external p2p_test package can call it.) TestP2PConfigAcceptInterval now transitively pins both sides; verified by moving the constant to 19ms, which fails it with "expected: 10ms, actual: 19ms". No behavior change: the value is the same on both sides as before. Co-Authored-By: Claude Opus 5 --- sei-tendermint/config/config.go | 5 +++-- sei-tendermint/internal/p2p/routeroptions.go | 16 ++++++++++++---- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/sei-tendermint/config/config.go b/sei-tendermint/config/config.go index 81a252985d..ce3d978178 100644 --- a/sei-tendermint/config/config.go +++ b/sei-tendermint/config/config.go @@ -12,6 +12,7 @@ import ( "time" mempoolcfg "github.com/sei-protocol/sei-chain/sei-tendermint/internal/mempool" + "github.com/sei-protocol/sei-chain/sei-tendermint/internal/p2p" tmos "github.com/sei-protocol/sei-chain/sei-tendermint/libs/os" "github.com/sei-protocol/sei-chain/sei-tendermint/libs/utils" "github.com/sei-protocol/sei-chain/sei-tendermint/types" @@ -731,7 +732,7 @@ type P2PConfig struct { // accept loop only. The number of connections being handshaked concurrently // is bounded separately by RouterOptions.MaxConcurrentAccepts, which node // setup derives from max-connections minus the outbound reservation. - // Kept in sync with the p2p router's own MaxAcceptRate default. + // Defaults to p2p.DefaultAcceptInterval, the same constant the router falls back to. // max-incoming-connection-attempts caps concurrent connections per source IP // (with a re-connect delay applied only once a source drops back to zero); // note its default exceeds the inbound pool, so it does not bound a single @@ -768,7 +769,7 @@ func DefaultP2PConfig() *P2PConfig { HandshakeTimeout: 10 * time.Second, DialTimeout: 3 * time.Second, DialInterval: 10 * time.Second, - AcceptInterval: 10 * time.Millisecond, + AcceptInterval: p2p.DefaultAcceptInterval, TestDialFail: false, QueueType: "simple-priority", } diff --git a/sei-tendermint/internal/p2p/routeroptions.go b/sei-tendermint/internal/p2p/routeroptions.go index 79fffe0ca8..eed3d02c77 100644 --- a/sei-tendermint/internal/p2p/routeroptions.go +++ b/sei-tendermint/internal/p2p/routeroptions.go @@ -68,9 +68,9 @@ type RouterOptions struct { // MaxDialRate limits the rate at which router is dialing peers. Defaults to 0.1/s. MaxDialRate utils.Option[rate.Limit] - // MaxAcceptRate limits the rate at which router is accepting TCP connections. Defaults to 100/s, - // kept in sync with config.DefaultP2PConfig().AcceptInterval (config cannot import this - // package, so the two values are deliberate copies rather than a shared constant). + // MaxAcceptRate limits the rate at which router is accepting TCP connections. + // Defaults to rate.Every(DefaultAcceptInterval), the same constant config uses for + // the accept-interval key, so the two defaults cannot drift. // Node setup sets this from the p2p accept-interval config key; the default covers // embedders that construct RouterOptions directly. Keep it high enough to drain the // kernel accept backlog: a rate below the arrival rate leaves peers queued past @@ -170,8 +170,16 @@ func (o *RouterOptions) maxDialRate() rate.Limit { return o.MaxDialRate.Or(rate.Every(10 * time.Second)) } +// DefaultAcceptInterval paces the inbound accept loop at 100 accepts/s. Exported +// because config.DefaultP2PConfig() sets accept-interval from it: the router default +// and the config default are then one value, not two that a comment asks you to keep +// equal. Keep it well above the rate at which peers arrive on a public listener — a +// slower loop leaves the kernel accept backlog undrained, so peers wait past +// handshake-timeout and the node stops acquiring inbound peers while looking healthy. +const DefaultAcceptInterval = 10 * time.Millisecond + func (o *RouterOptions) maxAcceptRate() rate.Limit { - return o.MaxAcceptRate.Or(rate.Every(10 * time.Millisecond)) + return o.MaxAcceptRate.Or(rate.Every(DefaultAcceptInterval)) } func (o *RouterOptions) incomingConnectionWindow() time.Duration { From 0c95de0f8ffd891d95dbb5c731e2713457592708 Mon Sep 17 00:00:00 2001 From: bdchatham Date: Wed, 12 Aug 2026 08:44:37 -0700 Subject: [PATCH 08/12] conform godocs to the AGENTS.md Godoc rules added on main The main merge added a Godoc section: godocs say what a thing is, not why it came to be or how it works inside, and never record design history. Several comments in this PR predate that and violate it. * DefaultAcceptInterval explained why it is exported and how the accept backlog fails. Now states what it is. * MaxAcceptRate's field comment toured the surrounding system. Now names its default. * p2pRouterOptions recorded why it was split out of createRouter and what the bug had been. Now states what it returns. * AcceptInterval's field comment carried the same system tour. The operator-facing rationale it held already lives in the config.toml template, which is where an operator reads it. Rewritten rather than trimmed, per the section's own rule. Co-Authored-By: Claude Opus 5 --- sei-tendermint/config/config.go | 11 ++--------- sei-tendermint/internal/p2p/routeroptions.go | 16 ++++------------ sei-tendermint/node/setup.go | 7 ++----- 3 files changed, 8 insertions(+), 26 deletions(-) diff --git a/sei-tendermint/config/config.go b/sei-tendermint/config/config.go index ce3d978178..77c7d2a974 100644 --- a/sei-tendermint/config/config.go +++ b/sei-tendermint/config/config.go @@ -728,15 +728,8 @@ type P2PConfig struct { // How often node should dial a new peer. DialInterval time.Duration `mapstructure:"dial-interval"` - // How often node should accept a new inbound connection. This paces the - // accept loop only. The number of connections being handshaked concurrently - // is bounded separately by RouterOptions.MaxConcurrentAccepts, which node - // setup derives from max-connections minus the outbound reservation. - // Defaults to p2p.DefaultAcceptInterval, the same constant the router falls back to. - // max-incoming-connection-attempts caps concurrent connections per source IP - // (with a re-connect delay applied only once a source drops back to zero); - // note its default exceeds the inbound pool, so it does not bound a single - // source below that pool. A value of 0 disables the limiter. + // How often node should accept a new inbound connection. A value of 0 disables + // the limiter. Defaults to p2p.DefaultAcceptInterval. AcceptInterval time.Duration `mapstructure:"accept-interval"` // Testing params. diff --git a/sei-tendermint/internal/p2p/routeroptions.go b/sei-tendermint/internal/p2p/routeroptions.go index eed3d02c77..6660a7452d 100644 --- a/sei-tendermint/internal/p2p/routeroptions.go +++ b/sei-tendermint/internal/p2p/routeroptions.go @@ -69,12 +69,7 @@ type RouterOptions struct { MaxDialRate utils.Option[rate.Limit] // MaxAcceptRate limits the rate at which router is accepting TCP connections. - // Defaults to rate.Every(DefaultAcceptInterval), the same constant config uses for - // the accept-interval key, so the two defaults cannot drift. - // Node setup sets this from the p2p accept-interval config key; the default covers - // embedders that construct RouterOptions directly. Keep it high enough to drain the - // kernel accept backlog: a rate below the arrival rate leaves peers queued past - // handshake-timeout, so the node stops acquiring inbound peers while looking healthy. + // Defaults to rate.Every(DefaultAcceptInterval). MaxAcceptRate utils.Option[rate.Limit] // ResolveTimeout is the timeout for resolving NodeAddress URLs. @@ -170,12 +165,9 @@ func (o *RouterOptions) maxDialRate() rate.Limit { return o.MaxDialRate.Or(rate.Every(10 * time.Second)) } -// DefaultAcceptInterval paces the inbound accept loop at 100 accepts/s. Exported -// because config.DefaultP2PConfig() sets accept-interval from it: the router default -// and the config default are then one value, not two that a comment asks you to keep -// equal. Keep it well above the rate at which peers arrive on a public listener — a -// slower loop leaves the kernel accept backlog undrained, so peers wait past -// handshake-timeout and the node stops acquiring inbound peers while looking healthy. +// DefaultAcceptInterval is the interval between inbound connection accepts when +// MaxAcceptRate is unset, i.e. 100 accepts/s. config.DefaultP2PConfig sets +// accept-interval from it. const DefaultAcceptInterval = 10 * time.Millisecond func (o *RouterOptions) maxAcceptRate() rate.Limit { diff --git a/sei-tendermint/node/setup.go b/sei-tendermint/node/setup.go index df2b3f77a1..ca22970568 100644 --- a/sei-tendermint/node/setup.go +++ b/sei-tendermint/node/setup.go @@ -454,11 +454,8 @@ func buildFullnodeGigaConfig( }, nil } -// p2pRouterOptions derives the router's connection budget and pacing from the -// p2p config. Split out of createRouter so the derivation is testable on its -// own: any RouterOptions field left unset here silently falls back to a package -// default rather than failing, which is how the accept rate stayed pinned at -// its 1/s default while max-connections appeared to govern it. +// p2pRouterOptions returns the router's connection budget and pacing, derived +// from the p2p config. func p2pRouterOptions(cfg *config.Config, ep p2p.Endpoint, privatePeerIDs []types.NodeID) *p2p.RouterOptions { // MaxConnections defaults to 64 maxConns := 64 From a4b857e1f563999c2eff0ae3c72a8827caaaabff Mon Sep 17 00:00:00 2001 From: bdchatham Date: Wed, 12 Aug 2026 08:58:53 -0700 Subject: [PATCH 09/12] address human review: drop the cross-package default coupling and the sentinel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pompon0: the routeroptions default should not reference where it is set or claim to be kept in sync — that property is fragile, and the defaults are moving to sei-tendermint/config once the RouterOptions fields become required. Reverted the exported DefaultAcceptInterval and the config-side reference; both sides carry a plain literal again and neither comment mentions the other. This undoes the previous round's coupling, which pointed from internal/p2p at config, the inverse of the real dependency. pompon0: an artificial sentinel is bad practice. The wiring test now compares utils.Some(rate.Every(...)) against the Option directly, and the budget cases likewise compare utils.Some(want) rather than unwrapping with .Or(-1). amir-deris: dropped the "This PR's own diagnosis" preamble; the comment now states what the test holds in the present tense. Also from review: * Added a subtest for accept-interval = 0. The template promises it disables the limiter, and that promise was only pinned in config_test as rate.Every(0) == rate.Inf — a property of golang.org/x/time/rate rather than of this wiring passing the value through. * Trimmed the template comment from eight lines to four. Three of them described max-incoming-connection-attempts' relationship to the inbound pool, which is a cross-key numeric claim nothing checks and would go stale silently. Co-Authored-By: Claude Opus 5 --- sei-tendermint/config/config.go | 5 ++-- sei-tendermint/config/toml.go | 12 +++----- sei-tendermint/internal/p2p/routeroptions.go | 9 ++---- sei-tendermint/node/setup_test.go | 29 ++++++++++---------- 4 files changed, 23 insertions(+), 32 deletions(-) diff --git a/sei-tendermint/config/config.go b/sei-tendermint/config/config.go index 77c7d2a974..155136fb75 100644 --- a/sei-tendermint/config/config.go +++ b/sei-tendermint/config/config.go @@ -12,7 +12,6 @@ import ( "time" mempoolcfg "github.com/sei-protocol/sei-chain/sei-tendermint/internal/mempool" - "github.com/sei-protocol/sei-chain/sei-tendermint/internal/p2p" tmos "github.com/sei-protocol/sei-chain/sei-tendermint/libs/os" "github.com/sei-protocol/sei-chain/sei-tendermint/libs/utils" "github.com/sei-protocol/sei-chain/sei-tendermint/types" @@ -729,7 +728,7 @@ type P2PConfig struct { DialInterval time.Duration `mapstructure:"dial-interval"` // How often node should accept a new inbound connection. A value of 0 disables - // the limiter. Defaults to p2p.DefaultAcceptInterval. + // the limiter. AcceptInterval time.Duration `mapstructure:"accept-interval"` // Testing params. @@ -762,7 +761,7 @@ func DefaultP2PConfig() *P2PConfig { HandshakeTimeout: 10 * time.Second, DialTimeout: 3 * time.Second, DialInterval: 10 * time.Second, - AcceptInterval: p2p.DefaultAcceptInterval, + AcceptInterval: 10 * time.Millisecond, TestDialFail: false, QueueType: "simple-priority", } diff --git a/sei-tendermint/config/toml.go b/sei-tendermint/config/toml.go index 80fe0a2a13..247b44af10 100644 --- a/sei-tendermint/config/toml.go +++ b/sei-tendermint/config/toml.go @@ -346,14 +346,10 @@ allow-duplicate-ip = {{ .P2P.AllowDuplicateIP }} handshake-timeout = "{{ .P2P.HandshakeTimeout }}" dial-timeout = "{{ .P2P.DialTimeout }}" -# How often the node accepts a new inbound connection. This paces the accept -# loop only: concurrent handshakes are capped at max-connections minus the -# outbound reservation. max-incoming-connection-attempts caps concurrent -# connections per source IP, and its default exceeds that inbound pool, so it -# does not bound a single source below it. A larger interval paces the loop more slowly; -# if the kernel accept backlog outpaces it, arriving peers wait past -# handshake-timeout and the node silently stops acquiring inbound peers. -# A value of 0 disables the limiter. +# How often the node accepts a new inbound connection. A larger interval paces +# the accept loop more slowly; if the kernel accept backlog outpaces it, arriving +# peers wait past handshake-timeout and the node silently stops acquiring inbound +# peers. A value of 0 disables the limiter. accept-interval = "{{ .P2P.AcceptInterval }}" # Time to wait before flushing messages out on the connection diff --git a/sei-tendermint/internal/p2p/routeroptions.go b/sei-tendermint/internal/p2p/routeroptions.go index 6660a7452d..4cb9cb66ea 100644 --- a/sei-tendermint/internal/p2p/routeroptions.go +++ b/sei-tendermint/internal/p2p/routeroptions.go @@ -69,7 +69,7 @@ type RouterOptions struct { MaxDialRate utils.Option[rate.Limit] // MaxAcceptRate limits the rate at which router is accepting TCP connections. - // Defaults to rate.Every(DefaultAcceptInterval). + // Defaults to 100/s. MaxAcceptRate utils.Option[rate.Limit] // ResolveTimeout is the timeout for resolving NodeAddress URLs. @@ -165,13 +165,8 @@ func (o *RouterOptions) maxDialRate() rate.Limit { return o.MaxDialRate.Or(rate.Every(10 * time.Second)) } -// DefaultAcceptInterval is the interval between inbound connection accepts when -// MaxAcceptRate is unset, i.e. 100 accepts/s. config.DefaultP2PConfig sets -// accept-interval from it. -const DefaultAcceptInterval = 10 * time.Millisecond - func (o *RouterOptions) maxAcceptRate() rate.Limit { - return o.MaxAcceptRate.Or(rate.Every(DefaultAcceptInterval)) + return o.MaxAcceptRate.Or(rate.Every(10 * time.Millisecond)) } func (o *RouterOptions) incomingConnectionWindow() time.Duration { diff --git a/sei-tendermint/node/setup_test.go b/sei-tendermint/node/setup_test.go index f642e59ca6..34677e2cfc 100644 --- a/sei-tendermint/node/setup_test.go +++ b/sei-tendermint/node/setup_test.go @@ -348,11 +348,8 @@ func TestPreparePersistentStateDir_EmptyStringIsNone(t *testing.T) { require.False(t, ok, "Some(\"\") must be cleared to None for in-memory mode") } -// This PR's own diagnosis is that a 1/s production accept rate survived because -// every RouterOptions construction site except this one substitutes rate.Inf, -// so nothing exercised the real wiring. Assert the derivation directly: a field -// dropped here, or wired to the wrong config key, falls back to a package -// default silently rather than failing. +// Every other RouterOptions construction site substitutes rate.Inf, so this +// derivation is the only place the production accept rate is exercised. func TestP2PRouterOptions_PacingAndBudgetWiring(t *testing.T) { ep, err := p2p.ResolveEndpoint("tcp://0000000000000000000000000000000000000000@127.0.0.1:26656") require.NoError(t, err) @@ -361,18 +358,22 @@ func TestP2PRouterOptions_PacingAndBudgetWiring(t *testing.T) { cfg := config.DefaultConfig() opts := p2pRouterOptions(cfg, ep, nil) - // Sentinel differs from every plausible real value, so .Or() returning it - // means the field was never set. - const unset = rate.Limit(-1) - require.Equal(t, rate.Every(cfg.P2P.AcceptInterval), opts.MaxAcceptRate.Or(unset)) - require.Equal(t, rate.Every(cfg.P2P.DialInterval), opts.MaxDialRate.Or(unset)) + require.Equal(t, utils.Some(rate.Every(cfg.P2P.AcceptInterval)), opts.MaxAcceptRate) + require.Equal(t, utils.Some(rate.Every(cfg.P2P.DialInterval)), opts.MaxDialRate) + }) + + t.Run("zero interval disables the limiter", func(t *testing.T) { + cfg := config.DefaultConfig() + cfg.P2P.AcceptInterval = 0 + opts := p2pRouterOptions(cfg, ep, nil) + require.Equal(t, utils.Some(rate.Inf), opts.MaxAcceptRate) }) t.Run("operator value flows through", func(t *testing.T) { cfg := config.DefaultConfig() cfg.P2P.AcceptInterval = 250 * time.Millisecond opts := p2pRouterOptions(cfg, ep, nil) - require.Equal(t, rate.Every(250*time.Millisecond), opts.MaxAcceptRate.Or(rate.Limit(-1))) + require.Equal(t, utils.Some(rate.Every(250*time.Millisecond)), opts.MaxAcceptRate) }) // Non-default totals, so the assertions track the derivation rather than @@ -389,10 +390,10 @@ func TestP2PRouterOptions_PacingAndBudgetWiring(t *testing.T) { cfg.P2P.MaxConnections = uint(tc.maxConns) opts := p2pRouterOptions(cfg, ep, nil) - require.Equal(t, tc.wantInbound, opts.MaxInbound.Or(-1)) - require.Equal(t, tc.wantOutbound, opts.MaxOutbound.Or(-1)) + require.Equal(t, utils.Some(tc.wantInbound), opts.MaxInbound) + require.Equal(t, utils.Some(tc.wantOutbound), opts.MaxOutbound) // MaxConcurrentAccepts tracks the inbound pool, not max-connections. - require.Equal(t, tc.wantInbound, opts.MaxConcurrentAccepts.Or(-1)) + require.Equal(t, utils.Some(tc.wantInbound), opts.MaxConcurrentAccepts) }) } } From 371b1ee09982b79ef2072626d7162240b60ffefc Mon Sep 17 00:00:00 2001 From: bdchatham Date: Wed, 12 Aug 2026 13:55:19 -0700 Subject: [PATCH 10/12] address review: clamp negative pacing intervals, pin the package defaults MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The negative-interval check added earlier does not fire on the path an operator's typo actually travels. interceptConfigs validates only when it creates config.toml, so an already-deployed node never reaches ValidateBasic, and rate.Every maps every non-positive interval to rate.Inf — accept-interval = "-1s" in an existing file silently disables accept pacing, which is the failure this PR exists to fix. Clamp it where every router passes instead: pacingRate() falls back to the config default for a negative interval, applied to both accept and dial. A configured 0 still disables the limiter, since that is documented. ValidateBasic keeps the negative check as the early, friendly error on the paths that do reach it. Doc comment and template now state both behaviours. Also from review: * Added routeroptions_test.go pinning maxAcceptRate/maxDialRate's package fallbacks. Every harness sets rate.Inf and every production path now sets the value explicitly, so the fallbacks were unexercised and the doc comment was their only record. * Trimmed the test doc comments in config_test.go and p2p_compat_test.go to what each test pins. They were narratives of the bug, which AGENTS.md rules out and the PR body already carries. Co-Authored-By: Claude Opus 5 --- sei-tendermint/config/config.go | 2 +- sei-tendermint/config/config_test.go | 14 +++-------- sei-tendermint/config/p2p_compat_test.go | 15 ++++-------- sei-tendermint/config/toml.go | 2 +- .../internal/p2p/routeroptions_test.go | 24 +++++++++++++++++++ sei-tendermint/node/setup.go | 18 ++++++++++++-- sei-tendermint/node/setup_test.go | 14 +++++++++++ 7 files changed, 64 insertions(+), 25 deletions(-) create mode 100644 sei-tendermint/internal/p2p/routeroptions_test.go diff --git a/sei-tendermint/config/config.go b/sei-tendermint/config/config.go index 155136fb75..f4686761a3 100644 --- a/sei-tendermint/config/config.go +++ b/sei-tendermint/config/config.go @@ -728,7 +728,7 @@ type P2PConfig struct { DialInterval time.Duration `mapstructure:"dial-interval"` // How often node should accept a new inbound connection. A value of 0 disables - // the limiter. + // the limiter; a negative value falls back to the default. AcceptInterval time.Duration `mapstructure:"accept-interval"` // Testing params. diff --git a/sei-tendermint/config/config_test.go b/sei-tendermint/config/config_test.go index 2ba67d8379..d9f3ce5685 100644 --- a/sei-tendermint/config/config_test.go +++ b/sei-tendermint/config/config_test.go @@ -39,12 +39,8 @@ func TestConfigValidateBasic(t *testing.T) { assert.Error(t, cfg.ValidateBasic()) } -// P2PConfig.ValidateBasic was unreachable from production: Config.ValidateBasic -// routed every other section but not [p2p], so its checks — including the -// pre-existing send-rate/recv-rate ones — never ran outside tests, and a negative -// accept-interval reached rate.Every as rate.Inf, silently disabling the accept -// limiter. Assert the section is routed, not merely that its own checks work; the -// latter passed the whole time nothing called them. +// Asserts Config.ValidateBasic routes the [p2p] section, not merely that the +// section's own checks work. func TestConfigValidateBasicRoutesP2P(t *testing.T) { cfg := DefaultConfig() require.NoError(t, cfg.ValidateBasic()) @@ -256,11 +252,7 @@ func TestP2PConfigValidateBasic(t *testing.T) { } } -// The accept loop paces itself off AcceptInterval. 10ms (100/s) sits well above -// the rate at which peers arrive on a public listener; a default admitting only a -// handful per second cannot drain the kernel accept backlog, so peers queue behind -// it, time out mid-handshake, and the node stops acquiring inbound peers while -// still reporting healthy. Pin the value exactly, so changing it is deliberate and +// Pins the accept-interval default exactly, so changing it is deliberate and // visible in the diff. func TestP2PConfigAcceptInterval(t *testing.T) { cfg := DefaultP2PConfig() diff --git a/sei-tendermint/config/p2p_compat_test.go b/sei-tendermint/config/p2p_compat_test.go index e09610fd40..dde8829588 100644 --- a/sei-tendermint/config/p2p_compat_test.go +++ b/sei-tendermint/config/p2p_compat_test.go @@ -17,10 +17,8 @@ import ( // global viper singleton via commands.ParseConfig, so they must not run in // parallel with other tests in this package. -// accept-interval is rendered in the template, but dial-interval is not (see -// checkConfig in toml_test.go), so for that key "absent from the template" and -// "not readable at all" would otherwise be indistinguishable. Cover both, since -// an operator sets them the same way. +// dial-interval is absent from the generated template (see checkConfig in +// toml_test.go), so nothing else shows it is readable at all. func TestP2PPacingKnobsParseFromExistingConfig(t *testing.T) { viper.Reset() t.Cleanup(viper.Reset) @@ -44,12 +42,9 @@ accept-interval = "20ms" require.NoError(t, cfg.P2P.ValidateBasic()) } -// TestP2PConfigPredatingPacingKnobsKeepsDefaults mirrors a config.toml written -// before these keys existed — the case every already-deployed node is in, since -// seid does not rewrite an existing config.toml — and verifies ParseConfig still -// produces the defaults. The failure it guards is silent: a zeroed AcceptInterval -// means rate.Every(0) == rate.Inf, disabling accept pacing entirely, and an -// absent key must not land there. +// TestP2PConfigPredatingPacingKnobsKeepsDefaults asserts a config.toml written +// before these keys existed still parses to the defaults rather than to zero, +// which rate.Every would read as "no pacing". func TestP2PConfigPredatingPacingKnobsKeepsDefaults(t *testing.T) { viper.Reset() t.Cleanup(viper.Reset) diff --git a/sei-tendermint/config/toml.go b/sei-tendermint/config/toml.go index 247b44af10..4d80e1f304 100644 --- a/sei-tendermint/config/toml.go +++ b/sei-tendermint/config/toml.go @@ -349,7 +349,7 @@ dial-timeout = "{{ .P2P.DialTimeout }}" # How often the node accepts a new inbound connection. A larger interval paces # the accept loop more slowly; if the kernel accept backlog outpaces it, arriving # peers wait past handshake-timeout and the node silently stops acquiring inbound -# peers. A value of 0 disables the limiter. +# peers. A value of 0 disables the limiter; a negative value falls back to the default. accept-interval = "{{ .P2P.AcceptInterval }}" # Time to wait before flushing messages out on the connection diff --git a/sei-tendermint/internal/p2p/routeroptions_test.go b/sei-tendermint/internal/p2p/routeroptions_test.go new file mode 100644 index 0000000000..158467a3ea --- /dev/null +++ b/sei-tendermint/internal/p2p/routeroptions_test.go @@ -0,0 +1,24 @@ +package p2p + +import ( + "testing" + "time" + + "golang.org/x/time/rate" + + "github.com/sei-protocol/sei-chain/sei-tendermint/libs/utils" + "github.com/sei-protocol/sei-chain/sei-tendermint/libs/utils/require" +) + +// Every test harness pins MaxAcceptRate/MaxDialRate to rate.Inf and node setup +// always sets them, so these fallbacks are otherwise unexercised. +func TestRouterOptionsPacingDefaults(t *testing.T) { + var o RouterOptions + + require.Equal(t, rate.Every(10*time.Millisecond), o.maxAcceptRate()) + require.Equal(t, rate.Every(10*time.Second), o.maxDialRate()) + + // An explicit value wins over the fallback. + o.MaxAcceptRate = utils.Some(rate.Inf) + require.Equal(t, rate.Inf, o.maxAcceptRate()) +} diff --git a/sei-tendermint/node/setup.go b/sei-tendermint/node/setup.go index ca22970568..16b529e13f 100644 --- a/sei-tendermint/node/setup.go +++ b/sei-tendermint/node/setup.go @@ -454,6 +454,19 @@ func buildFullnodeGigaConfig( }, nil } +// pacingRate returns the rate limit for a configured pacing interval, falling +// back to def when the interval is negative. +func pacingRate(interval, def time.Duration) rate.Limit { + // rate.Every maps every non-positive interval to rate.Inf. A configured 0 means + // "disable the limiter" and is honoured, but a negative value is a typo, and on + // an already-deployed node it never reaches ValidateBasic — interceptConfigs + // validates only when it creates the file. Clamp it where every router passes. + if interval < 0 { + interval = def + } + return rate.Every(interval) +} + // p2pRouterOptions returns the router's connection budget and pacing, derived // from the p2p config. func p2pRouterOptions(cfg *config.Config, ep p2p.Endpoint, privatePeerIDs []types.NodeID) *p2p.RouterOptions { @@ -473,6 +486,7 @@ func p2pRouterOptions(cfg *config.Config, ep p2p.Endpoint, privatePeerIDs []type // TODO(gprusak): eventually we should migrate configs to specify // MaxInbound and MaxOutbound explicitly, rather than doing the computation above. maxInbound := maxConns - maxOutbound + defaults := config.DefaultP2PConfig() connection := conn.DefaultMConnConfig() connection.FlushThrottle = cfg.P2P.FlushThrottleTimeout connection.SendRate = cfg.P2P.SendRate @@ -481,8 +495,8 @@ func p2pRouterOptions(cfg *config.Config, ep p2p.Endpoint, privatePeerIDs []type return &p2p.RouterOptions{ Endpoint: ep, MaxIncomingConnectionAttempts: utils.Some(cfg.P2P.MaxIncomingConnectionAttempts), - MaxDialRate: utils.Some(rate.Every(cfg.P2P.DialInterval)), - MaxAcceptRate: utils.Some(rate.Every(cfg.P2P.AcceptInterval)), + MaxDialRate: utils.Some(pacingRate(cfg.P2P.DialInterval, defaults.DialInterval)), + MaxAcceptRate: utils.Some(pacingRate(cfg.P2P.AcceptInterval, defaults.AcceptInterval)), HandshakeTimeout: utils.Some(cfg.P2P.HandshakeTimeout), DialTimeout: utils.Some(cfg.P2P.DialTimeout), PexOnHandshake: cfg.P2P.PexReactor, diff --git a/sei-tendermint/node/setup_test.go b/sei-tendermint/node/setup_test.go index 34677e2cfc..b257799695 100644 --- a/sei-tendermint/node/setup_test.go +++ b/sei-tendermint/node/setup_test.go @@ -362,6 +362,20 @@ func TestP2PRouterOptions_PacingAndBudgetWiring(t *testing.T) { require.Equal(t, utils.Some(rate.Every(cfg.P2P.DialInterval)), opts.MaxDialRate) }) + // A negative value never reaches ValidateBasic on an already-deployed node, + // so it must not read as "disable" the way rate.Every would treat it. + t.Run("negative interval falls back to the default", func(t *testing.T) { + cfg := config.DefaultConfig() + cfg.P2P.AcceptInterval = -1 * time.Second + cfg.P2P.DialInterval = -1 * time.Second + opts := p2pRouterOptions(cfg, ep, nil) + + defaults := config.DefaultP2PConfig() + require.Equal(t, utils.Some(rate.Every(defaults.AcceptInterval)), opts.MaxAcceptRate) + require.Equal(t, utils.Some(rate.Every(defaults.DialInterval)), opts.MaxDialRate) + require.NotEqual(t, utils.Some(rate.Inf), opts.MaxAcceptRate) + }) + t.Run("zero interval disables the limiter", func(t *testing.T) { cfg := config.DefaultConfig() cfg.P2P.AcceptInterval = 0 From d2b60ee0bb3a67faccf3c2d3df911e009616b1bc Mon Sep 17 00:00:00 2001 From: bdchatham Date: Wed, 12 Aug 2026 14:17:52 -0700 Subject: [PATCH 11/12] address review: stop documenting the clamp, and stop applying it silently MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The field doc and template advertised "a negative value falls back to the default" while ValidateBasic, added in the same diff, rejects it outright. Both were true per-path, which made the docs promise a tolerance the same file removes a few lines down, and invited operators to rely on a last-resort safety net rather than a supported input. Dropped the clause from both; the 0 escape hatch stays, since that one is real. The clamp itself was silent, and it fires on exactly the path where nothing else would tell the operator — an already-deployed config that never reaches ValidateBasic. A typo'd accept-interval was then indistinguishable from the default forever. pacingRate now logs a warning naming the key, the configured value and the substitute; package node already has a logger, so no plumbing. Also: * MaxAcceptRate's doc said "Defaults to 100/s", which reads as a ceiling. The limiter is built with burst = MaxConcurrentAccepts, so the rate is sustained, not absolute. Said so — that distinction is what let the old 1/s survive. * p2p_compat_test.go now decodes through a local viper.New() rather than commands.ParseConfig. ParseConfig is not seid's read path — interceptConfigs unmarshals through its own rootViper — so the fidelity it bought was illusory, while it mutated the global viper singleton and pulled cmd/tendermint/commands into config's test graph. This diverges from statesync_compat_test.go deliberately. Co-Authored-By: Claude Opus 5 --- sei-tendermint/config/config.go | 2 +- sei-tendermint/config/p2p_compat_test.go | 63 +++++++++----------- sei-tendermint/config/toml.go | 2 +- sei-tendermint/internal/p2p/routeroptions.go | 4 +- sei-tendermint/node/setup.go | 11 ++-- 5 files changed, 38 insertions(+), 44 deletions(-) diff --git a/sei-tendermint/config/config.go b/sei-tendermint/config/config.go index f4686761a3..155136fb75 100644 --- a/sei-tendermint/config/config.go +++ b/sei-tendermint/config/config.go @@ -728,7 +728,7 @@ type P2PConfig struct { DialInterval time.Duration `mapstructure:"dial-interval"` // How often node should accept a new inbound connection. A value of 0 disables - // the limiter; a negative value falls back to the default. + // the limiter. AcceptInterval time.Duration `mapstructure:"accept-interval"` // Testing params. diff --git a/sei-tendermint/config/p2p_compat_test.go b/sei-tendermint/config/p2p_compat_test.go index dde8829588..cba267a877 100644 --- a/sei-tendermint/config/p2p_compat_test.go +++ b/sei-tendermint/config/p2p_compat_test.go @@ -9,62 +9,53 @@ import ( "github.com/spf13/viper" "github.com/stretchr/testify/require" - "github.com/sei-protocol/sei-chain/sei-tendermint/cmd/tendermint/commands" tmconfig "github.com/sei-protocol/sei-chain/sei-tendermint/config" ) -// This test (and TestP2PConfigPredatingPacingKnobsKeepsDefaults) mutate the -// global viper singleton via commands.ParseConfig, so they must not run in -// parallel with other tests in this package. +// readP2PConfig decodes a config.toml into a default Config: absent keys keep +// the value already in the struct. +func readP2PConfig(t *testing.T, body string) *tmconfig.P2PConfig { + t.Helper() + + path := filepath.Join(t.TempDir(), "config.toml") + require.NoError(t, os.WriteFile(path, []byte(body), 0600)) + + v := viper.New() + v.SetConfigFile(path) + require.NoError(t, v.ReadInConfig()) + + cfg := tmconfig.DefaultConfig() + require.NoError(t, v.Unmarshal(cfg)) + return cfg.P2P +} // dial-interval is absent from the generated template (see checkConfig in // toml_test.go), so nothing else shows it is readable at all. func TestP2PPacingKnobsParseFromExistingConfig(t *testing.T) { - viper.Reset() - t.Cleanup(viper.Reset) - - configPath := filepath.Join(t.TempDir(), "config.toml") - err := os.WriteFile(configPath, []byte(` + p2p := readP2PConfig(t, ` [p2p] laddr = "tcp://0.0.0.0:26656" dial-interval = "5s" accept-interval = "20ms" -`), 0600) - require.NoError(t, err) +`) - viper.SetConfigFile(configPath) - require.NoError(t, viper.ReadInConfig()) - - cfg, err := commands.ParseConfig(tmconfig.DefaultConfig()) - require.NoError(t, err) - require.Equal(t, 5*time.Second, cfg.P2P.DialInterval) - require.Equal(t, 20*time.Millisecond, cfg.P2P.AcceptInterval) - require.NoError(t, cfg.P2P.ValidateBasic()) + require.Equal(t, 5*time.Second, p2p.DialInterval) + require.Equal(t, 20*time.Millisecond, p2p.AcceptInterval) + require.NoError(t, p2p.ValidateBasic()) } // TestP2PConfigPredatingPacingKnobsKeepsDefaults asserts a config.toml written // before these keys existed still parses to the defaults rather than to zero, // which rate.Every would read as "no pacing". func TestP2PConfigPredatingPacingKnobsKeepsDefaults(t *testing.T) { - viper.Reset() - t.Cleanup(viper.Reset) - - configPath := filepath.Join(t.TempDir(), "config.toml") - err := os.WriteFile(configPath, []byte(` + p2p := readP2PConfig(t, ` [p2p] laddr = "tcp://0.0.0.0:26656" -`), 0600) - require.NoError(t, err) - - viper.SetConfigFile(configPath) - require.NoError(t, viper.ReadInConfig()) - - cfg, err := commands.ParseConfig(tmconfig.DefaultConfig()) - require.NoError(t, err) +`) defaults := tmconfig.DefaultP2PConfig() - require.Equal(t, defaults.AcceptInterval, cfg.P2P.AcceptInterval) - require.Equal(t, defaults.DialInterval, cfg.P2P.DialInterval) - require.NotZero(t, cfg.P2P.AcceptInterval, "a zero accept-interval disables accept pacing entirely") - require.NoError(t, cfg.P2P.ValidateBasic()) + require.Equal(t, defaults.AcceptInterval, p2p.AcceptInterval) + require.Equal(t, defaults.DialInterval, p2p.DialInterval) + require.NotZero(t, p2p.AcceptInterval, "a zero accept-interval disables accept pacing entirely") + require.NoError(t, p2p.ValidateBasic()) } diff --git a/sei-tendermint/config/toml.go b/sei-tendermint/config/toml.go index 4d80e1f304..247b44af10 100644 --- a/sei-tendermint/config/toml.go +++ b/sei-tendermint/config/toml.go @@ -349,7 +349,7 @@ dial-timeout = "{{ .P2P.DialTimeout }}" # How often the node accepts a new inbound connection. A larger interval paces # the accept loop more slowly; if the kernel accept backlog outpaces it, arriving # peers wait past handshake-timeout and the node silently stops acquiring inbound -# peers. A value of 0 disables the limiter; a negative value falls back to the default. +# peers. A value of 0 disables the limiter. accept-interval = "{{ .P2P.AcceptInterval }}" # Time to wait before flushing messages out on the connection diff --git a/sei-tendermint/internal/p2p/routeroptions.go b/sei-tendermint/internal/p2p/routeroptions.go index 4cb9cb66ea..1f13568ae2 100644 --- a/sei-tendermint/internal/p2p/routeroptions.go +++ b/sei-tendermint/internal/p2p/routeroptions.go @@ -68,8 +68,8 @@ type RouterOptions struct { // MaxDialRate limits the rate at which router is dialing peers. Defaults to 0.1/s. MaxDialRate utils.Option[rate.Limit] - // MaxAcceptRate limits the rate at which router is accepting TCP connections. - // Defaults to 100/s. + // MaxAcceptRate limits the sustained rate at which router is accepting TCP + // connections; the limiter's burst is MaxConcurrentAccepts. Defaults to 100/s. MaxAcceptRate utils.Option[rate.Limit] // ResolveTimeout is the timeout for resolving NodeAddress URLs. diff --git a/sei-tendermint/node/setup.go b/sei-tendermint/node/setup.go index 16b529e13f..42b145f2cf 100644 --- a/sei-tendermint/node/setup.go +++ b/sei-tendermint/node/setup.go @@ -456,12 +456,15 @@ func buildFullnodeGigaConfig( // pacingRate returns the rate limit for a configured pacing interval, falling // back to def when the interval is negative. -func pacingRate(interval, def time.Duration) rate.Limit { +func pacingRate(key string, interval, def time.Duration) rate.Limit { // rate.Every maps every non-positive interval to rate.Inf. A configured 0 means // "disable the limiter" and is honoured, but a negative value is a typo, and on // an already-deployed node it never reaches ValidateBasic — interceptConfigs - // validates only when it creates the file. Clamp it where every router passes. + // validates only when it creates the file. Clamp it where every router passes, + // and say so: this is the one path where nothing else tells the operator. if interval < 0 { + logger.Warn("negative p2p interval in config; using default instead", + "key", key, "configured", interval, "default", def) interval = def } return rate.Every(interval) @@ -495,8 +498,8 @@ func p2pRouterOptions(cfg *config.Config, ep p2p.Endpoint, privatePeerIDs []type return &p2p.RouterOptions{ Endpoint: ep, MaxIncomingConnectionAttempts: utils.Some(cfg.P2P.MaxIncomingConnectionAttempts), - MaxDialRate: utils.Some(pacingRate(cfg.P2P.DialInterval, defaults.DialInterval)), - MaxAcceptRate: utils.Some(pacingRate(cfg.P2P.AcceptInterval, defaults.AcceptInterval)), + MaxDialRate: utils.Some(pacingRate("dial-interval", cfg.P2P.DialInterval, defaults.DialInterval)), + MaxAcceptRate: utils.Some(pacingRate("accept-interval", cfg.P2P.AcceptInterval, defaults.AcceptInterval)), HandshakeTimeout: utils.Some(cfg.P2P.HandshakeTimeout), DialTimeout: utils.Some(cfg.P2P.DialTimeout), PexOnHandshake: cfg.P2P.PexReactor, From c2b0acf2892b25bc2996808ffadf11be5fa371fa Mon Sep 17 00:00:00 2001 From: bdchatham Date: Thu, 13 Aug 2026 07:58:40 -0700 Subject: [PATCH 12/12] address review: refuse a negative pacing interval instead of clamping it Greg's call, and the consistent one. ValidateBasic already rejects a negative accept-interval or dial-interval wherever it runs, so warning and substituting the default here gave the same input two different outcomes depending on which path read the config. pacingRate now returns an error and p2pRouterOptions propagates it, so a node configured with a negative interval fails to start rather than starting with pacing quietly replaced. createRouter already returned an error, so nothing above it changes. A configured 0 still disables the limiter; that one is documented and real. The negative test case inverts accordingly and now covers both keys. Co-Authored-By: Claude Opus 5 --- sei-tendermint/node/setup.go | 41 ++++++++++++++++++------------- sei-tendermint/node/setup_test.go | 40 +++++++++++++++++------------- 2 files changed, 47 insertions(+), 34 deletions(-) diff --git a/sei-tendermint/node/setup.go b/sei-tendermint/node/setup.go index 42b145f2cf..425e82dd6a 100644 --- a/sei-tendermint/node/setup.go +++ b/sei-tendermint/node/setup.go @@ -454,25 +454,22 @@ func buildFullnodeGigaConfig( }, nil } -// pacingRate returns the rate limit for a configured pacing interval, falling -// back to def when the interval is negative. -func pacingRate(key string, interval, def time.Duration) rate.Limit { +// pacingRate returns the rate limit for a configured pacing interval. +func pacingRate(key string, interval time.Duration) (rate.Limit, error) { // rate.Every maps every non-positive interval to rate.Inf. A configured 0 means - // "disable the limiter" and is honoured, but a negative value is a typo, and on - // an already-deployed node it never reaches ValidateBasic — interceptConfigs - // validates only when it creates the file. Clamp it where every router passes, - // and say so: this is the one path where nothing else tells the operator. + // "disable the limiter" and is honoured; a negative value is a typo that would + // silently disable pacing instead. ValidateBasic rejects it wherever it runs, + // but an already-deployed config never reaches ValidateBasic, so refuse it here + // too rather than letting the two paths disagree about the same input. if interval < 0 { - logger.Warn("negative p2p interval in config; using default instead", - "key", key, "configured", interval, "default", def) - interval = def + return 0, fmt.Errorf("p2p %v must not be negative, got %v", key, interval) } - return rate.Every(interval) + return rate.Every(interval), nil } // p2pRouterOptions returns the router's connection budget and pacing, derived // from the p2p config. -func p2pRouterOptions(cfg *config.Config, ep p2p.Endpoint, privatePeerIDs []types.NodeID) *p2p.RouterOptions { +func p2pRouterOptions(cfg *config.Config, ep p2p.Endpoint, privatePeerIDs []types.NodeID) (*p2p.RouterOptions, error) { // MaxConnections defaults to 64 maxConns := 64 if cfg.P2P.MaxConnections > 0 { @@ -489,17 +486,24 @@ func p2pRouterOptions(cfg *config.Config, ep p2p.Endpoint, privatePeerIDs []type // TODO(gprusak): eventually we should migrate configs to specify // MaxInbound and MaxOutbound explicitly, rather than doing the computation above. maxInbound := maxConns - maxOutbound - defaults := config.DefaultP2PConfig() connection := conn.DefaultMConnConfig() connection.FlushThrottle = cfg.P2P.FlushThrottleTimeout connection.SendRate = cfg.P2P.SendRate connection.RecvRate = cfg.P2P.RecvRate connection.MaxPacketMsgPayloadSize = cfg.P2P.MaxPacketMsgPayloadSize + dialRate, err := pacingRate("dial-interval", cfg.P2P.DialInterval) + if err != nil { + return nil, err + } + acceptRate, err := pacingRate("accept-interval", cfg.P2P.AcceptInterval) + if err != nil { + return nil, err + } return &p2p.RouterOptions{ Endpoint: ep, MaxIncomingConnectionAttempts: utils.Some(cfg.P2P.MaxIncomingConnectionAttempts), - MaxDialRate: utils.Some(pacingRate("dial-interval", cfg.P2P.DialInterval, defaults.DialInterval)), - MaxAcceptRate: utils.Some(pacingRate("accept-interval", cfg.P2P.AcceptInterval, defaults.AcceptInterval)), + MaxDialRate: utils.Some(dialRate), + MaxAcceptRate: utils.Some(acceptRate), HandshakeTimeout: utils.Some(cfg.P2P.HandshakeTimeout), DialTimeout: utils.Some(cfg.P2P.DialTimeout), PexOnHandshake: cfg.P2P.PexReactor, @@ -508,7 +512,7 @@ func p2pRouterOptions(cfg *config.Config, ep p2p.Endpoint, privatePeerIDs []type MaxOutbound: utils.Some(maxOutbound), MaxConcurrentAccepts: utils.Some(maxInbound), Connection: connection, - } + }, nil } func createRouter( @@ -532,7 +536,10 @@ func createRouter( privatePeerIDs = append(privatePeerIDs, types.NodeID(id)) } - options := p2pRouterOptions(cfg, ep, privatePeerIDs) + options, err := p2pRouterOptions(cfg, ep, privatePeerIDs) + if err != nil { + return nil, closer, noneDB, err + } if addr := cfg.P2P.ExternalAddress; addr != "" { nodeAddr, err := p2p.ParseNodeAddress(nodeKey.ID().AddressString(addr)) if err != nil { diff --git a/sei-tendermint/node/setup_test.go b/sei-tendermint/node/setup_test.go index b257799695..1dd5120c77 100644 --- a/sei-tendermint/node/setup_test.go +++ b/sei-tendermint/node/setup_test.go @@ -356,37 +356,42 @@ func TestP2PRouterOptions_PacingAndBudgetWiring(t *testing.T) { t.Run("defaults reach the router", func(t *testing.T) { cfg := config.DefaultConfig() - opts := p2pRouterOptions(cfg, ep, nil) + opts, err := p2pRouterOptions(cfg, ep, nil) + require.NoError(t, err) require.Equal(t, utils.Some(rate.Every(cfg.P2P.AcceptInterval)), opts.MaxAcceptRate) require.Equal(t, utils.Some(rate.Every(cfg.P2P.DialInterval)), opts.MaxDialRate) }) - // A negative value never reaches ValidateBasic on an already-deployed node, - // so it must not read as "disable" the way rate.Every would treat it. - t.Run("negative interval falls back to the default", func(t *testing.T) { - cfg := config.DefaultConfig() - cfg.P2P.AcceptInterval = -1 * time.Second - cfg.P2P.DialInterval = -1 * time.Second - opts := p2pRouterOptions(cfg, ep, nil) - - defaults := config.DefaultP2PConfig() - require.Equal(t, utils.Some(rate.Every(defaults.AcceptInterval)), opts.MaxAcceptRate) - require.Equal(t, utils.Some(rate.Every(defaults.DialInterval)), opts.MaxDialRate) - require.NotEqual(t, utils.Some(rate.Inf), opts.MaxAcceptRate) - }) + // A negative value never reaches ValidateBasic on an already-deployed node, and + // rate.Every would read it as "disable". Refuse it rather than start unpaced. + for _, key := range []string{"accept-interval", "dial-interval"} { + t.Run("negative "+key+" refuses to build options", func(t *testing.T) { + cfg := config.DefaultConfig() + switch key { + case "accept-interval": + cfg.P2P.AcceptInterval = -1 * time.Second + case "dial-interval": + cfg.P2P.DialInterval = -1 * time.Second + } + _, err := p2pRouterOptions(cfg, ep, nil) + require.Error(t, err) + }) + } t.Run("zero interval disables the limiter", func(t *testing.T) { cfg := config.DefaultConfig() cfg.P2P.AcceptInterval = 0 - opts := p2pRouterOptions(cfg, ep, nil) + opts, err := p2pRouterOptions(cfg, ep, nil) + require.NoError(t, err) require.Equal(t, utils.Some(rate.Inf), opts.MaxAcceptRate) }) t.Run("operator value flows through", func(t *testing.T) { cfg := config.DefaultConfig() cfg.P2P.AcceptInterval = 250 * time.Millisecond - opts := p2pRouterOptions(cfg, ep, nil) + opts, err := p2pRouterOptions(cfg, ep, nil) + require.NoError(t, err) require.Equal(t, utils.Some(rate.Every(250*time.Millisecond)), opts.MaxAcceptRate) }) @@ -402,7 +407,8 @@ func TestP2PRouterOptions_PacingAndBudgetWiring(t *testing.T) { t.Run(fmt.Sprintf("budget derives from max-connections=%d", tc.maxConns), func(t *testing.T) { cfg := config.DefaultConfig() cfg.P2P.MaxConnections = uint(tc.maxConns) - opts := p2pRouterOptions(cfg, ep, nil) + opts, err := p2pRouterOptions(cfg, ep, nil) + require.NoError(t, err) require.Equal(t, utils.Some(tc.wantInbound), opts.MaxInbound) require.Equal(t, utils.Some(tc.wantOutbound), opts.MaxOutbound)