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

Make compatible with Polkadot, Kusama and Rococo #217

Open
wants to merge 80 commits into
base: ilya/parachain-update
Choose a base branch
from

Conversation

ayushmishra2005
Copy link
Contributor

@ayushmishra2005 ayushmishra2005 commented Aug 9, 2021

resolves #216

This PR contains the changes to update parachain code to be compatible for Kusama.

  • 1. - update only codebase to be compatible with Kusama version ... based on confirmation from the community

  • 1.1. - Upgrade to Polkadot 0.9.10

  • 1.2. - Upgrade to Polkadot 0.9.11

  • 1.3. - Upgrade to Polkadot 0.9.12

  • 2.1. - testing has been performed but limited to building the code and executing test-cases including .

  • 2.2. - removed use of rustfmt to be consistent with substrate-parachain-template and DataHighway-DHX/node 'master' branch to maintain ease of developability

  • 2.3. - test against polkadot (not westend) running in --chain <dev|local|staging> using polkadot-launch

  • 2.4. - Test registering parachain locally before attempting to connect to Rococo!

  • 3.1.1 - requested rococo tokens

  • 3.1.2 - register paraId on Rococo testnet

  • 3.1.3 - other relevant testing on Rococo to check that our desired parachain functionality works on a parachain testnet

  • 3.2 - optionally repeat 3.1.1 - 3.1.4 for Westend

  • 4. - repeat 3.1.1 - 3.1.4 for Polkadot

  • 5. request to merge update into 'ilya/parachain'

node/src/chain_spec.rs Outdated Show resolved Hide resolved
[RelayChainCli::executable_name().to_string()].iter().chain(cli.relaychain_args.iter()),
);

let id = ParaId::from(cli.run.parachain_id.or(para_id).unwrap_or(200));
let id = ParaId::from(cli.run.parachain_id.or(para_id).unwrap_or(2000));
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we need to specifically request this Parachain ID?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

@ltfschoen ltfschoen Aug 16, 2021

Choose a reason for hiding this comment

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

well this PR is for our parachain that connects to Kusama, so i'd imagine the default parachain id that it'd fallback to in the cli would be our DataHighway parachain id. so far it displays the live Kusama parachain id's that are being used here https://polkadot.js.org/apps/?rpc=wss%3A%2F%2Fkusama.api.onfinality.io%2Fpublic-ws#/parachains, and it looks like 2000 has already been taken by Karura, so i think we should use expect("Missing ParaId"). i don't know why they switched to a fallback to the Karura parachain id in what's supposed to be a "template".

i'd suggest finding out how to register a parachain id in the Parachain Technical room on Matrix.

Copy link
Collaborator

@ltfschoen ltfschoen Aug 16, 2021

Choose a reason for hiding this comment

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

it appears that para id 2000 may be Rococo-specific, according to https://substrate.dev/cumulus-workshop/#/en/3-parachains/2-register, please double check in Parachain Technical room on Matrix if it applies to Kusama too.
Note that in the README it says "When registering to Rococo, ensure you set a ParaId > 1000, below 1000 is meant for system parachains." https://github.com/substrate-developer-hub/substrate-parachain-template/tree/40b858149b212e493da08d80e0fc5b06a6b0b72d#register-on-the-relay-with-sudo, so please find out what rules apply when registering to Kusama

Copy link
Collaborator

@ltfschoen ltfschoen Aug 16, 2021

Choose a reason for hiding this comment

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

please find out how to reserve a parachain id as specified here https://substrate.dev/cumulus-workshop/#/en/3-parachains/2-register?id=registration-transaction. it has a link to "paraID being reserved" but it returns 404, so i'd suggest asking in Parachain Technical room on Matrix how to do it and then create a PR to fix the broken link in their docs.

it also mentions how to do it here in the README of the 'master' branch https://github.com/substrate-developer-hub/substrate-parachain-template/tree/40b858149b212e493da08d80e0fc5b06a6b0b72d#register-on-the-relay-with-sudo, did you try registering on a relay chain to test it like they mention?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

@ltfschoen ltfschoen left a comment

Choose a reason for hiding this comment

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

minor comments

node/src/command.rs Outdated Show resolved Hide resolved
@sheenhx
Copy link

sheenhx commented Aug 26, 2021

any idea we can allow the users in Supernodes to also participate in governance ?

node/src/chain_spec.rs Outdated Show resolved Hide resolved
node/src/chain_spec.rs Outdated Show resolved Hide resolved
node/src/chain_spec.rs Outdated Show resolved Hide resolved
node/src/chain_spec.rs Outdated Show resolved Hide resolved
node/src/chain_spec.rs Outdated Show resolved Hide resolved
node/src/chain_spec.rs Outdated Show resolved Hide resolved
node/src/chain_spec.rs Outdated Show resolved Hide resolved
node/src/chain_spec.rs Outdated Show resolved Hide resolved
node/src/chain_spec.rs Outdated Show resolved Hide resolved
node/src/chain_spec.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@ltfschoen ltfschoen left a comment

Choose a reason for hiding this comment

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

  • - in rococo_development_config function, change name from "Development" to "DataHighway Rococo Development Testnet", and id from "dev" to "datahighway-rococo-dev"

  • - in rococo_local_testnet_config function, change name from "Local Testnet" to "DataHighway Rococo Local Testnet", and id from "local_testnet" to "datahighway-rococo-local" (horizontal dash rather than an underscore)

  • - in chachacha_development_config function, change name from "Development" to "DataHighway ChaChaCha Development Testnet", and id from "dev" to "datahighway-chachacha-dev"

  • - in chachacha_local_testnet_config function, change name from "Development" to "DataHighway ChaChaCha Local Testnet", and id from "local_testnet" to "datahighway-chachacha-local" (horizontal dash rather than an underscore)

  • - in rococo_parachain_config function, change name from "DataHighway Spreehafen Parachain Testnet" to "DataHighway Spreehafen Rococo Parachain Testnet", and id from "datahighway_spreehafen" to "datahighway-spreehafen-rococo-parachain-testnet" (horizontal dash rather than an underscore)

  • - in chachacha_parachain_config function, change name from "DataHighway Spreehafen Parachain Testnet" to "DataHighway Spreehafen ChaChaCha Parachain Testnet", and id from "datahighway_spreehafen" to "datahighway-spreehafen-chachacha-parachain-testnet" (horizontal dash rather than an underscore)

  • - in westend_development_config function, change name from "Development" to "DataHighway Westend Development Testnet", and id from "dev" to "datahighway-westend-dev"

  • - in westend_local_testnet_config function, change name from "Local Testnet" to "DataHighway Westend Local Testnet", and id from "local_testnet" to "datahighway-westend-local" (horizontal dash rather than an underscore)

  • - in polkadot_development_config function, change name from "Development" to "DataHighway Polkadot Development Testnet", and id from "dev" to "datahighway-polkadot-dev"

  • - in polkadot_local_testnet_config function, change name from "Local Testnet" to "DataHighway Polkadot Local Testnet", and id from "local_testnet" to "datahighway-polkadot-local" (horizontal dash rather than an underscore)

  • - in westend_parachain_config function, change name from "DataHighway Baikal Parachain Testnet" to "DataHighway Baikal Westend Parachain Testnet", change id from "datahighway_baikal" to "datahighway-baikal-westend-parachain-testnet" (horizontal dash rather than an underscore)

  • - in westend_parachain_config function, i think we should use a different token other than DHX, since the token on Westend is supposed to be valueless.
    On some parachains they actually have a faucet for their parachain testnet and give users the opportunity to win NFTs if they have over a certain amount of their parachain testnet tokens

    • Update 26 Oct - Luke asked team internally about their thoughts on this
    • Update 27 Oct - Ayush - I took follow up on same thread and suggested token name based on testnet name
    • Update 27 Oct - Ayush - After getting confirmation on discord, we changed it to BKL token
  • - in westend_parachain_config function, the relay chain is not "chachacha", its "westend"

  • - in polkadot_parachain_config function, change name from "DataHighway Tanganika Parachain Testnet" to "DataHighway Tanganika Polkadot Parachain", and id from "datahighway_tanganika" to "datahighway-tanganika-polkadot-parachain" (horizontal dash rather than an underscore)

  • - in polkadot_parachain_config function, the relay chain is not "chachacha", its "polkadot"

  • - "in node/src/command.rs please check that all the CLI commands work"

    • Update 26 Oct:
    • - running the following:
     cargo build --release
     ./target/release/datahighway-collator --alice --collator --tmp --chain dev
    

    gives error:

     Error: Input("Relay chain argument error: Invalid input: `rococo-dev` only supported with `rococo-native` feature enabled.")
    
      - Update 1 Nov: Luke - Done based on feedback obtained by Ayush below
    
    • - running the following:
     cargo build --release
     ./target/release/datahighway-collator --alice --collator --tmp --chain rococo-local
    

    gives error:

     Error: Input("Relay chain argument error: Invalid input: `rococo-local` only supported with `rococo-native` feature enabled.")
    

    We can either use preconfigured files from here https://docs.substrate.io/tutorials/v3/cumulus/start-relay/#pre-configured-chain-spec-files or we can build using these steps https://docs.substrate.io/tutorials/v3/cumulus/start-relay/#generate-a-plain-chain-spec for rococo-dev and rococo-local

    I downloaded https://docs.substrate.io/assets/tutorials/cumulus/chain-specs/rococo-custom-2-raw.json and executed this command

     ./target/release/datahighway-collator --alice --collator --tmp -- --execution wasm --chain rococo-custom-2-raw.json
    

    Its working fine.

    • Update 29th Oct Ayush: @ltfschoen I didn't get answer from the community yet. However, I continued my investigation.

    For rococo-dev, rococo-local and rococo, we have to specify the spec path in chain argument. For rococo-dev and rococo-local, as I mentioned above We can either use preconfigured files from here https://docs.substrate.io/tutorials/v3/cumulus/start-relay/#pre-configured-chain-spec-files or we can build using these steps https://docs.substrate.io/tutorials/v3/cumulus/start-relay/#generate-a-plain-chain-spec for rococo-dev and rococo-local.

    I tried this on local and connected parachain. Parachain is able to produce blocks.

    For rococo, we can use https://raw.githubusercontent.com/paritytech/polkadot/v0.9.10/node/service/res/rococo.json. It is also working fine.

    • - running the following:
     cargo build --release
     ./target/release/datahighway-collator --alice --collator --tmp --chain chachacha-dev
    

    gives error:

     Error: Input("Relay chain argument error: Invalid input: Error opening spec file: No such file or directory (os error 2)")
    
      - Update 1 Nov: Luke - Done based on feedback obtained by Ayush below
    
    • - running the following:
     cargo build --release
     ./target/release/datahighway-collator --alice --collator --tmp --chain chachacha-local
    

    gives error:

     Error: Input("Relay chain argument error: Invalid input: Error opening spec file: No such file or directory (os error 2)")
    
      - Update 1 Nov: Luke - Done based on feedback obtained by Ayush below - if ChaChaCha community doesn't know we'll just use rococo-local instead of chachacha-local until they respond
    
    • - running the following:
     cargo build --release
     ./target/release/datahighway-collator --alice --collator --tmp --chain rococo
    

    successfully runs a node, but why does it mention Babe when we're using Aura instead? are empty Babe epoch changes necessary?

     Creating empty BABE epoch changes on what appears to be first startup.
    
    • - running the following:
     cargo build --release
     ./target/release/datahighway-collator --alice --collator --tmp --chain chachacha
    

    gives error:

     Error: Input("Relay chain argument error: Invalid input: Error opening spec file: No such file or directory (os error 2)")
    
    • Update 1st Nov Luke: closing on the assumption that we just "have to specify the spec path in chain as argument" as mentioned above by Ayush, "use preconfigured files" or "build ..." using the steps specified. Or use rococo otherwise
    • - running the following:
     cargo build --release
     ./target/release/datahighway-collator --alice --collator --tmp --chain westend-dev
    

    gives error:

     Error: Input("Relay chain argument error: Invalid input: `westend-dev` only supported with `westend-native` feature enabled.")
    
    • Update 1 Nov: Luke - Done based on feedback obtained by Ayush
    • - running the following:
     cargo build --release
     ./target/release/datahighway-collator --alice --collator --tmp --chain westend-local
    

    gives error:

     Error: Input("Relay chain argument error: Invalid input: `westend-local` only supported with `westend-native` feature enabled.")
    
    • Update 1 Nov: Luke - Done based on feedback obtained by Ayush
    • - running the following:
     cargo build --release
     ./target/release/datahighway-collator --alice --collator --tmp --chain westend
    

    successfully runs a node, but why does it mention Babe when we're using Aura instead? are empty Babe epoch changes necessary?

     Creating empty BABE epoch changes on what appears to be first startup.
    
    • - running the following:
     cargo build --release
     ./target/release/datahighway-collator --alice --collator --tmp --chain polkadot-dev
    

    successfully runs a node, but why does it mention Babe when we're using Aura instead? are empty Babe epoch changes necessary?

     Creating empty BABE epoch changes on what appears to be first startup.
    
    • - running the following:
     cargo build --release
     ./target/release/datahighway-collator --alice --collator --tmp --chain polkadot-local
    

    successfully runs a node, but why does it mention Babe when we're using Aura instead? are empty Babe epoch changes necessary?

     Creating empty BABE epoch changes on what appears to be first startup.
    
    • - running the following:
     cargo build --release
     ./target/release/datahighway-collator --alice --collator --tmp --chain polkadot
    

    successfully syncs with polkadot relay chain, but why does it mention Babe when we're using Aura instead? are empty Babe epoch changes necessary?

     Creating empty BABE epoch changes on what appears to be first startup.
    
  • Answer by Ayush: 26th Oct - @ltfschoen I have shared keys again. New keys have already been updated
  • - Added 26 Oct - datahighway_chachacha_parachain_config function hasn't been updated with the new keys in (i.e. authority keys, sudo key, endowed accounts) that you shared for rococo (assuming we're using the same keys for rococo and chachacha)
  • Answer by Ayush: 26th Oct - @ltfschoen I have shared keys again. New keys have already been updated
  • Update 1 Nov Luke: checked all latest keys that were added match those that we have secretly stored

@ltfschoen
Copy link
Collaborator

ltfschoen commented Nov 1, 2021

  • Rococo

@ayushmishra2005

  • please update the title to mention that you have made this compatible with other relay chains like Polkadot, Rococo, etc
  • please update this initial checklist Make compatible with Polkadot, Kusama and Rococo #217 (comment) once you have finished 3.2, 4 (add an additional step for testing on Polkadot), then we can merge per dot point 5 into 'ilya/parachain'

@ayushmishra2005 ayushmishra2005 changed the title Make compatible with kusama Make compatible with Polkadot, Kusama and Rococo Nov 1, 2021
@ayushmishra2005
Copy link
Contributor Author

  • Rococo

@ayushmishra2005

  • please update the title to mention that you have made this compatible with other relay chains like Polkadot, Rococo, etc
  • please update this initial checklist Make compatible with kusama #217 (comment) once you have finished 3.2, 4 (add an additional step for testing on Polkadot), then we can merge per dot point 5 into 'ilya/parachain'

@ltfschoen I have updated the checklist based on recent discussion with community.

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