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

Update local-deployment.md with PP config #2486

Merged
merged 4 commits into from
Nov 20, 2024

Conversation

vcastellm
Copy link
Collaborator

@vcastellm vcastellm commented Nov 12, 2024

Update doc and specify how to PP chain

Update doc and especify how to PP chain
Copy link
Collaborator

@mitchpolygon mitchpolygon left a comment

Choose a reason for hiding this comment

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

@vcastellm Overall it looks good, can you make the following changes please:

  1. The system requirements and instructions are different between the PP and FEP local deployment right?
  2. Therefore, within the navigation we will need both the FEP and PP local deployment guides
  3. Instead of updating the existing file, can you create a new file titled "Pessimistic proofs (PP) guide"
  4. Update the existing file to be titled "Full execution proofs (FEP) guide"
  5. Nest both of these guides under the "Local deployment guide" navigation section

@vcastellm
Copy link
Collaborator Author

vcastellm commented Nov 13, 2024

@vcastellm Overall it looks good, can you make the following changes please:

  1. The system requirements and instructions are different between the PP and FEP local deployment right?

No, system requirements are the same for both, same for Kurtosis.

  1. Therefore, within the navigation we will need both the FEP and PP local deployment guides
  2. Instead of updating the existing file, can you create a new file titled "Pessimistic proofs (PP) guide"
  3. Update the existing file to be titled "Full execution proofs (FEP) guide"
  4. Nest both of these guides under the "Local deployment guide" navigation section

Given that the differences are minimal, IMO splitting them in different sections would create a lot of duplication.

Generally, our goal in CDK and in DevTools is to provide users with something that's super easy to use, ideally it should be a one-liner to run the stack, therefore the changes in the document are minimal.

That would create a 98% duplicated documents, I think it doesn't makes sense to artificially create 2 different documents.

I wanted to be more explicit on the configuration doc but we can come up with 2 one-liners for each path:

PP

kurtosis run --enclave cdk --args-file "https://raw.githubusercontent.com/0xPolygon/kurtosis-cdk/refs/heads/main/.github/tests/fork12-pessimistic.yml" github.com/0xPolygon/kurtosis-cdk

FEP

kurtosis run --enclave cdk github.com/0xPolygon/kurtosis-cdk

@vcastellm
Copy link
Collaborator Author

@mitchpolygon I added the oneliners to a new section in the doc called: "Quick setup" that should be the super easy setup process for lazy people, the rest of the document is more detailed setup for curious people.


### Pesimistic Proof

kurtosis run --enclave cdk --args-file "https://raw.githubusercontent.com/0xPolygon/kurtosis-cdk/refs/heads/main/.github/tests/fork12-pessimistic.yml" github.com/0xPolygon/kurtosis-cdk
Copy link
Member

Choose a reason for hiding this comment

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

You might want to set a verision tag or commit hash to future proof against refactors. this would be need both for the args file and for the selector

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is even temporary until the release, because it's going to be moved most prob.?

Copy link
Member

@leovct leovct left a comment

Choose a reason for hiding this comment

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

lgtm

@mitchpolygon mitchpolygon marked this pull request as ready for review November 15, 2024 13:42
@vcastellm vcastellm marked this pull request as draft November 18, 2024 09:48
@mitchpolygon mitchpolygon marked this pull request as ready for review November 20, 2024 14:07
@mitchpolygon mitchpolygon merged commit ec3b0aa into main Nov 20, 2024
2 checks passed
@mitchpolygon mitchpolygon deleted the CDK-554-local-deployment-guide-pp-specifics branch November 20, 2024 14:25
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