From 4c49b2b5e0e4a35c4160284902e072e18999b7ac Mon Sep 17 00:00:00 2001 From: monty-sei Date: Mon, 10 Aug 2026 10:51:03 +1000 Subject: [PATCH 1/8] feat(seeds): ship Sei Labs seeds as the default bootstrap-peers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A fresh `seid init --chain-id pacific-1` (or atlantic-2) now writes a config.toml with bootstrap-peers already populated, so a node bootstraps peer discovery with no operator configuration. Previously the field defaulted to "" and operators had to source a peer list out of band. Adds app/seeds, mirroring the existing app/genesis pattern for well-known chain data, and consults it in InitCmd after the chain-id is resolved. - An operator-supplied bootstrap-peers always wins; we only fill an empty value. An unrecognised chain-id is a no-op, so private and local chains are unaffected. - Seeds go in bootstrap-peers rather than persistent-peers: they seed the address book via PEX and may then be dropped, and pinning operators to long-lived connections against them is wrong for them and a load multiplier for us. - arctic-1 is deliberately excluded. It is a devnet with no Cosmos chain-registry entry, and a devnet is the most likely network to be reset or re-keyed — the worst case for an address baked permanently into a release. Also corrects the --chain-id flag help, which claimed "if left blank will use sei" while the code panics on an empty value. Verified end to end: init on pacific-1 and atlantic-2 yields three seeds each, arctic-1 and unknown chain-ids yield "". --- app/seeds/seeds.go | 71 ++++++++++++++++++++++++++++++ app/seeds/seeds_test.go | 95 +++++++++++++++++++++++++++++++++++++++++ cmd/seid/cmd/init.go | 12 +++++- 3 files changed, 177 insertions(+), 1 deletion(-) create mode 100644 app/seeds/seeds.go create mode 100644 app/seeds/seeds_test.go diff --git a/app/seeds/seeds.go b/app/seeds/seeds.go new file mode 100644 index 0000000000..dc233aad05 --- /dev/null +++ b/app/seeds/seeds.go @@ -0,0 +1,71 @@ +// Package seeds ships the Sei Labs operated P2P seed nodes for the public Sei +// networks, so a freshly initialised node bootstraps peer discovery with no +// operator configuration. +// +// Seeds are dialled to populate the address book via the PEX reactor and may +// then be dropped, which is why they belong in `bootstrap-peers` rather than +// `persistent-peers` — an operator should not hold connections open against +// them indefinitely. +package seeds + +import "strings" + +// chainSeeds maps a well-known chain-id to its Sei Labs seed nodes, each in +// CometBFT's `NodeID@host:port` form. Three per network, one per cell +// (eu-central-1, eu-west-1, us-east-2), so losing a region does not cost +// bootstrap capability. +// +// PERMANENCE: these strings ship inside released binaries and operators pin +// them. The secret-connection handshake verifies the NodeID, so a changed ID +// is a rejected dial, not a degraded one — and a release already in the wild +// cannot be recalled. Retiring an address therefore means keeping it dialable +// until every release carrying it is out of use. Treat edits here as one-way. +// +// arctic-1 is deliberately absent. It is a devnet: it has no Cosmos +// chain-registry entry, so it is not an operator-facing network, and a devnet +// is the most likely to be reset or re-keyed — exactly the case where baking a +// permanent address into a binary is wrong. Devnet users set bootstrap-peers +// explicitly. +// +// Source of truth: clusters///seeds/seed-N/seed-N.yaml in +// sei-protocol/platform (the SeiNode's externalAddress plus its NodeID). +var chainSeeds = map[string][]string{ + "pacific-1": { + "0cd5f57c249b5aca815710338e1fe7a14797585d@seed-0-p2p.pacific-1.prod.platform.sei.io:26656", + "f0f057f1593d28bec11591cf146bd223e0be1866@seed-1-p2p.pacific-1.prod-euw1.platform.sei.io:26656", + "8e28f62368a1ceae0102645db8584b218650930d@seed-2-p2p.pacific-1.prod-use2.platform.sei.io:26656", + }, + "atlantic-2": { + "362f934ead3654fca9cafdac63b52b47b2f9a95e@seed-0-p2p.atlantic-2.prod.platform.sei.io:26656", + "1f55cd51183d3a6cad8a3667b91d08d0338bd52e@seed-1-p2p.atlantic-2.prod-euw1.platform.sei.io:26656", + "7152be2e4c1a057d2b2467723058c5f0ec790472@seed-2-p2p.atlantic-2.prod-use2.platform.sei.io:26656", + }, +} + +// ForChain returns the Sei Labs seed addresses for a well-known chain, or nil +// when the chain-id is not recognised (private and local chains included). +// The returned slice is a copy; callers may not mutate package state. +func ForChain(chainID string) []string { + s, ok := chainSeeds[chainID] + if !ok { + return nil + } + out := make([]string, len(s)) + copy(out, s) + return out +} + +// BootstrapPeers returns the seeds for a chain as the comma-separated value +// CometBFT's `bootstrap-peers` expects, or "" when the chain is unrecognised. +func BootstrapPeers(chainID string) string { + return strings.Join(ForChain(chainID), ",") +} + +// Chains returns the chain-ids that ship with seeds. Order is unspecified. +func Chains() []string { + out := make([]string, 0, len(chainSeeds)) + for id := range chainSeeds { + out = append(out, id) + } + return out +} diff --git a/app/seeds/seeds_test.go b/app/seeds/seeds_test.go new file mode 100644 index 0000000000..2ba36696a5 --- /dev/null +++ b/app/seeds/seeds_test.go @@ -0,0 +1,95 @@ +package seeds + +import ( + "regexp" + "strings" + "testing" + + "github.com/sei-protocol/sei-chain/app/genesis" +) + +// addrRe matches CometBFT's `NodeID@host:port`, where NodeID is the 20-byte +// address as 40 lowercase hex characters. +var addrRe = regexp.MustCompile(`^[0-9a-f]{40}@[a-zA-Z0-9.-]+:[0-9]{1,5}$`) + +// A malformed NodeID is rejected at the secret-connection handshake, so the +// seed silently never connects — no error, no metric (seed mode serves none). +// This is the test that stops that shipping. +func TestSeedAddressesAreWellFormed(t *testing.T) { + for chainID, addrs := range chainSeeds { + if len(addrs) == 0 { + t.Errorf("%s: no seeds configured", chainID) + } + seen := make(map[string]bool, len(addrs)) + for _, a := range addrs { + if !addrRe.MatchString(a) { + t.Errorf("%s: malformed seed address %q (want NodeID@host:port)", chainID, a) + continue + } + id, hostPort, _ := strings.Cut(a, "@") + if seen[id] { + t.Errorf("%s: duplicate NodeID %s", chainID, id) + } + seen[id] = true + if !strings.HasSuffix(hostPort, ":26656") { + t.Errorf("%s: seed %q does not use the default P2P port 26656", chainID, a) + } + } + } +} + +// Every chain we ship seeds for must also be a chain seid can initialise, or +// the entry is a typo that would silently never apply. The converse is not +// asserted: arctic-1 is intentionally well-known for genesis but has no seeds. +func TestSeedChainsAreWellKnown(t *testing.T) { + for _, chainID := range Chains() { + if !genesis.IsWellKnown(chainID) { + t.Errorf("chain %q has seeds but is not a well-known chain (typo?)", chainID) + } + } +} + +func TestArcticIsDeliberatelyExcluded(t *testing.T) { + if got := ForChain("arctic-1"); got != nil { + t.Fatalf("arctic-1 is a devnet and must not ship seeds, got %v", got) + } + // Guard the premise of the exclusion: arctic-1 is still initialisable. + if !genesis.IsWellKnown("arctic-1") { + t.Error("arctic-1 is expected to remain a well-known chain for genesis") + } +} + +func TestForChain(t *testing.T) { + for _, chainID := range []string{"pacific-1", "atlantic-2"} { + if got := ForChain(chainID); len(got) != 3 { + t.Errorf("%s: expected 3 seeds, got %d", chainID, len(got)) + } + } + for _, unknown := range []string{"", "unknown-1", "Pacific-1", "pacific-1 "} { + if got := ForChain(unknown); got != nil { + t.Errorf("chain %q: expected nil (exact match only), got %v", unknown, got) + } + } +} + +// The returned slice must not alias package state. +func TestForChainReturnsCopy(t *testing.T) { + first := ForChain("pacific-1") + first[0] = "tampered" + if second := ForChain("pacific-1"); second[0] == "tampered" { + t.Fatal("ForChain leaked the package-level slice to callers") + } +} + +func TestBootstrapPeers(t *testing.T) { + got := BootstrapPeers("pacific-1") + if n := len(strings.Split(got, ",")); n != 3 { + t.Errorf("expected 3 comma-separated entries, got %d (%q)", n, got) + } + if strings.Contains(got, " ") { + t.Errorf("bootstrap-peers must not contain spaces: %q", got) + } + if BootstrapPeers("unknown-1") != "" { + t.Error("unknown chain must yield an empty bootstrap-peers value") + } +} diff --git a/cmd/seid/cmd/init.go b/cmd/seid/cmd/init.go index a568425513..f9c2a3175d 100644 --- a/cmd/seid/cmd/init.go +++ b/cmd/seid/cmd/init.go @@ -11,6 +11,7 @@ import ( "github.com/pkg/errors" "github.com/sei-protocol/sei-chain/app/genesis" "github.com/sei-protocol/sei-chain/app/params" + "github.com/sei-protocol/sei-chain/app/seeds" tmcfg "github.com/sei-protocol/sei-chain/sei-tendermint/config" "github.com/sei-protocol/sei-chain/sei-tendermint/libs/cli" tmos "github.com/sei-protocol/sei-chain/sei-tendermint/libs/os" @@ -119,6 +120,15 @@ For validator or seed nodes, pass --mode validator or --mode seed so RPC and P2P panic("chain-id is required, please set using --chain-id") } + // Public networks ship with the Sei Labs seeds pre-populated so a fresh + // node bootstraps peer discovery with no further configuration. An + // operator-supplied value always wins, and an unrecognised chain-id is a + // no-op. Seeds go in bootstrap-peers, not persistent-peers: they seed the + // address book via PEX and may then be dropped. + if tmConfig.P2P.BootstrapPeers == "" { + tmConfig.P2P.BootstrapPeers = seeds.BootstrapPeers(chainID) + } + // Get bip39 mnemonic var mnemonic string recoverFlag, _ := cmd.Flags().GetBool(FlagRecover) @@ -196,7 +206,7 @@ For validator or seed nodes, pass --mode validator or --mode seed so RPC and P2P cmd.Flags().String(cli.HomeFlag, defaultNodeHome, "node's home directory") cmd.Flags().BoolP(FlagOverwrite, "o", false, "overwrite the genesis.json and existing config files (config.toml, app.toml)") cmd.Flags().Bool(FlagRecover, false, "provide seed phrase to recover existing key instead of creating") - cmd.Flags().String(flags.FlagChainID, "", "genesis file chain-id, if left blank will use sei") + cmd.Flags().String(flags.FlagChainID, "", "chain-id to initialise for (required), e.g. pacific-1 or atlantic-2") cmd.Flags().String(FlagMode, "full", "node mode: validator, full, seed, or archive") return cmd From f75f72e3f48e377627e5a0a6c3d3a361170072a6 Mon Sep 17 00:00:00 2001 From: monty-sei Date: Mon, 10 Aug 2026 11:16:46 +1000 Subject: [PATCH 2/8] =?UTF-8?q?refactor(seeds):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20name=20the=20init=20step,=20cover=20the=20wiring?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extracts the inline bootstrap-peers block into applyDefaultBootstrapPeers, so RunE reads as a sequence of named steps and the rationale lives in a doc comment rather than mid-flow. The extraction is what makes the wiring testable. TestInitCmdWrites- DefaultBootstrapPeers now executes InitCmd against a temp home and asserts the written config.toml; previously the wiring had no coverage at all — deleting the call left the whole suite green. Test hardening in app/seeds: - Hold NodeIDs against types.NodeID.Validate instead of a local regex, so the check cannot drift from CometBFT's definition (the old pattern also accepted a five-digit port). - Check NodeID and host uniqueness across the whole table rather than per chain. The likeliest copy/paste error is a pacific-1 entry pasted into the atlantic-2 block, which a per-chain check could not catch. Drops the exported Chains(), whose only caller was a same-package test that can range over chainSeeds directly. Also documents the actual --overwrite behaviour on the helper: init has no bootstrap-peers flag, so the empty check is defensive rather than an operator-precedence mechanism, and `init --overwrite` replaces a hand-edited value with the seeds (previously with ""). --- app/seeds/seeds.go | 9 -- app/seeds/seeds_test.go | 38 +++++--- cmd/seid/cmd/bootstrap_peers_test.go | 128 +++++++++++++++++++++++++++ cmd/seid/cmd/init.go | 33 +++++-- 4 files changed, 178 insertions(+), 30 deletions(-) create mode 100644 cmd/seid/cmd/bootstrap_peers_test.go diff --git a/app/seeds/seeds.go b/app/seeds/seeds.go index dc233aad05..b4a33dc354 100644 --- a/app/seeds/seeds.go +++ b/app/seeds/seeds.go @@ -60,12 +60,3 @@ func ForChain(chainID string) []string { func BootstrapPeers(chainID string) string { return strings.Join(ForChain(chainID), ",") } - -// Chains returns the chain-ids that ship with seeds. Order is unspecified. -func Chains() []string { - out := make([]string, 0, len(chainSeeds)) - for id := range chainSeeds { - out = append(out, id) - } - return out -} diff --git a/app/seeds/seeds_test.go b/app/seeds/seeds_test.go index 2ba36696a5..8d4eb6115a 100644 --- a/app/seeds/seeds_test.go +++ b/app/seeds/seeds_test.go @@ -1,39 +1,51 @@ package seeds import ( - "regexp" "strings" "testing" "github.com/sei-protocol/sei-chain/app/genesis" + "github.com/sei-protocol/sei-chain/sei-tendermint/types" ) -// addrRe matches CometBFT's `NodeID@host:port`, where NodeID is the 20-byte -// address as 40 lowercase hex characters. -var addrRe = regexp.MustCompile(`^[0-9a-f]{40}@[a-zA-Z0-9.-]+:[0-9]{1,5}$`) - // A malformed NodeID is rejected at the secret-connection handshake, so the // seed silently never connects — no error, no metric (seed mode serves none). // This is the test that stops that shipping. +// +// The ID is held against types.NodeID.Validate rather than a local pattern, so +// this cannot drift from CometBFT's actual definition. Uniqueness is checked +// across the whole table, not per chain: the likeliest copy/paste error is a +// pacific-1 entry pasted into the atlantic-2 block, which a per-chain check +// would miss. func TestSeedAddressesAreWellFormed(t *testing.T) { + seenID := map[string]string{} + seenHost := map[string]string{} + for chainID, addrs := range chainSeeds { if len(addrs) == 0 { t.Errorf("%s: no seeds configured", chainID) } - seen := make(map[string]bool, len(addrs)) for _, a := range addrs { - if !addrRe.MatchString(a) { - t.Errorf("%s: malformed seed address %q (want NodeID@host:port)", chainID, a) + id, hostPort, ok := strings.Cut(a, "@") + if !ok { + t.Errorf("%s: seed %q is not in NodeID@host:port form", chainID, a) continue } - id, hostPort, _ := strings.Cut(a, "@") - if seen[id] { - t.Errorf("%s: duplicate NodeID %s", chainID, id) + if err := types.NodeID(id).Validate(); err != nil { + t.Errorf("%s: seed %q has an invalid NodeID: %v", chainID, a, err) + continue } - seen[id] = true if !strings.HasSuffix(hostPort, ":26656") { t.Errorf("%s: seed %q does not use the default P2P port 26656", chainID, a) } + if prev, dup := seenID[id]; dup { + t.Errorf("NodeID %s appears in both %s and %s", id, prev, chainID) + } + seenID[id] = chainID + if prev, dup := seenHost[hostPort]; dup { + t.Errorf("host %s appears in both %s and %s", hostPort, prev, chainID) + } + seenHost[hostPort] = chainID } } } @@ -42,7 +54,7 @@ func TestSeedAddressesAreWellFormed(t *testing.T) { // the entry is a typo that would silently never apply. The converse is not // asserted: arctic-1 is intentionally well-known for genesis but has no seeds. func TestSeedChainsAreWellKnown(t *testing.T) { - for _, chainID := range Chains() { + for chainID := range chainSeeds { if !genesis.IsWellKnown(chainID) { t.Errorf("chain %q has seeds but is not a well-known chain (typo?)", chainID) } diff --git a/cmd/seid/cmd/bootstrap_peers_test.go b/cmd/seid/cmd/bootstrap_peers_test.go new file mode 100644 index 0000000000..72d3b89ec7 --- /dev/null +++ b/cmd/seid/cmd/bootstrap_peers_test.go @@ -0,0 +1,128 @@ +package cmd + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/sei-protocol/sei-chain/app" + "github.com/sei-protocol/sei-chain/app/seeds" + "github.com/sei-protocol/sei-chain/sei-cosmos/client" + tmcfg "github.com/sei-protocol/sei-chain/sei-tendermint/config" + "github.com/stretchr/testify/require" +) + +// Covers the init-time wiring itself, not just the seed data. Without this, +// removing the applyDefaultBootstrapPeers call from InitCmd leaves every test +// in the tree passing. +func TestApplyDefaultBootstrapPeers(t *testing.T) { + tests := []struct { + name string + chainID string + want string + }{ + {"mainnet gets seeds", "pacific-1", seeds.BootstrapPeers("pacific-1")}, + {"testnet gets seeds", "atlantic-2", seeds.BootstrapPeers("atlantic-2")}, + // arctic-1 is a devnet and deliberately ships no seeds. + {"devnet gets none", "arctic-1", ""}, + {"unknown chain gets none", "my-private-chain", ""}, + {"empty chain-id gets none", "", ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := tmcfg.DefaultConfig() + applyDefaultBootstrapPeers(cfg, tt.chainID) + if cfg.P2P.BootstrapPeers != tt.want { + t.Errorf("bootstrap-peers = %q, want %q", cfg.P2P.BootstrapPeers, tt.want) + } + }) + } +} + +// The public networks must actually get a usable set, not just a non-empty +// string — a silently truncated table would still satisfy the table test above. +func TestApplyDefaultBootstrapPeersPopulatesPublicNetworks(t *testing.T) { + for _, chainID := range []string{"pacific-1", "atlantic-2"} { + cfg := tmcfg.DefaultConfig() + applyDefaultBootstrapPeers(cfg, chainID) + if n := len(strings.Split(cfg.P2P.BootstrapPeers, ",")); n != 3 { + t.Errorf("%s: got %d bootstrap peers, want 3 (%q)", chainID, n, cfg.P2P.BootstrapPeers) + } + } +} + +// A pre-populated value is never overwritten. Not reachable through `seid init` +// today (it has no bootstrap-peers flag), but the guard is the reason this stays +// true for any future caller. +func TestApplyDefaultBootstrapPeersPreservesExistingValue(t *testing.T) { + const existing = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef@peer.example.com:26656" + + cfg := tmcfg.DefaultConfig() + cfg.P2P.BootstrapPeers = existing + applyDefaultBootstrapPeers(cfg, "pacific-1") + + if cfg.P2P.BootstrapPeers != existing { + t.Errorf("existing bootstrap-peers was overwritten: got %q, want %q", cfg.P2P.BootstrapPeers, existing) + } +} + +// runInit executes the real InitCmd against a temp home and returns the written +// config.toml. Testing the helper alone does not cover the call site — without +// this, deleting applyDefaultBootstrapPeers from RunE leaves the suite green. +func runInit(t *testing.T, chainID string) string { + t.Helper() + home := t.TempDir() + // The root command creates the home layout and client.toml before init runs. + // Standing InitCmd up directly, the test owns that scaffolding. + configDir := filepath.Join(home, "config") + require.NoError(t, os.MkdirAll(configDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(configDir, "client.toml"), []byte( + "chain-id = \"\"\nkeyring-backend = \"test\"\noutput = \"text\"\nnode = \"tcp://localhost:26657\"\nbroadcast-mode = \"sync\"\n", + ), 0o644)) + + encCfg := app.MakeEncodingConfig() + clientCtx := client.Context{}.WithCodec(encCfg.Marshaler).WithHomeDir(home).WithViper("") + + cmd := InitCmd(app.ModuleBasics, home) + cmd.SetArgs([]string{"testnode", "--chain-id", chainID, "--home", home}) + cmd.SilenceUsage = true + cmd.SilenceErrors = true + + ctx := context.WithValue(context.Background(), client.ClientContextKey, &clientCtx) + require.NoError(t, cmd.ExecuteContext(ctx)) + + data, err := os.ReadFile(filepath.Join(home, "config", "config.toml")) + require.NoError(t, err) + return string(data) +} + +// bootstrapPeersLine returns the rendered `bootstrap-peers = "..."` value. +func bootstrapPeersLine(t *testing.T, configToml string) string { + t.Helper() + for _, line := range strings.Split(configToml, "\n") { + if after, ok := strings.CutPrefix(strings.TrimSpace(line), "bootstrap-peers = "); ok { + return strings.Trim(after, `"`) + } + } + t.Fatal("config.toml has no bootstrap-peers line") + return "" +} + +// The end-to-end assertion: `seid init` on a public network writes the seeds. +func TestInitCmdWritesDefaultBootstrapPeers(t *testing.T) { + for _, chainID := range []string{"pacific-1", "atlantic-2"} { + t.Run(chainID, func(t *testing.T) { + got := bootstrapPeersLine(t, runInit(t, chainID)) + require.Equal(t, seeds.BootstrapPeers(chainID), got) + require.Len(t, strings.Split(got, ","), 3) + }) + } +} + +// arctic-1 is a devnet and ships no seeds, so init must leave the field empty. +func TestInitCmdLeavesDevnetBootstrapPeersEmpty(t *testing.T) { + require.Empty(t, bootstrapPeersLine(t, runInit(t, "arctic-1"))) +} diff --git a/cmd/seid/cmd/init.go b/cmd/seid/cmd/init.go index f9c2a3175d..a249fe0983 100644 --- a/cmd/seid/cmd/init.go +++ b/cmd/seid/cmd/init.go @@ -120,14 +120,7 @@ For validator or seed nodes, pass --mode validator or --mode seed so RPC and P2P panic("chain-id is required, please set using --chain-id") } - // Public networks ship with the Sei Labs seeds pre-populated so a fresh - // node bootstraps peer discovery with no further configuration. An - // operator-supplied value always wins, and an unrecognised chain-id is a - // no-op. Seeds go in bootstrap-peers, not persistent-peers: they seed the - // address book via PEX and may then be dropped. - if tmConfig.P2P.BootstrapPeers == "" { - tmConfig.P2P.BootstrapPeers = seeds.BootstrapPeers(chainID) - } + applyDefaultBootstrapPeers(tmConfig, chainID) // Get bip39 mnemonic var mnemonic string @@ -212,6 +205,30 @@ For validator or seed nodes, pass --mode validator or --mode seed so RPC and P2P return cmd } +// applyDefaultBootstrapPeers fills bootstrap-peers with the Sei Labs seeds for +// the public networks, so a freshly initialised node bootstraps peer discovery +// with no operator configuration. An unrecognised chain-id leaves the field +// untouched, so private and local chains are unaffected. +// +// Seeds belong in bootstrap-peers rather than persistent-peers: they seed the +// address book via PEX and may then be dropped, whereas persistent-peers holds +// connections open indefinitely. +// +// The empty check is defensive rather than load-bearing today. `seid init` +// builds its config from tmcfg.DefaultConfig() and exposes no flag for +// bootstrap-peers, so the field is always empty here — but the guard keeps the +// behaviour correct if a future caller pre-populates it. Note that +// `init --overwrite` rewrites config.toml wholesale, so an operator's +// hand-edited bootstrap-peers is replaced by the seeds (previously it was +// replaced by ""); without --overwrite, init refuses to touch an existing +// config at all. +func applyDefaultBootstrapPeers(cfg *tmcfg.Config, chainID string) { + if cfg.P2P.BootstrapPeers != "" { + return + } + cfg.P2P.BootstrapPeers = seeds.BootstrapPeers(chainID) +} + func checkConfigOverwrite(configPath string, overwrite bool) error { if overwrite { return nil From ccc9849f0cbe743e9fa89f32bc036f57e98c92ab Mon Sep 17 00:00:00 2001 From: monty-sei Date: Wed, 12 Aug 2026 14:14:00 +1000 Subject: [PATCH 3/8] =?UTF-8?q?refactor(seeds):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20drop=20the=20exported=20helper,=20parse=20with=20p2?= =?UTF-8?q?p?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Collapses app/seeds to the address table plus BootstrapPeers, the only thing anything outside the package uses. ForChain had no caller beyond this package, so exporting it (and defensively copying the slice for callers that did not exist) was API for nobody. With it gone, BootstrapPeers reads straight off the map: a missing chain yields nil, and strings.Join renders that as "". Re-exports ParseNodeAddress from sei-tendermint/config so the tests validate addresses with the parser the router actually dials with, rather than string-slicing and re-encoding the format locally. The parser lives under sei-tendermint/internal and is unreachable from app/seeds; config already surfaces p2p types (AutobahnValidator.NodeKey) and does not import back into it, so the re-export costs nothing. Test changes follow from that: - parse each address instead of splitting on "@", which also covers the port without asserting a specific one — 26656 is our convention, not a protocol rule - assert the seed list is non-empty rather than exactly three; the count is an artefact of how many we happen to run - drop the no-spaces assertion: SplitAndTrimEmpty trims the cutset, so the parser tolerates spaces and the assertion tested nothing - assert empty rather than nil for unknown chains, which is the contract - chain names as constants, testify to match app/genesis and app/params - drop the copy-semantics test, moot now the copy is gone Adds a build-tagged integration suite (go test -tags=integration) that dials every published seed and requires it to send its preface. A seed that accepts TCP and then says nothing is otherwise invisible: the pod reports Ready and seed mode publishes no metrics. --- app/seeds/seeds.go | 20 +---- app/seeds/seeds_integration_test.go | 71 ++++++++++++++++ app/seeds/seeds_test.go | 118 ++++++++++---------------- sei-tendermint/config/node_address.go | 19 +++++ 4 files changed, 141 insertions(+), 87 deletions(-) create mode 100644 app/seeds/seeds_integration_test.go create mode 100644 sei-tendermint/config/node_address.go diff --git a/app/seeds/seeds.go b/app/seeds/seeds.go index b4a33dc354..0cba33bb2c 100644 --- a/app/seeds/seeds.go +++ b/app/seeds/seeds.go @@ -42,21 +42,9 @@ var chainSeeds = map[string][]string{ }, } -// ForChain returns the Sei Labs seed addresses for a well-known chain, or nil -// when the chain-id is not recognised (private and local chains included). -// The returned slice is a copy; callers may not mutate package state. -func ForChain(chainID string) []string { - s, ok := chainSeeds[chainID] - if !ok { - return nil - } - out := make([]string, len(s)) - copy(out, s) - return out -} - -// BootstrapPeers returns the seeds for a chain as the comma-separated value -// CometBFT's `bootstrap-peers` expects, or "" when the chain is unrecognised. +// BootstrapPeers returns the Sei Labs seeds for a chain as the comma-separated +// value CometBFT's `bootstrap-peers` expects, or "" when the chain-id is not +// recognised (private and local chains included). func BootstrapPeers(chainID string) string { - return strings.Join(ForChain(chainID), ",") + return strings.Join(chainSeeds[chainID], ",") } diff --git a/app/seeds/seeds_integration_test.go b/app/seeds/seeds_integration_test.go new file mode 100644 index 0000000000..683563ee5f --- /dev/null +++ b/app/seeds/seeds_integration_test.go @@ -0,0 +1,71 @@ +//go:build integration + +// Build-tagged off by default: these make real network calls to the published +// seed endpoints, so they belong on CI (and on demand), not in the unit suite. +// +// go test -tags=integration ./app/seeds/... +package seeds + +import ( + "net" + "strconv" + "strings" + "testing" + "time" + + "github.com/sei-protocol/sei-chain/sei-tendermint/config" + "github.com/stretchr/testify/require" +) + +const dialTimeout = 10 * time.Second + +// A seed that is reachable at the TCP layer but never speaks is the failure +// mode this test exists for: the listener accepts, the pod reports Ready, seed +// mode publishes no metrics, and inbound is silently closed. Only the bytes on +// the wire distinguish that from a healthy seed, so assert them. +// +// A conforming node sends its ephemeral-key preface immediately on connect +// without waiting for the dialer, so a seed that sends nothing is broken +// regardless of why. +func TestSeedsAreReachableAndSpeakP2P(t *testing.T) { + for chainID, addrs := range chainSeeds { + for _, entry := range addrs { + addr, err := config.ParseNodeAddress(entry) + require.NoErrorf(t, err, "%s: %q", chainID, entry) + + hostPort := net.JoinHostPort(addr.Hostname, strconv.Itoa(int(addr.Port))) + t.Run(chainID+"/"+addr.Hostname, func(t *testing.T) { + ips, err := net.LookupIP(addr.Hostname) + require.NoErrorf(t, err, "DNS lookup failed for %s", addr.Hostname) + require.NotEmptyf(t, ips, "%s resolved to no addresses", addr.Hostname) + + conn, err := net.DialTimeout("tcp", hostPort, dialTimeout) + require.NoErrorf(t, err, "could not connect to %s", hostPort) + defer conn.Close() + + require.NoError(t, conn.SetReadDeadline(time.Now().Add(dialTimeout))) + buf := make([]byte, 64) + n, err := conn.Read(buf) + require.NoErrorf(t, err, + "%s accepted the connection but sent nothing: inbound P2P is closed even though the listener is up", hostPort) + require.NotZerof(t, n, "%s sent an empty preface", hostPort) + }) + } + } +} + +// The published address must round-trip through the parser the router uses, and +// the NodeID must be the 40-hex form the handshake pins. A mismatch here is a +// dial every operator makes and every operator loses. +func TestSeedAddressesAreDialableForm(t *testing.T) { + for chainID, addrs := range chainSeeds { + for _, entry := range addrs { + addr, err := config.ParseNodeAddress(entry) + require.NoErrorf(t, err, "%s: %q", chainID, entry) + require.Lenf(t, string(addr.NodeID), 40, "%s: %q", chainID, entry) + require.NotZerof(t, addr.Port, "%s: %q has no port", chainID, entry) + require.Truef(t, strings.Contains(addr.Hostname, "."), + "%s: %q should publish a DNS name, not a bare host", chainID, entry) + } + } +} diff --git a/app/seeds/seeds_test.go b/app/seeds/seeds_test.go index 8d4eb6115a..71ea9addcb 100644 --- a/app/seeds/seeds_test.go +++ b/app/seeds/seeds_test.go @@ -5,47 +5,44 @@ import ( "testing" "github.com/sei-protocol/sei-chain/app/genesis" - "github.com/sei-protocol/sei-chain/sei-tendermint/types" + "github.com/sei-protocol/sei-chain/sei-tendermint/config" + "github.com/stretchr/testify/require" ) -// A malformed NodeID is rejected at the secret-connection handshake, so the -// seed silently never connects — no error, no metric (seed mode serves none). -// This is the test that stops that shipping. +const ( + pacific = "pacific-1" + atlantic = "atlantic-2" + arctic = "arctic-1" +) + +// Addresses are parsed with the same parser the router uses when it dials, so +// this cannot drift from what p2p actually accepts. What it protects against is +// a typo in the table above: the NodeID is verified during the +// secret-connection handshake, so a wrong one is a rejected dial rather than a +// degraded connection, and seed mode serves no metrics to notice it by. // -// The ID is held against types.NodeID.Validate rather than a local pattern, so -// this cannot drift from CometBFT's actual definition. Uniqueness is checked -// across the whole table, not per chain: the likeliest copy/paste error is a -// pacific-1 entry pasted into the atlantic-2 block, which a per-chain check -// would miss. -func TestSeedAddressesAreWellFormed(t *testing.T) { +// Uniqueness is asserted across the whole table rather than per chain: the +// likeliest copy/paste error when adding a network is a pacific-1 entry landing +// in the atlantic-2 block, which a per-chain check cannot see. +func TestSeedAddressesParseAndAreUnique(t *testing.T) { seenID := map[string]string{} seenHost := map[string]string{} for chainID, addrs := range chainSeeds { - if len(addrs) == 0 { - t.Errorf("%s: no seeds configured", chainID) - } - for _, a := range addrs { - id, hostPort, ok := strings.Cut(a, "@") - if !ok { - t.Errorf("%s: seed %q is not in NodeID@host:port form", chainID, a) - continue - } - if err := types.NodeID(id).Validate(); err != nil { - t.Errorf("%s: seed %q has an invalid NodeID: %v", chainID, a, err) - continue - } - if !strings.HasSuffix(hostPort, ":26656") { - t.Errorf("%s: seed %q does not use the default P2P port 26656", chainID, a) - } - if prev, dup := seenID[id]; dup { - t.Errorf("NodeID %s appears in both %s and %s", id, prev, chainID) - } + require.NotEmptyf(t, addrs, "%s has no seeds", chainID) + + for _, entry := range addrs { + addr, err := config.ParseNodeAddress(entry) + require.NoErrorf(t, err, "%s: %q", chainID, entry) + + id := string(addr.NodeID) + require.NotContainsf(t, seenID, id, + "NodeID %s appears in both %s and %s", id, seenID[id], chainID) seenID[id] = chainID - if prev, dup := seenHost[hostPort]; dup { - t.Errorf("host %s appears in both %s and %s", hostPort, prev, chainID) - } - seenHost[hostPort] = chainID + + require.NotContainsf(t, seenHost, addr.Hostname, + "host %s appears in both %s and %s", addr.Hostname, seenHost[addr.Hostname], chainID) + seenHost[addr.Hostname] = chainID } } } @@ -55,53 +52,32 @@ func TestSeedAddressesAreWellFormed(t *testing.T) { // asserted: arctic-1 is intentionally well-known for genesis but has no seeds. func TestSeedChainsAreWellKnown(t *testing.T) { for chainID := range chainSeeds { - if !genesis.IsWellKnown(chainID) { - t.Errorf("chain %q has seeds but is not a well-known chain (typo?)", chainID) - } + require.Truef(t, genesis.IsWellKnown(chainID), + "chain %q has seeds but is not a well-known chain (typo?)", chainID) } } func TestArcticIsDeliberatelyExcluded(t *testing.T) { - if got := ForChain("arctic-1"); got != nil { - t.Fatalf("arctic-1 is a devnet and must not ship seeds, got %v", got) - } + require.Empty(t, BootstrapPeers(arctic), "arctic-1 is a devnet and must not ship seeds") // Guard the premise of the exclusion: arctic-1 is still initialisable. - if !genesis.IsWellKnown("arctic-1") { - t.Error("arctic-1 is expected to remain a well-known chain for genesis") - } + require.True(t, genesis.IsWellKnown(arctic)) } -func TestForChain(t *testing.T) { - for _, chainID := range []string{"pacific-1", "atlantic-2"} { - if got := ForChain(chainID); len(got) != 3 { - t.Errorf("%s: expected 3 seeds, got %d", chainID, len(got)) - } - } - for _, unknown := range []string{"", "unknown-1", "Pacific-1", "pacific-1 "} { - if got := ForChain(unknown); got != nil { - t.Errorf("chain %q: expected nil (exact match only), got %v", unknown, got) +func TestBootstrapPeers(t *testing.T) { + for _, chainID := range []string{pacific, atlantic} { + got := BootstrapPeers(chainID) + require.NotEmptyf(t, got, "%s should ship seeds", chainID) + // Round-trip the rendered value through the parser the way seid does, + // so the joined form is asserted rather than just the table entries. + for _, entry := range strings.Split(got, ",") { + _, err := config.ParseNodeAddress(entry) + require.NoErrorf(t, err, "%s: %q", chainID, entry) } } -} -// The returned slice must not alias package state. -func TestForChainReturnsCopy(t *testing.T) { - first := ForChain("pacific-1") - first[0] = "tampered" - if second := ForChain("pacific-1"); second[0] == "tampered" { - t.Fatal("ForChain leaked the package-level slice to callers") - } -} - -func TestBootstrapPeers(t *testing.T) { - got := BootstrapPeers("pacific-1") - if n := len(strings.Split(got, ",")); n != 3 { - t.Errorf("expected 3 comma-separated entries, got %d (%q)", n, got) - } - if strings.Contains(got, " ") { - t.Errorf("bootstrap-peers must not contain spaces: %q", got) - } - if BootstrapPeers("unknown-1") != "" { - t.Error("unknown chain must yield an empty bootstrap-peers value") + // Exact match only — a chain-id we do not recognise must contribute nothing, + // so private and local chains are unaffected. + for _, unknown := range []string{"", "unknown-1", "Pacific-1", pacific + " "} { + require.Emptyf(t, BootstrapPeers(unknown), "chain %q should ship no seeds", unknown) } } diff --git a/sei-tendermint/config/node_address.go b/sei-tendermint/config/node_address.go new file mode 100644 index 0000000000..cd85aec2bd --- /dev/null +++ b/sei-tendermint/config/node_address.go @@ -0,0 +1,19 @@ +package config + +import "github.com/sei-protocol/sei-chain/sei-tendermint/internal/p2p" + +// NodeAddress is a peer address in `NodeID@host:port` form, as the p2p router +// understands it. +// +// Re-exported here because the parser lives under sei-tendermint/internal and +// is therefore unreachable from packages outside this module — app/seeds, which +// ships the default bootstrap-peers, being the case that prompted this. Without +// it, callers re-implement the address format and drift from what p2p actually +// accepts. This package already surfaces p2p types (see AutobahnValidator.NodeKey). +type NodeAddress = p2p.NodeAddress + +// ParseNodeAddress parses and validates a peer address, applying exactly the +// rules the router applies when it dials one. +func ParseNodeAddress(address string) (NodeAddress, error) { + return p2p.ParseNodeAddress(address) +} From a29c05b2e636515f9c125b8d244b40711a67d736 Mon Sep 17 00:00:00 2001 From: monty-sei Date: Wed, 12 Aug 2026 14:41:36 +1000 Subject: [PATCH 4/8] fix(seeds): pin the P2P port, which parsing alone does not check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ParseNodeAddress substitutes 26657 — the RPC port — when the port is missing or zero, so the previous NotZero assertion could never fail. A seed entry written without ":26656", the likeliest paste error, parsed clean, passed every test, and would have shipped pointing at the RPC port with no way to recall it. Assert the port explicitly in both the unit and integration suites. That gap was introduced when the tests moved from string-slicing to the parser: parsing is the stronger check for everything except the one field the parser silently fills in. Also: - document the port substitution on ParseNodeAddress, where callers meet it, and correct the doc's claim about internal visibility (it is scoped to the sei-tendermint tree, not the module — the repo has a single go.mod) - cut applyDefaultBootstrapPeers' godoc to what the function does; the PEX rationale is already in the app/seeds package doc and the --overwrite behaviour belongs in the release notes - compile-check the build-tagged integration file in `make lint`, since the untagged `go vet ./...` skips it and it would otherwise rot --- Makefile | 3 +++ app/seeds/seeds_integration_test.go | 6 +++++- app/seeds/seeds_test.go | 7 +++++++ cmd/seid/cmd/init.go | 20 +++----------------- sei-tendermint/config/node_address.go | 18 +++++++++--------- 5 files changed, 27 insertions(+), 27 deletions(-) diff --git a/Makefile b/Makefile index 82448449f0..d7e1ff4fe2 100644 --- a/Makefile +++ b/Makefile @@ -174,6 +174,9 @@ lint: go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.8.0 run go fmt ./... go vet ./... + # Build-tagged files are skipped by the vet above, so they would rot + # unnoticed against API changes. Compile-check them here. + go vet -tags=integration ./app/seeds/... go mod tidy go mod verify diff --git a/app/seeds/seeds_integration_test.go b/app/seeds/seeds_integration_test.go index 683563ee5f..028a5ef7cd 100644 --- a/app/seeds/seeds_integration_test.go +++ b/app/seeds/seeds_integration_test.go @@ -63,7 +63,11 @@ func TestSeedAddressesAreDialableForm(t *testing.T) { addr, err := config.ParseNodeAddress(entry) require.NoErrorf(t, err, "%s: %q", chainID, entry) require.Lenf(t, string(addr.NodeID), 40, "%s: %q", chainID, entry) - require.NotZerof(t, addr.Port, "%s: %q has no port", chainID, entry) + // Not NotZero: ParseNodeAddress fills a missing port with 26657, + // so that assertion could never fail and a dropped ":26656" would + // ship pointing at the RPC port. + require.EqualValuesf(t, 26656, addr.Port, + "%s: %q must publish the P2P port explicitly", chainID, entry) require.Truef(t, strings.Contains(addr.Hostname, "."), "%s: %q should publish a DNS name, not a bare host", chainID, entry) } diff --git a/app/seeds/seeds_test.go b/app/seeds/seeds_test.go index 71ea9addcb..9b553ab03f 100644 --- a/app/seeds/seeds_test.go +++ b/app/seeds/seeds_test.go @@ -35,6 +35,13 @@ func TestSeedAddressesParseAndAreUnique(t *testing.T) { addr, err := config.ParseNodeAddress(entry) require.NoErrorf(t, err, "%s: %q", chainID, entry) + // Parsing alone does not cover the port: ParseNodeAddress + // substitutes 26657, the RPC port, when one is missing or zero. + // A dropped ":26656" would therefore parse clean, pass every other + // assertion here, and ship a permanently wrong port to operators. + require.EqualValuesf(t, 26656, addr.Port, + "%s: %q must publish the P2P port explicitly", chainID, entry) + id := string(addr.NodeID) require.NotContainsf(t, seenID, id, "NodeID %s appears in both %s and %s", id, seenID[id], chainID) diff --git a/cmd/seid/cmd/init.go b/cmd/seid/cmd/init.go index a249fe0983..c18141cdbd 100644 --- a/cmd/seid/cmd/init.go +++ b/cmd/seid/cmd/init.go @@ -205,23 +205,9 @@ For validator or seed nodes, pass --mode validator or --mode seed so RPC and P2P return cmd } -// applyDefaultBootstrapPeers fills bootstrap-peers with the Sei Labs seeds for -// the public networks, so a freshly initialised node bootstraps peer discovery -// with no operator configuration. An unrecognised chain-id leaves the field -// untouched, so private and local chains are unaffected. -// -// Seeds belong in bootstrap-peers rather than persistent-peers: they seed the -// address book via PEX and may then be dropped, whereas persistent-peers holds -// connections open indefinitely. -// -// The empty check is defensive rather than load-bearing today. `seid init` -// builds its config from tmcfg.DefaultConfig() and exposes no flag for -// bootstrap-peers, so the field is always empty here — but the guard keeps the -// behaviour correct if a future caller pre-populates it. Note that -// `init --overwrite` rewrites config.toml wholesale, so an operator's -// hand-edited bootstrap-peers is replaced by the seeds (previously it was -// replaced by ""); without --overwrite, init refuses to touch an existing -// config at all. +// applyDefaultBootstrapPeers sets bootstrap-peers to the Sei Labs seeds for +// chainID, leaving the field unchanged when it is already set or the chain +// ships no seeds. func applyDefaultBootstrapPeers(cfg *tmcfg.Config, chainID string) { if cfg.P2P.BootstrapPeers != "" { return diff --git a/sei-tendermint/config/node_address.go b/sei-tendermint/config/node_address.go index cd85aec2bd..7a51f98274 100644 --- a/sei-tendermint/config/node_address.go +++ b/sei-tendermint/config/node_address.go @@ -3,17 +3,17 @@ package config import "github.com/sei-protocol/sei-chain/sei-tendermint/internal/p2p" // NodeAddress is a peer address in `NodeID@host:port` form, as the p2p router -// understands it. -// -// Re-exported here because the parser lives under sei-tendermint/internal and -// is therefore unreachable from packages outside this module — app/seeds, which -// ships the default bootstrap-peers, being the case that prompted this. Without -// it, callers re-implement the address format and drift from what p2p actually -// accepts. This package already surfaces p2p types (see AutobahnValidator.NodeKey). +// understands it. It is re-exported here because the parser lives under +// sei-tendermint/internal and is unreachable from packages outside the +// sei-tendermint tree. type NodeAddress = p2p.NodeAddress -// ParseNodeAddress parses and validates a peer address, applying exactly the -// rules the router applies when it dials one. +// ParseNodeAddress parses and validates a peer address, applying the same rules +// the router applies when it dials one. +// +// A missing or zero port is substituted with 26657, so a caller that requires a +// particular port must assert it separately; parsing alone will not catch an +// omitted one. func ParseNodeAddress(address string) (NodeAddress, error) { return p2p.ParseNodeAddress(address) } From 83dcfc2ae6e724a7f4e8f1d9ca9958c97d07d4df Mon Sep 17 00:00:00 2001 From: monty-sei Date: Thu, 13 Aug 2026 09:58:27 +1000 Subject: [PATCH 5/8] refactor(seeds): assert the address round-trips instead of pinning a port Replaces the explicit 26656 assertion with a parse round-trip. The property that matters is that a published address survives the parser unchanged, not that the port equals a particular number; pinning the number read as though the protocol required it. The round-trip still catches the case that prompted the assertion, since ParseNodeAddress substitutes 26657 for a missing port and the re-rendered string then differs from what was written. Verified by dropping a port from the table and watching the test fail. Deletes TestSeedAddressesAreDialableForm. It made no network calls, so the integration tag meant it never ran, and its parse and port checks duplicated the unit test. Its one unique assertion, that seeds publish a DNS name rather than a bare host, moves to seeds_test.go where it runs by default. The integration file now holds only the reachability check, which is the sole thing there that needs a network. The cmd tests no longer pin the seed count; they assert init writes seeds.BootstrapPeers(chainID) unaltered, which is what the wiring is responsible for. Cardinality and address shape belong to app/seeds, where the table lives. Drops the `go vet -tags=integration` line from `make lint`: `make lint` is not a CI job, so it only ever fired locally and did not deliver the compile check it claimed. Wiring the tagged package into CI is the real fix and is still open. --- Makefile | 3 --- app/seeds/seeds_integration_test.go | 32 ++++----------------------- app/seeds/seeds_test.go | 18 ++++++++++----- cmd/seid/cmd/bootstrap_peers_test.go | 13 ++++++----- sei-tendermint/config/node_address.go | 7 +++--- 5 files changed, 27 insertions(+), 46 deletions(-) diff --git a/Makefile b/Makefile index d7e1ff4fe2..82448449f0 100644 --- a/Makefile +++ b/Makefile @@ -174,9 +174,6 @@ lint: go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.8.0 run go fmt ./... go vet ./... - # Build-tagged files are skipped by the vet above, so they would rot - # unnoticed against API changes. Compile-check them here. - go vet -tags=integration ./app/seeds/... go mod tidy go mod verify diff --git a/app/seeds/seeds_integration_test.go b/app/seeds/seeds_integration_test.go index 028a5ef7cd..1081646b87 100644 --- a/app/seeds/seeds_integration_test.go +++ b/app/seeds/seeds_integration_test.go @@ -1,7 +1,7 @@ //go:build integration -// Build-tagged off by default: these make real network calls to the published -// seed endpoints, so they belong on CI (and on demand), not in the unit suite. +// Build-tagged off by default: this makes real network calls to the published +// seed endpoints, so it belongs on CI (and on demand), not in the unit suite. // // go test -tags=integration ./app/seeds/... package seeds @@ -9,7 +9,6 @@ package seeds import ( "net" "strconv" - "strings" "testing" "time" @@ -26,7 +25,8 @@ const dialTimeout = 10 * time.Second // // A conforming node sends its ephemeral-key preface immediately on connect // without waiting for the dialer, so a seed that sends nothing is broken -// regardless of why. +// regardless of why. Everything that can be checked without a network lives in +// seeds_test.go, which runs by default. func TestSeedsAreReachableAndSpeakP2P(t *testing.T) { for chainID, addrs := range chainSeeds { for _, entry := range addrs { @@ -35,10 +35,6 @@ func TestSeedsAreReachableAndSpeakP2P(t *testing.T) { hostPort := net.JoinHostPort(addr.Hostname, strconv.Itoa(int(addr.Port))) t.Run(chainID+"/"+addr.Hostname, func(t *testing.T) { - ips, err := net.LookupIP(addr.Hostname) - require.NoErrorf(t, err, "DNS lookup failed for %s", addr.Hostname) - require.NotEmptyf(t, ips, "%s resolved to no addresses", addr.Hostname) - conn, err := net.DialTimeout("tcp", hostPort, dialTimeout) require.NoErrorf(t, err, "could not connect to %s", hostPort) defer conn.Close() @@ -53,23 +49,3 @@ func TestSeedsAreReachableAndSpeakP2P(t *testing.T) { } } } - -// The published address must round-trip through the parser the router uses, and -// the NodeID must be the 40-hex form the handshake pins. A mismatch here is a -// dial every operator makes and every operator loses. -func TestSeedAddressesAreDialableForm(t *testing.T) { - for chainID, addrs := range chainSeeds { - for _, entry := range addrs { - addr, err := config.ParseNodeAddress(entry) - require.NoErrorf(t, err, "%s: %q", chainID, entry) - require.Lenf(t, string(addr.NodeID), 40, "%s: %q", chainID, entry) - // Not NotZero: ParseNodeAddress fills a missing port with 26657, - // so that assertion could never fail and a dropped ":26656" would - // ship pointing at the RPC port. - require.EqualValuesf(t, 26656, addr.Port, - "%s: %q must publish the P2P port explicitly", chainID, entry) - require.Truef(t, strings.Contains(addr.Hostname, "."), - "%s: %q should publish a DNS name, not a bare host", chainID, entry) - } - } -} diff --git a/app/seeds/seeds_test.go b/app/seeds/seeds_test.go index 9b553ab03f..de98b17465 100644 --- a/app/seeds/seeds_test.go +++ b/app/seeds/seeds_test.go @@ -35,12 +35,18 @@ func TestSeedAddressesParseAndAreUnique(t *testing.T) { addr, err := config.ParseNodeAddress(entry) require.NoErrorf(t, err, "%s: %q", chainID, entry) - // Parsing alone does not cover the port: ParseNodeAddress - // substitutes 26657, the RPC port, when one is missing or zero. - // A dropped ":26656" would therefore parse clean, pass every other - // assertion here, and ship a permanently wrong port to operators. - require.EqualValuesf(t, 26656, addr.Port, - "%s: %q must publish the P2P port explicitly", chainID, entry) + // Round-trip rather than parse alone. ParseNodeAddress substitutes + // 26657 for a missing port, so a dropped ":26656" parses clean and + // would ship pointing at the RPC port; re-rendering catches that, + // and any other silent normalisation, without pinning a port number + // as though the protocol required one. + require.Equalf(t, entry, strings.TrimPrefix(addr.String(), "mconn://"), + "%s: %q does not survive a parse round-trip", chainID, entry) + + // Seeds publish DNS names, not bare hosts, so the address outlives + // any IP change behind it. + require.Containsf(t, addr.Hostname, ".", + "%s: %q should publish a DNS name", chainID, entry) id := string(addr.NodeID) require.NotContainsf(t, seenID, id, diff --git a/cmd/seid/cmd/bootstrap_peers_test.go b/cmd/seid/cmd/bootstrap_peers_test.go index 72d3b89ec7..14b3ff8f36 100644 --- a/cmd/seid/cmd/bootstrap_peers_test.go +++ b/cmd/seid/cmd/bootstrap_peers_test.go @@ -42,15 +42,16 @@ func TestApplyDefaultBootstrapPeers(t *testing.T) { } } -// The public networks must actually get a usable set, not just a non-empty -// string — a silently truncated table would still satisfy the table test above. +// What the wiring owes is the seed list for the chain, whole and unaltered. +// How many addresses that is, and whether they are well formed, belongs to +// app/seeds, where the table and its per-cell rationale live. func TestApplyDefaultBootstrapPeersPopulatesPublicNetworks(t *testing.T) { for _, chainID := range []string{"pacific-1", "atlantic-2"} { cfg := tmcfg.DefaultConfig() applyDefaultBootstrapPeers(cfg, chainID) - if n := len(strings.Split(cfg.P2P.BootstrapPeers, ",")); n != 3 { - t.Errorf("%s: got %d bootstrap peers, want 3 (%q)", chainID, n, cfg.P2P.BootstrapPeers) - } + require.NotEmptyf(t, cfg.P2P.BootstrapPeers, "%s should ship seeds", chainID) + require.Equalf(t, seeds.BootstrapPeers(chainID), cfg.P2P.BootstrapPeers, + "%s: init must write the seed list unaltered", chainID) } } @@ -116,8 +117,8 @@ func TestInitCmdWritesDefaultBootstrapPeers(t *testing.T) { for _, chainID := range []string{"pacific-1", "atlantic-2"} { t.Run(chainID, func(t *testing.T) { got := bootstrapPeersLine(t, runInit(t, chainID)) + require.NotEmpty(t, got) require.Equal(t, seeds.BootstrapPeers(chainID), got) - require.Len(t, strings.Split(got, ","), 3) }) } } diff --git a/sei-tendermint/config/node_address.go b/sei-tendermint/config/node_address.go index 7a51f98274..9efaf84dbe 100644 --- a/sei-tendermint/config/node_address.go +++ b/sei-tendermint/config/node_address.go @@ -3,9 +3,10 @@ package config import "github.com/sei-protocol/sei-chain/sei-tendermint/internal/p2p" // NodeAddress is a peer address in `NodeID@host:port` form, as the p2p router -// understands it. It is re-exported here because the parser lives under -// sei-tendermint/internal and is unreachable from packages outside the -// sei-tendermint tree. +// understands it. +// +// Aliased here so callers outside the sei-tendermint tree, which cannot reach +// internal/p2p, can still name the type. type NodeAddress = p2p.NodeAddress // ParseNodeAddress parses and validates a peer address, applying the same rules From 6666ff7406a112fb0d15bdadb2b4571add997d05 Mon Sep 17 00:00:00 2001 From: monty-sei Date: Thu, 13 Aug 2026 10:06:23 +1000 Subject: [PATCH 6/8] docs(seeds): state the cell mapping, unify test idiom, flag the CI gap Names the unsuffixed `prod` cell as eu-central-1 in the seed table's comment. The other two are inferable from the hostnames (prod-euw1, prod-use2) but that one was only inferable by elimination, and whoever adds or retires a cell reads exactly this comment. Uses require.* throughout bootstrap_peers_test.go rather than mixing it with t.Errorf in the three helper-level tests. States plainly in the integration file's header that nothing runs or compile-checks it: the `integration` tag is used nowhere else, and both go vet and golangci-lint skip tagged files. It is an on-demand tool today, not coverage, and saying so beats a header that claims CI it does not have. The scheduled job is tracked separately and should land after the seeds are healthy, so its first run is green. --- app/seeds/seeds.go | 7 ++++--- app/seeds/seeds_integration_test.go | 10 ++++++++-- cmd/seid/cmd/bootstrap_peers_test.go | 9 +++------ 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/app/seeds/seeds.go b/app/seeds/seeds.go index 0cba33bb2c..f3f68e6d5f 100644 --- a/app/seeds/seeds.go +++ b/app/seeds/seeds.go @@ -11,9 +11,10 @@ package seeds import "strings" // chainSeeds maps a well-known chain-id to its Sei Labs seed nodes, each in -// CometBFT's `NodeID@host:port` form. Three per network, one per cell -// (eu-central-1, eu-west-1, us-east-2), so losing a region does not cost -// bootstrap capability. +// CometBFT's `NodeID@host:port` form. Three per network, one per cell, so +// losing a region does not cost bootstrap capability. The cell is encoded in +// the hostname: unsuffixed `prod` is eu-central-1, `prod-euw1` is eu-west-1, +// and `prod-use2` is us-east-2. // // PERMANENCE: these strings ship inside released binaries and operators pin // them. The secret-connection handshake verifies the NodeID, so a changed ID diff --git a/app/seeds/seeds_integration_test.go b/app/seeds/seeds_integration_test.go index 1081646b87..89f2d52126 100644 --- a/app/seeds/seeds_integration_test.go +++ b/app/seeds/seeds_integration_test.go @@ -1,9 +1,15 @@ //go:build integration -// Build-tagged off by default: this makes real network calls to the published -// seed endpoints, so it belongs on CI (and on demand), not in the unit suite. +// Reachability checks against the live published seed endpoints, build-tagged +// off by default because they make real network calls: // // go test -tags=integration ./app/seeds/... +// +// NOT WIRED TO CI. Nothing runs this file and nothing compile-checks it: the +// `integration` tag is used nowhere else, and go vet and golangci-lint both +// skip tagged files. Treat it as an on-demand tool, not as coverage. A +// scheduled job is tracked separately, and should land after the seeds it +// dials are all healthy, so its first run is green rather than red. package seeds import ( diff --git a/cmd/seid/cmd/bootstrap_peers_test.go b/cmd/seid/cmd/bootstrap_peers_test.go index 14b3ff8f36..7524023247 100644 --- a/cmd/seid/cmd/bootstrap_peers_test.go +++ b/cmd/seid/cmd/bootstrap_peers_test.go @@ -35,9 +35,7 @@ func TestApplyDefaultBootstrapPeers(t *testing.T) { t.Run(tt.name, func(t *testing.T) { cfg := tmcfg.DefaultConfig() applyDefaultBootstrapPeers(cfg, tt.chainID) - if cfg.P2P.BootstrapPeers != tt.want { - t.Errorf("bootstrap-peers = %q, want %q", cfg.P2P.BootstrapPeers, tt.want) - } + require.Equal(t, tt.want, cfg.P2P.BootstrapPeers) }) } } @@ -65,9 +63,8 @@ func TestApplyDefaultBootstrapPeersPreservesExistingValue(t *testing.T) { cfg.P2P.BootstrapPeers = existing applyDefaultBootstrapPeers(cfg, "pacific-1") - if cfg.P2P.BootstrapPeers != existing { - t.Errorf("existing bootstrap-peers was overwritten: got %q, want %q", cfg.P2P.BootstrapPeers, existing) - } + require.Equal(t, existing, cfg.P2P.BootstrapPeers, + "an existing bootstrap-peers value must never be overwritten") } // runInit executes the real InitCmd against a temp home and returns the written From 71a92b39309f22d073d2ace9172c105c18bd4ae2 Mon Sep 17 00:00:00 2001 From: monty-sei Date: Fri, 14 Aug 2026 11:26:47 +1000 Subject: [PATCH 7/8] test(seeds): run seed reachability in CI, tolerating one seed per network Adds a workflow that dials every published seed and asserts it actually speaks P2P rather than merely accepting the TCP connection, answering the review ask that these be hooked to a job rather than left as a file nothing runs. It also vets the tagged package, which the untagged vet and golangci-lint both skip, so it cannot rot against an API change. The suite now fails per network only when more than one seed is not serving. That is the property the seeds actually owe: three per network exist so losing a region does not cost bootstrap capability, and a node bootstraps fine on the remaining two. Failing on any single unreachable seed would make this a liveness alarm for individual pods rather than a check that the published set still does its job. A tolerated failure is still named in the output, so a seed that stays dark is visible rather than silently absorbed. Manual trigger only for now. It talks to live external infrastructure, so it is not a PR gate: a seed hiccup or CI egress trouble must not block unrelated work. The schedule, and tightening the tolerance to zero, both follow once every published seed is serving, so the first automated run is green rather than red. --- .github/workflows/seed-reachability.yml | 43 ++++++++++++ app/seeds/seeds_integration_test.go | 91 +++++++++++++++++-------- 2 files changed, 106 insertions(+), 28 deletions(-) create mode 100644 .github/workflows/seed-reachability.yml diff --git a/.github/workflows/seed-reachability.yml b/.github/workflows/seed-reachability.yml new file mode 100644 index 0000000000..42933b3749 --- /dev/null +++ b/.github/workflows/seed-reachability.yml @@ -0,0 +1,43 @@ +name: Seed Reachability +run-name: Seed Reachability / published seeds serve P2P + +# Dials the published Sei Labs seed endpoints and asserts each one actually +# speaks P2P, rather than merely accepting the TCP connection. Seed mode +# publishes no Prometheus metrics and the pods report Ready while serving +# nothing, so this is the only signal that would catch a seed going dark. +# +# Manual for now. It talks to live external infrastructure, so it is not a PR +# gate: a seed hiccup or CI egress trouble must not block unrelated work. The +# scheduled trigger is added once every published seed is serving, so its first +# automated run is green rather than red. +on: + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + reachability: + name: Reachability + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-go@v5 + with: + go-version-file: go.mod + cache: true + + # Compile-checked as well as run: build-tagged files are skipped by the + # untagged vet and by golangci-lint, so without this the package can rot + # against an API change and stay green. + - name: Vet + run: go vet -tags=integration ./app/seeds/... + + - name: Seed reachability + run: go test -tags=integration -v -count=1 -timeout=5m ./app/seeds/... diff --git a/app/seeds/seeds_integration_test.go b/app/seeds/seeds_integration_test.go index 89f2d52126..4fd0ffd05f 100644 --- a/app/seeds/seeds_integration_test.go +++ b/app/seeds/seeds_integration_test.go @@ -5,14 +5,11 @@ // // go test -tags=integration ./app/seeds/... // -// NOT WIRED TO CI. Nothing runs this file and nothing compile-checks it: the -// `integration` tag is used nowhere else, and go vet and golangci-lint both -// skip tagged files. Treat it as an on-demand tool, not as coverage. A -// scheduled job is tracked separately, and should land after the seeds it -// dials are all healthy, so its first run is green rather than red. +// Run in CI by .github/workflows/seed-reachability.yml. package seeds import ( + "fmt" "net" "strconv" "testing" @@ -24,34 +21,72 @@ import ( const dialTimeout = 10 * time.Second +// maxUnreachablePerNetwork is how many seeds in one network may fail to speak +// before this suite fails. +// +// One rather than zero, because that is the property the seeds actually owe: +// three per network exist so losing a region does not cost bootstrap +// capability, and a node bootstraps fine on the remaining two. Failing on any +// single unreachable seed would make this a liveness alarm for individual pods +// rather than a check that the published set still does its job. +// +// A tolerated failure is still named in the output, so a seed that stays dark +// is visible rather than silently absorbed. +// +// Tighten this to zero once every published seed is serving. +const maxUnreachablePerNetwork = 1 + // A seed that is reachable at the TCP layer but never speaks is the failure -// mode this test exists for: the listener accepts, the pod reports Ready, seed -// mode publishes no metrics, and inbound is silently closed. Only the bytes on -// the wire distinguish that from a healthy seed, so assert them. +// mode this exists for: the listener accepts, the pod reports Ready, seed mode +// publishes no metrics, and inbound is silently closed. Only the bytes on the +// wire distinguish that from a healthy seed, so assert them. // // A conforming node sends its ephemeral-key preface immediately on connect // without waiting for the dialer, so a seed that sends nothing is broken -// regardless of why. Everything that can be checked without a network lives in -// seeds_test.go, which runs by default. +// regardless of why. func TestSeedsAreReachableAndSpeakP2P(t *testing.T) { for chainID, addrs := range chainSeeds { - for _, entry := range addrs { - addr, err := config.ParseNodeAddress(entry) - require.NoErrorf(t, err, "%s: %q", chainID, entry) - - hostPort := net.JoinHostPort(addr.Hostname, strconv.Itoa(int(addr.Port))) - t.Run(chainID+"/"+addr.Hostname, func(t *testing.T) { - conn, err := net.DialTimeout("tcp", hostPort, dialTimeout) - require.NoErrorf(t, err, "could not connect to %s", hostPort) - defer conn.Close() - - require.NoError(t, conn.SetReadDeadline(time.Now().Add(dialTimeout))) - buf := make([]byte, 64) - n, err := conn.Read(buf) - require.NoErrorf(t, err, - "%s accepted the connection but sent nothing: inbound P2P is closed even though the listener is up", hostPort) - require.NotZerof(t, n, "%s sent an empty preface", hostPort) - }) - } + t.Run(chainID, func(t *testing.T) { + var unreachable []string + + for _, entry := range addrs { + addr, err := config.ParseNodeAddress(entry) + require.NoErrorf(t, err, "%s: %q", chainID, entry) + + if err := speaksP2P(addr); err != nil { + unreachable = append(unreachable, fmt.Sprintf("%s: %v", addr.Hostname, err)) + t.Logf("UNREACHABLE %s", addr.Hostname) + continue + } + t.Logf("ok %s", addr.Hostname) + } + + require.LessOrEqualf(t, len(unreachable), maxUnreachablePerNetwork, + "%s: %d of %d seeds are not serving P2P (tolerating up to %d):\n %v", + chainID, len(unreachable), len(addrs), maxUnreachablePerNetwork, unreachable) + }) + } +} + +// speaksP2P dials the seed and waits for it to send its handshake preface. +func speaksP2P(addr config.NodeAddress) error { + hostPort := net.JoinHostPort(addr.Hostname, strconv.Itoa(int(addr.Port))) + + conn, err := net.DialTimeout("tcp", hostPort, dialTimeout) + if err != nil { + return fmt.Errorf("dial: %w", err) + } + defer conn.Close() + + if err := conn.SetReadDeadline(time.Now().Add(dialTimeout)); err != nil { + return fmt.Errorf("set deadline: %w", err) + } + n, err := conn.Read(make([]byte, 64)) + if err != nil { + return fmt.Errorf("accepted the connection but sent nothing: %w", err) + } + if n == 0 { + return fmt.Errorf("sent an empty preface") } + return nil } From c842f93e9a6d63ceb873ac3627d699e014fe4737 Mon Sep 17 00:00:00 2001 From: monty-sei Date: Fri, 14 Aug 2026 11:33:45 +1000 Subject: [PATCH 8/8] docs(ci): state when to run seed reachability, and why it is not scheduled It is a verification tool rather than a monitor: run it after the seeds are rolled, before cutting a release that ships the defaults, or when someone reports trouble bootstrapping. No cron. Continuous detection of a seed going dark belongs in the monitoring stack, which cannot see seeds today because seed mode starts no Prometheus listener, and a schedule here would paper over that rather than fix it. --- .github/workflows/seed-reachability.yml | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/.github/workflows/seed-reachability.yml b/.github/workflows/seed-reachability.yml index 42933b3749..d59117e565 100644 --- a/.github/workflows/seed-reachability.yml +++ b/.github/workflows/seed-reachability.yml @@ -2,14 +2,20 @@ name: Seed Reachability run-name: Seed Reachability / published seeds serve P2P # Dials the published Sei Labs seed endpoints and asserts each one actually -# speaks P2P, rather than merely accepting the TCP connection. Seed mode -# publishes no Prometheus metrics and the pods report Ready while serving -# nothing, so this is the only signal that would catch a seed going dark. +# speaks P2P, rather than merely accepting the TCP connection. # -# Manual for now. It talks to live external infrastructure, so it is not a PR -# gate: a seed hiccup or CI egress trouble must not block unrelated work. The -# scheduled trigger is added once every published seed is serving, so its first -# automated run is green rather than red. +# Deliberately manual. This is a verification tool, not a monitor: +# +# - run it after the seeds are rolled onto a new build, to confirm they came +# back +# - run it before cutting a release that ships the seed defaults +# - run it when someone reports trouble bootstrapping +# +# Not a PR gate, because it talks to live external infrastructure and a seed +# hiccup or CI egress trouble must not block unrelated work. Not scheduled +# either: continuous detection of a seed going dark belongs in the monitoring +# stack, which cannot see seeds today because seed mode starts no Prometheus +# listener. A cron here would paper over that rather than fix it. on: workflow_dispatch: