diff --git a/rs/rosetta-api/icp/client/src/lib.rs b/rs/rosetta-api/icp/client/src/lib.rs index 2a56a0026db..b8b30fb5e08 100644 --- a/rs/rosetta-api/icp/client/src/lib.rs +++ b/rs/rosetta-api/icp/client/src/lib.rs @@ -214,9 +214,12 @@ impl RosettaClient { amount: None, coin_change: None, metadata: Some( - NeuronIdentifierMetadata { neuron_index } - .try_into() - .map_err(|e| anyhow::anyhow!("Failed to convert metadata: {:?}", e))?, + NeuronIdentifierMetadata { + neuron_index, + controller: None, + } + .try_into() + .map_err(|e| anyhow::anyhow!("Failed to convert metadata: {:?}", e))?, ), }]) } @@ -275,9 +278,12 @@ impl RosettaClient { amount: None, coin_change: None, metadata: Some( - NeuronIdentifierMetadata { neuron_index } - .try_into() - .map_err(|e| anyhow::anyhow!("Failed to convert metadata: {:?}", e))?, + NeuronIdentifierMetadata { + neuron_index, + controller: None, + } + .try_into() + .map_err(|e| anyhow::anyhow!("Failed to convert metadata: {:?}", e))?, ), }]) } @@ -300,9 +306,12 @@ impl RosettaClient { amount: None, coin_change: None, metadata: Some( - NeuronIdentifierMetadata { neuron_index } - .try_into() - .map_err(|e| anyhow::anyhow!("Failed to convert metadata: {:?}", e))?, + NeuronIdentifierMetadata { + neuron_index, + controller: None, + } + .try_into() + .map_err(|e| anyhow::anyhow!("Failed to convert metadata: {:?}", e))?, ), }]) } @@ -596,6 +605,7 @@ impl RosettaClient { pub fn build_refresh_voting_power_operations( signer_principal: Principal, neuron_index: u64, + principal_id: Option, ) -> anyhow::Result> { Ok(vec![Operation { operation_identifier: OperationIdentifier { @@ -611,9 +621,12 @@ impl RosettaClient { amount: None, coin_change: None, metadata: Some( - NeuronIdentifierMetadata { neuron_index } - .try_into() - .map_err(|e| anyhow::anyhow!("Failed to convert metadata: {:?}", e))?, + NeuronIdentifierMetadata { + neuron_index, + controller: principal_id.map(PublicKeyOrPrincipal::Principal), + } + .try_into() + .map_err(|e| anyhow::anyhow!("Failed to convert metadata: {:?}", e))?, ), }]) } @@ -1433,6 +1446,7 @@ impl RosettaClient { network_identifier: NetworkIdentifier, signer_keypair: &T, neuron_index: u64, + controller_principal_id: Option, ) -> anyhow::Result where T: RosettaSupportedKeyPair, @@ -1440,6 +1454,7 @@ impl RosettaClient { let refresh_voting_power_operations = RosettaClient::build_refresh_voting_power_operations( signer_keypair.generate_principal_id()?.0, neuron_index, + controller_principal_id, )?; self.make_submit_and_wait_for_transaction( signer_keypair, diff --git a/rs/rosetta-api/icp/src/convert.rs b/rs/rosetta-api/icp/src/convert.rs index f0ab787b948..dedc72a7a30 100644 --- a/rs/rosetta-api/icp/src/convert.rs +++ b/rs/rosetta-api/icp/src/convert.rs @@ -160,7 +160,8 @@ pub fn operations_to_requests( } OperationType::Stake => { validate_neuron_management_op()?; - let NeuronIdentifierMetadata { neuron_index } = o.metadata.clone().try_into()?; + let NeuronIdentifierMetadata { neuron_index, .. } = + o.metadata.clone().try_into()?; state.stake(account, neuron_index)?; } OperationType::SetDissolveTimestamp => { @@ -186,12 +187,14 @@ pub fn operations_to_requests( OperationType::StartDissolving => { validate_neuron_management_op()?; - let NeuronIdentifierMetadata { neuron_index } = o.metadata.clone().try_into()?; + let NeuronIdentifierMetadata { neuron_index, .. } = + o.metadata.clone().try_into()?; state.start_dissolve(account, neuron_index)?; } OperationType::StopDissolving => { validate_neuron_management_op()?; - let NeuronIdentifierMetadata { neuron_index } = o.metadata.clone().try_into()?; + let NeuronIdentifierMetadata { neuron_index, .. } = + o.metadata.clone().try_into()?; state.stop_dissolve(account, neuron_index)?; } OperationType::AddHotkey => { @@ -301,9 +304,17 @@ pub fn operations_to_requests( state.follow(account, pid, neuron_index, topic, followees)?; } OperationType::RefreshVotingPower => { - let NeuronIdentifierMetadata { neuron_index } = o.metadata.clone().try_into()?; + let NeuronIdentifierMetadata { + neuron_index, + controller, + } = o.metadata.clone().try_into()?; validate_neuron_management_op()?; - state.refresh_voting_power(account, neuron_index)?; + // convert from pkp in operation to principal in request. + let pid = match controller { + None => None, + Some(p) => Some(principal_id_from_public_key_or_principal(p)?), + }; + state.refresh_voting_power(account, neuron_index, pid)?; } } } diff --git a/rs/rosetta-api/icp/src/convert/state.rs b/rs/rosetta-api/icp/src/convert/state.rs index ab14dbb01a4..e9977554746 100644 --- a/rs/rosetta-api/icp/src/convert/state.rs +++ b/rs/rosetta-api/icp/src/convert/state.rs @@ -397,12 +397,14 @@ impl State { &mut self, account: icp_ledger::AccountIdentifier, neuron_index: u64, + controller: Option, ) -> Result<(), ApiError> { self.flush()?; self.actions .push(Request::RefreshVotingPower(RefreshVotingPower { account, neuron_index, + controller, })); Ok(()) } diff --git a/rs/rosetta-api/icp/src/convert/tests.rs b/rs/rosetta-api/icp/src/convert/tests.rs index 6808e1f1c66..5af315c7f60 100644 --- a/rs/rosetta-api/icp/src/convert/tests.rs +++ b/rs/rosetta-api/icp/src/convert/tests.rs @@ -48,6 +48,18 @@ impl OperationBuilder { }) } + fn neuron_controller(self, controller: Option) -> Self { + let mut metadata = self.0.metadata.unwrap_or_default(); + metadata.insert( + "controller".to_owned(), + serde_json::to_value(controller).unwrap(), + ); + Self(Operation { + metadata: Some(metadata), + ..self.0 + }) + } + fn build(self) -> Operation { self.0 } @@ -124,6 +136,7 @@ fn test_transfer_and_stake_requests_to_operations() { OperationBuilder::new(3, OperationType::Stake) .account(test_account(2)) .neuron_index(1) + .neuron_controller(None) .build(), ]) ); diff --git a/rs/rosetta-api/icp/src/request.rs b/rs/rosetta-api/icp/src/request.rs index 48e6c7c5267..7df27f34232 100644 --- a/rs/rosetta-api/icp/src/request.rs +++ b/rs/rosetta-api/icp/src/request.rs @@ -155,11 +155,14 @@ impl Request { neuron_index: *neuron_index, controller: controller.map(PublicKeyOrPrincipal::Principal), }), - Request::RefreshVotingPower(RefreshVotingPower { neuron_index, .. }) => { - Ok(RequestType::RefreshVotingPower { - neuron_index: *neuron_index, - }) - } + Request::RefreshVotingPower(RefreshVotingPower { + neuron_index, + controller, + .. + }) => Ok(RequestType::RefreshVotingPower { + neuron_index: *neuron_index, + controller: controller.map(PublicKeyOrPrincipal::Principal), + }), } } @@ -516,13 +519,25 @@ impl TryFrom<&models::Request> for Request { Err(ApiError::invalid_request("Invalid follow request.")) } } - RequestType::RefreshVotingPower { neuron_index } => { + RequestType::RefreshVotingPower { + neuron_index, + controller, + } => { if let Some(Command::RefreshVotingPower(manage_neuron::RefreshVotingPower {})) = manage_neuron()? { + let pid = match controller + .clone() + .map(principal_id_from_public_key_or_principal) + { + None => None, + Some(Ok(pid)) => Some(pid), + Some(Err(e)) => return Err(e), + }; Ok(Request::RefreshVotingPower(RefreshVotingPower { neuron_index: *neuron_index, account, + controller: pid, })) } else { Err(ApiError::invalid_request( diff --git a/rs/rosetta-api/icp/src/request_handler/construction_parse.rs b/rs/rosetta-api/icp/src/request_handler/construction_parse.rs index 8eea124709c..35d54a2d5f3 100644 --- a/rs/rosetta-api/icp/src/request_handler/construction_parse.rs +++ b/rs/rosetta-api/icp/src/request_handler/construction_parse.rs @@ -108,9 +108,10 @@ impl RosettaRequestHandler { neuron_index, controller, } => follow(&mut requests, arg, from, neuron_index, controller)?, - RequestType::RefreshVotingPower { neuron_index } => { - refresh_voting_power(&mut requests, arg, from, neuron_index)? - } + RequestType::RefreshVotingPower { + neuron_index, + controller, + } => refresh_voting_power(&mut requests, arg, from, neuron_index, controller)?, } } @@ -610,15 +611,26 @@ fn refresh_voting_power( arg: Blob, from: AccountIdentifier, neuron_index: u64, + controller: Option, ) -> Result<(), ApiError> { let manage: ManageNeuron = candid::decode_one(arg.0.as_ref()).map_err(|e| { ApiError::internal_error(format!("Could not decode ManageNeuron argument: {:?}", e)) })?; if let Some(Command::RefreshVotingPower(manage_neuron::RefreshVotingPower {})) = manage.command { + let pid = match controller.map(convert::principal_id_from_public_key_or_principal) { + None => None, + Some(Ok(pid)) => Some(pid), + _ => { + return Err(ApiError::invalid_request( + "Invalid refresh voting power request.", + )); + } + }; requests.push(Request::RefreshVotingPower(RefreshVotingPower { neuron_index, account: from, + controller: pid, })); } else { return Err(ApiError::internal_error( diff --git a/rs/rosetta-api/icp/src/request_handler/construction_payloads.rs b/rs/rosetta-api/icp/src/request_handler/construction_payloads.rs index d0b23a0d082..eb3ea494ce6 100644 --- a/rs/rosetta-api/icp/src/request_handler/construction_payloads.rs +++ b/rs/rosetta-api/icp/src/request_handler/construction_payloads.rs @@ -907,11 +907,15 @@ fn handle_refresh_voting_power( ) -> Result<(), ApiError> { let account = req.account; let neuron_index = req.neuron_index; + let controller = req.controller; let command = Command::RefreshVotingPower(manage_neuron::RefreshVotingPower {}); add_neuron_management_payload( - RequestType::RefreshVotingPower { neuron_index }, + RequestType::RefreshVotingPower { + neuron_index, + controller: controller.map(PublicKeyOrPrincipal::Principal), + }, account, - None, + controller, neuron_index, command, payloads, diff --git a/rs/rosetta-api/icp/src/request_types.rs b/rs/rosetta-api/icp/src/request_types.rs index cdb5ef88637..87aead171fe 100644 --- a/rs/rosetta-api/icp/src/request_types.rs +++ b/rs/rosetta-api/icp/src/request_types.rs @@ -106,7 +106,10 @@ pub enum RequestType { }, #[serde(rename = "REFRESH_VOTING_POWER")] #[serde(alias = "RefreshVotingPower")] - RefreshVotingPower { neuron_index: u64 }, + RefreshVotingPower { + neuron_index: u64, + controller: Option, + }, } impl RequestType { @@ -388,6 +391,7 @@ pub struct Follow { pub struct RefreshVotingPower { pub account: icp_ledger::AccountIdentifier, pub neuron_index: u64, + pub controller: Option, } #[derive(Clone, Eq, Ord, PartialOrd, Debug, Deserialize, Serialize)] @@ -496,6 +500,7 @@ impl TryFrom> for SetDissolveTimestampMetadata { pub struct NeuronIdentifierMetadata { #[serde(default)] pub neuron_index: u64, + pub controller: Option, } impl TryFrom> for NeuronIdentifierMetadata { @@ -1093,6 +1098,7 @@ impl TransactionBuilder { metadata: Some( NeuronIdentifierMetadata { neuron_index: *neuron_index, + controller: None, } .try_into()?, ), @@ -1176,6 +1182,7 @@ impl TransactionBuilder { metadata: Some( NeuronIdentifierMetadata { neuron_index: *neuron_index, + controller: None, } .try_into()?, ), @@ -1200,6 +1207,7 @@ impl TransactionBuilder { metadata: Some( NeuronIdentifierMetadata { neuron_index: *neuron_index, + controller: None, } .try_into()?, ), @@ -1481,6 +1489,7 @@ impl TransactionBuilder { let RefreshVotingPower { neuron_index, account, + controller, } = refresh_voting_power; let operation_identifier = self.allocate_op_id(); self.ops.push(Operation { @@ -1494,6 +1503,7 @@ impl TransactionBuilder { metadata: Some( NeuronIdentifierMetadata { neuron_index: *neuron_index, + controller: pkp_from_principal(controller), } .try_into()?, ), diff --git a/rs/rosetta-api/icp/tests/system_tests/test_cases/neuron_management.rs b/rs/rosetta-api/icp/tests/system_tests/test_cases/neuron_management.rs index 7252733304b..a97e7f72822 100644 --- a/rs/rosetta-api/icp/tests/system_tests/test_cases/neuron_management.rs +++ b/rs/rosetta-api/icp/tests/system_tests/test_cases/neuron_management.rs @@ -28,6 +28,7 @@ use lazy_static::lazy_static; use proptest::strategy::Strategy; use proptest::test_runner::Config as TestRunnerConfig; use proptest::test_runner::TestRunner; +use rosetta_core::models::RosettaSupportedKeyPair; use rosetta_core::objects::ObjectMap; use rosetta_core::request_types::CallRequest; use std::{ @@ -1371,60 +1372,122 @@ fn test_list_neurons() { #[test] fn test_refresh_voting_power() { - let rt = Runtime::new().unwrap(); - rt.block_on(async { - let env = RosettaTestingEnvironment::builder() - .with_initial_balances( - vec![( - AccountIdentifier::from(TEST_IDENTITY.sender().unwrap()), - // A hundred million ICP should be enough - icp_ledger::Tokens::from_tokens(100_000_000).unwrap(), - )] - .into_iter() - .collect(), - ) - .with_governance_canister() - .build() - .await; + let mut runner = TestRunner::new(TestRunnerConfig { + max_shrink_iters: 0, + cases: *NUM_TEST_CASES, + ..Default::default() + }); - // Stake the minimum amount 100 million e8s - let staked_amount = 100_000_000u64; - let neuron_index = 0; - let from_subaccount = [0; 32]; + runner + .run( + &(basic_identity_strategy().no_shrink()), + |hot_key_identity| { + let rt = Runtime::new().unwrap(); + rt.block_on(async { + let env = RosettaTestingEnvironment::builder() + .with_initial_balances( + vec![( + AccountIdentifier::from(TEST_IDENTITY.sender().unwrap()), + // A hundred million ICP should be enough + icp_ledger::Tokens::from_tokens(100_000_000).unwrap(), + )] + .into_iter() + .collect(), + ) + .with_governance_canister() + .build() + .await; - env.rosetta_client - .create_neuron( - env.network_identifier.clone(), - &(*TEST_IDENTITY).clone(), - RosettaCreateNeuronArgs::builder(staked_amount.into()) - .with_from_subaccount(from_subaccount) - .with_neuron_index(neuron_index) - .build(), - ) - .await - .unwrap(); + // Stake the minimum amount 100 million e8s + let staked_amount = 100_000_000u64; + let neuron_index = 0; + let from_subaccount = [0; 32]; - let agent = get_test_agent(env.pocket_ic.url().unwrap().port().unwrap()).await; - let neuron = list_neurons(&agent).await.full_neurons[0].to_owned(); - let refresh_timestamp = neuron.voting_power_refreshed_timestamp_seconds.unwrap(); + env.rosetta_client + .create_neuron( + env.network_identifier.clone(), + &(*TEST_IDENTITY).clone(), + RosettaCreateNeuronArgs::builder(staked_amount.into()) + .with_from_subaccount(from_subaccount) + .with_neuron_index(neuron_index) + .build(), + ) + .await + .unwrap(); - // Wait for a second before updating the voting power. This is done so the timestamp is sure to have a different value when refreshed - std::thread::sleep(std::time::Duration::from_secs(1)); - TransactionOperationResults::try_from( - env.rosetta_client - .refresh_voting_power( - env.network_identifier.clone(), - &(*TEST_IDENTITY).clone(), - neuron_index, - ) - .await - .unwrap() - .metadata, + // Add a hot key to the neuron + env.rosetta_client + .add_hot_key( + env.network_identifier.clone(), + &(*TEST_IDENTITY).clone(), + RosettaHotKeyArgs::builder(neuron_index) + .with_principal_id(hot_key_identity.sender().unwrap().into()) + .build(), + ) + .await + .unwrap(); + + let agent = get_test_agent(env.pocket_ic.url().unwrap().port().unwrap()).await; + + // Test with hot key identity. + let neuron = list_neurons(&agent).await.full_neurons[0].to_owned(); + let refresh_timestamp = + neuron.voting_power_refreshed_timestamp_seconds.unwrap(); + + // Wait for a second before updating the voting power. This is done so the timestamp is sure to have a different value when refreshed + std::thread::sleep(std::time::Duration::from_secs(1)); + + let hotkey_caller_identity = Arc::new(hot_key_identity); + TransactionOperationResults::try_from( + env.rosetta_client + .refresh_voting_power( + env.network_identifier.clone(), + &hotkey_caller_identity.clone(), + neuron_index, + (*TEST_IDENTITY).clone().generate_principal_id().ok(), + ) + .await + .unwrap() + .metadata, + ) + .unwrap(); + + let neuron = list_neurons(&agent).await.full_neurons[0].to_owned(); + // The voting power should have been refreshed + assert!( + neuron.voting_power_refreshed_timestamp_seconds.unwrap() + > refresh_timestamp + ); + + // Test with neuron owner's identity, without specifying a controller. + let refresh_timestamp = + neuron.voting_power_refreshed_timestamp_seconds.unwrap(); + + // Wait for a second before updating the voting power. This is done so the timestamp is sure to have a different value when refreshed + std::thread::sleep(std::time::Duration::from_secs(1)); + TransactionOperationResults::try_from( + env.rosetta_client + .refresh_voting_power( + env.network_identifier.clone(), + &(*TEST_IDENTITY).clone(), + neuron_index, + /*controller=*/ None, + ) + .await + .unwrap() + .metadata, + ) + .unwrap(); + + let neuron = list_neurons(&agent).await.full_neurons[0].to_owned(); + // The voting power should have been refreshed + assert!( + neuron.voting_power_refreshed_timestamp_seconds.unwrap() + > refresh_timestamp + ); + }); + Ok(()) + }, ) .unwrap(); - - let neuron = list_neurons(&agent).await.full_neurons[0].to_owned(); - // The voting power should have been refreshed - assert!(neuron.voting_power_refreshed_timestamp_seconds.unwrap() > refresh_timestamp); - }); }