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: new vitepress site #197

Merged
merged 16 commits into from
Aug 8, 2023
Merged

feat: new vitepress site #197

merged 16 commits into from
Aug 8, 2023

Conversation

jcstein
Copy link
Member

@jcstein jcstein commented Jul 28, 2023

Overview

Preview at https://jcstein.github.io/rollkit-vitepress/
resolves #196
resolves #48

Checklist

  • New and updated code has appropriate documentation
  • New and updated code has new and/or updated testing
  • Required CI checks are passing
  • Visual proof for any user facing features like CLI or documentation updates
  • Linked issues closed with keywords

@jcstein
Copy link
Member Author

jcstein commented Jul 28, 2023

oh it's because the old workflows are being run, not current

have tested on my branch and it works https://github.com/jcstein/rollkit-vitepress/actions/runs/5697896798

https://jcstein.github.io/rollkit-vitepress/

@jcstein jcstein self-assigned this Jul 28, 2023
@jcstein jcstein added documentation Improvements or additions to documentation enhancement New feature or request labels Jul 28, 2023
@jcstein
Copy link
Member Author

jcstein commented Jul 31, 2023

swapped to yarn and tested on my version successfully: https://github.com/jcstein/rollkit-vitepress/actions/runs/5714578601

@github-actions
Copy link
Contributor

PR Preview Action v1.4.4
🚀 Deployed preview to https://rollkit.github.io/docs/pr-preview/pr-197/
on branch gh-pages at 2023-07-31 12:50 UTC

@jcstein jcstein requested a review from nashqueue July 31, 2023 12:56
@jcstein jcstein marked this pull request as ready for review July 31, 2023 12:56
@jcstein jcstein requested a review from MSevey July 31, 2023 16:38
@jcstein
Copy link
Member Author

jcstein commented Jul 31, 2023

  • look into .vitepress/cache/** files and remove if not necessary

@Manav-Aggarwal
Copy link
Member

Thanks for working on this! Looks great so far.

Some initial comments:

  • Under the Why Rollkit? section under Customizability, maybe we can emphasize on the four main components we aim to customize - Execution Environments, Data Availability layers, Proof System, and Sequencer Schemes.

  • Under the Rollkit Stack section, we mention the following paragraph:

And don't forget to replace another dependency, tendermint, with rollkit/tendermint, which has an enhanced ABCI interface that includes the methods needed for state fraud proofs.

This was removed recently from main, can we please update this with a cometbft version that works with us? This requires a new release. cc: @gupadhyaya

  • In the same section under Data Availability, we mention the following paragraph:

The DataAvailabilityLayerClient interface includes essential lifecycle methods (Init, Start, Stop) as well as data availability methods (SubmitBlock, CheckBlockAvailability).

We recently got rid of the CheckBlockAvailability method from the interface and updated SubmitBlock to SubmitBlocks. Suggestion to update paragraph to:

The DataAvailabilityLayerClient interface includes essential lifecycle methods (Init, Start, Stop) as well as data availability methods (SubmitBlocks, RetrieveBlocks).

  • In the same section we describe how the DA implementation for Celestia works. We recently updated Rollkit to use celestia-openrpc instead of go-cnc so reflecting those changes here would make sense. Also, mentioning other existing DA implementations like Bitcoin, Mock, gRPC could make sense here too.

  • Comment out the following paragraph for now since we've removed State Fraud Proofs from the main for now.

Receive state fraud proofs: state fraud proofs are received through a channel FraudProofInCh and Rollkit nodes attempt to verify them. Note that we plan to make this configurable for full nodes since full nodes also produce state fraud proofs on their own.

We can uncomment/add it back once we are ready to add State Fraud Proofs back.

  • Perhaps we should modify references to Tendermint to CometBFT instead now that Rollkit uses CometBFT.

  • The link to the Fraud Proofs ADR under Optimistic Rollups is currently broken and can be updated to point to this link instead.

  • Under the Building with Rollkit section, we should update the rollkit and cometbft versions to new releases without State Fraud Proofs.

I'll also go through all the tutorials and follow up with any additional comments.

@Manav-Aggarwal Manav-Aggarwal self-requested a review August 1, 2023 03:04
@jcstein
Copy link
Member Author

jcstein commented Aug 1, 2023

thanks for the feedback @Manav-Aggarwal !

And don't forget to replace another dependency, tendermint, with rollkit/tendermint, which has an enhanced ABCI interface that includes the methods needed for state fraud proofs.
This was removed recently from main, can we please update this with a cometbft version that works with us? This requires a new release. cc: @gupadhyaya

  • I implemented all changes except for the above. I think that we can merge docs and update this later, as the docs in this PR are mirroring the current site. note: since the tutorials still use tendermint, i did not modify the tutorials and have opened a new issue for this in change tendermint -> cometbft in tutorials and across docs #199

Also, mentioning other existing DA implementations like Bitcoin, Mock, gRPC could make sense here too.

  • added this as plain text with no links for now. what's the best place to link to these examples?

Under the Building with Rollkit section, we should update the rollkit and cometbft versions to new releases without State Fraud Proofs.

is any of this blocking merging docs and then making the remaining changes in this comment after?

@jcstein jcstein requested a review from YazzyYaz August 1, 2023 16:20
@jcstein
Copy link
Member Author

jcstein commented Aug 1, 2023

note: the preview built on my jcstein.github.io page has "edit this page" linking to edit pages on docs (that don't exist yet), but will work once this is live

@nashqueue
Copy link
Member

Thank you Josh , I agree that we should merge the migrations and make any adjustments after to not block this.

@Manav-Aggarwal
Copy link
Member

I think the issue you linked should cover everything, thanks! Approved.

@MSevey MSevey merged commit e40103a into main Aug 8, 2023
2 checks passed
@MSevey MSevey deleted the jcs/new-site branch August 8, 2023 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update link to State Fraud Proofs ADR on website add draft diagrams to docs
4 participants