Add remove program function to entropy-client (#1023)

* Add a method to remove a program to entropy-client, and a test

* Add a command to the test-cli to remove a program

* Changelog

* Update tracing spans

* Readme

* Add negative test to try to remove a program which is in use

CHANGELOG.md

@@ -28,6 +28,7 @@ At the moment this project **does not** adhere to
 - Add `network-jumpstart` command to `entropy-test-cli` ([#1004](https://github.com/entropyxyz/entropy-core/pull/1004))
 - Attestation pallet ([#1003](https://github.com/entropyxyz/entropy-core/pull/1003))
 - Update test CLI for new registration and signing flows ([#1008](https://github.com/entropyxyz/entropy-core/pull/1008))
+- Add remove program function to entropy-client ([#1023](https://github.com/entropyxyz/entropy-core/pull/1023))
 
 ### Changed
 - Fix TSS `AccountId` keys in chainspec ([#993](https://github.com/entropyxyz/entropy-core/pull/993))

crates/client/src/client.rs

@@ -215,7 +215,6 @@ pub async fn sign(
 #[tracing::instrument(
     skip_all,
     fields(
-        signature_request_account,
         deployer = ?deployer_pair.public(),
     )
 )]
@@ -228,14 +227,14 @@ pub async fn store_program(
     auxiliary_data_interface: Vec<u8>,
     oracle_data_pointer: Vec<u8>,
 ) -> Result<<EntropyConfig as Config>::Hash, ClientError> {
-    let update_program_tx = entropy::tx().programs().set_program(
+    let set_program_tx = entropy::tx().programs().set_program(
         program,
         configuration_interface,
         auxiliary_data_interface,
         oracle_data_pointer,
     );
     let in_block =
-        submit_transaction_with_pair(api, rpc, deployer_pair, &update_program_tx, None).await?;
+        submit_transaction_with_pair(api, rpc, deployer_pair, &set_program_tx, None).await?;
     let result_event = in_block.find_first::<entropy::programs::events::ProgramCreated>()?;
     Ok(result_event.ok_or(ClientError::CannotConfirmProgramCreated)?.program_hash)
 }
@@ -254,6 +253,35 @@ pub async fn update_programs(
     submit_transaction_with_pair(entropy_api, rpc, deployer_pair, &update_pointer_tx, None).await?;
     Ok(())
 }
+
+/// Removed a stored a program with a given hash
+#[tracing::instrument(
+    skip_all,
+    fields(
+        program_hash,
+        deployer = ?deployer_pair.public(),
+    )
+)]
+pub async fn remove_program(
+    api: &OnlineClient<EntropyConfig>,
+    rpc: &LegacyRpcMethods<EntropyConfig>,
+    deployer_pair: &sr25519::Pair,
+    program_hash: <EntropyConfig as Config>::Hash,
+) -> Result<(), ClientError> {
+    let remove_program_tx = entropy::tx().programs().remove_program(program_hash);
+    let in_block =
+        submit_transaction_with_pair(api, rpc, deployer_pair, &remove_program_tx, None).await?;
+
+    let event = in_block
+        .find_first::<entropy::programs::events::ProgramRemoved>()?
+        .ok_or(ClientError::CannotConfirmProgramRemoved)?;
+
+    if event.old_program_hash != program_hash {
+        return Err(ClientError::CannotConfirmProgramRemoved);
+    }
+    Ok(())
+}
+
 /// Get info on all registered accounts
 pub async fn get_accounts(
     api: &OnlineClient<EntropyConfig>,

crates/client/src/errors.rs

@@ -100,6 +100,8 @@ pub enum ClientError {
     NoSyncedValidators,
     #[error("Cannot confirm program was created")]
     CannotConfirmProgramCreated,
+    #[error("Cannot confirm program was removed")]
+    CannotConfirmProgramRemoved,
     #[error("Subgroup fetch error")]
     SubgroupFetch,
     #[error("Cannot query whether validator is synced")]

crates/client/src/tests.rs

@@ -1,17 +1,28 @@
 use crate::{
     chain_api::{
         entropy::{
-            runtime_types::pallet_staking_extension::pallet::ServerInfo, staking_extension::events,
+            self,
+            runtime_types::{
+                bounded_collections::bounded_vec::BoundedVec,
+                pallet_registry::pallet::ProgramInstance,
+                pallet_staking_extension::pallet::ServerInfo,
+            },
+            staking_extension::events,
         },
-        get_api, get_rpc,
+        get_api, get_rpc, EntropyConfig,
     },
-    change_endpoint, change_threshold_accounts,
+    change_endpoint, change_threshold_accounts, register, remove_program, store_program,
+    substrate::query_chain,
+    update_programs,
+};
+use entropy_testing_utils::{
+    constants::TEST_PROGRAM_WASM_BYTECODE, jump_start_network,
+    substrate_context::test_context_stationary, test_node_process_testing_state,
 };
-use entropy_testing_utils::substrate_context::test_context_stationary;
 use serial_test::serial;
-use sp_core::Pair;
+use sp_core::{sr25519, Pair, H256};
 use sp_keyring::AccountKeyring;
-use subxt::utils::AccountId32;
+use subxt::{tx::PairSigner, utils::AccountId32};
 
 #[tokio::test]
 #[serial]
@@ -68,3 +79,103 @@ async fn test_change_threshold_accounts() {
         )
     );
 }
+
+#[tokio::test]
+#[serial]
+async fn test_store_and_remove_program() {
+    let program_owner = AccountKeyring::Ferdie.pair();
+    let substrate_context = test_context_stationary().await;
+
+    let api = get_api(&substrate_context.node_proc.ws_url).await.unwrap();
+    let rpc = get_rpc(&substrate_context.node_proc.ws_url).await.unwrap();
+
+    // Store a program
+    let program_hash = store_program(
+        &api,
+        &rpc,
+        &program_owner,
+        TEST_PROGRAM_WASM_BYTECODE.to_owned(),
+        vec![],
+        vec![],
+        vec![],
+    )
+    .await
+    .unwrap();
+
+    // Check that the program was stored
+    let program_query = entropy::storage().programs().programs(program_hash);
+    let program_info = query_chain(&api, &rpc, program_query, None).await.unwrap().unwrap();
+    assert_eq!(program_info.deployer.0, program_owner.public().0);
+
+    // Remove the program
+    remove_program(&api, &rpc, &program_owner, program_hash).await.unwrap();
+
+    // Check that the program is no longer stored
+    let program_query = entropy::storage().programs().programs(program_hash);
+    assert!(query_chain(&api, &rpc, program_query, None).await.unwrap().is_none());
+
+    // Removing program fails because program has already been removed
+    assert!(remove_program(&api, &rpc, &program_owner, program_hash).await.is_err());
+}
+
+#[tokio::test]
+#[serial]
+async fn test_remove_program_reference_counter() {
+    let program_owner = AccountKeyring::Ferdie.pair();
+
+    let force_authoring = true;
+    let substrate_context = test_node_process_testing_state(force_authoring).await;
+    let api = get_api(&substrate_context.ws_url).await.unwrap();
+    let rpc = get_rpc(&substrate_context.ws_url).await.unwrap();
+
+    // Jumpstart the network
+    let alice = AccountKeyring::Alice;
+    let signer = PairSigner::<EntropyConfig, sr25519::Pair>::new(alice.clone().into());
+    jump_start_network(&api, &rpc, &signer).await;
+
+    // Store a program
+    let program_pointer = store_program(
+        &api,
+        &rpc,
+        &program_owner,
+        TEST_PROGRAM_WASM_BYTECODE.to_owned(),
+        vec![],
+        vec![],
+        vec![],
+    )
+    .await
+    .unwrap();
+
+    // Register, using that program
+    let register_on_chain = true;
+    let (verifying_key, _registered_info) = register(
+        &api,
+        &rpc,
+        program_owner.clone(),
+        AccountId32(program_owner.public().0),
+        BoundedVec(vec![ProgramInstance { program_pointer, program_config: vec![] }]),
+        register_on_chain,
+    )
+    .await
+    .unwrap();
+
+    // Removing program fails because program is being used
+    assert!(remove_program(&api, &rpc, &program_owner, program_pointer).await.is_err());
+
+    // Now stop using the program
+    update_programs(
+        &api,
+        &rpc,
+        verifying_key,
+        &program_owner,
+        BoundedVec(vec![ProgramInstance {
+            program_pointer: H256([0; 32]),
+            program_config: vec![],
+        }]),
+    )
+    .await
+    .unwrap();
+
+    // We can now remove the program because no-one is using it
+    remove_program(&api, &rpc, &program_owner, program_pointer).await.unwrap();
+}

crates/test-cli/README.md

@@ -140,7 +140,14 @@ a program you can use the `store-program` command.
 You need to give the account which will store the program, and the path to a program binary file you
 wish to store, for example:
 
-`entropy-test-cli store-program ./crates/testing-utils/example_barebones_with_auxilary.wasm //Alice`
+`entropy-test-cli store-program ./crates/testing-utils/example_barebones_with_auxilary.wasm -m //Alice`
+
+### Remove program
+
+To remove a program you need to give the account which 'owns' the program (the one which stored it)
+and the hex-encoded hash of the program you wish to remove, for example:
+
+`entropy-test-cli remove-program a2a16982fa6176e3fa9ae8dc408386ff040bf91196d3ec0aa981e5ba3fc1bbac -m //Alice`
 
 ### Update programs
 

crates/test-cli/src/lib.rs

@@ -26,7 +26,8 @@ use entropy_client::{
     },
     client::{
         change_endpoint, change_threshold_accounts, get_accounts, get_api, get_programs, get_rpc,
-        jumpstart_network, register, sign, store_program, update_programs, VERIFYING_KEY_LENGTH,
+        jumpstart_network, register, remove_program, sign, store_program, update_programs,
+        VERIFYING_KEY_LENGTH,
     },
 };
 use sp_core::{sr25519, Hasher, Pair};
@@ -127,6 +128,14 @@ enum CliCommand {
         #[arg(short, long)]
         mnemonic_option: Option<String>,
     },
+    /// Remove a given program from chain
+    RemoveProgram {
+        /// The 32 bytes hash of the program to remove, encoded as hex
+        hash: String,
+        /// The mnemonic to use for the call, which must be the program deployer
+        #[arg(short, long)]
+        mnemonic_option: Option<String>,
+    },
     /// Allows a validator to change their endpoint
     ChangeEndpoint {
         /// New endpoint to change to (ex. "127.0.0.1:3001")
@@ -283,6 +292,23 @@ pub async fn run_command(
             .await?;
             Ok(format!("Program stored {hash}"))
         },
+        CliCommand::RemoveProgram { mnemonic_option, hash } => {
+            let mnemonic = if let Some(mnemonic_option) = mnemonic_option {
+                mnemonic_option
+            } else {
+                passed_mnemonic.expect("No Mnemonic set")
+            };
+            let keypair = <sr25519::Pair as Pair>::from_string(&mnemonic, None)?;
+            println!("Removing program using account: {}", keypair.public());
+
+            let hash: [u8; 32] = hex::decode(hash)?
+                .try_into()
+                .map_err(|_| anyhow!("Program hash must be 32 bytes"))?;
+
+            remove_program(&api, &rpc, &keypair, H256(hash)).await?;
+
+            Ok("Program removed".to_string())
+        },
         CliCommand::UpdatePrograms { signature_verifying_key, mnemonic_option, programs } => {
             let mnemonic = if let Some(mnemonic_option) = mnemonic_option {
                 mnemonic_option