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

Cherry-pick: Add MsgRegisterCounterparty Struct and Handler from ibc-lite #6982

Merged
merged 20 commits into from
Jul 31, 2024
Merged
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
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
Next Next commit
feat(lite): counterparty client logic (#6307)
* imp: added counterparty client store

* imp: added provide counterparty to proto

* imp: ran 'make proto-all'

* imp: added logic to counterparty client

* imp: fix proto

* imp: fix proto

* imp: ran 'make proto-all'

* feat: finished counterparty client logic
srdtrk authored and sangier committed Jul 29, 2024
commit dea6dcd1d2a39db9cc9f38e444a8d151a9057ab2
25 changes: 25 additions & 0 deletions modules/core/02-client/keeper/keeper.go
Original file line number Diff line number Diff line change
@@ -322,6 +322,31 @@ func (k *Keeper) GetSelfConsensusState(ctx sdk.Context, height exported.Height)
return k.consensusHost.GetSelfConsensusState(ctx, height)
}

// GetCounterparty returns the counterparty client's identifier for a given client identifier.
// If the counterparty client identifier is not found, an empty string is returned.
func (k *Keeper) GetCounterparty(ctx sdk.Context, clientID string) string {
return string(k.ClientStore(ctx, clientID).Get(host.CounterpartyKey()))
}

// SetCounterparty sets the counterparty client's identifier for a given client identifier.
func (k *Keeper) SetCounterparty(ctx sdk.Context, clientID, counterpartyClientID string) {
k.ClientStore(ctx, clientID).Set(host.CounterpartyKey(), []byte(counterpartyClientID))
}

// GetCreator returns the creator of the client.
func (k *Keeper) GetCreator(ctx sdk.Context, clientID string) string {
// the creator key is imported from types instead of host because
// the creator key is not a part of the ics-24 host specification
return string(k.ClientStore(ctx, clientID).Get([]byte(types.CreatorKey)))
}

// SetCreator sets the creator of the client.
func (k *Keeper) SetCreator(ctx sdk.Context, clientID, creator string) {
// the creator key is imported from types instead of host because
// the creator key is not a part of the ics-24 host specification
k.ClientStore(ctx, clientID).Set([]byte(types.CreatorKey), []byte(creator))
}

// ValidateSelfClient validates the client parameters for a client of the running chain.
// This function is only used to validate the client state the counterparty stores for this chain.
// NOTE: If the client type is not of type Tendermint then delegate to a custom client validator function.
11 changes: 11 additions & 0 deletions modules/core/02-client/keeper/keeper_test.go
Original file line number Diff line number Diff line change
@@ -129,6 +129,17 @@ func (suite *KeeperTestSuite) TestSetClientState() {
suite.Require().Equal(clientState, retrievedState, "Client states are not equal")
}

func (suite *KeeperTestSuite) TestSetCounterparty() {
counterpartyClientID := suite.keeper.GetCounterparty(suite.ctx, testClientID)
suite.Require().Equal("", counterpartyClientID, "Counterparty client ID is not empty")

testCounterpartyID := "counterparty-1"
suite.keeper.SetCounterparty(suite.ctx, testClientID, testCounterpartyID)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: could add a delete here to test that as well

counterpartyClientID = suite.keeper.GetCounterparty(suite.ctx, testClientID)
suite.Require().Equal(testCounterpartyID, counterpartyClientID, "Counterparty client ID is not equal")
}

func (suite *KeeperTestSuite) TestSetClientConsensusState() {
suite.keeper.SetClientConsensusState(suite.ctx, testClientID, testClientHeight, suite.consensusState)

1 change: 1 addition & 0 deletions modules/core/02-client/types/errors.go
Original file line number Diff line number Diff line change
@@ -38,4 +38,5 @@ var (
ErrFailedNonMembershipVerification = errorsmod.Register(SubModuleName, 31, "non-membership verification failed")
ErrRouteNotFound = errorsmod.Register(SubModuleName, 32, "light client module route not found")
ErrClientTypeNotSupported = errorsmod.Register(SubModuleName, 33, "client type not supported")
ErrInvalidCounterparty = errorsmod.Register(SubModuleName, 34, "invalid counterparty identifier")
)
3 changes: 3 additions & 0 deletions modules/core/02-client/types/keys.go
Original file line number Diff line number Diff line change
@@ -32,6 +32,9 @@ const (
// AllowAllClients is the value that if set in AllowedClients param
// would allow any wired up light client modules to be allowed
AllowAllClients = "*"

// CreatorKey is the key used to store the client creator in the client store
CreatorKey = "creator"
)

// FormatClientIdentifier returns the client identifier with the sequence appended.
30 changes: 30 additions & 0 deletions modules/core/02-client/types/msgs.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package types

import (
"strings"

errorsmod "cosmossdk.io/errors"
upgradetypes "cosmossdk.io/x/upgrade/types"

@@ -20,6 +22,7 @@ var (
_ sdk.Msg = (*MsgUpdateParams)(nil)
_ sdk.Msg = (*MsgIBCSoftwareUpgrade)(nil)
_ sdk.Msg = (*MsgRecoverClient)(nil)
_ sdk.Msg = (*MsgProvideCounterparty)(nil)

_ sdk.HasValidateBasic = (*MsgCreateClient)(nil)
_ sdk.HasValidateBasic = (*MsgUpdateClient)(nil)
@@ -28,6 +31,7 @@ var (
_ sdk.HasValidateBasic = (*MsgUpdateParams)(nil)
_ sdk.HasValidateBasic = (*MsgIBCSoftwareUpgrade)(nil)
_ sdk.HasValidateBasic = (*MsgRecoverClient)(nil)
_ sdk.HasValidateBasic = (*MsgProvideCounterparty)(nil)

_ codectypes.UnpackInterfacesMessage = (*MsgCreateClient)(nil)
_ codectypes.UnpackInterfacesMessage = (*MsgUpdateClient)(nil)
@@ -265,6 +269,32 @@ func (msg *MsgRecoverClient) ValidateBasic() error {
return nil
}

// NewMsgProvideCounterparty creates a new MsgProvideCounterparty instance
func NewMsgProvideCounterparty(signer, clientID, counterpartyID string) *MsgProvideCounterparty {
return &MsgProvideCounterparty{
Signer: signer,
ClientId: clientID,
CounterpartyId: counterpartyID,
}
}

// ValidateBasic performs basic checks on a MsgProvideCounterparty.
func (msg *MsgProvideCounterparty) ValidateBasic() error {
if _, err := sdk.AccAddressFromBech32(msg.Signer); err != nil {
return errorsmod.Wrapf(ibcerrors.ErrInvalidAddress, "string could not be parsed as address: %v", err)
}

if err := host.ClientIdentifierValidator(msg.ClientId); err != nil {
return err
}

if strings.TrimSpace(msg.CounterpartyId) == "" {
return errorsmod.Wrapf(ErrInvalidCounterparty, "counterparty client id cannot be empty")
}

return nil
}

// NewMsgIBCSoftwareUpgrade creates a new MsgIBCSoftwareUpgrade instance
func NewMsgIBCSoftwareUpgrade(signer string, plan upgradetypes.Plan, upgradedClientState exported.ClientState) (*MsgIBCSoftwareUpgrade, error) {
anyClient, err := PackClientState(upgradedClientState)
577 changes: 520 additions & 57 deletions modules/core/02-client/types/tx.pb.go

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions modules/core/24-host/client_keys.go
Original file line number Diff line number Diff line change
@@ -40,3 +40,7 @@ func FullConsensusStateKey(clientID string, height exported.Height) []byte {
func ConsensusStateKey(height exported.Height) []byte {
return []byte(ConsensusStatePath(height))
}

func CounterpartyKey() []byte {
return []byte(KeyCounterparty)
}
7 changes: 7 additions & 0 deletions modules/core/24-host/client_paths.go
Original file line number Diff line number Diff line change
@@ -12,6 +12,7 @@ var KeyClientStorePrefix = []byte("clients")
const (
KeyClientState = "clientState"
KeyConsensusStatePrefix = "consensusStates"
KeyCounterparty = "counterparty"
)

// FullClientPath returns the full path of a specific client path in the format:
@@ -42,6 +43,12 @@ func FullConsensusStatePath(clientID string, height exported.Height) string {
return FullClientPath(clientID, ConsensusStatePath(height))
}

// FullCounterpartyPath takes a client identifier and returns a Path under which to
// store the counterparty of a client.
func FullCounterpartyPath(clientID string) string {
return FullClientPath(clientID, KeyCounterparty)
}

// ConsensusStatePath returns the suffix store key for the consensus state at a
// particular height stored in a client prefixed store.
func ConsensusStatePath(height exported.Height) string {
25 changes: 24 additions & 1 deletion modules/core/keeper/msg_server.go
Original file line number Diff line number Diff line change
@@ -3,6 +3,7 @@ package keeper
import (
"context"
"errors"
"strings"

errorsmod "cosmossdk.io/errors"

@@ -43,7 +44,13 @@ func (k *Keeper) CreateClient(goCtx context.Context, msg *clienttypes.MsgCreateC
return nil, err
}

return &clienttypes.MsgCreateClientResponse{ClientId: clientID}, nil
k.ClientKeeper.SetCreator(ctx, clientID, msg.Signer)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove this logic since we can do it in a MultiMsg

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AdityaSripal the creator setting k.ClientKeeper.SetCreator(ctx, clientID, msg.Signer) should be removed too?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe so? Should only remove the counterparty being set, creator is needed for the provide msg (but we can delete it from state there, possibly in follow up)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, sorry we should keep this logic my bad.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should also be able to add an assetion in the test func for this handler that asserts creator is set as expected

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems we only have indirect testing for this? see no TestCreateClient in msg_server_test.go


if strings.TrimSpace(msg.CounterpartyId) != "" {
k.ClientKeeper.SetCounterparty(ctx, clientID, msg.CounterpartyId)
}

return &clienttypes.MsgCreateClientResponse{}, nil
}

// UpdateClient defines a rpc handler method for MsgUpdateClient.
@@ -135,6 +142,22 @@ func (k *Keeper) IBCSoftwareUpgrade(goCtx context.Context, msg *clienttypes.MsgI
return &clienttypes.MsgIBCSoftwareUpgradeResponse{}, nil
}

// ProvideCounterparty defines a rpc handler method for MsgProvideCounterparty.
func (k *Keeper) ProvideCounterparty(goCtx context.Context, msg *clienttypes.MsgProvideCounterparty) (*clienttypes.MsgProvideCounterpartyResponse, error) {
ctx := sdk.UnwrapSDKContext(goCtx)

if creator := k.ClientKeeper.GetCreator(ctx, msg.ClientId); creator != msg.Signer {
return nil, errorsmod.Wrapf(ibcerrors.ErrUnauthorized, "expected %s, got %s", creator, msg.Signer)
}

if counterparty := k.ClientKeeper.GetCounterparty(ctx, msg.ClientId); counterparty != "" {
return nil, errorsmod.Wrapf(clienttypes.ErrInvalidCounterparty, "counterparty already exists for client %s", msg.ClientId)
}
k.ClientKeeper.SetCounterparty(ctx, msg.ClientId, msg.CounterpartyId)

return &clienttypes.MsgProvideCounterpartyResponse{}, nil
}

// ConnectionOpenInit defines a rpc handler method for MsgConnectionOpenInit.
func (k *Keeper) ConnectionOpenInit(goCtx context.Context, msg *connectiontypes.MsgConnectionOpenInit) (*connectiontypes.MsgConnectionOpenInitResponse, error) {
ctx := sdk.UnwrapSDKContext(goCtx)
24 changes: 24 additions & 0 deletions proto/ibc/core/client/v1/tx.proto
Original file line number Diff line number Diff line change
@@ -29,6 +29,9 @@ service Msg {
// RecoverClient defines a rpc handler method for MsgRecoverClient.
rpc RecoverClient(MsgRecoverClient) returns (MsgRecoverClientResponse);

// ProvideCounterparty defines a rpc handler method for MsgProvideCounterparty.
rpc ProvideCounterparty(MsgProvideCounterparty) returns (MsgProvideCounterpartyResponse);

// IBCSoftwareUpgrade defines a rpc handler method for MsgIBCSoftwareUpgrade.
rpc IBCSoftwareUpgrade(MsgIBCSoftwareUpgrade) returns (MsgIBCSoftwareUpgradeResponse);

@@ -49,6 +52,8 @@ message MsgCreateClient {
google.protobuf.Any consensus_state = 2;
// signer address
string signer = 3;
// optional counterparty client identifier
string counterparty_id = 4;
}

// MsgCreateClientResponse defines the Msg/CreateClient response type.
@@ -140,6 +145,25 @@ message MsgRecoverClient {
// MsgRecoverClientResponse defines the Msg/RecoverClient response type.
message MsgRecoverClientResponse {}

// MsgProvideCounterparty defines the message used to provide the counterparty client
// identifier. Can only be invoked one time by the signer of MsgCreateClient if the counterparty
// client identifier was not provided in the initial MsgCreateClient message.
message MsgProvideCounterparty {
option (cosmos.msg.v1.signer) = "signer";

option (gogoproto.goproto_getters) = false;

// client unique identifier
string client_id = 1;
// counterparty client identifier
string counterparty_id = 2;
// signer address
string signer = 3;
}

// MsgProvideCounterpartyResponse defines the Msg/ProvideCounterparty response type.
message MsgProvideCounterpartyResponse {}

// MsgIBCSoftwareUpgrade defines the message used to schedule an upgrade of an IBC client using a v1 governance proposal
message MsgIBCSoftwareUpgrade {
option (cosmos.msg.v1.signer) = "signer";