Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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))
}
102 changes: 83 additions & 19 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,20 +132,46 @@ 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()
// 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()")
}

assert.Equal(t, errEncode, rlp.Encode(io.Discard, hdr), "via rlp.Encode()")
assert.Equal(t, errDecode, rlp.DecodeBytes([]byte{0}, hdr), "via rlp.DecodeBytes()")
setStub() // [stubHeaderHooks] completely overrides the Header
{
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()")
}
})
}
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