From b6241e367af7d0c7b915b50dd6bac19ab1647453 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Wed, 12 Feb 2025 11:40:23 +0000 Subject: [PATCH 01/13] refactor: paired parentheses at same indent --- core/types/backwards_compat.libevm_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/core/types/backwards_compat.libevm_test.go b/core/types/backwards_compat.libevm_test.go index b7dcc20cb46..b7c921adc57 100644 --- a/core/types/backwards_compat.libevm_test.go +++ b/core/types/backwards_compat.libevm_test.go @@ -86,8 +86,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 From 756068e1fc9bfe7f0ec95af9ce45ce3f2dda52d1 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Wed, 12 Feb 2025 13:34:19 +0000 Subject: [PATCH 02/13] feat(core/types): `Block` RLP overriding --- .../backwards_compat_diffpkg.libevm_test.go | 2 +- core/types/block.go | 9 +- core/types/block.libevm.go | 128 +++++++++--------- core/types/block.libevm_test.go | 2 +- core/types/rlp_payload.libevm.go | 95 ++++++++++--- core/types/state_account.libevm_test.go | 4 +- .../state_account_storage.libevm_test.go | 8 +- 7 files changed, 160 insertions(+), 88 deletions(-) diff --git a/core/types/backwards_compat_diffpkg.libevm_test.go b/core/types/backwards_compat_diffpkg.libevm_test.go index 669c3fd0ea7..e4c0f2b6d1b 100644 --- a/core/types/backwards_compat_diffpkg.libevm_test.go +++ b/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{}, ]() }, diff --git a/core/types/block.go b/core/types/block.go index f6a46e1bfb3..cb8ab100614 100644 --- a/core/types/block.go +++ b/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 @@ -468,6 +474,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 { diff --git a/core/types/block.libevm.go b/core/types/block.libevm.go index a02abbdb423..8faf5b51f41 100644 --- a/core/types/block.libevm.go +++ b/core/types/block.libevm.go @@ -18,7 +18,6 @@ package types import ( "encoding/json" - "fmt" "io" "github.com/ava-labs/libevm/libevm/pseudo" @@ -35,19 +34,6 @@ type HeaderHooks interface { PostCopy(dst *Header) } -// hooks returns the Header's registered HeaderHooks, if any, otherwise a -// [NOOPHeaderHooks] suitable for running default behaviour. -func (h *Header) hooks() HeaderHooks { - if r := registeredExtras; r.Registered() { - return r.Get().hooks.hooksFromHeader(h) - } - return new(NOOPHeaderHooks) -} - -func (e ExtraPayloads[HPtr, BPtr, SA]) hooksFromHeader(h *Header) HeaderHooks { - return e.Header.Get(h) -} - var _ interface { rlp.Encoder rlp.Decoder @@ -75,18 +61,6 @@ func (h *Header) DecodeRLP(s *rlp.Stream) error { return h.hooks().DecodeRLP(h, s) } -func (h *Header) extraPayload() *pseudo.Type { - r := registeredExtras - if !r.Registered() { - // See params.ChainConfig.extraPayload() for panic rationale. - panic(fmt.Sprintf("%T.extraPayload() called before RegisterExtras()", r)) - } - if h.extra == nil { - h.extra = r.Get().newHeader() - } - return h.extra -} - // NOOPHeaderHooks implements [HeaderHooks] such that they are equivalent to // no type having been registered. type NOOPHeaderHooks struct{} @@ -111,71 +85,101 @@ func (*NOOPHeaderHooks) DecodeRLP(h *Header, s *rlp.Stream) error { } func (*NOOPHeaderHooks) PostCopy(dst *Header) {} -var _ interface { +func (b *Body) cloneExtra() *pseudo.Type { + return b.extra // TODO(arr4n) implement this // DO NOT MERGE +} + +func (b *Block) cloneExtra() *pseudo.Type { + return b.extra // TODO(arr4n) implement this // DO NOT MERGE +} + +var _ = []interface { rlp.Encoder rlp.Decoder -} = (*Body)(nil) +}{ + (*Body)(nil), + (*extblock)(nil), +} // EncodeRLP implements the [rlp.Encoder] interface. func (b *Body) EncodeRLP(w io.Writer) error { - return b.hooks().RLPFieldsForEncoding(b).EncodeRLP(w) + return b.hooks().BodyRLPFieldsForEncoding(b).EncodeRLP(w) } // DecodeRLP implements the [rlp.Decoder] interface. func (b *Body) DecodeRLP(s *rlp.Stream) error { - return b.hooks().RLPFieldPointersForDecoding(b).DecodeRLP(s) + return b.hooks().BodyRLPFieldPointersForDecoding(b).DecodeRLP(s) +} + +// BlockRLPProxy exports the geth-internal type used for RLP {en,de}coding of a +// [Block]. +type BlockRLPProxy extblock + +func (b *extblock) EncodeRLP(w io.Writer) error { + bb := (*BlockRLPProxy)(b) + return b.hooks.BlockRLPFieldsForEncoding(bb).EncodeRLP(w) } -// BodyHooks are required for all types registered with [RegisterExtras] for -// [Body] payloads. -type BodyHooks interface { - RLPFieldsForEncoding(*Body) *rlp.Fields - RLPFieldPointersForDecoding(*Body) *rlp.Fields +func (b *extblock) DecodeRLP(s *rlp.Stream) error { + bb := (*BlockRLPProxy)(b) + return b.hooks.BlockRLPFieldPointersForDecoding(bb).DecodeRLP(s) } -func (b *Body) hooks() BodyHooks { - if r := registeredExtras; r.Registered() { - return r.Get().hooks.hooksFromBody(b) - } - return NOOPBodyHooks{} +// BlockBodyHooks are required for all types registered with [RegisterExtras] +// for [Block] and [Body] payloads. +type BlockBodyHooks interface { + BlockRLPFieldsForEncoding(*BlockRLPProxy) *rlp.Fields + BlockRLPFieldPointersForDecoding(*BlockRLPProxy) *rlp.Fields + BodyRLPFieldsForEncoding(*Body) *rlp.Fields + BodyRLPFieldPointersForDecoding(*Body) *rlp.Fields } -// NOOPBodyHooks implements [BodyHooks] such that they are equivalent to no type -// having been registered. -type NOOPBodyHooks struct{} +// NOOPBlockBodyHooks implements [BlockBodyHooks] such that they are equivalent +// to no type having been registered. +type NOOPBlockBodyHooks struct{} -// 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 +// The RLP-related methods of [NOOPBlockBodyHooks] make assumptions about the +// struct fields and their order, which we lock in here as a change detector. If +// these break then they MUST be updated and the RLP methods reviewed + new // backwards-compatibility tests added. -var _ = &Body{[]*Transaction{}, []*Header{}, []*Withdrawal{}, nil /* extra unexported type */} +var ( + _ = &Body{ + []*Transaction{}, []*Header{}, []*Withdrawal{}, // geth + &pseudo.Type{}, // libevm + } + _ = extblock{ + &Header{}, []*Transaction{}, []*Header{}, []*Withdrawal{}, // geth + BlockBodyHooks(nil), // libevm + } + // Demonstrate identity of these two types, by definition but useful for + // inspection here. + _ = extblock(BlockRLPProxy{}) +) -func (NOOPBodyHooks) RLPFieldsForEncoding(b *Body) *rlp.Fields { +func (NOOPBlockBodyHooks) BlockRLPFieldsForEncoding(b *BlockRLPProxy) *rlp.Fields { return &rlp.Fields{ - Required: []any{b.Transactions, b.Uncles}, + Required: []any{b.Header, b.Txs, b.Uncles}, Optional: []any{b.Withdrawals}, } } -func (NOOPBodyHooks) RLPFieldPointersForDecoding(b *Body) *rlp.Fields { +func (NOOPBlockBodyHooks) BlockRLPFieldPointersForDecoding(b *BlockRLPProxy) *rlp.Fields { return &rlp.Fields{ - Required: []any{&b.Transactions, &b.Uncles}, + Required: []any{&b.Header, &b.Txs, &b.Uncles}, Optional: []any{&b.Withdrawals}, } } -func (e ExtraPayloads[HPtr, BPtr, SA]) hooksFromBody(b *Body) BodyHooks { - return e.Body.Get(b) +func (NOOPBlockBodyHooks) BodyRLPFieldsForEncoding(b *Body) *rlp.Fields { + return &rlp.Fields{ + Required: []any{b.Transactions, b.Uncles}, + Optional: []any{b.Withdrawals}, + } } -func (b *Body) extraPayload() *pseudo.Type { - r := registeredExtras - if !r.Registered() { - // See params.ChainConfig.extraPayload() for panic rationale. - panic(fmt.Sprintf("%T.extraPayload() called before RegisterExtras()", r)) - } - if b.extra == nil { - b.extra = r.Get().newBody() +func (NOOPBlockBodyHooks) BodyRLPFieldPointersForDecoding(b *Body) *rlp.Fields { + return &rlp.Fields{ + Required: []any{&b.Transactions, &b.Uncles}, + Optional: []any{&b.Withdrawals}, } - return b.extra } diff --git a/core/types/block.libevm_test.go b/core/types/block.libevm_test.go index 39898c7aec3..fa4302e6d6a 100644 --- a/core/types/block.libevm_test.go +++ b/core/types/block.libevm_test.go @@ -88,7 +88,7 @@ func TestHeaderHooks(t *testing.T) { extras := RegisterExtras[ stubHeaderHooks, *stubHeaderHooks, - NOOPBodyHooks, *NOOPBodyHooks, + NOOPBlockBodyHooks, *NOOPBlockBodyHooks, struct{}, ]() rng := ethtest.NewPseudoRand(13579) diff --git a/core/types/rlp_payload.libevm.go b/core/types/rlp_payload.libevm.go index db9863908c8..ed47d62393b 100644 --- a/core/types/rlp_payload.libevm.go +++ b/core/types/rlp_payload.libevm.go @@ -47,7 +47,7 @@ func RegisterExtras[ *H }, B any, BPtr interface { - BodyHooks + BlockBodyHooks *B }, SA any, @@ -61,6 +61,10 @@ func RegisterExtras[ (*Body).extraPayload, func(b *Body, t *pseudo.Type) { b.extra = t }, ), + Block: pseudo.NewAccessor[*Block, BPtr]( + (*Block).extraPayload, + func(b *Block, t *pseudo.Type) { b.extra = t }, + ), StateAccount: pseudo.NewAccessor[StateOrSlimAccount, SA]( func(a StateOrSlimAccount) *pseudo.Type { return a.extra().payload() }, func(a StateOrSlimAccount, t *pseudo.Type) { a.extra().t = t }, @@ -72,13 +76,13 @@ func RegisterExtras[ return fmt.Sprintf("%T", x) }(), // The [ExtraPayloads] that we returns is based on [HPtr,BPtr,SA], not - // [H,B,SA] so our constructors MUST match that. This guarantees that calls to - // the [HeaderHooks] and [BodyHooks] methods will never be performed on a nil pointer. - newHeader: pseudo.NewConstructor[H]().NewPointer, // i.e. non-nil HPtr - newBody: pseudo.NewConstructor[B]().NewPointer, // i.e. non-nil BPtr - newStateAccount: pseudo.NewConstructor[SA]().Zero, - cloneStateAccount: extra.cloneStateAccount, - hooks: extra, + // [H,B,SA] so our constructors MUST match that. This guarantees that + // calls to the [HeaderHooks] and [BlockBodyHooks] methods will never be + // performed on a nil pointer. + newHeader: pseudo.NewConstructor[H]().NewPointer, // i.e. non-nil HPtr + newBlockOrBody: pseudo.NewConstructor[B]().NewPointer, // i.e. non-nil BPtr + newStateAccount: pseudo.NewConstructor[SA]().Zero, + hooks: extra, }) return extra } @@ -96,15 +100,67 @@ func TestOnlyClearRegisteredExtras() { var registeredExtras register.AtMostOnce[*extraConstructors] type extraConstructors struct { - stateAccountType string - newHeader func() *pseudo.Type - newBody func() *pseudo.Type - newStateAccount func() *pseudo.Type - cloneStateAccount func(*StateAccountExtra) *StateAccountExtra - hooks interface { + stateAccountType string + newHeader func() *pseudo.Type + newBlockOrBody func() *pseudo.Type + newStateAccount func() *pseudo.Type + hooks interface { hooksFromHeader(*Header) HeaderHooks - hooksFromBody(*Body) BodyHooks + hooksFromBody(*Body) BlockBodyHooks + hooksFromBlock(*Block) BlockBodyHooks + cloneStateAccount(*StateAccountExtra) *StateAccountExtra + } +} + +func extraPayloadOrSetDefault(field **pseudo.Type, construct func(*extraConstructors) *pseudo.Type) *pseudo.Type { + r := registeredExtras + if !r.Registered() { + // See params.ChainConfig.extraPayload() for panic rationale. + panic(".extraPayload() called before RegisterExtras()") + } + if *field == nil { + *field = construct(r.Get()) + } + return *field +} + +func (h *Header) extraPayload() *pseudo.Type { + return extraPayloadOrSetDefault(&h.extra, func(c *extraConstructors) *pseudo.Type { + return c.newHeader() + }) +} + +func (b *Body) extraPayload() *pseudo.Type { + return extraPayloadOrSetDefault(&b.extra, func(c *extraConstructors) *pseudo.Type { + return c.newBlockOrBody() + }) +} + +func (b *Block) extraPayload() *pseudo.Type { + return extraPayloadOrSetDefault(&b.extra, func(c *extraConstructors) *pseudo.Type { + return c.newBlockOrBody() + }) +} + +func (h *Header) hooks() HeaderHooks { + if r := registeredExtras; r.Registered() { + return r.Get().hooks.hooksFromHeader(h) + } + return new(NOOPHeaderHooks) +} + +func (b *Body) hooks() BlockBodyHooks { + if r := registeredExtras; r.Registered() { + return r.Get().hooks.hooksFromBody(b) + } + return NOOPBlockBodyHooks{} +} + +func (b *Block) hooks() BlockBodyHooks { + if r := registeredExtras; r.Registered() { + return r.Get().hooks.hooksFromBlock(b) } + return NOOPBlockBodyHooks{} } func (e *StateAccountExtra) clone() *StateAccountExtra { @@ -112,19 +168,24 @@ func (e *StateAccountExtra) clone() *StateAccountExtra { case !r.Registered(), e == nil: return nil default: - return r.Get().cloneStateAccount(e) + return r.Get().hooks.cloneStateAccount(e) } } // ExtraPayloads provides strongly typed access to the extra payload carried by // [Header], [Body], [StateAccount], and [SlimAccount] structs. The only valid way to // construct an instance is by a call to [RegisterExtras]. -type ExtraPayloads[HPtr HeaderHooks, BPtr BodyHooks, SA any] struct { +type ExtraPayloads[HPtr HeaderHooks, BPtr BlockBodyHooks, SA any] struct { Header pseudo.Accessor[*Header, HPtr] + Block pseudo.Accessor[*Block, BPtr] Body pseudo.Accessor[*Body, BPtr] StateAccount pseudo.Accessor[StateOrSlimAccount, SA] // Also provides [SlimAccount] access. } +func (e ExtraPayloads[HPtr, BPtr, SA]) hooksFromHeader(h *Header) HeaderHooks { return e.Header.Get(h) } +func (e ExtraPayloads[HPtr, BPtr, SA]) hooksFromBody(b *Body) BlockBodyHooks { return e.Body.Get(b) } +func (e ExtraPayloads[HPtr, BPtr, SA]) hooksFromBlock(b *Block) BlockBodyHooks { return e.Block.Get(b) } + func (ExtraPayloads[HPtr, BPtr, SA]) cloneStateAccount(s *StateAccountExtra) *StateAccountExtra { v := pseudo.MustNewValue[SA](s.t) return &StateAccountExtra{ diff --git a/core/types/state_account.libevm_test.go b/core/types/state_account.libevm_test.go index 8ae8185d38e..9a91017674b 100644 --- a/core/types/state_account.libevm_test.go +++ b/core/types/state_account.libevm_test.go @@ -48,7 +48,7 @@ func TestStateAccountRLP(t *testing.T) { register: func() { RegisterExtras[ NOOPHeaderHooks, *NOOPHeaderHooks, - NOOPBodyHooks, *NOOPBodyHooks, + NOOPBlockBodyHooks, *NOOPBlockBodyHooks, bool, ]() }, @@ -82,7 +82,7 @@ func TestStateAccountRLP(t *testing.T) { register: func() { RegisterExtras[ NOOPHeaderHooks, *NOOPHeaderHooks, - NOOPBodyHooks, *NOOPBodyHooks, + NOOPBlockBodyHooks, *NOOPBlockBodyHooks, bool, ]() }, diff --git a/core/types/state_account_storage.libevm_test.go b/core/types/state_account_storage.libevm_test.go index 29311a8d1db..340621e591f 100644 --- a/core/types/state_account_storage.libevm_test.go +++ b/core/types/state_account_storage.libevm_test.go @@ -75,7 +75,7 @@ func TestStateAccountExtraViaTrieStorage(t *testing.T) { registerAndSetExtra: func(a *types.StateAccount) (*types.StateAccount, assertion) { e := types.RegisterExtras[ types.NOOPHeaderHooks, *types.NOOPHeaderHooks, - types.NOOPBodyHooks, *types.NOOPBodyHooks, + types.NOOPBlockBodyHooks, *types.NOOPBlockBodyHooks, bool, ]() e.StateAccount.Set(a, true) @@ -90,7 +90,7 @@ func TestStateAccountExtraViaTrieStorage(t *testing.T) { registerAndSetExtra: func(a *types.StateAccount) (*types.StateAccount, assertion) { e := types.RegisterExtras[ types.NOOPHeaderHooks, *types.NOOPHeaderHooks, - types.NOOPBodyHooks, *types.NOOPBodyHooks, + types.NOOPBlockBodyHooks, *types.NOOPBlockBodyHooks, bool, ]() e.StateAccount.Set(a, false) // the explicit part @@ -106,7 +106,7 @@ func TestStateAccountExtraViaTrieStorage(t *testing.T) { registerAndSetExtra: func(a *types.StateAccount) (*types.StateAccount, assertion) { e := types.RegisterExtras[ types.NOOPHeaderHooks, *types.NOOPHeaderHooks, - types.NOOPBodyHooks, *types.NOOPBodyHooks, + types.NOOPBlockBodyHooks, *types.NOOPBlockBodyHooks, bool, ]() // Note that `a` is reflected, unchanged (the implicit part). @@ -121,7 +121,7 @@ func TestStateAccountExtraViaTrieStorage(t *testing.T) { registerAndSetExtra: func(a *types.StateAccount) (*types.StateAccount, assertion) { e := types.RegisterExtras[ types.NOOPHeaderHooks, *types.NOOPHeaderHooks, - types.NOOPBodyHooks, *types.NOOPBodyHooks, + types.NOOPBlockBodyHooks, *types.NOOPBlockBodyHooks, arbitraryPayload, ]() p := arbitraryPayload{arbitraryData} From 843bf2f640085a34230ed12669f201b8bd6b72c0 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Wed, 12 Feb 2025 14:11:32 +0000 Subject: [PATCH 03/13] test(core/types): `Block` RLP overriding --- core/types/backwards_compat.libevm_test.go | 106 ++++++++++++++++++--- 1 file changed, 95 insertions(+), 11 deletions(-) diff --git a/core/types/backwards_compat.libevm_test.go b/core/types/backwards_compat.libevm_test.go index b7c921adc57..cf50f00cefe 100644 --- a/core/types/backwards_compat.libevm_test.go +++ b/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(hashLow byte) *Header { return &Header{ParentHash: common.Hash{hashLow}} } +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() { @@ -114,17 +117,90 @@ 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. 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 @@ -134,13 +210,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{ @@ -153,6 +229,14 @@ func (e *cChainBodyExtras) RLPFieldPointersForDecoding(b *Body) *rlp.Fields { } } +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] From 5103121203a120d8ae847eb4c14dd97af6258608 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Wed, 12 Feb 2025 16:44:02 +0000 Subject: [PATCH 04/13] fix(core/state): update `NOOPBodyHooks` to new type name --- core/state/state.libevm_test.go | 2 +- core/state/state_object.libevm_test.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/state/state.libevm_test.go b/core/state/state.libevm_test.go index bd909203102..0d816038f0c 100644 --- a/core/state/state.libevm_test.go +++ b/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 diff --git a/core/state/state_object.libevm_test.go b/core/state/state_object.libevm_test.go index 034764d419f..299241495ce 100644 --- a/core/state/state_object.libevm_test.go +++ b/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) }, From dc35c47bba5f77a8b0be7a2a40d5665ece98cc0b Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Thu, 13 Feb 2025 10:39:55 +0000 Subject: [PATCH 05/13] feat: payload copying between `Block` and `Body` --- core/types/backwards_compat.libevm_test.go | 2 + core/types/block.go | 2 + core/types/block.libevm.go | 14 ++++- core/types/block.libevm_test.go | 72 ++++++++++++++++++++++ core/types/rlp_payload.libevm.go | 45 +++++++++++--- 5 files changed, 126 insertions(+), 9 deletions(-) diff --git a/core/types/backwards_compat.libevm_test.go b/core/types/backwards_compat.libevm_test.go index cf50f00cefe..24535fc1404 100644 --- a/core/types/backwards_compat.libevm_test.go +++ b/core/types/backwards_compat.libevm_test.go @@ -200,6 +200,8 @@ type cChainBodyExtras struct { var _ BlockBodyHooks = (*cChainBodyExtras)(nil) +func (e *cChainBodyExtras) DeepCopy() *cChainBodyExtras { return e } + 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 diff --git a/core/types/block.go b/core/types/block.go index cb8ab100614..44cf1458fc2 100644 --- a/core/types/block.go +++ b/core/types/block.go @@ -464,6 +464,7 @@ func (b *Block) WithSeal(header *Header) *Block { transactions: b.transactions, uncles: b.uncles, withdrawals: b.withdrawals, + extra: b.cloneExtra(), } } @@ -489,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)) diff --git a/core/types/block.libevm.go b/core/types/block.libevm.go index 8faf5b51f41..799f6ba2b27 100644 --- a/core/types/block.libevm.go +++ b/core/types/block.libevm.go @@ -86,11 +86,17 @@ func (*NOOPHeaderHooks) DecodeRLP(h *Header, s *rlp.Stream) error { func (*NOOPHeaderHooks) PostCopy(dst *Header) {} func (b *Body) cloneExtra() *pseudo.Type { - return b.extra // TODO(arr4n) implement this // DO NOT MERGE + if r := registeredExtras; r.Registered() { + return r.Get().hooks.cloneBodyPayload(b) + } + return nil } func (b *Block) cloneExtra() *pseudo.Type { - return b.extra // TODO(arr4n) implement this // DO NOT MERGE + if r := registeredExtras; r.Registered() { + return r.Get().hooks.cloneBlockPayload(b) + } + return nil } var _ = []interface { @@ -138,6 +144,10 @@ type BlockBodyHooks interface { // to no type having been registered. type NOOPBlockBodyHooks struct{} +var _ BlockBodyPayload[*NOOPBlockBodyHooks] = NOOPBlockBodyHooks{} + +func (NOOPBlockBodyHooks) DeepCopy() *NOOPBlockBodyHooks { return &NOOPBlockBodyHooks{} } + // The RLP-related methods of [NOOPBlockBodyHooks] make assumptions about the // struct fields and their order, which we lock in here as a change detector. If // these break then they MUST be updated and the RLP methods reviewed + new diff --git a/core/types/block.libevm_test.go b/core/types/block.libevm_test.go index fa4302e6d6a..14db4225dea 100644 --- a/core/types/block.libevm_test.go +++ b/core/types/block.libevm_test.go @@ -21,6 +21,8 @@ import ( "errors" "fmt" "io" + "reflect" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -200,3 +202,73 @@ func TestHeaderHooks(t *testing.T) { } }) } + +type blockPayload struct { + NOOPBlockBodyHooks + x int + copied bool +} + +func (p *blockPayload) DeepCopy() *blockPayload { + p.copied = true + return &blockPayload{x: p.x} +} + +func TestBlockWithX(t *testing.T) { + TestOnlyClearRegisteredExtras() + t.Cleanup(TestOnlyClearRegisteredExtras) + + extras := RegisterExtras[ + NOOPHeaderHooks, *NOOPHeaderHooks, + blockPayload, *blockPayload, + struct{}, + ]() + + typ := reflect.TypeOf(&Block{}) + for i := 0; i < typ.NumMethod(); i++ { + method := typ.Method(i).Name + if method == "Withdrawals" || !strings.HasPrefix(method, "With") { + continue + } + + block := NewBlockWithHeader(&Header{}) + const initialPayload = int(42) + payload := &blockPayload{ + x: initialPayload, + } + extras.Block.Set(block, payload) + + t.Run(method, func(t *testing.T) { + var newBlock *Block + + switch method { + case "WithBody": + var body Body + extras.Body.Set(&body, payload) + newBlock = block.WithBody(body) + case "WithSeal": + newBlock = block.WithSeal(&Header{}) + case "WithWithdrawals": + newBlock = block.WithWithdrawals(nil) + default: + t.Fatal("method call not implemented") + } + + payload.x++ + // This specifically uses `require` instead of `assert` because a + // failure here invalidates the next test, which demonstrates a deep + // copy. + require.Equalf(t, initialPayload+1, extras.Block.Get(block).x, "%T payload %T after modification via pointer") + + switch got := extras.Block.Get(newBlock); got.x { + case initialPayload: // expected + case 0: + t.Errorf("%T payload %T got zero value; the payload was probably not copied, resulting in a default being created", newBlock, got) + case initialPayload + 1: + t.Errorf("%T payload %T got same value as modified original; the payload was probably shallow copied", newBlock, got) + default: + t.Errorf("%T payload %T got %d, want %d; this is unexpected even as an error so you're on your own here", newBlock, got, got.x, initialPayload) + } + }) + } +} diff --git a/core/types/rlp_payload.libevm.go b/core/types/rlp_payload.libevm.go index ed47d62393b..9a87b2304d0 100644 --- a/core/types/rlp_payload.libevm.go +++ b/core/types/rlp_payload.libevm.go @@ -27,9 +27,9 @@ import ( ) // RegisterExtras registers the type `HPtr` to be carried as an extra payload in -// [Header] structs and the type `SA` in [StateAccount] and [SlimAccount] -// structs. It is expected to be called in an `init()` function and MUST NOT be -// called more than once. +// [Header] structs, the type `BPtr` in [Block] and [Body] structs, and the type +// `SA` in [StateAccount] and [SlimAccount] structs. It is expected to be called +// in an `init()` function and MUST NOT be called more than once. // // The `SA` payload will be treated as an extra struct field for the purposes of // RLP encoding and decoding. RLP handling is plumbed through to the `SA` via @@ -39,15 +39,15 @@ import ( // The payloads can be accessed via the [pseudo.Accessor] methods of the // [ExtraPayloads] returned by RegisterExtras. The default `SA` value accessed // in this manner will be a zero-value `SA` while the default value from a -// [Header] is a non-nil `HPtr`. The latter guarantee ensures that hooks won't -// be called on nil-pointer receivers. +// [Header] or [Block] / [Body] is a non-nil `HPtr` or `BPtr` respectively. The +// latter guarantee ensures that hooks won't be called on nil-pointer receivers. func RegisterExtras[ H any, HPtr interface { HeaderHooks *H }, B any, BPtr interface { - BlockBodyHooks + BlockBodyPayload[BPtr] *B }, SA any, @@ -87,6 +87,14 @@ func RegisterExtras[ return extra } +// A BlockBodyPayload is an implementation of [BlockBodyHooks] that is also able +// to clone itself. Both [Block.Body] and [Block.WithBody] require this +// functionality to clone the payload between the types. +type BlockBodyPayload[BPtr any] interface { + BlockBodyHooks + DeepCopy() BPtr +} + // TestOnlyClearRegisteredExtras clears the [Extras] previously passed to // [RegisterExtras]. It panics if called from a non-testing call stack. // @@ -108,6 +116,8 @@ type extraConstructors struct { hooksFromHeader(*Header) HeaderHooks hooksFromBody(*Body) BlockBodyHooks hooksFromBlock(*Block) BlockBodyHooks + cloneBlockPayload(*Block) *pseudo.Type + cloneBodyPayload(*Body) *pseudo.Type cloneStateAccount(*StateAccountExtra) *StateAccountExtra } } @@ -175,7 +185,7 @@ func (e *StateAccountExtra) clone() *StateAccountExtra { // ExtraPayloads provides strongly typed access to the extra payload carried by // [Header], [Body], [StateAccount], and [SlimAccount] structs. The only valid way to // construct an instance is by a call to [RegisterExtras]. -type ExtraPayloads[HPtr HeaderHooks, BPtr BlockBodyHooks, SA any] struct { +type ExtraPayloads[HPtr HeaderHooks, BPtr BlockBodyPayload[BPtr], SA any] struct { Header pseudo.Accessor[*Header, HPtr] Block pseudo.Accessor[*Block, BPtr] Body pseudo.Accessor[*Body, BPtr] @@ -193,6 +203,27 @@ func (ExtraPayloads[HPtr, BPtr, SA]) cloneStateAccount(s *StateAccountExtra) *St } } +func (ExtraPayloads[HPtr, BPtr, SA]) cloneBodyPayload(b *Body) *pseudo.Type { + return cloneBlockBodyPayload[*Body, BPtr](b) +} + +func (ExtraPayloads[HPtr, BPtr, SA]) cloneBlockPayload(b *Block) *pseudo.Type { + return cloneBlockBodyPayload[*Block, BPtr](b) +} + +// cloneBlockBodyPayload MUST NOT be used directly. Instead call +// [ExtraPayloads.cloneBodyPayload] or its Block equivalent. +func cloneBlockBodyPayload[ + T interface { + extraPayload() *pseudo.Type + *Body | *Block + }, + BPtr BlockBodyPayload[BPtr], +](b T) *pseudo.Type { + v := pseudo.MustNewValue[BPtr](b.extraPayload()) + return pseudo.From(v.Get().DeepCopy()).Type +} + // StateOrSlimAccount is implemented by both [StateAccount] and [SlimAccount], // allowing for their [StateAccountExtra] payloads to be accessed in a type-safe // manner by [ExtraPayloads] instances. From 9e3a46460e44f8f862cd19e3dbad383471eb8e79 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Thu, 13 Feb 2025 10:43:41 +0000 Subject: [PATCH 06/13] refactor: rename `BlockBodyPayload.DeepCopy()` to `Copy()` --- core/types/backwards_compat.libevm_test.go | 2 +- core/types/block.libevm.go | 2 +- core/types/block.libevm_test.go | 2 +- core/types/rlp_payload.libevm.go | 6 +++--- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/core/types/backwards_compat.libevm_test.go b/core/types/backwards_compat.libevm_test.go index 24535fc1404..4519bf1b427 100644 --- a/core/types/backwards_compat.libevm_test.go +++ b/core/types/backwards_compat.libevm_test.go @@ -200,7 +200,7 @@ type cChainBodyExtras struct { var _ BlockBodyHooks = (*cChainBodyExtras)(nil) -func (e *cChainBodyExtras) DeepCopy() *cChainBodyExtras { return e } +func (e *cChainBodyExtras) Copy() *cChainBodyExtras { return e } func (e *cChainBodyExtras) BodyRLPFieldsForEncoding(b *Body) *rlp.Fields { // The Avalanche C-Chain uses all of the geth required fields (but none of diff --git a/core/types/block.libevm.go b/core/types/block.libevm.go index 799f6ba2b27..b5a15e8cc25 100644 --- a/core/types/block.libevm.go +++ b/core/types/block.libevm.go @@ -146,7 +146,7 @@ type NOOPBlockBodyHooks struct{} var _ BlockBodyPayload[*NOOPBlockBodyHooks] = NOOPBlockBodyHooks{} -func (NOOPBlockBodyHooks) DeepCopy() *NOOPBlockBodyHooks { return &NOOPBlockBodyHooks{} } +func (NOOPBlockBodyHooks) Copy() *NOOPBlockBodyHooks { return &NOOPBlockBodyHooks{} } // The RLP-related methods of [NOOPBlockBodyHooks] make assumptions about the // struct fields and their order, which we lock in here as a change detector. If diff --git a/core/types/block.libevm_test.go b/core/types/block.libevm_test.go index 14db4225dea..18127bffc3e 100644 --- a/core/types/block.libevm_test.go +++ b/core/types/block.libevm_test.go @@ -209,7 +209,7 @@ type blockPayload struct { copied bool } -func (p *blockPayload) DeepCopy() *blockPayload { +func (p *blockPayload) Copy() *blockPayload { p.copied = true return &blockPayload{x: p.x} } diff --git a/core/types/rlp_payload.libevm.go b/core/types/rlp_payload.libevm.go index 9a87b2304d0..4d6192725ea 100644 --- a/core/types/rlp_payload.libevm.go +++ b/core/types/rlp_payload.libevm.go @@ -89,10 +89,10 @@ func RegisterExtras[ // A BlockBodyPayload is an implementation of [BlockBodyHooks] that is also able // to clone itself. Both [Block.Body] and [Block.WithBody] require this -// functionality to clone the payload between the types. +// functionality to copy the payload between the types. type BlockBodyPayload[BPtr any] interface { BlockBodyHooks - DeepCopy() BPtr + Copy() BPtr } // TestOnlyClearRegisteredExtras clears the [Extras] previously passed to @@ -221,7 +221,7 @@ func cloneBlockBodyPayload[ BPtr BlockBodyPayload[BPtr], ](b T) *pseudo.Type { v := pseudo.MustNewValue[BPtr](b.extraPayload()) - return pseudo.From(v.Get().DeepCopy()).Type + return pseudo.From(v.Get().Copy()).Type } // StateOrSlimAccount is implemented by both [StateAccount] and [SlimAccount], From 7accc33751fced90a08c89a591f0e11e72d0d6e6 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Thu, 13 Feb 2025 10:44:46 +0000 Subject: [PATCH 07/13] fix: forgotten formatting arguments in test --- core/types/block.libevm_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/types/block.libevm_test.go b/core/types/block.libevm_test.go index 18127bffc3e..571c9875964 100644 --- a/core/types/block.libevm_test.go +++ b/core/types/block.libevm_test.go @@ -258,7 +258,7 @@ func TestBlockWithX(t *testing.T) { // This specifically uses `require` instead of `assert` because a // failure here invalidates the next test, which demonstrates a deep // copy. - require.Equalf(t, initialPayload+1, extras.Block.Get(block).x, "%T payload %T after modification via pointer") + require.Equalf(t, initialPayload+1, extras.Block.Get(block).x, "%T payload %T after modification via pointer", block, payload) switch got := extras.Block.Get(newBlock); got.x { case initialPayload: // expected From 04ead3c74bfb8d4032794fa67455296d8369f984 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Thu, 13 Feb 2025 12:24:55 +0000 Subject: [PATCH 08/13] refactor: simplify `Block` and `Body` payload cloning --- core/types/block.libevm.go | 14 ----------- core/types/rlp_payload.libevm.go | 42 ++++++++++++++++++++++---------- 2 files changed, 29 insertions(+), 27 deletions(-) diff --git a/core/types/block.libevm.go b/core/types/block.libevm.go index b5a15e8cc25..a512118dd41 100644 --- a/core/types/block.libevm.go +++ b/core/types/block.libevm.go @@ -85,20 +85,6 @@ func (*NOOPHeaderHooks) DecodeRLP(h *Header, s *rlp.Stream) error { } func (*NOOPHeaderHooks) PostCopy(dst *Header) {} -func (b *Body) cloneExtra() *pseudo.Type { - if r := registeredExtras; r.Registered() { - return r.Get().hooks.cloneBodyPayload(b) - } - return nil -} - -func (b *Block) cloneExtra() *pseudo.Type { - if r := registeredExtras; r.Registered() { - return r.Get().hooks.cloneBlockPayload(b) - } - return nil -} - var _ = []interface { rlp.Encoder rlp.Decoder diff --git a/core/types/rlp_payload.libevm.go b/core/types/rlp_payload.libevm.go index 4d6192725ea..d4e4db9ef7f 100644 --- a/core/types/rlp_payload.libevm.go +++ b/core/types/rlp_payload.libevm.go @@ -203,27 +203,43 @@ func (ExtraPayloads[HPtr, BPtr, SA]) cloneStateAccount(s *StateAccountExtra) *St } } -func (ExtraPayloads[HPtr, BPtr, SA]) cloneBodyPayload(b *Body) *pseudo.Type { - return cloneBlockBodyPayload[*Body, BPtr](b) +func (e ExtraPayloads[HPtr, BPtr, SA]) cloneBodyPayload(b *Body) *pseudo.Type { + return e.cloneBlockOrBodyPayload(b) } -func (ExtraPayloads[HPtr, BPtr, SA]) cloneBlockPayload(b *Block) *pseudo.Type { - return cloneBlockBodyPayload[*Block, BPtr](b) +func (e ExtraPayloads[HPtr, BPtr, SA]) cloneBlockPayload(b *Block) *pseudo.Type { + return e.cloneBlockOrBodyPayload(b) } -// cloneBlockBodyPayload MUST NOT be used directly. Instead call -// [ExtraPayloads.cloneBodyPayload] or its Block equivalent. -func cloneBlockBodyPayload[ - T interface { - extraPayload() *pseudo.Type - *Body | *Block - }, - BPtr BlockBodyPayload[BPtr], -](b T) *pseudo.Type { +func (ExtraPayloads[HPtr, BPtr, SA]) cloneBlockOrBodyPayload(b blockOrBody) *pseudo.Type { v := pseudo.MustNewValue[BPtr](b.extraPayload()) return pseudo.From(v.Get().Copy()).Type } +func (b *Body) cloneExtra() *pseudo.Type { + if r := registeredExtras; r.Registered() { + return r.Get().hooks.cloneBodyPayload(b) + } + return nil +} + +func (b *Block) cloneExtra() *pseudo.Type { + if r := registeredExtras; r.Registered() { + return r.Get().hooks.cloneBlockPayload(b) + } + return nil +} + +// blockOrBody is an interface for use as a method argument as they can't +// introduce new generic type parameters. +type blockOrBody interface { + isBlockOrBody() // noop purely for tagging + extraPayload() *pseudo.Type +} + +func (*Block) isBlockOrBody() {} +func (*Body) isBlockOrBody() {} + // StateOrSlimAccount is implemented by both [StateAccount] and [SlimAccount], // allowing for their [StateAccountExtra] payloads to be accessed in a type-safe // manner by [ExtraPayloads] instances. From 1e49ad6f1f836f744c468fc5e776ad5834a942df Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Thu, 13 Feb 2025 12:32:58 +0000 Subject: [PATCH 09/13] refactor: readability cleanup before review --- core/types/block.libevm_test.go | 4 +--- core/types/rlp_payload.libevm.go | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/core/types/block.libevm_test.go b/core/types/block.libevm_test.go index 571c9875964..7716d5cb842 100644 --- a/core/types/block.libevm_test.go +++ b/core/types/block.libevm_test.go @@ -205,12 +205,10 @@ func TestHeaderHooks(t *testing.T) { type blockPayload struct { NOOPBlockBodyHooks - x int - copied bool + x int } func (p *blockPayload) Copy() *blockPayload { - p.copied = true return &blockPayload{x: p.x} } diff --git a/core/types/rlp_payload.libevm.go b/core/types/rlp_payload.libevm.go index d4e4db9ef7f..e758d03e1b9 100644 --- a/core/types/rlp_payload.libevm.go +++ b/core/types/rlp_payload.libevm.go @@ -203,6 +203,16 @@ func (ExtraPayloads[HPtr, BPtr, SA]) cloneStateAccount(s *StateAccountExtra) *St } } +// blockOrBody is an interface for use as a method argument as they can't +// introduce new generic type parameters. +type blockOrBody interface { + isBlockOrBody() // noop purely for tagging + extraPayload() *pseudo.Type +} + +func (*Block) isBlockOrBody() {} +func (*Body) isBlockOrBody() {} + func (e ExtraPayloads[HPtr, BPtr, SA]) cloneBodyPayload(b *Body) *pseudo.Type { return e.cloneBlockOrBodyPayload(b) } @@ -230,16 +240,6 @@ func (b *Block) cloneExtra() *pseudo.Type { return nil } -// blockOrBody is an interface for use as a method argument as they can't -// introduce new generic type parameters. -type blockOrBody interface { - isBlockOrBody() // noop purely for tagging - extraPayload() *pseudo.Type -} - -func (*Block) isBlockOrBody() {} -func (*Body) isBlockOrBody() {} - // StateOrSlimAccount is implemented by both [StateAccount] and [SlimAccount], // allowing for their [StateAccountExtra] payloads to be accessed in a type-safe // manner by [ExtraPayloads] instances. From e5b3c5dba29538400f6b848b17a961e27f46b5df Mon Sep 17 00:00:00 2001 From: Arran Schlosberg <519948+ARR4N@users.noreply.github.com> Date: Thu, 13 Feb 2025 13:25:46 +0000 Subject: [PATCH 10/13] Apply suggestions from code review Co-authored-by: Quentin McGaw Signed-off-by: Arran Schlosberg <519948+ARR4N@users.noreply.github.com> --- core/types/block.go | 2 +- core/types/block.libevm_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/types/block.go b/core/types/block.go index 44cf1458fc2..95d0df68f72 100644 --- a/core/types/block.go +++ b/core/types/block.go @@ -212,7 +212,7 @@ type Block struct { ReceivedAt time.Time ReceivedFrom interface{} - extra *pseudo.Type // See RegisterExtras() + extra *pseudo.Type // See [RegisterExtras] } // "external" block encoding. used for eth protocol, etc. diff --git a/core/types/block.libevm_test.go b/core/types/block.libevm_test.go index 7716d5cb842..8e9a250e759 100644 --- a/core/types/block.libevm_test.go +++ b/core/types/block.libevm_test.go @@ -249,7 +249,7 @@ func TestBlockWithX(t *testing.T) { case "WithWithdrawals": newBlock = block.WithWithdrawals(nil) default: - t.Fatal("method call not implemented") + t.Fatalf("method call not implemented: %s", method) } payload.x++ From 29901b57ef4f081e921e66a77c76ef8861e91d63 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Thu, 13 Feb 2025 13:26:37 +0000 Subject: [PATCH 11/13] refactor: responses to PR review --- core/types/backwards_compat.libevm_test.go | 2 +- core/types/rlp_payload.libevm.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/types/backwards_compat.libevm_test.go b/core/types/backwards_compat.libevm_test.go index 4519bf1b427..9dde279ee34 100644 --- a/core/types/backwards_compat.libevm_test.go +++ b/core/types/backwards_compat.libevm_test.go @@ -31,7 +31,7 @@ import ( ) func newTx(nonce uint64) *Transaction { return NewTx(&LegacyTx{Nonce: nonce}) } -func newHdr(hashLow byte) *Header { return &Header{ParentHash: common.Hash{hashLow}} } +func newHdr(parentHashHigh byte) *Header { return &Header{ParentHash: common.Hash{parentHashHigh}} } func newWithdraw(idx uint64) *Withdrawal { return &Withdrawal{Index: idx} } func blockBodyRLPTestInputs() []*Body { diff --git a/core/types/rlp_payload.libevm.go b/core/types/rlp_payload.libevm.go index e758d03e1b9..98aebe1cc4b 100644 --- a/core/types/rlp_payload.libevm.go +++ b/core/types/rlp_payload.libevm.go @@ -206,7 +206,7 @@ func (ExtraPayloads[HPtr, BPtr, SA]) cloneStateAccount(s *StateAccountExtra) *St // blockOrBody is an interface for use as a method argument as they can't // introduce new generic type parameters. type blockOrBody interface { - isBlockOrBody() // noop purely for tagging + isBlockOrBody() // noop to restrict type as [Header.extraPayload] otherwise matches extraPayload() *pseudo.Type } From 927ac8b33f734aa70e5d8534d0def842a42aa0b0 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Thu, 13 Feb 2025 13:38:54 +0000 Subject: [PATCH 12/13] doc: `cChainBodyExtras` intent and limitations --- core/types/backwards_compat.libevm_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/types/backwards_compat.libevm_test.go b/core/types/backwards_compat.libevm_test.go index 9dde279ee34..d1e29cb91c9 100644 --- a/core/types/backwards_compat.libevm_test.go +++ b/core/types/backwards_compat.libevm_test.go @@ -193,6 +193,10 @@ func TestBlockRLPBackwardsCompatibility(t *testing.T) { // cChainBodyExtras carries the same additional fields as the Avalanche C-Chain // (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 From 59d6682e7c7e35e2961fb6605ed15a7f2e841cad Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Thu, 13 Feb 2025 14:48:47 +0000 Subject: [PATCH 13/13] refactor: responses to PR review --- core/types/backwards_compat.libevm_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/core/types/backwards_compat.libevm_test.go b/core/types/backwards_compat.libevm_test.go index d1e29cb91c9..dca9245fcde 100644 --- a/core/types/backwards_compat.libevm_test.go +++ b/core/types/backwards_compat.libevm_test.go @@ -204,8 +204,6 @@ type cChainBodyExtras struct { var _ BlockBodyHooks = (*cChainBodyExtras)(nil) -func (e *cChainBodyExtras) Copy() *cChainBodyExtras { return e } - 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 @@ -235,6 +233,12 @@ func (e *cChainBodyExtras) BodyRLPFieldPointersForDecoding(b *Body) *rlp.Fields } } +// See [cChainBodyExtras] intent. + +func (e *cChainBodyExtras) Copy() *cChainBodyExtras { + panic("unimplemented") +} + func (e *cChainBodyExtras) BlockRLPFieldsForEncoding(b *BlockRLPProxy) *rlp.Fields { panic("unimplemented") }