[BCF - 3339] - Codec and CR hashed topics support (smartcontractkit#1…

…4016)

* Add better handling for CR EVM filtering by hashed indexed topics

* Add comments for CR event codec usage

* Improve err handling in CR init

* Add a TODO for CR QueryKey primitive remapping handling

* Update codec test to match most recent changes

* Add changeset and run solidity prettier

* Add contracts changeset for ChainReaderTester contract changes

* simplify getNativeAndCheckedTypesForArg FixedBytesTy case

.changeset/thin-rings-count.md

@@ -0,0 +1,5 @@
+---
+"chainlink": minor
+---
+
+#internal Add evm Chain Reader GetLatestValue support for filtering on indexed topic types that get hashed.

contracts/.changeset/itchy-turtles-agree.md

@@ -0,0 +1,5 @@
+---
+'@chainlink/contracts': minor
+---
+
+#internal Add an event with indexed topics that get hashed to Chain Reader Tester contract.

contracts/src/v0.8/shared/test/helpers/ChainReaderTester.sol

@@ -40,6 +40,9 @@ contract ChainReaderTester {
   // First topic is event hash
   event TriggeredWithFourTopics(int32 indexed field1, int32 indexed field2, int32 indexed field3);
 
+  // first topic is event hash, second and third topics get hashed before getting stored
+  event TriggeredWithFourTopicsWithHashed(string indexed field1, uint8[32] indexed field2, bytes32 indexed field3);
+
   TestStruct[] private s_seen;
   uint64[] private s_arr;
   uint64 private s_value;
@@ -125,4 +128,9 @@ contract ChainReaderTester {
   function triggerWithFourTopics(int32 field1, int32 field2, int32 field3) public {
     emit TriggeredWithFourTopics(field1, field2, field3);
   }
+
+  // first topic is event hash, second and third topics get hashed before getting stored
+  function triggerWithFourTopicsWithHashed(string memory field1, uint8[32] memory field2, bytes32 field3) public {
+    emit TriggeredWithFourTopicsWithHashed(field1, field2, field3);
+  }
 }

core/gethwrappers/generation/generated-wrapper-dependency-versions-do-not-edit.txt

@@ -24,7 +24,7 @@ batch_vrf_coordinator_v2: ../../contracts/solc/v0.8.6/BatchVRFCoordinatorV2/Batc
 batch_vrf_coordinator_v2plus: ../../contracts/solc/v0.8.19/BatchVRFCoordinatorV2Plus/BatchVRFCoordinatorV2Plus.abi ../../contracts/solc/v0.8.19/BatchVRFCoordinatorV2Plus/BatchVRFCoordinatorV2Plus.bin f13715b38b5b9084b08bffa571fb1c8ef686001535902e1255052f074b31ad4e
 blockhash_store: ../../contracts/solc/v0.8.19/BlockhashStore/BlockhashStore.abi ../../contracts/solc/v0.8.19/BlockhashStore/BlockhashStore.bin 31b118f9577240c8834c35f8b5a1440e82a6ca8aea702970de2601824b6ab0e1
 chain_module_base: ../../contracts/solc/v0.8.19/ChainModuleBase/ChainModuleBase.abi ../../contracts/solc/v0.8.19/ChainModuleBase/ChainModuleBase.bin 39dfce79330e921e5c169051b11c6e5ea15cd4db5a7b09c06aabbe9658148915
-chain_reader_tester: ../../contracts/solc/v0.8.19/ChainReaderTester/ChainReaderTester.abi ../../contracts/solc/v0.8.19/ChainReaderTester/ChainReaderTester.bin b3718dad488f54de97d124221d96b867c81e11210084a1fad379cb8385d37ffe
+chain_reader_tester: ../../contracts/solc/v0.8.19/ChainReaderTester/ChainReaderTester.abi ../../contracts/solc/v0.8.19/ChainReaderTester/ChainReaderTester.bin b207f9e6bf71e445a2664a602677011b87b80bf95c6352fd7869f1a9ddb08a5b
 chain_specific_util_helper: ../../contracts/solc/v0.8.6/ChainSpecificUtilHelper/ChainSpecificUtilHelper.abi ../../contracts/solc/v0.8.6/ChainSpecificUtilHelper/ChainSpecificUtilHelper.bin 66eb30b0717fefe05672df5ec863c0b9a5a654623c4757307a2726d8f31e26b1
 counter: ../../contracts/solc/v0.8.6/Counter/Counter.abi ../../contracts/solc/v0.8.6/Counter/Counter.bin 6ca06e000e8423573ffa0bdfda749d88236ab3da2a4cbb4a868c706da90488c9
 cron_upkeep_factory_wrapper: ../../contracts/solc/v0.8.6/CronUpkeepFactory/CronUpkeepFactory.abi - dacb0f8cdf54ae9d2781c5e720fc314b32ed5e58eddccff512c75d6067292cd7

core/services/relay/evm/chain_reader.go

@@ -128,6 +128,10 @@ func (cr *chainReader) init(chainContractReaders map[string]types.ChainContractR
 				return err
 			}
 		}
+
+		if cr.bindings.contractBindings[contractName] == nil {
+			return fmt.Errorf("%w: no read bindings added for contract: %s", commontypes.ErrInvalidConfig, contractName)
+		}
 		cr.bindings.contractBindings[contractName].pollingFilter = chainContractReader.PollingFilter.ToLPFilter(eventSigsForContractFilter)
 	}
 	return nil
@@ -259,7 +263,7 @@ func (cr *chainReader) addEvent(contractName, eventName string, a abi.ABI, chain
 		return err
 	}
 
-	// Encoder def's codec won't be used to encode, only for its type as input for GetLatestValue
+	// Encoder defs codec won't be used for encoding, but for storing caller filtering params which won't be hashed.
 	if err := cr.addEncoderDef(contractName, eventName, filterArgs, nil, chainReaderDefinition.InputModifications); err != nil {
 		return err
 	}
@@ -327,9 +331,11 @@ func (cr *chainReader) addQueryingReadBindings(contractName string, genericTopic
 	}
 }
 
+// getEventInput returns codec entry for expected incoming event params and the modifier to be applied to the params.
 func (cr *chainReader) getEventInput(def types.ChainReaderDefinition, contractName, eventName string) (
 	types.CodecEntry, codec.Modifier, error) {
 	inputInfo := cr.parsed.EncoderDefs[WrapItemType(contractName, eventName, true)]
+	// TODO can this be simplified? Isn't this same as inputInfo.Modifier()? BCI-3909
 	inMod, err := def.InputModifications.ToModifier(DecoderHooks...)
 	if err != nil {
 		return nil, nil, err
@@ -378,6 +384,8 @@ func (cr *chainReader) addDecoderDef(contractName, itemType string, outputs abi.
 	return output.Init()
 }
 
+// setupEventInput returns abi args where indexed flag is set to false because we expect caller to filter with params that aren't hashed.
+// codecEntry has expected onchain types set, for e.g. indexed topics of type string or uint8[32] array are expected as common.Hash onchain.
 func setupEventInput(event abi.Event, inputFields []string) ([]abi.Argument, types.CodecEntry, map[string]bool) {
 	topicFieldDefs := map[string]bool{}
 	for _, value := range inputFields {

core/services/relay/evm/event_binding.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/ethereum/go-ethereum/accounts/abi"
 	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/crypto"
 	"github.com/google/uuid"
 
 	"github.com/smartcontractkit/chainlink-common/pkg/codec"
@@ -209,11 +210,13 @@ func (e *eventBinding) getLatestValueWithFilters(
 		return err
 	}
 
+	// convert caller chain agnostic params types to types representing onchain abi types, for e.g. bytes32.
 	checkedParams, err := e.inputModifier.TransformToOnChain(offChain, "" /* unused */)
 	if err != nil {
 		return err
 	}
 
+	// convert onchain params to native types similarly to generated abi wrappers, for e.g. fixed bytes32 abi type to [32]uint8.
 	nativeParams, err := e.inputInfo.ToNative(reflect.ValueOf(checkedParams))
 	if err != nil {
 		return err
@@ -252,6 +255,8 @@ func (e *eventBinding) getLatestValueWithFilters(
 	return e.decodeLog(ctx, logToUse, into)
 }
 
+// convertToOffChainType creates a struct based on contract abi with applied codec modifiers.
+// Created type shouldn't have hashed types for indexed topics since incoming params wouldn't be hashed.
 func (e *eventBinding) convertToOffChainType(params any) (any, error) {
 	offChain, err := e.codec.CreateType(WrapItemType(e.contractName, e.eventName, true), true)
 	if err != nil {
@@ -287,43 +292,35 @@ func matchesRemainingFilters(log *logpoller.Log, filters []common.Hash) bool {
 	return true
 }
 
-func (e *eventBinding) encodeParams(item reflect.Value) ([]common.Hash, error) {
-	for item.Kind() == reflect.Pointer {
-		item = reflect.Indirect(item)
+// encodeParams accepts nativeParams and encodes them to match onchain topics.
+func (e *eventBinding) encodeParams(nativeParams reflect.Value) ([]common.Hash, error) {
+	for nativeParams.Kind() == reflect.Pointer {
+		nativeParams = reflect.Indirect(nativeParams)
 	}
 
-	var topics []any
-	switch item.Kind() {
+	var params []any
+	switch nativeParams.Kind() {
 	case reflect.Array, reflect.Slice:
-		native, err := representArray(item, e.inputInfo)
+		native, err := representArray(nativeParams, e.inputInfo)
 		if err != nil {
 			return nil, err
 		}
-		topics = []any{native}
+		params = []any{native}
 	case reflect.Struct, reflect.Map:
 		var err error
-		if topics, err = unrollItem(item, e.inputInfo); err != nil {
+		if params, err = unrollItem(nativeParams, e.inputInfo); err != nil {
 			return nil, err
 		}
 	default:
-		return nil, fmt.Errorf("%w: cannot encode kind %v", commontypes.ErrInvalidType, item.Kind())
+		return nil, fmt.Errorf("%w: cannot encode kind %v", commontypes.ErrInvalidType, nativeParams.Kind())
 	}
 
-	// abi params allow you to Pack a pointers, but MakeTopics doesn't work with pointers.
-	if err := e.derefTopics(topics); err != nil {
+	// abi params allow you to Pack a pointers, but makeTopics doesn't work with pointers.
+	if err := e.derefTopics(params); err != nil {
 		return nil, err
 	}
 
-	hashes, err := abi.MakeTopics(topics)
-	if err != nil {
-		return nil, wrapInternalErr(err)
-	}
-
-	if len(hashes) != 1 {
-		return nil, fmt.Errorf("%w: expected 1 filter set, got %d", commontypes.ErrInternal, len(hashes))
-	}
-
-	return hashes[0], nil
+	return e.makeTopics(params)
 }
 
 func (e *eventBinding) derefTopics(topics []any) error {
@@ -340,11 +337,38 @@ func (e *eventBinding) derefTopics(topics []any) error {
 	return nil
 }
 
+// makeTopics encodes and hashes params filtering values to match onchain indexed topics.
+func (e *eventBinding) makeTopics(params []any) ([]common.Hash, error) {
+	// make topic value for non-fixed bytes array manually because geth MakeTopics doesn't support it
+	for i, topic := range params {
+		if abiArg := e.inputInfo.Args()[i]; abiArg.Type.T == abi.ArrayTy && (abiArg.Type.Elem != nil && abiArg.Type.Elem.T == abi.UintTy) {
+			packed, err := abi.Arguments{abiArg}.Pack(topic)
+			if err != nil {
+				return nil, err
+			}
+			params[i] = crypto.Keccak256Hash(packed)
+		}
+	}
+
+	hashes, err := abi.MakeTopics(params)
+	if err != nil {
+		return nil, wrapInternalErr(err)
+	}
+
+	if len(hashes) != 1 {
+		return nil, fmt.Errorf("%w: expected 1 filter set, got %d", commontypes.ErrInternal, len(hashes))
+	}
+
+	return hashes[0], nil
+}
+
 func (e *eventBinding) decodeLog(ctx context.Context, log *logpoller.Log, into any) error {
+	// decode non indexed topics and apply output modifiers
 	if err := e.codec.Decode(ctx, log.Data, into, WrapItemType(e.contractName, e.eventName, false)); err != nil {
 		return err
 	}
 
+	// decode indexed topics which is rarely useful since most indexed topic types get Keccak256 hashed and should be just used for log filtering.
 	topics := make([]common.Hash, len(e.codecTopicInfo.Args()))
 	if len(log.Topics) < len(topics)+1 {
 		return fmt.Errorf("%w: not enough topics to decode", commontypes.ErrInvalidType)
@@ -436,6 +460,7 @@ func (e *eventBinding) remapExpression(key string, expression query.Expression)
 // remap chain agnostic primitives to chain specific
 func (e *eventBinding) remapPrimitive(key string, expression query.Expression) (query.Expression, error) {
 	switch primitive := expression.Primitive.(type) {
+	// TODO comparator primitive should undergo codec transformations and do hashed types handling similarly to how GetLatestValue handles it BCI-3910
 	case *primitives.Comparator:
 		if val, ok := e.eventDataWords[primitive.Name]; ok {
 			return logpoller.NewEventByWordFilter(e.hash, val, primitive.ValueComparators), nil

core/services/relay/evm/evmtesting/chain_reader_interface_tester.go

@@ -32,9 +32,10 @@ import (
 )
 
 const (
-	triggerWithDynamicTopic = "TriggeredEventWithDynamicTopic"
-	triggerWithAllTopics    = "TriggeredWithFourTopics"
-	finalityDepth           = 4
+	triggerWithDynamicTopic        = "TriggeredEventWithDynamicTopic"
+	triggerWithAllTopics           = "TriggeredWithFourTopics"
+	triggerWithAllTopicsWithHashed = "TriggeredWithFourTopicsWithHashed"
+	finalityDepth                  = 4
 )
 
 type EVMChainReaderInterfaceTesterHelper[T TestingT[T]] interface {
@@ -96,7 +97,7 @@ func (it *EVMChainReaderInterfaceTester[T]) Setup(t T) {
 			AnyContractName: {
 				ContractABI: chain_reader_tester.ChainReaderTesterMetaData.ABI,
 				ContractPollingFilter: types.ContractPollingFilter{
-					GenericEventNames: []string{EventName, EventWithFilterName},
+					GenericEventNames: []string{EventName, EventWithFilterName, triggerWithAllTopicsWithHashed},
 				},
 				Configs: map[string]*types.ChainReaderDefinition{
 					MethodTakingLatestParamsReturningTestStruct: &methodTakingLatestParamsReturningTestStructConfig,
@@ -145,6 +146,13 @@ func (it *EVMChainReaderInterfaceTester[T]) Setup(t T) {
 						// These float values can map to different finality concepts across chains.
 						ConfidenceConfirmations: map[string]int{"0.0": int(evmtypes.Unconfirmed), "1.0": int(evmtypes.Finalized)},
 					},
+					triggerWithAllTopicsWithHashed: {
+						ChainSpecificName: triggerWithAllTopicsWithHashed,
+						ReadType:          types.Event,
+						EventDefinitions: &types.EventDefinitions{
+							InputFields: []string{"Field1", "Field2", "Field3"},
+						},
+					},
 					MethodReturningSeenStruct: {
 						ChainSpecificName: "returnSeen",
 						InputModifications: codec.ModifiersConfig{

core/services/relay/evm/evmtesting/run_tests.go

@@ -12,10 +12,9 @@ import (
 
 	clcommontypes "github.com/smartcontractkit/chainlink-common/pkg/types"
 	"github.com/smartcontractkit/chainlink-common/pkg/types/query/primitives"
+	"github.com/smartcontractkit/chainlink/v2/core/services/relay/evm"
 
 	. "github.com/smartcontractkit/chainlink-common/pkg/types/interfacetests" //nolint common practice to import test mods with .
-
-	"github.com/smartcontractkit/chainlink/v2/core/services/relay/evm"
 )
 
 func RunChainReaderEvmTests[T TestingT[T]](t T, it *EVMChainReaderInterfaceTester[T]) {
@@ -74,6 +73,31 @@ func RunChainReaderEvmTests[T TestingT[T]](t T, it *EVMChainReaderInterfaceTeste
 		assert.Equal(t, int32(3), latest.Field3)
 	})
 
+	t.Run("Filtering can be done on indexed topics that get hashed", func(t T) {
+		it.Setup(t)
+		it.dirtyContracts = true
+		triggerFourTopicsWithHashed(t, it, "1", [32]uint8{2}, [32]byte{5})
+		triggerFourTopicsWithHashed(t, it, "2", [32]uint8{2}, [32]byte{3})
+		triggerFourTopicsWithHashed(t, it, "1", [32]uint8{3}, [32]byte{3})
+
+		ctx := it.Helper.Context(t)
+		cr := it.GetChainReader(t)
+		require.NoError(t, cr.Bind(ctx, it.GetBindings(t)))
+		var latest struct {
+			Field3 [32]byte
+		}
+		params := struct {
+			Field1 string
+			Field2 [32]uint8
+			Field3 [32]byte
+		}{Field1: "1", Field2: [32]uint8{2}, Field3: [32]byte{5}}
+
+		time.Sleep(it.MaxWaitTimeForEvents())
+		require.NoError(t, cr.GetLatestValue(ctx, AnyContractName, triggerWithAllTopicsWithHashed, primitives.Unconfirmed, params, &latest))
+		// only checking Field3 topic makes sense since it isn't hashed, to check other fields we'd have to replicate solidity encoding and hashing
+		assert.Equal(t, [32]uint8{5}, latest.Field3)
+	})
+
 	t.Run("Bind returns error on missing contract at address", func(t T) {
 		it.Setup(t)
 
@@ -95,3 +119,12 @@ func triggerFourTopics[T TestingT[T]](t T, it *EVMChainReaderInterfaceTester[T],
 	it.IncNonce()
 	it.AwaitTx(t, tx)
 }
+
+func triggerFourTopicsWithHashed[T TestingT[T]](t T, it *EVMChainReaderInterfaceTester[T], i1 string, i2 [32]uint8, i3 [32]byte) {
+	tx, err := it.contractTesters[it.address].ChainReaderTesterTransactor.TriggerWithFourTopicsWithHashed(it.GetAuthWithGasSet(t), i1, i2, i3)
+	require.NoError(t, err)
+	require.NoError(t, err)
+	it.Helper.Commit()
+	it.IncNonce()
+	it.AwaitTx(t, tx)
+}

core/services/relay/evm/types/codec_entry.go

@@ -200,7 +200,7 @@ func getNativeAndCheckedTypesForArg(arg *abi.Argument) (reflect.Type, reflect.Ty
 				return reflect.TypeOf(common.Hash{}), reflect.TypeOf(common.Hash{}), nil
 			}
 			fallthrough
-		case abi.SliceTy, abi.TupleTy, abi.FixedBytesTy, abi.FixedPointTy, abi.FunctionTy:
+		case abi.SliceTy, abi.TupleTy, abi.FixedPointTy, abi.FunctionTy:
 			// https://github.com/ethereum/go-ethereum/blob/release/1.12/accounts/abi/topics.go#L78
 			return nil, nil, fmt.Errorf("%w: unsupported indexed type: %v", commontypes.ErrInvalidConfig, arg.Type)
 		default:

core/services/relay/evm/types/codec_entry_test.go

@@ -273,17 +273,27 @@ func TestCodecEntry(t *testing.T) {
 		assertHaveSameStructureAndNames(t, iNative.Type(), entry.CheckedType())
 	})
 
-	t.Run("Indexed non basic types change to hash", func(t *testing.T) {
-		anyType, err := abi.NewType("string", "", []abi.ArgumentMarshaling{})
+	t.Run("Indexed string and bytes array change to hash", func(t *testing.T) {
+		stringType, err := abi.NewType("string", "", []abi.ArgumentMarshaling{})
 		require.NoError(t, err)
-		entry := NewCodecEntry(abi.Arguments{{Name: "Name", Type: anyType, Indexed: true}}, nil, nil)
-		require.NoError(t, entry.Init())
-		nativeField, ok := entry.CheckedType().FieldByName("Name")
-		require.True(t, ok)
-		assert.Equal(t, reflect.TypeOf(&common.Hash{}), nativeField.Type)
-		native, err := entry.ToNative(reflect.New(entry.CheckedType()))
+		arrayType, err := abi.NewType("uint8[32]", "", []abi.ArgumentMarshaling{})
 		require.NoError(t, err)
-		assertHaveSameStructureAndNames(t, native.Type().Elem(), entry.CheckedType())
+
+		abiArgs := abi.Arguments{
+			{Name: "String", Type: stringType, Indexed: true},
+			{Name: "Array", Type: arrayType, Indexed: true},
+		}
+
+		for i := 0; i < len(abiArgs); i++ {
+			entry := NewCodecEntry(abi.Arguments{abiArgs[i]}, nil, nil)
+			require.NoError(t, entry.Init())
+			nativeField, ok := entry.CheckedType().FieldByName(abiArgs[i].Name)
+			require.True(t, ok)
+			assert.Equal(t, reflect.TypeOf(&common.Hash{}), nativeField.Type)
+			native, err := entry.ToNative(reflect.New(entry.CheckedType()))
+			require.NoError(t, err)
+			assertHaveSameStructureAndNames(t, native.Type().Elem(), entry.CheckedType())
+		}
 	})
 
 	t.Run("Too many indexed items returns an error", func(t *testing.T) {