Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BCFR-147] - Add DecodeHook to convert between string and common.Address types #14508

Draft
wants to merge 11 commits into
base: develop
Choose a base branch
from
5 changes: 5 additions & 0 deletions .changeset/nasty-rules-sort.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"chainlink": minor
---

#internal Add DecoderHook to parse address as string
5 changes: 5 additions & 0 deletions contracts/.changeset/tiny-wolves-deny.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@chainlink/contracts': minor
---

#internal Add event inside Chain Reader testing contract to check the new address to string decoder hook
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ contract ChainReaderTester {
MidLevelTestStruct nestedStruct
);

event TriggeredWithAddress(address indexed field1, address field2);

event TriggeredEventWithDynamicTopic(string indexed fieldHash, string field);

// First topic is event hash
Expand Down Expand Up @@ -133,4 +135,7 @@ contract ChainReaderTester {
function triggerWithFourTopicsWithHashed(string memory field1, uint8[32] memory field2, bytes32 field3) public {
emit TriggeredWithFourTopicsWithHashed(field1, field2, field3);
}
function triggerWithAddress(address field1, address field2) public {
emit TriggeredWithAddress(field1, field2);
}
}

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -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 50e455ed1a81b0f8a7e75bc199a78671c6d2c460ce10e523da3d6da408bb26ce
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
Expand Down
102 changes: 102 additions & 0 deletions core/services/relay/evm/codec/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ var DecoderHooks = []mapstructure.DecodeHookFunc{
commoncodec.SliceToArrayVerifySizeHook,
sizeVerifyBigIntHook,
commoncodec.NumberHook,
addressStringDecodeHook,
}

// NewCodec creates a new [commontypes.RemoteCodec] for EVM.
Expand Down Expand Up @@ -146,6 +147,10 @@ func decodeAccountAndAllowArraySliceHook(from, to reflect.Type, data any) (any,
return data, nil
}

// decodeAddress attempts to decode a hex-encoded Ethereum address from a string.
// Returns an error in the following cases:
// - If the input is not a valid hex string
// - If the decoded length does not match the expected length
func decodeAddress(data any) (any, error) {
decoded, err := hexutil.Decode(data.(string))
if err != nil {
Expand All @@ -159,3 +164,100 @@ func decodeAddress(data any) (any, error) {

return common.Address(decoded), nil
}

// addressStringDecodeHook is a decode hook that converts between `from` and `to` types involving string and common.Address types.
// It handles the following conversions:
// 1. `from` string or *string -> `to` common.Address or *common.Address
// 2. `from` common.Address or *common.Address -> `to` string or *string
//
// The function handles invalid `from` values and `nil` pointers:
// - If `from` is a string or *string and is invalid (e.g., an empty string or a non-hex string),
// it returns an appropriate error (types.ErrInvalidType).
// - If `from` is an empty common.Address{} or *common.Address, the function returns an error
// (types.ErrInvalidType) instead of treating it as the zero address ("0x0000000000000000000000000000000000000000").
// - If `from` is a nil *string or nil *common.Address, the function returns an error (types.ErrInvalidType).
//
// For unsupported `from` and `to` conversions, the function returns the original value unchanged.
func addressStringDecodeHook(from reflect.Type, to reflect.Type, value any) (any, error) {
// Helper variables for types to improve readability
stringType, stringPtrType := reflect.TypeOf(""), reflect.PointerTo(reflect.TypeOf(""))
addressType, addressPtrType := reflect.TypeOf(common.Address{}), reflect.TypeOf(&common.Address{})
Comment on lines +183 to +184
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These can be constants outside the function scope. This would save them being declared every time the function runs.


if (from == stringType || from == stringPtrType) &&
(to == addressType || to == addressPtrType) {
strValue, err := extractStringValue(from, value)
if err != nil {
return nil, err
}

address, err := decodeAddress(strValue)
if err != nil {
return nil, err
}

return returnAddressValueOrPtr(to, address)
}

if (from == addressType || from == addressPtrType) &&
(to == stringType || to == stringPtrType) {
if from == addressPtrType && (value == nil || reflect.ValueOf(value).IsNil()) {
return nil, fmt.Errorf("%w: nil *common.Address value", commontypes.ErrInvalidType)
}

addressStr, err := extractAddressToHexString(from, value)
if err != nil {
return nil, err
}

return returnStringValueOrPtr(to, addressStr)
}

// Return the original value unchanged for unsupported conversions
return value, nil
}

// Helper function to extract string or *string values
func extractStringValue(from reflect.Type, value any) (string, error) {
if from == reflect.PointerTo(reflect.TypeOf("")) {
if value == nil || reflect.ValueOf(value).IsNil() {
return "", fmt.Errorf("%w: nil *string value", commontypes.ErrInvalidType)
}
return *value.(*string), nil
}

return value.(string), nil
}

// Helper function to return *common.Address or common.Address depending on 'to' type
func returnAddressValueOrPtr(to reflect.Type, address any) (any, error) {
if to == reflect.TypeOf(&common.Address{}) {
addr := address.(common.Address)
return &addr, nil
}
return address, nil
}

// Helper function to extract hex string from common.Address or *common.Address
func extractAddressToHexString(from reflect.Type, value any) (string, error) {
if from == reflect.TypeOf(&common.Address{}) {
if (*value.(*common.Address) == common.Address{}) {
return "", fmt.Errorf("%w: empty address", commontypes.ErrInvalidType)
}
return value.(*common.Address).Hex(), nil
}

if (value.(common.Address) == common.Address{}) {
return "", fmt.Errorf("%w: empty address", commontypes.ErrInvalidType)
}

return value.(common.Address).Hex(), nil
}

// Helper function to return *string or string depending on 'to' type
func returnStringValueOrPtr(to reflect.Type, addressStr string) (any, error) {
if to == reflect.PointerTo(reflect.TypeOf("")) {
return &addressStr, nil
}

return addressStr, nil
}
149 changes: 149 additions & 0 deletions core/services/relay/evm/codec/codec_hooks_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
package codec

import (
"errors"
"reflect"
"testing"

"github.com/ethereum/go-ethereum/common"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/smartcontractkit/chainlink-common/pkg/types"
)

func TestAddressStringDecodeHook(t *testing.T) {
t.Parallel()

// Helper vars
var nilString *string
var nilAddress *common.Address
hexString := "0xDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEF"
address := common.HexToAddress(hexString)
addressToString := address.Hex()
emptyAddress := common.Address{}
emptyString := ""
stringType, stringPtrType := reflect.TypeOf(""), reflect.PointerTo(reflect.TypeOf(""))
addressType, addressPtrType := reflect.TypeOf(common.Address{}), reflect.TypeOf(&common.Address{})

t.Run("Converts from string to common.Address", func(t *testing.T) {
result, err := addressStringDecodeHook(stringType, addressType, hexString)
require.NoError(t, err)
require.IsType(t, common.Address{}, result)
assert.Equal(t, address, result)
})

t.Run("Converts from string to *common.Address", func(t *testing.T) {
result, err := addressStringDecodeHook(stringType, addressPtrType, hexString)
require.NoError(t, err)
assert.Equal(t, &address, result)
})

t.Run("Converts from *string to common.Address", func(t *testing.T) {
result, err := addressStringDecodeHook(stringPtrType, addressType, &hexString)
require.NoError(t, err)
require.IsType(t, common.Address{}, result)
assert.Equal(t, address, result)
})

t.Run("Converts from *string to *common.Address", func(t *testing.T) {
result, err := addressStringDecodeHook(stringPtrType, addressPtrType, &hexString)
require.NoError(t, err)
assert.Equal(t, &address, result)
})

t.Run("Converts from common.Address to string", func(t *testing.T) {
result, err := addressStringDecodeHook(addressType, stringType, address)
require.NoError(t, err)
require.IsType(t, "", result)
assert.Equal(t, addressToString, result)
})

t.Run("Converts from common.Address to *string", func(t *testing.T) {
result, err := addressStringDecodeHook(addressType, stringPtrType, address)
require.NoError(t, err)
assert.Equal(t, &addressToString, result)
})

t.Run("Converts from *common.Address to string", func(t *testing.T) {
result, err := addressStringDecodeHook(addressPtrType, stringType, &address)
require.NoError(t, err)
assert.Equal(t, addressToString, result)
})

t.Run("Converts from *common.Address to *string", func(t *testing.T) {
result, err := addressStringDecodeHook(addressPtrType, stringPtrType, &address)
require.NoError(t, err)
assert.Equal(t, &addressToString, result)
})

t.Run("Returns error on invalid hex string", func(t *testing.T) {
_, err := addressStringDecodeHook(stringType, addressType, "NotAHexString")
assert.True(t, errors.Is(err, types.ErrInvalidType))
_, err = addressStringDecodeHook(stringType, addressPtrType, "NotAHexString")
assert.True(t, errors.Is(err, types.ErrInvalidType))
})

t.Run("Returns error on empty string and empty *string", func(t *testing.T) {
_, err := addressStringDecodeHook(stringType, addressType, emptyString)
assert.True(t, errors.Is(err, types.ErrInvalidType), "Expected an error for empty string")
_, err = addressStringDecodeHook(stringType, addressPtrType, emptyString)
assert.True(t, errors.Is(err, types.ErrInvalidType), "Expected an error for empty string")
_, err = addressStringDecodeHook(stringPtrType, addressType, &emptyString)
assert.True(t, errors.Is(err, types.ErrInvalidType), "Expected an error for empty string")
_, err = addressStringDecodeHook(stringPtrType, addressPtrType, &emptyString)
assert.True(t, errors.Is(err, types.ErrInvalidType), "Expected an error for empty string")
})

t.Run("Returns error for empty common.Address and empty *common.Address", func(t *testing.T) {
_, err := addressStringDecodeHook(addressType, stringType, emptyAddress)
assert.True(t, errors.Is(err, types.ErrInvalidType), "Expected error for empty common.Address")
_, err = addressStringDecodeHook(addressType, stringPtrType, emptyAddress)
assert.True(t, errors.Is(err, types.ErrInvalidType), "Expected error for empty common.Address")
_, err = addressStringDecodeHook(addressPtrType, stringType, &emptyAddress)
assert.True(t, errors.Is(err, types.ErrInvalidType), "Expected error for empty *common.Address")
_, err = addressStringDecodeHook(addressPtrType, stringPtrType, &emptyAddress)
assert.True(t, errors.Is(err, types.ErrInvalidType), "Expected error for empty *common.Address")
})
t.Run("Returns error for nil *string", func(t *testing.T) {
result, err := addressStringDecodeHook(stringPtrType, addressType, nilString)
require.Error(t, err, "Expected error for nil *string input")
assert.Contains(t, err.Error(), "nil *string value")
assert.Nil(t, result, "Expected result to be nil for nil *string input")

result, err = addressStringDecodeHook(stringPtrType, addressPtrType, nilString)
require.Error(t, err, "Expected error for nil *string input")
assert.Contains(t, err.Error(), "nil *string value")
assert.Nil(t, result, "Expected result to be nil for nil *string input")
})

t.Run("Returns error for nil *common.Address", func(t *testing.T) {
result, err := addressStringDecodeHook(addressPtrType, stringType, nilAddress)
require.Error(t, err, "Expected error for nil *common.Address input")
assert.Contains(t, err.Error(), "nil *common.Address value")
assert.Nil(t, result, "Expected result to be nil for nil *common.Address input")

result, err = addressStringDecodeHook(addressPtrType, stringPtrType, nilAddress)
require.Error(t, err, "Expected error for nil *common.Address input")
assert.Contains(t, err.Error(), "nil *common.Address value")
assert.Nil(t, result, "Expected result to be nil for nil *common.Address input")
})

t.Run("Returns input unchanged for unsupported conversion", func(t *testing.T) {
unsupportedCases := []struct {
fromType reflect.Type
toType reflect.Type
input interface{}
}{
{fromType: reflect.TypeOf(12345), toType: addressType, input: 12345},
{fromType: reflect.TypeOf(12345), toType: stringType, input: 12345},
{fromType: reflect.TypeOf([]byte{}), toType: addressType, input: []byte{0x01, 0x02, 0x03}},
}

for _, tc := range unsupportedCases {
result, err := addressStringDecodeHook(tc.fromType, tc.toType, tc.input)
require.NoError(t, err)
assert.Equal(t, tc.input, result, "Expected original value to be returned for unsupported conversion")
}
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ const (
triggerWithDynamicTopic = "TriggeredEventWithDynamicTopic"
triggerWithAllTopics = "TriggeredWithFourTopics"
triggerWithAllTopicsWithHashed = "TriggeredWithFourTopicsWithHashed"
triggerWithAddress = "TriggeredWithAddress"
finalityDepth = 4
)

Expand Down Expand Up @@ -119,7 +120,7 @@ func (it *EVMChainComponentsInterfaceTester[T]) Setup(t T) {
AnyContractName: {
ContractABI: chain_reader_tester.ChainReaderTesterMetaData.ABI,
ContractPollingFilter: types.ContractPollingFilter{
GenericEventNames: []string{EventName, EventWithFilterName, triggerWithAllTopicsWithHashed},
GenericEventNames: []string{EventName, EventWithFilterName, triggerWithAllTopicsWithHashed, triggerWithAddress},
},
Configs: map[string]*types.ChainReaderDefinition{
MethodTakingLatestParamsReturningTestStruct: &methodTakingLatestParamsReturningTestStructConfig,
Expand Down Expand Up @@ -190,6 +191,11 @@ func (it *EVMChainComponentsInterfaceTester[T]) Setup(t T) {
&codec.RenameModifierConfig{Fields: map[string]string{"NestedStruct.Inner.IntVal": "I"}},
},
},
triggerWithAddress: {
ChainSpecificName: triggerWithAddress,
ReadType: types.Event,
EventDefinitions: &types.EventDefinitions{},
},
},
},
AnySecondContractName: {
Expand Down Expand Up @@ -253,6 +259,12 @@ func (it *EVMChainComponentsInterfaceTester[T]) Setup(t T) {
GasLimit: 2_000_000,
Checker: "simulate",
},
"triggerWithAddress": {
ChainSpecificName: "triggerWithAddress",
FromAddress: it.Helper.Accounts(t)[1].From,
GasLimit: 2_000_000,
Checker: "simulate",
},
},
},
AnySecondContractName: {
Expand Down
Loading
Loading