From 606881b9a787e29223fd649a2b365f08d10818dd Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Wed, 18 Dec 2024 14:48:28 +0000 Subject: [PATCH 1/5] refactor: `internalise` `Header.MarshalJSON` and `UnmarshalJSON` --- core/types/block.go | 1 + core/types/block.libevm.go | 13 +++++++++++++ core/types/gen_header_json.go | 4 ++-- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/core/types/block.go b/core/types/block.go index 9cf53db28a4..692cf01c076 100644 --- a/core/types/block.go +++ b/core/types/block.go @@ -60,6 +60,7 @@ func (n *BlockNonce) UnmarshalText(input []byte) error { } //go:generate go run github.com/fjl/gencodec -type Header -field-override headerMarshaling -out gen_header_json.go +//go:generate go run ../../libevm/cmd/internalise -file gen_header_json.go Header.MarshalJSON Header.UnmarshalJSON //go:generate go run ../../rlp/rlpgen -type Header -out gen_header_rlp.go //go:generate go run ../../libevm/cmd/internalise -file gen_header_rlp.go Header.EncodeRLP diff --git a/core/types/block.libevm.go b/core/types/block.libevm.go index 1a4dca03818..07fc7e2e613 100644 --- a/core/types/block.libevm.go +++ b/core/types/block.libevm.go @@ -17,6 +17,7 @@ package types import ( + "encoding/json" "fmt" "io" @@ -34,8 +35,20 @@ type HeaderHooks interface { var _ interface { rlp.Encoder rlp.Decoder + json.Marshaler + json.Unmarshaler } = (*Header)(nil) +// MarshalJSON implements the [json.Marshaler] interface. +func (h *Header) MarshalJSON() ([]byte, error) { + return h.marshalJSON() +} + +// UnmarshalJSON implements the [json.Unmarshaler] interface. +func (h *Header) UnmarshalJSON(b []byte) error { + return h.unmarshalJSON(b) +} + // EncodeRLP implements the [rlp.Encoder] interface. func (h *Header) EncodeRLP(w io.Writer) error { if r := registeredExtras; r.Registered() { diff --git a/core/types/gen_header_json.go b/core/types/gen_header_json.go index f69d033ad33..66d22535631 100644 --- a/core/types/gen_header_json.go +++ b/core/types/gen_header_json.go @@ -14,7 +14,7 @@ import ( var _ = (*headerMarshaling)(nil) // MarshalJSON marshals as JSON. -func (h Header) MarshalJSON() ([]byte, error) { +func (h Header) marshalJSON() ([]byte, error) { type Header struct { ParentHash common.Hash `json:"parentHash" gencodec:"required"` UncleHash common.Hash `json:"sha3Uncles" gencodec:"required"` @@ -64,7 +64,7 @@ func (h Header) MarshalJSON() ([]byte, error) { } // UnmarshalJSON unmarshals from JSON. -func (h *Header) UnmarshalJSON(input []byte) error { +func (h *Header) unmarshalJSON(input []byte) error { type Header struct { ParentHash *common.Hash `json:"parentHash" gencodec:"required"` UncleHash *common.Hash `json:"sha3Uncles" gencodec:"required"` From df20255997600520b54e84c72add22a695822ba3 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Wed, 18 Dec 2024 15:35:44 +0000 Subject: [PATCH 2/5] feat: `HeaderHooks` JSON round-tripping --- core/types/block.libevm.go | 51 +++++++++++++++++++-------------- core/types/block.libevm_test.go | 8 ++++++ 2 files changed, 37 insertions(+), 22 deletions(-) diff --git a/core/types/block.libevm.go b/core/types/block.libevm.go index 07fc7e2e613..ca1e6d7e57d 100644 --- a/core/types/block.libevm.go +++ b/core/types/block.libevm.go @@ -28,10 +28,25 @@ import ( // HeaderHooks are required for all types registered with [RegisterExtras] for // [Header] payloads. type HeaderHooks interface { + MarshalJSON(*Header) ([]byte, error) //nolint:govet // Type-specific override hook + UnmarshalJSON(*Header, []byte) error //nolint:govet EncodeRLP(*Header, io.Writer) error DecodeRLP(*Header, *rlp.Stream) error } +// 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, SA]) hooksFromHeader(h *Header) HeaderHooks { + return e.Header.Get(h) +} + var _ interface { rlp.Encoder rlp.Decoder @@ -41,39 +56,22 @@ var _ interface { // MarshalJSON implements the [json.Marshaler] interface. func (h *Header) MarshalJSON() ([]byte, error) { - return h.marshalJSON() + return h.hooks().MarshalJSON(h) } // UnmarshalJSON implements the [json.Unmarshaler] interface. func (h *Header) UnmarshalJSON(b []byte) error { - return h.unmarshalJSON(b) + return h.hooks().UnmarshalJSON(h, b) } // EncodeRLP implements the [rlp.Encoder] interface. func (h *Header) EncodeRLP(w io.Writer) error { - if r := registeredExtras; r.Registered() { - return r.Get().hooks.hooksFromHeader(h).EncodeRLP(h, w) - } - return h.encodeRLP(w) -} - -// decodeHeaderRLPDirectly bypasses the [Header.DecodeRLP] method to avoid -// infinite recursion. -func decodeHeaderRLPDirectly(h *Header, s *rlp.Stream) error { - type withoutMethods Header - return s.Decode((*withoutMethods)(h)) + return h.hooks().EncodeRLP(h, w) } // DecodeRLP implements the [rlp.Decoder] interface. func (h *Header) DecodeRLP(s *rlp.Stream) error { - if r := registeredExtras; r.Registered() { - return r.Get().hooks.hooksFromHeader(h).DecodeRLP(h, s) - } - return decodeHeaderRLPDirectly(h, s) -} - -func (e ExtraPayloads[HPtr, SA]) hooksFromHeader(h *Header) HeaderHooks { - return e.Header.Get(h) + return h.hooks().DecodeRLP(h, s) } func (h *Header) extraPayload() *pseudo.Type { @@ -94,10 +92,19 @@ type NOOPHeaderHooks struct{} var _ HeaderHooks = (*NOOPHeaderHooks)(nil) +func (*NOOPHeaderHooks) MarshalJSON(h *Header) ([]byte, error) { //nolint:govet + return h.marshalJSON() +} + +func (*NOOPHeaderHooks) UnmarshalJSON(h *Header, b []byte) error { //nolint:govet + return h.unmarshalJSON(b) +} + func (*NOOPHeaderHooks) EncodeRLP(h *Header, w io.Writer) error { return h.encodeRLP(w) } func (*NOOPHeaderHooks) DecodeRLP(h *Header, s *rlp.Stream) error { - return decodeHeaderRLPDirectly(h, s) + type withoutMethods Header + return s.Decode((*withoutMethods)(h)) } diff --git a/core/types/block.libevm_test.go b/core/types/block.libevm_test.go index 54740136239..a612ff351ce 100644 --- a/core/types/block.libevm_test.go +++ b/core/types/block.libevm_test.go @@ -42,6 +42,14 @@ func fakeHeaderRLP(h *Header, suffix []byte) []byte { return append(crypto.Keccak256(h.ParentHash[:]), suffix...) } +func (hh *stubHeaderHooks) MarshalJSON(h *Header) ([]byte, error) { //nolint:govet + return nil, errors.New("TODO") +} + +func (hh *stubHeaderHooks) UnmarshalJSON(h *Header, b []byte) error { //nolint:govet + return errors.New("TODO") +} + func (hh *stubHeaderHooks) EncodeRLP(h *Header, w io.Writer) error { if _, err := w.Write(fakeHeaderRLP(h, hh.rlpSuffix)); err != nil { return err From 02bf2581a194b1cfe6e78810d708a3370f7c6633 Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Wed, 18 Dec 2024 16:00:57 +0000 Subject: [PATCH 3/5] test: `HeaderHooks` JSON round-tripping --- core/types/block.libevm_test.go | 80 +++++++++++++++++++++++++-------- 1 file changed, 61 insertions(+), 19 deletions(-) diff --git a/core/types/block.libevm_test.go b/core/types/block.libevm_test.go index a612ff351ce..c7f8a7dfd02 100644 --- a/core/types/block.libevm_test.go +++ b/core/types/block.libevm_test.go @@ -17,7 +17,9 @@ package types_test import ( + "encoding/json" "errors" + "fmt" "io" "testing" @@ -31,11 +33,15 @@ import ( ) type stubHeaderHooks struct { - rlpSuffix []byte - gotRawRLPToDecode []byte - setHeaderToOnDecode Header + suffix []byte + gotRawJSONToUnmarshal, gotRawRLPToDecode []byte + setHeaderToOnUnmarshalOrDecode Header - errEncode, errDecode error + errMarshal, errUnmarshal, errEncode, errDecode error +} + +func fakeHeaderJSON(h *Header, suffix []byte) []byte { + return []byte(fmt.Sprintf(`"%#x:%#x"`, h.ParentHash, suffix)) } func fakeHeaderRLP(h *Header, suffix []byte) []byte { @@ -43,15 +49,17 @@ func fakeHeaderRLP(h *Header, suffix []byte) []byte { } func (hh *stubHeaderHooks) MarshalJSON(h *Header) ([]byte, error) { //nolint:govet - return nil, errors.New("TODO") + return fakeHeaderJSON(h, hh.suffix), hh.errMarshal } func (hh *stubHeaderHooks) UnmarshalJSON(h *Header, b []byte) error { //nolint:govet - return errors.New("TODO") + hh.gotRawJSONToUnmarshal = b + *h = hh.setHeaderToOnUnmarshalOrDecode + return hh.errUnmarshal } func (hh *stubHeaderHooks) EncodeRLP(h *Header, w io.Writer) error { - if _, err := w.Write(fakeHeaderRLP(h, hh.rlpSuffix)); err != nil { + if _, err := w.Write(fakeHeaderRLP(h, hh.suffix)); err != nil { return err } return hh.errEncode @@ -63,7 +71,7 @@ func (hh *stubHeaderHooks) DecodeRLP(h *Header, s *rlp.Stream) error { return err } hh.gotRawRLPToDecode = r - *h = hh.setHeaderToOnDecode + *h = hh.setHeaderToOnUnmarshalOrDecode return hh.errDecode } @@ -74,14 +82,36 @@ func TestHeaderHooks(t *testing.T) { extras := RegisterExtras[stubHeaderHooks, *stubHeaderHooks, struct{}]() rng := ethtest.NewPseudoRand(13579) - t.Run("EncodeRLP", func(t *testing.T) { - suffix := rng.Bytes(8) + suffix := rng.Bytes(8) + hdr := &Header{ + ParentHash: rng.Hash(), + } + extras.Header.Get(hdr).suffix = append([]byte{}, suffix...) - hdr := &Header{ - ParentHash: rng.Hash(), + t.Run("MarshalJSON", func(t *testing.T) { + got, err := json.Marshal(hdr) + require.NoError(t, err, "json.Marshal(%T)", hdr) + assert.Equal(t, fakeHeaderJSON(hdr, suffix), got) + }) + + t.Run("UnmarshalJSON", func(t *testing.T) { + hdr := new(Header) + stub := &stubHeaderHooks{ + setHeaderToOnUnmarshalOrDecode: Header{ + Extra: []byte("can you solve this puzzle? 0xbda01b6cf56c303bd3f581599c0d5c0b"), + }, } - extras.Header.Get(hdr).rlpSuffix = append([]byte{}, suffix...) + extras.Header.Set(hdr, stub) + + input := fmt.Sprintf("%q", "hello, JSON world") + err := json.Unmarshal([]byte(input), hdr) + require.NoErrorf(t, err, "json.Unmarshal()") + + assert.Equal(t, input, string(stub.gotRawJSONToUnmarshal), "raw JSON received by hook") + assert.Equal(t, &stub.setHeaderToOnUnmarshalOrDecode, hdr, "%T after JSON unmarshalling with hook", hdr) + }) + t.Run("EncodeRLP", func(t *testing.T) { got, err := rlp.EncodeToBytes(hdr) require.NoError(t, err, "rlp.EncodeToBytes(%T)", hdr) assert.Equal(t, fakeHeaderRLP(hdr, suffix), got) @@ -93,7 +123,7 @@ func TestHeaderHooks(t *testing.T) { hdr := new(Header) stub := &stubHeaderHooks{ - setHeaderToOnDecode: Header{ + setHeaderToOnUnmarshalOrDecode: Header{ Extra: []byte("arr4n was here"), }, } @@ -102,19 +132,31 @@ func TestHeaderHooks(t *testing.T) { require.NoErrorf(t, err, "rlp.DecodeBytes(%#x)", input) assert.Equal(t, input, stub.gotRawRLPToDecode, "raw RLP received by hooks") - assert.Equalf(t, &stub.setHeaderToOnDecode, hdr, "%T after RLP decoding with hook", hdr) + assert.Equalf(t, &stub.setHeaderToOnUnmarshalOrDecode, hdr, "%T after RLP decoding with hook", hdr) }) t.Run("error_propagation", func(t *testing.T) { + errMarshal := errors.New("whoops") + errUnmarshal := errors.New("is it broken?") errEncode := errors.New("uh oh") errDecode := errors.New("something bad happened") hdr := new(Header) - extras.Header.Set(hdr, &stubHeaderHooks{ - errEncode: errEncode, - errDecode: errDecode, - }) + setStub := func() { + extras.Header.Set(hdr, &stubHeaderHooks{ + errMarshal: errMarshal, + errUnmarshal: errUnmarshal, + errEncode: errEncode, + errDecode: errDecode, + }) + } + + setStub() + _, err := json.Marshal(hdr) + assert.ErrorIs(t, err, errMarshal, "via json.Marshal()") + assert.Equal(t, errUnmarshal, json.Unmarshal([]byte("{}"), hdr), "via json.Unmarshal()") + setStub() // [stubHeaderHooks] completely overrides the Header assert.Equal(t, errEncode, rlp.Encode(io.Discard, hdr), "via rlp.Encode()") assert.Equal(t, errDecode, rlp.DecodeBytes([]byte{0}, hdr), "via rlp.DecodeBytes()") }) From 3f918999fa651cc66f0bfce4156291a007803fab Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Wed, 18 Dec 2024 16:06:18 +0000 Subject: [PATCH 4/5] chore: linter false positive --- 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 c7f8a7dfd02..8eb69d33014 100644 --- a/core/types/block.libevm_test.go +++ b/core/types/block.libevm_test.go @@ -153,7 +153,7 @@ func TestHeaderHooks(t *testing.T) { setStub() _, err := json.Marshal(hdr) - assert.ErrorIs(t, err, errMarshal, "via json.Marshal()") + assert.ErrorIs(t, err, errMarshal, "via json.Marshal()") //nolint:testifylint // require is inappropriate here as we wish to keep going assert.Equal(t, errUnmarshal, json.Unmarshal([]byte("{}"), hdr), "via json.Unmarshal()") setStub() // [stubHeaderHooks] completely overrides the Header From d1f48166156e43d9d2309b1fb6b4bab38a468a2b Mon Sep 17 00:00:00 2001 From: Arran Schlosberg Date: Thu, 19 Dec 2024 16:23:35 +0000 Subject: [PATCH 5/5] refactor: explicit `err` variables in tests --- core/types/block.libevm_test.go | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/core/types/block.libevm_test.go b/core/types/block.libevm_test.go index 8eb69d33014..7f5cfd83c54 100644 --- a/core/types/block.libevm_test.go +++ b/core/types/block.libevm_test.go @@ -152,12 +152,26 @@ func TestHeaderHooks(t *testing.T) { } setStub() - _, err := json.Marshal(hdr) - assert.ErrorIs(t, err, errMarshal, "via json.Marshal()") //nolint:testifylint // require is inappropriate here as we wish to keep going - assert.Equal(t, errUnmarshal, json.Unmarshal([]byte("{}"), hdr), "via json.Unmarshal()") + // The { } blocks are defensive, avoiding accidentally having the wrong + // error checked in a future refactor. The verbosity is acceptable for + // clarity in tests. + { + _, err := json.Marshal(hdr) + assert.ErrorIs(t, err, errMarshal, "via json.Marshal()") //nolint:testifylint // require is inappropriate here as we wish to keep going + } + { + err := json.Unmarshal([]byte("{}"), hdr) + assert.Equal(t, errUnmarshal, err, "via json.Unmarshal()") + } setStub() // [stubHeaderHooks] completely overrides the Header - assert.Equal(t, errEncode, rlp.Encode(io.Discard, hdr), "via rlp.Encode()") - assert.Equal(t, errDecode, rlp.DecodeBytes([]byte{0}, hdr), "via rlp.DecodeBytes()") + { + err := rlp.Encode(io.Discard, hdr) + assert.Equal(t, errEncode, err, "via rlp.Encode()") + } + { + err := rlp.DecodeBytes([]byte{0}, hdr) + assert.Equal(t, errDecode, err, "via rlp.DecodeBytes()") + } }) }