Adding setters to CommitStore for GasEstimator and SourceMaxGasPrice (#…

…1047)

## Motivation

CommitStore currently is constructed with dependencies from both a
Source Chain Relayer and Dest Chain Relayer. This violates a key
constraint of our system when moving to LOOPPs. This change is one step
to separate the construction of a `CommitStore` from requiring the
source max gas price and source gas price estimator.
## Solution

Instead of setting `sourceMaxGasPrice` and `estimator` in the
construction of a `CommitStore`, we now use explicit setter methods.

This will be consumed in future PRs to add a provider based
implementation of a gas estimator in the reporting plugin factory.

---------

Co-authored-by: dimitris <[email protected]>

.github/workflows/ci-scripts.yml

@@ -14,6 +14,7 @@ jobs:
         with:
           id: scripts
           name: lint-scripts
+          version: v1.56
           go-directory: core/scripts/ccip
           go-version-file: core/scripts/go.mod
           go-module-file: core/scripts/go.sum

core/services/ocr2/plugins/ccip/ccipcommit/initializers.go

@@ -106,7 +106,7 @@ func UnregisterCommitPluginLpFilters(ctx context.Context, lggr logger.Logger, jb
 	versionFinder := factory.NewEvmVersionFinder()
 	unregisterFuncs := []func() error{
 		func() error {
-			return factory.CloseCommitStoreReader(lggr, versionFinder, params.commitStoreAddress, params.destChain.Client(), params.destChain.LogPoller(), params.sourceChain.GasEstimator(), params.sourceChain.Config().EVM().GasEstimator().PriceMax().ToInt())
+			return factory.CloseCommitStoreReader(lggr, versionFinder, params.commitStoreAddress, params.destChain.Client(), params.destChain.LogPoller())
 		},
 		func() error {
 			return factory.CloseOnRampReader(lggr, versionFinder, params.commitStoreStaticCfg.SourceChainSelector, params.commitStoreStaticCfg.ChainSelector, cciptypes.Address(params.commitStoreStaticCfg.OnRamp.String()), params.sourceChain.LogPoller(), params.sourceChain.Client())
@@ -140,10 +140,21 @@ func jobSpecToCommitPluginConfig(ctx context.Context, orm cciporm.ORM, lggr logg
 		"DestChainSelector", params.commitStoreStaticCfg.ChainSelector)
 
 	versionFinder := factory.NewEvmVersionFinder()
-	commitStoreReader, err := factory.NewCommitStoreReader(lggr, versionFinder, params.commitStoreAddress, params.destChain.Client(), params.destChain.LogPoller(), params.sourceChain.GasEstimator(), params.sourceChain.Config().EVM().GasEstimator().PriceMax().ToInt())
+	commitStoreReader, err := factory.NewCommitStoreReader(lggr, versionFinder, params.commitStoreAddress, params.destChain.Client(), params.destChain.LogPoller())
 	if err != nil {
 		return nil, nil, nil, nil, errors.Wrap(err, "could not create commitStore reader")
 	}
+
+	err = commitStoreReader.SetGasEstimator(ctx, params.sourceChain.GasEstimator())
+	if err != nil {
+		return nil, nil, nil, nil, fmt.Errorf("could not set gas estimator: %w", err)
+	}
+
+	err = commitStoreReader.SetSourceMaxGasPrice(ctx, params.sourceChain.Config().EVM().GasEstimator().PriceMax().ToInt())
+	if err != nil {
+		return nil, nil, nil, nil, fmt.Errorf("could not set source max gas price: %w", err)
+	}
+
 	sourceChainName, destChainName, err := ccipconfig.ResolveChainNames(params.sourceChain.ID().Int64(), params.destChain.ID().Int64())
 	if err != nil {
 		return nil, nil, nil, nil, err

core/services/ocr2/plugins/ccip/ccipcommit/ocr2_test.go

@@ -344,7 +344,7 @@ func TestCommitReportingPlugin_Report(t *testing.T) {
 			})).Return(destDecimals, nil).Maybe()
 
 			lp := mocks2.NewLogPoller(t)
-			commitStoreReader, err := v1_2_0.NewCommitStore(logger.TestLogger(t), utils.RandomAddress(), nil, lp, nil, nil)
+			commitStoreReader, err := v1_2_0.NewCommitStore(logger.TestLogger(t), utils.RandomAddress(), nil, lp)
 			assert.NoError(t, err)
 
 			healthCheck := ccipcachemocks.NewChainHealthcheck(t)

core/services/ocr2/plugins/ccip/ccipexec/initializers.go

@@ -113,7 +113,7 @@ func UnregisterExecPluginLpFilters(ctx context.Context, lggr logger.Logger, jb j
 	versionFinder := factory.NewEvmVersionFinder()
 	unregisterFuncs := []func() error{
 		func() error {
-			return factory.CloseCommitStoreReader(lggr, versionFinder, params.offRampConfig.CommitStore, params.destChain.Client(), params.destChain.LogPoller(), params.sourceChain.GasEstimator(), params.sourceChain.Config().EVM().GasEstimator().PriceMax().ToInt())
+			return factory.CloseCommitStoreReader(lggr, versionFinder, params.offRampConfig.CommitStore, params.destChain.Client(), params.destChain.LogPoller())
 		},
 		func() error {
 			return factory.CloseOnRampReader(lggr, versionFinder, params.offRampConfig.SourceChainSelector, params.offRampConfig.ChainSelector, params.offRampConfig.OnRamp, params.sourceChain.LogPoller(), params.sourceChain.Client())
@@ -223,11 +223,21 @@ func jobSpecToExecPluginConfig(ctx context.Context, lggr logger.Logger, jb job.J
 		return nil, nil, nil, nil, errors.Wrap(err, "could not get source native token")
 	}
 
-	commitStoreReader, err := factory.NewCommitStoreReader(lggr, versionFinder, params.offRampConfig.CommitStore, params.destChain.Client(), params.destChain.LogPoller(), params.sourceChain.GasEstimator(), params.sourceChain.Config().EVM().GasEstimator().PriceMax().ToInt())
+	commitStoreReader, err := factory.NewCommitStoreReader(lggr, versionFinder, params.offRampConfig.CommitStore, params.destChain.Client(), params.destChain.LogPoller())
 	if err != nil {
 		return nil, nil, nil, nil, errors.Wrap(err, "could not load commitStoreReader reader")
 	}
 
+	err = commitStoreReader.SetGasEstimator(ctx, params.sourceChain.GasEstimator())
+	if err != nil {
+		return nil, nil, nil, nil, fmt.Errorf("could not set gas estimator: %w", err)
+	}
+
+	err = commitStoreReader.SetSourceMaxGasPrice(ctx, params.sourceChain.Config().EVM().GasEstimator().PriceMax().ToInt())
+	if err != nil {
+		return nil, nil, nil, nil, fmt.Errorf("could not set source max gas price: %w", err)
+	}
+
 	tokenDataProviders, err := initTokenDataProviders(lggr, jobIDToString(jb.ID), params.pluginConfig, params.sourceChain.LogPoller())
 	if err != nil {
 		return nil, nil, nil, nil, errors.Wrap(err, "could not get token data providers")

core/services/ocr2/plugins/ccip/internal/ccipdata/commit_store_reader.go

@@ -1,8 +1,12 @@
 package ccipdata
 
 import (
+	"context"
+	"math/big"
 	"time"
 
+	"github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas"
+
 	"github.com/ethereum/go-ethereum/accounts/abi/bind"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/pkg/errors"
@@ -56,6 +60,8 @@ func NewCommitOffchainConfig(
 //go:generate mockery --quiet --name CommitStoreReader --filename commit_store_reader_mock.go --case=underscore
 type CommitStoreReader interface {
 	cciptypes.CommitStoreReader
+	SetGasEstimator(ctx context.Context, gpe gas.EvmFeeEstimator) error
+	SetSourceMaxGasPrice(ctx context.Context, sourceMaxGasPrice *big.Int) error
 }
 
 // FetchCommitStoreStaticConfig provides access to a commitStore's static config, which is required to access the source chain ID.

core/services/ocr2/plugins/ccip/internal/ccipdata/commit_store_reader_test.go

@@ -189,10 +189,18 @@ func TestCommitStoreReaders(t *testing.T) {
 	ge.On("L1Oracle").Return(lm)
 
 	maxGasPrice := big.NewInt(1e8)
-	c10r, err := factory.NewCommitStoreReader(lggr, factory.NewEvmVersionFinder(), ccipcalc.EvmAddrToGeneric(addr), ec, lp, ge, maxGasPrice)
+	c10r, err := factory.NewCommitStoreReader(lggr, factory.NewEvmVersionFinder(), ccipcalc.EvmAddrToGeneric(addr), ec, lp) // ge, maxGasPrice
+	require.NoError(t, err)
+	err = c10r.SetGasEstimator(ctx, ge)
+	require.NoError(t, err)
+	err = c10r.SetSourceMaxGasPrice(ctx, maxGasPrice)
 	require.NoError(t, err)
 	assert.Equal(t, reflect.TypeOf(c10r).String(), reflect.TypeOf(&v1_0_0.CommitStore{}).String())
-	c12r, err := factory.NewCommitStoreReader(lggr, factory.NewEvmVersionFinder(), ccipcalc.EvmAddrToGeneric(addr2), ec, lp, ge, maxGasPrice)
+	c12r, err := factory.NewCommitStoreReader(lggr, factory.NewEvmVersionFinder(), ccipcalc.EvmAddrToGeneric(addr2), ec, lp)
+	require.NoError(t, err)
+	err = c12r.SetGasEstimator(ctx, ge)
+	require.NoError(t, err)
+	err = c12r.SetSourceMaxGasPrice(ctx, maxGasPrice)
 	require.NoError(t, err)
 	assert.Equal(t, reflect.TypeOf(c12r).String(), reflect.TypeOf(&v1_2_0.CommitStore{}).String())
 
@@ -399,7 +407,7 @@ func TestNewCommitStoreReader(t *testing.T) {
 			if tc.expectedErr == "" {
 				lp.On("RegisterFilter", mock.Anything, mock.Anything).Return(nil)
 			}
-			_, err = factory.NewCommitStoreReader(logger.TestLogger(t), factory.NewEvmVersionFinder(), addr, c, lp, nil, nil)
+			_, err = factory.NewCommitStoreReader(logger.TestLogger(t), factory.NewEvmVersionFinder(), addr, c, lp)
 			if tc.expectedErr != "" {
 				require.EqualError(t, err, tc.expectedErr)
 			} else {

core/services/ocr2/plugins/ccip/internal/ccipdata/factory/commit_store.go

@@ -1,15 +1,12 @@
 package factory
 
 import (
-	"math/big"
-
 	"github.com/Masterminds/semver/v3"
 	"github.com/pkg/errors"
 
 	cciptypes "github.com/smartcontractkit/chainlink-common/pkg/types/ccip"
 
 	"github.com/smartcontractkit/chainlink/v2/core/chains/evm/client"
-	"github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas"
 	"github.com/smartcontractkit/chainlink/v2/core/chains/evm/logpoller"
 	"github.com/smartcontractkit/chainlink/v2/core/chains/evm/txmgr"
 	"github.com/smartcontractkit/chainlink/v2/core/gethwrappers/ccip/generated/commit_store"
@@ -24,16 +21,16 @@ import (
 	"github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ccip/internal/ccipdata/v1_5_0"
 )
 
-func NewCommitStoreReader(lggr logger.Logger, versionFinder VersionFinder, address cciptypes.Address, ec client.Client, lp logpoller.LogPoller, estimator gas.EvmFeeEstimator, sourceMaxGasPrice *big.Int) (ccipdata.CommitStoreReader, error) {
-	return initOrCloseCommitStoreReader(lggr, versionFinder, address, ec, lp, estimator, sourceMaxGasPrice, false)
+func NewCommitStoreReader(lggr logger.Logger, versionFinder VersionFinder, address cciptypes.Address, ec client.Client, lp logpoller.LogPoller) (ccipdata.CommitStoreReader, error) {
+	return initOrCloseCommitStoreReader(lggr, versionFinder, address, ec, lp, false)
 }
 
-func CloseCommitStoreReader(lggr logger.Logger, versionFinder VersionFinder, address cciptypes.Address, ec client.Client, lp logpoller.LogPoller, estimator gas.EvmFeeEstimator, sourceMaxGasPrice *big.Int) error {
-	_, err := initOrCloseCommitStoreReader(lggr, versionFinder, address, ec, lp, estimator, sourceMaxGasPrice, true)
+func CloseCommitStoreReader(lggr logger.Logger, versionFinder VersionFinder, address cciptypes.Address, ec client.Client, lp logpoller.LogPoller) error {
+	_, err := initOrCloseCommitStoreReader(lggr, versionFinder, address, ec, lp, true)
 	return err
 }
 
-func initOrCloseCommitStoreReader(lggr logger.Logger, versionFinder VersionFinder, address cciptypes.Address, ec client.Client, lp logpoller.LogPoller, estimator gas.EvmFeeEstimator, sourceMaxGasPrice *big.Int, closeReader bool) (ccipdata.CommitStoreReader, error) {
+func initOrCloseCommitStoreReader(lggr logger.Logger, versionFinder VersionFinder, address cciptypes.Address, ec client.Client, lp logpoller.LogPoller, closeReader bool) (ccipdata.CommitStoreReader, error) {
 	contractType, version, err := versionFinder.TypeAndVersion(address, ec)
 	if err != nil {
 		return nil, errors.Wrapf(err, "unable to read type and version")
@@ -47,11 +44,11 @@ func initOrCloseCommitStoreReader(lggr logger.Logger, versionFinder VersionFinde
 		return nil, err
 	}
 
-	lggr.Infow("Initializing CommitStore Reader", "version", version.String(), "sourceMaxGasPrice", sourceMaxGasPrice.String())
+	lggr.Infow("Initializing CommitStore Reader", "version", version.String())
 
 	switch version.String() {
 	case ccipdata.V1_0_0, ccipdata.V1_1_0: // Versions are identical
-		cs, err := v1_0_0.NewCommitStore(lggr, evmAddr, ec, lp, estimator, sourceMaxGasPrice)
+		cs, err := v1_0_0.NewCommitStore(lggr, evmAddr, ec, lp)
 		if err != nil {
 			return nil, err
 		}
@@ -60,7 +57,7 @@ func initOrCloseCommitStoreReader(lggr logger.Logger, versionFinder VersionFinde
 		}
 		return cs, cs.RegisterFilters()
 	case ccipdata.V1_2_0:
-		cs, err := v1_2_0.NewCommitStore(lggr, evmAddr, ec, lp, estimator, sourceMaxGasPrice)
+		cs, err := v1_2_0.NewCommitStore(lggr, evmAddr, ec, lp)
 		if err != nil {
 			return nil, err
 		}
@@ -69,7 +66,7 @@ func initOrCloseCommitStoreReader(lggr logger.Logger, versionFinder VersionFinde
 		}
 		return cs, cs.RegisterFilters()
 	case ccipdata.V1_5_0:
-		cs, err := v1_5_0.NewCommitStore(lggr, evmAddr, ec, lp, estimator, sourceMaxGasPrice)
+		cs, err := v1_5_0.NewCommitStore(lggr, evmAddr, ec, lp)
 		if err != nil {
 			return nil, err
 		}

core/services/ocr2/plugins/ccip/internal/ccipdata/factory/commit_store_test.go

@@ -26,12 +26,12 @@ func TestCommitStore(t *testing.T) {
 
 		lp.On("RegisterFilter", mock.Anything, mock.Anything).Return(nil)
 		versionFinder := newMockVersionFinder(ccipconfig.CommitStore, *semver.MustParse(versionStr), nil)
-		_, err := NewCommitStoreReader(lggr, versionFinder, addr, nil, lp, nil, nil)
+		_, err := NewCommitStoreReader(lggr, versionFinder, addr, nil, lp)
 		assert.NoError(t, err)
 
 		expFilterName := logpoller.FilterName(v1_0_0.EXEC_REPORT_ACCEPTS, addr)
 		lp.On("UnregisterFilter", mock.Anything, expFilterName).Return(nil)
-		err = CloseCommitStoreReader(lggr, versionFinder, addr, nil, lp, nil, nil)
+		err = CloseCommitStoreReader(lggr, versionFinder, addr, nil, lp)
 		assert.NoError(t, err)
 	}
 }

core/services/ocr2/plugins/ccip/internal/ccipdata/v1_0_0/commit_store.go

@@ -44,7 +44,7 @@ type CommitStore struct {
 	lggr                      logger.Logger
 	lp                        logpoller.LogPoller
 	address                   common.Address
-	estimator                 gas.EvmFeeEstimator
+	estimator                 *gas.EvmFeeEstimator
 	sourceMaxGasPrice         *big.Int
 	filters                   []logpoller.Filter
 	reportAcceptedSig         common.Hash
@@ -183,6 +183,20 @@ func (c *CommitStore) GasPriceEstimator(context.Context) (cciptypes.GasPriceEsti
 	return c.gasPriceEstimator, nil
 }
 
+func (c *CommitStore) SetGasEstimator(ctx context.Context, gpe gas.EvmFeeEstimator) error {
+	c.configMu.RLock()
+	defer c.configMu.RUnlock()
+	c.estimator = &gpe
+	return nil
+}
+
+func (c *CommitStore) SetSourceMaxGasPrice(ctx context.Context, sourceMaxGasPrice *big.Int) error {
+	c.configMu.RLock()
+	defer c.configMu.RUnlock()
+	c.sourceMaxGasPrice = sourceMaxGasPrice
+	return nil
+}
+
 // CommitOffchainConfig is a legacy version of CommitOffchainConfig, used for CommitStore version 1.0.0 and 1.1.0
 type CommitOffchainConfig struct {
 	SourceFinalityDepth    uint32
@@ -224,8 +238,18 @@ func (c *CommitStore) ChangeConfig(_ context.Context, onchainConfig []byte, offc
 		return "", err
 	}
 	c.configMu.Lock()
+	defer c.configMu.Unlock()
+
+	if c.estimator == nil {
+		return "", fmt.Errorf("this CommitStore estimator is nil. SetGasEstimator should be called before ChangeConfig")
+	}
+
+	if c.sourceMaxGasPrice == nil {
+		return "", fmt.Errorf("this CommitStore sourceMaxGasPrice is nil. SetSourceMaxGasPrice should be called before ChangeConfig")
+	}
+
 	c.gasPriceEstimator = prices.NewExecGasPriceEstimator(
-		c.estimator,
+		*c.estimator,
 		c.sourceMaxGasPrice,
 		int64(offchainConfigV1.FeeUpdateDeviationPPB))
 	c.offchainConfig = ccipdata.NewCommitOffchainConfig(
@@ -235,7 +259,6 @@ func (c *CommitStore) ChangeConfig(_ context.Context, onchainConfig []byte, offc
 		offchainConfigV1.FeeUpdateHeartBeat.Duration(),
 		offchainConfigV1.InflightCacheExpiry.Duration(),
 		offchainConfigV1.PriceReportingDisabled)
-	c.configMu.Unlock()
 	c.lggr.Infow("ChangeConfig",
 		"offchainConfig", offchainConfigV1,
 		"onchainConfig", onchainConfigParsed,
@@ -379,7 +402,7 @@ func (c *CommitStore) RegisterFilters() error {
 	return logpollerutil.RegisterLpFilters(c.lp, c.filters)
 }
 
-func NewCommitStore(lggr logger.Logger, addr common.Address, ec client.Client, lp logpoller.LogPoller, estimator gas.EvmFeeEstimator, sourceMaxGasPrice *big.Int) (*CommitStore, error) {
+func NewCommitStore(lggr logger.Logger, addr common.Address, ec client.Client, lp logpoller.LogPoller) (*CommitStore, error) {
 	commitStore, err := commit_store_1_0_0.NewCommitStore(addr, ec)
 	if err != nil {
 		return nil, err
@@ -396,12 +419,13 @@ func NewCommitStore(lggr logger.Logger, addr common.Address, ec client.Client, l
 		},
 	}
 	return &CommitStore{
-		commitStore:       commitStore,
-		address:           addr,
-		lggr:              lggr,
-		lp:                lp,
-		estimator:         estimator,
-		sourceMaxGasPrice: sourceMaxGasPrice,
+		commitStore: commitStore,
+		address:     addr,
+		lggr:        lggr,
+		lp:          lp,
+
+		// Note that sourceMaxGasPrice and estimator now have explicit setters (CCIP-2493)
+
 		filters:           filters,
 		commitReportArgs:  commitReportArgs,
 		reportAcceptedSig: eventSig,

core/services/ocr2/plugins/ccip/internal/ccipdata/v1_0_0/commit_store_test.go

@@ -35,7 +35,7 @@ func TestCommitReportEncoding(t *testing.T) {
 		Interval:   cciptypes.CommitStoreInterval{Min: 1, Max: 10},
 	}
 
-	c, err := NewCommitStore(logger.TestLogger(t), utils.RandomAddress(), nil, mocks.NewLogPoller(t), nil, nil)
+	c, err := NewCommitStore(logger.TestLogger(t), utils.RandomAddress(), nil, mocks.NewLogPoller(t))
 	assert.NoError(t, err)
 
 	encodedReport, err := c.EncodeCommitReport(ctx, report)