Merge pull request #4720 from ethereum-optimism/clabby/op-node/system-config-cleanup

fix(op-node): Use a reader to parse `SystemConfig` update events

op-node/rollup/derive/system_config.go

@@ -27,6 +27,17 @@ 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
@@ -69,50 +80,94 @@ func ProcessSystemConfigUpdateLogEvent(destSysCfg *eth.SystemConfig, ev *types.L
 	}
 	// indexed 1
 	updateType := ev.Topics[2]
-	// unindexed data
+
+	// 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:
-		if len(ev.Data) != 32*3 {
-			return fmt.Errorf("expected 32*3 bytes in batcher hash update, but got %d bytes", len(ev.Data))
+		// 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 x := common.BytesToHash(ev.Data[:32]); x != (common.Hash{31: 32}) {
-			return fmt.Errorf("expected offset to point to length location, but got %s", x)
+
+		// 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 x := common.BytesToHash(ev.Data[32:64]); x != (common.Hash{31: 32}) {
-			return fmt.Errorf("expected length of 1 bytes32, but got %s", x)
+
+		// 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)
 		}
-		if !bytes.Equal(ev.Data[64:64+12], make([]byte, 12)) {
-			return fmt.Errorf("expected version 0 batcher hash with zero padding, but got %x", ev.Data)
+		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)))
 		}
-		destSysCfg.BatcherAddr.SetBytes(ev.Data[64+12:])
+
 		return nil
-	case SystemConfigUpdateGasConfig: // left padded uint8
-		if len(ev.Data) != 32*4 {
-			return fmt.Errorf("expected 32*4 bytes in GPO params update data, but got %d", len(ev.Data))
+	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 x := common.BytesToHash(ev.Data[:32]); x != (common.Hash{31: 32}) {
-			return fmt.Errorf("expected offset to point to length location, but got %s", x)
+
+		// 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 x := common.BytesToHash(ev.Data[32:64]); x != (common.Hash{31: 64}) {
-			return fmt.Errorf("expected length of 2 bytes32, but got %s", x)
+
+		// 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)))
 		}
-		copy(destSysCfg.Overhead[:], ev.Data[64:96])
-		copy(destSysCfg.Scalar[:], ev.Data[96:128])
+
 		return nil
 	case SystemConfigUpdateGasLimit:
-		if len(ev.Data) != 32*3 {
-			return fmt.Errorf("expected 32*3 bytes in gas limit update, but got %d bytes", len(ev.Data))
+		// 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 x := common.BytesToHash(ev.Data[:32]); x != (common.Hash{31: 32}) {
-			return fmt.Errorf("expected offset to point to length location, but got %s", x)
+
+		// 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 x := common.BytesToHash(ev.Data[32:64]); x != (common.Hash{31: 32}) {
-			return fmt.Errorf("expected length of 1 bytes32, but got %s", x)
+
+		// 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)
 		}
-		if !bytes.Equal(ev.Data[64:64+24], make([]byte, 24)) {
-			return fmt.Errorf("expected zero padding for gaslimit, but got %x", ev.Data)
+		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)))
 		}
-		destSysCfg.GasLimit = binary.BigEndian.Uint64(ev.Data[64+24:])
+
 		return nil
 	case SystemConfigUpdateUnsafeBlockSigner:
 		// Ignored in derivation. This configurable applies to runtime configuration outside of the derivation.