fix: support new substrate-contracts-node structure and stabilize int…

…egration tests (#360)

* fix: parse new structure substrate-contracts-node

* fix: paseo+coretime integration test

* fix: sourcing latest version substrate-contracts-node

* refactor: set_executable_permission function

* fix: clippy

* chore: CI configuration

* test: specify port in run_contracts_node

* fix: use random ports instead of hardcoded ones

.github/workflows/ci.yml

@@ -4,7 +4,7 @@ on:
   push:
     branches: [ "main" ]
   pull_request:
-    branches: [ "main" ]
+    types: [ opened, synchronize, reopened, ready_for_review ]
 
 env:
   CARGO_TERM_COLOR: always

crates/pop-cli/src/commands/up/contract.rs

@@ -26,6 +26,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)]
@@ -145,7 +146,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:{}",

crates/pop-cli/src/common/contracts.rs

@@ -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};
 
@@ -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: {}",

crates/pop-cli/tests/contract.rs

@@ -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,
@@ -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
@@ -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

crates/pop-common/src/lib.rs

@@ -12,6 +12,7 @@ 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"));

crates/pop-common/src/sourcing/mod.rs

@@ -495,9 +495,18 @@ async fn download(url: &str, dest: &Path) -> Result<(), Error> {
 	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(())
 }
 

crates/pop-contracts/src/call.rs

@@ -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";
 
@@ -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(
@@ -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")),
@@ -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?;
@@ -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,
 		})
@@ -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)");

crates/pop-contracts/src/lib.rs

@@ -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, get_contract_code, get_instantiate_payload,
 	get_upload_payload, instantiate_smart_contract, set_up_deployment, set_up_upload,

crates/pop-contracts/src/node/mod.rs

@@ -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
@@ -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()?));
@@ -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;
@@ -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 {
@@ -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?);

crates/pop-contracts/src/testing.rs

@@ -4,6 +4,7 @@ use crate::{create_smart_contract, Contract};
 use anyhow::Result;
 use std::{
 	fs::{copy, create_dir},
+	net::TcpListener,
 	path::Path,
 };
 
@@ -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()
+}

crates/pop-contracts/src/up.rs

@@ -319,10 +319,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";
@@ -479,7 +481,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(
@@ -492,8 +495,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(),
@@ -502,7 +507,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?;
@@ -526,7 +531,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?;

crates/pop-parachains/src/build.rs

@@ -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> {
@@ -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)
 	}
 

crates/pop-parachains/tests/parachain.rs

@@ -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?;