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] better test structure #82

Merged
merged 12 commits into from
Mar 25, 2025

Conversation

noel2004
Copy link
Member

@noel2004 noel2004 commented Mar 15, 2025

Base on the desination in notion

Pending tasks:

  • Complete test data samples (after some feature in PR target has completed)
  • Test all entries in Makefile

With the new test data structure we have 3 types of assets:

  1. Block Witness: which is the starting point of almost tests, and they would be kept stable for many release since they are just ethereum blocks now
  2. Tasks: for most cases we obtain some task data from coordinator side and test them so it do not need to put samle data within the directory
  3. Proofs: also, we do not need to put data within this directory in advance since the commitment change too frequent. But some test for base layer (like chunk tests) can put their output proofs here and being used by the tests for "higher" layer (like batch or bundle)

@github-actions github-actions bot added crate-integration Updates to the integration crate crate-circuits Any update made to the circuits, i.e. commitments reflect a change labels Mar 15, 2025
@roynalnaruto
Copy link
Collaborator

I agree with @lispc . The test structure and test modules should be as easy as possible to read/understand.

@roynalnaruto
Copy link
Collaborator

@noel2004 any progress with this?

@noel2004
Copy link
Member Author

@noel2004 any progress with this?

We have to complete #81 first to enable full test here

@noel2004 noel2004 force-pushed the feat/better_test_structure branch from d2f0b51 to a10ed21 Compare March 24, 2025 02:22
@github-actions github-actions bot removed the crate-circuits Any update made to the circuits, i.e. commitments reflect a change label Mar 24, 2025
@github-actions github-actions bot added the crate-prover Updates to the prover crate label Mar 24, 2025
@noel2004 noel2004 marked this pull request as ready for review March 24, 2025 13:12
@noel2004 noel2004 requested a review from roynalnaruto as a code owner March 24, 2025 13:12
Copy link
Contributor

@lispc lispc left a comment

Choose a reason for hiding this comment

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

now V3 / V7 / version: 6,7 / phase1 / phase2 is too complex..
anyway we can fix it in later commits.

@lispc lispc merged commit 4364b22 into feat/towards-runtime-multi-version Mar 25, 2025
8 checks passed
roynalnaruto pushed a commit that referenced this pull request Mar 25, 2025
* circuit::types can support multi versions

* phase feature free for batch circuit

* fix issue after merging

* make chunk circuit feature free

* prune unnecessary commitment file, and revert verify_commitments update

* fix: soundnes in bundle circuit and typo

* bundle circuit feature free

* refactoring:
Rename codec_version => fork_name
unitfy the occurance of fork_name field among task and witness (not info)

* resume multiple version of bundle circuit

* update build-guest

* update make file

* fix build-guest

* update lock

* fix issue in build guest input

* enable co-exist of mutiple bundle app

* fix typo

* fix "most legacy" issue

* update release scripts

* [FEAT] better test structure (#82)

* better structure for testdata (WIP)

* fix chunk test and prune batch test (WIP)

* update make file and test samples

* test new batch test scheme

* add batch invariant test

* update makefile for required chunk proof

* prune unnecessary container cleanning

* fixings after rebase

* fix according to review: use more straight fashion for chunk task generations

* FIX: the correct order in batchheader enum

* better version setting for test task

* add readme files

* rename v3 to v6

* rename "legacy" to "euclidv1" according to review

* renaming in scripts

* task must use explicitily specified `fork_name`

* update the sanity check of bundle hash in e2e of phase v1 since the "version" field in batchheader changed (7->6)

---------

Co-authored-by: noelwei <fan@scroll.io>
Co-authored-by: Ho <noelwei@gmail.com>
roynalnaruto added a commit that referenced this pull request Mar 27, 2025
* phase1 tested

* seems ok

* lint

* cleanup

* fix(TOB-SCREUC-1): at least 1 pi | pis and pi_hashes are equal in length

* chore: rename chunk infos

* fix(TOB-SCREUC-2): insufficient validation of chunk info between witness and blob

* fix(TOB-SCREUC-5): lax parsing of batch payload

* fix: code quality recommendations

* chore: update commitments

* fix: update v3 payload parsing

* chore: update commitments

* [Fix] for phase1 rc9 (#87)

* fix pi hash in bundle

* add santiy check for bundle task

* fix typo and expression

* add local task test, output local task in e2e

* + add bundle local task sample, update batch local task sample
+ sanity check for final e2e bundle pi

* rc9

* rc9

* phase2

* done

* feat: extend prover config

* fix(integration): compile OK

* chore: bump rc10 and update commitments

* upgrade openvm (#97)

* upgrade openvm

* chore: update snark-verifier-sdk

* chore: update tiny-keccak

* chore: upgrade openvm (alloy-core, revm, reth, sbv)

* chore: update sbv

* chore(build-guest): build both since openvm upgraded

* chore: update commitments

---------

Co-authored-by: Rohit Narurkar <rohit.narurkar@proton.me>

* Update the comment for kzg g2 setup (#88)

* chore: update commitments

* Phase-2 audit fixes (#98)

* fix(TOB-SCREUC2-3): use github output instead of github env

* fix(TOB-SCREUC2-4): pin 3rd party actions

* fix(TOB-SCREUC2-5): do not persist git credentials

* feat: runtime multi version (#81)

* circuit::types can support multi versions

* phase feature free for batch circuit

* fix issue after merging

* make chunk circuit feature free

* prune unnecessary commitment file, and revert verify_commitments update

* fix: soundnes in bundle circuit and typo

* bundle circuit feature free

* refactoring:
Rename codec_version => fork_name
unitfy the occurance of fork_name field among task and witness (not info)

* resume multiple version of bundle circuit

* update build-guest

* update make file

* fix build-guest

* update lock

* fix issue in build guest input

* enable co-exist of mutiple bundle app

* fix typo

* fix "most legacy" issue

* update release scripts

* [FEAT] better test structure (#82)

* better structure for testdata (WIP)

* fix chunk test and prune batch test (WIP)

* update make file and test samples

* test new batch test scheme

* add batch invariant test

* update makefile for required chunk proof

* prune unnecessary container cleanning

* fixings after rebase

* fix according to review: use more straight fashion for chunk task generations

* FIX: the correct order in batchheader enum

* better version setting for test task

* add readme files

* rename v3 to v6

* rename "legacy" to "euclidv1" according to review

* renaming in scripts

* task must use explicitily specified `fork_name`

* update the sanity check of bundle hash in e2e of phase v1 since the "version" field in batchheader changed (7->6)

---------

Co-authored-by: noelwei <fan@scroll.io>
Co-authored-by: Ho <noelwei@gmail.com>

* chore: update commitments

* upgrade openvm and revm; fix pairing edge cases

* add commitments

* fmt

* pruge unused bls12_381 openvm extension in bundle circuit

* fix(TOB-SCREUC2-3) follow-up: extract variable and use

* feat: rv32 chunk fallback (#100)

* feat: batch support rv32 chunk

* add ChunkRv32Prover and e2e tests, testing

* add commitment verify for rv32 fallback in verifier

* testing e2e

* add commitments

* toolchain & lint

* docker

* tag 0.2.0

* update commits

---------

Co-authored-by: noelwei <fan@scroll.io>

* clean

* chore: cleanup

* chore: update commitments

* feat(build-guest): write digests

* fix(build-guest): add digests

* fix: batch task v7 contains unpadded blob bytes, envelope v7 expects padded

* [Feat] sanity check for pi while generating proof (#102)

* sanity check for pi in proof gen

* update according to review

* fix another issue

* add missed assert

* resume the comment

* chore: update commitments

* fix: update release script to push digests from correct source path

* fix(tests): prev msg queue hash population for euclidv1

* fix: sanity check on wrapped proof

* fix: tests

* chore: update commitments

---------

Co-authored-by: Rohit Narurkar <rohit.narurkar@proton.me>
Co-authored-by: Ho <fan@scroll.io>
Co-authored-by: xkx <xiakunxian130@gmail.com>
Co-authored-by: Ho <noelwei@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate-integration Updates to the integration crate crate-prover Updates to the prover crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants