fix: verificationState not included in unmarshal size check

- include all fields in size check
- add unit test to catch regressions in this area
- move constant near others so divergences are easier to track
- move AddressLength constant to where Address is defined

Author: johnsaigle

node/pkg/common/chainlock.go

@@ -17,9 +17,6 @@ import (
 )
 
 const (
-	// AddressLength is the length of a normalized Wormhole address in bytes.
-	AddressLength = 32
-
 	// Wormhole supports arbitrary payloads due to the variance in transaction and block sizes between chains.
 	// However, during serialization, payload lengths are limited by Go slice length constraints and violations
 	// of these limits can cause panics.
@@ -54,6 +51,20 @@ const (
 	// minMsgIdLen is the minimum length of a message ID. It is used to uniquely identify
 	// messages in the case of a duplicate message ID and is stored in the database.
 	MinMsgIdLen = len("1/0000000000000000000000000290fb167208af455bb137780163b7b7a9a10c16/0")
+
+	// fixedFieldsLen is the minimum length of the fixed portion of a message publication.
+	// It is the sum of the sizes of each of the fields plus length information for the Payload.
+	// This is used to check that the data is long enough for the rest of the message after reading the TxID.
+	fixedFieldsLen = 8 + // Timestamp (int64)
+		4 + // Nonce (uint32)
+		8 + // Sequence (uint64)
+		1 + // ConsistencyLevel (uint8)
+		2 + // EmitterChain (uint16)
+		32 + // EmitterAddress (32 bytes)
+		1 + // IsReobservation (bool)
+		1 + // Unreliable (bool)
+		1 + // verificationState (uint8)
+		8 // Payload length (uint64)
 )
 
 var (
@@ -467,7 +478,7 @@ func UnmarshalMessagePublication(data []byte) (*MessagePublication, error) {
 	}
 
 	emitterAddress := vaa.Address{}
-	if n, err := reader.Read(emitterAddress[:]); err != nil || n != AddressLength {
+	if n, err := reader.Read(emitterAddress[:]); err != nil || n != int(vaa.AddressLength) {
 		return nil, fmt.Errorf("failed to read emitter address [%d]: %w", n, err)
 	}
 	msg.EmitterAddress = emitterAddress
@@ -492,19 +503,6 @@ func UnmarshalMessagePublication(data []byte) (*MessagePublication, error) {
 // UnmarshalBinary implements the BinaryUnmarshaler interface for MessagePublication.
 func (m *MessagePublication) UnmarshalBinary(data []byte) error {
 
-	// fixedFieldsLen is the minimum length of the fixed portion of a message publication.
-	// It is the sum of the sizes of each of the fields plus length information for the Payload.
-	// This is used to check that the data is long enough for the rest of the message after reading the TxID.
-	const fixedFieldsLen = 8 + // Timestamp (int64)
-		4 + // Nonce (uint32)
-		8 + // Sequence (uint64)
-		1 + // ConsistencyLevel (uint8)
-		2 + // EmitterChain (uint16)
-		32 + // EmitterAddress (32 bytes)
-		1 + // IsReobservation (bool)
-		1 + // Unreliable (bool)
-		8 // Payload length (uint64)
-
 	// Calculate minimum required length for the fixed portion
 	// (excluding variable-length fields: TxID and Payload)
 	if len(data) < marshaledMsgLenMin {

node/pkg/common/chainlock_test.go

@@ -135,6 +135,19 @@ func TestMessagePublicationUnmarshalBinaryErrors(t *testing.T) {
 				assert.Contains(t, inputSizeErr.Error(), "data too short")
 			},
 		},
+		{
+			name: "data too short - after non-empty TxID",
+			errorChecker: func(t *testing.T, err error) {
+				var inputSizeErr ErrInputSize
+				require.ErrorAs(t, err, &inputSizeErr)
+				assert.Contains(t, inputSizeErr.Error(), "data too short after reading TxID")
+			},
+			setupData: func() []byte {
+				data := make([]byte, 1+1+fixedFieldsLen-1)
+				data[offsetTxIDLength] = 1
+				return data
+			},
+		},
 		{
 			name:        "invalid IsReobservation boolean - value 0x02",
 			expectedErr: ErrInvalidBinaryBool,

sdk/vaa/structs.go

@@ -54,20 +54,20 @@ type (
 	// Address is a Wormhole protocol address otherwise referred to as a "wormhole normalized address." It typically
 	// contains the native chain's address for chains where the addresses are <= 32 bytes.
 	//
-	// If the address data type of a chain is < 32 bytes, the value is zero-padded on the left.
+	// If the address data type of a chain is less than [AddressLength] bytes, the value is zero-padded on the left.
 	//
-	// For chains with addresses that support lengths greater than 32 bytes, this type MAY be used
+	// For chains with addresses that support lengths greater than [AddressLength] bytes, this type MAY be used
 	// to represent an on-chain address by hashing it using an algorithm like sha256.
 	//
 	// For chains with variable length addresses, users MUST use this type consistently such that addresses
 	// can be uniquely identified. For example, if a chain supports addresses between length 2 and 64, this type
 	// should always encode addresses for that chain as sha256 digests. This helps to avoid a situation where
-	// addresses with length < 32 have zero-padded EVM-like addresses, but larger addresses on the same chain
+	// addresses with length < [AddressLength] have zero-padded EVM-like addresses, but larger addresses on the same chain
 	// are represented as digests.
 	//
 	// Other uses of this type are NOT RECOMMENDED given that they can overload the semantic meaning of this field
 	// which already does a lot of conceptual work across disparate blockchain implementations.
-	Address [32]byte
+	Address [AddressLength]byte
 
 	// Signature of a single guardian
 	Signature struct {
@@ -126,6 +126,9 @@ const (
 	ConsistencyLevelSafe               = uint8(201)
 	ConsistencyLevelFinalized          = uint8(202)
 	ConsistencyLevelCustom             = uint8(203)
+
+	// AddressLength is the length of an normalized [Address] in bytes.
+	AddressLength uint8 = 32
 )
 
 //nolint:unparam // error is always nil here but the return type is required to satisfy the interface.