chore: require app version be non-empty when protocol version is vers…

…ion 2.
cosmos · Aug 26, 2024 · 6d7d618 · 6d7d618
1 parent f4783d1
commit 6d7d618
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 6 deletions.
diff --git a/modules/core/04-channel/types/packet.go b/modules/core/04-channel/types/packet.go
@@ -3,6 +3,7 @@ package types
 import (
 	"crypto/sha256"
 	"slices"
+	"strings"
 
 	errorsmod "cosmossdk.io/errors"
 
@@ -87,11 +88,9 @@ func commitV2Packet(packet Packet) []byte {
 	destinationHash := sha256.Sum256([]byte(packet.GetDestChannel()))
 	buf = append(buf, destinationHash[:]...)
 
-	// hash the version only if it is nonempty
-	if packet.AppVersion != "" {
-		versionHash := sha256.Sum256([]byte(packet.AppVersion))
-		buf = append(buf, versionHash[:]...)
-	}
+	// hash the app version.
+	versionHash := sha256.Sum256([]byte(packet.AppVersion))
+	buf = append(buf, versionHash[:]...)
 
 	// hash the data
 	dataHash := sha256.Sum256(packet.GetData())
@@ -199,6 +198,10 @@ func (p Packet) ValidateBasic() error {
 	if p.AppVersion != "" && slices.Contains([]IBCVersion{IBC_VERSION_UNSPECIFIED, IBC_VERSION_1}, p.ProtocolVersion) {
 		return errorsmod.Wrapf(ErrInvalidPacket, "app version cannot be specified when packet does not use protocol %s", IBC_VERSION_2)
 	}
+	if strings.TrimSpace(p.AppVersion) == "" && p.ProtocolVersion == IBC_VERSION_2 {
+		return errorsmod.Wrapf(ErrInvalidPacket, "app version must be specified when packet uses protocol %s", IBC_VERSION_2)
+	}
+
 	return nil
 }
 

diff --git a/modules/core/04-channel/types/packet_test.go b/modules/core/04-channel/types/packet_test.go
@@ -49,6 +49,7 @@ func TestPacketValidateBasic(t *testing.T) {
 		errMsg  string
 	}{
 		{types.NewPacket(validPacketData, 1, portid, chanid, cpportid, cpchanid, timeoutHeight, timeoutTimestamp), true, ""},
+		{types.NewPacket(unknownPacketData, 1, portid, chanid, cpportid, cpchanid, timeoutHeight, timeoutTimestamp), true, ""},
 		{types.NewPacket(validPacketData, 0, portid, chanid, cpportid, cpchanid, timeoutHeight, timeoutTimestamp), false, "invalid sequence"},
 		{types.NewPacket(validPacketData, 1, invalidPort, chanid, cpportid, cpchanid, timeoutHeight, timeoutTimestamp), false, "invalid source port"},
 		{types.NewPacket(validPacketData, 1, portid, invalidChannel, cpportid, cpchanid, timeoutHeight, timeoutTimestamp), false, "invalid source channel"},
@@ -60,7 +61,8 @@ func TestPacketValidateBasic(t *testing.T) {
 		{types.NewPacketWithVersion(validPacketData, 1, portid, chanid, cpportid, cpchanid, timeoutHeight, timeoutTimestamp, "version"), true, "valid v2 packet"},
 		{types.Packet{1, portid, chanid, cpportid, cpchanid, validPacketData, timeoutHeight, timeoutTimestamp, types.IBC_VERSION_1, "version"}, false, "invalid specifying of app version with protocol version 1"},
 		{types.Packet{1, portid, chanid, cpportid, cpchanid, validPacketData, timeoutHeight, timeoutTimestamp, types.IBC_VERSION_UNSPECIFIED, "version"}, false, "invalid specifying of app version with unspecified protocol version"},
-		{types.NewPacket(unknownPacketData, 1, portid, chanid, cpportid, cpchanid, timeoutHeight, timeoutTimestamp), true, ""},
+		{types.Packet{1, portid, chanid, cpportid, cpchanid, validPacketData, timeoutHeight, timeoutTimestamp, types.IBC_VERSION_2, ""}, false, "app version must be specified when packet uses protocol version 2"},
+		{types.Packet{1, portid, chanid, cpportid, cpchanid, validPacketData, timeoutHeight, timeoutTimestamp, types.IBC_VERSION_2, "      "}, false, "app version must be specified when packet uses protocol version 2"},
 	}
 
 	for i, tc := range testCases {