Advanced Querying for ChainReader (#14511)

* Advanced Querying for ChainReader * Handle pointer type value comparator encoding * Handle geth abi panic when encoding data word value comparators * fix linting issues * [Bot] Update changeset file with jira issues * fix linting issues * [Bot] Update changeset file with jira issues * fix linting issues * [Bot] Update changeset file with jira issues * changed function name and added comments --------- Co-authored-by: ilija <[email protected]> Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com>
smartcontractkit · Sep 28, 2024 · 8fa9a67 · 8fa9a67
1 parent 7bbf4b2
commit 8fa9a67
Show file tree

Hide file tree

Showing 18 changed files with 352 additions and 161 deletions.
diff --git a/.changeset/nervous-books-push.md b/.changeset/nervous-books-push.md
@@ -0,0 +1,5 @@
+---
+"chainlink": minor
+---
+
+#internal Updated QueryKey to be able to do advanced queries on contract event data words
diff --git a/contracts/.changeset/tall-donkeys-flow.md b/contracts/.changeset/tall-donkeys-flow.md
@@ -0,0 +1,10 @@
+---
+'@chainlink/contracts': minor
+---
+
+#internal Updated ChainReaderTester to include dynamic and static nested structs in TestStruct
+
+
+PR issue: BCFR-44
+
+Solidity Review issue: BCFR-957
diff --git a/contracts/src/v0.8/shared/test/helpers/ChainReaderTester.sol b/contracts/src/v0.8/shared/test/helpers/ChainReaderTester.sol
@@ -10,29 +10,41 @@ struct TestStruct {
   address Account;
   address[] Accounts;
   int192 BigField;
-  MidLevelTestStruct NestedStruct;
+  MidLevelDynamicTestStruct NestedDynamicStruct;
+  MidLevelStaticTestStruct NestedStaticStruct;
 }
 
-struct MidLevelTestStruct {
+struct MidLevelDynamicTestStruct {
   bytes2 FixedBytes;
-  InnerTestStruct Inner;
+  InnerDynamicTestStruct Inner;
 }
 
-struct InnerTestStruct {
+struct InnerDynamicTestStruct {
   int64 IntVal;
   string S;
 }
 
+struct MidLevelStaticTestStruct {
+  bytes2 FixedBytes;
+  InnerStaticTestStruct Inner;
+}
+
+struct InnerStaticTestStruct {
+  int64 IntVal;
+  address A;
+}
+
 contract ChainReaderTester {
   event Triggered(
     int32 indexed field,
     uint8 oracleId,
+    MidLevelDynamicTestStruct nestedDynamicStruct,
+    MidLevelStaticTestStruct nestedStaticStruct,
     uint8[32] oracleIds,
     address Account,
     address[] Accounts,
     string differentField,
-    int192 bigField,
-    MidLevelTestStruct nestedStruct
+    int192 bigField
   );
 
   event TriggeredEventWithDynamicTopic(string indexed fieldHash, string field);
@@ -61,9 +73,22 @@ contract ChainReaderTester {
     address account,
     address[] calldata accounts,
     int192 bigField,
-    MidLevelTestStruct calldata nestedStruct
+    MidLevelDynamicTestStruct calldata nestedDynamicStruct,
+    MidLevelStaticTestStruct calldata nestedStaticStruct
   ) public {
-    s_seen.push(TestStruct(field, differentField, oracleId, oracleIds, account, accounts, bigField, nestedStruct));
+    s_seen.push(
+      TestStruct(
+        field,
+        differentField,
+        oracleId,
+        oracleIds,
+        account,
+        accounts,
+        bigField,
+        nestedDynamicStruct,
+        nestedStaticStruct
+      )
+    );
   }
 
   function setAlterablePrimitiveValue(uint64 value) public {
@@ -78,9 +103,21 @@ contract ChainReaderTester {
     address account,
     address[] calldata accounts,
     int192 bigField,
-    MidLevelTestStruct calldata nestedStruct
+    MidLevelDynamicTestStruct calldata nestedDynamicStruct,
+    MidLevelStaticTestStruct calldata nestedStaticStruct
   ) public pure returns (TestStruct memory) {
-    return TestStruct(field, differentField, oracleId, oracleIds, account, accounts, bigField, nestedStruct);
+    return
+      TestStruct(
+        field,
+        differentField,
+        oracleId,
+        oracleIds,
+        account,
+        accounts,
+        bigField,
+        nestedDynamicStruct,
+        nestedStaticStruct
+      );
   }
 
   function getElementAtIndex(uint256 i) public view returns (TestStruct memory) {
@@ -110,14 +147,25 @@ contract ChainReaderTester {
   function triggerEvent(
     int32 field,
     uint8 oracleId,
+    MidLevelDynamicTestStruct calldata nestedDynamicStruct,
+    MidLevelStaticTestStruct calldata nestedStaticStruct,
     uint8[32] calldata oracleIds,
     address account,
     address[] calldata accounts,
     string calldata differentField,
-    int192 bigField,
-    MidLevelTestStruct calldata nestedStruct
+    int192 bigField
   ) public {
-    emit Triggered(field, oracleId, oracleIds, account, accounts, differentField, bigField, nestedStruct);
+    emit Triggered(
+      field,
+      oracleId,
+      nestedDynamicStruct,
+      nestedStaticStruct,
+      oracleIds,
+      account,
+      accounts,
+      differentField,
+      bigField
+    );
   }
 
   function triggerEventWithDynamicTopic(string calldata field) public {

diff --git a/core/chains/evm/logpoller/orm_test.go b/core/chains/evm/logpoller/orm_test.go
@@ -916,7 +916,7 @@ func TestORM_DataWords(t *testing.T) {
 		},
 	}))
 
-	wordFilter := func(wordIdx uint8, word1, word2 uint64) []query.Expression {
+	wordFilter := func(wordIdx int, word1, word2 uint64) []query.Expression {
 		return []query.Expression{
 			logpoller.NewAddressFilter(addr),
 			logpoller.NewEventSigFilter(eventSig),

diff --git a/core/chains/evm/logpoller/parser.go b/core/chains/evm/logpoller/parser.go
@@ -504,11 +504,11 @@ type HashedValueComparator struct {
 }
 
 type eventByWordFilter struct {
-	WordIndex            uint8
+	WordIndex            int
 	HashedValueComparers []HashedValueComparator
 }
 
-func NewEventByWordFilter(wordIndex uint8, valueComparers []HashedValueComparator) query.Expression {
+func NewEventByWordFilter(wordIndex int, valueComparers []HashedValueComparator) query.Expression {
 	return query.Expression{Primitive: &eventByWordFilter{
 		WordIndex:            wordIndex,
 		HashedValueComparers: valueComparers,

diff --git a/core/gethwrappers/generated/chain_reader_tester/chain_reader_tester.go b/core/gethwrappers/generated/chain_reader_tester/chain_reader_tester.go
diff --git a/core/gethwrappers/generation/generated-wrapper-dependency-versions-do-not-edit.txt b/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 7a82cc28014761090185c2650239ad01a0901181f1b2b899b42ca293bcda3741
-chain_reader_tester: ../../contracts/solc/v0.8.19/ChainReaderTester/ChainReaderTester.abi ../../contracts/solc/v0.8.19/ChainReaderTester/ChainReaderTester.bin 84c4223c4dbd51aafd77a6787f4b84ce80f661ce86a907c1431c5b82d633f2ad
+chain_reader_tester: ../../contracts/solc/v0.8.19/ChainReaderTester/ChainReaderTester.abi ../../contracts/solc/v0.8.19/ChainReaderTester/ChainReaderTester.bin b9a488fc786f584a617764d8dc1722acdb30defb6b8f638e0ae03442795eaf3e
 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

diff --git a/core/services/relay/evm/chain_reader.go b/core/services/relay/evm/chain_reader.go
@@ -365,8 +365,10 @@ func (cr *chainReader) initDWQuerying(contractName, eventName string, eventDWs m
 	dWsDetail := make(map[string]read.DataWordDetail)
 
 	for genericName, onChainName := range dWDefs {
-		for _, dWDetail := range eventDWs {
-			if dWDetail.Name == onChainName {
+		for eventID, dWDetail := range eventDWs {
+			// Extract field name in this manner to account for nested fields
+			fieldName := strings.Join(strings.Split(eventID, ".")[1:], ".")
+			if fieldName == onChainName {
 				dWsDetail[genericName] = dWDetail
 
 				dwTypeID := eventName + "." + genericName
@@ -425,24 +427,11 @@ func getEventTypes(event abi.Event) ([]abi.Argument, types.CodecEntry, map[strin
 	indexedAsUnIndexedTypes := make([]abi.Argument, 0, types.MaxTopicFields)
 	indexedTypes := make([]abi.Argument, 0, len(event.Inputs))
 	dataWords := make(map[string]read.DataWordDetail)
-	hadDynamicType := false
-	var dwIndex uint8
+	var dwIndex int
 
 	for _, input := range event.Inputs {
 		if !input.Indexed {
-			// there are some cases where we can calculate the exact data word index even if there was a dynamic type before, but it is complex and probably not needed.
-			if input.Type.T == abi.TupleTy || input.Type.T == abi.SliceTy || input.Type.T == abi.StringTy || input.Type.T == abi.BytesTy {
-				hadDynamicType = true
-			}
-			if hadDynamicType {
-				continue
-			}
-
-			dataWords[event.Name+"."+input.Name] = read.DataWordDetail{
-				Index:    dwIndex,
-				Argument: input,
-			}
-			dwIndex++
+			dwIndex = calculateFieldDWIndex(input, event.Name+"."+input.Name, dataWords, dwIndex)
 			continue
 		}
 
@@ -456,6 +445,54 @@ func getEventTypes(event abi.Event) ([]abi.Argument, types.CodecEntry, map[strin
 	return indexedAsUnIndexedTypes, types.NewCodecEntry(indexedTypes, nil, nil), dataWords
 }
 
+// calculateFieldDWIndex recursively calculates the indices of all static unindexed fields in the event
+// and calculates the offset for all unsearchable / dynamic fields.
+func calculateFieldDWIndex(arg abi.Argument, fieldPath string, dataWords map[string]read.DataWordDetail, index int) int {
+	if isDynamic(arg.Type) {
+		return index + 1
+	}
+
+	return processFields(arg.Type, fieldPath, dataWords, index)
+}
+
+func processFields(fieldType abi.Type, parentFieldPath string, dataWords map[string]read.DataWordDetail, index int) int {
+	switch fieldType.T {
+	case abi.TupleTy:
+		// Recursively process tuple elements
+		for i, tupleElem := range fieldType.TupleElems {
+			fieldName := fieldType.TupleRawNames[i]
+			fullFieldPath := fmt.Sprintf("%s.%s", parentFieldPath, fieldName)
+			index = processFields(*tupleElem, fullFieldPath, dataWords, index)
+		}
+		return index
+	case abi.ArrayTy:
+		// Static arrays are not searchable, however, we can reliably calculate their size so that the fields
+		// after them can be searched.
+		return index + fieldType.Size
+	default:
+		dataWords[parentFieldPath] = read.DataWordDetail{
+			Index:    index,
+			Argument: abi.Argument{Type: fieldType},
+		}
+		return index + 1
+	}
+}
+
+func isDynamic(fieldType abi.Type) bool {
+	switch fieldType.T {
+	case abi.StringTy, abi.SliceTy, abi.BytesTy:
+		return true
+	case abi.TupleTy:
+		// If one element in a struct is dynamic, the whole struct is treated as dynamic.
+		for _, elem := range fieldType.TupleElems {
+			if isDynamic(*elem) {
+				return true
+			}
+		}
+	}
+	return false
+}
+
 // ConfirmationsFromConfig maps chain agnostic confidence levels defined in config to predefined EVM finality.
 func ConfirmationsFromConfig(values map[string]int) (map[primitives.ConfidenceLevel]evmtypes.Confirmations, error) {
 	mappings := map[primitives.ConfidenceLevel]evmtypes.Confirmations{

diff --git a/core/services/relay/evm/codec/codec.go b/core/services/relay/evm/codec/codec.go
@@ -93,6 +93,11 @@ func (c *evmCodec) CreateType(itemType string, forEncoding bool) (any, error) {
 		return nil, fmt.Errorf("%w: cannot find type name %s", commontypes.ErrInvalidType, itemType)
 	}
 
+	// we don't need double pointers, and they can also mess up reflection variable creation and mapstruct decode
+	if def.CheckedType().Kind() == reflect.Pointer {
+		return reflect.New(def.CheckedType()).Elem().Interface(), nil
+	}
+
 	return reflect.New(def.CheckedType()).Interface(), nil
 }
 

diff --git a/core/services/relay/evm/codec/codec_test.go b/core/services/relay/evm/codec/codec_test.go
@@ -205,7 +205,8 @@ func (it *codecInterfaceTester) GetCodec(t *testing.T) commontypes.Codec {
 
 		if k != sizeItemType && k != NilType {
 			entry.ModifierConfigs = commoncodec.ModifiersConfig{
-				&commoncodec.RenameModifierConfig{Fields: map[string]string{"NestedStruct.Inner.IntVal": "I"}},
+				&commoncodec.RenameModifierConfig{Fields: map[string]string{"NestedDynamicStruct.Inner.IntVal": "I"}},
+				&commoncodec.RenameModifierConfig{Fields: map[string]string{"NestedStaticStruct.Inner.IntVal": "I"}},
 			}
 		}
 
@@ -280,14 +281,24 @@ func packArgs(t *testing.T, allArgs []any, oargs abi.Arguments, request *EncodeR
 	return bytes
 }
 
-var inner = []abi.ArgumentMarshaling{
+var innerDynamic = []abi.ArgumentMarshaling{
 	{Name: "IntVal", Type: "int64"},
 	{Name: "S", Type: "string"},
 }
 
-var nested = []abi.ArgumentMarshaling{
+var nestedDynamic = []abi.ArgumentMarshaling{
 	{Name: "FixedBytes", Type: "bytes2"},
-	{Name: "Inner", Type: "tuple", Components: inner},
+	{Name: "Inner", Type: "tuple", Components: innerDynamic},
+}
+
+var innerStatic = []abi.ArgumentMarshaling{
+	{Name: "IntVal", Type: "int64"},
+	{Name: "A", Type: "address"},
+}
+
+var nestedStatic = []abi.ArgumentMarshaling{
+	{Name: "FixedBytes", Type: "bytes2"},
+	{Name: "Inner", Type: "tuple", Components: innerStatic},
 }
 
 var ts = []abi.ArgumentMarshaling{
@@ -298,7 +309,8 @@ var ts = []abi.ArgumentMarshaling{
 	{Name: "Account", Type: "address"},
 	{Name: "Accounts", Type: "address[]"},
 	{Name: "BigField", Type: "int192"},
-	{Name: "NestedStruct", Type: "tuple", Components: nested},
+	{Name: "NestedDynamicStruct", Type: "tuple", Components: nestedDynamic},
+	{Name: "NestedStaticStruct", Type: "tuple", Components: nestedStatic},
 }
 
 const sizeItemType = "item for size"
@@ -355,6 +367,7 @@ func argsFromTestStruct(ts TestStruct) []any {
 		common.Address(ts.Account),
 		getAccounts(ts),
 		ts.BigField,
-		evmtesting.MidToInternalType(ts.NestedStruct),
+		evmtesting.MidDynamicToInternalType(ts.NestedDynamicStruct),
+		evmtesting.MidStaticToInternalType(ts.NestedStaticStruct),
 	}
 }
diff --git a/core/services/relay/evm/codec/encoder.go b/core/services/relay/evm/codec/encoder.go
@@ -97,7 +97,7 @@ func RepresentArray(item reflect.Value, info types.CodecEntry) (any, error) {
 		return nil, err
 	}
 
-	return native.Elem().Interface(), nil
+	return native.Interface(), nil
 }
 
 func UnrollItem(item reflect.Value, info types.CodecEntry) ([]any, error) {
@@ -122,7 +122,6 @@ func UnrollItem(item reflect.Value, info types.CodecEntry) ([]any, error) {
 		}
 	}
 
-	item = reflect.Indirect(item)
 	length := item.NumField()
 	values := make([]any, length)
 	iType := item.Type()