Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core/types): Header Copy hook #106

Open
wants to merge 7 commits into
base: darioush/libevm-phase-2.5
Choose a base branch
from
+25 −0
Open

feat(core/types): Header Copy hook #106

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0508241
feat(core/types): header copy hooks
qdm12 Jan 21, 2025
b9e5186
Export `copyHeader` as `CopyEthHeader`
qdm12 Jan 21, 2025
f8a0ea3
Move to a PostCopy hook 🎉
qdm12 Jan 31, 2025
9236b52
Change `PostCopy(dst, src)` -> `PostCopy(dst)`
qdm12 Jan 31, 2025
df41892
add test case
qdm12 Feb 3, 2025
f598993
Update core/types/block.libevm_test.go
qdm12 Feb 4, 2025
edde146
Rename stubHeaderHooks copied -> toCopy
qdm12 Feb 4, 2025
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
5 changes: 3 additions & 2 deletions core/types/block.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,8 +277,9 @@ func NewBlockWithWithdrawals(header *Header, txs []*Transaction, uncles []*Heade
return b.WithWithdrawals(withdrawals)
}

// CopyHeader creates a deep copy of a block header.
func CopyHeader(h *Header) *Header {
// CopyEthHeader creates a deep copy of an Ethereum block header.
// Use [CopyHeader] instead if your header has any registered extra.
func CopyEthHeader(h *Header) *Header {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This introduces a breaking change, which we should only do as an absolute last resort because it can have flow-on effects that require changes elsewhere now or in the future. This was a primary source of difficulty when performing upgrades under the old forking approach as I explained in the proposal for this project:

A core tenet of [the libevm] approach is that geth MUST be considered to be a “user” of libevm. As they update their own code, they assume that the rest of it is intact—this may be a tautology, but an important one nonetheless.

It also places the entire burden of copying on the libevm importer when all that's needed is for them to copy the registered extra.

What if the hook was PostCopy(dst, src *Header), called at the end of this function, and the NOOPHeaderHooks implementation was empty?

func CopyHeader(h *Header) *Header {
// ...
  h.hooks().PostCopy(cpy, h)
  return cpy
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's PostCopy(dst) in the end, given the source is already accessible by the header hooks.

Copy link
Collaborator

Choose a reason for hiding this comment

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

How is it already accessible? Can you share an example snippet, please?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Never mind, I had too many dead brain cells when writing this.
PostCopy doesn't (at least for now) need to do anything with the source header, since it's meant to only copy the extra payload (aka HeaderExtra). For example in coreth:

https://github.com/ava-labs/coreth/pull/759/files#diff-c238f33a661d072dc51dd5a20ab81dbd341f59478b5c26d25db686822b492521R85

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's fair. We can always add it later as we don't make any interface-stability guarantees.

cpy := *h
if cpy.Difficulty = new(big.Int); h.Difficulty != nil {
cpy.Difficulty.Set(h.Difficulty)
Expand Down
10 changes: 10 additions & 0 deletions core/types/block.libevm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ type HeaderHooks interface {
UnmarshalJSON(*Header, []byte) error //nolint:govet
EncodeRLP(*Header, io.Writer) error
DecodeRLP(*Header, *rlp.Stream) error
Copy(*Header) *Header
}

// hooks returns the Header's registered HeaderHooks, if any, otherwise a
Expand Down Expand Up @@ -108,3 +109,12 @@ func (*NOOPHeaderHooks) DecodeRLP(h *Header, s *rlp.Stream) error {
type withoutMethods Header
return s.Decode((*withoutMethods)(h))
}

func (n *NOOPHeaderHooks) Copy(h *Header) *Header {
return CopyEthHeader(h)
}

// CopyHeader creates a deep copy of a block header.
func CopyHeader(h *Header) *Header {
return h.hooks().Copy(h)
}
4 changes: 4 additions & 0 deletions core/types/block.libevm_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ func (hh *stubHeaderHooks) DecodeRLP(h *Header, s *rlp.Stream) error {
return hh.errDecode
}

func (hh *stubHeaderHooks) Copy(h *Header) *Header {
return h
}

func TestHeaderHooks(t *testing.T) {
TestOnlyClearRegisteredExtras()
defer TestOnlyClearRegisteredExtras()
Expand Down