Skip to content

feat(core): implement UpgradeClient#39

Open
tbruyelle wants to merge 15 commits intomasterfrom
tbruyelle/feat/upgrade-client
Open

feat(core): implement UpgradeClient#39
tbruyelle wants to merge 15 commits intomasterfrom
tbruyelle/feat/upgrade-client

Conversation

@tbruyelle
Copy link
Copy Markdown
Collaborator

Implement UpgradeClient for IBC v2 - allows upgrading a Tendermint light
client to a new client state when the counterparty chain has committed
upgrade information at the UpgradePath.

This follows the IBC spec but is rarely needed in practice since most
chain upgrades only require normal UpdateClient calls via relayers.
UpgradeClient would only be needed if IBC itself changed its client
state format, which has never occurred.

Changes:

  • lightclient.Interface: update VerifyUpgradeAndUpdateState signature
  • tendermint: implement VerifyUpgradeAndUpdateState with proof verification
  • state: add ProtoMarshal methods for ClientState, ConsensusState, Fraction
  • types/height: add ProtoMarshal method
  • client: implement UpgradeClient entry point
  • README: document UpgradeClient with note about scarcity of use case

Closes #16

Implement UpgradeClient for IBC v2 - allows upgrading a Tendermint light
client to a new client state when the counterparty chain has committed
upgrade information at the UpgradePath.

This follows the IBC spec but is rarely needed in practice since most
chain upgrades only require normal UpdateClient calls via relayers.
UpgradeClient would only be needed if IBC itself changed its client
state format, which has never occurred.

Changes:
- lightclient.Interface: update VerifyUpgradeAndUpdateState signature
- tendermint: implement VerifyUpgradeAndUpdateState with proof verification
- state: add ProtoMarshal methods for ClientState, ConsensusState, Fraction
- types/height: add ProtoMarshal method
- client: implement UpgradeClient entry point
- README: document UpgradeClient with note about scarcity of use case

Closes #16
The counterparty chain commits to the upgraded client with customizable
fields (TrustLevel, TrustingPeriod, MaxClockDrift, FrozenHeight) zeroed
out, so the proof must be checked against that same shape. Add
ZeroCustomFields on tendermint.ClientState and use it before marshaling
for the upgrade-client proof. Wire the previously-skipped ProofSpecs
field 8 by adding ProtoMarshal for ProofSpec/LeafOp/InnerSpec, plus a
packed-varint helper for InnerSpec.ChildOrder.
Drop the strict equality checks on ChainID, revision number, TrustLevel,
UnbondingPeriod, MaxClockDrift, ProofSpecs and UpgradePath — a chain
upgrade is allowed to change all of these. The only precondition is that
the upgraded latest height be greater than the current latest height.

After proof verification, construct the new client by taking
chain-specified fields (ChainID, UnbondingPeriod, LatestHeight,
ProofSpecs, UpgradePath) from the upgraded client and preserving
customizable fields (TrustLevel, TrustingPeriod, MaxClockDrift) from the
current client, with FrozenHeight reset. When the unbonding period
shrinks, the trusting period is scaled proportionally via
calculateNewTrustingPeriod to keep the security ratio.
Replace speculative commentary about how rarely UpgradeClient is used
with a description of when it actually applies (breaking upgrades:
ChainID/revision change, parameter changes that UpdateClient can't
follow), the verification preconditions, and how the new client state
is constructed from a mix of upgraded and current fields.
Collapse the two-armed type-assertion switches on the proof inputs
into single asserts — the explicit []byte arm was dead under default.
Type-mismatch errors now name the expected and got types instead of a
generic "invalid type" string. Also clarify the UpgradeClient godoc to
describe what's actually verified rather than referring to a
non-existent "specified upgrade height" parameter.
- Overwrite the upgraded consensus state's Root with SentinelRoot
  before storing. The chain commits a placeholder Root because no real
  blocks exist at the upgrade height; storing the sentinel locally
  keeps this consensus state unusable for packet proof verification
  until real headers arrive via UpdateClient.
- Gate UpgradeClient on ensureAuthorizedRelayer to match the project's
  whitelist pattern across CreateClient/UpdateClient.
- Expand the README section with a lifecycle walkthrough, explicit
  preconditions, and the field mapping between current and upgraded
  client states.
Each prefix element of UpgradePath is now a separate KeyPath segment
instead of being joined with '/' into a single byte slice; only the
final element gets the height and leaf-key suffix. This matches
ibc-go's constructUpgrade{Client,ConsState}MerklePath and produces the
correct path for any UpgradePath shape, not just the canonical
two-element ["upgrade", "upgradedIBCState"].

Also fail fast when the client's UpgradePath is empty rather than
silently returning a single-element path that the proof verification
would reject anyway.
Add AttributeKeyConsensusHeight (singular, matching ibc-go's event for
the same op) and emit the upgraded client's new latest height alongside
client_id and client_type. Without it, observers had no way to read the
post-upgrade height directly from the event stream.
Unit tests:
- proto.AppendPackedVarintInt32 — empty omission, zero values inside
  packed payload, multi-byte varints.
- ics23.{ProofSpec,LeafOp,InnerSpec}.ProtoMarshal — golden hex bytes
  pinned against IavlSpec; nil-receiver handling; optional fields.
- tendermint.ClientState.ZeroCustomFields — preserved vs zeroed
  fields, caller's value untouched.
- tendermint.ClientState.ProtoMarshal — confirms ProofSpecs (field 8)
  is now emitted (no longer skipped).
- tendermint.calculateNewTrustingPeriod — proportional shrink, no-op
  when unbonding unchanged, divide-by-zero guard, sub-second
  truncation behaviour.
- tendermint.buildUpgradeMerklePath — canonical 2-element path,
  single-element path, three-element path (each prefix segment kept
  separate).
- TMLightClient.VerifyUpgradeAndUpdateState — 10 early-failure paths:
  type-assertion failures, ValidateBasic failures, empty UpgradePath,
  height-not-greater, proof type/empty checks.

Filetests for the realm entrypoint:
- z10a — client not found.
- z10b — caller not in relayer whitelist.

Happy-path filetest requires generating real ICS-23 proofs against
the upgrade path, which would require extending the gen-proof CLI
with new commitment-type cases. Left as a follow-up.
Counterparties using cdc.MarshalInterface (the SDK proto codec path
ibc-go takes for upgrade-client and upgrade-consensus-state) commit
the values wrapped in a google.protobuf.Any with a type-url prefix.
Reconstructing the bytes for proof verification therefore requires
the same Any wrapping; raw ProtoMarshal output won't match what the
chain stored.

Add proto.MarshalAny, expose Tendermint type URL constants, and wrap
both values before VerifyMembership in VerifyUpgradeAndUpdateState.
This is a prerequisite to a happy-path filetest that uses real
ibc-go-generated proofs.
Adds the happy path that was deferred from the original test commit, now
that the byte-level marshaling matches what an ibc-go counterparty
actually commits.

Three correctness fixes were needed to get a real ibc-go-generated proof
to verify under our Gno code:

1. The SDK upgrade module key for the upgraded consensus state is
   "upgradedConsState" (no "ensus") — our code used
   "upgradedConsensusState".

2. ibc-go's tendermint.ClientState marks TrustLevel, TrustingPeriod,
   UnbondingPeriod, MaxClockDrift, FrozenHeight and LatestHeight with
   `(gogoproto.nullable) = false`, and the same for ConsensusState's
   Timestamp and Root. These fields are emitted on the wire even when
   zero, as length-0 nested messages. AppendLengthDelimited skips
   empty bytes (correct proto3 default). Add
   AppendAlwaysLengthDelimited and route the always-emit fields
   through it so ZeroCustomFields().ProtoMarshal() produces bytes
   byte-equal to gogoproto's output.

3. Extend cmd/gen-proof with an `upgrade` subcommand that uses ibc-go's
   actual tendermint.ClientState/ConsensusState plus
   cdc.MarshalInterface to commit Any-wrapped values to an upgrade
   IAVL store, then emits the chained ICS-23 proofs as Go literals for
   embedding in the filetest. The new filetest sets up a current
   client whose consensus root equals the multistore apphash gen-proof
   committed, and exercises the full UpgradeClient path against the
   embedded proofs.
Mirror the existing TimeMarshal/AppendTime pair: DurationMarshal
returns the inner google.protobuf.Duration encoding, AppendDuration
wraps it as a length-delimited field. Drop the local helper from the
tendermint state package.

Add tests for TimeMarshal/AppendTime alongside the new
DurationMarshal/AppendDuration, including negative-nanos
normalization.
- Add zz_upgrade_client_example_filetest.gno and
  zz_recover_client_example_filetest.gno mirroring the pattern of the
  other zz_*_example filetests (placeholder values, // XXX update
  markers, Error: line for the natural panic).
- Move the long UpgradeClient lifecycle / preconditions / field-mapping
  prose into upgrade-client.md alongside recover-client.md, and shrink
  the README's UpgradeClient section to a brief link block matching
  RecoverClient's.
- Link both examples from the README.
Match ibc-go's MsgUpgradeClient.ValidateBasic by calling
host.ClientIdentifierValidator on the clientID at entry. Previously a
malformed identifier fell through to store.getClient and surfaced as
"client X not found", which conflated unknown clients with malformed
inputs.
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.

Implement core.UpgradeClient()

1 participant