From cc2d740ff7aca03302c2ebc44fbe15857c9a827f Mon Sep 17 00:00:00 2001 From: peg Date: Fri, 23 Aug 2024 14:58:51 +0200 Subject: [PATCH] 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 | 1 + crates/client/src/client.rs | 34 +++++++++- crates/client/src/errors.rs | 2 + crates/client/src/tests.rs | 123 ++++++++++++++++++++++++++++++++++-- crates/test-cli/README.md | 9 ++- crates/test-cli/src/lib.rs | 28 +++++++- 6 files changed, 186 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5bcdafc26..5e9e1b18d 100644 --- a/CHANGELOG.md +++ b/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)) diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index 2609cfe1b..172ac1f8b 100644 --- a/crates/client/src/client.rs +++ b/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, oracle_data_pointer: Vec, ) -> Result<::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::()?; 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, + rpc: &LegacyRpcMethods, + deployer_pair: &sr25519::Pair, + program_hash: ::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::()? + .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, diff --git a/crates/client/src/errors.rs b/crates/client/src/errors.rs index 54da8b69b..163059b11 100644 --- a/crates/client/src/errors.rs +++ b/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")] diff --git a/crates/client/src/tests.rs b/crates/client/src/tests.rs index 4bc60134f..68667b841 100644 --- a/crates/client/src/tests.rs +++ b/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::::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(); +} diff --git a/crates/test-cli/README.md b/crates/test-cli/README.md index 20f2ecdfa..c37a09970 100644 --- a/crates/test-cli/README.md +++ b/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 diff --git a/crates/test-cli/src/lib.rs b/crates/test-cli/src/lib.rs index d16e37857..78984891f 100644 --- a/crates/test-cli/src/lib.rs +++ b/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, }, + /// 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, + }, /// 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 = ::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