Skip to content

feat(seed): bound concurrent connections per source IP - #45

Merged
bdchatham merged 1 commit into
mainfrom
feat/seed-bound-per-source-ip-connections
Jul 28, 2026
Merged

feat(seed): bound concurrent connections per source IP#45
bdchatham merged 1 commit into
mainfrom
feat/seed-bound-per-source-ip-connections

Conversation

@bdchatham

Copy link
Copy Markdown
Collaborator

Problem

max_incoming_connection_attempts caps concurrent connections per source IP (internal/p2p/conn_tracker.go:33-38), not attempt rate as the name suggests. Upstream sei-tendermint sets it equal to max_connections — 100/100 — where it can never bind.

applySeedOverrides raises max_connections to 1000 (≈980 inbound after the outbound reserve), which makes the per-IP cap the first real constraint on a seed. It was still carrying the inherited 100, letting a single source hold ~10% of the seed's inbound capacity, and requiring only ~10 addresses to fill it.

AllowDuplicateIP = true looks like it covers this, but sei-tendermint declares the field, defaults it, and renders it into config.toml without ever reading it. It documents intent; it provides nothing.

Change

  • MaxIncomingConnectionAttempts = 32 for seed mode. One source gets ~3% of inbound — ample for an operator running several nodes behind one NAT, while forcing a flood across ~31 addresses.
  • Comment recording that AllowDuplicateIP is inert, pointing at the knob that actually applies.
  • TestDefaultForMode_SeedBoundsPerSourceIPConnections pins the value and the relationship (the cap must sit below max_connections, or it constrains nothing). Both assertions verified to fail under mutation.

Sequencing — read before merging

The cap multiplies against distinct source addresses as seid observes them. Behind a SNAT'ing load balancer that is the balancer's ENI count, not the peer count, so lowering this value reduces total inbound capacity instead of limiting abuse.

arctic-1's seed-0 is in exactly that state today: its NLB is ip-target with preserve_client_ip.enabled=false, and all 34 of its inbound connections present one of 3 ENI addresses. At 32 its ceiling would be 3 × 32 = 96. sei-protocol/platform#1440 must land and be verified first. seed-1/seed-2 are instance-target, already preserve client IPs, and see 11 distinct peer addresses.

This is inert until a seed rolls — config reaches a running SeiNode only through an update plan, which fires on image or sidecar drift.

Verification

make test / make vet clean. make lint reports 47 issues, all pre-existing and identical on the unmodified baseline; none in defaults.go.

Open item

The value is the judgment call here. 32 comes from our own observed pattern (~7 nodes per NAT address per cell) with 4–5× headroom, not from measured external data — and nothing today can distinguish "cap working" from "cap rejecting legitimate peers", since the rejection path at router.go:212 emits no metric and seed mode serves no Prometheus listener on the deployed build. Worth revisiting once that counter exists.

🤖 Generated with Claude Code

max_incoming_connection_attempts caps CONCURRENT connections per source IP
(conn_tracker.go), not attempt rate as the name suggests. Upstream leaves it
equal to max_connections at 100/100, where it can never bind. The seed profile
raises the total to 1000, which makes the per-IP cap the first real constraint —
and it was still carrying the inherited 100, letting one source hold ~10% of a
seed's inbound capacity.

Set it to 32: ample for an operator running several nodes behind one NAT, while
forcing a flood to spread across ~31 addresses.

This is only meaningful where the load balancer preserves client IPs. Under SNAT
the cap multiplies against the balancer's address count rather than the peers',
so a lower value shrinks total inbound capacity instead of limiting abuse.

Also records that AllowDuplicateIP is inert — sei-tendermint declares and
renders it but never reads it, so it documents intent rather than providing the
protection its name implies.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@cursor

cursor Bot commented Jul 28, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes public seed P2P admission defaults and can reject or cap peers per observed source IP; effectiveness depends on whether client IPs are preserved through load balancers (SNAT can shrink effective capacity).

Overview
Seed mode now sets MaxIncomingConnectionAttempts to 32 in applySeedOverrides, so the per–source-IP concurrent connection limit is meaningful once max_connections is raised to 1000 (baseline still leaves both at 100, so the cap never bites).

Comments clarify that AllowDuplicateIP is only written to config.toml and is not enforced by sei-tendermint, and that the misnamed max_incoming_connection_attempts field is what actually limits concurrent inbound connections per IP.

A new test TestDefaultForMode_SeedBoundsPerSourceIPConnections asserts the value is 32 and stays below max_connections; the existing seed test message for duplicate IPs is updated to match that behavior.

Reviewed by Cursor Bugbot for commit 103d634. Bugbot is set up for automated code reviews on this repo. Configure here.

@bdchatham
bdchatham merged commit ef3be69 into main Jul 28, 2026
3 checks passed
bdchatham added a commit that referenced this pull request Jul 28, 2026
* chore: release v0.0.25 (seed per-source-IP connection cap)

Carries #45: applySeedOverrides sets max_incoming_connection_attempts to 32.
That key caps CONCURRENT connections per source IP rather than attempt rate,
and upstream leaves it equal to max_connections (100/100) where it can never
bind — so raising the seed total to 1000 had left one source able to hold ~10%
of a seed's inbound. Also records that AllowDuplicateIP is inert: sei-tendermint
declares and renders it but never reads it.

Consumed next by seictl, whose sidecar renders config.toml
(sidecar/tasks/config_apply.go) and is therefore what makes this real on a
running node, and by sei-k8s-controller to keep its pin in sync.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(defaults): state consequences in present tense per xreview

Comment-only; no behavior change.

The giga comment described a prior tree state ("previously rendered"), which a
reader cannot verify against the code in front of them. Restated as present fact,
keeping the pin instruction for nodes that must stay on the v2 executor.

The seed cap comment gave the mechanism ("multiplies against the LB's address
count") without the consequence, so it read as benign re-accounting: under SNAT
the ceiling collapses and legitimate peers are dropped, silently, because seed
mode serves no metrics. It now leads with the silent-rejection property, says
outright that this tightens 100 to 32, carries the measured basis for 32 (a fleet
maximum of 8 real peers per address) so the number stays checkable, and records
that the accept path is NodeID-blind and so bounds inbound persistent and
unconditional peers too.

Raised independently by idiomatic-reviewer and prose-steward while blinded.
Ledger: bdchatham-designs designs/seed-node-mode/xreview/sei-config-v0.0.25-rollout.md

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant