Merge branch 'develop' into jtw/rename-pkg-errors

common/txmgr/resender.go

@@ -140,7 +140,8 @@ func (er *Resender[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) resendUnco
 		return fmt.Errorf("Resender failed getting enabled keys for chain %s: %w", er.chainID.String(), err)
 	}
 
-	resendAddresses = append(resendAddresses, er.tracker.GetAbandonedAddresses()...)
+	// Tracker currently disabled for BCI-2638; refactor required
+	// resendAddresses = append(resendAddresses, er.tracker.GetAbandonedAddresses()...)
 
 	ageThreshold := er.txConfig.ResendAfterThreshold()
 	maxInFlightTransactions := er.txConfig.MaxInFlight()

common/txmgr/txmgr.go

@@ -191,10 +191,12 @@ func (b *Txm[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) Start(ctx
 			return fmt.Errorf("Txm: Estimator failed to start: %w", err)
 		}
 
+		/* Tracker currently disabled for BCI-2638; refactor required
 		b.logger.Info("Txm starting tracker")
 		if err := ms.Start(ctx, b.tracker); err != nil {
 			return fmt.Errorf("Txm: Tracker failed to start: %w", err)
 		}
+		*/
 
 		b.logger.Info("Txm starting runLoop")
 		b.wg.Add(1)
@@ -274,9 +276,11 @@ func (b *Txm[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) Close() (m
 			merr = errors.Join(merr, fmt.Errorf("Txm: failed to close TxAttemptBuilder: %w", err))
 		}
 
+		/* Tracker currently disabled for BCI-2638; refactor required
 		if err := b.tracker.Close(); err != nil {
 			merr = errors.Join(merr, fmt.Errorf("Txm: failed to close Tracker: %w", err))
 		}
+		*/
 
 		return nil
 	})
@@ -391,7 +395,8 @@ func (b *Txm[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) runLoop()
 			b.broadcaster.Trigger(address)
 		case head := <-b.chHeads:
 			b.confirmer.mb.Deliver(head)
-			b.tracker.mb.Deliver(head.BlockNumber())
+			// Tracker currently disabled for BCI-2638; refactor required
+			// b.tracker.mb.Deliver(head.BlockNumber())
 		case reset := <-b.reset:
 			// This check prevents the weird edge-case where you can select
 			// into this block after chStop has already been closed and the
@@ -419,10 +424,12 @@ func (b *Txm[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) runLoop()
 			if err != nil && (!errors.Is(err, services.ErrAlreadyStopped) || !errors.Is(err, services.ErrCannotStopUnstarted)) {
 				b.logger.Errorw(fmt.Sprintf("Failed to Close Confirmer: %v", err), "err", err)
 			}
+			/* Tracker currently disabled for BCI-2638; refactor required
 			err = b.tracker.Close()
 			if err != nil && (!errors.Is(err, services.ErrAlreadyStopped) || !errors.Is(err, services.ErrCannotStopUnstarted)) {
 				b.logger.Errorw(fmt.Sprintf("Failed to Close Tracker: %v", err), "err", err)
 			}
+			*/
 			return
 		case <-keysChanged:
 			// This check prevents the weird edge-case where you can select

core/chains/evm/txmgr/tracker_test.go

@@ -49,6 +49,7 @@ func containsID(txes []*txmgr.Tx, id int64) bool {
 }
 
 func TestEvmTracker_Initialization(t *testing.T) {
+	t.Skip("BCI-2638 tracker disabled")
 	t.Parallel()
 
 	tracker, _, _, _ := newTestEvmTrackerSetup(t)
@@ -65,6 +66,7 @@ func TestEvmTracker_Initialization(t *testing.T) {
 }
 
 func TestEvmTracker_AddressTracking(t *testing.T) {
+	t.Skip("BCI-2638 tracker disabled")
 	t.Parallel()
 
 	t.Run("track abandoned addresses", func(t *testing.T) {
@@ -94,6 +96,7 @@ func TestEvmTracker_AddressTracking(t *testing.T) {
 	})
 
 	t.Run("stop tracking finalized tx", func(t *testing.T) {
+		t.Skip("BCI-2638 tracker disabled")
 		tracker, txStore, _, _ := newTestEvmTrackerSetup(t)
 		confirmedAddr := cltest.MustGenerateRandomKey(t).Address
 		_ = mustInsertConfirmedEthTxWithReceipt(t, txStore, confirmedAddr, 123, 1)
@@ -118,6 +121,7 @@ func TestEvmTracker_AddressTracking(t *testing.T) {
 }
 
 func TestEvmTracker_ExceedingTTL(t *testing.T) {
+	t.Skip("BCI-2638 tracker disabled")
 	t.Parallel()
 
 	t.Run("confirmed but unfinalized transaction still tracked", func(t *testing.T) {

core/scripts/go.mod

@@ -19,7 +19,7 @@ require (
 	github.com/pelletier/go-toml/v2 v2.1.1
 	github.com/shopspring/decimal v1.3.1
 	github.com/smartcontractkit/chainlink-automation v1.0.2-0.20240118014648-1ab6a88c9429
-	github.com/smartcontractkit/chainlink-common v0.1.7-0.20240119143538-04c7f63ad53a
+	github.com/smartcontractkit/chainlink-common v0.1.7-0.20240123141952-a879db9d23b3
 	github.com/smartcontractkit/chainlink-vrf v0.0.0-20231120191722-fef03814f868
 	github.com/smartcontractkit/chainlink/v2 v2.0.0-00010101000000-000000000000
 	github.com/smartcontractkit/libocr v0.0.0-20240112202000-6359502d2ff1

core/scripts/go.sum

@@ -1167,8 +1167,8 @@ github.com/smartcontractkit/caigo v0.0.0-20230621050857-b29a4ca8c704 h1:T3lFWumv
 github.com/smartcontractkit/caigo v0.0.0-20230621050857-b29a4ca8c704/go.mod h1:2QuJdEouTWjh5BDy5o/vgGXQtR4Gz8yH1IYB5eT7u4M=
 github.com/smartcontractkit/chainlink-automation v1.0.2-0.20240118014648-1ab6a88c9429 h1:xkejUBZhcBpBrTSfxc91Iwzadrb6SXw8ks69bHIQ9Ww=
 github.com/smartcontractkit/chainlink-automation v1.0.2-0.20240118014648-1ab6a88c9429/go.mod h1:wJmVvDf4XSjsahWtfUq3wvIAYEAuhr7oxmxYnEL/LGQ=
-github.com/smartcontractkit/chainlink-common v0.1.7-0.20240119143538-04c7f63ad53a h1:lgM0yPo0KqSntLY4Y42RAH3avdv+Kyne8n+VM7cwlxo=
-github.com/smartcontractkit/chainlink-common v0.1.7-0.20240119143538-04c7f63ad53a/go.mod h1:05rRF84QKlIOF5LfTBPkHdw4UpBI2G3zxRcuZ65bPjk=
+github.com/smartcontractkit/chainlink-common v0.1.7-0.20240123141952-a879db9d23b3 h1:uSLn+JbClhldYn7zm0GEALGvi6KAfrY5Kp46IX9sy0E=
+github.com/smartcontractkit/chainlink-common v0.1.7-0.20240123141952-a879db9d23b3/go.mod h1:05rRF84QKlIOF5LfTBPkHdw4UpBI2G3zxRcuZ65bPjk=
 github.com/smartcontractkit/chainlink-cosmos v0.4.1-0.20240120192246-4bb04c997ca0 h1:NALwENz6vQ972DuD9AZjqRjyNSxH9ptNapizQGLI+2s=
 github.com/smartcontractkit/chainlink-cosmos v0.4.1-0.20240120192246-4bb04c997ca0/go.mod h1:NcVAT/GETDBvIoAej5K6OYqAtDOkF6vO5pYw/hLuYVU=
 github.com/smartcontractkit/chainlink-data-streams v0.0.0-20231204152908-a6e3fe8ff2a1 h1:xYqRgZO0nMSO8CBCMR0r3WA+LZ4kNL8a6bnbyk/oBtQ=

core/services/relay/evm/chain_reader_test.go

@@ -76,10 +76,10 @@ func TestChainReader(t *testing.T) {
 			return cr.GetLatestValue(ctx, AnyContractName, triggerWithDynamicTopic, input, output) == nil
 		}, it.MaxWaitTimeForEvents(), time.Millisecond*10)
 
-		assert.Equal(t, anyString, rOutput.FieldByName("Field").Interface())
+		assert.Equal(t, &anyString, rOutput.FieldByName("Field").Interface())
 		topic, err := abi.MakeTopics([]any{anyString})
 		require.NoError(t, err)
-		assert.Equal(t, topic[0][0], rOutput.FieldByName("FieldHash").Interface())
+		assert.Equal(t, &topic[0][0], rOutput.FieldByName("FieldHash").Interface())
 	})
 
 	t.Run("Multiple topics can filter together", func(t *testing.T) {
@@ -143,7 +143,9 @@ func (it *chainReaderInterfaceTester) MaxWaitTimeForEvents() time.Duration {
 func (it *chainReaderInterfaceTester) Setup(t *testing.T) {
 	t.Cleanup(func() {
 		// DB may be closed by the test already, ignore errors
-		_ = it.cr.Close()
+		if it.cr != nil {
+			_ = it.cr.Close()
+		}
 		it.cr = nil
 		it.evmTest = nil
 	})
@@ -213,6 +215,7 @@ func (it *chainReaderInterfaceTester) Setup(t *testing.T) {
 									"Account":  hexutil.Encode(testStruct.Account),
 								},
 							},
+							&codec.RenameModifierConfig{Fields: map[string]string{"NestedStruct.Inner.IntVal": "I"}},
 						},
 						OutputModifications: codec.ModifiersConfig{
 							&codec.HardCodeModifierConfig{OffChainValues: map[string]any{"ExtraField": anyExtraValue}},
@@ -285,7 +288,7 @@ func (it *chainReaderInterfaceTester) sendTxWithTestStruct(t *testing.T, testStr
 	tx, err := fn(
 		&it.evmTest.LatestValueHolderTransactor,
 		it.auth,
-		testStruct.Field,
+		*testStruct.Field,
 		testStruct.DifferentField,
 		uint8(testStruct.OracleID),
 		convertOracleIDs(testStruct.OracleIDs),
@@ -401,7 +404,7 @@ func getOracleIDs(first TestStruct) [32]byte {
 
 func toInternalType(testStruct TestStruct) chain_reader_example.TestStruct {
 	return chain_reader_example.TestStruct{
-		Field:          testStruct.Field,
+		Field:          *testStruct.Field,
 		DifferentField: testStruct.DifferentField,
 		OracleId:       byte(testStruct.OracleID),
 		OracleIds:      convertOracleIDs(testStruct.OracleIDs),

core/services/relay/evm/codec.go

@@ -18,14 +18,18 @@ import (
 	"github.com/smartcontractkit/chainlink/v2/core/services/relay/evm/types"
 )
 
-// decodeAccountHook allows strings to be converted to [32]byte allowing config to represent them as 0x...
+// decodeAccountAndAllowArraySliceHook allows:
+//
+//	strings to be converted to [32]byte allowing config to represent them as 0x...
+//	slices or arrays to be converted to a pointer to that type
+//
 // BigIntHook allows *big.Int to be represented as any integer type or a string and to go back to them.
 // Useful for config, or if when a model may use a go type that isn't a *big.Int when Pack expects one.
 // Eg: int32 in a go struct from a plugin could require a *big.Int in Pack for int24, if it fits, we shouldn't care.
 // SliceToArrayVerifySizeHook verifies that slices have the correct size when converting to an array
 // sizeVerifyBigIntHook allows our custom types that verify the number fits in the on-chain type to be converted as-if
 // it was a *big.Int
-var evmDecoderHooks = []mapstructure.DecodeHookFunc{decodeAccountHook, codec.BigIntHook, codec.SliceToArrayVerifySizeHook, sizeVerifyBigIntHook}
+var evmDecoderHooks = []mapstructure.DecodeHookFunc{decodeAccountAndAllowArraySliceHook, codec.BigIntHook, codec.SliceToArrayVerifySizeHook, sizeVerifyBigIntHook}
 
 // NewCodec creates a new [commontypes.RemoteCodec] for EVM.
 // Note that names in the ABI are converted to Go names using [abi.ToCamelCase],
@@ -113,18 +117,30 @@ func sizeVerifyBigIntHook(from, to reflect.Type, data any) (any, error) {
 	return converted, converted.Verify()
 }
 
-func decodeAccountHook(from, to reflect.Type, data any) (any, error) {
-	if from.Kind() == reflect.String && to == reflect.TypeOf(common.Address{}) {
-		decoded, err := hexutil.Decode(data.(string))
-		if err != nil {
-			return nil, fmt.Errorf("%w: %w", commontypes.ErrInvalidType, err)
-		} else if len(decoded) != common.AddressLength {
-			return nil, fmt.Errorf(
-				"%w: wrong number size for address expected %v got %v",
-				commontypes.ErrSliceWrongLen,
-				common.AddressLength, len(decoded))
-		}
-		return common.Address(decoded), nil
+func decodeAccountAndAllowArraySliceHook(from, to reflect.Type, data any) (any, error) {
+	if from.Kind() == reflect.String &&
+		(to == reflect.TypeOf(common.Address{}) || to == reflect.TypeOf(&common.Address{})) {
+		return decodeAddress(data)
 	}
+
+	if from.Kind() == reflect.Pointer && to.Kind() != reflect.Pointer && from != nil &&
+		(from.Elem().Kind() == reflect.Slice || from.Elem().Kind() == reflect.Array) {
+		return reflect.ValueOf(data).Elem().Interface(), nil
+	}
+
 	return data, nil
 }
+
+func decodeAddress(data any) (any, error) {
+	decoded, err := hexutil.Decode(data.(string))
+	if err != nil {
+		return nil, fmt.Errorf("%w: %w", commontypes.ErrInvalidType, err)
+	} else if len(decoded) != common.AddressLength {
+		return nil, fmt.Errorf(
+			"%w: wrong number size for address expected %v got %v",
+			commontypes.ErrSliceWrongLen,
+			common.AddressLength, len(decoded))
+	}
+
+	return common.Address(decoded), nil
+}

core/services/relay/evm/event_binding.go

@@ -224,6 +224,11 @@ func (e *eventBinding) encodeParams(item reflect.Value) ([]common.Hash, error) {
 		return nil, fmt.Errorf("%w: cannot encode kind %v", commontypes.ErrInvalidType, item.Kind())
 	}
 
+	// abi params allow you to Pack a pointers, but MakeTopics doesn't work with pointers.
+	if err := e.derefTopics(topics); err != nil {
+		return nil, err
+	}
+
 	hashes, err := abi.MakeTopics(topics)
 	if err != nil {
 		return nil, wrapInternalErr(err)
@@ -236,6 +241,20 @@ func (e *eventBinding) encodeParams(item reflect.Value) ([]common.Hash, error) {
 	return hashes[0], nil
 }
 
+func (e *eventBinding) derefTopics(topics []any) error {
+	for i, topic := range topics {
+		rTopic := reflect.ValueOf(topic)
+		if rTopic.Kind() == reflect.Pointer {
+			if rTopic.IsNil() {
+				return fmt.Errorf(
+					"%w: input topic %s cannot be nil", commontypes.ErrInvalidType, e.inputInfo.Args()[i].Name)
+			}
+			topics[i] = rTopic.Elem().Interface()
+		}
+	}
+	return nil
+}
+
 func (e *eventBinding) decodeLog(ctx context.Context, log *logpoller.Log, into any) error {
 	dataType := wrapItemType(e.contractName, e.eventName, false)
 	if err := e.codec.Decode(ctx, log.Data, into, dataType); err != nil {

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

@@ -138,8 +138,8 @@ func (entry *codecEntry) Init() error {
 		checked[i] = reflect.StructField{Name: name, Type: checkedArg}
 	}
 
-	entry.nativeType = reflect.StructOf(native)
-	entry.checkedType = reflect.StructOf(checked)
+	entry.nativeType = structOfPointers(native)
+	entry.checkedType = structOfPointers(checked)
 	return nil
 }
 
@@ -227,13 +227,9 @@ func createTupleType(curType *abi.Type, converter func(reflect.Type) reflect.Typ
 		return curType.TupleType, curType.TupleType, nil
 	}
 
-	// Create native type ourselves to assure that it'll always have the exact memory layout of checked types
-	// Otherwise, the "unsafe" casting that will be done to convert from checked to native won't be safe.
-	// At the time of writing, the way the TupleType is built it will be the same, but I don't want to rely on that
-	// If they ever add private fields for internal tracking
-	// or anything it would break us if we don't build the native type.
-	// As an example of how it could possibly change in the future, I've seen struct{}
-	// added with tags to the top of generated structs to allow metadata exploration.
+	// Our naive types always have the same layout as the checked ones.
+	// This differs intentionally from the type.GetType() in abi as fields on structs are pointers in ours to
+	// verify that fields are intentionally set.
 	nativeFields := make([]reflect.StructField, len(curType.TupleElems))
 	checkedFields := make([]reflect.StructField, len(curType.TupleElems))
 	for i, elm := range curType.TupleElems {
@@ -247,5 +243,14 @@ func createTupleType(curType *abi.Type, converter func(reflect.Type) reflect.Typ
 		nativeFields[i].Type = nativeArgType
 		checkedFields[i].Type = checkedArgType
 	}
-	return converter(reflect.StructOf(nativeFields)), converter(reflect.StructOf(checkedFields)), nil
+	return converter(structOfPointers(nativeFields)), converter(structOfPointers(checkedFields)), nil
+}
+
+func structOfPointers(fields []reflect.StructField) reflect.Type {
+	for i := range fields {
+		if fields[i].Type.Kind() != reflect.Pointer {
+			fields[i].Type = reflect.PointerTo(fields[i].Type)
+		}
+	}
+	return reflect.StructOf(fields)
 }

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

@@ -36,8 +36,10 @@ func TestCodecEntry(t *testing.T) {
 		require.NoError(t, entry.Init())
 		checked := reflect.New(entry.CheckedType())
 		iChecked := reflect.Indirect(checked)
-		iChecked.FieldByName("Field1").Set(reflect.ValueOf(uint16(2)))
-		iChecked.FieldByName("Field2").Set(reflect.ValueOf("any string"))
+		f1 := uint16(2)
+		iChecked.FieldByName("Field1").Set(reflect.ValueOf(&f1))
+		f2 := "any string"
+		iChecked.FieldByName("Field2").Set(reflect.ValueOf(&f2))
 
 		f3 := big.NewInt( /*2^24 - 1*/ 16777215)
 		setAndVerifyLimit(t, (*uint24)(f3), f3, iChecked.FieldByName("Field3"))
@@ -72,8 +74,11 @@ func TestCodecEntry(t *testing.T) {
 
 		checked := reflect.New(entry.CheckedType())
 		iChecked := reflect.Indirect(checked)
-		iChecked.FieldByName("Field1").Set(reflect.ValueOf(uint16(2)))
+		f1 := uint16(2)
+		iChecked.FieldByName("Field1").Set(reflect.ValueOf(&f1))
 		f2 := iChecked.FieldByName("Field2")
+		f2.Set(reflect.New(f2.Type().Elem()))
+		f2 = reflect.Indirect(f2)
 		f3 := big.NewInt( /*2^24 - 1*/ 16777215)
 		setAndVerifyLimit(t, (*uint24)(f3), f3, f2.FieldByName("Field3"))
 		f4 := big.NewInt( /*2^23 - 1*/ 8388607)
@@ -83,7 +88,7 @@ func TestCodecEntry(t *testing.T) {
 		require.NoError(t, err)
 		iNative := reflect.Indirect(native)
 		require.Equal(t, iNative.Field(0).Interface(), iChecked.Field(0).Interface())
-		nF2 := iNative.Field(1)
+		nF2 := reflect.Indirect(iNative.Field(1))
 		assert.Equal(t, nF2.Field(0).Interface(), f3)
 		assert.Equal(t, nF2.Field(1).Interface(), f4)
 		assertHaveSameStructureAndNames(t, iNative.Type(), entry.CheckedType())
@@ -100,11 +105,11 @@ func TestCodecEntry(t *testing.T) {
 		checked := reflect.New(entry.CheckedType())
 		iChecked := reflect.Indirect(checked)
 		anyValue := int16(2)
-		iChecked.FieldByName("Field1").Set(reflect.ValueOf(anyValue))
+		iChecked.FieldByName("Field1").Set(reflect.ValueOf(&anyValue))
 		native, err := entry.ToNative(checked)
 		require.NoError(t, err)
 		iNative := reflect.Indirect(native)
-		assert.Equal(t, anyValue, iNative.FieldByName("Field1").Interface())
+		assert.Equal(t, &anyValue, iNative.FieldByName("Field1").Interface())
 		assertHaveSameStructureAndNames(t, iNative.Type(), entry.CheckedType())
 	})
 
@@ -116,7 +121,7 @@ func TestCodecEntry(t *testing.T) {
 		require.NoError(t, entry.Init())
 		checked := reflect.New(entry.CheckedType())
 		iChecked := reflect.Indirect(checked)
-		anySliceValue := []int16{2, 3}
+		anySliceValue := &[]int16{2, 3}
 		iChecked.FieldByName("Field1").Set(reflect.ValueOf(anySliceValue))
 		native, err := entry.ToNative(checked)
 		require.NoError(t, err)
@@ -132,7 +137,7 @@ func TestCodecEntry(t *testing.T) {
 		require.NoError(t, entry.Init())
 		checked := reflect.New(entry.CheckedType())
 		iChecked := reflect.Indirect(checked)
-		anySliceValue := [3]int16{2, 3, 30}
+		anySliceValue := &[3]int16{2, 3, 30}
 		iChecked.FieldByName("Field1").Set(reflect.ValueOf(anySliceValue))
 		native, err := entry.ToNative(checked)
 		require.NoError(t, err)
@@ -157,7 +162,7 @@ func TestCodecEntry(t *testing.T) {
 
 		checked := reflect.New(entry.CheckedType())
 		iChecked := reflect.Indirect(checked)
-		anyAddr := common.Address{1, 2, 3}
+		anyAddr := &common.Address{1, 2, 3}
 		iChecked.FieldByName("Foo").Set(reflect.ValueOf(anyAddr))
 
 		native, err := entry.ToNative(checked)
@@ -188,7 +193,7 @@ func TestCodecEntry(t *testing.T) {
 		require.NoError(t, entry.Init())
 		checkedField, ok := entry.CheckedType().FieldByName("Name")
 		require.True(t, ok)
-		assert.Equal(t, reflect.TypeOf(int16(0)), checkedField.Type)
+		assert.Equal(t, reflect.TypeOf((*int16)(nil)), checkedField.Type)
 		native, err := entry.ToNative(reflect.New(entry.CheckedType()))
 		require.NoError(t, err)
 		iNative := reflect.Indirect(native)
@@ -202,7 +207,7 @@ func TestCodecEntry(t *testing.T) {
 		require.NoError(t, entry.Init())
 		nativeField, ok := entry.CheckedType().FieldByName("Name")
 		require.True(t, ok)
-		assert.Equal(t, reflect.TypeOf(common.Hash{}), nativeField.Type)
+		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())