op-node: Simplify parsing in ProcessSystemConfigUpdateLogEvent

op-node/rollup/derive/l1_block_info.go

@@ -2,6 +2,7 @@ package derive
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	"math/big"
 
@@ -126,7 +127,9 @@ func (info *L1BlockInfo) UnmarshalBinary(data []byte) error {
 	if info.L1FeeScalar, err = solabi.ReadEthBytes32(reader); err != nil {
 		return err
 	}
-	// TODO: solabi.EmptyReader
+	if !solabi.EmptyReader(reader) {
+		return errors.New("too many bytes")
+	}
 	return nil
 }
 

op-node/rollup/derive/system_config.go

@@ -2,7 +2,7 @@ package derive
 
 import (
 	"bytes"
-	"encoding/binary"
+	"errors"
 	"fmt"
 
 	"github.com/ethereum/go-ethereum/common"
@@ -12,6 +12,7 @@ import (
 
 	"github.com/ethereum-optimism/optimism/op-node/eth"
 	"github.com/ethereum-optimism/optimism/op-node/rollup"
+	"github.com/ethereum-optimism/optimism/op-service/solabi"
 )
 
 var (
@@ -27,17 +28,6 @@ var (
 	ConfigUpdateEventVersion0 = common.Hash{}
 )
 
-var (
-	// A left-padded uint256 equal to 32.
-	oneWordUint = common.Hash{31: 32}
-	// A left-padded uint256 equal to 64.
-	twoWordUint = common.Hash{31: 64}
-	// 24 zero bytes (the padding for a uint64 in a 32 byte word)
-	uint64Padding = make([]byte, 24)
-	// 12 zero bytes (the padding for an Ethereum address in a 32 byte word)
-	addressPadding = make([]byte, 12)
-)
-
 // 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) error {
 	var result error
@@ -84,90 +74,60 @@ func ProcessSystemConfigUpdateLogEvent(destSysCfg *eth.SystemConfig, ev *types.L
 	// Create a reader of the unindexed data
 	reader := bytes.NewReader(ev.Data)
 
-	// Counter for the number of bytes read from `reader` via `readWord`
-	countReadBytes := 0
-
-	// Helper function to read a word from the log data reader
-	readWord := func() (b [32]byte) {
-		if _, err := reader.Read(b[:]); err != nil {
-			// If there is an error reading the next 32 bytes from the reader, return an empty
-			// 32 byte array. We always check that the number of bytes read (`countReadBytes`)
-			// is equal to the expected amount at the end of each switch case.
-			return b
-		}
-		countReadBytes += 32
-		return b
-	}
-
 	// Attempt to read unindexed data
 	switch updateType {
 	case SystemConfigUpdateBatcher:
-		// Read the pointer, it should always equal 32.
-		if word := readWord(); word != oneWordUint {
-			return fmt.Errorf("expected offset to point to length location, but got %s", word)
+		if pointer, err := solabi.ReadUint64(reader); err != nil || pointer != 32 {
+			return NewCriticalError(errors.New("invalid pointer field"))
 		}
-
-		// Read the length, it should also always equal 32.
-		if word := readWord(); word != oneWordUint {
-			return fmt.Errorf("expected length to be 32 bytes, but got %s", word)
+		if length, err := solabi.ReadUint64(reader); err != nil || length != 32 {
+			return NewCriticalError(errors.New("invalid length field"))
 		}
-
-		// Indexing `word` directly is always safe here, it is guaranteed to be 32 bytes in length.
-		// Check that the batcher address is correctly zero-padded.
-		word := readWord()
-		if !bytes.Equal(word[:12], addressPadding) {
-			return fmt.Errorf("expected version 0 batcher hash with zero padding, but got %x", word)
+		address, err := solabi.ReadAddress(reader)
+		if err != nil {
+			return NewCriticalError(errors.New("could not read address"))
 		}
-		destSysCfg.BatcherAddr.SetBytes(word[12:])
-
-		if countReadBytes != 32*3 {
-			return NewCriticalError(fmt.Errorf("expected 32*3 bytes in batcher hash update, but got %d bytes", len(ev.Data)))
+		if !solabi.EmptyReader(reader) {
+			return NewCriticalError(errors.New("too many bytes"))
 		}
-
+		destSysCfg.BatcherAddr = address
 		return nil
 	case SystemConfigUpdateGasConfig:
-		// Read the pointer, it should always equal 32.
-		if word := readWord(); word != oneWordUint {
-			return fmt.Errorf("expected offset to point to length location, but got %s", word)
+		if pointer, err := solabi.ReadUint64(reader); err != nil || pointer != 32 {
+			return NewCriticalError(errors.New("invalid pointer field"))
 		}
-
-		// Read the length, it should always equal 64.
-		if word := readWord(); word != twoWordUint {
-			return fmt.Errorf("expected length to be 64 bytes, but got %s", word)
+		if length, err := solabi.ReadUint64(reader); err != nil || length != 64 {
+			return NewCriticalError(errors.New("invalid length field"))
 		}
-
-		// Set the system config's overhead and scalar values to the values read from the log
-		destSysCfg.Overhead = readWord()
-		destSysCfg.Scalar = readWord()
-
-		if countReadBytes != 32*4 {
-			return NewCriticalError(fmt.Errorf("expected 32*4 bytes in GPO params update data, but got %d", len(ev.Data)))
+		overhead, err := solabi.ReadEthBytes32(reader)
+		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"))
+		}
+		destSysCfg.Overhead = overhead
+		destSysCfg.Scalar = scalar
 		return nil
 	case SystemConfigUpdateGasLimit:
-		// Read the pointer, it should always equal 32.
-		if word := readWord(); word != oneWordUint {
-			return fmt.Errorf("expected offset to point to length location, but got %s", word)
+		if pointer, err := solabi.ReadUint64(reader); err != nil || pointer != 32 {
+			return NewCriticalError(errors.New("invalid pointer field"))
 		}
-
-		// Read the length, it should also always equal 32.
-		if word := readWord(); word != oneWordUint {
-			return fmt.Errorf("expected length to be 32 bytes, but got %s", word)
+		if length, err := solabi.ReadUint64(reader); err != nil || length != 32 {
+			return NewCriticalError(errors.New("invalid length field"))
 		}
-
-		// Indexing `word` directly is always safe here, it is guaranteed to be 32 bytes in length.
-		// Check that the gas limit is correctly zero-padded.
-		word := readWord()
-		if !bytes.Equal(word[:24], uint64Padding) {
-			return fmt.Errorf("expected zero padding for gaslimit, but got %x", word)
+		gasLimit, err := solabi.ReadUint64(reader)
+		if err != nil {
+			return NewCriticalError(errors.New("could not read gas limit"))
 		}
-		destSysCfg.GasLimit = binary.BigEndian.Uint64(word[24:])
-
-		if countReadBytes != 32*3 {
-			return NewCriticalError(fmt.Errorf("expected 32*3 bytes in gas limit update, but got %d bytes", len(ev.Data)))
+		if !solabi.EmptyReader(reader) {
+			return NewCriticalError(errors.New("too many bytes"))
 		}
-
+		destSysCfg.GasLimit = gasLimit
 		return nil
 	case SystemConfigUpdateUnsafeBlockSigner:
 		// Ignored in derivation. This configurable applies to runtime configuration outside of the derivation.

op-service/solabi/util.go

@@ -79,6 +79,12 @@ func ReadUint256(r io.Reader) (*big.Int, error) {
 	return new(big.Int).SetBytes(n[:]), nil
 }
 
+func EmptyReader(r io.Reader) bool {
+	var t [1]byte
+	n, err := r.Read(t[:])
+	return n == 0 && err == io.EOF
+}
+
 func WriteSignature(w io.Writer, sig []byte) error {
 	_, err := w.Write(sig)
 	return err

op-service/solabi/utils_test.go

+package solabi_test
+
+import (
+	"bytes"
+	"testing"
+
+	"github.com/ethereum-optimism/optimism/op-service/solabi"
+	"github.com/stretchr/testify/require"
+)
+
+func TestEmptyReader(t *testing.T) {
+	t.Run("empty", func(t *testing.T) {
+		r := new(bytes.Buffer)
+		require.True(t, solabi.EmptyReader(r))
+	})
+	t.Run("empty after read", func(t *testing.T) {
+		r := bytes.NewBufferString("not empty")
+		tmp := make([]byte, 9)
+		n, err := r.Read(tmp)
+		require.Equal(t, 9, n)
+		require.NoError(t, err)
+		require.True(t, solabi.EmptyReader(r))
+	})
+	t.Run("extra bytes", func(t *testing.T) {
+		r := bytes.NewBufferString("not empty")
+		require.False(t, solabi.EmptyReader(r))
+	})
+}