From b2bb2f412ae500384a7816a657e03a192941dbad Mon Sep 17 00:00:00 2001 From: John Saigle Date: Thu, 7 May 2026 10:31:04 -0400 Subject: [PATCH 1/2] node: Remove minimum TxID length --- node/pkg/common/chainlock.go | 21 +++++---------------- node/pkg/common/chainlock_test.go | 5 +---- node/pkg/common/pendingmessage_test.go | 20 -------------------- node/pkg/processor/delegate_obs.go | 6 ------ 4 files changed, 6 insertions(+), 46 deletions(-) diff --git a/node/pkg/common/chainlock.go b/node/pkg/common/chainlock.go index 86d42c51c0d..543f2eb5736 100644 --- a/node/pkg/common/chainlock.go +++ b/node/pkg/common/chainlock.go @@ -17,8 +17,6 @@ import ( ) const ( - // TxIDLenMin is the minimum length of a txID. - TxIDLenMin = 32 // AddressLength is the length of a normalized Wormhole address in bytes. AddressLength = 32 @@ -37,7 +35,6 @@ const ( // The minimum size of a marshaled message publication. It is the sum of the sizes of each of // the fields plus length information for fields with variable lengths (TxID and Payload). marshaledMsgLenMin = 1 + // TxID length (uint8) - TxIDLenMin + // TxID ([]byte), minimum length of 32 bytes (but may be longer) 8 + // Timestamp (int64) 4 + // Nonce (uint32) 8 + // Sequence (uint64) @@ -103,7 +100,6 @@ var ErrInputTooLarge = errors.New("input data exceeds maximum allowed size") var ( ErrBinaryWrite = errors.New("failed to write binary data") ErrTxIDTooLong = errors.New("field TxID too long") - ErrTxIDTooShort = errors.New("field TxID too short") ErrInvalidPayload = errors.New("field payload too long") ErrDataTooShort = errors.New("data too short") ErrTimestampTooShort = errors.New("data too short for timestamp") @@ -191,6 +187,8 @@ type MessagePublication struct { // whose buckets reach quorum at different observation subsets can still // pick different majorities; recovery for that case is the audit / // missing_observations reobs flow rather than the delegate-quorum path. + // + // TxID identifies the transaction that emitted this message. There is no minimum length. TxID []byte Timestamp time.Time @@ -357,17 +355,12 @@ func (msg *MessagePublication) MarshalBinary() ([]byte, error) { return nil, ErrInputSize{Msg: "TxID too long", Want: TxIDSizeMax, Got: txIDLen} } - if txIDLen < TxIDLenMin { - return nil, ErrInputSize{Msg: "TxID too short", Want: TxIDLenMin, Got: txIDLen} - } - payloadLen := len(msg.Payload) // Set up for serialization var ( be = binary.BigEndian // Size of the buffer needed to hold the serialized message. - // TxIDLenMin is already accounted for in the marshaledMsgLenMin calculation. - bufSize = (marshaledMsgLenMin - TxIDLenMin) + txIDLen + payloadLen + bufSize = marshaledMsgLenMin + txIDLen + payloadLen buf = make([]byte, 0, bufSize) ) @@ -528,12 +521,8 @@ func (m *MessagePublication) UnmarshalBinary(data []byte) error { txIDLen := uint8(data[pos]) pos++ - // Bounds checks. TxID length should be at least TxIDLenMin, but not larger than the length of the data. - // The second check is to avoid panics. - if int(txIDLen) < TxIDLenMin { - return ErrInputSize{Msg: "TxID length is too short", Got: int(txIDLen), Want: TxIDLenMin} - } - + // Bounds check. TxID length should not be larger than the length of the data. + // This check avoids panics. if int(txIDLen) > len(data) { return ErrInputSize{Msg: "TxID length is longer than bytes", Got: int(txIDLen)} } diff --git a/node/pkg/common/chainlock_test.go b/node/pkg/common/chainlock_test.go index 56d636c81f0..38a896ae20f 100644 --- a/node/pkg/common/chainlock_test.go +++ b/node/pkg/common/chainlock_test.go @@ -299,7 +299,7 @@ func TestMessagePublicationUnmarshalBinaryErrors(t *testing.T) { func FuzzMessagePublicationUnmarshalBinary(f *testing.F) { // Create a valid message publication for seeding orig := &MessagePublication{ - TxID: make([]byte, TxIDLenMin), // Use minimum valid TxID length + TxID: make([]byte, 32), // 32-byte tx hash (typical Ethereum hash) Timestamp: time.Unix(int64(1654516425), 0), Nonce: 123456, Sequence: 789101112131415, @@ -349,9 +349,6 @@ func FuzzMessagePublicationUnmarshalBinary(f *testing.F) { if len(mp.TxID) > 255 { t.Errorf("TxID length %d exceeds maximum of 255", len(mp.TxID)) } - if len(mp.TxID) < TxIDLenMin && len(mp.TxID) > 0 { - t.Errorf("TxID length %d is less than minimum of %d (unless empty)", len(mp.TxID), TxIDLenMin) - } // Verify that a successful unmarshal can be marshaled back _, marshalErr := mp.MarshalBinary() diff --git a/node/pkg/common/pendingmessage_test.go b/node/pkg/common/pendingmessage_test.go index 2fb92870bfd..f4b73b30ed1 100644 --- a/node/pkg/common/pendingmessage_test.go +++ b/node/pkg/common/pendingmessage_test.go @@ -69,10 +69,6 @@ func TestPendingMessage_MarshalError(t *testing.T) { var ( longTxID = bytes.NewBuffer(make([]byte, math.MaxUint8+1)) ) - emitter, err := vaa.StringToAddress("0x707f9118e33a9b8998bea41dd0d46f38bb963fc8") - require.NoError(t, err) - - require.NoError(t, err) tests := []test{ { @@ -82,22 +78,6 @@ func TestPendingMessage_MarshalError(t *testing.T) { }, errMsg: "wrong size: TxID too long", }, - { - label: "txID too short", - input: common.MessagePublication{ - TxID: []byte{}, - Timestamp: time.Unix(int64(1654516425), 0), - Nonce: 123456, - Sequence: 789101112131415, - EmitterChain: vaa.ChainIDEthereum, - EmitterAddress: emitter, - Payload: []byte{}, - ConsistencyLevel: 32, - Unreliable: true, - IsReobservation: true, - }, - errMsg: "wrong size: TxID too short", - }, } for _, tc := range tests { diff --git a/node/pkg/processor/delegate_obs.go b/node/pkg/processor/delegate_obs.go index b871ec4be7f..3d04aadbba9 100644 --- a/node/pkg/processor/delegate_obs.go +++ b/node/pkg/processor/delegate_obs.go @@ -59,9 +59,6 @@ func delegateObservationToMessagePublication(d *gossipv1.DelegateObservation) (* if txIDLen > TxIDSizeMax { return nil, fmt.Errorf("delegate observation tx_hash too long: got %d; want at most %d", txIDLen, TxIDSizeMax) } - if txIDLen < node_common.TxIDLenMin { - return nil, fmt.Errorf("delegate observation tx_hash too short: got %d; want at least %d", txIDLen, node_common.TxIDLenMin) - } if d.ConsistencyLevel > math.MaxUint8 { return nil, fmt.Errorf("invalid delegate observation consistency : %d", d.ConsistencyLevel) @@ -117,9 +114,6 @@ func messagePublicationToDelegateObservation(m *node_common.MessagePublication) if txIDLen > TxIDSizeMax { return nil, fmt.Errorf("message publication tx_hash too long: got %d; want at most %d", txIDLen, TxIDSizeMax) } - if txIDLen < node_common.TxIDLenMin { - return nil, fmt.Errorf("message publication tx_hash too short: got %d; want at least %d", txIDLen, node_common.TxIDLenMin) - } // Check if payload length is within max message size for p2p if len(m.Payload) > node_common.DelegatedPayloadLenMax { From ad5cb3426b8cb569e6a1855e11b317a61b7fae5a Mon Sep 17 00:00:00 2001 From: John Saigle Date: Tue, 16 Jun 2026 07:12:00 -0400 Subject: [PATCH 2/2] 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 --- node/pkg/common/chainlock.go | 32 +++++++++++++++---------------- node/pkg/common/chainlock_test.go | 13 +++++++++++++ sdk/vaa/structs.go | 11 +++++++---- 3 files changed, 35 insertions(+), 21 deletions(-) diff --git a/node/pkg/common/chainlock.go b/node/pkg/common/chainlock.go index 543f2eb5736..660c82af88f 100644 --- a/node/pkg/common/chainlock.go +++ b/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 { diff --git a/node/pkg/common/chainlock_test.go b/node/pkg/common/chainlock_test.go index 38a896ae20f..fe29a4beb65 100644 --- a/node/pkg/common/chainlock_test.go +++ b/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, diff --git a/sdk/vaa/structs.go b/sdk/vaa/structs.go index ec9e6f7fc5e..c1057ad48b1 100644 --- a/sdk/vaa/structs.go +++ b/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.