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

Str-998: Implement different transaction types for the load testing. #658

Merged
merged 7 commits into from
Feb 10, 2025

Conversation

evgenyzdanovich
Copy link
Contributor

Description

  • Implement different transaction types for the load testing.
    • transfers
    • smart contracts (Counter, ERC20, basic Uniswap)
    • all transactions support legacy transaction format, EIP2930 (access-list txs) and EIP1559 (London fork txs)
  • Structured logging for each load job.
    • special logging for transactions so it's clear what is the meaning for every transaction.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature/Enhancement (non-breaking change which adds functionality or enhances an existing one)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactor
  • New or updated tests
  • Dependency Update

Notes to Reviewers

Apologies for the big change, I tried my best to make it as structured and polished as possible.

Checklist

  • I have performed a self-review of my code.
  • I have commented my code where necessary.
  • I have updated the documentation if needed.
  • My changes do not introduce new warnings.
  • I have added tests that prove my changes are effective or that my feature works.
  • New and existing tests pass with my changes.

Related Issues

@evgenyzdanovich evgenyzdanovich changed the title Str-962: Implement different transaction types for the load testing. Str-998: Implement different transaction types for the load testing. Feb 7, 2025
Copy link

codecov bot commented Feb 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 54.66%. Comparing base (004be7c) to head (812e814).
Report is 3 commits behind head on main.

@@            Coverage Diff             @@
##             main     #658      +/-   ##
==========================================
- Coverage   54.68%   54.66%   -0.02%     
==========================================
  Files         317      317              
  Lines       33929    33939      +10     
==========================================
- Hits        18553    18552       -1     
- Misses      15376    15387      +11     

see 5 files with indirect coverage changes

@evgenyzdanovich evgenyzdanovich marked this pull request as ready for review February 7, 2025 15:45
@evgenyzdanovich evgenyzdanovich requested review from a team as code owners February 7, 2025 15:45
Copy link
Contributor

github-actions bot commented Feb 7, 2025

Commit: 0fb4c7e

SP1 Performance Test Results

program cycles success
BTC_BLOCKSPACE 30,370,596
EL_BLOCK 97,975
CL_BLOCK 89,783
L1_BATCH 30,419,814
L2_BATCH 3,566
CHECKPOINT 25,699

delbonis
delbonis previously approved these changes Feb 7, 2025
Copy link
Contributor

@delbonis delbonis left a comment

Choose a reason for hiding this comment

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

Comment, not sure if correct:

If we have these solidity contracts in the repo now does that mean that the load testing run requires having a solidity compiler in ci and running it? If so, would it instead be faster to have compiled initcode checked in and deploy from that instead?

@evgenyzdanovich
Copy link
Contributor Author

evgenyzdanovich commented Feb 7, 2025

@delbonis the load test is disabled for now in the CI.

your suggestion makes sense, I think that's a way to go if/when we enable it for the CI

@delbonis delbonis added this pull request to the merge queue Feb 10, 2025
@delbonis delbonis removed this pull request from the merge queue due to a manual request Feb 10, 2025
@delbonis delbonis added this pull request to the merge queue Feb 10, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 10, 2025
Copy link
Member

@storopoli storopoli left a comment

Choose a reason for hiding this comment

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

ACK 812e814

@evgenyzdanovich evgenyzdanovich added this pull request to the merge queue Feb 10, 2025
Merged via the queue into main with commit ed8d602 Feb 10, 2025
19 checks passed
@evgenyzdanovich evgenyzdanovich deleted the str-962-eth-loadgen branch February 10, 2025 20:31
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.

3 participants