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.

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: add reth test-vectors compact --write|--read #11954

Merged
merged 18 commits into from
Oct 24, 2024
Merged

feat: add reth test-vectors compact --write|--read #11954

merged 18 commits into from
Oct 24, 2024

Conversation

joshieDo
Copy link
Collaborator

@joshieDo joshieDo commented Oct 22, 2024

closes #11924

  1. Adds another CI job that will on every PR:
    a) checkout main
    b) randomly generate and serialize to disk many different type vectors with Compact (eg. Headers, Transaction, etc)
    c) checkout pr
    d) deserialize previously generated test vectors
  2. Adds static test_decode tests for transaction types, receipt and headers.

This ensures that the PR does not introduce a non-backwards compatible change to a type that implements Compact.

For now, the CI job is disabled. It would fail since main does not have the command. It will be enabled later on.

--

There's some follow-ups to be done here:

    // Signature todo we for v we only store parity(true || false), while v can take more values
    // Bytecode, // todo revm arbitrary
    // MerkleCheckpoint, // todo storedsubnode -> branchnodecompact arbitrary alloy-trie
    // StorageTrieEntry, // todo branchnodecompact arbitrary alloy-trie
    // StoredSubNode, // todo branchnodecompact arbitrary alloy-trie

@joshieDo joshieDo added C-debt A clean up/refactor of existing code A-db Related to the database C-test A change that impacts how or what we test A-ci Related to github workflows or other build and lint tools labels Oct 22, 2024
@github-actions github-actions bot added the C-enhancement New feature or request label Oct 22, 2024
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

are these helper functions to generate test vectors and we run this once to get static test vectors
or are these always generated?

@joshieDo
Copy link
Collaborator Author

joshieDo commented Oct 22, 2024

we can do both

are these always generated?

I was leaning towards this one. during PR CI, generate with main branch, and read with branch

@joshieDo joshieDo force-pushed the joshie/tcomp branch 7 times, most recently from ad05e60 to cbe43c8 Compare October 23, 2024 07:58
@joshieDo joshieDo marked this pull request as ready for review October 23, 2024 07:58
@joshieDo joshieDo marked this pull request as draft October 23, 2024 08:08
@joshieDo joshieDo force-pushed the joshie/tcomp branch 2 times, most recently from 76fb9ce to e7c59cd Compare October 23, 2024 08:20
@joshieDo joshieDo force-pushed the joshie/tcomp branch 3 times, most recently from 8116c45 to 8aaaa4c Compare October 23, 2024 09:11
@joshieDo joshieDo changed the title feat: add reth test-vectors compact --write|--read to CI feat: add reth test-vectors compact --write|--read Oct 23, 2024
@joshieDo joshieDo marked this pull request as draft October 23, 2024 15:44
@joshieDo joshieDo marked this pull request as ready for review October 23, 2024 15:57
@mattsse
Copy link
Collaborator

mattsse commented Oct 24, 2024

looks like the only thing missing here is some op feature combination

@mattsse mattsse added this pull request to the merge queue Oct 24, 2024
Merged via the queue into main with commit 777417a Oct 24, 2024
40 checks passed
@mattsse mattsse deleted the joshie/tcomp branch October 24, 2024 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ci Related to github workflows or other build and lint tools A-db Related to the database C-debt A clean up/refactor of existing code C-enhancement New feature or request C-test A change that impacts how or what we test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add more compact sanity tests
2 participants