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
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
8 changes: 8 additions & 0 deletions contracts/.changeset/tiny-wolves-deny.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'@chainlink/contracts': minor
---

#internal Add event inside Chain Reader testing contract to check the new address to string decoder hook


BCFR-147
16 changes: 13 additions & 3 deletions contracts/src/v0.8/shared/test/helpers/ChainReaderTester.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ struct TestStruct {
uint8 OracleId;
uint8[32] OracleIds;
address Account;
address AccountStr;
address[] Accounts;
int192 BigField;
MidLevelTestStruct NestedStruct;
Expand All @@ -29,12 +30,15 @@ contract ChainReaderTester {
uint8 oracleId,
uint8[32] oracleIds,
address Account,
address AccountStr,
address[] Accounts,
string differentField,
int192 bigField,
MidLevelTestStruct nestedStruct
);

event TriggeredWithAddress(address indexed field1, address field2);

event TriggeredEventWithDynamicTopic(string indexed fieldHash, string field);

// First topic is event hash
Expand All @@ -59,11 +63,12 @@ contract ChainReaderTester {
uint8 oracleId,
uint8[32] calldata oracleIds,
address account,
address accountStr,
address[] calldata accounts,
int192 bigField,
MidLevelTestStruct calldata nestedStruct
) public {
s_seen.push(TestStruct(field, differentField, oracleId, oracleIds, account, accounts, bigField, nestedStruct));
s_seen.push(TestStruct(field, differentField, oracleId, oracleIds, account, accountStr, accounts, bigField, nestedStruct));
}

function setAlterablePrimitiveValue(uint64 value) public {
Expand All @@ -76,11 +81,12 @@ contract ChainReaderTester {
uint8 oracleId,
uint8[32] calldata oracleIds,
address account,
address accountStr,
address[] calldata accounts,
int192 bigField,
MidLevelTestStruct calldata nestedStruct
) public pure returns (TestStruct memory) {
return TestStruct(field, differentField, oracleId, oracleIds, account, accounts, bigField, nestedStruct);
return TestStruct(field, differentField, oracleId, oracleIds, account, accountStr, accounts, bigField, nestedStruct);
}

function getElementAtIndex(uint256 i) public view returns (TestStruct memory) {
Expand Down Expand Up @@ -112,12 +118,13 @@ contract ChainReaderTester {
uint8 oracleId,
uint8[32] calldata oracleIds,
address account,
address accountStr,
address[] calldata accounts,
string calldata differentField,
int192 bigField,
MidLevelTestStruct calldata nestedStruct
) public {
emit Triggered(field, oracleId, oracleIds, account, accounts, differentField, bigField, nestedStruct);
emit Triggered(field, oracleId, oracleIds, account, accountStr, accounts, differentField, bigField, nestedStruct);
}

function triggerEventWithDynamicTopic(string calldata field) public {
Expand All @@ -133,4 +140,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 59f352d071d5e205a78da5c3d8a065873b8cf868d271d4a124c10c7a94bd6c8f
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
2 changes: 1 addition & 1 deletion core/scripts/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ require (
github.com/prometheus/client_golang v1.20.0
github.com/shopspring/decimal v1.4.0
github.com/smartcontractkit/chainlink-automation v1.0.4
github.com/smartcontractkit/chainlink-common v0.2.3-0.20240919092417-53e784c2e420
github.com/smartcontractkit/chainlink-common v0.2.3-0.20240925055251-337542a780e3
github.com/smartcontractkit/chainlink/v2 v2.0.0-00010101000000-000000000000
github.com/smartcontractkit/libocr v0.0.0-20240717100443-f6226e09bee7
github.com/spf13/cobra v1.8.1
Expand Down
4 changes: 2 additions & 2 deletions core/scripts/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1083,8 +1083,8 @@ github.com/smartcontractkit/chainlink-automation v1.0.4 h1:iyW181JjKHLNMnDleI8um
github.com/smartcontractkit/chainlink-automation v1.0.4/go.mod h1:u4NbPZKJ5XiayfKHD/v3z3iflQWqvtdhj13jVZXj/cM=
github.com/smartcontractkit/chainlink-ccip v0.0.0-20240919174352-8d485ebc0953 h1:/Bx9MUUQ+TfS8kIdER8gujpJWfYu8ft4FYzpH8gSPJY=
github.com/smartcontractkit/chainlink-ccip v0.0.0-20240919174352-8d485ebc0953/go.mod h1:KP82vFCqm+M1G1t6Vos5CewGUGYJkxxCEdxnta4uLlE=
github.com/smartcontractkit/chainlink-common v0.2.3-0.20240919092417-53e784c2e420 h1:+xNnYYgkxzKUIkLCOfzfAKUxeLLtuxlalDI70kNJ8No=
github.com/smartcontractkit/chainlink-common v0.2.3-0.20240919092417-53e784c2e420/go.mod h1:zm+l8gN4LQS1+YvwQDhRz/njirVeWGNiDJKIhCGwaoQ=
github.com/smartcontractkit/chainlink-common v0.2.3-0.20240925055251-337542a780e3 h1:ODtevYqRlC36wB9TzykKtZOH+rmDu62ozEd3qJCFUqM=
github.com/smartcontractkit/chainlink-common v0.2.3-0.20240925055251-337542a780e3/go.mod h1:F6WUS6N4mP5ScwpwyTyAJc9/vjR+GXbMCRUOVekQi1g=
github.com/smartcontractkit/chainlink-cosmos v0.4.1-0.20240911175228-daf2600bb7b7 h1:lTGIOQYLk1Ufn++X/AvZnt6VOcuhste5yp+C157No/Q=
github.com/smartcontractkit/chainlink-cosmos v0.4.1-0.20240911175228-daf2600bb7b7/go.mod h1:BMYE1vC/pGmdFSsOJdPrAA0/4gZ0Xo0SxTMdGspBtRo=
github.com/smartcontractkit/chainlink-data-streams v0.0.0-20240916152957-433914114bd2 h1:yRk4ektpx/UxwarqAfgxUXLrsYXlaNeP1NOwzHGrK2Q=
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")
}
})
}
Loading
Loading