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

1176 - Migrate from other chains #1178

Merged
merged 36 commits into from
Sep 15, 2023
Merged

Conversation

KMCreatesWorlds
Copy link
Collaborator

@KMCreatesWorlds KMCreatesWorlds commented Jun 26, 2023

What does this PR fix/introduce?

This PR introduces the overview of how the move from some blockchains can be made to Casper.

Additional context

The PR gives insight into Ethereum, Near, Solana, and Aptos and how they compare to Casper regarding aspects such as Contract Lifecycle, State Management, or how to interact with the Contract.

This is not a comprehensive explanation of how the migration of a Contract for example on Ethereum can be made on Casper, but instead gives high-level hints on how to approach the individual aspects to make the move to Casper easier.

Checklist

  • Docs are successfully building - yarn install && yarn run build.
  • For new internal links I used relative file paths (with .md extension) - e.g. ../../faq/faq-general.md - instead of introducing absolute file path, or relative/absolute URL.
  • All external links have been verified with yarn run check:externals.

Reviewers

@caspermel

@KMCreatesWorlds KMCreatesWorlds linked an issue Jun 26, 2023 that may be closed by this pull request
@KMCreatesWorlds KMCreatesWorlds added the TT new content New content created by Tiger Team label Jun 26, 2023
@ACStone-MTS ACStone-MTS requested a review from ipopescu June 28, 2023 16:44
@ipopescu ipopescu mentioned this pull request Jul 4, 2023
4 tasks
Copy link
Collaborator

@ipopescu ipopescu left a comment

Choose a reason for hiding this comment

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

Thanks for putting this together! Not an easy document to write, so it needs a bit more polishing.

I have submitted a thorough editing pass with PR #1183. Please take a look at this first. After reviewing my updates, please rename the file to moving-to-casper.md to remove any ambiguity with the word “migrate”.

In addition, I have a few more requests.

Please try to remove repeated explanations in the five categories you defined (lifecycle, storage, methods etc.). For example, we are referencing Named Keys and Dictionaries three times. Resources are defined twice.

State management can be combined with storage, actually. If you want to keep them separate, try to find another way to organize the content so there is less duplication.

I would like to see more links to coding examples. There are some, but many sections seem theoretical. You could link to any relevant topics (either in the Casper docs or externally).
For example, when explaining get_key, at a minimum, link to docs.rs. Also, point to the reading-and-writing-to-the-blockchain.md file.
This request applies to all concepts and explanations introduced.

Finally, a little more structure would be needed. For example, what are the comparison points in each tab? Are they covered for each blockchain?

Please also check the Casper docs style guidelines before submitting a PR. It's not in this PR's checklist, but it's not optional.

source/docs/casper/resources/migrate.md Outdated Show resolved Hide resolved
source/docs/casper/resources/migrate.md Outdated Show resolved Hide resolved
source/docs/casper/resources/migrate.md Outdated Show resolved Hide resolved
source/docs/casper/resources/migrate.md Outdated Show resolved Hide resolved
source/docs/casper/resources/migrate.md Outdated Show resolved Hide resolved
source/docs/casper/resources/migrate.md Outdated Show resolved Hide resolved
source/docs/casper/resources/migrate.md Outdated Show resolved Hide resolved
source/docs/casper/resources/migrate.md Outdated Show resolved Hide resolved
source/docs/casper/resources/migrate.md Outdated Show resolved Hide resolved
@ipopescu ipopescu requested a review from a user July 4, 2023 11:17
ipopescu and others added 4 commits July 6, 2023 17:01
Co-authored-by: Adam Stone <[email protected]>
Co-authored-by: Adam Stone <[email protected]>
Co-authored-by: Adam Stone <[email protected]>
@ghost ghost mentioned this pull request Jul 17, 2023
4 tasks
@KMCreatesWorlds KMCreatesWorlds requested a review from ipopescu July 24, 2023 15:38
@ipopescu
Copy link
Collaborator

@KMCreatesWorlds @andrzej-casper Acknowledging the review request.

@ipopescu
Copy link
Collaborator

ipopescu commented Aug 8, 2023

@KMCreatesWorlds, please address or respond to my requests from the previous review feedback. The document still needs some polishing. Summarizing my previous feedback here:

  • rename the file to moving-to-casper.md to remove ambiguity with the word “migrate”
  • try to remove repeated explanations in the five categories you defined (lifecycle, storage, methods, etc.). For example, we are referencing Named Keys and Dictionaries three times. Resources are defined twice.
  • combine state management with storage; or try to find another way to remove duplication
  • add cross-links to each topic in the Casper docs or link out to coding examples when introducing a concept/feature
  • try to add the same structure in each tab. What are the comparison points in each tab? Are they covered for each blockchain?

cc @andrzej-casper @melpadden

@Adrian-Wrona
Copy link
Contributor

@KMCreatesWorlds could you take a look at @ipopescu review feedback?

@github-actions
Copy link

github-actions bot commented Sep 13, 2023

Copy link
Collaborator

@ipopescu ipopescu left a comment

Choose a reason for hiding this comment

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

Thanks for the updates, @KMCreatesWorlds.
Here are a few minor edit requests. From the previous feedback, I see the following have been addressed - thanks!

  • Renamed file to moving-to-casper.md
  • Combined state management with storage
  • Removed repeated definitions (Resouces, Named Keys, Dictionaries)

Let's discuss the following on the live call:

  • Cross-links to each topic in the Casper docs or link out to coding examples
  • Tab structure. What are the comparison points in each tab? Are they covered for each blockchain?

cc @ACStoneCL

source/docs/casper/resources/moving-to-casper.md Outdated Show resolved Hide resolved
source/docs/casper/resources/moving-to-casper.md Outdated Show resolved Hide resolved
source/docs/casper/resources/moving-to-casper.md Outdated Show resolved Hide resolved
source/docs/casper/resources/moving-to-casper.md Outdated Show resolved Hide resolved
source/docs/casper/resources/moving-to-casper.md Outdated Show resolved Hide resolved
source/docs/casper/resources/moving-to-casper.md Outdated Show resolved Hide resolved
source/docs/casper/resources/moving-to-casper.md Outdated Show resolved Hide resolved
source/docs/casper/resources/moving-to-casper.md Outdated Show resolved Hide resolved
@@ -281,6 +281,7 @@ module.exports = {
"resources/build-on-casper",
"resources/casper-open-source-software",
"resources/quick-start",
"resources/moving-to-casper",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would move this higher, perhaps after build-on-casper.
Also, the table on the resouces/index.md page needs to be updated with this topic (the table matches the left nav order).

@ipopescu
Copy link
Collaborator

Let's discuss the following on the live call:

  • Cross-links to each topic in the Casper docs or link out to coding examples
  • Tab structure. What are the comparison points in each tab? Are they covered for each blockchain?

cc @ACStoneCL

Karol and I spoke and we are on the same page. We have cross-links for Casper, but we won't add links on the other tabs. Developers coming from other protocols should already be familiar with them.
Finally, the tabs have structure, but not every topic is a 1-to-1 mapping. This is ok.

@ipopescu ipopescu self-requested a review September 15, 2023 12:19
Copy link
Collaborator

@ipopescu ipopescu left a comment

Choose a reason for hiding this comment

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

I ran this locally and it looks great. Thank you, @KMCreatesWorlds!

Screenshot 2023-09-15 at 14 27 31

@KMCreatesWorlds KMCreatesWorlds merged commit bb0ddd2 into dev Sep 15, 2023
16 checks passed
@KMCreatesWorlds KMCreatesWorlds deleted the 1176-migrate-from-other-chains branch September 15, 2023 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TT new content New content created by Tiger Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate from other chains
3 participants