From 646a3c95638dff611aa5f04dc71f711a15640fef Mon Sep 17 00:00:00 2001 From: axelKingsley Date: Fri, 14 Nov 2025 14:00:19 -0600 Subject: [PATCH 1/3] SystemConfig: Parse L1 Receipts Atomically Before Application --- op-node/rollup/derive/system_config.go | 252 +++++++++++++------- op-node/rollup/derive/system_config_test.go | 107 +++++++++ 2 files changed, 275 insertions(+), 84 deletions(-) diff --git a/op-node/rollup/derive/system_config.go b/op-node/rollup/derive/system_config.go index b1a7cb67f1a..02072339ac1 100644 --- a/op-node/rollup/derive/system_config.go +++ b/op-node/rollup/derive/system_config.go @@ -36,18 +36,38 @@ var ( // UpdateSystemConfigWithL1Receipts filters all L1 receipts to find config updates and applies the config updates to the given sysCfg func UpdateSystemConfigWithL1Receipts(sysCfg *eth.SystemConfig, receipts []*types.Receipt, cfg *rollup.Config, l1Time uint64) error { var result error + + // copy sysConfig to an update accumulator + // to allow for pre-parsing of all updates prior to application + updated := eth.SystemConfig{ + BatcherAddr: sysCfg.BatcherAddr, + Overhead: sysCfg.Overhead, + Scalar: sysCfg.Scalar, + GasLimit: sysCfg.GasLimit, + EIP1559Params: sysCfg.EIP1559Params, + OperatorFeeParams: sysCfg.OperatorFeeParams, + MinBaseFee: sysCfg.MinBaseFee, + DAFootprintGasScalar: sysCfg.DAFootprintGasScalar, + } for i, rec := range receipts { if rec.Status != types.ReceiptStatusSuccessful { continue } for j, log := range rec.Logs { if log.Address == cfg.L1SystemConfigAddress && len(log.Topics) > 0 && log.Topics[0] == ConfigUpdateEventABIHash { - if err := ProcessSystemConfigUpdateLogEvent(sysCfg, log, cfg, l1Time); err != nil { + if err := ProcessSystemConfigUpdateLogEvent(&updated, log, cfg, l1Time); err != nil { + if errors.Is(err, ErrParsingSystemConfig) { + // if the config can't be parsed, ignore the entire update + // an unparsable update should be treated as unusable and thus ignored, there are no other recovery mechanisms + return nil + } result = multierror.Append(result, fmt.Errorf("malformatted L1 system sysCfg log in receipt %d, log %d: %w", i, j, err)) } } } } + // apply the accumulated updates to the original sysCfg + *sysCfg = updated return result } @@ -77,43 +97,20 @@ func ProcessSystemConfigUpdateLogEvent(destSysCfg *eth.SystemConfig, ev *types.L 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) - if err != nil { - return NewCriticalError(errors.New("could not read overhead")) - } - scalar, err := solabi.ReadEthBytes32(reader) + overhead, scalar, err := parseSystemConfigUpdateFeeScalars(ev.Data) 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,50 +126,23 @@ 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 @@ -180,34 +150,16 @@ func ProcessSystemConfigUpdateLogEvent(destSysCfg *eth.SystemConfig, ev *types.L // 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 @@ -215,3 +167,135 @@ func ProcessSystemConfigUpdateLogEvent(destSysCfg *eth.SystemConfig, ev *types.L 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 +} diff --git a/op-node/rollup/derive/system_config_test.go b/op-node/rollup/derive/system_config_test.go index a17514f2a85..326bf32da1b 100644 --- a/op-node/rollup/derive/system_config_test.go +++ b/op-node/rollup/derive/system_config_test.go @@ -297,3 +297,110 @@ func TestProcessSystemConfigUpdateLogEvent(t *testing.T) { }) } } + +func TestUpdateSystemConfigWithL1Receipts_Atomicity(t *testing.T) { + t.Run("applies all updates when all receipts well-formed", func(t *testing.T) { + sysCfg := eth.SystemConfig{} + l1Addr := common.Address{19: 0x42} + cfg := rollup.Config{ + L1SystemConfigAddress: l1Addr, + } + // Build a well-formed Batcher update + newBatcher := common.Address{19: 0xaa} + addrData, err := addressArgs.Pack(&newBatcher) + require.NoError(t, err) + batcherData, err := bytesArgs.Pack(addrData) + require.NoError(t, err) + batcherLog := &types.Log{ + Address: l1Addr, + Topics: []common.Hash{ + ConfigUpdateEventABIHash, + ConfigUpdateEventVersion0, + SystemConfigUpdateBatcher, + }, + Data: batcherData, + } + // Build a well-formed GasLimit update + gasLimit := big.NewInt(0xbb) + gasDataEnc, err := oneUint256.Pack(gasLimit) + require.NoError(t, err) + gasData, err := bytesArgs.Pack(gasDataEnc) + require.NoError(t, err) + gasLog := &types.Log{ + Address: l1Addr, + Topics: []common.Hash{ + ConfigUpdateEventABIHash, + ConfigUpdateEventVersion0, + SystemConfigUpdateGasLimit, + }, + Data: gasData, + } + receipts := []*types.Receipt{ + { + Status: types.ReceiptStatusSuccessful, + Logs: []*types.Log{batcherLog}, + }, + { + Status: types.ReceiptStatusSuccessful, + Logs: []*types.Log{gasLog}, + }, + } + err = UpdateSystemConfigWithL1Receipts(&sysCfg, receipts, &cfg, 0) + require.NoError(t, err) + require.Equal(t, newBatcher, sysCfg.BatcherAddr) + require.Equal(t, uint64(0xbb), sysCfg.GasLimit) + }) + + t.Run("no update applied if any receipt malformed", func(t *testing.T) { + // Start with a non-zero initial config so we can detect accidental partial updates + initial := eth.SystemConfig{ + BatcherAddr: common.Address{19: 0x11}, + GasLimit: 0x1234, + } + sysCfg := initial + l1Addr := common.Address{19: 0x43} + cfg := rollup.Config{ + L1SystemConfigAddress: l1Addr, + } + // Well-formed Batcher update (would change value if applied) + newBatcher := common.Address{19: 0xaa} + addrData, err := addressArgs.Pack(&newBatcher) + require.NoError(t, err) + batcherData, err := bytesArgs.Pack(addrData) + require.NoError(t, err) + batcherLog := &types.Log{ + Address: l1Addr, + Topics: []common.Hash{ + ConfigUpdateEventABIHash, + ConfigUpdateEventVersion0, + SystemConfigUpdateBatcher, + }, + Data: batcherData, + } + // Malformed GasLimit update (invalid data to trigger parse failure) + malformedGasLog := &types.Log{ + Address: l1Addr, + Topics: []common.Hash{ + ConfigUpdateEventABIHash, + ConfigUpdateEventVersion0, + SystemConfigUpdateGasLimit, + }, + Data: []byte{0x00}, // insufficient bytes for pointer/length -> parse error + } + receipts := []*types.Receipt{ + { + Status: types.ReceiptStatusSuccessful, + Logs: []*types.Log{batcherLog}, + }, + { + Status: types.ReceiptStatusSuccessful, + Logs: []*types.Log{malformedGasLog}, + }, + } + err = UpdateSystemConfigWithL1Receipts(&sysCfg, receipts, &cfg, 0) + // No error should be returned; the entire update batch is ignored + require.NoError(t, err) + // Confirm original config is unchanged + require.Equal(t, initial, sysCfg) + }) +} From 7e065d6f1aae10bcfe13d442f6699a6ac47f02b6 Mon Sep 17 00:00:00 2001 From: axelKingsley Date: Tue, 18 Nov 2025 10:56:10 -0600 Subject: [PATCH 2/3] Change Apply rules to all Valid ; PR Comments --- op-node/rollup/derive/attributes.go | 8 +- op-node/rollup/derive/l1_traversal.go | 5 +- op-node/rollup/derive/l1_traversal_managed.go | 5 +- op-node/rollup/derive/system_config.go | 42 ++---- op-node/rollup/derive/system_config_test.go | 140 +++++++++++++++++- 5 files changed, 160 insertions(+), 40 deletions(-) diff --git a/op-node/rollup/derive/attributes.go b/op-node/rollup/derive/attributes.go index 51cfb7829e3..5d201f1ded5 100644 --- a/op-node/rollup/derive/attributes.go +++ b/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 diff --git a/op-node/rollup/derive/l1_traversal.go b/op-node/rollup/derive/l1_traversal.go index 47e36d99139..c107545d388 100644 --- a/op-node/rollup/derive/l1_traversal.go +++ b/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 diff --git a/op-node/rollup/derive/l1_traversal_managed.go b/op-node/rollup/derive/l1_traversal_managed.go index 7eb7018e173..bbd2efae117 100644 --- a/op-node/rollup/derive/l1_traversal_managed.go +++ b/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") diff --git a/op-node/rollup/derive/system_config.go b/op-node/rollup/derive/system_config.go index 02072339ac1..cefb42ef9c0 100644 --- a/op-node/rollup/derive/system_config.go +++ b/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,40 +33,29 @@ 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 - - // copy sysConfig to an update accumulator - // to allow for pre-parsing of all updates prior to application - updated := eth.SystemConfig{ - BatcherAddr: sysCfg.BatcherAddr, - Overhead: sysCfg.Overhead, - Scalar: sysCfg.Scalar, - GasLimit: sysCfg.GasLimit, - EIP1559Params: sysCfg.EIP1559Params, - OperatorFeeParams: sysCfg.OperatorFeeParams, - MinBaseFee: sysCfg.MinBaseFee, - DAFootprintGasScalar: sysCfg.DAFootprintGasScalar, - } - 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(&updated, log, cfg, l1Time); err != nil { - if errors.Is(err, ErrParsingSystemConfig) { - // if the config can't be parsed, ignore the entire update - // an unparsable update should be treated as unusable and thus ignored, there are no other recovery mechanisms - return 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) } } } } - // apply the accumulated updates to the original sysCfg - *sysCfg = updated return result } @@ -96,8 +84,6 @@ func ProcessSystemConfigUpdateLogEvent(destSysCfg *eth.SystemConfig, ev *types.L // indexed 1 updateType := ev.Topics[2] - // Create a reader of the unindexed data - // Attempt to read unindexed data switch updateType { case SystemConfigUpdateBatcher: diff --git a/op-node/rollup/derive/system_config_test.go b/op-node/rollup/derive/system_config_test.go index 326bf32da1b..87a3bc1e291 100644 --- a/op-node/rollup/derive/system_config_test.go +++ b/op-node/rollup/derive/system_config_test.go @@ -351,7 +351,7 @@ func TestUpdateSystemConfigWithL1Receipts_Atomicity(t *testing.T) { require.Equal(t, uint64(0xbb), sysCfg.GasLimit) }) - t.Run("no update applied if any receipt malformed", func(t *testing.T) { + t.Run("all valid updates apply, any invalid updates are not applied and return errors", func(t *testing.T) { // Start with a non-zero initial config so we can detect accidental partial updates initial := eth.SystemConfig{ BatcherAddr: common.Address{19: 0x11}, @@ -398,9 +398,141 @@ func TestUpdateSystemConfigWithL1Receipts_Atomicity(t *testing.T) { }, } err = UpdateSystemConfigWithL1Receipts(&sysCfg, receipts, &cfg, 0) - // No error should be returned; the entire update batch is ignored + // Error should be returned due to malformed update, but valid updates should apply + require.Error(t, err) + // Confirm valid update applied + require.Equal(t, newBatcher, sysCfg.BatcherAddr) + // Confirm invalid update did not apply; GasLimit remains unchanged + require.Equal(t, initial.GasLimit, sysCfg.GasLimit) + }) + + t.Run("applies multiple updates within a single receipt", func(t *testing.T) { + sysCfg := eth.SystemConfig{} + l1Addr := common.Address{19: 0x44} + cfg := rollup.Config{ + L1SystemConfigAddress: l1Addr, + } + // Build well-formed Batcher update + newBatcher := common.Address{19: 0xbb} + addrData, err := addressArgs.Pack(&newBatcher) + require.NoError(t, err) + batcherData, err := bytesArgs.Pack(addrData) + require.NoError(t, err) + batcherLog := &types.Log{ + Address: l1Addr, + Topics: []common.Hash{ + ConfigUpdateEventABIHash, + ConfigUpdateEventVersion0, + SystemConfigUpdateBatcher, + }, + Data: batcherData, + } + // Build well-formed GasLimit update + gasLimit := big.NewInt(0xcc) + gasDataEnc, err := oneUint256.Pack(gasLimit) + require.NoError(t, err) + gasData, err := bytesArgs.Pack(gasDataEnc) require.NoError(t, err) - // Confirm original config is unchanged - require.Equal(t, initial, sysCfg) + gasLog := &types.Log{ + Address: l1Addr, + Topics: []common.Hash{ + ConfigUpdateEventABIHash, + ConfigUpdateEventVersion0, + SystemConfigUpdateGasLimit, + }, + Data: gasData, + } + receipts := []*types.Receipt{ + { + Status: types.ReceiptStatusSuccessful, + Logs: []*types.Log{batcherLog, gasLog}, + }, + } + err = UpdateSystemConfigWithL1Receipts(&sysCfg, receipts, &cfg, 0) + require.NoError(t, err) + require.Equal(t, newBatcher, sysCfg.BatcherAddr) + require.Equal(t, uint64(0xcc), sysCfg.GasLimit) + }) + + t.Run("applies updates across multiple receipts within the same block", func(t *testing.T) { + sysCfg := eth.SystemConfig{} + l1Addr := common.Address{19: 0x45} + cfg := rollup.Config{ + L1SystemConfigAddress: l1Addr, + } + blockHash := common.Hash{0: 0xaa} + blockNumber := big.NewInt(12345) + // Build well-formed Batcher update (tx 0) + newBatcher := common.Address{19: 0xcc} + addrData, err := addressArgs.Pack(&newBatcher) + require.NoError(t, err) + batcherData, err := bytesArgs.Pack(addrData) + require.NoError(t, err) + batcherLog := &types.Log{ + Address: l1Addr, + Topics: []common.Hash{ + ConfigUpdateEventABIHash, + ConfigUpdateEventVersion0, + SystemConfigUpdateBatcher, + }, + Data: batcherData, + } + // Build well-formed GasLimit update (tx 1) + gasLimit := big.NewInt(0xdd) + gasDataEnc, err := oneUint256.Pack(gasLimit) + require.NoError(t, err) + gasData, err := bytesArgs.Pack(gasDataEnc) + require.NoError(t, err) + gasLog := &types.Log{ + Address: l1Addr, + Topics: []common.Hash{ + ConfigUpdateEventABIHash, + ConfigUpdateEventVersion0, + SystemConfigUpdateGasLimit, + }, + Data: gasData, + } + // Build well-formed MinBaseFee update (tx 2) + minBaseFeeEnc, err := oneUint256.Pack(new(big.Int).SetUint64(minBaseFee)) + require.NoError(t, err) + minBaseFeeData, err := bytesArgs.Pack(minBaseFeeEnc) + require.NoError(t, err) + minBaseFeeLog := &types.Log{ + Address: l1Addr, + Topics: []common.Hash{ + ConfigUpdateEventABIHash, + ConfigUpdateEventVersion0, + SystemConfigUpdateMinBaseFee, + }, + Data: minBaseFeeData, + } + receipts := []*types.Receipt{ + { + Status: types.ReceiptStatusSuccessful, + BlockHash: blockHash, + BlockNumber: blockNumber, + TransactionIndex: 0, + Logs: []*types.Log{batcherLog}, + }, + { + Status: types.ReceiptStatusSuccessful, + BlockHash: blockHash, + BlockNumber: blockNumber, + TransactionIndex: 1, + Logs: []*types.Log{gasLog}, + }, + { + Status: types.ReceiptStatusSuccessful, + BlockHash: blockHash, + BlockNumber: blockNumber, + TransactionIndex: 2, + Logs: []*types.Log{minBaseFeeLog}, + }, + } + err = UpdateSystemConfigWithL1Receipts(&sysCfg, receipts, &cfg, 0) + require.NoError(t, err) + require.Equal(t, newBatcher, sysCfg.BatcherAddr) + require.Equal(t, uint64(0xdd), sysCfg.GasLimit) + require.Equal(t, minBaseFee, sysCfg.MinBaseFee) }) } From c442197bfb8ebfe2aaa0e300105fe2cdaaef4786 Mon Sep 17 00:00:00 2001 From: axelKingsley Date: Wed, 19 Nov 2025 10:32:17 -0600 Subject: [PATCH 3/3] Add indicies back to error --- op-node/rollup/derive/system_config.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/op-node/rollup/derive/system_config.go b/op-node/rollup/derive/system_config.go index cefb42ef9c0..0d03dd056d3 100644 --- a/op-node/rollup/derive/system_config.go +++ b/op-node/rollup/derive/system_config.go @@ -37,11 +37,11 @@ var ( // 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 _, rec := range receipts { + for i, rec := range receipts { if rec.Status != types.ReceiptStatusSuccessful { continue } - for _, log := range rec.Logs { + for j, 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 { @@ -51,7 +51,7 @@ func UpdateSystemConfigWithL1Receipts(sysCfg *eth.SystemConfig, receipts []*type *sysCfg = updated } else { // or append the error to the result - result = multierror.Append(result, err) + result = multierror.Append(result, fmt.Errorf("malformatted L1 system sysCfg log in receipt %d, log %d: %w", i, j, err)) } } }