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

fix: support new substrate-contracts-node structure and stabilize integration tests #360

Merged
merged 8 commits into from
Dec 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ on:
push:
branches: [ "main" ]
pull_request:
branches: [ "main" ]
types: [ opened, synchronize, reopened, ready_for_review ]

env:
CARGO_TERM_COLOR: always
Expand Down
4 changes: 3 additions & 1 deletion crates/pop-cli/src/commands/up/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use url::Url;

const COMPLETE: &str = "🚀 Deployment complete";
const DEFAULT_URL: &str = "ws://localhost:9944/";
const DEFAULT_PORT: u16 = 9944;
const FAILED: &str = "🚫 Deployment failed.";

#[derive(Args, Clone)]
Expand Down Expand Up @@ -140,7 +141,8 @@ impl UpContractCommand {
let spinner = spinner();
spinner.start("Starting local node...");

let process = run_contracts_node(binary_path, Some(log.as_file())).await?;
let process =
run_contracts_node(binary_path, Some(log.as_file()), DEFAULT_PORT).await?;
let bar = Style::new().magenta().dim().apply_to(Emoji("│", "|"));
spinner.stop(format!(
"Local node started successfully:{}",
Expand Down
5 changes: 3 additions & 2 deletions crates/pop-cli/src/common/contracts.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-3.0

use cliclack::{confirm, log::warning, spinner};
use pop_common::manifest::from_path;
use pop_common::{manifest::from_path, sourcing::set_executable_permission};
use pop_contracts::contracts_node_generator;
use std::path::{Path, PathBuf};

Expand Down Expand Up @@ -53,8 +53,9 @@ pub async fn check_contracts_node_and_prompt(skip_confirm: bool) -> anyhow::Resu
let spinner = spinner();
spinner.start("📦 Sourcing substrate-contracts-node...");

binary.use_latest();
binary = contracts_node_generator(crate::cache()?, binary.latest()).await?;
binary.source(false, &(), true).await?;
set_executable_permission(binary.path())?;

spinner.stop(format!(
"✅ substrate-contracts-node successfully sourced. Cached at: {}",
Expand Down
6 changes: 4 additions & 2 deletions crates/pop-cli/tests/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

use anyhow::Result;
use assert_cmd::Command;
use pop_common::templates::Template;
use pop_common::{set_executable_permission, templates::Template};
use pop_contracts::{
contracts_node_generator, dry_run_gas_estimate_instantiate, instantiate_smart_contract,
run_contracts_node, set_up_deployment, Contract, UpOpts,
Expand All @@ -14,6 +14,7 @@ use url::Url;
/// Test the contract lifecycle: new, build, up, call
#[tokio::test]
async fn contract_lifecycle() -> Result<()> {
const DEFAULT_PORT: u16 = 9944;
let temp = tempfile::tempdir().unwrap();
let temp_dir = temp.path();
//let temp_dir = Path::new("./"); //For testing locally
Expand Down Expand Up @@ -44,7 +45,8 @@ async fn contract_lifecycle() -> Result<()> {

let binary = contracts_node_generator(temp_dir.to_path_buf().clone(), None).await?;
binary.source(false, &(), true).await?;
let process = run_contracts_node(binary.path(), None).await?;
set_executable_permission(binary.path())?;
let process = run_contracts_node(binary.path(), None, DEFAULT_PORT).await?;

// Only upload the contract
// pop up contract --upload-only
Expand Down
1 change: 1 addition & 0 deletions crates/pop-common/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
pub mod build;

Check warning on line 1 in crates/pop-common/src/lib.rs

View workflow job for this annotation

GitHub Actions / clippy

missing documentation for a module

warning: missing documentation for a module --> crates/pop-common/src/lib.rs:1:1 | 1 | pub mod build; | ^^^^^^^^^^^^^
pub mod errors;

Check warning on line 2 in crates/pop-common/src/lib.rs

View workflow job for this annotation

GitHub Actions / clippy

missing documentation for a module

warning: missing documentation for a module --> crates/pop-common/src/lib.rs:2:1 | 2 | pub mod errors; | ^^^^^^^^^^^^^^
pub mod git;

Check warning on line 3 in crates/pop-common/src/lib.rs

View workflow job for this annotation

GitHub Actions / clippy

missing documentation for a module

warning: missing documentation for a module --> crates/pop-common/src/lib.rs:3:1 | 3 | pub mod git; | ^^^^^^^^^^^
pub mod helpers;

Check warning on line 4 in crates/pop-common/src/lib.rs

View workflow job for this annotation

GitHub Actions / clippy

missing documentation for a module

warning: missing documentation for a module --> crates/pop-common/src/lib.rs:4:1 | 4 | pub mod helpers; | ^^^^^^^^^^^^^^^
pub mod manifest;

Check warning on line 5 in crates/pop-common/src/lib.rs

View workflow job for this annotation

GitHub Actions / clippy

missing documentation for a module

warning: missing documentation for a module --> crates/pop-common/src/lib.rs:5:1 | 5 | pub mod manifest; | ^^^^^^^^^^^^^^^^
pub mod polkadot_sdk;
pub mod sourcing;

Check warning on line 7 in crates/pop-common/src/lib.rs

View workflow job for this annotation

GitHub Actions / clippy

missing documentation for a module

warning: missing documentation for a module --> crates/pop-common/src/lib.rs:7:1 | 7 | pub mod sourcing; | ^^^^^^^^^^^^^^^^
pub mod templates;

Check warning on line 8 in crates/pop-common/src/lib.rs

View workflow job for this annotation

GitHub Actions / clippy

missing documentation for a module

warning: missing documentation for a module --> crates/pop-common/src/lib.rs:8:1 | 8 | pub mod templates; | ^^^^^^^^^^^^^^^^^

pub use build::Profile;
pub use errors::Error;
pub use git::{Git, GitHub, Release};
pub use helpers::{get_project_name_from_path, prefix_with_current_dir_if_needed, replace_in_file};
pub use manifest::{add_crate_to_workspace, find_workspace_toml};
pub use sourcing::set_executable_permission;
pub use templates::extractor::extract_template_files;

static APP_USER_AGENT: &str = concat!(env!("CARGO_PKG_NAME"), "/", env!("CARGO_PKG_VERSION"));
Expand Down
13 changes: 11 additions & 2 deletions crates/pop-common/src/sourcing/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,19 @@
use url::Url;

#[derive(Error, Debug)]
pub enum Error {

Check warning on line 23 in crates/pop-common/src/sourcing/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

missing documentation for an enum

warning: missing documentation for an enum --> crates/pop-common/src/sourcing/mod.rs:23:1 | 23 | pub enum Error { | ^^^^^^^^^^^^^^
#[error("Anyhow error: {0}")]
AnyhowError(#[from] anyhow::Error),

Check warning on line 25 in crates/pop-common/src/sourcing/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

missing documentation for a variant

warning: missing documentation for a variant --> crates/pop-common/src/sourcing/mod.rs:25:2 | 25 | AnyhowError(#[from] anyhow::Error), | ^^^^^^^^^^^
#[error("Archive error: {0}")]
ArchiveError(String),

Check warning on line 27 in crates/pop-common/src/sourcing/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

missing documentation for a variant

warning: missing documentation for a variant --> crates/pop-common/src/sourcing/mod.rs:27:2 | 27 | ArchiveError(String), | ^^^^^^^^^^^^
#[error("HTTP error: {0}")]
HttpError(#[from] reqwest::Error),

Check warning on line 29 in crates/pop-common/src/sourcing/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

missing documentation for a variant

warning: missing documentation for a variant --> crates/pop-common/src/sourcing/mod.rs:29:2 | 29 | HttpError(#[from] reqwest::Error), | ^^^^^^^^^
#[error("IO error: {0}")]
IO(#[from] std::io::Error),

Check warning on line 31 in crates/pop-common/src/sourcing/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

missing documentation for a variant

warning: missing documentation for a variant --> crates/pop-common/src/sourcing/mod.rs:31:2 | 31 | IO(#[from] std::io::Error), | ^^
#[error("Missing binary: {0}")]
MissingBinary(String),

Check warning on line 33 in crates/pop-common/src/sourcing/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

missing documentation for a variant

warning: missing documentation for a variant --> crates/pop-common/src/sourcing/mod.rs:33:2 | 33 | MissingBinary(String), | ^^^^^^^^^^^^^
#[error("ParseError error: {0}")]
ParseError(#[from] url::ParseError),

Check warning on line 35 in crates/pop-common/src/sourcing/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

missing documentation for a variant

warning: missing documentation for a variant --> crates/pop-common/src/sourcing/mod.rs:35:2 | 35 | ParseError(#[from] url::ParseError), | ^^^^^^^^^^
}

/// The source of a binary.
Expand Down Expand Up @@ -495,9 +495,18 @@
let mut file = File::create(dest)?;
file.write_all(&response.bytes().await?)?;
// Make executable
let mut perms = metadata(dest)?.permissions();
set_executable_permission(dest)?;
Ok(())
}

/// Sets the executable permission for a given file.
///
/// # Arguments
/// * `path` - The file path to which permissions should be granted.
pub fn set_executable_permission<P: AsRef<Path>>(path: P) -> Result<(), Error> {
let mut perms = metadata(&path)?.permissions();
perms.set_mode(0o755);
std::fs::set_permissions(dest, perms)?;
std::fs::set_permissions(path, perms)?;
Ok(())
}

Expand Down Expand Up @@ -828,7 +837,7 @@
}
}

pub mod traits {

Check warning on line 840 in crates/pop-common/src/sourcing/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

missing documentation for a module

warning: missing documentation for a module --> crates/pop-common/src/sourcing/mod.rs:840:1 | 840 | pub mod traits { | ^^^^^^^^^^^^^^
use crate::{sourcing::Error, GitHub};
use strum::EnumProperty;

Expand Down
22 changes: 14 additions & 8 deletions crates/pop-contracts/src/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,11 +180,13 @@ mod tests {
use crate::{
contracts_node_generator, dry_run_gas_estimate_instantiate, errors::Error,
instantiate_smart_contract, mock_build_process, new_environment, run_contracts_node,
set_up_deployment, UpOpts,
set_up_deployment, testing::find_free_port, UpOpts,
};
use anyhow::Result;
use pop_common::set_executable_permission;
use sp_core::Bytes;
use std::{env, process::Command};
use std::{env, process::Command, time::Duration};
use tokio::time::sleep;

const CONTRACTS_NETWORK_URL: &str = "wss://rpc2.paseo.popnetwork.xyz";

Expand Down Expand Up @@ -334,7 +336,8 @@ mod tests {

#[tokio::test]
async fn call_works() -> Result<()> {
const LOCALHOST_URL: &str = "ws://127.0.0.1:9944";
let random_port = find_free_port();
let localhost_url = format!("ws://127.0.0.1:{}", random_port);
let temp_dir = new_environment("testing")?;
let current_dir = env::current_dir().expect("Failed to get current directory");
mock_build_process(
Expand All @@ -347,7 +350,10 @@ mod tests {

let binary = contracts_node_generator(cache.clone(), None).await?;
binary.source(false, &(), true).await?;
let process = run_contracts_node(binary.path(), None).await?;
set_executable_permission(binary.path())?;
let process = run_contracts_node(binary.path(), None, random_port).await?;
// Wait 5 secs more to give time for the node to be ready
sleep(Duration::from_millis(5000)).await;
// Instantiate a Smart Contract.
let instantiate_exec = set_up_deployment(UpOpts {
path: Some(temp_dir.path().join("testing")),
Expand All @@ -357,7 +363,7 @@ mod tests {
gas_limit: None,
proof_size: None,
salt: Some(Bytes::from(vec![0x00])),
url: Url::parse(LOCALHOST_URL)?,
url: Url::parse(&localhost_url)?,
suri: "//Alice".to_string(),
})
.await?;
Expand All @@ -372,7 +378,7 @@ mod tests {
value: "0".to_string(),
gas_limit: None,
proof_size: None,
url: Url::parse(LOCALHOST_URL)?,
url: Url::parse(&localhost_url)?,
suri: "//Alice".to_string(),
execute: false,
})
Expand All @@ -388,15 +394,15 @@ mod tests {
value: "0".to_string(),
gas_limit: None,
proof_size: None,
url: Url::parse(LOCALHOST_URL)?,
url: Url::parse(&localhost_url)?,
suri: "//Alice".to_string(),
execute: false,
})
.await?;
let weight = dry_run_gas_estimate_call(&call_exec).await?;
assert!(weight.ref_time() > 0);
assert!(weight.proof_size() > 0);
call_smart_contract(call_exec, weight, &Url::parse(LOCALHOST_URL)?).await?;
call_smart_contract(call_exec, weight, &Url::parse(&localhost_url)?).await?;
// Assert that the value has been flipped.
query = dry_run_call(&query_exec).await?;
assert_eq!(query, "Ok(true)");
Expand Down
2 changes: 1 addition & 1 deletion crates/pop-contracts/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub use new::{create_smart_contract, is_valid_contract_name};
pub use node::{contracts_node_generator, is_chain_alive, run_contracts_node};
pub use templates::{Contract, ContractType};
pub use test::{test_e2e_smart_contract, test_smart_contract};
pub use testing::{mock_build_process, new_environment};
pub use testing::{find_free_port, mock_build_process, new_environment};
pub use up::{
dry_run_gas_estimate_instantiate, dry_run_upload, instantiate_smart_contract,
set_up_deployment, set_up_upload, upload_smart_contract, UpOpts,
Expand Down
33 changes: 26 additions & 7 deletions crates/pop-contracts/src/node/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ impl TryInto for Chain {
/// * `latest` - If applicable, some specifier used to determine the latest source.
fn try_into(&self, tag: Option<String>, latest: Option<String>) -> Result<Source, Error> {
let archive = archive_name_by_target()?;
let archive_bin_path = release_directory_by_target()?;
let archive_bin_path = release_directory_by_target(tag.as_deref())?;
Ok(match self {
&Chain::ContractsNode => {
// Source from GitHub release asset
Expand Down Expand Up @@ -115,12 +115,15 @@ pub async fn contracts_node_generator(
///
/// * `binary_path` - The path where the binary is stored. Can be the binary name itself if in PATH.
/// * `output` - The optional log file for node output.
/// * `port` - The WebSocket port on which the node will listen for connections.
pub async fn run_contracts_node(
binary_path: PathBuf,
output: Option<&File>,
port: u16,
) -> Result<Child, Error> {
let mut command = Command::new(binary_path);
command.arg("-linfo,runtime::contracts=debug");
command.arg(format!("--rpc-port={}", port));
if let Some(output) = output {
command.stdout(Stdio::from(output.try_clone()?));
command.stderr(Stdio::from(output.try_clone()?));
Expand All @@ -141,16 +144,30 @@ fn archive_name_by_target() -> Result<String, Error> {
}
}

fn release_directory_by_target() -> Result<&'static str, Error> {
fn release_directory_by_target(tag: Option<&str>) -> Result<&'static str, Error> {
// The structure of the binary changed in v0.42.0
let is_old_structure = matches!(tag, Some(tag) if tag < "v0.42.0");
match OS {
"macos" => Ok("artifacts/substrate-contracts-node-mac/substrate-contracts-node"),
"linux" => Ok("artifacts/substrate-contracts-node-linux/substrate-contracts-node"),
"macos" =>
if is_old_structure {
Ok("artifacts/substrate-contracts-node-mac/substrate-contracts-node")
} else {
Ok("substrate-contracts-node-mac/substrate-contracts-node")
},
"linux" =>
if is_old_structure {
Ok("artifacts/substrate-contracts-node-linux/substrate-contracts-node")
} else {
Ok("substrate-contracts-node-linux/substrate-contracts-node")
},
_ => Err(Error::UnsupportedPlatform { arch: ARCH, os: OS }),
}
}

#[cfg(test)]
mod tests {
use crate::testing::find_free_port;

use super::*;
use anyhow::{Error, Result};
use std::process::Command;
Expand Down Expand Up @@ -185,7 +202,7 @@ mod tests {
let version = "v0.40.0";
let binary = contracts_node_generator(cache.clone(), Some(version)).await?;
let archive = archive_name_by_target()?;
let archive_bin_path = release_directory_by_target()?;
let archive_bin_path = release_directory_by_target(Some(version))?;
assert!(matches!(binary, Binary::Source { name, source, cache}
if name == expected.binary() &&
source == Source::GitHub(ReleaseArchive {
Expand All @@ -206,15 +223,17 @@ mod tests {
#[ignore = "Works fine locally but is causing issues when running tests in parallel in the CI environment."]
#[tokio::test]
async fn run_contracts_node_works() -> Result<(), Error> {
let local_url = url::Url::parse("ws://localhost:9944")?;
let random_port = find_free_port();
let localhost_url = format!("ws://127.0.0.1:{}", random_port);
let local_url = url::Url::parse(&localhost_url)?;

let temp_dir = tempfile::tempdir().expect("Could not create temp dir");
let cache = temp_dir.path().join("");

let version = "v0.40.0";
let binary = contracts_node_generator(cache.clone(), Some(version)).await?;
binary.source(false, &(), true).await?;
let process = run_contracts_node(binary.path(), None).await?;
let process = run_contracts_node(binary.path(), None, 9947).await?;

// Check if the node is alive
assert!(is_chain_alive(local_url).await?);
Expand Down
10 changes: 10 additions & 0 deletions crates/pop-contracts/src/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::{create_smart_contract, Contract};
use anyhow::Result;
use std::{
fs::{copy, create_dir},
net::TcpListener,
path::Path,
};

Expand Down Expand Up @@ -37,3 +38,12 @@ where
copy(metadata_file, target_contract_dir.join("ink/testing.json"))?;
Ok(())
}

/// Finds an available port by binding to port 0 and retrieving the assigned port.
pub fn find_free_port() -> u16 {
TcpListener::bind("127.0.0.1:0")
.expect("Failed to bind to an available port")
.local_addr()
.expect("Failed to retrieve local address")
.port()
}
19 changes: 12 additions & 7 deletions crates/pop-contracts/src/up.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,10 +224,12 @@ mod tests {
use super::*;
use crate::{
contracts_node_generator, errors::Error, mock_build_process, new_environment,
run_contracts_node,
run_contracts_node, testing::find_free_port,
};
use anyhow::Result;
use std::{env, process::Command};
use pop_common::set_executable_permission;
use std::{env, process::Command, time::Duration};
use tokio::time::sleep;
use url::Url;

const CONTRACTS_NETWORK_URL: &str = "wss://rpc2.paseo.popnetwork.xyz";
Expand Down Expand Up @@ -364,7 +366,8 @@ mod tests {

#[tokio::test]
async fn instantiate_and_upload() -> Result<()> {
const LOCALHOST_URL: &str = "ws://127.0.0.1:9944";
let random_port = find_free_port();
let localhost_url = format!("ws://127.0.0.1:{}", random_port);
let temp_dir = new_environment("testing")?;
let current_dir = env::current_dir().expect("Failed to get current directory");
mock_build_process(
Expand All @@ -377,8 +380,10 @@ mod tests {

let binary = contracts_node_generator(cache.clone(), None).await?;
binary.source(false, &(), true).await?;
let process = run_contracts_node(binary.path(), None).await?;

set_executable_permission(binary.path())?;
let process = run_contracts_node(binary.path(), None, random_port).await?;
// Wait 5 secs more to give time for the node to be ready
sleep(Duration::from_millis(5000)).await;
let upload_exec = set_up_upload(UpOpts {
path: Some(temp_dir.path().join("testing")),
constructor: "new".to_string(),
Expand All @@ -387,7 +392,7 @@ mod tests {
gas_limit: None,
proof_size: None,
salt: None,
url: Url::parse(LOCALHOST_URL)?,
url: Url::parse(&localhost_url)?,
suri: "//Alice".to_string(),
})
.await?;
Expand All @@ -411,7 +416,7 @@ mod tests {
gas_limit: None,
proof_size: None,
salt: Some(Bytes::from(vec![0x00])),
url: Url::parse(LOCALHOST_URL)?,
url: Url::parse(&localhost_url)?,
suri: "//Alice".to_string(),
})
.await?;
Expand Down
15 changes: 3 additions & 12 deletions crates/pop-parachains/src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,14 +321,8 @@ mod tests {
Zombienet,
};
use anyhow::Result;
use pop_common::manifest::Dependency;
use std::{
fs,
fs::{metadata, write},
io::Write,
os::unix::fs::PermissionsExt,
path::Path,
};
use pop_common::{manifest::Dependency, set_executable_permission};
use std::{fs, fs::write, io::Write, path::Path};
use tempfile::{tempdir, Builder};

fn setup_template_and_instantiate() -> Result<tempfile::TempDir> {
Expand Down Expand Up @@ -405,10 +399,7 @@ default_command = "pop-node"
let content = fs::read(&binary_path)?;
write(temp_dir.join("target/release/parachain-template-node"), content)?;
// Make executable
let mut perms =
metadata(temp_dir.join("target/release/parachain-template-node"))?.permissions();
perms.set_mode(0o755);
fs::set_permissions(temp_dir.join("target/release/parachain-template-node"), perms)?;
set_executable_permission(temp_dir.join("target/release/parachain-template-node"))?;
Ok(binary_path)
}

Expand Down
2 changes: 1 addition & 1 deletion crates/pop-parachains/tests/parachain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ async fn launch_paseo_and_system_parachain() -> Result<()> {
Some(BINARY_VERSION),
None,
Some(BINARY_VERSION),
None,
Some("v1.3.3"), // 1.3.3 is where coretime-paseo-local was introduced.
None,
)
.await?;
Expand Down
Loading