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: l2 native token tests #2916

Merged
merged 63 commits into from
Oct 8, 2024
Merged

Conversation

kelemeno
Copy link
Contributor

What ❔

Why ❔

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.

kelemeno and others added 16 commits October 2, 2024 12:24
## What ❔

<!-- What are the changes this PR brings about? -->
<!-- Example: This PR adds a PR template to the repo. -->
<!-- (For bigger PRs adding more context is appreciated) -->

## Why ❔

<!-- Why are these changes done? What goal do they contribute to? What
are the principles behind them? -->
<!-- Example: PR templates ensure PR reviewers, observers, and future
iterators are in context about the evolution of repos. -->

## Checklist

<!-- Check your PR fulfills the following items. -->
<!-- For draft PRs check the boxes as you complete them. -->

- [ ] 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_supervisor fmt` and `zk_supervisor
lint`.

---------

Co-authored-by: Raid Ateir <[email protected]>
Co-authored-by: kelemeno <[email protected]>
Copy link
Contributor

@StanislavBreadless StanislavBreadless left a comment

Choose a reason for hiding this comment

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

the contracts commit used here is not the latest one from the branch in contracts

@@ -0,0 +1,50 @@
sudo rm -rf ./volumes && zk_supervisor clean containers && zk_inception up -o false
Copy link
Contributor

Choose a reason for hiding this comment

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

is this script ever used? storing it in .github implies that it is used in CI, but it is not the case

Copy link
Contributor Author

@kelemeno kelemeno Oct 4, 2024

Choose a reason for hiding this comment

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

the contracts commit used here is not the latest one from the branch in contracts

its just system contract hashes, did not bump for that.

is this script ever used? storing it in .github implies that it is used in CI, but it is not the case

I use it for local setup, testing, yes probably this is not the right place, I'll put it in ./bin ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you discuss it with @Deniallugo? I just don't feel this is something we want to leave committed forever, if it is something we will delete before merging to main, then okay let's keep it here, but it maybe it is helpful for the platform team/toolbox in general

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We already have things like this in sync-layer-stable, but yes will discuss:

https://github.com/matter-labs/zksync-era/blob/sync-layer-stable/bin/start_sync_layer_l3_chain.sh

Copy link
Contributor Author

Choose a reason for hiding this comment

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

He is OOO today, so I messaged him, but I think we can merge before he replies

Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need such scripts?
actually zk_supervisor clean containers already removing volumes and adding one another binary for it seems inappropriate

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tbh I think it is very similar to zki e init --dev . The difference is that it includes GW.

We can either have it as a bash script, or include it into zki e init --dev .

core/bin/zksync_server/src/main.rs Outdated Show resolved Hide resolved
core/lib/config/src/configs/gateway.rs Outdated Show resolved Hide resolved
core/tests/ts-integration/tests/l2-erc20.test.ts Outdated Show resolved Hide resolved
core/tests/ts-integration/tests/l2-erc20.test.ts Outdated Show resolved Hide resolved
core/tests/ts-integration/tests/l2-erc20.test.ts Outdated Show resolved Hide resolved
zk_toolbox/crates/common/src/withdraw.rs Outdated Show resolved Hide resolved
zk_toolbox/crates/common/src/withdraw.rs Show resolved Hide resolved
core/tests/ts-integration/tests/l2-erc20.test.ts Outdated Show resolved Hide resolved
@kelemeno kelemeno merged commit dfe03d3 into sync-layer-stable Oct 8, 2024
33 checks passed
@kelemeno kelemeno deleted the kl/l2-native-token-support branch October 8, 2024 09:56
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.

4 participants