suggestions and fix docs_test

smartcontractkit · Oct 29, 2024 · 41ffa23 · 41ffa23
1 parent 2bd1657
commit 41ffa23
Show file tree

Hide file tree

Showing 5 changed files with 296 additions and 40 deletions.
diff --git a/core/chains/evm/config/toml/config.go b/core/chains/evm/config/toml/config.go
@@ -784,6 +784,25 @@ const (
 	DAOracleCustomCalldata = DAOracleType("custom_calldata")
 )
 
+func (o *DAOracle) ValidateConfig() (err error) {
+	if o.OracleType != nil {
+		if *o.OracleType == DAOracleOPStack {
+			if o.OracleAddress == nil {
+				err = multierr.Append(err, commonconfig.ErrMissing{Name: "OracleAddress", Msg: "required for 'opstack' oracle types"})
+			}
+		}
+		if *o.OracleType == DAOracleCustomCalldata {
+			if o.OracleAddress == nil {
+				err = multierr.Append(err, commonconfig.ErrMissing{Name: "OracleAddress", Msg: "required for 'custom_calldata' oracle types"})
+			}
+			if o.CustomGasPriceCalldata == nil {
+				err = multierr.Append(err, commonconfig.ErrMissing{Name: "CustomGasPriceCalldata", Msg: "required for 'custom_calldata' oracle type"})
+			}
+		}
+	}
+	return
+}
+
 func (o DAOracleType) IsValid() bool {
 	switch o {
 	case "", DAOracleOPStack, DAOracleArbitrum, DAOracleZKSync, DAOracleCustomCalldata:

diff --git a/core/chains/evm/config/toml/defaults/fallback.toml b/core/chains/evm/config/toml/defaults/fallback.toml
@@ -60,6 +60,11 @@ TransactionPercentile = 60
 [GasEstimator.FeeHistory]
 CacheTimeout = '10s'
 
+[GasEstimator.DAOracle]
+OracleType = ''
+OracleAddress = '0x0000000000000000000000000000000000000000'
+CustomGasPriceCalldata = ''
+
 [HeadTracker]
 HistoryDepth = 100
 MaxBufferSize = 3

diff --git a/core/chains/evm/gas/rollups/op_l1_oracle.go b/core/chains/evm/gas/rollups/op_l1_oracle.go
@@ -32,12 +32,12 @@ type optimismL1Oracle struct {
 	pollPeriod time.Duration
 	logger     logger.SugaredLogger
 
-	daOracleConfig evmconfig.DAOracle
-	l1GasPriceMu   sync.RWMutex
-	l1GasPrice     priceEntry
-	isEcotone      bool
-	isFjord        bool
-	upgradeCheckTs time.Time
+	daOracleAddress common.Address
+	l1GasPriceMu    sync.RWMutex
+	l1GasPrice      priceEntry
+	isEcotone       bool
+	isFjord         bool
+	upgradeCheckTs  time.Time
 
 	chInitialised chan struct{}
 	chStop        services.StopChan
@@ -96,12 +96,13 @@ func NewOpStackL1GasOracle(lggr logger.Logger, ethClient l1OracleClient, chainTy
 	if *daOracle.OracleType() != toml.DAOracleOPStack {
 		return nil, fmt.Errorf("expected %s oracle type, got %s", toml.DAOracleOPStack, *daOracle.OracleType())
 	}
-	if daOracle.OracleAddress() == nil || *daOracle.OracleAddress() == "" {
-		return nil, errors.New("OracleAddress is required but was nil or empty")
-	}
 	if daOracle.CustomGasPriceCalldata() != nil && *daOracle.CustomGasPriceCalldata() != "" {
 		lggr.Warnf("CustomGasPriceCalldata is set but will be ignored for OPStack DA oracle")
 	}
+	if daOracle.OracleAddress() == nil || *daOracle.OracleAddress() == "" {
+		return nil, errors.New("OracleAddress is required but was nil or empty")
+	}
+	oracleAddress := *daOracle.OracleAddress()
 
 	getL1FeeMethodAbi, err := abi.JSON(strings.NewReader(GetL1FeeAbiString))
 	if err != nil {
@@ -156,10 +157,10 @@ func NewOpStackL1GasOracle(lggr logger.Logger, ethClient l1OracleClient, chainTy
 		pollPeriod: PollPeriod,
 		logger:     logger.Sugared(logger.Named(lggr, fmt.Sprintf("L1GasOracle(%s)", chainType))),
 
-		daOracleConfig: daOracle,
-		isEcotone:      false,
-		isFjord:        false,
-		upgradeCheckTs: time.Time{},
+		daOracleAddress: oracleAddress.Address(),
+		isEcotone:       false,
+		isFjord:         false,
+		upgradeCheckTs:  time.Time{},
 
 		chInitialised: make(chan struct{}),
 		chStop:        make(chan struct{}),
@@ -292,20 +293,14 @@ func (o *optimismL1Oracle) checkForUpgrade(ctx context.Context) error {
 		return nil
 	}
 
-	// Check in case somehow DA oracle address got reset to nil
-	if o.daOracleConfig.OracleAddress() == nil {
-		return errors.New("OracleAddress is required but was nil")
-	}
-
-	oracleAddress := *o.daOracleConfig.OracleAddress()
 	o.upgradeCheckTs = time.Now()
 	rpcBatchCalls := []rpc.BatchElem{
 		{
 			Method: "eth_call",
 			Args: []any{
 				map[string]interface{}{
 					"from": common.Address{},
-					"to":   oracleAddress.String(),
+					"to":   o.daOracleAddress.String(),
 					"data": hexutil.Bytes(o.isFjordCalldata),
 				},
 				"latest",
@@ -317,7 +312,7 @@ func (o *optimismL1Oracle) checkForUpgrade(ctx context.Context) error {
 			Args: []any{
 				map[string]interface{}{
 					"from": common.Address{},
-					"to":   oracleAddress.String(),
+					"to":   o.daOracleAddress.String(),
 					"data": hexutil.Bytes(o.isEcotoneCalldata),
 				},
 				"latest",
@@ -358,14 +353,8 @@ func (o *optimismL1Oracle) checkForUpgrade(ctx context.Context) error {
 }
 
 func (o *optimismL1Oracle) getV1GasPrice(ctx context.Context) (*big.Int, error) {
-	if o.daOracleConfig.OracleAddress() == nil {
-		return nil, errors.New("OracleAddress is required for OP DA oracle but was nil")
-	}
-
-	oracleAddress := *o.daOracleConfig.OracleAddress()
-	oracleCommonAddress := oracleAddress.Address()
 	b, err := o.client.CallContract(ctx, ethereum.CallMsg{
-		To:   &oracleCommonAddress,
+		To:   &o.daOracleAddress,
 		Data: o.l1BaseFeeCalldata,
 	}, nil)
 	if err != nil {
@@ -381,18 +370,13 @@ func (o *optimismL1Oracle) getV1GasPrice(ctx context.Context) (*big.Int, error)
 // Returns the scaled gas price using baseFeeScalar, l1BaseFee, blobBaseFeeScalar, and blobBaseFee fields from the oracle
 // Confirmed the same calculation is used to determine gas price for both Ecotone and Fjord
 func (o *optimismL1Oracle) getEcotoneFjordGasPrice(ctx context.Context) (*big.Int, error) {
-	if o.daOracleConfig.OracleAddress() == nil {
-		return nil, errors.New("OracleAddress is required for OP DA oracle but was nil")
-	}
-
-	oracleAddress := *o.daOracleConfig.OracleAddress()
 	rpcBatchCalls := []rpc.BatchElem{
 		{
 			Method: "eth_call",
 			Args: []any{
 				map[string]interface{}{
 					"from": common.Address{},
-					"to":   oracleAddress.String(),
+					"to":   o.daOracleAddress.String(),
 					"data": hexutil.Bytes(o.l1BaseFeeCalldata),
 				},
 				"latest",
@@ -404,7 +388,7 @@ func (o *optimismL1Oracle) getEcotoneFjordGasPrice(ctx context.Context) (*big.In
 			Args: []any{
 				map[string]interface{}{
 					"from": common.Address{},
-					"to":   oracleAddress.String(),
+					"to":   o.daOracleAddress.String(),
 					"data": hexutil.Bytes(o.baseFeeScalarCalldata),
 				},
 				"latest",
@@ -416,7 +400,7 @@ func (o *optimismL1Oracle) getEcotoneFjordGasPrice(ctx context.Context) (*big.In
 			Args: []any{
 				map[string]interface{}{
 					"from": common.Address{},
-					"to":   oracleAddress.String(),
+					"to":   o.daOracleAddress.String(),
 					"data": hexutil.Bytes(o.blobBaseFeeCalldata),
 				},
 				"latest",
@@ -428,7 +412,7 @@ func (o *optimismL1Oracle) getEcotoneFjordGasPrice(ctx context.Context) (*big.In
 			Args: []any{
 				map[string]interface{}{
 					"from": common.Address{},
-					"to":   oracleAddress.String(),
+					"to":   o.daOracleAddress.String(),
 					"data": hexutil.Bytes(o.blobBaseFeeScalarCalldata),
 				},
 				"latest",
@@ -440,7 +424,7 @@ func (o *optimismL1Oracle) getEcotoneFjordGasPrice(ctx context.Context) (*big.In
 			Args: []any{
 				map[string]interface{}{
 					"from": common.Address{},
-					"to":   oracleAddress.String(),
+					"to":   o.daOracleAddress.String(),
 					"data": hexutil.Bytes(o.decimalsCalldata),
 				},
 				"latest",

diff --git a/core/config/docs/docs_test.go b/core/config/docs/docs_test.go
@@ -97,8 +97,10 @@ func TestDoc(t *testing.T) {
 		docDefaults.Transactions.AutoPurge.Threshold = nil
 		docDefaults.Transactions.AutoPurge.MinAttempts = nil
 
-		// GasEstimator.DAOracle.OracleAddress is only set if DA oracle config is used
-		docDefaults.GasEstimator.DAOracle.OracleAddress = nil
+		// Fallback oracle address must be an EIP55-compatible address
+		zeroAddress, err := types.NewEIP55Address("0x0000000000000000000000000000000000000000")
+		require.NoError(t, err)
+		docDefaults.GasEstimator.DAOracle.OracleAddress = &zeroAddress
 
 		assertTOML(t, fallbackDefaults, docDefaults)
 	})