feat: params.ChainConfig and params.Rules extra payloads
#1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ARR4N
added
the
Status: 🔴 DO NOT MERGE
darioush
reviewed
Sep 5, 2024
ARR4N
force-pushed
the
arr4n/libevm-params-extras
branch
from
5781184 to
a6742fd
Compare
ARR4N
removed
the
Status: 🔴 DO NOT MERGE
darioush
approved these changes
Sep 9, 2024
ARR4N
added a commit
that referenced
this pull request
Dec 17, 2024
## Why this should be merged The `types.Header` fields of both [`coreth`](https://pkg.go.dev/github.com/ava-labs/coreth/core/types#Header) and [`subnet-evm`](https://pkg.go.dev/github.com/ava-labs/subnet-evm/core/types#Header) have been modified such that their RLP encodings (i.e. block hashes) aren't compatible with vanilla `geth` nor each other. This PR adds support for arbitrary RLP encoding coupled with type-safe extra payloads. ## How this works Equivalent to #1 (`params`) and #44 (`types.StateAccount`) registration of pseudo-generic payloads. The only major difference is the guarantee of a non-nil payload pointer, which means that the payload hooks are never called on nil pointers as this would make it difficult to decode RLP into them. ## How this was tested Round-trip RLP {en,de}coding via a registered stub hook. --------- Signed-off-by: Arran Schlosberg <[email protected]> Co-authored-by: Quentin McGaw <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why this should be merged
The
params.ChainConfigandparams.Rulestypes are (possibly the most) widely used configuration structs in geth. This makes them ideal candidates for carrying arbitrary data payloads without altering geth plumbing. This PR introduces a method for adding arbitrary payloads that is:Note
This chain of PRs will all be merged into the
arr4n/libevmbranch, which will itself be squash-merged intolibevm. This keeps the commit history intact so each PR's diff base will automatically update.How this works
Start by reviewing
params/example.libevm_test.gofor the intended usage.Both
ChainConfigandRuleshaveextrafields added to them to carry the payloads. Without making the structs generic, some mechanism must exist for indicating the concrete types of the fields when constructing new instances (including JSON unmarshalling). The newparams.RegisterExtras[C,R]()function allows for registration of all the necessary types and logic for construction.Although the
extrafields could have been regular interfaces, this left them too vulnerable to unintentional override with arbitrary values. Thepseudopackage was thus introduced and its rationale is explained in its package comment. However, its usage is entirely hidden and MUST remain so; a primary goal of this implementation was to make the exported API easy to use and hard to misuse.In follow-up PRs,
CandRtype parameters will no longer beanybut interfaces required to add other configurable behaviour via hooks. See:params.Extrashooks #2params.RulesHooks.CanExecuteTransaction()receives EIP-2930 access list #5How this was tested
Unit tests of new functionality, without modification of existing geth tests. A testable example acts as an integration test.