Genesis validation skips the next-sequence check when the highest identifier sequence is zero

[ipezygj]

Genesis validation skips the next-sequence check when the highest identifier sequence is zero

Summary

GenesisState.Validate() in 02-client, 03-connection and 04-channel all guard the
next-sequence invariant the same way:

if maxSequence != 0 && maxSequence >= gs.NextXSequence {
    return ... // next sequence must be greater than the max sequence in use
}

The maxSequence != 0 term is intended to distinguish "no identifiers present" from "identifiers
are present", but maxSequence cannot express that difference: a genesis whose highest identifier
sequence is genuinely 0 takes the same branch as a genesis with no identifiers at all, and the
check is skipped.

• modules/core/02-client/types/genesis.go (NextClientSequence)
• modules/core/03-connection/types/genesis.go (NextConnectionSequence)
• modules/core/04-channel/types/genesis.go (NextChannelSequence)

Why sequence 0 is the common case, not an edge case

GenerateClientIdentifier, GenerateConnectionIdentifier and GenerateChannelIdentifier all
format the identifier before incrementing the counter, so 07-tendermint-0, connection-0 and
channel-0 are the first identifiers every chain issues. next_client_sequence,
next_connection_sequence and next_channel_sequence are proto uint64 fields, so a
hand-assembled genesis that lists identifiers and simply omits the counter gets 0.

Such a genesis validates clean today, and the next identifier generated after start-up is the one
already in state.

Impact

For connections and channels this means the next generated identifier overwrites an existing entry.

For clients it is worse, because nothing downstream re-checks:

• CreateClient (modules/core/02-client/keeper/client.go) generates the identifier, routes, and
  calls Initialize — it never checks whether the identifier is already in use;
• 07-tendermint's Initialize calls setClientState / setConsensusState unconditionally.

So the existing client state is replaced, along with the verification root that every connection
and channel above that client trusts. MsgCreateClient is permissionless.

Reachability — why I do not believe this is exploitable on a live chain

A genesis produced by ExportGenesis cannot have this shape: it reads the counter from state,
and that always exceeds the highest live sequence. The reachable trigger is a hand-assembled or
migration-assembled genesis, i.e. an operator error rather than an intended configuration. A chain
that launched this way would also break the first time anyone created an identifier of that kind,
which is a short window.

I am therefore raising this publicly as a validation/correctness gap rather than through the
bug bounty process. If maintainers assess it differently, say so and I will take it private.

Existing fixtures encode the current behaviour

This is the part I would most like a maintainer opinion on. Four existing test fixtures describe a
genesis that is not actually valid, and they pass today only because the check is skipped:

• modules/core/03-connection/types/genesis_test.go — "valid genesis" uses connection-0 with
  NextConnectionSequence: 0
• the same file's "invalid params" case, which reaches params validation only because the
  sequence check is skipped
• modules/core/genesis_test.go — "valid genesis", in both its connection section
  (connection-0, next sequence 0) and its channel section (channel-0/channel-1, next
  sequence 0)

There is also no existing coverage of this guard in two of the three modules: neither
02-client/types/genesis_test.go nor 03-connection/types/genesis_test.go contains the string
Sequence.

Proposed fix

Track whether any identifier contributed a sequence at all, and gate on that:

if hasSequence && maxSequence >= gs.NextXSequence {

02-client additionally needs to exclude the localhost client identifier from sequence tracking,
because ParseClientIdentifier special-cases it and reports 0 rather than a generated sequence.
03-connection already excludes the localhost connection for the same reason.

I have this implemented with tests (each new case fails before the change and passes after) and
can open a PR if the approach looks right.