-
Notifications
You must be signed in to change notification settings - Fork 379
Conversation
It looks like the CI issue is the same as mentioned here: #605 (comment). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good so far. I just think that the README could get easily outdated so I prefer a simpler version here.
Co-authored-by: Ricardo Rius <9488369+riusricardo@users.noreply.github.com>
Co-authored-by: Ricardo Rius <9488369+riusricardo@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found some nitpicks, looks good besides that!
parachain-template/node/src/lib.rs
Outdated
@@ -0,0 +1,3 @@ | |||
pub mod chain_spec; | |||
mod rpc; | |||
pub mod service; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this file is not used anywhere, for Substrate's node-template
I removed it here.
"flags": [ | ||
"--force-authoring", | ||
"--", | ||
"--execution=wasm" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we actually need to specify the execution engine?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, that value is passed to the polkadot full node in the collator node and it is required to make it always follow the latest relay chain logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know if there's a reason why this value is not the default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No idea, maybe because in the future we want to decouple the polkadot node from the collator giving the user more flexibility. But I'm guessing here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You only need this because we don't use strict spec_version bumps etc. It is also important that you use the same commit in polkadot as you use in your template from polkadot to have the exact same native runtime.
As we can not ensure this, running everything in wasm is more safe.
parathreads [here](https://wiki.polkadot.network/docs/learn-parathreads). | ||
|
||
To learn about how to actually use the template to hack together your own parachain check out the | ||
`README` from the [`substrate-parachain-template` repository](https://github.com/substrate-developer-hub/substrate-parachain-template/). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about adding a note here explaining that the template is geared towards Rococo (and thus ROC
) and an explanatory link for Rococo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Co-authored-by: Michael Müller <michi@parity.io>
Co-authored-by: Michael Müller <michi@parity.io>
@bkchr do you want to take a quick look at this, or should we go ahead and merge it? |
This stems from the discussions in paritytech/canvas#88.
The idea is that projects building parachains have to keep up with pretty recent versions
of Cumulus, Polkadot, and Substrate. However, there's no good resource to use as a
reference for what a parachain runtime and node should look like with the latest changes
across all of these repos. As such, we need an equivalent to the
node-template
for Cumulus.This template is based off the Canvas parachain, which in turn is based off the
substrate-parachain-template
. Both of these projects hope to use this new template astheir reference going forward.