feat(core/types): Block RLP overriding (#133)

## Why this should be merged

Support for configurable `core/types.Block` with RLP encoding, including
interplay with `Body`.

## How this works

`Block` doesn't export most of its fields so relies on an internal type,
`extblock`, for RLP encoding. This type is modified to implement the
`rlp.Encoder` and `Decoder` methods as a point to inject hooks using
`rlp.Fields` (as in #120 for `Body`).

`Block` shares the same registered extra type as `Body`. Unlike
`Header`, which has its own field in a `Block`, the fields in `Body` are
promoted to be carried directly. This suggests that (at least for pure
data payloads) the modifications might be equivalent (and
`ava-labs/coreth` evidences this). Should different payloads be
absolutely required in the future, we can split the types—the
`RegisterExtras` signature is already too verbose though 😢.

## How this was tested

Explicit inclusion of a backwards-compatibility test for
`NOOPBlockBodyHooks` + implicit testing via the multiple upstream tests
in `block_test.go`. Re implicit testing: default behaviour is now to use
the noop hooks even when no registration is performed, but if we change
this then the tests in `block_test.go` can still be called as subtests
from a test that explicitly registers noops.

---------

Signed-off-by: Arran Schlosberg <[email protected]>
Co-authored-by: Quentin McGaw <[email protected]>

Author: ARR4N

core/state/state.libevm_test.go

@@ -47,7 +47,7 @@ func TestGetSetExtra(t *testing.T) {
 	// test deep copying.
 	payloads := types.RegisterExtras[
 		types.NOOPHeaderHooks, *types.NOOPHeaderHooks,
-		types.NOOPBodyHooks, *types.NOOPBodyHooks,
+		types.NOOPBlockBodyHooks, *types.NOOPBlockBodyHooks,
 		*accountExtra,
 	]().StateAccount
 

core/state/state_object.libevm_test.go

@@ -48,7 +48,7 @@ func TestStateObjectEmpty(t *testing.T) {
 			registerAndSet: func(acc *types.StateAccount) {
 				types.RegisterExtras[
 					types.NOOPHeaderHooks, *types.NOOPHeaderHooks,
-					types.NOOPBodyHooks, *types.NOOPBodyHooks,
+					types.NOOPBlockBodyHooks, *types.NOOPBlockBodyHooks,
 					bool,
 				]().StateAccount.Set(acc, false)
 			},
@@ -59,7 +59,7 @@ func TestStateObjectEmpty(t *testing.T) {
 			registerAndSet: func(*types.StateAccount) {
 				types.RegisterExtras[
 					types.NOOPHeaderHooks, *types.NOOPHeaderHooks,
-					types.NOOPBodyHooks, *types.NOOPBodyHooks,
+					types.NOOPBlockBodyHooks, *types.NOOPBlockBodyHooks,
 					bool,
 				]()
 			},
@@ -70,7 +70,7 @@ func TestStateObjectEmpty(t *testing.T) {
 			registerAndSet: func(acc *types.StateAccount) {
 				types.RegisterExtras[
 					types.NOOPHeaderHooks, *types.NOOPHeaderHooks,
-					types.NOOPBodyHooks, *types.NOOPBodyHooks,
+					types.NOOPBlockBodyHooks, *types.NOOPBlockBodyHooks,
 					bool,
 				]().StateAccount.Set(acc, true)
 			},

core/types/backwards_compat.libevm_test.go

@@ -30,11 +30,11 @@ import (
 	"github.com/ava-labs/libevm/rlp"
 )
 
-func TestBodyRLPBackwardsCompatibility(t *testing.T) {
-	newTx := func(nonce uint64) *Transaction { return NewTx(&LegacyTx{Nonce: nonce}) }
-	newHdr := func(hashLow byte) *Header { return &Header{ParentHash: common.Hash{hashLow}} }
-	newWithdraw := func(idx uint64) *Withdrawal { return &Withdrawal{Index: idx} }
+func newTx(nonce uint64) *Transaction    { return NewTx(&LegacyTx{Nonce: nonce}) }
+func newHdr(parentHashHigh byte) *Header { return &Header{ParentHash: common.Hash{parentHashHigh}} }
+func newWithdraw(idx uint64) *Withdrawal { return &Withdrawal{Index: idx} }
 
+func blockBodyRLPTestInputs() []*Body {
 	// We build up test-case [Body] instances from the Cartesian product of each
 	// of these components.
 	txMatrix := [][]*Transaction{
@@ -61,8 +61,11 @@ func TestBodyRLPBackwardsCompatibility(t *testing.T) {
 			}
 		}
 	}
+	return bodies
+}
 
-	for _, body := range bodies {
+func TestBodyRLPBackwardsCompatibility(t *testing.T) {
+	for _, body := range blockBodyRLPTestInputs() {
 		t.Run("", func(t *testing.T) {
 			t.Cleanup(func() {
 				if t.Failed() {
@@ -86,8 +89,10 @@ func TestBodyRLPBackwardsCompatibility(t *testing.T) {
 			t.Run("Decode", func(t *testing.T) {
 				got := new(Body)
 				err := rlp.DecodeBytes(wantRLP, got)
-				require.NoErrorf(t, err, "rlp.DecodeBytes(rlp.EncodeToBytes(%T), %T) resulted in %s",
-					(*withoutMethods)(body), got, pretty.Sprint(got))
+				require.NoErrorf(
+					t, err, "rlp.DecodeBytes(rlp.EncodeToBytes(%T), %T) resulted in %s",
+					(*withoutMethods)(body), got, pretty.Sprint(got),
+				)
 
 				want := body
 				// Regular RLP decoding will never leave these non-optional
@@ -112,17 +117,94 @@ func TestBodyRLPBackwardsCompatibility(t *testing.T) {
 	}
 }
 
+func TestBlockRLPBackwardsCompatibility(t *testing.T) {
+	TestOnlyClearRegisteredExtras()
+	t.Cleanup(TestOnlyClearRegisteredExtras)
+
+	RegisterExtras[
+		NOOPHeaderHooks, *NOOPHeaderHooks,
+		NOOPBlockBodyHooks, *NOOPBlockBodyHooks, // types under test
+		struct{},
+	]()
+
+	// Note that there are also a number of tests in `block_test.go` that ensure
+	// backwards compatibility as [NOOPBlockBodyHooks] are used by default when
+	// nothing is registered (the above registration is only for completeness).
+
+	for _, body := range blockBodyRLPTestInputs() {
+		t.Run("", func(t *testing.T) {
+			// [Block] doesn't export most of its fields so uses [extblock] as a
+			// proxy for RLP encoding, which is what we therefore use as the
+			// backwards-compatible gold standard.
+			hdr := newHdr(99)
+			block := extblock{
+				Header:      hdr,
+				Txs:         body.Transactions,
+				Uncles:      body.Uncles,
+				Withdrawals: body.Withdrawals,
+			}
+
+			// We've added [extblock.EncodeRLP] and [extblock.DecodeRLP] for our
+			// hooks.
+			type withoutMethods extblock
+
+			wantRLP, err := rlp.EncodeToBytes(withoutMethods(block))
+			require.NoErrorf(t, err, "rlp.EncodeToBytes([%T with methods stripped])", block)
+
+			// Our input to RLP might not be the canonical RLP output.
+			var wantBlock extblock
+			err = rlp.DecodeBytes(wantRLP, (*withoutMethods)(&wantBlock))
+			require.NoErrorf(t, err, "rlp.DecodeBytes(..., [%T with methods stripped])", &wantBlock)
+
+			t.Run("Encode", func(t *testing.T) {
+				b := NewBlockWithHeader(hdr).WithBody(*body).WithWithdrawals(body.Withdrawals)
+				got, err := rlp.EncodeToBytes(b)
+				require.NoErrorf(t, err, "rlp.EncodeToBytes(%T)", b)
+
+				assert.Equalf(t, wantRLP, got, "expect %T RLP identical to that from %T struct stripped of methods", got, extblock{})
+			})
+
+			t.Run("Decode", func(t *testing.T) {
+				var gotBlock Block
+				err := rlp.DecodeBytes(wantRLP, &gotBlock)
+				require.NoErrorf(t, err, "rlp.DecodeBytes(..., %T)", &gotBlock)
+
+				got := extblock{
+					gotBlock.Header(),
+					gotBlock.Transactions(),
+					gotBlock.Uncles(),
+					gotBlock.Withdrawals(),
+					nil, // unexported libevm hooks
+				}
+
+				opts := cmp.Options{
+					cmp.Comparer((*Header).equalHash),
+					cmp.Comparer((*Transaction).equalHash),
+					cmpopts.IgnoreUnexported(extblock{}),
+				}
+				if diff := cmp.Diff(wantBlock, got, opts); diff != "" {
+					t.Errorf("rlp.DecodeBytes([RLP from %T stripped of methods], ...) diff (-want +got):\n%s", extblock{}, diff)
+				}
+			})
+		})
+	}
+}
+
 // cChainBodyExtras carries the same additional fields as the Avalanche C-Chain
-// (ava-labs/coreth) [Body] and implements [BodyHooks] to achieve equivalent RLP
-// {en,de}coding.
+// (ava-labs/coreth) [Body] and implements [BlockBodyHooks] to achieve
+// equivalent RLP {en,de}coding.
+//
+// It is not intended as a full test of ava-labs/coreth existing functionality,
+// which should be implemented when that module consumes libevm, but as proof of
+// equivalence of the [rlp.Fields] approach.
 type cChainBodyExtras struct {
 	Version uint32
 	ExtData *[]byte
 }
 
-var _ BodyHooks = (*cChainBodyExtras)(nil)
+var _ BlockBodyHooks = (*cChainBodyExtras)(nil)
 
-func (e *cChainBodyExtras) RLPFieldsForEncoding(b *Body) *rlp.Fields {
+func (e *cChainBodyExtras) BodyRLPFieldsForEncoding(b *Body) *rlp.Fields {
 	// 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
@@ -132,13 +214,13 @@ func (e *cChainBodyExtras) RLPFieldsForEncoding(b *Body) *rlp.Fields {
 	// compatibility so this is safe to do, but only for the required fields.
 	return &rlp.Fields{
 		Required: append(
-			NOOPBodyHooks{}.RLPFieldsForEncoding(b).Required,
+			NOOPBlockBodyHooks{}.BodyRLPFieldsForEncoding(b).Required,
 			e.Version, e.ExtData,
 		),
 	}
 }
 
-func (e *cChainBodyExtras) RLPFieldPointersForDecoding(b *Body) *rlp.Fields {
+func (e *cChainBodyExtras) BodyRLPFieldPointersForDecoding(b *Body) *rlp.Fields {
 	// An alternative to the pattern used above is to explicitly list all
 	// fields for better introspection.
 	return &rlp.Fields{
@@ -151,6 +233,20 @@ func (e *cChainBodyExtras) RLPFieldPointersForDecoding(b *Body) *rlp.Fields {
 	}
 }
 
+// See [cChainBodyExtras] intent.
+
+func (e *cChainBodyExtras) Copy() *cChainBodyExtras {
+	panic("unimplemented")
+}
+
+func (e *cChainBodyExtras) BlockRLPFieldsForEncoding(b *BlockRLPProxy) *rlp.Fields {
+	panic("unimplemented")
+}
+
+func (e *cChainBodyExtras) BlockRLPFieldPointersForDecoding(b *BlockRLPProxy) *rlp.Fields {
+	panic("unimplemented")
+}
+
 func TestBodyRLPCChainCompat(t *testing.T) {
 	// The inputs to this test were used to generate the expected RLP with
 	// ava-labs/coreth. This serves as both an example of how to use [BodyHooks]

core/types/backwards_compat_diffpkg.libevm_test.go

@@ -42,7 +42,7 @@ func TestHeaderRLPBackwardsCompatibility(t *testing.T) {
 			register: func() {
 				RegisterExtras[
 					NOOPHeaderHooks, *NOOPHeaderHooks,
-					NOOPBodyHooks, *NOOPBodyHooks,
+					NOOPBlockBodyHooks, *NOOPBlockBodyHooks,
 					struct{},
 				]()
 			},

core/types/block.go

@@ -211,6 +211,8 @@ type Block struct {
 	// inter-peer block relay.
 	ReceivedAt   time.Time
 	ReceivedFrom interface{}
+
+	extra *pseudo.Type // See [RegisterExtras]
 }
 
 // "external" block encoding. used for eth protocol, etc.
@@ -219,6 +221,8 @@ type extblock struct {
 	Txs         []*Transaction
 	Uncles      []*Header
 	Withdrawals []*Withdrawal `rlp:"optional"`
+
+	hooks BlockBodyHooks // libevm: MUST be unexported + populated from [Block.hooks]
 }
 
 // NewBlock creates a new block. The input data is copied, changes to header and to the
@@ -318,6 +322,7 @@ func CopyHeader(h *Header) *Header {
 // DecodeRLP decodes a block from RLP.
 func (b *Block) DecodeRLP(s *rlp.Stream) error {
 	var eb extblock
+	eb.hooks = b.hooks()
 	_, size, _ := s.Kind()
 	if err := s.Decode(&eb); err != nil {
 		return err
@@ -334,13 +339,14 @@ func (b *Block) EncodeRLP(w io.Writer) error {
 		Txs:         b.transactions,
 		Uncles:      b.uncles,
 		Withdrawals: b.withdrawals,
+		hooks:       b.hooks(),
 	})
 }
 
 // Body returns the non-header content of the block.
 // Note the returned data is not an independent copy.
 func (b *Block) Body() *Body {
-	return &Body{b.transactions, b.uncles, b.withdrawals, nil /* unexported extras field */}
+	return &Body{b.transactions, b.uncles, b.withdrawals, b.cloneExtra()}
 }
 
 // Accessors for body data. These do not return a copy because the content
@@ -458,6 +464,7 @@ func (b *Block) WithSeal(header *Header) *Block {
 		transactions: b.transactions,
 		uncles:       b.uncles,
 		withdrawals:  b.withdrawals,
+		extra:        b.cloneExtra(),
 	}
 }
 
@@ -468,6 +475,7 @@ func (b *Block) WithBody(body Body) *Block {
 		transactions: make([]*Transaction, len(body.Transactions)),
 		uncles:       make([]*Header, len(body.Uncles)),
 		withdrawals:  b.withdrawals,
+		extra:        body.cloneExtra(),
 	}
 	copy(block.transactions, body.Transactions)
 	for i := range body.Uncles {
@@ -482,6 +490,7 @@ func (b *Block) WithWithdrawals(withdrawals []*Withdrawal) *Block {
 		header:       b.header,
 		transactions: b.transactions,
 		uncles:       b.uncles,
+		extra:        b.cloneExtra(),
 	}
 	if withdrawals != nil {
 		block.withdrawals = make([]*Withdrawal, len(withdrawals))