Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: modify VerifyUpgradeAndUpdateState to set upgraded client and consensus state #598

Merged
merged 7 commits into from
Mar 16, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 4 additions & 8 deletions modules/core/02-client/keeper/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,23 +165,20 @@ func (k Keeper) UpgradeClient(ctx sdk.Context, clientID string, upgradedClient e
return sdkerrors.Wrapf(types.ErrClientNotActive, "cannot upgrade client (%s) with status %s", clientID, status)
}

updatedClientState, updatedConsState, err := clientState.VerifyUpgradeAndUpdateState(ctx, k.cdc, clientStore,
err := clientState.VerifyUpgradeAndUpdateState(ctx, k.cdc, clientStore,
upgradedClient, upgradedConsState, proofUpgradeClient, proofUpgradeConsState)
if err != nil {
return sdkerrors.Wrapf(err, "cannot upgrade client with ID %s", clientID)
}

k.SetClientState(ctx, clientID, updatedClientState)
k.SetClientConsensusState(ctx, clientID, updatedClientState.GetLatestHeight(), updatedConsState)

k.Logger(ctx).Info("client state upgraded", "client-id", clientID, "height", updatedClientState.GetLatestHeight().String())
k.Logger(ctx).Info("client state upgraded", "client-id", clientID)

defer func() {
telemetry.IncrCounterWithLabels(
[]string{"ibc", "client", "upgrade"},
1,
[]metrics.Label{
telemetry.NewLabel(types.LabelClientType, updatedClientState.ClientType()),
telemetry.NewLabel(types.LabelClientType, upgradedClient.ClientType()),
telemetry.NewLabel(types.LabelClientID, clientID),
},
)
Expand All @@ -192,8 +189,7 @@ func (k Keeper) UpgradeClient(ctx sdk.Context, clientID string, upgradedClient e
sdk.NewEvent(
types.EventTypeUpgradeClient,
sdk.NewAttribute(types.AttributeKeyClientID, clientID),
sdk.NewAttribute(types.AttributeKeyClientType, updatedClientState.ClientType()),
sdk.NewAttribute(types.AttributeKeyConsensusHeight, updatedClientState.GetLatestHeight().String()),
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
sdk.NewAttribute(types.AttributeKeyClientType, upgradedClient.ClientType()),
),
)

Expand Down
2 changes: 1 addition & 1 deletion modules/core/02-client/legacy/v100/solomachine.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func (cs ClientState) CheckSubstituteAndUpdateState(
func (cs ClientState) VerifyUpgradeAndUpdateState(
_ sdk.Context, _ codec.BinaryCodec, _ sdk.KVStore,
_ exported.ClientState, _ exported.ConsensusState, _, _ []byte,
) (exported.ClientState, exported.ConsensusState, error) {
) error {
panic("legacy solo machine is deprecated!")
}

Expand Down
3 changes: 2 additions & 1 deletion modules/core/exported/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ type ClientState interface {
// height of the current revision is somehow encoded in the proof verification process.
// This is to ensure that no premature upgrades occur, since upgrade plans committed to by the counterparty
// may be cancelled or modified before the last planned height.
// If the upgrade is verified, the upgraded client and consensus states must be set in the client store.
VerifyUpgradeAndUpdateState(
ctx sdk.Context,
cdc codec.BinaryCodec,
Expand All @@ -78,7 +79,7 @@ type ClientState interface {
newConsState ConsensusState,
proofUpgradeClient,
proofUpgradeConsState []byte,
) (ClientState, ConsensusState, error)
) error
// Utility function that zeroes out any client customizable fields in client state
// Ledger enforced fields are maintained while all custom fields are zero values
// Used to verify upgrades
Expand Down
4 changes: 2 additions & 2 deletions modules/light-clients/06-solomachine/types/client_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,8 @@ func (cs ClientState) ExportMetadata(_ sdk.KVStore) []exported.GenesisMetadata {
func (cs ClientState) VerifyUpgradeAndUpdateState(
_ sdk.Context, _ codec.BinaryCodec, _ sdk.KVStore,
_ exported.ClientState, _ exported.ConsensusState, _, _ []byte,
) (exported.ClientState, exported.ConsensusState, error) {
return nil, nil, sdkerrors.Wrap(clienttypes.ErrInvalidUpgradeClient, "cannot upgrade solomachine client")
) error {
return sdkerrors.Wrap(clienttypes.ErrInvalidUpgradeClient, "cannot upgrade solomachine client")
}

// VerifyClientState verifies a proof of the client state of the running chain
Expand Down
7 changes: 7 additions & 0 deletions modules/light-clients/07-tendermint/types/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,13 @@ var (
KeyIteration = []byte("/iterationKey")
)

// setClientState stores the client state
func setClientState(clientStore sdk.KVStore, cdc codec.BinaryCodec, clientState *ClientState) {
key := host.ClientStateKey()
val := clienttypes.MustMarshalClientState(cdc, clientState)
clientStore.Set(key, val)
}

// SetConsensusState stores the consensus state at the given height.
func SetConsensusState(clientStore sdk.KVStore, cdc codec.BinaryCodec, consensusState *ConsensusState, height exported.Height) {
key := host.ConsensusStateKey(height)
Expand Down
31 changes: 16 additions & 15 deletions modules/light-clients/07-tendermint/types/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,16 @@ func (cs ClientState) VerifyUpgradeAndUpdateState(
ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore,
upgradedClient exported.ClientState, upgradedConsState exported.ConsensusState,
proofUpgradeClient, proofUpgradeConsState []byte,
) (exported.ClientState, exported.ConsensusState, error) {
) error {
if len(cs.UpgradePath) == 0 {
return nil, nil, sdkerrors.Wrap(clienttypes.ErrInvalidUpgradeClient, "cannot upgrade client, no upgrade path set")
return sdkerrors.Wrap(clienttypes.ErrInvalidUpgradeClient, "cannot upgrade client, no upgrade path set")
}

// last height of current counterparty chain must be client's latest height
lastHeight := cs.GetLatestHeight()

if !upgradedClient.GetLatestHeight().GT(lastHeight) {
return nil, nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidHeight, "upgraded client height %s must be at greater than current client height %s",
return sdkerrors.Wrapf(sdkerrors.ErrInvalidHeight, "upgraded client height %s must be at greater than current client height %s",
upgradedClient.GetLatestHeight(), lastHeight)
}

Expand All @@ -45,52 +45,52 @@ func (cs ClientState) VerifyUpgradeAndUpdateState(
// counterparty must also commit to the upgraded consensus state at a sub-path under the upgrade path specified
tmUpgradeClient, ok := upgradedClient.(*ClientState)
if !ok {
return nil, nil, sdkerrors.Wrapf(clienttypes.ErrInvalidClientType, "upgraded client must be Tendermint client. expected: %T got: %T",
return sdkerrors.Wrapf(clienttypes.ErrInvalidClientType, "upgraded client must be Tendermint client. expected: %T got: %T",
&ClientState{}, upgradedClient)
}
tmUpgradeConsState, ok := upgradedConsState.(*ConsensusState)
if !ok {
return nil, nil, sdkerrors.Wrapf(clienttypes.ErrInvalidConsensus, "upgraded consensus state must be Tendermint consensus state. expected %T, got: %T",
return sdkerrors.Wrapf(clienttypes.ErrInvalidConsensus, "upgraded consensus state must be Tendermint consensus state. expected %T, got: %T",
&ConsensusState{}, upgradedConsState)
}

// unmarshal proofs
var merkleProofClient, merkleProofConsState commitmenttypes.MerkleProof
if err := cdc.Unmarshal(proofUpgradeClient, &merkleProofClient); err != nil {
return nil, nil, sdkerrors.Wrapf(commitmenttypes.ErrInvalidProof, "could not unmarshal client merkle proof: %v", err)
return sdkerrors.Wrapf(commitmenttypes.ErrInvalidProof, "could not unmarshal client merkle proof: %v", err)
}
if err := cdc.Unmarshal(proofUpgradeConsState, &merkleProofConsState); err != nil {
return nil, nil, sdkerrors.Wrapf(commitmenttypes.ErrInvalidProof, "could not unmarshal consensus state merkle proof: %v", err)
return sdkerrors.Wrapf(commitmenttypes.ErrInvalidProof, "could not unmarshal consensus state merkle proof: %v", err)
}

// Must prove against latest consensus state to ensure we are verifying against latest upgrade plan
// This verifies that upgrade is intended for the provided revision, since committed client must exist
// at this consensus state
consState, err := GetConsensusState(clientStore, cdc, lastHeight)
if err != nil {
return nil, nil, sdkerrors.Wrap(err, "could not retrieve consensus state for lastHeight")
return sdkerrors.Wrap(err, "could not retrieve consensus state for lastHeight")
}

// Verify client proof
bz, err := cdc.MarshalInterface(upgradedClient)
if err != nil {
return nil, nil, sdkerrors.Wrapf(clienttypes.ErrInvalidClient, "could not marshal client state: %v", err)
return sdkerrors.Wrapf(clienttypes.ErrInvalidClient, "could not marshal client state: %v", err)
}
// construct clientState Merkle path
upgradeClientPath := constructUpgradeClientMerklePath(cs.UpgradePath, lastHeight)
if err := merkleProofClient.VerifyMembership(cs.ProofSpecs, consState.GetRoot(), upgradeClientPath, bz); err != nil {
return nil, nil, sdkerrors.Wrapf(err, "client state proof failed. Path: %s", upgradeClientPath.Pretty())
return sdkerrors.Wrapf(err, "client state proof failed. Path: %s", upgradeClientPath.Pretty())
}

// Verify consensus state proof
bz, err = cdc.MarshalInterface(upgradedConsState)
if err != nil {
return nil, nil, sdkerrors.Wrapf(clienttypes.ErrInvalidConsensus, "could not marshal consensus state: %v", err)
return sdkerrors.Wrapf(clienttypes.ErrInvalidConsensus, "could not marshal consensus state: %v", err)
}
// construct consensus state Merkle path
upgradeConsStatePath := constructUpgradeConsStateMerklePath(cs.UpgradePath, lastHeight)
if err := merkleProofConsState.VerifyMembership(cs.ProofSpecs, consState.GetRoot(), upgradeConsStatePath, bz); err != nil {
return nil, nil, sdkerrors.Wrapf(err, "consensus state proof failed. Path: %s", upgradeConsStatePath.Pretty())
return sdkerrors.Wrapf(err, "consensus state proof failed. Path: %s", upgradeConsStatePath.Pretty())
}

// Construct new client state and consensus state
Expand All @@ -104,7 +104,7 @@ func (cs ClientState) VerifyUpgradeAndUpdateState(
)

if err := newClientState.Validate(); err != nil {
return nil, nil, sdkerrors.Wrap(err, "updated client state failed basic validation")
return sdkerrors.Wrap(err, "updated client state failed basic validation")
}

// The new consensus state is merely used as a trusted kernel against which headers on the new
Expand All @@ -118,10 +118,11 @@ func (cs ClientState) VerifyUpgradeAndUpdateState(
tmUpgradeConsState.Timestamp, commitmenttypes.NewMerkleRoot([]byte(SentinelRoot)), tmUpgradeConsState.NextValidatorsHash,
)

// set metadata for this consensus state
setClientState(clientStore, cdc, newClientState)
SetConsensusState(clientStore, cdc, newConsState, newClientState.LatestHeight)
setConsensusMetadata(ctx, clientStore, tmUpgradeClient.LatestHeight)

return newClientState, newConsState, nil
return nil
}

// construct MerklePath for the committed client from upgradePath
Expand Down
11 changes: 6 additions & 5 deletions modules/light-clients/07-tendermint/types/upgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ func (suite *TendermintTestSuite) TestVerifyUpgrade() {
// Call ZeroCustomFields on upgraded clients to clear any client-chosen parameters in test-case upgradedClient
upgradedClient = upgradedClient.ZeroCustomFields()

clientState, consensusState, err := cs.VerifyUpgradeAndUpdateState(
err := cs.VerifyUpgradeAndUpdateState(
suite.chainA.GetContext(),
suite.cdc,
clientStore,
Expand All @@ -469,14 +469,15 @@ func (suite *TendermintTestSuite) TestVerifyUpgrade() {

if tc.expPass {
suite.Require().NoError(err, "verify upgrade failed on valid case: %s", tc.name)

clientState := suite.chainA.GetClientState(path.EndpointA.ClientID)
suite.Require().NotNil(clientState, "verify upgrade failed on valid case: %s", tc.name)

consensusState, found := suite.chainA.GetConsensusState(path.EndpointA.ClientID, clientState.GetLatestHeight())
suite.Require().NotNil(consensusState, "verify upgrade failed on valid case: %s", tc.name)
suite.Require().True(found)
} else {
suite.Require().Error(err, "verify upgrade passed on invalid case: %s", tc.name)
suite.Require().Nil(clientState, "verify upgrade passed on invalid case: %s", tc.name)

suite.Require().Nil(consensusState, "verify upgrade passed on invalid case: %s", tc.name)

}
}
}
4 changes: 2 additions & 2 deletions modules/light-clients/09-localhost/types/client_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,8 @@ func (cs ClientState) CheckSubstituteAndUpdateState(
func (cs ClientState) VerifyUpgradeAndUpdateState(
_ sdk.Context, _ codec.BinaryCodec, _ sdk.KVStore,
_ exported.ClientState, _ exported.ConsensusState, _, _ []byte,
) (exported.ClientState, exported.ConsensusState, error) {
return nil, nil, sdkerrors.Wrap(clienttypes.ErrInvalidUpgradeClient, "cannot upgrade localhost client")
) error {
return sdkerrors.Wrap(clienttypes.ErrInvalidUpgradeClient, "cannot upgrade localhost client")
}

// VerifyClientState verifies that the localhost client state is stored locally
Expand Down