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

fix(Validium): Remove obscure use of VALIDIUM_MODE env #1242

Conversation

ilitteri
Copy link
Member

@ilitteri ilitteri commented Feb 23, 2024

Disclaimer ⚠️

This PR has a counterpart in the era-contracts repo. The PR in question is matter-labs/era-contracts#225. The era-contracts submodule is pointing to said PR's branch. Before merging this PR, after it is approved and the era-contracts counterpart is merged, we need to update this PR's submodule commit.

What ❔

This PR removes the use of VALIDIUM_MODE env for the contracts side and replaces it with the passthrough of the flag to the l1-contracts deploy script.

Why ❔

zk tool is setting VALIDIUM_MODE environment variable, which is only used by a script in a different repository (era-contracts). This is a very implicit and hard to track approach.

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.
  • Spellcheck has been run via zk spellcheck.
  • Linkcheck has been run via zk linkcheck.

@ilitteri ilitteri requested a review from popzxc February 28, 2024 19:45
Copy link
Member

@popzxc popzxc left a comment

Choose a reason for hiding this comment

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

Nice!

@ilitteri ilitteri merged commit 51875a3 into matter-labs:feat_validium_pubdata_abstraction Feb 29, 2024
34 of 36 checks passed
@ilitteri ilitteri deleted the feat_remove_validium_mode_env_usage branch February 29, 2024 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants