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

feat: base token integration tests #2509

Open
wants to merge 91 commits into
base: main
Choose a base branch
from
Open

feat: base token integration tests #2509

wants to merge 91 commits into from

Conversation

cytadela8
Copy link
Member

@cytadela8 cytadela8 commented Jul 26, 2024 •
edited
Loading

What ❔

  • Introduce 2 new tests for base token:
    • gas calculation
    • proper application of dynamic changes to base token ratio
  • Improve forced price client fluctuations implementation to limit difference between consecutive values.

Why ❔

  • We want better integration test coverage of custom base token

Checklist

  • PR title corresponds to the body of PR (we generate changelog entries from PRs).
  • Tests for the changes have been added / updated.
  • Documentation comments have been added / updated.
  • Code has been formatted via zk fmt and zk lint.

cytadela8 and others added 27 commits August 26, 2024 14:25
@cytadela8
Use forced price api client by default
87fc80f
@cytadela8
fix: smoothed ForcedPriceClient
0d8b455
@cytadela8
feat: Integration test gas price incl calculation
0ca2b5e
@cytadela8
fix: Fetch base ratio immediately on start
f15bade
@cytadela8
fix: enable all fees tests
ee7800b
@cytadela8
fix: increase alice wallet balance in fees.test.ts
6112552
@cytadela8
fix: style
e3e1356
@cytadela8
fix(tests): Kill with sigint
fd5fa8b
@cytadela8
tmp: remove randomness in base token price
7592dfd
@cytadela8
Try upgrade zksync-ethers to 6.11.0
a656770
@cytadela8
debug
ce0d00f
@cytadela8
Revert "Try upgrade zksync-ethers to 6.11.0"
73cebec 
This reverts commit 7e3213f.
@cytadela8
fix: properly kill server after fees test
ecf18a4
@cytadela8
Avoid gasEstimate in fees test
cb76faa
@cytadela8
fix: style
04b023a
@cytadela8
debug: more logs
79e691f
@cytadela8
debug: log addresses
8e554a2
@cytadela8
fix: increase test ETH per account due to large gas price test and re…
50fcd67 
…move transfer
@cytadela8
Revert "fix: increase test ETH per account due to large gas price tes…
baa5261 
…t and remove transfer"

This reverts commit 838b8a2.
@cytadela8
fix(test): Increase fees.test.ts alice account balance
c5213b1
@cytadela8
fix(test): Lower l1 gas price in expected gas price test
644c230
@cytadela8
fix(test): Lower l1 gas price in expected gas price test even more
0a9676b
@cytadela8
Revert "Avoid gasEstimate in fees test"
46d5414 
This reverts commit 90183b3.
@cytadela8
Revert "tmp: remove randomness in base token price"
c902dc2 
This reverts commit 6b330a4.
@cytadela8
fix: add validium compute overhead
93126b5
@ischasny @cytadela8
fix tests
358769d
@ischasny @cytadela8
reduce variation range
6c1f1af
@perekopskiy
Copy link
Contributor

it seems base ERC20 contract checks › Base token ratio is updated on L1 is flaky (check last run logs)

@cytadela8
Copy link
Member Author

Due to removal of zk based tests, this PR is blocked by #2898

@perekopskiy
Copy link
Contributor

@Deniallugo ^

@cytadela8
Copy link
Member Author

it seems base ERC20 contract checks › Base token ratio is updated on L1 is flaky (check last run logs)

I believe I found where the price was not set properly. Seems this was not "flaky", but permanently broken. pushing a fix...

@cytadela8
Merge remote-tracking branch 'origin/main' into apu-base-token-integr…
5360744 
…ation
@cytadela8
feat: migrated to new way of starting server in fees test
f11a099 
also refactored a little how server is configured for fees test
@cytadela8
fix: import issues, missing kill, format
70fae20
@cytadela8
debug: debug logs
64c7240
@cytadela8
Merge remote-tracking branch 'origin/main' into apu-base-token-integr…
ef3c924 
…ation
@cytadela8
Merge remote-tracking branch 'origin/main' into apu-base-token-integr…
e8c087e 
…ation
@cytadela8
fix: node spawning bug
1c6de49
@cytadela8
feat: simplify node spawning to avoid forgotten nodes
b03433b
@cytadela8
Merge remote-tracking branch 'origin/main' into apu-base-token-integr…
7ca8434 
…ation
@cytadela8
lint
ef0e4ed
@cytadela8
feat: nonce stable deposit of token sin integration test context-owne…
7026fa1 
…r.ts
@cytadela8
Revert "feat: nonce stable deposit of token sin integration test cont…
5148250 
…ext-owner.ts"

This reverts commit 7026fa1.
@cytadela8
feat: less stupid yaml config override code
f05a21a
@cytadela8
fix: distribute eth to token_multplier_setter
9ebfc33
@cytadela8
fix: Add 1 block wait to gas price test
509dab3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@popzxc popzxc popzxc left review comments

@perekopskiy perekopskiy perekopskiy left review comments

@yorik yorik Awaiting requested review from yorik yorik is a code owner automatically assigned from matter-labs/devops

@alexandrst88 alexandrst88 Awaiting requested review from alexandrst88 alexandrst88 is a code owner automatically assigned from matter-labs/devops

@artmakh artmakh Awaiting requested review from artmakh artmakh is a code owner automatically assigned from matter-labs/devops

@hatemosphere hatemosphere Awaiting requested review from hatemosphere hatemosphere is a code owner automatically assigned from matter-labs/devops

@onyxet onyxet Awaiting requested review from onyxet onyxet is a code owner automatically assigned from matter-labs/devops

@otani88 otani88 Awaiting requested review from otani88 otani88 is a code owner automatically assigned from matter-labs/devops

@iluwaa iluwaa Awaiting requested review from iluwaa iluwaa is a code owner automatically assigned from matter-labs/devops

@ischasny ischasny Awaiting requested review from ischasny

At least 1 approving review is required to merge this pull request.

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
@cytadela8 @perekopskiy @popzxc @ischasny