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(l1): enhance el assertoor ci test by cherrypicking more checks #963

Merged
merged 16 commits into from
Oct 25, 2024

Conversation

rodrigo-o
Copy link
Collaborator

@rodrigo-o rodrigo-o commented Oct 24, 2024

Motivation

This PR tries to add as much test as possible to our assertoor CI run.

Description

The idea is to add every check that is useful for validating the el execution without incurring in long running times to avoid making the CI slower. Right now every useful check from the supported is added except for:

  • check_consensus_finality | time: ~10m
  • check_consensus_attestation_stats | time: ~10m

Apart from checks, we are not using most of the generate_* tasks due to timing or errors:

  • run_transaction_test | time: ~10m
  • run_blob_transaction_test | time: ~6m
  • run_opcodes_transaction_test | time: ~5m (We have issues processing some of the transactions)

For the already available assertor playbooks, see here.

The run_blob_transaction_test is the only one added until now from transaction checks and the total test take nearly 15m (which is less than the largest hive test in the CI).

Closes #953

@rodrigo-o rodrigo-o marked this pull request as ready for review October 25, 2024 01:14
@rodrigo-o rodrigo-o requested a review from a team as a code owner October 25, 2024 01:14
@mpaulucci
Copy link
Collaborator

Are all blocks proposed by our execution client empty blocks (i.e no transactions)?

@rodrigo-o
Copy link
Collaborator Author

rodrigo-o commented Oct 25, 2024

Are all blocks proposed by our execution client empty blocks (i.e no transactions)?

In most of the test, yes, our blocks are empty. But, due to the addition of the run_blob_transaction_tests during that execution our blocks have blobs (the test generates transactions for all clients instead of spamming one and expecting P2P to be in place). Unfortunately the run_transaction_tests which adds legacy and dfyne transactions and checks it's addition makes the job to last past the longest run (19m), but it worked, I already tested it!

@mpaulucci
Copy link
Collaborator

But if our blocks have blobs, they have a associated transaction too, right?

@rodrigo-o
Copy link
Collaborator Author

But if our blocks have blobs, they have a associated transaction too, right?

Yes, exactly. But just to be clear that happens only while that specific test is running, for most of the rest of the run our blocks are empty.

@mpaulucci
Copy link
Collaborator

Ok, but at least we have one test with transactions proposed by us!

@rodrigo-o
Copy link
Collaborator Author

rodrigo-o commented Oct 25, 2024

Ok, but at least we have one test with transactions proposed by us!

Yes! and here is a proof of that:

image

@rodrigo-o rodrigo-o added this pull request to the merge queue Oct 25, 2024
Merged via the queue into main with commit 3bca636 Oct 25, 2024
11 checks passed
@rodrigo-o rodrigo-o deleted the enhance-el-assertoor-ci-test branch October 25, 2024 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] Cherry-pick assertoor-test checks that could be added to the CI run
2 participants