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

Feature: Pallet Engine (Add pallet - MVP) #43

Closed
wants to merge 71 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
71 commits
Select commit Hold shift + click to select a range
5f60e9a
init: Pallet-engine
weezy20 Mar 4, 2024
3539460
Merge branch 'main' into add-pallet
weezy20 Mar 6, 2024
ee058b3
tidy
weezy20 Mar 6, 2024
e7b791a
step_builder for pallet-engine
weezy20 Mar 6, 2024
c3dcfa0
insert_import_stmts
weezy20 Mar 7, 2024
60cee28
Merge branch 'main' into add-pallet
weezy20 Mar 7, 2024
0cfec78
fix build errors
weezy20 Mar 7, 2024
685f3c7
prepare_config
weezy20 Mar 8, 2024
6dd21d1
fix more runtime errors
weezy20 Mar 8, 2024
4214d2c
Merge branch 'main' into add-pallet
weezy20 Mar 8, 2024
7e81cc2
update lockfile
weezy20 Mar 8, 2024
69d9491
minor adjustments
weezy20 Mar 8, 2024
2e6aef6
prepare test harness for add-pallet
weezy20 Mar 8, 2024
bc788de
Merge branch 'main' into add-pallet
weezy20 Mar 8, 2024
83d2d2c
test : add_parachain_pallet_template
weezy20 Mar 8, 2024
cc3ca54
improve test
weezy20 Mar 9, 2024
0d40634
style command
weezy20 Mar 9, 2024
736deb4
tweaks for correct cursor tracking
weezy20 Mar 9, 2024
85a7e47
default to use insert_str_runtime for implicit runtime decl entries.
weezy20 Mar 9, 2024
e4eb10f
Merge branch 'main' into add-pallet
weezy20 Mar 9, 2024
3e2c150
typo
weezy20 Mar 9, 2024
13c84bb
prepare dependency-injection
weezy20 Mar 9, 2024
a588c92
Merge branch 'main' into add-pallet
weezy20 Mar 9, 2024
d066526
refactor: add pallet subcommand
AlexD10S Mar 9, 2024
8932833
Revert "refactor: add pallet subcommand"
weezy20 Mar 10, 2024
7c2b7ff
add pop add pallet subsubcommand
weezy20 Mar 10, 2024
70881b8
fix tests
weezy20 Mar 10, 2024
2182ade
Merge branch 'main' into add-pallet
weezy20 Mar 12, 2024
023fdbf
sub0 - template runtime dep injection
weezy20 Mar 12, 2024
c158721
fix template toml path
weezy20 Mar 12, 2024
a6e713f
fix TomlEditor runtime path
weezy20 Mar 12, 2024
ea01ead
use consistent naming for template pallet : pallet-parachain-template
weezy20 Mar 12, 2024
8ab1ba9
remove - -> _ substitution in pallet name
weezy20 Mar 12, 2024
1e6d690
fix: use 9090 as pop node id
al3mart Mar 12, 2024
7304889
fix last_import step -_-
weezy20 Mar 12, 2024
aed5436
set pallet-template version 1.0.0
weezy20 Mar 12, 2024
1f49718
fix: in pallet template Cargo use same dependencies versions as base-…
AlexD10S Mar 12, 2024
49850b2
merge add-pallet
al3mart Mar 12, 2024
a39b1c2
style: fmt
al3mart Mar 12, 2024
a2532ad
Merge pull request #60 from r0gue-io/al3mart/fix-paraid-zombienet
al3mart Mar 12, 2024
19c7637
fix inaccurate import cursor
weezy20 Mar 17, 2024
5a0e6b4
add now checks for uncommitted changes
weezy20 Mar 17, 2024
b80cb50
Merge branch 'main' into add-pallet
weezy20 Mar 17, 2024
b122568
feature["parachain"] gate add
weezy20 Mar 17, 2024
9ff1318
optional deps for add-pallet
weezy20 Mar 17, 2024
1bb0385
add note for add test failure
weezy20 Mar 17, 2024
33c2a95
modify add-pallet test
weezy20 Mar 18, 2024
3c55dac
test fails due to unclean git state on pop new parachain
weezy20 Mar 18, 2024
7996257
manually invoke git, then run add-pallet
weezy20 Mar 18, 2024
c6d1991
comments
weezy20 Mar 18, 2024
1d92314
separate dependency module
weezy20 Mar 19, 2024
5f806d8
generalize dependency injection
weezy20 Mar 19, 2024
4176187
git config for CI
weezy20 Mar 19, 2024
f5ff852
fix issue #67
weezy20 Mar 20, 2024
96401ed
fmt
weezy20 Mar 20, 2024
215b028
review fixes
weezy20 Mar 20, 2024
354116e
review fixes 2
weezy20 Mar 20, 2024
99f4b64
review fixes 3 + fix add_new_line cursor
weezy20 Mar 21, 2024
0e682f5
Merge branch 'main' into add-pallet
weezy20 Mar 21, 2024
159b998
use pallet-parachain-template path relative to workspace root
weezy20 Mar 21, 2024
4309bb2
fix order for pallet-engine construction
weezy20 Mar 21, 2024
0363844
include checks for try-runtime and runtime-benchmarks features in run…
weezy20 Mar 21, 2024
7e7478d
fix(deps): remove deprecated dependency
evilrobot-01 Mar 21, 2024
6f7ddd8
remove todo comment for name-conflict-resolution-strategy
weezy20 Mar 22, 2024
a939826
remove commented code
weezy20 Mar 27, 2024
0754e6a
add help msg for add pallet
weezy20 Mar 27, 2024
75fceb9
Merge branch 'main' into add-pallet
weezy20 Mar 29, 2024
e93dc0c
Merge branch 'main' into add-pallet
weezy20 Apr 1, 2024
44eed89
fmt
weezy20 Apr 1, 2024
dd05112
Merge branch 'main' into add-pallet
weezy20 Apr 8, 2024
e6733e5
rename test
weezy20 Apr 8, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
866 changes: 489 additions & 377 deletions Cargo.lock

Large diffs are not rendered by default.

28 changes: 23 additions & 5 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,24 @@ console = "0.15"
duct = "0.13"
git2 = "0.18"
log = "0.4"
# semver = "1.0.20"
semver = "1.0.20"
strum = "0.26"
strum_macros = "0.26"
tempfile = "3.8"
tokio = { version = "1.0", features = ["macros", "rt-multi-thread"] }
url = { version = "2.5", optional = true }
walkdir = "2.4"
toml_edit = { version = "0.22.7" }
regex="1.5.4"

# contracts
contract-build = { version = "4.0.0-rc.3", optional = true }
contract-extrinsics = { version = "4.0.0-rc.3", optional = true }
sp-core = { version = "30.0.0", optional = true }
subxt-signer = { version = "0.34.0", features = ["subxt", "sr25519"], optional = true }
subxt-signer = { version = "0.34.0", features = [
"subxt",
"sr25519",
], optional = true }
subxt = { version = "0.34.0", optional = true }
ink_env = { version = "5.0.0-rc.2", optional = true }
sp-weights = { version = "29.0.0", optional = true }
Expand All @@ -43,10 +47,18 @@ reqwest = { version = "0.11", optional = true }
serde_json = { version = "1.0", optional = true }
serde = { version = "1.0", features = ["derive"], optional = true }
symlink = { version = "0.1", optional = true }
toml_edit = { version = "0.22", optional = true }
tracing-subscriber = { version = "0.3", optional = true }
zombienet-sdk = { git = "https://github.com/r0gue-io/zombienet-sdk", branch = "pop", optional = true }
zombienet-support = { git = "https://github.com/r0gue-io/zombienet-sdk", branch = "pop", optional = true }
uuid = { version = "1.7.0", features = ["v4"], optional = true }
frame-support-procedural-tools = { version = "9.0.0", optional = true }
proc-macro2 = { version = "1.0.70", features = [
"span-locations",
], optional = true }
quote = { version = "1.0.33", optional = true }
syn = { version = "2.0.52", features = ["full"], optional = true }
cfg-expr = { version = "0.15.5", optional = true }
prettyplease = { version = "0.2.15", optional = true }

[dev-dependencies]
assert_cmd = "2.0.14"
Expand All @@ -72,11 +84,17 @@ parachain = [
"dep:reqwest",
"dep:serde_json",
"dep:symlink",
"dep:toml_edit",
"dep:tracing-subscriber",
"dep:url",
"dep:zombienet-sdk",
"dep:zombienet-support"
"dep:zombienet-support",
"uuid",
"frame-support-procedural-tools",
"proc-macro2",
"quote",
"syn",
"cfg-expr",
"prettyplease",
]
e2e_parachain = []

2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ There's also the shorter version:
pop new parachain my-app -s DOT -d 6 -i 1_000_000_000
```

To create a new pallet, simply `pop new pallet`. And that's it. You will have a new `pallet-template` ready for hacking.
To create a new pallet, simply `pop new pallet`. And that's it. You will have a new `pallet-parachain-template` ready for hacking.
To customize the new pallet you can follow these options:

```sh
Expand Down
85 changes: 85 additions & 0 deletions src/commands/add/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
#![cfg(feature = "parachain")]
use std::path::PathBuf;

use crate::engines::pallet_engine;
use clap::{Args, Subcommand};
use cliclack::{intro, outro};
use console::style;

#[derive(Args)]
#[command(args_conflicts_with_subcommands = true)]
pub(crate) struct AddArgs {
#[command(subcommand)]
commands: AddCommands,
#[arg(global = true, short, long = "runtime")]
/// Runtime path; for example: `sub0/runtime/src/lib.rs`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as Cargo.toml is infered maybe for UX we can add the whole runtime path too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't follow what you're saying

/// Runtime cargo manifest path will be inferred as `(parent of lib.rs)/Cargo.toml`
pub(crate) runtime_path: Option<String>,
}
#[derive(Subcommand)]
#[command(subcommand_required = true)]
pub(crate) enum AddCommands {
#[command(subcommand)]
weezy20 marked this conversation as resolved.
Show resolved Hide resolved
/// Add a pallet
#[clap(alias = "p")]
Pallet(AddPallet),
}

#[derive(Subcommand, Clone)]
#[command(subcommand_required = true)]
pub(crate) enum AddPallet {
/// Insert `pallet-parachain-template` into the runtime.
Template,
/// Insert a frame-pallet into the runtime.
Frame(FrameArgs),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I personally don’t like to have the FRAME logic there if is not working. It might be confusing for the user.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why would the user be confused when they're met with a not implemented message ? If I delete this scaffolding code which is minimal btw, I will have to rewrite it when we start with Frame which we are going to do anyways.

Copy link
Collaborator

Choose a reason for hiding this comment

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

With git there is no need to rewrite things, just take it from here into another branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry I have to disagree with that approach. Doesn't solve anything.

}

#[derive(Args, Clone)]
pub(crate) struct FrameArgs {
#[arg(short, long)]
// TODO: Not ready for use
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above, if doesn't work might be better delete it

pub(crate) name: String,
}

impl AddArgs {
pub(crate) fn execute(&self) -> anyhow::Result<()> {
match self.commands {
AddCommands::Pallet(ref cmd) => cmd.clone().execute(&self.runtime_path),
}
}
}
impl AddPallet {
pub(crate) fn execute(self, runtime_path: &Option<String>) -> anyhow::Result<()> {
let runtime_path = match runtime_path {
Some(ref s) => {
let path = PathBuf::from(s);
if !path.exists() {
anyhow::bail!("Invalid runtime path: {}", path.display());
}
path
},
None => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

For UX, Is not better to use clap errors?. Will be add runtime_path as mandatory above

If i run cargo run add pallet template it panics

thread 'main' panicked at src/commands/add/mod.rs:62:17:
not implemented: provide a runtime path until feat:cache is implemented: --runtime <path>
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

If I run cargo run up parachain without the mandatory commands it shows me how to use the tool:

error: the following required arguments were not provided:
  --file <FILE>

Usage: pop up parachain --file <FILE>

For more information, try '--help'.

Copy link
Contributor Author

@weezy20 weezy20 Mar 22, 2024

Choose a reason for hiding this comment

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

I agree. This makes more sense when we have a functional cache.. Just for the record,reason to use an Option here is to rely on automatic inference via the pop new cache in the case that user omits -r. But yeah seems right to make it required

// TODO: Fetch runtime either from cache
Copy link
Collaborator

Choose a reason for hiding this comment

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

For TODOs is better to create an Issue, than lines of code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some of it is already in the issue and PR description. These are just placeholder comments from where future work can be easily picked up where these were left off.

unimplemented!(
"provide a runtime path until feat:cache is implemented: --runtime <path>"
);
},
};
let pallet = match self {
AddPallet::Template => "pallet-parachain-template".to_string(),
AddPallet::Frame(FrameArgs { .. }) => {
eprintln!("Sorry, frame pallets cannot be added right now");
std::process::exit(1);
// format!("FRAME pallet-{name}")
},
};
intro(format!(
"{}: Adding pallet \"{}\"!",
style(" Pop CLI ").black().on_magenta(),
&pallet,
))?;
pallet_engine::execute(self, runtime_path.clone())?;
outro(format!("Added {}\n-> to {}", pallet, runtime_path.display()))?;
Ok(())
}
}
1 change: 1 addition & 0 deletions src/commands/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
pub(crate) mod add;
pub(crate) mod build;
pub(crate) mod call;
pub(crate) mod new;
Expand Down
2 changes: 1 addition & 1 deletion src/commands/new/pallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use std::fs;

#[derive(Args)]
pub struct NewPalletCommand {
#[arg(help = "Name of the pallet", default_value = "pallet-template")]
#[arg(help = "Name of the pallet", default_value = "pallet-parachain-template")]
pub(crate) name: String,
#[arg(short, long, help = "Name of authors", default_value = "Anonymous")]
pub(crate) authors: Option<String>,
Expand Down
137 changes: 137 additions & 0 deletions src/engines/pallet_engine/dependency.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
//! Dependency representations for Pallets
use std::path::{self, Path, PathBuf};
use strum_macros::{Display, EnumString};

#[derive(EnumString, Display, Debug)]
pub(super) enum Features {
#[strum(serialize = "std")]
Std,
#[strum(serialize = "runtime-benchmarks")]
RuntimeBenchmarks,
#[strum(serialize = "try-runtime")]
TryRuntime,
/// Custom feature
Custom(String),
}
#[derive(Display, Debug, Clone)]
pub(super) enum Location {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is everything for Location needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.

/// Local path is form `path = "X"`
Local(std::path::PathBuf, Option<semver::Version>),
/// `git = {url}`
Git(reqwest::Url, Option<semver::Version>),
weezy20 marked this conversation as resolved.
Show resolved Hide resolved
/// String should be of format `version = "X"`
CratesIO(semver::Version),
}
impl From<reqwest::Url> for Location {
fn from(url: reqwest::Url) -> Self {
Self::Git(url, None)
}
}
impl From<std::path::PathBuf> for Location {
fn from(path: std::path::PathBuf) -> Self {
Self::Local(path, None)
}
}
impl From<(std::path::PathBuf, semver::Version)> for Location {
fn from(info: (std::path::PathBuf, semver::Version)) -> Self {
Self::Local(info.0, Some(info.1))
}
}
impl<'a> From<&'a std::path::Path> for Location {
fn from(path: &'a std::path::Path) -> Self {
Self::Local(path.to_path_buf(), None)
}
}
impl<'a> From<(&'a std::path::Path, semver::Version)> for Location {
fn from(info: (&'a std::path::Path, semver::Version)) -> Self {
Self::Local(info.0.to_path_buf(), Some(info.1.into()))
}
}
impl From<semver::Version> for Location {
fn from(version: semver::Version) -> Self {
Self::CratesIO(version)
}
}
impl Into<String> for Location {
fn into(self) -> String {
match self {
Location::Local(path, Some(version)) => {
format!("{{ path = \"{}\", version = \"{}\" }}", path.display(), version)
},
Location::Local(path, _) => format!("{{ path = \"{}\" }}", path.display()),
Location::Git(url, Some(version)) => {
format!("{{ git = \"{}\", version = \"{}\" }}", url, version)
},
Location::Git(url, _) => format!("{{ git = \"{}\" }}", url),
Location::CratesIO(version) => format!("{{ version = \"{}\" }}", version),
}
}
}
impl Into<toml_edit::Value> for Location {
fn into(self) -> toml_edit::Value {
let s = Into::<String>::into(self);
let t = s
.parse::<toml_edit::Value>()
.expect("Location String parse as Toml Value failed");
toml_edit::Value::InlineTable(
t.as_inline_table().expect(" Parsed Location -> ILT cast infallible").to_owned(),
)
}
}

#[derive(Debug)]
pub(super) struct Dependency {
/// Name for the dependency
pub(super) name: String,
/// Additional features that need to be enabled. Format -> {name}/{feature}
pub(super) features: Vec<Features>,
/// Maybe local path, git url, or from crates.io in which case we will use this for version
pub(super) path: Location,
/// Default features such as `std` are disabled by default for runtime pallet dependencies
pub(super) default_features: bool,
}

impl Dependency {
/// Generate the main dependency as an inline table
pub(super) fn entry(&self) -> toml_edit::InlineTable {
let mut t = toml_edit::Table::new();
let location = Into::<toml_edit::Value>::into(self.path.clone());
t.extend(
location
.as_inline_table()
.expect("Location to String should produce valid inline table")
.to_owned(),
);
t["default-features"] = toml_edit::value(self.default_features);
t.into_inline_table()
}
/// Create dependencies required for adding a local pallet to runtime
/// $(workspace_root)/pallets/pallet-name
// Todo: Some values are hardcoded for now as this only deals with the pallet-parachain-template
// Should be moved to args once that's not the case
pub(super) fn local_template_runtime(pallet_name: &'static str, workspace: &PathBuf) -> Self {
let name = format!("{pallet_name}");
Self {
features: vec![Features::RuntimeBenchmarks, Features::TryRuntime, Features::Std],
path: (
// The reason is, `pop new pallet` places a new pallet on $(workspace_root)/pallets
workspace.join("pallets/").join(&name).to_path_buf(),
semver::Version::new(1, 0, 0),
)
.into(),
default_features: false,
name,
}
}
// TODO: Remove code - Node doesn't require template pallet deps by default
Copy link
Collaborator

Choose a reason for hiding this comment

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

As it says in the TODO: remove the code

// but this maybe desirable for custom pallets.
// /// Create dependencies required for adding a pallet-parachain-template to node
// pub(super) fn template_node() -> Self {
// Self {
// features: vec![Features::RuntimeBenchmarks, Features::TryRuntime],
// // TODO hardcode for now
// path: format!("../pallets/pallet-parachain-template"),
// default_features: true,
// }
// }
}
Loading
Loading