Have MsgsBetweenSeqNums populate TxHash (#1337)

* Have MsgsBetweenSeqNums populate TxHash

* Fix lint issue

* Add chain accessor wrapper

* Use existing mock chain accessor

* Commit plugin uses ChainAccessorWrapper

* Move PopulateTxHashEnabled check to CCIPChainReader

* Add unit tests

Author: rstout

commit/factory.go

@@ -187,6 +187,7 @@ func (p *PluginFactory) NewReportingPlugin(ctx context.Context, config ocr3types
 		p.ocrConfig.Config.ChainSelector,
 		p.ocrConfig.Config.OfframpAddress,
 		p.addrCodec,
+		offchainConfig.PopulateTxHashEnabled,
 	)
 	if err != nil {
 		return nil, ocr3types.ReportingPluginInfo{}, fmt.Errorf("failed to create CCIP chain reader: %w", err)

execute/factory.go

@@ -151,6 +151,7 @@ func (p PluginFactory) NewReportingPlugin(
 		p.ocrConfig.Config.ChainSelector,
 		p.ocrConfig.Config.OfframpAddress,
 		p.addrCodec,
+		offchainConfig.PopulateTxHashEnabled,
 	)
 	if err != nil {
 		return nil, ocr3types.ReportingPluginInfo{}, fmt.Errorf("failed to create ccip reader: %w", err)

pkg/chainaccessor/default_accessor.go

@@ -8,6 +8,7 @@ import (
 	"strconv"
 	"time"
 
+	"github.com/ethereum/go-ethereum/common/hexutil"
 	"github.com/smartcontractkit/chainlink-common/pkg/logger"
 	"github.com/smartcontractkit/chainlink-common/pkg/types"
 	cciptypes "github.com/smartcontractkit/chainlink-common/pkg/types/ccipocr3"
@@ -378,6 +379,12 @@ func (l *DefaultAccessor) MsgsBetweenSeqNums(
 		}
 
 		msg.Message.Header.OnRamp = onRampAddress
+
+		// Populate TxHash from Sequence item
+		if len(item.TxHash) > 0 {
+			msg.Message.Header.TxHash = hexutil.Encode(item.TxHash)
+		}
+
 		msgs = append(msgs, msg.Message)
 	}
 

pkg/chainaccessor/default_accessor_test.go

@@ -3,6 +3,7 @@ package chainaccessor
 import (
 	"context"
 	"fmt"
+	"math/big"
 	"reflect"
 	"testing"
 
@@ -381,3 +382,206 @@ func TestDefaultAccessor_GetSourceChainsConfig(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, []byte(expectedRouterB), cfgB.Router)
 }
+
+// Helper function to create a valid SendRequestedEvent for testing
+func createValidSendRequestedEvent(seqNum cciptypes.SeqNum) *SendRequestedEvent {
+	return &SendRequestedEvent{
+		DestChainSelector: chainB,
+		SequenceNumber:    seqNum,
+		Message: cciptypes.Message{
+			Header: cciptypes.RampMessageHeader{
+				SourceChainSelector: chainA,
+				DestChainSelector:   chainB,
+				SequenceNumber:      seqNum,
+				MessageID:           cciptypes.Bytes32{byte(seqNum)},
+			},
+			Sender:         cciptypes.UnknownAddress("sender"),
+			Receiver:       cciptypes.UnknownAddress("receiver"),
+			FeeToken:       cciptypes.UnknownAddress("feeToken"),
+			FeeTokenAmount: cciptypes.NewBigInt(big.NewInt(100)),
+		},
+	}
+}
+
+func TestMsgsBetweenSeqNums(t *testing.T) {
+	tests := []struct {
+		name              string
+		seqNumRange       cciptypes.SeqNumRange
+		destChainSelector cciptypes.ChainSelector
+		sequences         []types.Sequence
+		expectedError     bool
+		expectedMsgCount  int
+		validateTxHash    func(t *testing.T, msgs []cciptypes.Message)
+	}{
+		{
+			name:              "TxHash populated from item.TxHash",
+			seqNumRange:       cciptypes.NewSeqNumRange(1, 3),
+			destChainSelector: chainB,
+			sequences: []types.Sequence{
+				{
+					Cursor: "100-1-0xabc123",
+					TxHash: []byte{0xab, 0xcd, 0xef, 0x12, 0x34, 0x56},
+					Data:   createValidSendRequestedEvent(1),
+				},
+				{
+					Cursor: "100-2-0xdef456",
+					TxHash: []byte{0xde, 0xad, 0xbe, 0xef, 0x78, 0x90},
+					Data:   createValidSendRequestedEvent(2),
+				},
+			},
+			expectedError:    false,
+			expectedMsgCount: 2,
+			validateTxHash: func(t *testing.T, msgs []cciptypes.Message) {
+				require.Len(t, msgs, 2)
+				// TxHash should be populated from item.TxHash with 0x prefix
+				assert.Equal(t, "0xabcdef123456", msgs[0].Header.TxHash)
+				assert.Equal(t, "0xdeadbeef7890", msgs[1].Header.TxHash)
+			},
+		},
+		{
+			name:              "Mixed: some with item.TxHash, some without",
+			seqNumRange:       cciptypes.NewSeqNumRange(1, 3),
+			destChainSelector: chainB,
+			sequences: []types.Sequence{
+				{
+					Cursor: "100-1-0xcursor111",
+					TxHash: []byte{0x11, 0x22, 0x33}, // Has TxHash
+					Data:   createValidSendRequestedEvent(1),
+				},
+				{
+					Cursor: "100-2-0xcursor222",
+					TxHash: nil, // No TxHash
+					Data:   createValidSendRequestedEvent(2),
+				},
+				{
+					Cursor: "100-3-0xcursor333",
+					TxHash: []byte{0x44, 0x55, 0x66}, // Has TxHash
+					Data:   createValidSendRequestedEvent(3),
+				},
+			},
+			expectedError:    false,
+			expectedMsgCount: 3,
+			validateTxHash: func(t *testing.T, msgs []cciptypes.Message) {
+				require.Len(t, msgs, 3)
+				assert.Equal(t, "0x112233", msgs[0].Header.TxHash) // From item.TxHash
+				assert.Equal(t, "", msgs[1].Header.TxHash)         // Empty - no TxHash provided
+				assert.Equal(t, "0x445566", msgs[2].Header.TxHash) // From item.TxHash
+			},
+		},
+		{
+			name:              "Empty TxHash when item.TxHash is not provided",
+			seqNumRange:       cciptypes.NewSeqNumRange(1, 1),
+			destChainSelector: chainB,
+			sequences: []types.Sequence{
+				{
+					Cursor: "100-1-0xabc123",
+					TxHash: nil, // No TxHash
+					Data:   createValidSendRequestedEvent(1),
+				},
+			},
+			expectedError:    false,
+			expectedMsgCount: 1,
+			validateTxHash: func(t *testing.T, msgs []cciptypes.Message) {
+				require.Len(t, msgs, 1)
+				// TxHash should be empty when item.TxHash is not provided
+				assert.Equal(t, "", msgs[0].Header.TxHash)
+			},
+		},
+		{
+			name:              "Filter out invalid sequence number",
+			seqNumRange:       cciptypes.NewSeqNumRange(1, 2),
+			destChainSelector: chainB,
+			sequences: []types.Sequence{
+				{
+					Cursor: "100-1-0xabc123",
+					TxHash: []byte{0xab, 0xcd},
+					Data:   createValidSendRequestedEvent(1),
+				},
+				{
+					Cursor: "100-2-0xdef456",
+					TxHash: []byte{0xde, 0xef},
+					Data:   createValidSendRequestedEvent(10), // Out of range
+				},
+			},
+			expectedError:    false,
+			expectedMsgCount: 1,
+			validateTxHash: func(t *testing.T, msgs []cciptypes.Message) {
+				require.Len(t, msgs, 1)
+				assert.Equal(t, "0xabcd", msgs[0].Header.TxHash)
+			},
+		},
+		{
+			name:              "Wrong data type in sequence",
+			seqNumRange:       cciptypes.NewSeqNumRange(1, 2),
+			destChainSelector: chainB,
+			sequences: []types.Sequence{
+				{
+					Cursor: "100-1-0xabc123",
+					TxHash: []byte{0xab, 0xcd},
+					Data:   "invalid data type", // Not SendRequestedEvent
+				},
+			},
+			expectedError:    true,
+			expectedMsgCount: 0,
+			validateTxHash:   nil,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Setup mocks
+			mockReader := reader_mocks.NewMockExtended(t)
+			mockWriter := writer_mocks.NewMockContractWriter(t)
+			codec := internal.NewMockAddressCodecHex(t)
+
+			accessor := &DefaultAccessor{
+				lggr:           logger.Test(t),
+				chainSelector:  chainA,
+				contractReader: mockReader,
+				contractWriter: mockWriter,
+				addrCodec:      codec,
+			}
+
+			// Setup mock expectations
+			mockReader.On("ExtendedQueryKey", mock.Anything, mock.Anything,
+				mock.Anything, mock.Anything, mock.Anything).
+				Return(tt.sequences, nil).Once()
+
+			// Setup GetBindings mock for GetContractAddress call
+			// The address string will be converted to bytes by the codec
+			onRampAddressStr := "0x1234567890abcdef"
+			onRampAddress := cciptypes.UnknownAddress{0x12, 0x34, 0x56, 0x78, 0x90, 0xab, 0xcd, 0xef}
+			bindings := []contractreader.ExtendedBoundContract{
+				{
+					Binding: types.BoundContract{
+						Name:    consts.ContractNameOnRamp,
+						Address: onRampAddressStr,
+					},
+				},
+			}
+			mockReader.On("GetBindings", consts.ContractNameOnRamp).
+				Return(bindings).Once()
+
+			// Execute test
+			msgs, err := accessor.MsgsBetweenSeqNums(context.Background(), tt.destChainSelector, tt.seqNumRange)
+
+			// Verify results
+			if tt.expectedError {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+				assert.Len(t, msgs, tt.expectedMsgCount)
+				if tt.validateTxHash != nil {
+					tt.validateTxHash(t, msgs)
+				}
+				// Verify OnRamp is always set
+				for _, msg := range msgs {
+					assert.Equal(t, onRampAddress, msg.Header.OnRamp)
+				}
+			}
+
+			// Verify all mock expectations were met
+			mockReader.AssertExpectations(t)
+		})
+	}
+}

pkg/reader/ccip.go

@@ -39,11 +39,12 @@ type ccipChainReader struct {
 	contractReaders map[cciptypes.ChainSelector]contractreader.Extended
 	contractWriters map[cciptypes.ChainSelector]types.ContractWriter
 
-	destChain      cciptypes.ChainSelector
-	offrampAddress string
-	configPoller   ConfigPoller
-	addrCodec      cciptypes.AddressCodec
-	donAddressBook *addressbook.Book
+	destChain             cciptypes.ChainSelector
+	offrampAddress        string
+	configPoller          ConfigPoller
+	addrCodec             cciptypes.AddressCodec
+	donAddressBook        *addressbook.Book
+	populateTxHashEnabled bool
 }
 
 func newCCIPChainReaderInternal(
@@ -55,6 +56,7 @@ func newCCIPChainReaderInternal(
 	destChain cciptypes.ChainSelector,
 	offrampAddress []byte,
 	addrCodec cciptypes.AddressCodec,
+	populateTxHashEnabled bool,
 ) (*ccipChainReader, error) {
 	return newCCIPChainReaderWithConfigPollerInternal(
 		ctx,
@@ -66,6 +68,7 @@ func newCCIPChainReaderInternal(
 		offrampAddress,
 		addrCodec,
 		nil,
+		populateTxHashEnabled,
 	)
 }
 
@@ -79,6 +82,7 @@ func newCCIPChainReaderWithConfigPollerInternal(
 	offrampAddress []byte,
 	addrCodec cciptypes.AddressCodec,
 	configPoller ConfigPoller,
+	populateTxHashEnabled bool,
 ) (*ccipChainReader, error) {
 	var crs = make(map[cciptypes.ChainSelector]contractreader.Extended)
 	for chainSelector, cr := range contractReaders {
@@ -92,14 +96,15 @@ func newCCIPChainReaderWithConfigPollerInternal(
 	}
 
 	reader := &ccipChainReader{
-		lggr:            lggr,
-		contractReaders: crs,
-		contractWriters: contractWriters,
-		accessors:       chainAccessors,
-		destChain:       destChain,
-		offrampAddress:  offrampAddrStr,
-		addrCodec:       addrCodec,
-		donAddressBook:  addressbook.NewBook(),
+		lggr:                  lggr,
+		contractReaders:       crs,
+		contractWriters:       contractWriters,
+		accessors:             chainAccessors,
+		destChain:             destChain,
+		offrampAddress:        offrampAddrStr,
+		addrCodec:             addrCodec,
+		donAddressBook:        addressbook.NewBook(),
+		populateTxHashEnabled: populateTxHashEnabled,
 	}
 
 	// Initialize cache with readers
@@ -269,6 +274,13 @@ func (r *ccipChainReader) MsgsBetweenSeqNums(
 		return nil, fmt.Errorf("onRamp address has changed from %s to %s", onRampAddressBeforeQuery, onRampAddressAfterQuery)
 	}
 
+	// Clear TxHash if population is disabled
+	if !r.populateTxHashEnabled {
+		for i := range messages {
+			messages[i].Header.TxHash = ""
+		}
+	}
+
 	return messages, nil
 }
 

pkg/reader/ccip_interface.go

@@ -89,6 +89,7 @@ func NewCCIPChainReader(
 	destChain cciptypes.ChainSelector,
 	offrampAddress []byte,
 	addrCodec cciptypes.AddressCodec,
+	populateTxHashEnabled bool,
 ) (CCIPReader, error) {
 	reader, err := newCCIPChainReaderInternal(
 		ctx,
@@ -99,6 +100,7 @@ func NewCCIPChainReader(
 		destChain,
 		offrampAddress,
 		addrCodec,
+		populateTxHashEnabled,
 	)
 	if err != nil {
 		return nil, err
@@ -131,6 +133,7 @@ func NewCCIPReaderWithExtendedContractReaders(
 		destChain,
 		offrampAddress,
 		addrCodec,
+		false, // populateTxHashEnabled - default to false for deprecated test function
 	)
 	if err != nil {
 		// Panic here since right now this is only called from tests in core