SystemConfig: Parse L1 Receipts Atomically Before Application

op-node/rollup/derive/attributes.go

@@ -93,10 +93,10 @@ func (ba *FetchingAttributesBuilder) PreparePayloadAttributes(ctx context.Contex
 			// deposits may never be ignored. Failing to process them is a critical error.
 			return nil, NewCriticalError(fmt.Errorf("failed to derive some deposits: %w", err))
 		}
-		// apply sysCfg changes
-		if err := UpdateSystemConfigWithL1Receipts(&sysConfig, receipts, ba.rollupCfg, info.Time()); err != nil {
-			return nil, NewCriticalError(fmt.Errorf("failed to apply derived L1 sysCfg updates: %w", err))
-		}
+
+		// errors from UpdateSystemConfigWithL1Receipts are ignored as they represent malformed or invalid updates
+		// and there is no recovery mechanism for malformed updates, we must process past them.
+		_ = UpdateSystemConfigWithL1Receipts(&sysConfig, receipts, ba.rollupCfg, info.Time())
 
 		l1Info = info
 		depositTxs = deposits

op-node/rollup/derive/l1_traversal.go

@@ -76,8 +76,9 @@ func (l1t *L1Traversal) AdvanceL1Block(ctx context.Context) error {
 		return NewTemporaryError(fmt.Errorf("failed to fetch receipts of L1 block %s (parent: %s) for L1 sysCfg update: %w", nextL1Origin, origin, err))
 	}
 	if err := UpdateSystemConfigWithL1Receipts(&l1t.sysCfg, receipts, l1t.cfg, nextL1Origin.Time); err != nil {
-		// the sysCfg changes should always be formatted correctly.
-		return NewCriticalError(fmt.Errorf("failed to update L1 sysCfg with receipts from block %s: %w", nextL1Origin, err))
+		// if UpdateSystemConfigWithL1Receipts returns an error, it is because one or more of the receipts are malformed or invalid
+		// failure to apply is just informational, so we just log the error and continue
+		l1t.log.Warn("failed to fully update L1 sysCfg with receipts from block", "block", nextL1Origin, "error", err)
 	}
 
 	l1t.block = nextL1Origin

op-node/rollup/derive/l1_traversal_managed.go

@@ -112,8 +112,9 @@ func (l1t *L1TraversalManaged) ProvideNextL1(ctx context.Context, nextL1 eth.L1B
 			nextL1, nextL1.ParentID(), err))
 	}
 	if err := UpdateSystemConfigWithL1Receipts(&l1t.sysCfg, receipts, l1t.cfg, nextL1.Time); err != nil {
-		// the sysCfg changes should always be formatted correctly.
-		return NewCriticalError(fmt.Errorf("failed to update L1 sysCfg with receipts from block %s: %w", nextL1, err))
+		// if UpdateSystemConfigWithL1Receipts returns an error, it is because one or more of the receipts are malformed or invalid
+		// failure to apply is just informational, so we just log the error and continue
+		l1t.log.Warn("failed to fully update L1 sysCfg with receipts from block", "block", nextL1, "error", err)
 	}
 
 	logger.Info("Derivation continued with next L1 block")

op-node/rollup/derive/system_config.go

@@ -5,11 +5,10 @@ import (
 	"errors"
 	"fmt"
 
-	"github.com/hashicorp/go-multierror"
-
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/crypto"
+	"github.com/hashicorp/go-multierror"
 
 	"github.com/ethereum-optimism/optimism/op-node/rollup"
 	"github.com/ethereum-optimism/optimism/op-service/eth"
@@ -34,16 +33,25 @@ var (
 )
 
 // UpdateSystemConfigWithL1Receipts filters all L1 receipts to find config updates and applies the config updates to the given sysCfg
+// Updates are applied individually, and any malformed or invalid updates are ignored.
+// Any errors encountered during the update process are returned as a multierror.
 func UpdateSystemConfigWithL1Receipts(sysCfg *eth.SystemConfig, receipts []*types.Receipt, cfg *rollup.Config, l1Time uint64) error {
 	var result error
-	for i, rec := range receipts {
+	for _, rec := range receipts {
 		if rec.Status != types.ReceiptStatusSuccessful {
 			continue
 		}
-		for j, log := range rec.Logs {
+		for _, log := range rec.Logs {
+			// copy sysConfig to an update structure to preserve the original in case of error
+			updated := *sysCfg
 			if log.Address == cfg.L1SystemConfigAddress && len(log.Topics) > 0 && log.Topics[0] == ConfigUpdateEventABIHash {
-				if err := ProcessSystemConfigUpdateLogEvent(sysCfg, log, cfg, l1Time); err != nil {
-					result = multierror.Append(result, fmt.Errorf("malformatted L1 system sysCfg log in receipt %d, log %d: %w", i, j, err))
+				err := ProcessSystemConfigUpdateLogEvent(&updated, log, cfg, l1Time)
+				if err == nil {
+					// apply the updated structure
+					*sysCfg = updated
+				} else {
+					// or append the error to the result
+					result = multierror.Append(result, err)
 				}
 			}
 		}
@@ -76,44 +84,19 @@ func ProcessSystemConfigUpdateLogEvent(destSysCfg *eth.SystemConfig, ev *types.L
 	// indexed 1
 	updateType := ev.Topics[2]
 
-	// Create a reader of the unindexed data
-	reader := bytes.NewReader(ev.Data)
-
 	// Attempt to read unindexed data
 	switch updateType {
 	case SystemConfigUpdateBatcher:
-		if pointer, err := solabi.ReadUint64(reader); err != nil || pointer != 32 {
-			return NewCriticalError(errors.New("invalid pointer field"))
-		}
-		if length, err := solabi.ReadUint64(reader); err != nil || length != 32 {
-			return NewCriticalError(errors.New("invalid length field"))
-		}
-		address, err := solabi.ReadAddress(reader)
+		addr, err := parseSystemConfigUpdateBatcher(ev.Data)
 		if err != nil {
-			return NewCriticalError(errors.New("could not read address"))
-		}
-		if !solabi.EmptyReader(reader) {
-			return NewCriticalError(errors.New("too many bytes"))
+			return err
 		}
-		destSysCfg.BatcherAddr = address
+		destSysCfg.BatcherAddr = addr
 		return nil
 	case SystemConfigUpdateFeeScalars:
-		if pointer, err := solabi.ReadUint64(reader); err != nil || pointer != 32 {
-			return NewCriticalError(errors.New("invalid pointer field"))
-		}
-		if length, err := solabi.ReadUint64(reader); err != nil || length != 64 {
-			return NewCriticalError(errors.New("invalid length field"))
-		}
-		overhead, err := solabi.ReadEthBytes32(reader)
+		overhead, scalar, err := parseSystemConfigUpdateFeeScalars(ev.Data)
 		if err != nil {
-			return NewCriticalError(errors.New("could not read overhead"))
-		}
-		scalar, err := solabi.ReadEthBytes32(reader)
-		if err != nil {
-			return NewCriticalError(errors.New("could not read scalar"))
-		}
-		if !solabi.EmptyReader(reader) {
-			return NewCriticalError(errors.New("too many bytes"))
+			return err
 		}
 		if rollupCfg.IsEcotone(l1Time) {
 			if err := eth.CheckEcotoneL1SystemConfigScalar(scalar); err != nil {
@@ -129,89 +112,176 @@ func ProcessSystemConfigUpdateLogEvent(destSysCfg *eth.SystemConfig, ev *types.L
 		}
 		return nil
 	case SystemConfigUpdateGasLimit:
-		if pointer, err := solabi.ReadUint64(reader); err != nil || pointer != 32 {
-			return NewCriticalError(errors.New("invalid pointer field"))
-		}
-		if length, err := solabi.ReadUint64(reader); err != nil || length != 32 {
-			return NewCriticalError(errors.New("invalid length field"))
-		}
-		gasLimit, err := solabi.ReadUint64(reader)
+		gasLimit, err := parseSystemConfigUpdateGasLimit(ev.Data)
 		if err != nil {
-			return NewCriticalError(errors.New("could not read gas limit"))
-		}
-		if !solabi.EmptyReader(reader) {
-			return NewCriticalError(errors.New("too many bytes"))
+			return err
 		}
 		destSysCfg.GasLimit = gasLimit
 		return nil
 	case SystemConfigUpdateEIP1559Params:
-		if pointer, err := solabi.ReadUint64(reader); err != nil || pointer != 32 {
-			return NewCriticalError(errors.New("invalid pointer field"))
-		}
-		if length, err := solabi.ReadUint64(reader); err != nil || length != 32 {
-			return NewCriticalError(errors.New("invalid length field"))
-		}
-		params, err := solabi.ReadEthBytes32(reader)
+		params, err := parseSystemConfigUpdateEIP1559Params(ev.Data)
 		if err != nil {
-			return NewCriticalError(errors.New("could not read eip-1559 params"))
-		}
-		if !solabi.EmptyReader(reader) {
-			return NewCriticalError(errors.New("too many bytes"))
+			return err
 		}
 		copy(destSysCfg.EIP1559Params[:], params[24:32])
 		return nil
 	case SystemConfigUpdateOperatorFeeParams:
-		if pointer, err := solabi.ReadUint64(reader); err != nil || pointer != 32 {
-			return NewCriticalError(errors.New("invalid pointer field"))
-		}
-		if length, err := solabi.ReadUint64(reader); err != nil || length != 32 {
-			return NewCriticalError(errors.New("invalid length field"))
-		}
-		params, err := solabi.ReadEthBytes32(reader)
+		params, err := parseSystemConfigUpdateOperatorFeeParams(ev.Data)
 		if err != nil {
-			return NewCriticalError(errors.New("could not read operator fee params"))
-		}
-		if !solabi.EmptyReader(reader) {
-			return NewCriticalError(errors.New("too many bytes"))
+			return err
 		}
 		destSysCfg.OperatorFeeParams = params
 		return nil
 	case SystemConfigUpdateUnsafeBlockSigner:
 		// Ignored in derivation. This configurable applies to runtime configuration outside of the derivation.
 		return nil
 	case SystemConfigUpdateMinBaseFee:
-		if pointer, err := solabi.ReadUint64(reader); err != nil || pointer != 32 {
-			return NewCriticalError(errors.New("invalid pointer field"))
-		}
-		if length, err := solabi.ReadUint64(reader); err != nil || length != 32 {
-			return NewCriticalError(errors.New("invalid length field"))
-		}
-		minBaseFee, err := solabi.ReadUint64(reader)
+		minBaseFee, err := parseSystemConfigUpdateMinBaseFee(ev.Data)
 		if err != nil {
-			return NewCriticalError(errors.New("could not read minBaseFee"))
-		}
-		if !solabi.EmptyReader(reader) {
-			return NewCriticalError(errors.New("too many bytes"))
+			return err
 		}
 		destSysCfg.MinBaseFee = minBaseFee
 		return nil
 	case SystemConfigUpdateDAFootprintGasScalar:
-		if pointer, err := solabi.ReadUint64(reader); err != nil || pointer != 32 {
-			return NewCriticalError(errors.New("invalid pointer field"))
-		}
-		if length, err := solabi.ReadUint64(reader); err != nil || length != 32 {
-			return NewCriticalError(errors.New("invalid length field"))
-		}
-		daFootprintGasScalar, err := solabi.ReadUint16(reader)
+		daFootprintGasScalar, err := parseSystemConfigUpdateDAFootprintGasScalar(ev.Data)
 		if err != nil {
-			return NewCriticalError(errors.New("could not read DA footprint gas scalar"))
-		}
-		if !solabi.EmptyReader(reader) {
-			return NewCriticalError(errors.New("too many bytes"))
+			return err
 		}
 		destSysCfg.DAFootprintGasScalar = daFootprintGasScalar
 		return nil
 	default:
 		return fmt.Errorf("unrecognized L1 sysCfg update type: %s", updateType)
 	}
 }
+
+var ErrParsingSystemConfig = NewCriticalError(errors.New("error parsing system config"))
+
+func parseSystemConfigUpdateBatcher(data []byte) (common.Address, error) {
+	reader := bytes.NewReader(data)
+	if pointer, err := solabi.ReadUint64(reader); err != nil || pointer != 32 {
+		return common.Address{}, fmt.Errorf("%w: invalid pointer field", ErrParsingSystemConfig)
+	}
+	if length, err := solabi.ReadUint64(reader); err != nil || length != 32 {
+		return common.Address{}, fmt.Errorf("%w: invalid length field", ErrParsingSystemConfig)
+	}
+	address, err := solabi.ReadAddress(reader)
+	if err != nil {
+		return common.Address{}, fmt.Errorf("%w: could not read address", ErrParsingSystemConfig)
+	}
+	if !solabi.EmptyReader(reader) {
+		return common.Address{}, fmt.Errorf("%w: too many bytes", ErrParsingSystemConfig)
+	}
+	return address, nil
+}
+
+func parseSystemConfigUpdateFeeScalars(data []byte) (eth.Bytes32, eth.Bytes32, error) {
+	reader := bytes.NewReader(data)
+	if pointer, err := solabi.ReadUint64(reader); err != nil || pointer != 32 {
+		return eth.Bytes32{}, eth.Bytes32{}, fmt.Errorf("%w: invalid pointer field", ErrParsingSystemConfig)
+	}
+	if length, err := solabi.ReadUint64(reader); err != nil || length != 64 {
+		return eth.Bytes32{}, eth.Bytes32{}, fmt.Errorf("%w: invalid length field", ErrParsingSystemConfig)
+	}
+	overhead, err := solabi.ReadEthBytes32(reader)
+	if err != nil {
+		return eth.Bytes32{}, eth.Bytes32{}, fmt.Errorf("%w: could not read overhead", ErrParsingSystemConfig)
+	}
+	scalar, err := solabi.ReadEthBytes32(reader)
+	if err != nil {
+		return eth.Bytes32{}, eth.Bytes32{}, fmt.Errorf("%w: could not read scalar", ErrParsingSystemConfig)
+	}
+	if !solabi.EmptyReader(reader) {
+		return eth.Bytes32{}, eth.Bytes32{}, fmt.Errorf("%w: too many bytes", ErrParsingSystemConfig)
+	}
+	return overhead, scalar, nil
+}
+
+func parseSystemConfigUpdateGasLimit(data []byte) (uint64, error) {
+	reader := bytes.NewReader(data)
+	if pointer, err := solabi.ReadUint64(reader); err != nil || pointer != 32 {
+		return 0, fmt.Errorf("%w: invalid pointer field", ErrParsingSystemConfig)
+	}
+	if length, err := solabi.ReadUint64(reader); err != nil || length != 32 {
+		return 0, fmt.Errorf("%w: invalid length field", ErrParsingSystemConfig)
+	}
+	gasLimit, err := solabi.ReadUint64(reader)
+	if err != nil {
+		return 0, fmt.Errorf("%w: could not read gas limit", ErrParsingSystemConfig)
+	}
+	if !solabi.EmptyReader(reader) {
+		return 0, fmt.Errorf("%w: too many bytes", ErrParsingSystemConfig)
+	}
+	return gasLimit, nil
+}
+
+func parseSystemConfigUpdateEIP1559Params(data []byte) (eth.Bytes32, error) {
+	reader := bytes.NewReader(data)
+	if pointer, err := solabi.ReadUint64(reader); err != nil || pointer != 32 {
+		return eth.Bytes32{}, fmt.Errorf("%w: invalid pointer field", ErrParsingSystemConfig)
+	}
+	if length, err := solabi.ReadUint64(reader); err != nil || length != 32 {
+		return eth.Bytes32{}, fmt.Errorf("%w: invalid length field", ErrParsingSystemConfig)
+	}
+	params, err := solabi.ReadEthBytes32(reader)
+	if err != nil {
+		return eth.Bytes32{}, fmt.Errorf("%w: could not read eip-1559 params", ErrParsingSystemConfig)
+	}
+	if !solabi.EmptyReader(reader) {
+		return eth.Bytes32{}, fmt.Errorf("%w: too many bytes", ErrParsingSystemConfig)
+	}
+	return params, nil
+}
+
+func parseSystemConfigUpdateOperatorFeeParams(data []byte) (eth.Bytes32, error) {
+	reader := bytes.NewReader(data)
+	if pointer, err := solabi.ReadUint64(reader); err != nil || pointer != 32 {
+		return eth.Bytes32{}, fmt.Errorf("%w: invalid pointer field", ErrParsingSystemConfig)
+	}
+	if length, err := solabi.ReadUint64(reader); err != nil || length != 32 {
+		return eth.Bytes32{}, fmt.Errorf("%w: invalid length field", ErrParsingSystemConfig)
+	}
+	params, err := solabi.ReadEthBytes32(reader)
+	if err != nil {
+		return eth.Bytes32{}, fmt.Errorf("%w: could not read operator fee params", ErrParsingSystemConfig)
+	}
+	if !solabi.EmptyReader(reader) {
+		return eth.Bytes32{}, fmt.Errorf("%w: too many bytes", ErrParsingSystemConfig)
+	}
+	return params, nil
+}
+
+func parseSystemConfigUpdateMinBaseFee(data []byte) (uint64, error) {
+	reader := bytes.NewReader(data)
+	if pointer, err := solabi.ReadUint64(reader); err != nil || pointer != 32 {
+		return 0, fmt.Errorf("%w: invalid pointer field", ErrParsingSystemConfig)
+	}
+	if length, err := solabi.ReadUint64(reader); err != nil || length != 32 {
+		return 0, fmt.Errorf("%w: invalid length field", ErrParsingSystemConfig)
+	}
+	minBaseFee, err := solabi.ReadUint64(reader)
+	if err != nil {
+		return 0, fmt.Errorf("%w: could not read minBaseFee", ErrParsingSystemConfig)
+	}
+	if !solabi.EmptyReader(reader) {
+		return 0, fmt.Errorf("%w: too many bytes", ErrParsingSystemConfig)
+	}
+	return minBaseFee, nil
+}
+
+func parseSystemConfigUpdateDAFootprintGasScalar(data []byte) (uint16, error) {
+	reader := bytes.NewReader(data)
+	if pointer, err := solabi.ReadUint64(reader); err != nil || pointer != 32 {
+		return 0, fmt.Errorf("%w: invalid pointer field", ErrParsingSystemConfig)
+	}
+	if length, err := solabi.ReadUint64(reader); err != nil || length != 32 {
+		return 0, fmt.Errorf("%w: invalid length field", ErrParsingSystemConfig)
+	}
+	daFootprintGasScalar, err := solabi.ReadUint16(reader)
+	if err != nil {
+		return 0, fmt.Errorf("%w: could not read DA footprint gas scalar", ErrParsingSystemConfig)
+	}
+	if !solabi.EmptyReader(reader) {
+		return 0, fmt.Errorf("%w: too many bytes", ErrParsingSystemConfig)
+	}
+	return daFootprintGasScalar, nil
+}