fix(icp-rosetta): [FI-1649] support refresh voting power with hot key (…

…#4050) Callers should be able to refresh voting power in Rosetta ICP while signing with hotkeys, but currently Rosetta derives the neuron subaccount from the signer key, so if the signature is from the hotkey rather than the neuron owner, Rosetta is not able to derive the real neuron subaccount. This PR adds the `controller` field to the refresh voting power operation metadata, with which a hotkey signer can specify the real controller of a neuron so that Rosetta can correctly derive its subaccount. This was successfully tested using real neurons in mainnet.
dfinity · Feb 21, 2025 · e3d3eb8 · e3d3eb8
1 parent 5a20302
commit e3d3eb8
Show file tree

Hide file tree

Showing 9 changed files with 224 additions and 79 deletions.
diff --git 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<PrincipalId>,
     ) -> anyhow::Result<Vec<Operation>> {
         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,13 +1446,15 @@ impl RosettaClient {
         network_identifier: NetworkIdentifier,
         signer_keypair: &T,
         neuron_index: u64,
+        controller_principal_id: Option<PrincipalId>,
     ) -> anyhow::Result<ConstructionSubmitResponse>
     where
         T: RosettaSupportedKeyPair,
     {
         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
@@ -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
@@ -397,12 +397,14 @@ impl State {
         &mut self,
         account: icp_ledger::AccountIdentifier,
         neuron_index: u64,
+        controller: Option<PrincipalId>,
     ) -> 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
@@ -48,6 +48,18 @@ impl OperationBuilder {
         })
     }
 
+    fn neuron_controller(self, controller: Option<PublicKeyOrPrincipal>) -> 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
@@ -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
@@ -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<PublicKeyOrPrincipal>,
 ) -> 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
@@ -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
@@ -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<PublicKeyOrPrincipal>,
+    },
 }
 
 impl RequestType {
@@ -388,6 +391,7 @@ pub struct Follow {
 pub struct RefreshVotingPower {
     pub account: icp_ledger::AccountIdentifier,
     pub neuron_index: u64,
+    pub controller: Option<PrincipalId>,
 }
 
 #[derive(Clone, Eq, Ord, PartialOrd, Debug, Deserialize, Serialize)]
@@ -496,6 +500,7 @@ impl TryFrom<Option<ObjectMap>> for SetDissolveTimestampMetadata {
 pub struct NeuronIdentifierMetadata {
     #[serde(default)]
     pub neuron_index: u64,
+    pub controller: Option<PublicKeyOrPrincipal>,
 }
 
 impl TryFrom<Option<ObjectMap>> 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()?,
             ),