Skip to content

Commit

Permalink
Flexible handshake followup (#7454)
Browse files Browse the repository at this point in the history 
* rename provedID

rename provedID to counterpartyChosenID for connection and channel. Update if statement in handshake and tests. Ref: #7439 (comment)

* update docs

Co-authored-by: Christopher Goes <[email protected]>
  • Loading branch information
@colin-axner @cwgoes
colin-axner and cwgoes authored Oct 5, 2020
1 parent a84f4fb commit 6fa8330
Show file tree
Hide file tree
Showing 14 changed files with 332 additions and 320 deletions.
14 changes: 7 additions & 7 deletions proto/ibc/core/channel/v1/channel.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@ message MsgChannelOpenTry {
option (gogoproto.equal) = false;
option (gogoproto.goproto_getters) = false;

string port_id = 1 [(gogoproto.moretags) = "yaml:\"port_id\""];
string channel_id = 2 [(gogoproto.moretags) = "yaml:\"channel_id\""];
string proved_channel_id = 3 [(gogoproto.moretags) = "yaml:\"proved_channel_id\""];
Channel channel = 4 [(gogoproto.nullable) = false];
string counterparty_version = 5 [(gogoproto.moretags) = "yaml:\"counterparty_version\""];
bytes proof_init = 6 [(gogoproto.moretags) = "yaml:\"proof_init\""];
ibc.core.client.v1.Height proof_height = 7
string port_id = 1 [(gogoproto.moretags) = "yaml:\"port_id\""];
string desired_channel_id = 2 [(gogoproto.moretags) = "yaml:\"desired_channel_id\""];
string counterparty_chosen_channel_id = 3 [(gogoproto.moretags) = "yaml:\"counterparty_chosen_channel_id\""];
Channel channel = 4 [(gogoproto.nullable) = false];
string counterparty_version = 5 [(gogoproto.moretags) = "yaml:\"counterparty_version\""];
bytes proof_init = 6 [(gogoproto.moretags) = "yaml:\"proof_init\""];
ibc.core.client.v1.Height proof_height = 7
[(gogoproto.moretags) = "yaml:\"proof_height\"", (gogoproto.nullable) = false];
string signer = 8;
}
Expand Down
10 changes: 5 additions & 5 deletions proto/ibc/core/connection/v1/connection.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ message MsgConnectionOpenTry {
option (gogoproto.equal) = false;
option (gogoproto.goproto_getters) = false;

string client_id = 1 [(gogoproto.moretags) = "yaml:\"client_id\""];
string connection_id = 2 [(gogoproto.moretags) = "yaml:\"connection_id\""];
string proved_id = 3 [(gogoproto.moretags) = "yaml:\"proved_id\""];
google.protobuf.Any client_state = 4 [(gogoproto.moretags) = "yaml:\"client_state\""];
Counterparty counterparty = 5 [(gogoproto.nullable) = false];
string client_id = 1 [(gogoproto.moretags) = "yaml:\"client_id\""];
string desired_connection_id = 2 [(gogoproto.moretags) = "yaml:\"desired_connection_id\""];
string counterparty_chosen_connection_id = 3 [(gogoproto.moretags) = "yaml:\"counterparty_chosen_connection_id\""];
google.protobuf.Any client_state = 4 [(gogoproto.moretags) = "yaml:\"client_state\""];
Counterparty counterparty = 5 [(gogoproto.nullable) = false];
repeated string counterparty_versions = 6 [(gogoproto.moretags) = "yaml:\"counterparty_versions\""];
ibc.core.client.v1.Height proof_height = 7
[(gogoproto.moretags) = "yaml:\"proof_height\"", (gogoproto.nullable) = false];
Expand Down
4 changes: 2 additions & 2 deletions x/ibc/core/03-connection/handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func HandleMsgConnectionOpenTry(ctx sdk.Context, k keeper.Keeper, msg *types.Msg
}

if err := k.ConnOpenTry(
ctx, msg.ConnectionId, msg.ProvedId, msg.Counterparty, msg.ClientId, targetClient,
ctx, msg.DesiredConnectionId, msg.CounterpartyChosenConnectionId, msg.Counterparty, msg.ClientId, targetClient,
msg.CounterpartyVersions, msg.ProofInit, msg.ProofClient, msg.ProofConsensus,
msg.ProofHeight, msg.ConsensusHeight,
); err != nil {
Expand All @@ -53,7 +53,7 @@ func HandleMsgConnectionOpenTry(ctx sdk.Context, k keeper.Keeper, msg *types.Msg
ctx.EventManager().EmitEvents(sdk.Events{
sdk.NewEvent(
types.EventTypeConnectionOpenTry,
sdk.NewAttribute(types.AttributeKeyConnectionID, msg.ConnectionId),
sdk.NewAttribute(types.AttributeKeyConnectionID, msg.DesiredConnectionId),
sdk.NewAttribute(types.AttributeKeyClientID, msg.ClientId),
sdk.NewAttribute(types.AttributeKeyCounterpartyClientID, msg.Counterparty.ClientId),
sdk.NewAttribute(types.AttributeKeyCounterpartyConnectionID, msg.Counterparty.ConnectionId),
Expand Down
32 changes: 19 additions & 13 deletions x/ibc/core/03-connection/keeper/handshake.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ func (k Keeper) ConnOpenInit(
// - Identifiers are checked on msg validation
func (k Keeper) ConnOpenTry(
ctx sdk.Context,
connectionID, // desiredIdentifier
provedID string, // provedIdentifier
desiredConnectionID, // desiredIdentifier
counterpartyChosenConnectionID string, // counterparty used this identifier in proof
counterparty types.Counterparty, // counterpartyConnectionIdentifier, counterpartyPrefix and counterpartyClientIdentifier
clientID string, // clientID of chainA
clientState exported.ClientState, // clientState that chainA has for chainB
Expand Down Expand Up @@ -86,24 +86,27 @@ func (k Keeper) ConnOpenTry(
return clienttypes.ErrSelfConsensusStateNotFound
}

if provedID != connectionID && provedID != "" {
// If the connection id chosen for this connection end by the counterparty is empty then
// flexible connection identifier selection is allowed by using the desired connection id.
// Otherwise the desiredConnectionID must match the counterpartyChosenConnectionID.
if counterpartyChosenConnectionID != "" && counterpartyChosenConnectionID != desiredConnectionID {
return sdkerrors.Wrapf(
types.ErrInvalidConnectionIdentifier,
"proved identifier (%s) must equal connection identifier (%s) or be empty", provedID, connectionID,
"counterparty chosen connection ID (%s) must be empty or equal to the desired connection ID (%s)", counterpartyChosenConnectionID, desiredConnectionID,
)
}

// expectedConnection defines Chain A's ConnectionEnd
// NOTE: chain A's counterparty is chain B (i.e where this code is executed)
prefix := k.GetCommitmentPrefix()
expectedCounterparty := types.NewCounterparty(clientID, provedID, commitmenttypes.NewMerklePrefix(prefix.Bytes()))
expectedCounterparty := types.NewCounterparty(clientID, counterpartyChosenConnectionID, commitmenttypes.NewMerklePrefix(prefix.Bytes()))
expectedConnection := types.NewConnectionEnd(types.INIT, counterparty.ClientId, expectedCounterparty, counterpartyVersions)

// If connection already exists for connectionID, ensure that the existing connection's
// If connection already exists for desiredConnectionID, ensure that the existing connection's
// counterparty is chainA and connection is on INIT stage.
// Check that existing connection versions for initialized connection is equal to compatible
// versions for this chain.
previousConnection, found := k.GetConnection(ctx, connectionID)
previousConnection, found := k.GetConnection(ctx, desiredConnectionID)
if found && !(previousConnection.State == types.INIT &&
previousConnection.Counterparty.ConnectionId == counterparty.ConnectionId &&
bytes.Equal(previousConnection.Counterparty.Prefix.Bytes(), counterparty.Prefix.Bytes()) &&
Expand Down Expand Up @@ -149,12 +152,12 @@ func (k Keeper) ConnOpenTry(
}

// store connection in chainB state
if err := k.addConnectionToClient(ctx, clientID, connectionID); err != nil {
return sdkerrors.Wrapf(err, "failed to add connection with ID %s to client with ID %s", connectionID, clientID)
if err := k.addConnectionToClient(ctx, clientID, desiredConnectionID); err != nil {
return sdkerrors.Wrapf(err, "failed to add connection with ID %s to client with ID %s", desiredConnectionID, clientID)
}

k.SetConnection(ctx, connectionID, connection)
k.Logger(ctx).Info(fmt.Sprintf("connection %s state updated: %s -> TRYOPEN ", connectionID, previousConnection.State))
k.SetConnection(ctx, desiredConnectionID, connection)
k.Logger(ctx).Info(fmt.Sprintf("connection %s state updated: %s -> TRYOPEN ", desiredConnectionID, previousConnection.State))
return nil
}

Expand Down Expand Up @@ -189,10 +192,13 @@ func (k Keeper) ConnOpenAck(
return sdkerrors.Wrap(types.ErrConnectionNotFound, connectionID)
}

if counterpartyConnectionID != connection.Counterparty.ConnectionId && connection.Counterparty.ConnectionId != "" {
// If the previously set connection end allowed for the counterparty to select its own
// connection identifier then we use the counterpartyConnectionID. Otherwise the
// counterpartyConnectionID must match the previously set counterparty connection ID.
if connection.Counterparty.ConnectionId != "" && counterpartyConnectionID != connection.Counterparty.ConnectionId {
return sdkerrors.Wrapf(
types.ErrInvalidConnectionIdentifier,
"counterparty connection identifier (%s) must be empty or equal to stored connection ID for counterparty (%s)", counterpartyConnectionID, connection.Counterparty.ConnectionId,
"counterparty connection identifier (%s) must be equal to stored connection ID for counterparty (%s)", counterpartyConnectionID, connection.Counterparty.ConnectionId,
)
}

Expand Down
12 changes: 6 additions & 6 deletions x/ibc/core/03-connection/keeper/handshake_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func (suite *KeeperTestSuite) TestConnOpenTry() {
// retrieve client state of chainA to pass as counterpartyClient
counterpartyClient = suite.chainA.GetClientState(clientA)
}, true},
{"success with empty provedID", func() {
{"success with empty counterpartyChosenConnectionID", func() {
clientA, clientB = suite.coordinator.SetupClients(suite.chainA, suite.chainB, ibctesting.Tendermint)
connA, _, err := suite.coordinator.ConnOpenInit(suite.chainA, suite.chainB, clientA, clientB)
suite.Require().NoError(err)
Expand All @@ -123,7 +123,7 @@ func (suite *KeeperTestSuite) TestConnOpenTry() {
// retrieve client state of chainA to pass as counterpartyClient
counterpartyClient = suite.chainA.GetClientState(clientA)
}, true},
{"invalid provedID", func() {
{"counterpartyChosenConnectionID does not match desiredConnectionID", func() {
clientA, clientB = suite.coordinator.SetupClients(suite.chainA, suite.chainB, ibctesting.Tendermint)
connA, _, err := suite.coordinator.ConnOpenInit(suite.chainA, suite.chainB, clientA, clientB)
suite.Require().NoError(err)
Expand Down Expand Up @@ -299,11 +299,11 @@ func (suite *KeeperTestSuite) TestConnOpenTry() {
connB := suite.chainB.GetFirstTestConnection(clientB, clientA)
counterparty := types.NewCounterparty(clientA, connA.ID, suite.chainA.GetPrefix())

// get provedID
var provedID string
// get counterpartyChosenConnectionID
var counterpartyChosenConnectionID string
connection, found := suite.chainA.App.IBCKeeper.ConnectionKeeper.GetConnection(suite.chainA.GetContext(), connA.ID)
if found {
provedID = connection.Counterparty.ConnectionId
counterpartyChosenConnectionID = connection.Counterparty.ConnectionId
}

connectionKey := host.KeyConnection(connA.ID)
Expand All @@ -321,7 +321,7 @@ func (suite *KeeperTestSuite) TestConnOpenTry() {
proofClient, _ := suite.chainA.QueryProof(clientKey)

err := suite.chainB.App.IBCKeeper.ConnectionKeeper.ConnOpenTry(
suite.chainB.GetContext(), connB.ID, provedID, counterparty, clientB, counterpartyClient,
suite.chainB.GetContext(), connB.ID, counterpartyChosenConnectionID, counterparty, clientB, counterpartyClient,
versions, proofInit, proofClient, proofConsensus,
proofHeight, consensusHeight,
)
Expand Down
Loading

0 comments on commit 6fa8330

Please sign in to comment.