Add remove command (#837)

* first version of remove command integration

* import remove command

* fix some type errors for code_hash

* optimize parse_code_hash function use

* remove rpc call and code use

* get last changes

* use new way to get code_hash

* fix fmt

* change the way to get code hash

* add remove command integration test

* fix fmt

* ignore integration tests

* complete documentation for remove command

* remove unused parse code hash function

* complete remove command documentation with code-hash

* fix previous merge of last changes

* fix previous merge of last changes

* fix: fix code_hash check but still error variant issue

* fix code-hash handle

* optimize get hash and error message

* fix documentation

* improve code_hash

* fix error handling

* Add api imports

* Use correct event type

* Fix extrinsics.md

* Fix extrinsics.md

* Wrap long error message

* Error message formatting

---------

Co-authored-by: Andrew Jones <[email protected]>

CHANGELOG.md

@@ -33,6 +33,9 @@ First release candidate for compatibility with `ink! 4.0-rc`.
 ### Changed
 -  Extrinsics: allow specifying contract artifact directly [#893](https://github.com/paritytech/cargo-contract/pull/893)
 
+### Added
+- Add `cargo contract remove` command [#837](https://github.com/paritytech/cargo-contract/pull/837)
+
 ## [2.0.0-beta.2] - 2023-01-09
 
 ### Changed

README.md

@@ -117,6 +117,10 @@ This can be either an event, an invocation of a contract message, or an invocati
 
 The argument has to be given as hex-encoding, starting with `0x`.
 
+##### `cargo contract remove`
+
+Remove a contract from a `pallet-contracts` enabled chain. See [extrinsics](docs/extrinsics.md).
+
 ## License
 
 The entire code within this repository is licensed under the [GPLv3](LICENSE).

crates/cargo-contract/src/cmd/extrinsics/integration_tests.rs

@@ -193,22 +193,14 @@ async fn build_upload_instantiate_call() {
         .output()
         .expect("failed to execute process");
     println!("status: {}", output.status);
-    let stdout = str::from_utf8(&output.stdout).unwrap();
     let stderr = str::from_utf8(&output.stderr).unwrap();
     assert!(output.status.success(), "upload code failed: {stderr}");
 
-    // find the code hash in the output
-    let regex = regex::Regex::new("0x([0-9A-Fa-f]+)").unwrap();
-    let caps = regex.captures(stdout).expect("Failed to find codehash");
-    let code_hash = caps.get(1).unwrap().as_str();
-    assert_eq!(64, code_hash.len());
-
-    tracing::debug!("Instantiating the contract with code hash `{}`", code_hash);
+    tracing::debug!("Instantiating the contract");
     let output = cargo_contract(project_path.as_path())
         .arg("instantiate")
         .args(["--constructor", "new"])
         .args(["--args", "true"])
-        .args(["--code-hash", code_hash])
         .args(["--suri", "//Alice"])
         .output()
         .expect("failed to execute process");
@@ -253,3 +245,59 @@ async fn build_upload_instantiate_call() {
     // prevent the node_process from being dropped and killed
     let _ = node_process;
 }
+
+/// Sanity test the whole lifecycle of:
+/// build -> upload -> remove
+#[ignore]
+#[async_std::test]
+async fn build_upload_remove() {
+    init_tracing_subscriber();
+
+    let tmp_dir = tempfile::Builder::new()
+        .prefix("cargo-contract.cli.test.")
+        .tempdir()
+        .expect("temporary directory creation failed");
+
+    // Spawn the contracts node
+    let node_process = ContractsNodeProcess::spawn(CONTRACTS_NODE)
+        .await
+        .expect("Error spawning contracts node");
+
+    // cargo contract new flipper
+    cargo_contract(tmp_dir.path())
+        .arg("new")
+        .arg("incrementer")
+        .assert()
+        .success();
+
+    // cd incrementer
+    let mut project_path = tmp_dir.path().to_path_buf();
+    project_path.push("incrementer");
+
+    tracing::debug!("Building contract in {}", project_path.to_string_lossy());
+    cargo_contract(project_path.as_path())
+        .arg("build")
+        .assert()
+        .success();
+
+    tracing::debug!("Uploading the code to the substrate-contracts-node chain");
+    let output = cargo_contract(project_path.as_path())
+        .arg("upload")
+        .args(["--suri", "//Alice"])
+        .output()
+        .expect("failed to execute process");
+    println!("status: {}", output.status);
+    let stderr = str::from_utf8(&output.stderr).unwrap();
+    assert!(output.status.success(), "upload code failed: {stderr}");
+
+    tracing::debug!("Removing the contract");
+    let output = cargo_contract(project_path.as_path())
+        .arg("remove")
+        .args(["--suri", "//Alice"])
+        .output()
+        .expect("failed to execute process");
+    let stderr = str::from_utf8(&output.stderr).unwrap();
+    assert!(output.status.success(), "remove failed: {stderr}");
+
+    let _ = node_process;
+}

crates/cargo-contract/src/cmd/extrinsics/mod.rs

@@ -19,6 +19,7 @@ mod call;
 mod error;
 mod events;
 mod instantiate;
+mod remove;
 mod runtime_api;
 mod upload;
 
@@ -47,6 +48,7 @@ use std::{
 };
 
 use crate::DEFAULT_KEY_COL_WIDTH;
+
 use contract_build::{
     name_value_println,
     CrateMetadata,
@@ -86,6 +88,7 @@ use contract_metadata::ContractMetadata;
 pub use contract_transcode::ContractMessageTranscoder;
 pub use error::ErrorVariant;
 pub use instantiate::InstantiateCommand;
+pub use remove::RemoveCommand;
 pub use subxt::PolkadotConfig as DefaultConfig;
 pub use upload::UploadCommand;
 
@@ -431,6 +434,17 @@ fn print_gas_required_success(gas: Weight) {
     );
 }
 
+/// Parse a hex encoded 32 byte hash. Returns error if not exactly 32 bytes.
+pub fn parse_code_hash(input: &str) -> Result<<DefaultConfig as Config>::Hash> {
+    let bytes = contract_build::util::decode_hex(input)?;
+    if bytes.len() != 32 {
+        anyhow::bail!("Code hash should be 32 bytes in length")
+    }
+    let mut arr = [0u8; 32];
+    arr.copy_from_slice(&bytes);
+    Ok(arr.into())
+}
+
 /// Copy of `pallet_contracts_primitives::StorageDeposit` which implements `Serialize`, required
 /// for json output.
 #[derive(Eq, PartialEq, Ord, PartialOrd, Clone, serde::Serialize)]
@@ -459,3 +473,22 @@ impl From<&pallet_contracts_primitives::StorageDeposit<Balance>> for StorageDepo
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn parse_code_hash_works() {
+        // with 0x prefix
+        assert!(parse_code_hash(
+            "0xd43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d"
+        )
+        .is_ok());
+        // without 0x prefix
+        assert!(parse_code_hash(
+            "d43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d"
+        )
+        .is_ok())
+    }
+}

crates/cargo-contract/src/cmd/extrinsics/remove.rs

@@ -0,0 +1,145 @@
+// Copyright 2018-2022 Parity Technologies (UK) Ltd.
+// This file is part of cargo-contract.
+//
+// cargo-contract is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// cargo-contract is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with cargo-contract.  If not, see <http://www.gnu.org/licenses/>.
+
+use super::{
+    runtime_api::api::{
+        self,
+        contracts::events::CodeRemoved,
+    },
+    submit_extrinsic,
+    Client,
+    CodeHash,
+    ContractMessageTranscoder,
+    DefaultConfig,
+    ExtrinsicOpts,
+    PairSigner,
+    TokenMetadata,
+};
+use crate::{
+    cmd::extrinsics::{
+        events::DisplayEvents,
+        parse_code_hash,
+        ErrorVariant,
+    },
+    name_value_println,
+};
+use anyhow::Result;
+use std::fmt::Debug;
+use subxt::{
+    Config,
+    OnlineClient,
+};
+
+#[derive(Debug, clap::Args)]
+#[clap(name = "remove", about = "Remove a contract's code")]
+pub struct RemoveCommand {
+    /// The hash of the smart contract code already uploaded to the chain.
+    #[clap(long, value_parser = parse_code_hash)]
+    code_hash: Option<<DefaultConfig as Config>::Hash>,
+    #[clap(flatten)]
+    extrinsic_opts: ExtrinsicOpts,
+    /// Export the call output in JSON format.
+    #[clap(long, conflicts_with = "verbose")]
+    output_json: bool,
+}
+
+impl RemoveCommand {
+    pub fn is_json(&self) -> bool {
+        self.output_json
+    }
+
+    pub fn run(&self) -> Result<(), ErrorVariant> {
+        let artifacts = self.extrinsic_opts.contract_artifacts()?;
+        let transcoder = artifacts.contract_transcoder()?;
+        let signer = super::pair_signer(self.extrinsic_opts.signer()?);
+
+        let artifacts_path = artifacts.artifact_path().to_path_buf();
+
+        let final_code_hash = match (self.code_hash.as_ref(), artifacts.code.as_ref()) {
+            (Some(code_h), _) => {
+                Ok(code_h.0)
+            }
+            (None, Some(_)) => {
+                artifacts.code_hash()
+            }
+            (None, None) => {
+                Err(anyhow::anyhow!(
+                    "No code_hash was provided or contract code was not found from artifact \
+                     file {}. Please provide a code hash with --code-hash argument or specify the \
+                     path for artifacts files with --manifest-path",
+                    artifacts_path.display()
+                ))
+            }
+        }?;
+
+        async_std::task::block_on(async {
+            let url = self.extrinsic_opts.url_to_string();
+            let client = OnlineClient::from_url(url.clone()).await?;
+            if let Some(code_removed) = self
+                .remove_code(
+                    &client,
+                    sp_core::H256(final_code_hash),
+                    &signer,
+                    &transcoder,
+                )
+                .await?
+            {
+                let remove_result = code_removed.code_hash;
+
+                if self.output_json {
+                    println!("{}", &remove_result);
+                } else {
+                    name_value_println!("Code hash", format!("{remove_result:?}"));
+                }
+                Result::<(), ErrorVariant>::Ok(())
+            } else {
+                let error_code_hash = hex::encode(final_code_hash);
+                Err(anyhow::anyhow!(
+                    "Error removing the code for the supplied code hash: {}",
+                    error_code_hash
+                )
+                .into())
+            }
+        })
+    }
+
+    async fn remove_code(
+        &self,
+        client: &Client,
+        code_hash: CodeHash,
+        signer: &PairSigner,
+        transcoder: &ContractMessageTranscoder,
+    ) -> Result<Option<CodeRemoved>, ErrorVariant> {
+        let call = api::tx()
+            .contracts()
+            .remove_code(sp_core::H256(code_hash.0));
+
+        let result = submit_extrinsic(client, &call, signer).await?;
+        let display_events =
+            DisplayEvents::from_events(&result, Some(transcoder), &client.metadata())?;
+
+        let output = if self.output_json {
+            display_events.to_json()?
+        } else {
+            let token_metadata = TokenMetadata::query(client).await?;
+            display_events
+                .display_events(self.extrinsic_opts.verbosity()?, &token_metadata)?
+        };
+        println!("{output}");
+        let code_removed = result.find_first::<CodeRemoved>()?;
+        Ok(code_removed)
+    }
+}

crates/cargo-contract/src/cmd/mod.rs

@@ -30,5 +30,6 @@ pub(crate) use self::extrinsics::{
     CallCommand,
     ErrorVariant,
     InstantiateCommand,
+    RemoveCommand,
     UploadCommand,
 };

crates/cargo-contract/src/main.rs

@@ -25,6 +25,7 @@ use self::cmd::{
     DecodeCommand,
     ErrorVariant,
     InstantiateCommand,
+    RemoveCommand,
     UploadCommand,
 };
 use contract_build::{
@@ -122,6 +123,9 @@ enum Command {
     /// Decodes a contracts input or output data (supplied in hex-encoding)
     #[clap(name = "decode")]
     Decode(DecodeCommand),
+    /// Remove contract code
+    #[clap(name = "remove")]
+    Remove(RemoveCommand),
 }
 
 fn main() {
@@ -181,6 +185,11 @@ fn exec(cmd: Command) -> Result<()> {
                 .map_err(|err| map_extrinsic_err(err, call.is_json()))
         }
         Command::Decode(decode) => decode.run().map_err(format_err),
+        Command::Remove(remove) => {
+            remove
+                .run()
+                .map_err(|err| map_extrinsic_err(err, remove.is_json()))
+        }
     }
 }
 

docs/extrinsics.md

@@ -97,16 +97,34 @@ cargo contract call \
 - `--message` the name of the contract message to invoke.
 - `--args` accepts a space separated list of values, encoded in order as the arguments of the message to invoke.
 
+### `remove`
+
+Remove the Wasm code of the contract to the target chain. Invokes the [`remove_code`](https://github.com/paritytech/substrate/blob/master/frame/contracts/src/lib.rs#L581)
+dispatchable.
+
+e.g. `cargo contract remove --suri //Alice`
+
+Assumes that `cargo contract build` and `cargo contract upload` have already been run to produce the contract artifacts.
+This command will only succeed if there are no contract instances of this code. Contracts which have already been instantiated from this code must either `terminate` themselves or have their code changed via a `set_code` call to `pallet_contracts`.
+
+```
+cargo contract remove \
+       --suri //Alice \
+       --code-hash 0xbc1b42256696c8a4187ec3ed79fc602789fc11287c4c30926f5e31ed8169574e
+```
+
+- `--code-hash` the hash of the uploaded code, returned from a call to `contract upload`. 
+If not specified the code hash will be taken from the contract artifacts. 
+
 ## Specifying the contract artifact
 
 The above examples assume the working directory is the contract source code where the `Cargo.toml` file is located.
 This is used to determine the location of the contract artifacts. Alternatively, there is an optional positional
 argument to each of the extrinsic commands which allows specifying the contract artifact file directly. E.g.
 
-
-`cargo upload ../path/to/mycontract.wasm`
-`cargo instantiate ../path/to/mycontract.contract`
-`cargo call ..path/to/mycontract.json`
+- `cargo upload ../path/to/mycontract.wasm`
+- `cargo instantiate ../path/to/mycontract.contract`
+- `cargo call ..path/to/mycontract.json`