Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 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
8 changes: 8 additions & 0 deletions .github/workflows/integration-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,14 @@ jobs:
brew install protobuf
protoc --version

- name: Setup Docker (macOS)
if: matrix.os == 'macos-latest'
uses: douglascamata/setup-docker-macos-action@v1-alpha

- name: Ensure Docker is running
run: |
docker info || (sudo systemctl start docker && docker info)

- name: Install cargo-nextest
uses: taiki-e/install-action@v2
with:
Expand Down
12 changes: 4 additions & 8 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ sp-weights = { version = "33.0.0", default-features = false }
scale = { package = "parity-scale-codec", version = "3.7.5", features = ["derive"] }
scale-info = { version = "2.11.6", default-features = false, features = ["derive"] }
scale-value = { version = "0.18.0", default-features = false, features = ["from-string", "parser-ss58"] }
contract-build = { version = "6.0.0-beta.1", default-features = false }
contract-extrinsics = { version = "6.0.0-beta.1", default-features = false }
contract-transcode = { version = "6.0.0-beta.1", default-features = false }
contract-build = { git = "https://github.com/use-ink/cargo-contract", rev = "c7aba2e", default-features = false }
contract-extrinsics = { git = "https://github.com/use-ink/cargo-contract", rev = "c7aba2e", default-features = false }
contract-transcode = { git = "https://github.com/use-ink/cargo-contract", rev = "c7aba2e", default-features = false }
Copy link
Collaborator Author

@tsenovilla tsenovilla Nov 27, 2025

Choose a reason for hiding this comment

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

Temporarily needed til these changes are released. Please note that the base branch for this PR isn't main but feat/contract-verification so we can merge as soon as the review is done

heck = { version = "0.5.0", default-features = false }

# parachains
Expand Down
111 changes: 106 additions & 5 deletions crates/pop-cli/src/commands/build/contract.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
// SPDX-License-Identifier: GPL-3.0

use super::{BuildArgs, Profile};
use crate::cli;
use pop_contracts::{MetadataSpec, Verbosity, build_smart_contract};
use pop_contracts::{BuildMode, ImageVariant, MetadataSpec, Verbosity, build_smart_contract};
use std::path::PathBuf;

/// Configuration for building a smart contract.
pub struct BuildContract {
/// Path of the contract project.
pub(crate) path: PathBuf,
/// Build profile: `true` for release mode, `false` for debug mode.
pub(crate) release: bool,
/// Build profile: `true` for release mode, `false` for debug mode, `verifiable` for
/// deterministic, release mode.
pub(crate) build_mode: BuildMode,
/// Which specification to use for contract metadata.
pub(crate) metadata: Option<MetadataSpec>,
/// A custom image for a verifiable build
pub(crate) image: Option<ImageVariant>,
}

impl BuildContract {
Expand All @@ -27,10 +31,107 @@ impl BuildContract {
fn build(self, cli: &mut impl cli::traits::Cli) -> anyhow::Result<&'static str> {
cli.intro("Building your contract")?;
// Build contract.
let build_result =
build_smart_contract(&self.path, self.release, Verbosity::Default, self.metadata)?;
let build_result = build_smart_contract(
&self.path,
self.build_mode,
Verbosity::Default,
self.metadata,
self.image,
)?;
cli.success(build_result.display())?;
cli.outro("Build completed successfully!")?;
Ok("contract")
}
}

/// Resolve the `BuildMode` to use in a contract build depending on the specified args
///
/// # Arguments
/// * `args` - The `BuildArgs` needed to resolve the `BuildMode`
pub(super) fn resolve_build_mode(args: &BuildArgs) -> BuildMode {
match (&args.profile, args.verifiable) {
(Some(Profile::Release), false) | (Some(Profile::Production), false) => BuildMode::Release,
(None, true) => BuildMode::Verifiable,
(None, false) if args.release => BuildMode::Release,
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

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

[nitpick] The resolve_build_mode function doesn't handle the case where both verifiable=true and a profile is specified (e.g., Profile::Release + verifiable=true). Due to the clap conflicts_with_all constraint on line 89 of mod.rs, this should be prevented at the CLI level. However, if this function is called programmatically in tests or other code paths, it would silently fall through to the _ catch-all and return BuildMode::Debug, which is incorrect.

Consider adding a comment explaining that this case is prevented by CLI validation, or add an explicit match arm that clarifies the behavior:

(Some(_), true) => BuildMode::Debug, // Unreachable due to CLI conflicts_with_all
Suggested change
(None, false) if args.release => BuildMode::Release,
(None, false) if args.release => BuildMode::Release,
// This case is unreachable due to CLI `conflicts_with_all` validation,
// but is included for clarity in case the function is called programmatically.
(Some(_), true) => BuildMode::Debug, // Unreachable due to CLI conflicts_with_all

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

@tsenovilla tsenovilla Nov 28, 2025

Choose a reason for hiding this comment

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

While the review is right, the fallback is there to reduce the casuistry and just default to the simplest mode instead of failing if an invalid combination of args is used. As the review says, clap will handle this conflict, so I wouldn't add a specific case just for tests

// Fallback to debug mode
_ => BuildMode::Debug,
}
}

pub(super) fn resolve_image(args: &BuildArgs) -> anyhow::Result<Option<ImageVariant>> {
match (&args.image, args.verifiable) {
(Some(image), true) => Ok(Some(ImageVariant::Custom(image.clone()))),
(None, true) => Ok(Some(ImageVariant::Default)),
(None, false) => Ok(None),
(Some(_), false) =>
Err(anyhow::anyhow!("Custom images can only be used in verifiable builds")),
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn resolve_build_mode_works() {
// Profile::Release + verifiable=false -> Release
assert_eq!(
resolve_build_mode(&BuildArgs {
profile: Some(Profile::Release),
..Default::default()
}),
BuildMode::Release
);
// Profile::Production + verifiable=false -> Release
assert_eq!(
resolve_build_mode(&BuildArgs {
profile: Some(Profile::Production),
..Default::default()
}),
BuildMode::Release
);
// No profile + verifiable=true -> Verifiable
assert_eq!(
resolve_build_mode(&BuildArgs { verifiable: true, ..Default::default() }),
BuildMode::Verifiable
);
// No profile + verifiable=false + release=true -> Release
assert_eq!(
resolve_build_mode(&BuildArgs { release: true, ..Default::default() }),
BuildMode::Release
);
// Profile::Debug + verifiable=false -> Debug
assert_eq!(
resolve_build_mode(&BuildArgs { profile: Some(Profile::Debug), ..Default::default() }),
BuildMode::Debug
);
// No profile + verifiable=false + release=false -> Debug
assert_eq!(resolve_build_mode(&BuildArgs::default()), BuildMode::Debug);
}

#[test]
fn resolve_image_works() {
// Custom image + verifiable=true -> Custom image
assert!(matches!(resolve_image(&BuildArgs {
image: Some("my-image:latest".to_string()),
verifiable: true,
..Default::default()
}), Ok(Some(ImageVariant::Custom(custom))) if custom == "my-image:latest"
));
// No image + verifiable=true -> Default image
assert!(matches!(
resolve_image(&BuildArgs { verifiable: true, ..Default::default() }),
Ok(Some(ImageVariant::Default))
));
// No image + verifiable=false -> None
assert!(matches!(resolve_image(&BuildArgs::default()), Ok(None)));
// Custom image + verifiable=false -> Error
let err = resolve_image(&BuildArgs {
image: Some("my-image:latest".to_string()),
verifiable: false,
..Default::default()
})
.unwrap_err();
assert_eq!(err.to_string(), "Custom images can only be used in verifiable builds");
}
}
57 changes: 46 additions & 11 deletions crates/pop-cli/src/commands/build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ pub(crate) struct BuildArgs {
#[cfg(feature = "chain")]
pub command: Option<Command>,
/// Directory path with flag for your project [default: current directory]
#[arg(long)]
#[arg(long, visible_alias = "manifest-path")]
pub(crate) path: Option<PathBuf>,
/// Directory path without flag for your project [default: current directory]
#[arg(value_name = "PATH", index = 1, conflicts_with = "path")]
Expand Down Expand Up @@ -84,10 +84,15 @@ pub(crate) struct BuildArgs {
#[clap(long, help_heading = CONTRACT_HELP_HEADER)]
#[cfg(feature = "contract")]
pub(crate) metadata: Option<MetadataSpec>,
/// Whether to build in a way that the contract is verifiable
///#[clap(long, help_heading = CONTRACT_HELP_HEADER)]
///#[cfg(feature = "contract")]
///pub(crate) verifiable: bool
/// Whether to build in a way that the contract is verifiable. For verifiable contracts, use
/// --manifest-path instead of --path to directly point to your contracts Cargo.toml
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why? --manifest-path instead of --path to directly point to your contracts Cargo.toml
Can we not abstract this to the user?

Copy link
Collaborator Author

@tsenovilla tsenovilla Dec 1, 2025

Choose a reason for hiding this comment

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

Because the contract-build function we're using captures what's passed to the CLI: https://github.com/use-ink/cargo-contract/blob/master/crates/build/src/docker.rs#L523 and then pass it to cargo-contract CLI inside the Docker image. If we pass --path that CLI called inside docker will simply fail as it doesn't expect --path, you can try it yourself.

I was thinking on simply disabling --path if --verifiable is used, but I think it's still worth it to don't force the user to run the command inside the contract's dir.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah I see. The cargo-contract is a very confusing implementation no?
Because here we already get the path to the manifest path: https://github.com/r0gue-io/pop-cli/blob/main/crates/pop-contracts/src/build.rs#L22 and then we can call the build function very clean.
Not sure the best solution here to be honest, for the user might be confusing to be forced to use --manifest-path but i see your point

Copy link
Collaborator Author

@tsenovilla tsenovilla Dec 1, 2025

Choose a reason for hiding this comment

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

yeah it's not the best for third party integration, but normal anyway as it's designed to call cargo-contract inside the image :/

The line you mentioned is actually useless for verifiable contracts, as if the user specifies --manifest-path it's captured within the image, not passed to contract_build::execute at all.

So yeah it's not optimal but Idk if we have something better

#[clap(long, conflicts_with_all = ["release", "profile"], help_heading = CONTRACT_HELP_HEADER)]
#[cfg(feature = "contract")]
pub(crate) verifiable: bool,
/// Custom image for verifiable builds
#[clap(long, help_heading = CONTRACT_HELP_HEADER)]
Copy link
Collaborator

@AlexD10S AlexD10S Dec 1, 2025

Choose a reason for hiding this comment

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

Here you can add requires = "verifiable", image is useless if verifiable is not indicated no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Absolutely, let me fix it!

#[cfg(feature = "contract")]
pub(crate) image: Option<String>,
}

/// Subcommand for building chain artifacts.
Expand Down Expand Up @@ -120,12 +125,10 @@ impl Command {

#[cfg(feature = "contract")]
if pop_contracts::is_supported(&project_path)? {
// All commands originating from root command are valid
let release = match &args.profile {
Some(profile) => (*profile).into(),
None => args.release,
};
BuildContract { path: project_path, release, metadata: args.metadata }.execute()?;
let build_mode = contract::resolve_build_mode(&args);
let image = contract::resolve_image(&args)?;
BuildContract { path: project_path, build_mode, metadata: args.metadata, image }
.execute()?;
return Ok(());
}

Expand Down Expand Up @@ -361,6 +364,10 @@ mod tests {
only_runtime: false,
#[cfg(feature = "contract")]
metadata: None,
#[cfg(feature = "contract")]
verifiable: false,
#[cfg(feature = "contract")]
image: None,
},
project_path,
&mut cli,
Expand Down Expand Up @@ -429,6 +436,10 @@ mod tests {
only_runtime: false,
#[cfg(feature = "contract")]
metadata: None,
#[cfg(feature = "contract")]
verifiable: false,
#[cfg(feature = "contract")]
image: None,
})?;

Ok(())
Expand Down Expand Up @@ -474,6 +485,10 @@ mod tests {
only_runtime: false,
#[cfg(feature = "contract")]
metadata: None,
#[cfg(feature = "contract")]
verifiable: false,
#[cfg(feature = "contract")]
image: None,
})?;

// Test 2: Execute with production profile
Expand All @@ -496,6 +511,10 @@ mod tests {
only_runtime: false,
#[cfg(feature = "contract")]
metadata: None,
#[cfg(feature = "contract")]
verifiable: false,
#[cfg(feature = "contract")]
image: None,
})?;

// Test 3: Execute with custom features
Expand All @@ -515,6 +534,10 @@ mod tests {
only_runtime: false,
#[cfg(feature = "contract")]
metadata: None,
#[cfg(feature = "contract")]
verifiable: false,
#[cfg(feature = "contract")]
image: None,
})?;
}

Expand All @@ -538,6 +561,10 @@ mod tests {
only_runtime: false,
#[cfg(feature = "contract")]
metadata: None,
#[cfg(feature = "contract")]
verifiable: false,
#[cfg(feature = "contract")]
image: None,
})?;

// Test 5: Execute with path_pos instead of path
Expand All @@ -560,6 +587,10 @@ mod tests {
only_runtime: false,
#[cfg(feature = "contract")]
metadata: None,
#[cfg(feature = "contract")]
verifiable: false,
#[cfg(feature = "contract")]
image: None,
})?;

// Test 6: Execute with benchmark and try_runtime flags
Expand All @@ -579,6 +610,10 @@ mod tests {
only_runtime: false,
#[cfg(feature = "contract")]
metadata: None,
#[cfg(feature = "contract")]
verifiable: false,
#[cfg(feature = "contract")]
image: None,
})?;
}

Expand Down
12 changes: 9 additions & 3 deletions crates/pop-cli/src/commands/call/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ use clap::Args;
use cliclack::spinner;
use pop_common::{DefaultConfig, Keypair, parse_h160_account};
use pop_contracts::{
CallExec, CallOpts, ContractCallable, ContractFunction, ContractStorage, DefaultEnvironment,
Verbosity, Weight, build_smart_contract, call_smart_contract,
BuildMode, CallExec, CallOpts, ContractCallable, ContractFunction, ContractStorage,
DefaultEnvironment, Verbosity, Weight, build_smart_contract, call_smart_contract,
call_smart_contract_from_signed_payload, dry_run_gas_estimate_call, fetch_contract_storage,
get_call_payload, get_contract_storage_info, get_messages, set_up_call,
};
Expand Down Expand Up @@ -202,7 +202,13 @@ impl CallContractCommand {
cli.warning("NOTE: contract has not yet been built.")?;
let spinner = spinner();
spinner.start("Building contract in RELEASE mode...");
let result = match build_smart_contract(&project_path, true, Verbosity::Quiet, None) {
let result = match build_smart_contract(
&project_path,
BuildMode::Release,
Verbosity::Quiet,
None,
None,
) {
Ok(result) => result,
Err(e) => {
return Err(anyhow!(format!(
Expand Down
10 changes: 8 additions & 2 deletions crates/pop-cli/src/commands/up/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use clap::Args;
use cliclack::spinner;
use console::Emoji;
use pop_contracts::{
FunctionType, UpOpts, Verbosity, Weight, build_smart_contract,
BuildMode, FunctionType, UpOpts, Verbosity, Weight, build_smart_contract,
dry_run_gas_estimate_instantiate, dry_run_upload, extract_function, get_contract_code,
get_instantiate_payload, get_upload_payload, instantiate_contract_signed,
instantiate_smart_contract, is_chain_alive, run_eth_rpc_node, run_ink_node, set_up_deployment,
Expand Down Expand Up @@ -160,7 +160,13 @@ impl UpContractCommand {
}
let spinner = spinner();
spinner.start("Building contract in RELEASE mode...");
let result = match build_smart_contract(&self.path, true, Verbosity::Quiet, None) {
let result = match build_smart_contract(
&self.path,
BuildMode::Release,
Verbosity::Quiet,
None,
None,
) {
Ok(result) => result,
Err(e) => {
Cli.outro_cancel(format!("🚫 An error occurred building your contract: {e}\nUse `pop build` to retry with build output."))?;
Expand Down
Loading
Loading