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

Fork 12 and Deployment Refactor #266

Merged
merged 36 commits into from
Sep 23, 2024
Merged

Conversation

praetoriansentry
Copy link
Collaborator

@praetoriansentry praetoriansentry commented Sep 16, 2024

Description

This PR has a refactor of the logic for deploying l1 / l2 contracts along with some changes to the default images and forks. In particular, we're switching to fork 12 by default now.

Changes:

  • Linting is defaulted to sort parameters by default now so that order doesn't have to match
  • Image updates to the latest versions for erigon, node, dac, prover, bridge
  • Adding a flag to support using a real verifier
  • Deploying the pool manager before the rpc since the rpc depends on the pool manager
  • Adding the beginnings of a sanity check script to help with checking the status of our images
  • Simple bridge UI fixes related to changes in metamask behavior

Future work:

  • Improvements to the sanity check script
  • Changing to not deploy the mock prover if a real verifier is used
  • So mechanism to update the agglayer when a new cdk is attached

@praetoriansentry praetoriansentry marked this pull request as ready for review September 20, 2024 20:54
@praetoriansentry praetoriansentry changed the title Pool Manager Change Fork 12 and Deployment Refactor Sep 20, 2024
now that we have some visibility into stop service, it's a bit more
clear that the tx spammer is not functional. All of our tests failed
due to this check.

```
printing logs for tx-spammer-001--9a1d7d2bf172420ab8f1a9474aecf2f7
/usr/local/bin/spam.sh: line 14: cast: command not found
Deploying an ERC20 token...
```
Copy link
Collaborator

@jhkimqd jhkimqd left a comment

Choose a reason for hiding this comment

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

+1 lgtm, tested locally and tried attaching multiple CDKs as well. I think there may be some issues with certain components, but I think we should merge this first and raise additional PRs for those as we fix them.

@praetoriansentry praetoriansentry merged commit 8c060a5 into main Sep 23, 2024
18 checks passed
@praetoriansentry praetoriansentry deleted the jhilliard/version-updates branch September 23, 2024 02:06
@@ -9,13 +9,13 @@ PARAMS_YML_PATH="../../params.yml"

# Extracting default parameters from the different files.
echo "Extracting default parameters from input_parser.star..."
if ! sed -n '/^DEFAULT_ARGS = {/,/^}/ { s/DEFAULT_ARGS = //; s/}/}/; p; }' "$INPUT_PARSER_PATH" | yq --yaml-output >.input_parser.star; then
if ! sed -n '/^DEFAULT_ARGS = {/,/^}/ { s/DEFAULT_ARGS = //; s/}/}/; p; }' "$INPUT_PARSER_PATH" | yq -S --yaml-output >.input_parser.star; then
Copy link
Member

Choose a reason for hiding this comment

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

thx for the fix 🙏

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