diff --git a/cmd/lumera/cmd/config.go b/cmd/lumera/cmd/config.go index 4f7d10c0..7971c2a5 100644 --- a/cmd/lumera/cmd/config.go +++ b/cmd/lumera/cmd/config.go @@ -72,6 +72,38 @@ func initCometBFTConfig() *cmtcfg.Config { // cfg.P2P.MaxNumInboundPeers = 100 // cfg.P2P.MaxNumOutboundPeers = 40 + // RPC hardening — defense-in-depth against misbehaving WebSocket clients. + // + // Default CometBFT behaviour keeps a WebSocket connection open even when the + // client is not draining its subscription buffer, relying on the OS / peer to + // eventually close the socket. In practice this can take hours, and a buggy + // client (or one with a goroutine/socket leak — see sdk-go PR #17) can + // accumulate thousands of dormant subscriptions on `:26657` and saturate the + // listen backlog of a validator's RPC. + // + // Observed on lumera-devnet-1 val3: ~5,000 ESTABLISHED WS connections, listen + // queue full (Recv-Q=4097/4096), external RPC port unresponsive — while the + // chain itself kept producing blocks. Root cause was a client-side leak in + // sdk-go's wait-tx subscriber (fixed in sdk-go PR #17), but the server had no + // circuit breaker: it accepted, kept, and buffered for every slow subscriber + // indefinitely. + // + // Setting CloseOnSlowClient=true tells CometBFT to forcibly close any WS + // subscriber that cannot keep up with its subscription buffer. This is the + // upstream-recommended defense against the failure mode and protects mainnet + // validators from any third-party client (indexer, relayer, custom tooling) + // that exhibits the same pattern — sdk-go-based or otherwise. + // + // Trade-off: well-behaved clients that subscribe to a high-volume query and + // briefly stall (network blip, GC pause >subscription_buffer_size events) + // will be disconnected and must reconnect. They already had to handle + // reconnect for normal operational reasons (validator restart, network), so + // this is not a new client-side requirement. + // + // Subscription caps are left at CometBFT defaults (100 clients × 5 subs each), + // which are appropriate; no live evidence justifies changing them. + cfg.RPC.CloseOnSlowClient = true + return cfg } diff --git a/cmd/lumera/cmd/config_test.go b/cmd/lumera/cmd/config_test.go index b5e1812a..5df972dd 100644 --- a/cmd/lumera/cmd/config_test.go +++ b/cmd/lumera/cmd/config_test.go @@ -53,3 +53,30 @@ func TestInitAppConfigEVMDefaults(t *testing.T) { require.True(t, evmMempoolCfg.IsValid(), "Lumera.EVMMempool field not found") require.False(t, evmMempoolCfg.FieldByName("BroadcastDebug").Bool(), "broadcast debug must be disabled by default") } + +// TestInitCometBFTConfigRPCHardening locks in the RPC defense-in-depth +// overrides applied in initCometBFTConfig. These protect lumerad's RPC +// (port :26657) from misbehaving WebSocket clients accumulating dormant +// subscriptions (see sdk-go PR #17 / lumera-devnet-1 val3 incident, where +// ~5,000 ESTABLISHED conns saturated the listen backlog while the chain +// itself stayed healthy). Changing these defaults should require an +// explicit decision recorded against this test. +func TestInitCometBFTConfigRPCHardening(t *testing.T) { + t.Parallel() + + cfg := initCometBFTConfig() + + require.NotNil(t, cfg, "initCometBFTConfig must return a config") + require.NotNil(t, cfg.RPC, "RPC config must not be nil") + + require.True(t, cfg.RPC.CloseOnSlowClient, + "experimental_close_on_slow_client must default to true to prevent slow WS clients from holding subscription buffers indefinitely") + + // Subscription caps are upstream CometBFT defaults — we intentionally do + // not narrow them; this assertion guards against accidentally loosening + // them, which would re-open the saturation attack surface. + require.Equal(t, 100, cfg.RPC.MaxSubscriptionClients, + "max_subscription_clients must stay at the upstream default; relaxing it re-opens the val3-style saturation surface") + require.Equal(t, 5, cfg.RPC.MaxSubscriptionsPerClient, + "max_subscriptions_per_client must stay at the upstream default") +} diff --git a/tests/systemtests/system.go b/tests/systemtests/system.go index cc1413cf..b4486139 100644 --- a/tests/systemtests/system.go +++ b/tests/systemtests/system.go @@ -68,8 +68,8 @@ type SystemUnderTest struct { verbose bool ChainStarted bool projectName string - dirty bool // requires full reset when marked dirty - stopping bool // true while StopChain is running (suppresses exit errors) + dirty bool // requires full reset when marked dirty + stopping atomic.Bool // true while StopChain is running (suppresses expected exit errors) pidsLock mtxSync.RWMutex pids map[int]struct{} @@ -328,7 +328,7 @@ func (s *SystemUnderTest) StopChain() { if !s.ChainStarted { return } - s.stopping = true + s.stopping.Store(true) // Pre-cleanup: unsubscribe from events while nodes are still alive for _, c := range s.cleanupPreFn { @@ -374,7 +374,7 @@ func (s *SystemUnderTest) StopChain() { s.cleanupPostFn = nil s.ChainStarted = false - s.stopping = false + s.stopping.Store(false) } func (s *SystemUnderTest) withEachPid(cb func(p *os.Process)) { @@ -636,7 +636,7 @@ func (s *SystemUnderTest) startNodesAsync(t *testing.T, xargs ...string) { go func(pid int, cmd *exec.Cmd, nodeIndex int) { defer wg.Done() err := cmd.Wait() // blocks until shutdown - if err != nil { + if err != nil && !s.stopping.Load() { s.PrintBuffer() errChan <- fmt.Errorf("node %d exited unexpectedly: %w", nodeIndex, err) } else { @@ -653,13 +653,13 @@ func (s *SystemUnderTest) startNodesAsync(t *testing.T, xargs ...string) { // Wait in background and close the channel when all nodes are done go func() { wg.Wait() + close(errChan) for err := range errChan { - if err != nil && !s.stopping { + if err != nil { t.Errorf("%v", err) } } - close(errChan) }() }