Skip to content

Commit

Permalink
[BCFR-147][core] - Add codec chain agnostic modifier for converting b…
Browse files Browse the repository at this point in the history
…yte array address to string (#14620)

* most of iface tests working

* fix codec tests

* evm tests passing

* renamings

* cleanups

* rename codec to commoncodec

* bump common

* bump common

* lint and changeset

* [Bot] Update changeset file with jira issues

* chain agnostic modifier injected through relayer

* common bump

* addressing comments

* bump common

* compatibility with job spec

* contract generate

* bump common

* inject modifier in runtime

* bump common

* bump common

* general name

* bump common

* fix comment

* bump common

* fix common ref

* update solana ref

* Bump common refs

* fix lint

---------

Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com>
Co-authored-by: ilija <pavlovicilija42@gmail.com>
  • Loading branch information
3 people authored Oct 22, 2024
1 parent f3e66b2 commit 09145ba
Show file tree
Hide file tree
Showing 19 changed files with 243 additions and 63 deletions.
5 changes: 5 additions & 0 deletions .changeset/olive-bugs-relax.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"chainlink": minor
---

add address bytes to string modifier to chainReader. #internal
10 changes: 10 additions & 0 deletions contracts/.changeset/happy-planets-accept.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
'@chainlink/contracts': minor
---

add accountStr to chainReader test contract struct in order to test the new address bytes to string modifier. #internal


PR issue: BCFR-147

Solidity Review issue: BCFR-957
8 changes: 8 additions & 0 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;
MidLevelDynamicTestStruct NestedDynamicStruct;
Expand Down Expand Up @@ -42,6 +43,7 @@ contract ChainReaderTester {
MidLevelStaticTestStruct nestedStaticStruct,
uint8[32] oracleIds,
address Account,
address AccountStr,
address[] Accounts,
string differentField,
int192 bigField
Expand Down Expand Up @@ -74,6 +76,7 @@ contract ChainReaderTester {
uint8 oracleId,
uint8[32] calldata oracleIds,
address account,
address accountStr,
address[] calldata accounts,
int192 bigField,
MidLevelDynamicTestStruct calldata nestedDynamicStruct,
Expand All @@ -86,6 +89,7 @@ contract ChainReaderTester {
oracleId,
oracleIds,
account,
accountStr,
accounts,
bigField,
nestedDynamicStruct,
Expand All @@ -104,6 +108,7 @@ contract ChainReaderTester {
uint8 oracleId,
uint8[32] calldata oracleIds,
address account,
address accountStr,
address[] calldata accounts,
int192 bigField,
MidLevelDynamicTestStruct calldata nestedDynamicStruct,
Expand All @@ -116,6 +121,7 @@ contract ChainReaderTester {
oracleId,
oracleIds,
account,
accountStr,
accounts,
bigField,
nestedDynamicStruct,
Expand Down Expand Up @@ -154,6 +160,7 @@ contract ChainReaderTester {
MidLevelStaticTestStruct calldata nestedStaticStruct,
uint8[32] calldata oracleIds,
address account,
address accountStr,
address[] calldata accounts,
string calldata differentField,
int192 bigField
Expand All @@ -165,6 +172,7 @@ contract ChainReaderTester {
nestedStaticStruct,
oracleIds,
account,
accountStr,
accounts,
differentField,
bigField
Expand Down

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 21fcc5fae2a95ce11590bcfc9ea2bde5ad9158f8dcb4efce7264492f8ad2b0a6
chain_reader_tester: ../../contracts/solc/v0.8.19/ChainReaderTester/ChainReaderTester.abi ../../contracts/solc/v0.8.19/ChainReaderTester/ChainReaderTester.bin c25c1712046dfa691cafa58803e6fbfa163f325e6e3aabf0558f5a9d2ab1065e
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 @@ -24,7 +24,7 @@ require (
github.com/prometheus/client_golang v1.20.0
github.com/shopspring/decimal v1.4.0
github.com/smartcontractkit/chainlink-automation v0.8.0
github.com/smartcontractkit/chainlink-common v0.3.1-0.20241018143728-5248d7c4468a
github.com/smartcontractkit/chainlink-common v0.3.1-0.20241021103500-39a6e78c0286
github.com/smartcontractkit/chainlink/integration-tests v0.0.0-00010101000000-000000000000
github.com/smartcontractkit/chainlink/v2 v2.0.0-00010101000000-000000000000
github.com/smartcontractkit/libocr v0.0.0-20241007185508-adbe57025f12
Expand Down
4 changes: 2 additions & 2 deletions core/scripts/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1074,8 +1074,8 @@ github.com/smartcontractkit/chainlink-automation v0.8.0 h1:hFz2EHU06bkEfhcqhK8Jd
github.com/smartcontractkit/chainlink-automation v0.8.0/go.mod h1:ObdjDfgGIaiE48Bb3yYcx1CeGBm392WlEw92U83LlUA=
github.com/smartcontractkit/chainlink-ccip v0.0.0-20241017140434-6757be193e1c h1:BvzX0A659a9fShyW69P/jV3iVlA4/wlGbZ/4XXE3pxI=
github.com/smartcontractkit/chainlink-ccip v0.0.0-20241017140434-6757be193e1c/go.mod h1:4adKaHNaxFsRvV/lYfqtbsWyyvIPUMLR0FdOJN/ljis=
github.com/smartcontractkit/chainlink-common v0.3.1-0.20241018143728-5248d7c4468a h1:X0+2AbgCmPgfwY2dTvAK37KO8UvWLHMgfAFL3MA4BEs=
github.com/smartcontractkit/chainlink-common v0.3.1-0.20241018143728-5248d7c4468a/go.mod h1:tsGgeEJc5SUSlfVGSX0wR0EkRU3pM58D6SKF97V68ko=
github.com/smartcontractkit/chainlink-common v0.3.1-0.20241021103500-39a6e78c0286 h1:qx5p01fqee86cj6EUOCzFc2zILw56v1Q3c5DUuEQWLs=
github.com/smartcontractkit/chainlink-common v0.3.1-0.20241021103500-39a6e78c0286/go.mod h1:tsGgeEJc5SUSlfVGSX0wR0EkRU3pM58D6SKF97V68ko=
github.com/smartcontractkit/chainlink-cosmos v0.5.2-0.20241017133723-5277829bd53f h1:BwrIaQIx5Iy6eT+DfLhFfK2XqjxRm74mVdlX8gbu4dw=
github.com/smartcontractkit/chainlink-cosmos v0.5.2-0.20241017133723-5277829bd53f/go.mod h1:wHtwSR3F1CQSJJZDQKuqaqFYnvkT+kMyget7dl8Clvo=
github.com/smartcontractkit/chainlink-data-streams v0.1.1-0.20241018134907-a00ba3729b5e h1:JiETqdNM0bktAUGMc62COwXIaw3rR3M77Me6bBLG0Fg=
Expand Down
19 changes: 19 additions & 0 deletions core/services/relay/evm/chain_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ func (cr *chainReader) init(chainContractReaders map[string]types.ChainContractR

var eventSigsForContractFilter evmtypes.HashArray
for typeName, chainReaderDefinition := range chainContractReader.Configs {
injectEVMSpecificCodecModifiers(chainReaderDefinition)

switch chainReaderDefinition.ReadType {
case types.Method:
err = cr.addMethod(contractName, typeName, contractAbi, *chainReaderDefinition)
Expand Down Expand Up @@ -138,6 +140,23 @@ func (cr *chainReader) init(chainContractReaders map[string]types.ChainContractR
return nil
}

// injectEVMSpecificCodecModifiers injects an AddressModifier into Input/OutputModifications of a ChainReaderDefinition.
func injectEVMSpecificCodecModifiers(chainReaderDefinition *types.ChainReaderDefinition) {
for i, modConfig := range chainReaderDefinition.InputModifications {
if addrModifierConfig, ok := modConfig.(*commoncodec.AddressBytesToStringModifierConfig); ok {
addrModifierConfig.Modifier = codec.EVMAddressModifier{}
chainReaderDefinition.InputModifications[i] = addrModifierConfig
}
}

for i, modConfig := range chainReaderDefinition.OutputModifications {
if addrModifierConfig, ok := modConfig.(*commoncodec.AddressBytesToStringModifierConfig); ok {
addrModifierConfig.Modifier = codec.EVMAddressModifier{}
chainReaderDefinition.OutputModifications[i] = addrModifierConfig
}
}
}

func (cr *chainReader) Name() string { return cr.lggr.Name() }

// Start registers polling filters if contracts are already bound.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ func (cwh *ClientWithContractHistory) Init(_ context.Context, config types.Chain
continue
}

injectEVMSpecificCodecModifiers(readDef)

inputMod, err := readDef.InputModifications.ToModifier(codec.DecoderHooks...)
if err != nil {
return err
Expand Down
42 changes: 42 additions & 0 deletions core/services/relay/evm/codec/byte_string_modifier.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package codec

import (
"fmt"
"strings"

"github.com/ethereum/go-ethereum/common"

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

// EVMAddressModifier implements the AddressModifier interface for Ethereum addresses.
// It handles encoding and decoding Ethereum addresses with EIP-55 checksums and hex encoding.
type EVMAddressModifier struct{}

func (e EVMAddressModifier) EncodeAddress(bytes []byte) (string, error) {
if len(bytes) != e.Length() {
return "", fmt.Errorf("%w: got length %d, expected 20 for bytes %x", commontypes.ErrInvalidType, len(bytes), bytes)
}

return common.BytesToAddress(bytes).Hex(), nil
}

// DecodeAddress takes an EIP-55 encoded Ethereum address (e.g., "0x...") and decodes it to a 20-byte array.
func (e EVMAddressModifier) DecodeAddress(str string) ([]byte, error) {
str = strings.TrimPrefix(str, "0x")
if len(str) != 40 {
return nil, fmt.Errorf("%w: got length %d, expected 40 for address %s", commontypes.ErrInvalidType, len(str), str)
}

address := common.HexToAddress(str)
if address == (common.Address{}) {
return nil, fmt.Errorf("%w: address is zero", commontypes.ErrInvalidType)
}

return address.Bytes(), nil
}

// Length returns the expected length of an Ethereum address in bytes (20 bytes).
func (e EVMAddressModifier) Length() int {
return common.AddressLength
}
54 changes: 54 additions & 0 deletions core/services/relay/evm/codec/byte_string_modifier_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package codec_test

import (
"testing"

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

commontypes "github.com/smartcontractkit/chainlink-common/pkg/types"
"github.com/smartcontractkit/chainlink/v2/core/services/relay/evm/codec"
)

func TestEVMAddressModifier(t *testing.T) {
modifier := codec.EVMAddressModifier{}
validAddressStr := "0x5B38Da6a701c568545dCfcB03FcB875f56beddC4"
validAddressBytes := common.HexToAddress(validAddressStr).Bytes()
invalidLengthAddressStr := "0xabcdef1234567890abcdef"

t.Run("EncodeAddress encodes valid Ethereum address bytes", func(t *testing.T) {
encoded, err := modifier.EncodeAddress(validAddressBytes)
require.NoError(t, err)
assert.Equal(t, validAddressStr, encoded)
})

t.Run("EncodeAddress returns error for invalid byte length", func(t *testing.T) {
invalidBytes := []byte(invalidLengthAddressStr)
_, err := modifier.EncodeAddress(invalidBytes)
require.Error(t, err)
assert.Contains(t, err.Error(), commontypes.ErrInvalidType)
})

t.Run("DecodeAddress decodes valid Ethereum address", func(t *testing.T) {
decodedBytes, err := modifier.DecodeAddress(validAddressStr)
require.NoError(t, err)
assert.Equal(t, validAddressBytes, decodedBytes)
})

t.Run("DecodeAddress returns error for invalid address length", func(t *testing.T) {
_, err := modifier.DecodeAddress(invalidLengthAddressStr)
require.Error(t, err)
assert.Contains(t, err.Error(), commontypes.ErrInvalidType)
})

t.Run("DecodeAddress returns error for zero-value address", func(t *testing.T) {
_, err := modifier.DecodeAddress(common.Address{}.Hex())
require.Error(t, err)
assert.Contains(t, err.Error(), commontypes.ErrInvalidType)
})

t.Run("Length returns 20 for Ethereum addresses", func(t *testing.T) {
assert.Equal(t, common.AddressLength, modifier.Length())
})
}
16 changes: 16 additions & 0 deletions core/services/relay/evm/codec/codec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"math/big"
"testing"

"github.com/cometbft/cometbft/libs/strings"
"github.com/ethereum/go-ethereum/accounts/abi"
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/common/hexutil"
Expand Down Expand Up @@ -186,6 +187,10 @@ func (it *codecInterfaceTester) GetAccountBytes(i int) []byte {
return account[:]
}

func (it *codecInterfaceTester) GetAccountString(i int) string {
return common.BytesToAddress(it.GetAccountBytes(i)).Hex()
}

func (it *codecInterfaceTester) EncodeFields(t *testing.T, request *EncodeRequest) []byte {
if request.TestOn == TestItemType {
return encodeFieldsOnItem(t, request)
Expand All @@ -210,6 +215,15 @@ func (it *codecInterfaceTester) GetCodec(t *testing.T) commontypes.Codec {
}
}

if strings.StringInSlice(k, []string{TestItemType, TestItemSliceType, TestItemArray1Type, TestItemArray2Type, TestItemWithConfigExtra}) {
addressByteModifier := &commoncodec.AddressBytesToStringModifierConfig{
Fields: []string{"AccountStr"},
Modifier: codec.EVMAddressModifier{},
}

entry.ModifierConfigs = append(entry.ModifierConfigs, addressByteModifier)
}

if k == TestItemWithConfigExtra {
hardCode := &commoncodec.HardCodeModifierConfig{
OnChainValues: map[string]any{
Expand Down Expand Up @@ -307,6 +321,7 @@ var ts = []abi.ArgumentMarshaling{
{Name: "OracleId", Type: "uint8"},
{Name: "OracleIds", Type: "uint8[32]"},
{Name: "Account", Type: "address"},
{Name: "AccountStr", Type: "address"},
{Name: "Accounts", Type: "address[]"},
{Name: "BigField", Type: "int192"},
{Name: "NestedDynamicStruct", Type: "tuple", Components: nestedDynamic},
Expand Down Expand Up @@ -365,6 +380,7 @@ func argsFromTestStruct(ts TestStruct) []any {
uint8(ts.OracleID),
getOracleIDs(ts),
common.Address(ts.Account),
common.HexToAddress(ts.AccountStr),
getAccounts(ts),
ts.BigField,
evmtesting.MidDynamicToInternalType(ts.NestedDynamicStruct),
Expand Down
Loading

0 comments on commit 09145ba

Please sign in to comment.