Merge pull request #147 from siburu/fix-upg-init-safety

Make `tx channel-upgrade init` safer

cmd/tx.go

@@ -254,16 +254,9 @@ func channelUpgradeInitCmd(ctx *config.Context) *cobra.Command {
 			}
 
 			// check cp state
-			if unsafe, err := cmd.Flags().GetBool(flagUnsafe); err != nil {
+			permitUnsafe, err := cmd.Flags().GetBool(flagUnsafe)
+			if err != nil {
 				return err
-			} else if !unsafe {
-				if height, err := cp.LatestHeight(); err != nil {
-					return err
-				} else if chann, err := cp.QueryChannel(core.NewQueryContext(cmd.Context(), height)); err != nil {
-					return err
-				} else if state := chann.Channel.State; state >= chantypes.FLUSHING && state <= chantypes.FLUSHCOMPLETE {
-					return fmt.Errorf("stop channel upgrade initialization because the counterparty is in %v state", state)
-				}
 			}
 
 			// get ordering from flags
@@ -286,11 +279,16 @@ func channelUpgradeInitCmd(ctx *config.Context) *cobra.Command {
 				return err
 			}
 
-			return core.InitChannelUpgrade(chain, chantypes.UpgradeFields{
-				Ordering:       ordering,
-				ConnectionHops: connHops,
-				Version:        version,
-			})
+			return core.InitChannelUpgrade(
+				chain,
+				cp,
+				chantypes.UpgradeFields{
+					Ordering:       ordering,
+					ConnectionHops: connHops,
+					Version:        version,
+				},
+				permitUnsafe,
+			)
 		},
 	}
 

core/channel-upgrade.go

@@ -75,10 +75,40 @@ func (action UpgradeAction) String() string {
 }
 
 // InitChannelUpgrade builds `MsgChannelUpgradeInit` based on the specified UpgradeFields and sends it to the specified chain.
-func InitChannelUpgrade(chain *ProvableChain, upgradeFields chantypes.UpgradeFields) error {
+func InitChannelUpgrade(chain, cp *ProvableChain, upgradeFields chantypes.UpgradeFields, permitUnsafe bool) error {
 	logger := GetChannelLogger(chain.Chain)
 	defer logger.TimeTrack(time.Now(), "InitChannelUpgrade")
 
+	if h, err := chain.LatestHeight(); err != nil {
+		logger.Error("failed to get the latest height", err)
+		return err
+	} else if cpH, err := cp.LatestHeight(); err != nil {
+		logger.Error("failed to get the latest height of the counterparty chain", err)
+		return err
+	} else if chann, cpChann, err := QueryChannelPair(
+		NewQueryContext(context.TODO(), h),
+		NewQueryContext(context.TODO(), cpH),
+		chain,
+		cp,
+		false,
+	); err != nil {
+		logger.Error("failed to query for the channel pair", err)
+		return err
+	} else if chann.Channel.State != chantypes.OPEN || cpChann.Channel.State != chantypes.OPEN {
+		logger = &log.RelayLogger{Logger: logger.With(
+			"channel_state", chann.Channel.State,
+			"cp_channel_state", cpChann.Channel.State,
+		)}
+
+		if permitUnsafe {
+			logger.Info("unsafe channel upgrade is permitted")
+		} else {
+			err := errors.New("unsafe channel upgrade initialization")
+			logger.Error("unsafe channel upgrade is not permitted", err)
+			return err
+		}
+	}
+
 	addr, err := chain.GetAddress()
 	if err != nil {
 		logger.Error("failed to get address", err)

tests/cases/docker-compose-test.yaml

@@ -20,6 +20,8 @@ services:
       retries: 5
     networks:
       - *network-common
+    environment:
+      IBC_CHANNEL_UPGRADE_TIMEOUT: 20000000000
   tendermint-chain1: &tendermint-chain1
     container_name: tendermint-chain1
     image: tendermint-chain1:${TAG}
@@ -35,6 +37,8 @@ services:
       retries: 5
     networks:
       - *network-common
+    environment:
+      IBC_CHANNEL_UPGRADE_TIMEOUT: 20000000000
   tendermint-chain0-mock:
     <<: *tendermint-chain0
     container_name: tendermint-chain0-mock

tests/chains/tendermint/scripts/entrypoint.sh

@@ -1,6 +1,5 @@
 #!/bin/sh
 
 export IBC_AUTHORITY=`jq -r .address /root/${CHAINDIR}/${CHAINID}/key_seed.json`
-export IBC_CHANNEL_UPGRADE_TIMEOUT=20000000000 # 20sec = 20_000_000_000nsec
 
 simd --home /root/${CHAINDIR}/${CHAINID} start --pruning=nothing --grpc.address="0.0.0.0:${GRPCPORT}"