Decouple ChainType from config string [SHIP-2001] (#13272)

* fix: Decouple ChainType from config string * fix: receiver name and failing test * test: enhance config test to test for xdai specifically * refactor: directly unmarshal into ChainType * fix: validation * test: fix TestDoc/EVM * test: add xdai to warnings.xtar
smartcontractkit · May 24, 2024 · 4fbcf7d · 4fbcf7d
1 parent b38cb0c
commit 4fbcf7d
Show file tree

Hide file tree

Showing 16 changed files with 231 additions and 68 deletions.
diff --git a/.changeset/young-candles-brush.md b/.changeset/young-candles-brush.md
@@ -0,0 +1,5 @@
+---
+"chainlink": patch
+---
+
+#bugfix allow ChainType to be set to xdai
diff --git a/common/config/chaintype.go b/common/config/chaintype.go
@@ -5,10 +5,8 @@ import (
 	"strings"
 )
 
-// ChainType denotes the chain or network to work with
 type ChainType string
 
-// nolint
 const (
 	ChainArbitrum        ChainType = "arbitrum"
 	ChainCelo            ChainType = "celo"
@@ -18,11 +16,103 @@ const (
 	ChainOptimismBedrock ChainType = "optimismBedrock"
 	ChainScroll          ChainType = "scroll"
 	ChainWeMix           ChainType = "wemix"
-	ChainXDai            ChainType = "xdai" // Deprecated: use ChainGnosis instead
 	ChainXLayer          ChainType = "xlayer"
 	ChainZkSync          ChainType = "zksync"
 )
 
+// IsL2 returns true if this chain is a Layer 2 chain. Notably:
+//   - the block numbers used for log searching are different from calling block.number
+//   - gas bumping is not supported, since there is no tx mempool
+func (c ChainType) IsL2() bool {
+	switch c {
+	case ChainArbitrum, ChainMetis:
+		return true
+	default:
+		return false
+	}
+}
+
+func (c ChainType) IsValid() bool {
+	switch c {
+	case "", ChainArbitrum, ChainCelo, ChainGnosis, ChainKroma, ChainMetis, ChainOptimismBedrock, ChainScroll, ChainWeMix, ChainXLayer, ChainZkSync:
+		return true
+	}
+	return false
+}
+
+func ChainTypeFromSlug(slug string) ChainType {
+	switch slug {
+	case "arbitrum":
+		return ChainArbitrum
+	case "celo":
+		return ChainCelo
+	case "gnosis", "xdai":
+		return ChainGnosis
+	case "kroma":
+		return ChainKroma
+	case "metis":
+		return ChainMetis
+	case "optimismBedrock":
+		return ChainOptimismBedrock
+	case "scroll":
+		return ChainScroll
+	case "wemix":
+		return ChainWeMix
+	case "xlayer":
+		return ChainXLayer
+	case "zksync":
+		return ChainZkSync
+	default:
+		return ChainType(slug)
+	}
+}
+
+type ChainTypeConfig struct {
+	value ChainType
+	slug  string
+}
+
+func NewChainTypeConfig(slug string) *ChainTypeConfig {
+	return &ChainTypeConfig{
+		value: ChainTypeFromSlug(slug),
+		slug:  slug,
+	}
+}
+
+func (c *ChainTypeConfig) MarshalText() ([]byte, error) {
+	if c == nil {
+		return nil, nil
+	}
+	return []byte(c.slug), nil
+}
+
+func (c *ChainTypeConfig) UnmarshalText(b []byte) error {
+	c.slug = string(b)
+	c.value = ChainTypeFromSlug(c.slug)
+	return nil
+}
+
+func (c *ChainTypeConfig) Slug() string {
+	if c == nil {
+		return ""
+	}
+	return c.slug
+}
+
+func (c *ChainTypeConfig) ChainType() ChainType {
+	if c == nil {
+		return ""
+	}
+	return c.value
+}
+
+func (c *ChainTypeConfig) String() string {
+	if c == nil {
+		return ""
+	}
+	return string(c.value)
+}
+
 var ErrInvalidChainType = fmt.Errorf("must be one of %s or omitted", strings.Join([]string{
 	string(ChainArbitrum),
 	string(ChainCelo),
@@ -35,24 +125,3 @@ var ErrInvalidChainType = fmt.Errorf("must be one of %s or omitted", strings.Joi
 	string(ChainXLayer),
 	string(ChainZkSync),
 }, ", "))
-
-// IsValid returns true if the ChainType value is known or empty.
-func (c ChainType) IsValid() bool {
-	switch c {
-	case "", ChainArbitrum, ChainCelo, ChainGnosis, ChainKroma, ChainMetis, ChainOptimismBedrock, ChainScroll, ChainWeMix, ChainXDai, ChainXLayer, ChainZkSync:
-		return true
-	}
-	return false
-}
-
-// IsL2 returns true if this chain is a Layer 2 chain. Notably:
-//   - the block numbers used for log searching are different from calling block.number
-//   - gas bumping is not supported, since there is no tx mempool
-func (c ChainType) IsL2() bool {
-	switch c {
-	case ChainArbitrum, ChainMetis:
-		return true
-	default:
-		return false
-	}
-}
diff --git a/core/chains/evm/client/config_builder.go b/core/chains/evm/client/config_builder.go
@@ -8,6 +8,7 @@ import (
 	"go.uber.org/multierr"
 
 	commonconfig "github.com/smartcontractkit/chainlink-common/pkg/config"
+	"github.com/smartcontractkit/chainlink/v2/common/config"
 
 	commonclient "github.com/smartcontractkit/chainlink/v2/common/client"
 	evmconfig "github.com/smartcontractkit/chainlink/v2/core/chains/evm/config"
@@ -55,7 +56,7 @@ func NewClientConfigs(
 	chainConfig := &evmconfig.EVMConfig{
 		C: &toml.EVMConfig{
 			Chain: toml.Chain{
-				ChainType:           &chainType,
+				ChainType:           config.NewChainTypeConfig(chainType),
 				FinalityDepth:       finalityDepth,
 				FinalityTagEnabled:  finalityTagEnabled,
 				NoNewHeadsThreshold: commonconfig.MustNewDuration(noNewHeadsThreshold),

diff --git a/core/chains/evm/client/pool_test.go b/core/chains/evm/client/pool_test.go
@@ -169,7 +169,6 @@ func TestPool_Dial(t *testing.T) {
 			if err == nil {
 				t.Cleanup(func() { assert.NoError(t, p.Close()) })
 			}
-			assert.True(t, p.ChainType().IsValid())
 			assert.False(t, p.ChainType().IsL2())
 			if test.errStr != "" {
 				require.Error(t, err)
@@ -333,7 +332,6 @@ func TestUnit_Pool_BatchCallContextAll(t *testing.T) {
 
 	p := evmclient.NewPool(logger.Test(t), defaultConfig.NodeSelectionMode(), defaultConfig.LeaseDuration(), time.Second*0, nodes, sendonlys, &cltest.FixtureChainID, "")
 
-	assert.True(t, p.ChainType().IsValid())
 	assert.False(t, p.ChainType().IsL2())
 	require.NoError(t, p.BatchCallContextAll(ctx, b))
 }

diff --git a/core/chains/evm/config/chain_scoped.go b/core/chains/evm/config/chain_scoped.go
@@ -128,7 +128,7 @@ func (e *EVMConfig) ChainType() commonconfig.ChainType {
 	if e.C.ChainType == nil {
 		return ""
 	}
-	return commonconfig.ChainType(*e.C.ChainType)
+	return e.C.ChainType.ChainType()
 }
 
 func (e *EVMConfig) ChainID() *big.Int {

diff --git a/core/chains/evm/config/config_test.go b/core/chains/evm/config/config_test.go
@@ -406,7 +406,7 @@ func Test_chainScopedConfig_Validate(t *testing.T) {
 	t.Run("arbitrum-estimator", func(t *testing.T) {
 		t.Run("custom", func(t *testing.T) {
 			cfg := configWithChains(t, 0, &toml.Chain{
-				ChainType: ptr(string(commonconfig.ChainArbitrum)),
+				ChainType: commonconfig.NewChainTypeConfig(string(commonconfig.ChainArbitrum)),
 				GasEstimator: toml.GasEstimator{
 					Mode: ptr("BlockHistory"),
 				},
@@ -437,7 +437,7 @@ func Test_chainScopedConfig_Validate(t *testing.T) {
 	t.Run("optimism-estimator", func(t *testing.T) {
 		t.Run("custom", func(t *testing.T) {
 			cfg := configWithChains(t, 0, &toml.Chain{
-				ChainType: ptr(string(commonconfig.ChainOptimismBedrock)),
+				ChainType: commonconfig.NewChainTypeConfig(string(commonconfig.ChainOptimismBedrock)),
 				GasEstimator: toml.GasEstimator{
 					Mode: ptr("BlockHistory"),
 				},

diff --git a/core/chains/evm/config/toml/config.go b/core/chains/evm/config/toml/config.go
@@ -294,18 +294,14 @@ func (c *EVMConfig) ValidateConfig() (err error) {
 	} else if c.ChainID.String() == "" {
 		err = multierr.Append(err, commonconfig.ErrEmpty{Name: "ChainID", Msg: "required for all chains"})
 	} else if must, ok := ChainTypeForID(c.ChainID); ok { // known chain id
-		if c.ChainType == nil && must != "" {
-			err = multierr.Append(err, commonconfig.ErrMissing{Name: "ChainType",
-				Msg: fmt.Sprintf("only %q can be used with this chain id", must)})
-		} else if c.ChainType != nil && *c.ChainType != string(must) {
-			if *c.ChainType == "" {
-				err = multierr.Append(err, commonconfig.ErrEmpty{Name: "ChainType",
-					Msg: fmt.Sprintf("only %q can be used with this chain id", must)})
-			} else if must == "" {
-				err = multierr.Append(err, commonconfig.ErrInvalid{Name: "ChainType", Value: *c.ChainType,
+		// Check if the parsed value matched the expected value
+		is := c.ChainType.ChainType()
+		if is != must {
+			if must == "" {
+				err = multierr.Append(err, commonconfig.ErrInvalid{Name: "ChainType", Value: c.ChainType.ChainType(),
 					Msg: "must not be set with this chain id"})
 			} else {
-				err = multierr.Append(err, commonconfig.ErrInvalid{Name: "ChainType", Value: *c.ChainType,
+				err = multierr.Append(err, commonconfig.ErrInvalid{Name: "ChainType", Value: c.ChainType.ChainType(),
 					Msg: fmt.Sprintf("only %q can be used with this chain id", must)})
 			}
 		}
@@ -345,7 +341,7 @@ type Chain struct {
 	AutoCreateKey             *bool
 	BlockBackfillDepth        *uint32
 	BlockBackfillSkip         *bool
-	ChainType                 *string
+	ChainType                 *config.ChainTypeConfig
 	FinalityDepth             *uint32
 	FinalityTagEnabled        *bool
 	FlagsContractAddress      *types.EIP55Address
@@ -375,12 +371,8 @@ type Chain struct {
 }
 
 func (c *Chain) ValidateConfig() (err error) {
-	var chainType config.ChainType
-	if c.ChainType != nil {
-		chainType = config.ChainType(*c.ChainType)
-	}
-	if !chainType.IsValid() {
-		err = multierr.Append(err, commonconfig.ErrInvalid{Name: "ChainType", Value: *c.ChainType,
+	if !c.ChainType.ChainType().IsValid() {
+		err = multierr.Append(err, commonconfig.ErrInvalid{Name: "ChainType", Value: c.ChainType.ChainType(),
 			Msg: config.ErrInvalidChainType.Error()})
 	}
 

diff --git a/core/chains/evm/config/toml/defaults.go b/core/chains/evm/config/toml/defaults.go
@@ -94,10 +94,7 @@ func Defaults(chainID *big.Big, with ...*Chain) Chain {
 func ChainTypeForID(chainID *big.Big) (config.ChainType, bool) {
 	s := chainID.String()
 	if d, ok := defaults[s]; ok {
-		if d.ChainType == nil {
-			return "", true
-		}
-		return config.ChainType(*d.ChainType), true
+		return d.ChainType.ChainType(), true
 	}
 	return "", false
 }

diff --git a/core/chains/evm/gas/block_history_estimator_test.go b/core/chains/evm/gas/block_history_estimator_test.go
@@ -993,11 +993,6 @@ func TestBlockHistoryEstimator_Recalculate_NoEIP1559(t *testing.T) {
 		bhe.Recalculate(cltest.Head(0))
 		require.Equal(t, assets.NewWeiI(80), gas.GetGasPrice(bhe))
 
-		// Same for xDai (deprecated)
-		cfg.ChainTypeF = string(config.ChainXDai)
-		bhe.Recalculate(cltest.Head(0))
-		require.Equal(t, assets.NewWeiI(80), gas.GetGasPrice(bhe))
-
 		// And for X Layer
 		cfg.ChainTypeF = string(config.ChainXLayer)
 		bhe.Recalculate(cltest.Head(0))

diff --git a/core/chains/evm/gas/chain_specific.go b/core/chains/evm/gas/chain_specific.go
@@ -9,7 +9,7 @@ import (
 // chainSpecificIsUsable allows for additional logic specific to a particular
 // Config that determines whether a transaction should be used for gas estimation
 func chainSpecificIsUsable(tx evmtypes.Transaction, baseFee *assets.Wei, chainType config.ChainType, minGasPriceWei *assets.Wei) bool {
-	if chainType == config.ChainGnosis || chainType == config.ChainXDai || chainType == config.ChainXLayer {
+	if chainType == config.ChainGnosis || chainType == config.ChainXLayer {
 		// GasPrice 0 on most chains is great since it indicates cheap/free transactions.
 		// However, Gnosis and XLayer reserve a special type of "bridge" transaction with 0 gas
 		// price that is always processed at top priority. Ordinary transactions

diff --git a/core/config/docs/docs_test.go b/core/config/docs/docs_test.go
@@ -15,6 +15,7 @@ import (
 	stkcfg "github.com/smartcontractkit/chainlink-starknet/relayer/pkg/chainlink/config"
 
 	"github.com/smartcontractkit/chainlink-common/pkg/config"
+	commonconfig "github.com/smartcontractkit/chainlink/v2/common/config"
 	"github.com/smartcontractkit/chainlink/v2/core/chains/evm/assets"
 	evmcfg "github.com/smartcontractkit/chainlink/v2/core/chains/evm/config/toml"
 	"github.com/smartcontractkit/chainlink/v2/core/chains/evm/types"
@@ -46,7 +47,7 @@ func TestDoc(t *testing.T) {
 		fallbackDefaults := evmcfg.Defaults(nil)
 		docDefaults := defaults.EVM[0].Chain
 
-		require.Equal(t, "", *docDefaults.ChainType)
+		require.Equal(t, commonconfig.ChainType(""), docDefaults.ChainType.ChainType())
 		docDefaults.ChainType = nil
 
 		// clean up KeySpecific as a special case

diff --git a/core/services/chainlink/config.go b/core/services/chainlink/config.go
@@ -12,7 +12,6 @@ import (
 	"github.com/smartcontractkit/chainlink-solana/pkg/solana"
 	stkcfg "github.com/smartcontractkit/chainlink-starknet/relayer/pkg/chainlink/config"
 
-	commoncfg "github.com/smartcontractkit/chainlink/v2/common/config"
 	evmcfg "github.com/smartcontractkit/chainlink/v2/core/chains/evm/config/toml"
 	"github.com/smartcontractkit/chainlink/v2/core/config/docs"
 	"github.com/smartcontractkit/chainlink/v2/core/config/env"
@@ -79,10 +78,10 @@ func (c *Config) valueWarnings() (err error) {
 func (c *Config) deprecationWarnings() (err error) {
 	// ChainType xdai is deprecated and has been renamed to gnosis
 	for _, evm := range c.EVM {
-		if evm.ChainType != nil && *evm.ChainType == string(commoncfg.ChainXDai) {
+		if evm.ChainType != nil && evm.ChainType.Slug() == "xdai" {
 			err = multierr.Append(err, config.ErrInvalid{
 				Name:  "EVM.ChainType",
-				Value: *evm.ChainType,
+				Value: evm.ChainType.Slug(),
 				Msg:   "deprecated and will be removed in v2.13.0, use 'gnosis' instead",
 			})
 		}

diff --git a/core/services/chainlink/config_test.go b/core/services/chainlink/config_test.go
@@ -26,8 +26,8 @@ import (
 	solcfg "github.com/smartcontractkit/chainlink-solana/pkg/solana/config"
 	stkcfg "github.com/smartcontractkit/chainlink-starknet/relayer/pkg/chainlink/config"
 	commonconfig "github.com/smartcontractkit/chainlink/v2/common/config"
-
 	"github.com/smartcontractkit/chainlink/v2/core/chains/evm/assets"
+
 	"github.com/smartcontractkit/chainlink/v2/core/chains/evm/client"
 	evmcfg "github.com/smartcontractkit/chainlink/v2/core/chains/evm/config/toml"
 	"github.com/smartcontractkit/chainlink/v2/core/chains/evm/types"
@@ -495,7 +495,7 @@ func TestConfig_Marshal(t *testing.T) {
 				},
 				BlockBackfillDepth:   ptr[uint32](100),
 				BlockBackfillSkip:    ptr(true),
-				ChainType:            ptr("Optimism"),
+				ChainType:            commonconfig.NewChainTypeConfig("Optimism"),
 				FinalityDepth:        ptr[uint32](42),
 				FinalityTagEnabled:   ptr[bool](false),
 				FlagsContractAddress: mustAddress("0xae4E781a6218A8031764928E88d457937A954fC3"),
@@ -1626,7 +1626,7 @@ func TestConfig_warnings(t *testing.T) {
 		{
 			name: "Value warning - ChainType=xdai is deprecated",
 			config: Config{
-				EVM: evmcfg.EVMConfigs{{Chain: evmcfg.Chain{ChainType: ptr(string(commonconfig.ChainXDai))}}},
+				EVM: evmcfg.EVMConfigs{{Chain: evmcfg.Chain{ChainType: commonconfig.NewChainTypeConfig("xdai")}}},
 			},
 			expectedErrors: []string{"EVM.ChainType: invalid value (xdai): deprecated and will be removed in v2.13.0, use 'gnosis' instead"},
 		},

diff --git a/core/services/ocr/contract_tracker.go b/core/services/ocr/contract_tracker.go
@@ -400,7 +400,7 @@ func (t *OCRContractTracker) LatestBlockHeight(ctx context.Context) (blockheight
 		// care about the block height; we have no way of getting the L1 block
 		// height anyway
 		return 0, nil
-	case "", config.ChainArbitrum, config.ChainCelo, config.ChainGnosis, config.ChainKroma, config.ChainOptimismBedrock, config.ChainScroll, config.ChainWeMix, config.ChainXDai, config.ChainXLayer, config.ChainZkSync:
+	case "", config.ChainArbitrum, config.ChainCelo, config.ChainGnosis, config.ChainKroma, config.ChainOptimismBedrock, config.ChainScroll, config.ChainWeMix, config.ChainXLayer, config.ChainZkSync:
 		// continue
 	}
 	latestBlockHeight := t.getLatestBlockHeight()

diff --git a/core/services/ocrcommon/block_translator.go b/core/services/ocrcommon/block_translator.go
@@ -21,7 +21,7 @@ func NewBlockTranslator(cfg Config, client evmclient.Client, lggr logger.Logger)
 	switch cfg.ChainType() {
 	case config.ChainArbitrum:
 		return NewArbitrumBlockTranslator(client, lggr)
-	case "", config.ChainCelo, config.ChainGnosis, config.ChainKroma, config.ChainMetis, config.ChainOptimismBedrock, config.ChainScroll, config.ChainWeMix, config.ChainXDai, config.ChainXLayer, config.ChainZkSync:
+	case "", config.ChainCelo, config.ChainGnosis, config.ChainKroma, config.ChainMetis, config.ChainOptimismBedrock, config.ChainScroll, config.ChainWeMix, config.ChainXLayer, config.ChainZkSync:
 		fallthrough
 	default:
 		return &l1BlockTranslator{}