Fix to ensure proposedConnectionHops of MsgChannelUpgradeTry matches existing upgrade's one

.gas-snapshot

@@ -1,20 +1,20 @@
 IBCMockAppTest:testHandshake() (gas: 4420488)
 IBCMockAppTest:testHandshakeBetweenDifferentPorts() (gas: 3334373)
 IBCMockAppTest:testPacketRelay() (gas: 13935239)
-IBCMockAppTest:testPacketTimeout() (gas: 4279263)
+IBCMockAppTest:testPacketTimeout() (gas: 4284263)
 IBCTest:testBenchmarkCreateMockClient() (gas: 233366)
 IBCTest:testBenchmarkLCUpdateMockClient() (gas: 62005)
 IBCTest:testBenchmarkRecvPacket() (gas: 158870)
 IBCTest:testBenchmarkSendPacket() (gas: 128432)
 IBCTest:testBenchmarkUpdateMockClient() (gas: 160229)
 IBCTest:testToUint128((uint64,uint64)) (runs: 256, μ: 947, ~: 947)
-TestICS02:testCreateClient() (gas: 36540928)
-TestICS02:testInvalidCreateClient() (gas: 36438147)
-TestICS02:testInvalidUpdateClient() (gas: 36437055)
-TestICS02:testRegisterClient() (gas: 36092721)
-TestICS02:testRegisterClientDuplicatedClientType() (gas: 36078030)
-TestICS02:testRegisterClientInvalidClientType() (gas: 36107491)
-TestICS02:testUpdateClient() (gas: 36605255)
+TestICS02:testCreateClient() (gas: 36576736)
+TestICS02:testInvalidCreateClient() (gas: 36473955)
+TestICS02:testInvalidUpdateClient() (gas: 36472863)
+TestICS02:testRegisterClient() (gas: 36128529)
+TestICS02:testRegisterClientDuplicatedClientType() (gas: 36113838)
+TestICS02:testRegisterClientInvalidClientType() (gas: 36143299)
+TestICS02:testUpdateClient() (gas: 36641063)
 TestICS03Handshake:testConnOpenAck() (gas: 1858230)
 TestICS03Handshake:testConnOpenConfirm() (gas: 2054143)
 TestICS03Handshake:testConnOpenInit() (gas: 1429838)
@@ -30,7 +30,7 @@ TestICS03Version:testPickVersion() (gas: 25327)
 TestICS03Version:testVerifyProposedVersion() (gas: 11777)
 TestICS03Version:testVerifySupportedFeature() (gas: 4153)
 TestICS04Handshake:testBindPort() (gas: 124350)
-TestICS04Handshake:testChanClose() (gas: 12938854)
+TestICS04Handshake:testChanClose() (gas: 12973854)
 TestICS04Handshake:testChanOpenAck() (gas: 3459404)
 TestICS04Handshake:testChanOpenConfirm() (gas: 3770673)
 TestICS04Handshake:testChanOpenInit() (gas: 2543524)
@@ -40,34 +40,34 @@ TestICS04Handshake:testInvalidChanOpenConfirm() (gas: 2517338)
 TestICS04Handshake:testInvalidChanOpenInit() (gas: 1760558)
 TestICS04Handshake:testInvalidChanOpenTry() (gas: 1775528)
 TestICS04Packet:testAcknowledgementPacket() (gas: 3351116)
-TestICS04Packet:testInvalidSendPacket() (gas: 3551583)
-TestICS04Packet:testRecvPacket() (gas: 10996942)
+TestICS04Packet:testInvalidSendPacket() (gas: 3579083)
+TestICS04Packet:testRecvPacket() (gas: 11006942)
 TestICS04Packet:testRecvPacketTimeoutHeight() (gas: 3259727)
-TestICS04Packet:testRecvPacketTimeoutTimestamp() (gas: 3279103)
+TestICS04Packet:testRecvPacketTimeoutTimestamp() (gas: 3284103)
 TestICS04Packet:testSendPacket() (gas: 6412442)
 TestICS04Packet:testTimeoutOnClose() (gas: 3553289)
-TestICS04Upgrade:testUpgradeAuthorityCancel() (gas: 46742335)
+TestICS04Upgrade:testUpgradeAuthorityCancel() (gas: 46744545)
 TestICS04Upgrade:testUpgradeCannotCancelWithOldErrorReceipt() (gas: 3456630)
-TestICS04Upgrade:testUpgradeCannotRecvNextUpgradePacket() (gas: 5266005)
-TestICS04Upgrade:testUpgradeCounterpartyAdvanceNextSequenceBeforeOpen() (gas: 5236258)
-TestICS04Upgrade:testUpgradeCrossingHelloIncompatibleProposals() (gas: 4407259)
-TestICS04Upgrade:testUpgradeFull() (gas: 57784728)
-TestICS04Upgrade:testUpgradeInit() (gas: 3069408)
+TestICS04Upgrade:testUpgradeCannotRecvNextUpgradePacket() (gas: 5265976)
+TestICS04Upgrade:testUpgradeCounterpartyAdvanceNextSequenceBeforeOpen() (gas: 5236229)
+TestICS04Upgrade:testUpgradeCrossingHelloIncompatibleProposals() (gas: 4996384)
+TestICS04Upgrade:testUpgradeFull() (gas: 56608897)
+TestICS04Upgrade:testUpgradeInit() (gas: 3071908)
 TestICS04Upgrade:testUpgradeNoChanges() (gas: 2471936)
-TestICS04Upgrade:testUpgradeOutOfSync() (gas: 3903220)
-TestICS04Upgrade:testUpgradeRelaySuccessAtCounterpartyFlushComplete() (gas: 5237770)
-TestICS04Upgrade:testUpgradeRelaySuccessAtFlushing() (gas: 5610957)
-TestICS04Upgrade:testUpgradeSendPacketFailAtFlushingOrFlushComplete() (gas: 4071025)
-TestICS04Upgrade:testUpgradeTimeoutAbortAck() (gas: 17694261)
-TestICS04Upgrade:testUpgradeTimeoutAbortConfirm() (gas: 21333518)
-TestICS04Upgrade:testUpgradeTimeoutUpgrade() (gas: 71049040)
-TestICS04Upgrade:testUpgradeToOrdered() (gas: 56509529)
-TestICS04Upgrade:testUpgradeToUnordered() (gas: 45113829)
+TestICS04Upgrade:testUpgradeOutOfSync() (gas: 3904992)
+TestICS04Upgrade:testUpgradeRelaySuccessAtCounterpartyFlushComplete() (gas: 5237741)
+TestICS04Upgrade:testUpgradeRelaySuccessAtFlushing() (gas: 5610928)
+TestICS04Upgrade:testUpgradeSendPacketFailAtFlushingOrFlushComplete() (gas: 4070996)
+TestICS04Upgrade:testUpgradeTimeoutAbortAck() (gas: 17693913)
+TestICS04Upgrade:testUpgradeTimeoutAbortConfirm() (gas: 21333170)
+TestICS04Upgrade:testUpgradeTimeoutUpgrade() (gas: 71048576)
+TestICS04Upgrade:testUpgradeToOrdered() (gas: 56509239)
+TestICS04Upgrade:testUpgradeToUnordered() (gas: 45113597)
 TestICS04UpgradeApp:testUpgradeAuthorizationChanneNotFound() (gas: 61690)
 TestICS04UpgradeApp:testUpgradeAuthorizationRePropose() (gas: 2565532)
-TestICS04UpgradeApp:testUpgradeAuthorizationRemove() (gas: 2473992)
-TestICS20:testAddressToHex(address) (runs: 256, μ: 22726, ~: 22867)
+TestICS04UpgradeApp:testUpgradeAuthorizationRemove() (gas: 2475992)
+TestICS20:testAddressToHex(address) (runs: 256, μ: 22668, ~: 22804)
 TestICS20:testHexToAddress(string) (runs: 256, μ: 4776, ~: 4734)
 TestICS20:testIsEscapedString() (gas: 48979)
 TestICS20:testMarshaling() (gas: 148517)
-TestICS20:testParseAmount(uint256) (runs: 256, μ: 26449, ~: 24175)
+TestICS20:testParseAmount(uint256) (runs: 256, μ: 26349, ~: 23311)

contracts/core/04-channel/IBCChannelUpgrade.sol

@@ -457,10 +457,14 @@ contract IBCChannelUpgradeInitTryAck is IBCChannelUpgradeBase, IIBCChannelUpgrad
             // nextSequenceSend and timeout will be filled when we move to FLUSHING
             existingUpgrade.fields = upgradeFields;
         } else {
-            if (msg_.proposedConnectionHops.length != 0) {
-                revert IBCChannelUpgradeTryProposalMustEmptyIfExist();
+            if (msg_.proposedConnectionHops.length != 1) {
+                revert IBCChannelUpgradeTryProposedConnectionHopsEmpty();
             }
             upgradeFields = existingUpgrade.fields;
+            if (keccak256(bytes(upgradeFields.connection_hops[0])) != keccak256(bytes(msg_.proposedConnectionHops[0])))
+            {
+                revert IBCChannelUpgradeTryProposedConnectionHopsMismatch();
+            }
         }
 
         if (!isCompatibleUpgradeFields(upgradeFields, msg_.counterpartyUpgradeFields)) {

contracts/core/04-channel/IIBCChannelUpgradeErrors.sol

@@ -10,8 +10,11 @@ interface IIBCChannelUpgradeErrors is IIBCChannelErrors {
 
     error IBCChannelUpgradeInvalidUpgradeFields();
 
-    /// @notice proposal must be empty if upgrade exists
-    error IBCChannelUpgradeTryProposalMustEmptyIfExist();
+    /// @notice proposed upgrade must be non-empty
+    error IBCChannelUpgradeTryProposedConnectionHopsEmpty();
+
+    /// @notice proposal's connection hops mismatch with the existing proposal
+    error IBCChannelUpgradeTryProposedConnectionHopsMismatch();
 
     /// @param upgrader address of the upgrader
     error IBCChannelUpgradeUnauthorizedChannelUpgrader(address upgrader);

tests/foundry/src/ICS04Upgrade.t.sol

@@ -161,7 +161,7 @@ contract TestICS04Upgrade is ICS04UpgradeTestHelper, ICS04PacketEventTestHelper
                 channelId: channel0.channelId,
                 counterpartyUpgradeFields: mockApp.getUpgradeProposal(channel1.portId, channel1.channelId).fields,
                 counterpartyUpgradeSequence: 2,
-                proposedConnectionHops: new string[](0),
+                proposedConnectionHops: IBCChannelLib.buildConnectionHops(channel0.connectionId),
                 proofs: upgradeLocalhostProofs()
             });
             (bool ok, uint64 seq) = ibcHandler.channelUpgradeTry(msg_);
@@ -217,6 +217,30 @@ contract TestICS04Upgrade is ICS04UpgradeTestHelper, ICS04PacketEventTestHelper
         );
 
         // Try msg is reverted because the proposals are incompatible
+        {
+            IIBCChannelUpgradeBase.MsgChannelUpgradeTry memory msg_ = IIBCChannelUpgradeBase.MsgChannelUpgradeTry({
+                portId: channel0.portId,
+                channelId: channel0.channelId,
+                counterpartyUpgradeFields: mockApp.getUpgradeProposal(channel1.portId, channel1.channelId).fields,
+                counterpartyUpgradeSequence: 1,
+                proposedConnectionHops: IBCChannelLib.buildConnectionHops(channel0.connectionId),
+                proofs: upgradeLocalhostProofs()
+            });
+            vm.expectRevert(IIBCChannelUpgradeErrors.IBCChannelUpgradeIncompatibleProposal.selector);
+            ibcHandler.channelUpgradeTry(msg_);
+        }
+        {
+            IIBCChannelUpgradeBase.MsgChannelUpgradeTry memory msg_ = IIBCChannelUpgradeBase.MsgChannelUpgradeTry({
+                portId: channel0.portId,
+                channelId: channel0.channelId,
+                counterpartyUpgradeFields: mockApp.getUpgradeProposal(channel1.portId, channel1.channelId).fields,
+                counterpartyUpgradeSequence: 1,
+                proposedConnectionHops: IBCChannelLib.buildConnectionHops(channel1.connectionId),
+                proofs: upgradeLocalhostProofs()
+            });
+            vm.expectRevert(IIBCChannelUpgradeErrors.IBCChannelUpgradeTryProposedConnectionHopsMismatch.selector);
+            ibcHandler.channelUpgradeTry(msg_);
+        }
         {
             IIBCChannelUpgradeBase.MsgChannelUpgradeTry memory msg_ = IIBCChannelUpgradeBase.MsgChannelUpgradeTry({
                 portId: channel0.portId,
@@ -226,9 +250,33 @@ contract TestICS04Upgrade is ICS04UpgradeTestHelper, ICS04PacketEventTestHelper
                 proposedConnectionHops: new string[](0),
                 proofs: upgradeLocalhostProofs()
             });
+            vm.expectRevert(IIBCChannelUpgradeErrors.IBCChannelUpgradeTryProposedConnectionHopsEmpty.selector);
+            ibcHandler.channelUpgradeTry(msg_);
+        }
+        {
+            IIBCChannelUpgradeBase.MsgChannelUpgradeTry memory msg_ = IIBCChannelUpgradeBase.MsgChannelUpgradeTry({
+                portId: channel1.portId,
+                channelId: channel1.channelId,
+                counterpartyUpgradeFields: mockApp.getUpgradeProposal(channel0.portId, channel0.channelId).fields,
+                counterpartyUpgradeSequence: 1,
+                proposedConnectionHops: IBCChannelLib.buildConnectionHops(channel1.connectionId),
+                proofs: upgradeLocalhostProofs()
+            });
             vm.expectRevert(IIBCChannelUpgradeErrors.IBCChannelUpgradeIncompatibleProposal.selector);
             ibcHandler.channelUpgradeTry(msg_);
         }
+        {
+            IIBCChannelUpgradeBase.MsgChannelUpgradeTry memory msg_ = IIBCChannelUpgradeBase.MsgChannelUpgradeTry({
+                portId: channel1.portId,
+                channelId: channel1.channelId,
+                counterpartyUpgradeFields: mockApp.getUpgradeProposal(channel0.portId, channel0.channelId).fields,
+                counterpartyUpgradeSequence: 1,
+                proposedConnectionHops: IBCChannelLib.buildConnectionHops(channel0.connectionId),
+                proofs: upgradeLocalhostProofs()
+            });
+            vm.expectRevert(IIBCChannelUpgradeErrors.IBCChannelUpgradeTryProposedConnectionHopsMismatch.selector);
+            ibcHandler.channelUpgradeTry(msg_);
+        }
         {
             IIBCChannelUpgradeBase.MsgChannelUpgradeTry memory msg_ = IIBCChannelUpgradeBase.MsgChannelUpgradeTry({
                 portId: channel1.portId,
@@ -238,7 +286,7 @@ contract TestICS04Upgrade is ICS04UpgradeTestHelper, ICS04PacketEventTestHelper
                 proposedConnectionHops: new string[](0),
                 proofs: upgradeLocalhostProofs()
             });
-            vm.expectRevert(IIBCChannelUpgradeErrors.IBCChannelUpgradeIncompatibleProposal.selector);
+            vm.expectRevert(IIBCChannelUpgradeErrors.IBCChannelUpgradeTryProposedConnectionHopsEmpty.selector);
             ibcHandler.channelUpgradeTry(msg_);
         }
 
@@ -265,7 +313,7 @@ contract TestICS04Upgrade is ICS04UpgradeTestHelper, ICS04PacketEventTestHelper
                 channelId: channel0.channelId,
                 counterpartyUpgradeFields: mockApp.getUpgradeProposal(channel1.portId, channel1.channelId).fields,
                 counterpartyUpgradeSequence: 1,
-                proposedConnectionHops: new string[](0),
+                proposedConnectionHops: IBCChannelLib.buildConnectionHops(channel0.connectionId),
                 proofs: upgradeLocalhostProofs()
             });
             vm.expectRevert(
@@ -283,7 +331,7 @@ contract TestICS04Upgrade is ICS04UpgradeTestHelper, ICS04PacketEventTestHelper
                 channelId: channel1.channelId,
                 counterpartyUpgradeFields: mockApp.getUpgradeProposal(channel0.portId, channel0.channelId).fields,
                 counterpartyUpgradeSequence: 2,
-                proposedConnectionHops: new string[](0),
+                proposedConnectionHops: IBCChannelLib.buildConnectionHops(channel1.connectionId),
                 proofs: upgradeLocalhostProofs()
             });
             vm.expectRevert(IIBCChannelUpgradeErrors.IBCChannelUpgradeIncompatibleProposal.selector);
@@ -312,7 +360,7 @@ contract TestICS04Upgrade is ICS04UpgradeTestHelper, ICS04PacketEventTestHelper
                 channelId: channel0.channelId,
                 counterpartyUpgradeFields: mockApp.getUpgradeProposal(channel1.portId, channel1.channelId).fields,
                 counterpartyUpgradeSequence: 2,
-                proposedConnectionHops: new string[](0),
+                proposedConnectionHops: IBCChannelLib.buildConnectionHops(channel0.connectionId),
                 proofs: upgradeLocalhostProofs()
             })
         );
@@ -1210,12 +1258,6 @@ contract TestICS04Upgrade is ICS04UpgradeTestHelper, ICS04PacketEventTestHelper
                 proposedConnectionHops: IBCChannelLib.buildConnectionHops(proposals.p1.connectionId),
                 proofs: upgradeLocalhostProofs()
             });
-            if (flow.crossingHello) {
-                // If the upgrade already exists (i.e. crossing hello), `proposedConnectionHops` must be empty
-                vm.expectRevert();
-                ibcHandler.channelUpgradeTry(msg_);
-                msg_.proposedConnectionHops = new string[](0);
-            }
             (bool ok, uint64 seq) = ibcHandler.channelUpgradeTry(msg_);
             assertTrue(ok);
             assertEq(seq, upgradeSequence);