[wip] use header type from libevm #746
Conversation
|
|
||
| // HeaderSerializable defines the header of a block in the Ethereum blockchain, | ||
| // as it is to be serialized into RLP and JSON. | ||
| type HeaderSerializable struct { |
There was a problem hiding this comment.
is it possible to unexport this type? 🤔
| @@ -168,12 +168,13 @@ func (eng *DummyEngine) verifyHeaderGasFields(config *params.ChainConfig, header | |||
| } | |||
|
|
|||
| // Verify BlockGasCost, ExtDataGasUsed not present before AP4 | |||
| headerExtra := types.HeaderExtras(header) | |||
There was a problem hiding this comment.
Can we rename types.HeaderExtras to types.GetHeaderExtra? I don't like Get prefixes, but I prefer it compared to having the plural of extras despite accessing a single extra (and can't name it HeaderExtra due to type name conflict)
| return nil | ||
| } | ||
|
|
||
| //nolint:stdmethods |
There was a problem hiding this comment.
I think this is ineffective, the warning comes from gopls AfAIK and no way to disable it unfortunately (again, AFAIK)
| return out.MarshalJSON() | ||
| } | ||
|
|
||
| //nolint:stdmethods |
There was a problem hiding this comment.
I think this is ineffective, the warning comes from gopls AfAIK and no way to disable it unfortunately (again, AFAIK)
| ParentBeaconRoot *common.Hash `json:"parentBeaconBlockRoot" rlp:"optional"` | ||
| } | ||
|
|
||
| // field type overrides for gencodec |
There was a problem hiding this comment.
nit it would be great to expand on the comment since this has a sort of special role and may be thought of as unneeded by a developer
| // updateFromEth updates the HeaderSerializable from the ethtypes.Header | ||
| func updateFromEth(h *HeaderSerializable, eth *ethtypes.Header) { |
There was a problem hiding this comment.
this and the next few methods should be methods on the headerSerializable struct, so they don't name-conflict with future other types such as blockSerializable
| BloomLookup = ethtypes.BloomLookup | ||
| BytesToBloom = ethtypes.BytesToBloom | ||
| CreateBloom = ethtypes.CreateBloom | ||
| NewReceipt = ethtypes.NewReceipt | ||
| NewContractCreation = ethtypes.NewContractCreation | ||
| NewTransaction = ethtypes.NewTransaction | ||
| EncodeNonce = ethtypes.EncodeNonce | ||
| NewEmptyStateAccount = ethtypes.NewEmptyStateAccount | ||
| SlimAccountRLP = ethtypes.SlimAccountRLP | ||
| FullAccount = ethtypes.FullAccount | ||
| FullAccountRLP = ethtypes.FullAccountRLP |
There was a problem hiding this comment.
We might want to sort these (and existing ones too) alphabetically to have a consistent order with subnet-evm as well.
| return extras.Header.Get(h) | ||
| } | ||
|
|
||
| func WithHeaderExtras(h *Header, extra *HeaderExtra) *Header { |
|
|
||
| // HeaderExtra is a struct that contains extra fields used by Avalanche | ||
| // in the block header. | ||
| // This type uses HeaderSerializable to encode and decode the extra fields |
There was a problem hiding this comment.
nit for godoc automated links
| // This type uses HeaderSerializable to encode and decode the extra fields | |
| // This type uses [HeaderSerializable] to encode and decode the extra fields |
| extras.Header.Set(&cpy, &HeaderExtra{}) | ||
| cpyExtra := HeaderExtras(&cpy) | ||
| *cpyExtra = *HeaderExtras(h) |
There was a problem hiding this comment.
nit a bit less convoluted I think:
| extras.Header.Set(&cpy, &HeaderExtra{}) | |
| cpyExtra := HeaderExtras(&cpy) | |
| *cpyExtra = *HeaderExtras(h) | |
| extra := HeaderExtras(h) | |
| cpyExtra := &HeaderExtra{ | |
| ExtDataHash: extra.ExtDataHash, | |
| } | |
| cpy = *WithHeaderExtras(&cpy, cpyExtra) |
You could also define hExtra instead of extra here to save one line below
| replacer = exec.Command(gocmd, "mod", "edit", "-x", "-require", "github.com/ava-labs/[email protected]", "-replace", "github.com/ava-labs/libevm=github.com/ava-labs/[email protected]") | ||
| replacer.Dir = pkg | ||
| if out, err := replacer.CombinedOutput(); err != nil { | ||
| t.Fatalf("failed to replace binding test dependency to current source tree: %v\n%s", err, out) | ||
| } |
There was a problem hiding this comment.
A comment would be nice 😄
Maybe we can leave this commented with a comment explaining what it does for future work, before this gets merged / new libevm release.
No description provided.