refactor(core/types): simplify Body RLP override

core/types/block.libevm.go

@@ -112,79 +112,28 @@ func (*NOOPHeaderHooks) DecodeRLP(h *Header, s *rlp.Stream) error {
 }
 func (*NOOPHeaderHooks) PostCopy(dst *Header) {}
 
-var (
-	_ interface {
-		rlp.Encoder
-		rlp.Decoder
-	} = (*Body)(nil)
-
-	// The implementations of [Body.EncodeRLP] and [Body.DecodeRLP] make
-	// assumptions about the struct fields and their order, which we lock in here as a change
-	// detector. If this breaks then it MUST be updated and the RLP methods
-	// reviewed + new backwards-compatibility tests added.
-	_ = &Body{[]*Transaction{}, []*Header{}, []*Withdrawal{}}
-)
+var _ interface {
+	rlp.Encoder
+	rlp.Decoder
+} = (*Body)(nil)
 
 // EncodeRLP implements the [rlp.Encoder] interface.
 func (b *Body) EncodeRLP(dst io.Writer) error {
-	w := rlp.NewEncoderBuffer(dst)
-
-	return w.InList(func() error {
-		if err := rlp.EncodeListToBuffer(w, b.Transactions); err != nil {
-			return err
-		}
-		if err := rlp.EncodeListToBuffer(w, b.Uncles); err != nil {
-			return err
-		}
-
-		hasLaterOptionalField := b.Withdrawals != nil
-		if err := b.hooks().AppendRLPFields(w, hasLaterOptionalField); err != nil {
-			return err
-		}
-		if !hasLaterOptionalField {
-			return nil
-		}
-		return rlp.EncodeListToBuffer(w, b.Withdrawals)
-	})
+	req, opt := b.hooks().RLPFieldsForEncoding(b)
+	return rlp.EncodeStructFields(dst, req, opt)
 }
 
 // DecodeRLP implements the [rlp.Decoder] interface.
 func (b *Body) DecodeRLP(s *rlp.Stream) error {
-	return s.FromList(func() error {
-		txs, err := rlp.DecodeList[Transaction](s)
-		if err != nil {
-			return err
-		}
-		uncles, err := rlp.DecodeList[Header](s)
-		if err != nil {
-			return err
-		}
-		*b = Body{
-			Transactions: txs,
-			Uncles:       uncles,
-		}
-
-		if err := b.hooks().DecodeExtraRLPFields(s); err != nil {
-			return err
-		}
-		if !s.MoreDataInList() {
-			return nil
-		}
-
-		ws, err := rlp.DecodeList[Withdrawal](s)
-		if err != nil {
-			return err
-		}
-		b.Withdrawals = ws
-		return nil
-	})
+	req, opt := b.hooks().RLPFieldPointersForDecoding(b)
+	return s.DecodeStructFields(req, opt)
 }
 
 // BodyHooks are required for all types registered with [RegisterExtras] for
 // [Body] payloads.
 type BodyHooks interface {
-	AppendRLPFields(_ rlp.EncoderBuffer, mustWriteEmptyOptional bool) error
-	DecodeExtraRLPFields(*rlp.Stream) error
+	RLPFieldsForEncoding(*Body) (required, optional []any)
+	RLPFieldPointersForDecoding(*Body) (required, optional []any)
 }
 
 // TestOnlyRegisterBodyHooks is a temporary means of "registering" BodyHooks for
@@ -209,5 +158,16 @@ func (b *Body) hooks() BodyHooks {
 // having been registered.
 type NOOPBodyHooks struct{}
 
-func (NOOPBodyHooks) AppendRLPFields(rlp.EncoderBuffer, bool) error { return nil }
-func (NOOPBodyHooks) DecodeExtraRLPFields(*rlp.Stream) error        { return nil }
+// The RLP-related methods of [NOOPBodyHooks] make assumptions about the struct
+// fields and their order, which we lock in here as a change detector. If this
+// breaks then it MUST be updated and the RLP methods reviewed + new
+// backwards-compatibility tests added.
+var _ = &Body{[]*Transaction{}, []*Header{}, []*Withdrawal{}}
+
+func (NOOPBodyHooks) RLPFieldsForEncoding(b *Body) ([]any, []any) {
+	return []any{b.Transactions, b.Uncles}, []any{b.Withdrawals}
+}
+
+func (NOOPBodyHooks) RLPFieldPointersForDecoding(b *Body) ([]any, []any) {
+	return []any{&b.Transactions, &b.Uncles}, []any{&b.Withdrawals}
+}

core/types/rlp_backwards_compat.libevm_test.go

@@ -116,8 +116,8 @@ func TestBodyRLPBackwardsCompatibility(t *testing.T) {
 	newHdr := func(hashLow byte) *Header { return &Header{ParentHash: common.Hash{hashLow}} }
 	newWithdraw := func(idx uint64) *Withdrawal { return &Withdrawal{Index: idx} }
 
-	// We build up test-case [Body] instances from the power set of each of
-	// these components.
+	// We build up test-case [Body] instances from the Cartesian product of each
+	// of these components.
 	txMatrix := [][]*Transaction{
 		nil, {}, // Must be equivalent for non-optional field
 		{newTx(1)},
@@ -197,35 +197,27 @@ type cChainBodyExtras struct {
 
 var _ BodyHooks = (*cChainBodyExtras)(nil)
 
-func (e *cChainBodyExtras) AppendRLPFields(b rlp.EncoderBuffer, _ bool) error {
-	b.WriteUint64(uint64(e.Version))
-
-	var data []byte
-	if e.ExtData != nil {
-		data = *e.ExtData
-	}
-	b.WriteBytes(data)
-
-	return nil
+func (e *cChainBodyExtras) RLPFieldsForEncoding(b *Body) ([]any, []any) {
+	// The Avalanche C-Chain uses all of the geth required fields (but none of
+	// the optional ones) so there's no need to explicitly list them. This
+	// pattern might not be ideal for readability but is used here for
+	// demonstrative purposes.
+	//
+	// All new fields will always be tagged as optional for backwards
+	// compatibility so this is safe to do, but only for the required fields.
+	req, _ /*drop all optional*/ := NOOPBodyHooks{}.RLPFieldsForEncoding(b)
+	return append(req, e.Version, e.ExtData), nil
 }
 
-func (e *cChainBodyExtras) DecodeExtraRLPFields(s *rlp.Stream) error {
-	if err := s.Decode(&e.Version); err != nil {
-		return err
-	}
-
-	buf, err := s.Bytes()
-	if err != nil {
-		return err
-	}
-	if len(buf) > 0 {
-		e.ExtData = &buf
-	} else {
-		// Respect the `rlp:"nil"` field tag.
-		e.ExtData = nil
-	}
-
-	return nil
+func (e *cChainBodyExtras) RLPFieldPointersForDecoding(b *Body) ([]any, []any) {
+	// An alternative to the pattern used above is to explicitly list all
+	// fields for better introspection.
+	return []any{
+		&b.Transactions,
+		&b.Uncles,
+		&e.Version,
+		rlp.Nillable(&e.ExtData), // equivalent to `rlp:"nil"`
+	}, nil
 }
 
 func TestBodyRLPCChainCompat(t *testing.T) {
@@ -256,12 +248,14 @@ func TestBodyRLPCChainCompat(t *testing.T) {
 		wantRLPHex string
 	}{
 		{
+			name: "nil ExtData",
 			extra: &cChainBodyExtras{
 				Version: version,
 			},
 			wantRLPHex: `e5dedd2a80809400000000000000000000000000decafc0ffeebad8080808080c08304cb2f80`,
 		},
 		{
+			name: "non-nil ExtData",
 			extra: &cChainBodyExtras{
 				Version: version,
 				ExtData: &[]byte{1, 4, 2, 8, 5, 7},

rlp/list.libevm.go

@@ -16,6 +16,13 @@
 
 package rlp
 
+import (
+	"errors"
+	"fmt"
+	"io"
+	"reflect"
+)
+
 // InList is a convenience wrapper, calling `fn` between calls to
 // [EncoderBuffer.List] and [EncoderBuffer.ListEnd]. If `fn` returns an error,
 // it is propagated directly.
@@ -42,6 +49,64 @@ func EncodeListToBuffer[T any](b EncoderBuffer, vals []T) error {
 	})
 }
 
+// EncodeStructFields encodes the `required` and `optional` slices to `w`,
+// concatenated as a single list, as if they were fields in a struct. The
+// optional "fields" are treated identically to those tagged with
+// `rlp:"optional"`.
+//
+// See the example for [Stream.DecodeStructFields].
+func EncodeStructFields(w io.Writer, required, optional []any) error {
+	includeOptional, err := optionalFieldInclusionFlags(optional)
+	if err != nil {
+		return err
+	}
+
+	b := NewEncoderBuffer(w)
+	err = b.InList(func() error {
+		for _, v := range required {
+			if err := Encode(b, v); err != nil {
+				return err
+			}
+		}
+
+		for i, v := range optional {
+			if !includeOptional[i] {
+				return nil
+			}
+			if err := Encode(b, v); err != nil {
+				return err
+			}
+		}
+		return nil
+	})
+	if err != nil {
+		return err
+	}
+	return b.Flush()
+}
+
+var errUnsupportedOptionalFieldType = errors.New("unsupported optional field type")
+
+// optionalFieldInclusionFlags returns a slice of booleans, the same length as
+// `vals`, indicating whether or not the respective optional value MUST be
+// written to a list. A value must be written if it or any later value is
+// non-nil; the returned slice is therefore monotonic non-increasing from true
+// to false.
+func optionalFieldInclusionFlags(vals []any) ([]bool, error) {
+	flags := make([]bool, len(vals))
+	var include bool
+	for i := len(vals) - 1; i >= 0; i-- {
+		switch v := reflect.ValueOf(vals[i]); v.Kind() {
+		case reflect.Slice, reflect.Pointer:
+			include = include || !v.IsNil()
+		default:
+			return nil, fmt.Errorf("%w: %T", errUnsupportedOptionalFieldType, vals[i])
+		}
+		flags[i] = include
+	}
+	return flags, nil
+}
+
 // FromList is a convenience wrapper, calling `fn` between calls to
 // [Stream.List] and [Stream.ListEnd]. If `fn` returns an error, it is
 // propagated directly.
@@ -75,3 +140,53 @@ func DecodeList[T any](s *Stream) ([]*T, error) {
 	})
 	return vals, err
 }
+
+// DecodeStructFields is the inverse of [EncodeStructFields]. All destination
+// fields, be they `required` or `optional`, MUST be pointers and all `optional`
+// fields MUST be provided in case they are present in the RLP being decoded.
+//
+// Typically, the arguments to this function mirror those passed to
+// [EncodeStructFields] except for being pointers. See the example.
+func (s *Stream) DecodeStructFields(required, optional []any) error {
+	return s.FromList(func() error {
+		for _, v := range required {
+			if err := s.Decode(v); err != nil {
+				return err
+			}
+		}
+
+		for _, v := range optional {
+			if !s.MoreDataInList() {
+				return nil
+			}
+			if err := s.Decode(v); err != nil {
+				return err
+			}
+		}
+		return nil
+	})
+}
+
+// Nillable wraps `field` to mirror the behaviour of an `rlp:"nil"` tag; i.e. if
+// a zero-sized RLP item is decoded into the returned Decoder then it is dropped
+// and `*field` is set to nil, otherwise the RLP item is decoded directly into
+// `field`. The return argument is intended for use with
+// [Stream.DecodeStructFields].
+func Nillable[T any](field **T) Decoder {
+	return &nillable[T]{field}
+}
+
+type nillable[T any] struct{ v **T }
+
+func (n *nillable[T]) DecodeRLP(s *Stream) error {
+	_, size, err := s.Kind()
+	if err != nil {
+		return err
+	}
+	if size > 0 {
+		return s.Decode(n.v)
+	}
+	*n.v = nil
+	_, err = s.Raw() // consume the item
+	return err
+}