Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions core/types/block.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
60 changes: 40 additions & 20 deletions core/types/block.libevm.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package types

import (
"encoding/json"
"fmt"
"io"

Expand All @@ -27,40 +28,50 @@ 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)
}
Comment on lines +40 to +42
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit no need to assign to r:

Suggested change
if r := registeredExtras; r.Registered() {
return r.Get().hooks.hooksFromHeader(h)
}
if registeredExtras.Registered() {
return registeredExtras.Get().hooks.hooksFromHeader(h)
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did it to avoid unnecessary verbosity and code stuttering. One of the core guiding principles that I follow (and recommend) is for code to get out of the way of the human seeing the meaning being communicated to the computer. Here there's a trade-off between distraction (a 5-syllable variable being unnecessarily repeated, with 3 of them repeated again in the method name) and a meat-RAM allocation (remembering what r is). I chose the latter because it only needs to be remembered for one line.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was actually saying this for readability more than for memory usage 😄
Assigning to r made me think initially we were copying the variable because the method calls were perhaps modifying it. I feel it could hide something eventually if you see what I mean?

return new(NOOPHeaderHooks)
}

func (e ExtraPayloads[HPtr, SA]) hooksFromHeader(h *Header) HeaderHooks {
return e.Header.Get(h)
}

var _ interface {
rlp.Encoder
rlp.Decoder
json.Marshaler
json.Unmarshaler
} = (*Header)(nil)

// 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)
// MarshalJSON implements the [json.Marshaler] interface.
func (h *Header) MarshalJSON() ([]byte, error) {
return h.hooks().MarshalJSON(h)
}

// 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))
// UnmarshalJSON implements the [json.Unmarshaler] interface.
func (h *Header) UnmarshalJSON(b []byte) error {
return h.hooks().UnmarshalJSON(h, b)
}

// 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)
// EncodeRLP implements the [rlp.Encoder] interface.
func (h *Header) EncodeRLP(w io.Writer) error {
return h.hooks().EncodeRLP(h, w)
}

func (e ExtraPayloads[HPtr, SA]) hooksFromHeader(h *Header) HeaderHooks {
return e.Header.Get(h)
// DecodeRLP implements the [rlp.Decoder] interface.
func (h *Header) DecodeRLP(s *rlp.Stream) error {
return h.hooks().DecodeRLP(h, s)
}

func (h *Header) extraPayload() *pseudo.Type {
Expand All @@ -81,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))
}
84 changes: 67 additions & 17 deletions core/types/block.libevm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@
package types_test

import (
"encoding/json"
"errors"
"fmt"
"io"
"testing"

Expand All @@ -31,19 +33,33 @@ 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 {
return append(crypto.Keccak256(h.ParentHash[:]), suffix...)
}

func (hh *stubHeaderHooks) MarshalJSON(h *Header) ([]byte, error) { //nolint:govet
return fakeHeaderJSON(h, hh.suffix), hh.errMarshal
}

func (hh *stubHeaderHooks) UnmarshalJSON(h *Header, b []byte) error { //nolint:govet
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
Expand All @@ -55,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
}

Expand All @@ -66,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...)

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)
})

hdr := &Header{
ParentHash: rng.Hash(),
t.Run("UnmarshalJSON", func(t *testing.T) {
hdr := new(Header)
stub := &stubHeaderHooks{
setHeaderToOnUnmarshalOrDecode: Header{
Extra: []byte("can you solve this puzzle? 0xbda01b6cf56c303bd3f581599c0d5c0b"),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 🤔 🤔 🤔 🤔 🤔 🤔 🤔 🤔 🤔 🤔 🤔 🤔 🤔 🤔 🤔 🤔 🤔 🤔 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any hint on an encoding?
Is this your 256 bits private key? 😄

},
}
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)
Expand All @@ -85,7 +123,7 @@ func TestHeaderHooks(t *testing.T) {

hdr := new(Header)
stub := &stubHeaderHooks{
setHeaderToOnDecode: Header{
setHeaderToOnUnmarshalOrDecode: Header{
Extra: []byte("arr4n was here"),
},
}
Expand All @@ -94,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()") //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
assert.Equal(t, errEncode, rlp.Encode(io.Discard, hdr), "via rlp.Encode()")
assert.Equal(t, errDecode, rlp.DecodeBytes([]byte{0}, hdr), "via rlp.DecodeBytes()")
})
Expand Down
4 changes: 2 additions & 2 deletions core/types/gen_header_json.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading