Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
edff731
fix(p2p): make the inbound accept rate configurable and raise its def…
bdchatham Aug 11, 2026
cb0338f
test(config): pin accept-interval under the hidden-knob convention
bdchatham Aug 11, 2026
c704f4d
address review: render accept-interval, pin the default exactly, test…
bdchatham Aug 11, 2026
0ccc3ff
address review round 2: fix the default at the choke point, tighten t…
bdchatham Aug 11, 2026
b816df2
address review round 3: correct the connTracker rationale, close the …
bdchatham Aug 11, 2026
be9ad90
address review round 4: route [p2p] through Config.ValidateBasic
bdchatham Aug 11, 2026
a9607c5
Merge branch 'main' into fix/p2p-accept-rate-configurable
bdchatham Aug 11, 2026
e56a479
address review round 5: collapse the duplicated accept default to one…
bdchatham Aug 12, 2026
0c95de0
conform godocs to the AGENTS.md Godoc rules added on main
bdchatham Aug 12, 2026
a4b857e
address human review: drop the cross-package default coupling and the…
bdchatham Aug 12, 2026
371b1ee
address review: clamp negative pacing intervals, pin the package defa…
bdchatham Aug 12, 2026
d2b60ee
address review: stop documenting the clamp, and stop applying it sile…
bdchatham Aug 12, 2026
c2b0acf
address review: refuse a negative pacing interval instead of clamping it
bdchatham Aug 13, 2026
8f5a204
Merge branch 'main' into fix/p2p-accept-rate-configurable
bdchatham Aug 13, 2026
289b6db
Merge branch 'main' into fix/p2p-accept-rate-configurable
bdchatham Aug 13, 2026
6830f19
Merge branch 'main' into fix/p2p-accept-rate-configurable
bdchatham Aug 14, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions sei-tendermint/config/config.go
Comment thread
bdchatham marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -724,6 +724,14 @@ 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, and the
// per-source attempt rate by max-incoming-connection-attempts. A value of 0
Comment thread
bdchatham marked this conversation as resolved.
Outdated
// disables the limiter.
AcceptInterval time.Duration `mapstructure:"accept-interval"`
Comment thread
bdchatham marked this conversation as resolved.
Comment thread
bdchatham marked this conversation as resolved.

// Testing params.
// Force dial to fail
TestDialFail bool `mapstructure:"test-dial-fail"`
Expand Down Expand Up @@ -754,6 +762,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",
}
Expand All @@ -774,6 +783,9 @@ func (cfg *P2PConfig) ValidateBasic() error {
if cfg.RecvRate < 0 {
return errors.New("recv-rate can't be negative")
}
if cfg.AcceptInterval < 0 {
Comment thread
bdchatham marked this conversation as resolved.
return errors.New("accept-interval can't be negative")
Comment thread
bdchatham marked this conversation as resolved.
}
return nil
}

Expand Down
28 changes: 28 additions & 0 deletions sei-tendermint/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -230,6 +231,7 @@ func TestP2PConfigValidateBasic(t *testing.T) {
"MaxPacketMsgPayloadSize",
"SendRate",
"RecvRate",
"AcceptInterval",
Comment thread
bdchatham marked this conversation as resolved.
}

for _, fieldName := range fieldsToTest {
Expand All @@ -239,6 +241,32 @@ 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())

// 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,
Comment thread
bdchatham marked this conversation as resolved.
Outdated
Comment thread
bdchatham marked this conversation as resolved.
Outdated
"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) {
Expand Down
75 changes: 75 additions & 0 deletions sei-tendermint/config/p2p_compat_test.go
Original file line number Diff line number Diff line change
@@ -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
Comment thread
bdchatham marked this conversation as resolved.
Outdated
// (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())
Comment thread
bdchatham marked this conversation as resolved.
Outdated
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.
Comment thread
bdchatham marked this conversation as resolved.
Outdated
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())
}
8 changes: 8 additions & 0 deletions sei-tendermint/config/toml.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment thread
bdchatham marked this conversation as resolved.
Outdated
# 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
Comment thread
bdchatham marked this conversation as resolved.
Outdated
# 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 }}"
Comment thread
bdchatham marked this conversation as resolved.
Comment thread
bdchatham marked this conversation as resolved.

# Time to wait before flushing messages out on the connection
# TODO: Remove once MConnConnection is removed.
flush-throttle-timeout = "{{ .P2P.FlushThrottleTimeout }}"
Expand Down
19 changes: 19 additions & 0 deletions sei-tendermint/config/toml_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,25 @@ func checkConfig(t *testing.T, configFile string) {
t.Errorf("config file was not expected to contain %s", e)
}
}

// 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",
}
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 {
Expand Down
2 changes: 2 additions & 0 deletions sei-tendermint/internal/p2p/routeroptions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
54 changes: 32 additions & 22 deletions sei-tendermint/node/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment thread
bdchatham marked this conversation as resolved.
Outdated
// 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
Comment thread
bdchatham marked this conversation as resolved.
Outdated
// 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 {
Expand All @@ -496,10 +481,11 @@ 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)),
MaxAcceptRate: utils.Some(rate.Every(cfg.P2P.AcceptInterval)),
Comment thread
bdchatham marked this conversation as resolved.
Outdated
HandshakeTimeout: utils.Some(cfg.P2P.HandshakeTimeout),
DialTimeout: utils.Some(cfg.P2P.DialTimeout),
PexOnHandshake: cfg.P2P.PexReactor,
Expand All @@ -509,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 {
Expand Down
44 changes: 44 additions & 0 deletions sei-tendermint/node/setup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Comment thread
bdchatham marked this conversation as resolved.
Outdated
Comment thread
bdchatham marked this conversation as resolved.
Outdated
// 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.
Comment thread
bdchatham marked this conversation as resolved.
Outdated
func TestP2PRouterOptions_PacingAndBudgetWiring(t *testing.T) {
ep, err := p2p.ResolveEndpoint("tcp://" + string(types.NodeID("0000000000000000000000000000000000000000")) + "@127.0.0.1:26656")
Comment thread
bdchatham marked this conversation as resolved.
Outdated
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)
Comment thread
bdchatham marked this conversation as resolved.
Outdated
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)))
Comment thread
bdchatham marked this conversation as resolved.
Outdated
})

t.Run("concurrent accepts track the inbound pool, not max-connections", func(t *testing.T) {
cfg := config.DefaultConfig()
cfg.P2P.MaxConnections = 100
Comment thread
bdchatham marked this conversation as resolved.
Outdated
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))
})
}
Loading