Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
15 changes: 15 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,12 @@ 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 {
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
21 changes: 21 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,25 @@ 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
// visible in the diff.
func TestP2PConfigAcceptInterval(t *testing.T) {
cfg := DefaultP2PConfig()
require.NoError(t, cfg.ValidateBasic())

require.Equal(t, 10*time.Millisecond, cfg.AcceptInterval)

// 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))
}

// --- 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())
}
9 changes: 9 additions & 0 deletions sei-tendermint/config/toml.go
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,15 @@ 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. 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 }}"
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
8 changes: 6 additions & 2 deletions sei-tendermint/internal/p2p/routeroptions.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +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.
// 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.
Expand Down Expand Up @@ -165,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))
Comment thread
bdchatham marked this conversation as resolved.
Outdated
Comment thread
bdchatham marked this conversation as resolved.
Outdated
Comment thread
bdchatham marked this conversation as resolved.
Outdated
Comment thread
bdchatham marked this conversation as resolved.
Outdated
Comment thread
bdchatham marked this conversation as resolved.
Outdated
Comment thread
bdchatham marked this conversation as resolved.
Outdated
Comment thread
bdchatham marked this conversation as resolved.
Outdated
Comment thread
bdchatham marked this conversation as resolved.
Outdated
Comment thread
bdchatham marked this conversation as resolved.
Outdated
Comment thread
bdchatham marked this conversation as resolved.
Outdated
Comment thread
bdchatham marked this conversation as resolved.
Outdated
Comment thread
bdchatham marked this conversation as resolved.
Outdated
Comment thread
bdchatham marked this conversation as resolved.
Outdated
Comment thread
bdchatham marked this conversation as resolved.
Outdated
Comment thread
bdchatham marked this conversation as resolved.
Outdated
Comment thread
bdchatham marked this conversation as resolved.
Outdated
Comment thread
bdchatham marked this conversation as resolved.
Outdated
Comment thread
bdchatham marked this conversation as resolved.
Outdated
}

func (o *RouterOptions) incomingConnectionWindow() time.Duration {
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
51 changes: 51 additions & 0 deletions sei-tendermint/node/setup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package node

import (
"encoding/json"
"fmt"
"net/url"
"os"
"path/filepath"
Expand All @@ -10,6 +11,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 +347,52 @@ 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://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)
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))
})

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
})

// 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))
})
}
}
Loading