fix: guarentee proof availiability for channel upgrades (#5638)

* fix: add counterparty upgrade sequence to MsgChannelUpgradeOpen and handle edge case of counterparty initiating upgrade in the same block

* docs: add sufficient documentation for accidental breakage in the opening handshake

* lint lint lint

* test: add additional test case

* fix: proto numbering

* change error message

---------

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
(cherry picked from commit f67d60b)

# Conflicts:
#	docs/docs/01-ibc/06-channel-upgrades.md

modules/core/04-channel/keeper/handshake.go

@@ -336,6 +336,8 @@ func (k Keeper) ChanOpenConfirm(
 		counterpartyHops, channel.Version,
 	)
 
+	// NOTE: If the counterparty has initialized an upgrade in the same block as performing the
+	// ACK handshake step, this channel end will be incapable of opening.
 	return k.connectionKeeper.VerifyChannelState(
 		ctx, connectionEnd, proofHeight, ackProof,
 		channel.Counterparty.PortId, channel.Counterparty.ChannelId,

modules/core/04-channel/keeper/upgrade.go

@@ -473,6 +473,7 @@ func (k Keeper) ChanUpgradeOpen(
 	portID,
 	channelID string,
 	counterpartyChannelState types.State,
+	counterpartyUpgradeSequence uint64,
 	channelProof []byte,
 	proofHeight clienttypes.Height,
 ) error {
@@ -511,13 +512,21 @@ func (k Keeper) ChanUpgradeOpen(
 			return errorsmod.Wrapf(connectiontypes.ErrInvalidConnectionState, "connection state is not OPEN (got %s)", connectiontypes.State(upgradeConnection.GetState()).String())
 		}
 
+		// The counterparty upgrade sequence must be greater than or equal to
+		// the channel upgrade sequence. It should normally be equivalent, but
+		// in the unlikely case a new upgrade is initiated after it reopens,
+		// then the upgrade sequence will be greater than our upgrade sequence.
+		if counterpartyUpgradeSequence < channel.UpgradeSequence {
+			return errorsmod.Wrapf(types.ErrInvalidUpgradeSequence, "counterparty channel upgrade sequence (%d) must be greater than or equal to current upgrade sequence (%d)", counterpartyUpgradeSequence, channel.UpgradeSequence)
+		}
+
 		counterpartyChannel = types.Channel{
 			State:           types.OPEN,
 			Ordering:        upgrade.Fields.Ordering,
 			ConnectionHops:  []string{upgradeConnection.GetCounterparty().GetConnectionID()},
 			Counterparty:    types.NewCounterparty(portID, channelID),
 			Version:         upgrade.Fields.Version,
-			UpgradeSequence: channel.UpgradeSequence,
+			UpgradeSequence: counterpartyUpgradeSequence,
 		}
 
 	case types.FLUSHCOMPLETE:

modules/core/04-channel/keeper/upgrade_test.go

@@ -1187,6 +1187,29 @@ func (suite *KeeperTestSuite) TestChanUpgradeOpen() {
 			},
 			nil,
 		},
+		{
+			"success: counterparty initiated new upgrade after opening",
+			func() {
+				// create reason to upgrade
+				path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Version = mock.UpgradeVersion + "additional upgrade"
+
+				err := path.EndpointB.ChanUpgradeInit()
+				suite.Require().NoError(err)
+
+				err = path.EndpointA.UpdateClient()
+				suite.Require().NoError(err)
+			},
+			nil,
+		},
+		{
+			"success: counterparty upgrade sequence is incorrect",
+			func() {
+				counterpartyCh := path.EndpointB.GetChannel()
+				counterpartyCh.UpgradeSequence--
+				path.EndpointB.SetChannel(counterpartyCh)
+			},
+			types.ErrInvalidUpgradeSequence,
+		},
 		{
 			"channel not found",
 			func() {
@@ -1273,7 +1296,7 @@ func (suite *KeeperTestSuite) TestChanUpgradeOpen() {
 
 			err = suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.ChanUpgradeOpen(
 				suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID,
-				path.EndpointB.GetChannel().State, channelProof, proofHeight,
+				path.EndpointB.GetChannel().State, path.EndpointB.GetChannel().UpgradeSequence, channelProof, proofHeight,
 			)
 
 			if tc.expError == nil {

modules/core/04-channel/types/msgs.go

@@ -724,17 +724,19 @@ func NewMsgChannelUpgradeOpen(
 	portID,
 	channelID string,
 	counterpartyChannelState State,
+	counterpartyUpgradeSequence uint64,
 	channelProof []byte,
 	proofHeight clienttypes.Height,
 	signer string,
 ) *MsgChannelUpgradeOpen {
 	return &MsgChannelUpgradeOpen{
-		PortId:                   portID,
-		ChannelId:                channelID,
-		CounterpartyChannelState: counterpartyChannelState,
-		ProofChannel:             channelProof,
-		ProofHeight:              proofHeight,
-		Signer:                   signer,
+		PortId:                      portID,
+		ChannelId:                   channelID,
+		CounterpartyChannelState:    counterpartyChannelState,
+		CounterpartyUpgradeSequence: counterpartyUpgradeSequence,
+		ProofChannel:                channelProof,
+		ProofHeight:                 proofHeight,
+		Signer:                      signer,
 	}
 }
 
@@ -756,6 +758,10 @@ func (msg MsgChannelUpgradeOpen) ValidateBasic() error {
 		return errorsmod.Wrapf(ErrInvalidChannelState, "expected channel state to be one of: [%s, %s], got: %s", FLUSHCOMPLETE, OPEN, msg.CounterpartyChannelState)
 	}
 
+	if msg.CounterpartyUpgradeSequence == 0 {
+		return errorsmod.Wrap(ErrInvalidUpgradeSequence, "counterparty upgrade sequence must be non-zero")
+	}
+
 	_, err := sdk.AccAddressFromBech32(msg.Signer)
 	if err != nil {
 		return errorsmod.Wrapf(ibcerrors.ErrInvalidAddress, "string could not be parsed as address: %v", err)

modules/core/04-channel/types/msgs_test.go

@@ -1575,6 +1575,14 @@ func (suite *TypesTestSuite) TestMsgChannelUpgradeOpenValidateBasic() {
 			},
 			errorsmod.Wrapf(types.ErrInvalidChannelState, "expected channel state to be one of: [%s, %s], got: %s", types.FLUSHCOMPLETE, types.OPEN, types.CLOSED),
 		},
+		{
+			"invalid counterparty upgrade seqeuence",
+			func() {
+				msg.CounterpartyUpgradeSequence = 0
+			},
+			errorsmod.Wrapf(types.ErrInvalidUpgradeSequence, "counterparty upgrade sequence must be non-zero"),
+		},
+
 		{
 			"cannot submit an empty channel proof",
 			func() {
@@ -1596,7 +1604,7 @@ func (suite *TypesTestSuite) TestMsgChannelUpgradeOpenValidateBasic() {
 		suite.Run(tc.name, func() {
 			msg = types.NewMsgChannelUpgradeOpen(
 				ibctesting.MockPort, ibctesting.FirstChannelID,
-				types.FLUSHCOMPLETE, suite.proof,
+				types.FLUSHCOMPLETE, 1, suite.proof,
 				height, addr,
 			)