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

Jump to bottom

chore(core/types): header JSON and RLP serialization hooks (4) #746

Merged
merged 29 commits into from
Feb 25, 2025
Merged

chore(core/types): header JSON and RLP serialization hooks (4) #746

merged 29 commits into from
Feb 25, 2025

Conversation

darioush
Copy link
Collaborator

@darioush darioush commented Jan 13, 2025 •
edited by qdm12
Loading

  • Implement HeaderExtra RLP and JSON serialization methods
  • remove BlockNonce
  • remove EncodeNonce
  • new functions GetHeaderExtra and SetHeaderExtra
  • Migrate existing custom Header to HeaderSerializable in block_ext.go with only Hash method for RLP code generation
  • Rename files gen_header_json.go to gen_header_serializable_json.go
  • Rename files gen_header_rlp.go to gen_header_serializable_rlp.go

PS by qdm12: inappropriate branch name libevm-upstream-types for historical reasons

@darioush darioush changed the base branch from master to libevm January 13, 2025 23:14
qdm12
qdm12 reviewed Jan 21, 2025
View reviewed changes
This was referenced Feb 10, 2025
Closed
Closed
qdm12 added a commit that referenced this pull request Feb 10, 2025
@qdm12
chore(all): minimal changes for last libevm dependency
61a477f 
- Uses tip of main libevm dependency + cherry picked commit: ava-labs/libevm@d3f6a5e
- Precompile legacy gas fix (done in ava-labs/libevm@f906679)
- Added `HeaderExtra` not-implemented implementation
- Minimal changes shamelessly copied over from #746
qdm12 added a commit that referenced this pull request Feb 10, 2025
@qdm12
chore(all): minimal changes for last libevm dependency
75b9e0d 
- Uses tip of main libevm dependency + cherry picked commit: ava-labs/libevm@d3f6a5e
- Precompile legacy gas fix (done in ava-labs/libevm@f906679)
- Added `HeaderExtra` not-implemented implementation
- Minimal changes shamelessly copied over from #746
qdm12 added a commit that referenced this pull request Feb 10, 2025
@qdm12
chore(all): minimal changes for last libevm dependency
ab19679 
- Uses tip of main libevm dependency + cherry picked commit: ava-labs/libevm@d3f6a5e
- Precompile legacy gas fix (done in ava-labs/libevm@f906679)
- Added `HeaderExtra` not-implemented implementation
- Minimal changes shamelessly copied over from #746
@qdm12 qdm12 force-pushed the libevm-upstream-types branch from f133755 to ef168bd Compare February 10, 2025 15:20
@qdm12 qdm12 changed the title [wip] use header type from libevm chore(core/types): header JSON and RLP serialization hooks Feb 10, 2025
@qdm12 qdm12 changed the base branch from libevm to qdm12/libevm/structure February 10, 2025 15:22
@qdm12 qdm12 changed the title chore(core/types): header JSON and RLP serialization hooks chore(core/types): header JSON and RLP serialization hooks (3) Feb 10, 2025
@qdm12 qdm12 force-pushed the libevm-upstream-types branch 3 times, most recently from 269fb9c to d98039c Compare February 10, 2025 16:21
@qdm12 qdm12 force-pushed the qdm12/libevm/structure branch 2 times, most recently from 7093fc3 to 3e8d4e4 Compare February 12, 2025 12:30
@qdm12 qdm12 force-pushed the libevm-upstream-types branch from d98039c to d52d44f Compare February 12, 2025 12:43
@qdm12 qdm12 force-pushed the qdm12/libevm/structure branch from 3e8d4e4 to 806817d Compare February 12, 2025 12:53
@qdm12 qdm12 force-pushed the libevm-upstream-types branch 2 times, most recently from 59ef836 to c6f5f69 Compare February 12, 2025 12:55
qdm12
qdm12 reviewed Feb 12, 2025
View reviewed changes
Copy link
Collaborator

@qdm12 qdm12 left a comment

Choose a reason for hiding this comment

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

Self review

@qdm12 qdm12 force-pushed the libevm-upstream-types branch from 6ac9728 to e0537f8 Compare February 12, 2025 14:59
qdm12
qdm12 reviewed Feb 12, 2025
View reviewed changes
@qdm12 qdm12 force-pushed the qdm12/libevm/structure branch from 806817d to 9165b01 Compare February 13, 2025 16:52
@qdm12 qdm12 force-pushed the libevm-upstream-types branch from e0537f8 to 22d5be0 Compare February 13, 2025 16:54
@qdm12 qdm12 force-pushed the qdm12/libevm/structure branch from 9165b01 to 514bcdb Compare February 14, 2025 13:29
@qdm12 qdm12 force-pushed the libevm-upstream-types branch from 22d5be0 to f743475 Compare February 14, 2025 13:29
This was referenced Feb 14, 2025
Closed
Merged
@qdm12 qdm12 requested a review from ARR4N February 24, 2025 11:49
ARR4N
ARR4N requested changes Feb 24, 2025
View reviewed changes
Copy link
Contributor

@ARR4N ARR4N left a comment

Choose a reason for hiding this comment

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

The change to the test makes it possibly tautological so it's important to revert it.

darioush
darioush commented Feb 25, 2025
View reviewed changes
@qdm12 @darioush
scripts/lint_allowed_eth_imports.sh: use -name instead of -path f…
7a05828 
…or gen_*.go files

Co-authored-by: Darioush Jalali <[email protected]>
Signed-off-by: Quentin McGaw <[email protected]>
ARR4N
ARR4N previously approved these changes Feb 25, 2025
View reviewed changes
darioush
darioush commented Feb 25, 2025
View reviewed changes
Copy link
Collaborator Author

@darioush darioush left a comment

Choose a reason for hiding this comment

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

I can't approve this but LGTM

qdm12
qdm12 previously approved these changes Feb 25, 2025
View reviewed changes
Copy link
Collaborator

@qdm12 qdm12 left a comment

Choose a reason for hiding this comment

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

Self approving 🪃 !

ARR4N
ARR4N reviewed Feb 25, 2025
View reviewed changes
qdm12 and others added 9 commits February 25, 2025 16:09
@qdm12
PR feedback nits
aefb9c1 
- `ex` -> `headerExtra`
- `exportedFieldsDeepCopied` -> `exportedFieldsPointToDifferentMemory`
- use intrange when iterating over v.NumField()
- `testHeaderEncodings` -> `testHeaderEncodeDecode`
- Add comment why returned value of `testHeaderEncodeDecode` is ignored for TestHeaderJSON
- Rename `b` to `encoded` (named return) in `testHeaderEncodeDecode`
@qdm12
Remove core/types from scripts/eth-allowed-packages.txt
089432e
@qdm12
Add comment to HeaderSerializable.Hash method
3b9ef2a
@qdm12
Re-generate JSON for HeaderSerializable
728cbfe
@qdm12
Rename generated codec files
aab9d7e 
- gen_header_json.go -> gen_header_serializable_json.go
- gen_header_rlp.go -> gen_header_serializable_rlp.go
@qdm12
Add note on custom generators used to support type aliases
56b296b
@ceyonur @qdm12
remove vmerrs (#829)
a7f9917
@qdm12
Simplify assertDifferentPointers to not rely on testify/assert
4d2f143
@qdm12
Revert to using assertNonZero function in allExportedFieldsSet
57a6734
@qdm12 qdm12 force-pushed the libevm-upstream-types branch from 8ca5efd to 57a6734 Compare February 25, 2025 15:13
@qdm12 @ARR4N
Improve testHeaderEncodeDecode signature definition
d360c2c 
Co-authored-by: Arran Schlosberg <[email protected]>
Signed-off-by: Quentin McGaw <[email protected]>
@qdm12 qdm12 requested a review from ARR4N February 25, 2025 15:24
ARR4N
ARR4N approved these changes Feb 25, 2025
View reviewed changes
Copy link
Contributor

@ARR4N ARR4N left a comment

Choose a reason for hiding this comment

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

Noting that force push was a no-op.

qdm12
qdm12 approved these changes Feb 25, 2025
View reviewed changes
Copy link
Collaborator

@qdm12 qdm12 left a comment

Choose a reason for hiding this comment

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

Re-approving (given no-op force push)

@qdm12 qdm12 merged commit 0657877 into libevm Feb 25, 2025
6 checks passed
@qdm12 qdm12 deleted the libevm-upstream-types branch February 25, 2025 16:19
qdm12 added a commit that referenced this pull request Feb 25, 2025
@darioush @qdm12
@ARR4N @ceyonur
chore(core/types): header JSON and RLP serialization hooks (#746)
45b8b8b 
- Implement `HeaderExtra` RLP and JSON serialization methods
- remove `BlockNonce`
- remove `EncodeNonce`
- new functions `GetHeaderExtra` and `SetHeaderExtra`
- Migrate existing custom `Header` to `HeaderSerializable` in block_ext.go with only `Hash` method for RLP code generation
- Rename files gen_header_json.go to gen_header_serializable_json.go
- Rename files gen_header_rlp.go to gen_header_serializable_rlp.go

Signed-off-by: Quentin McGaw <[email protected]>
Co-authored-by: Quentin Mc Gaw <[email protected]>
Co-authored-by: Arran Schlosberg <[email protected]>
Co-authored-by: Ceyhun Onur <[email protected]>
@qdm12 qdm12 mentioned this pull request Feb 26, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ARR4N ARR4N ARR4N approved these changes

@qdm12 qdm12 qdm12 approved these changes

@ceyonur ceyonur Awaiting requested review from ceyonur ceyonur is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@darioush @ARR4N @qdm12 @ceyonur