From 26f0f607fc34ee41e1d4ee55384f9f6a58894f97 Mon Sep 17 00:00:00 2001 From: nagdahimanshu Date: Mon, 22 Jan 2024 11:12:37 +0100 Subject: [PATCH] :ok_hand: Applied suggestions from code review --- pkg/chain/chain_constants.go | 16 +++++++++------- pkg/chain/chain_constants_test.go | 8 ++++---- pkg/chain/contracts.go | 18 +++++++++--------- pkg/chain/contracts_test.go | 8 ++++---- 4 files changed, 26 insertions(+), 24 deletions(-) diff --git a/pkg/chain/chain_constants.go b/pkg/chain/chain_constants.go index aa8f40c..8b465bb 100644 --- a/pkg/chain/chain_constants.go +++ b/pkg/chain/chain_constants.go @@ -1,7 +1,5 @@ package chain -import "fmt" - // L2ChainIDs manages L2 network chainIDs. type L2ChainIDs struct { optimism uint64 @@ -48,9 +46,10 @@ type Contracts struct { networkType NetworkType } -// GetContractAddressesByChainID returns contract addresses by chainID. -func GetContractAddressesByChainID(chainID uint64) (Contracts, error) { - contractAddresses := map[uint64]Contracts{ +var contractAddresses map[uint64]Contracts + +func init() { + contractAddresses = map[uint64]Contracts{ l2NetworkChainIDs.optimism: { stateCommitmentChain: "0xBe5dAb4A2e9cd0F27300dB4aB94BeE3A233AEB19", optimismPortal: "0xbEb5Fc579115071764c7423A4f12eDde41f106Ed", @@ -123,12 +122,15 @@ func GetContractAddressesByChainID(chainID uint64) (Contracts, error) { networkType: L1, }, } +} +// GetContractAddressesByChainID returns contract addresses by network chainID. +func GetContractAddressesByChainID(chainID uint64) (Contracts, bool) { filteredContracts := contractAddresses[chainID] if len(filteredContracts.l2OutputOracle) == 0 { - return filteredContracts, fmt.Errorf("contract information is unavailable for the chain %v", chainID) + return filteredContracts, false } - return filteredContracts, nil + return filteredContracts, true } diff --git a/pkg/chain/chain_constants_test.go b/pkg/chain/chain_constants_test.go index 4d7f920..e7ffea9 100644 --- a/pkg/chain/chain_constants_test.go +++ b/pkg/chain/chain_constants_test.go @@ -10,18 +10,18 @@ func TestGetContractAddressesByChainID(t *testing.T) { assert := assert.New(t) const availableChainID uint64 = 10 - contractAddresses, err := GetContractAddressesByChainID(availableChainID) + contractAddresses, areAddressesAvailable := GetContractAddressesByChainID(availableChainID) contractAddressesExpected := Contracts{ stateCommitmentChain: "0xBe5dAb4A2e9cd0F27300dB4aB94BeE3A233AEB19", optimismPortal: "0xbEb5Fc579115071764c7423A4f12eDde41f106Ed", l2OutputOracle: "0xdfe97868233d1aa22e815a266982f2cf17685a27", networkType: "L1", } - assert.NoError(err) + assert.Equal(true, areAddressesAvailable) assert.Equal(contractAddressesExpected, contractAddresses) const unavailableChainID uint64 = 5 - contractAddresses, err = GetContractAddressesByChainID(unavailableChainID) - assert.Error(err) + contractAddresses, areAddressesAvailable = GetContractAddressesByChainID(unavailableChainID) + assert.Equal(false, areAddressesAvailable) assert.Equal(0, len(contractAddresses.l2OutputOracle)) } diff --git a/pkg/chain/contracts.go b/pkg/chain/contracts.go index 9b00557..0f55efd 100644 --- a/pkg/chain/contracts.go +++ b/pkg/chain/contracts.go @@ -24,31 +24,31 @@ type ConfigOptions struct { L2OutputOracleContractAddress string } -func getL1OracleContractAddressByChainID(chainID uint64) (string, error) { - cAddr, err := GetContractAddressesByChainID(chainID) - if err != nil { - return "", err +func getL1OracleContractAddressByChainID(chainID uint64) (string, bool) { + cAddr, areAddressesAvailable := GetContractAddressesByChainID(chainID) + if !areAddressesAvailable { + return "", false } - return cAddr.l2OutputOracle, nil + return cAddr.l2OutputOracle, true } // NewOracleContract returns [OracleAccessor] with contract instance. -func NewOracleContract(ctx context.Context, opts ConfigOptions) (*OracleAccessor, error) { +func NewOracleContract(ctx context.Context, opts *ConfigOptions) (*OracleAccessor, error) { client, err := ethclient.DialContext(ctx, opts.L1RPCEndpoint) if err != nil { return nil, err } - oracleContractAddress, err := getL1OracleContractAddressByChainID(opts.ChainID) + oracleContractAddress, isAddressExists := getL1OracleContractAddressByChainID(opts.ChainID) // Verify if oracle contract address is available in the chain constants // If not available, use l2OutputContractAddress from the config options - if err != nil { + if !isAddressExists { if len(opts.L2OutputOracleContractAddress) > 0 { oracleContractAddress = opts.L2OutputOracleContractAddress } else { - return nil, fmt.Errorf("L2 output oracle contract address is not available") + return nil, fmt.Errorf("L2OutputOracleContractAddress is not available") } } diff --git a/pkg/chain/contracts_test.go b/pkg/chain/contracts_test.go index 09ff3c2..6d6458d 100644 --- a/pkg/chain/contracts_test.go +++ b/pkg/chain/contracts_test.go @@ -11,12 +11,12 @@ func TestGetL1OracleContractAddressByChainID(t *testing.T) { const availableChainID uint64 = 10 oracleContractAddressExpected := "0xdfe97868233d1aa22e815a266982f2cf17685a27" - contractAddress, err := getL1OracleContractAddressByChainID(availableChainID) - assert.NoError(err) + contractAddress, isAddressExists := getL1OracleContractAddressByChainID(availableChainID) + assert.Equal(true, isAddressExists) assert.Equal(oracleContractAddressExpected, contractAddress) const unavailableChainID uint64 = 5 - contractAddress, err = getL1OracleContractAddressByChainID(unavailableChainID) - assert.Error(err) + contractAddress, isAddressExists = getL1OracleContractAddressByChainID(unavailableChainID) + assert.Equal(false, isAddressExists) assert.Equal(0, len(contractAddress)) }