Skip to content

Commit 9220172

Browse files
refactor(nns): Delete ManageNeuronResponse from governance.proto. (dfinity#3573)
The idea here is very much the same as the [previous PR] in this thread/series. Therefore, please see the description of the aforementioned PR for an overview, and details. [previous PR]:dfinity#3546 One difference here is that we only deleted the response type, not the request type. In this case, deleting the request type is not possible, because it is used (indirectly) by `Governance` (via `Proposal`). This is an unusual situation; most request types do not have this property.
1 parent a4dd638 commit 9220172

File tree

16 files changed

+256
-928
lines changed

16 files changed

+256
-928
lines changed

rs/nns/governance/api/src/ic_nns_governance.pb.v1.rs

Lines changed: 161 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use ic_base_types::PrincipalId;
2-
use ic_nns_common::pb::v1::NeuronId;
2+
use ic_nns_common::pb::v1::{NeuronId, ProposalId};
33
use icp_ledger::protobuf::AccountIdentifier;
44

55
/// The entity that owns the nodes that run the network.
@@ -1276,6 +1276,166 @@ pub mod manage_neuron_response {
12761276
#[prost(message, tag = "14")]
12771277
RefreshVotingPower(RefreshVotingPowerResponse),
12781278
}
1279+
1280+
// Below, we should remove `manage_neuron_response::`, but that should be
1281+
// done later, so that the original PR that transplanted this code does not
1282+
// have "extra" refactoring in it.
1283+
impl ManageNeuronResponse {
1284+
pub fn is_err(&self) -> bool {
1285+
matches!(
1286+
&self.command,
1287+
Some(manage_neuron_response::Command::Error(_))
1288+
)
1289+
}
1290+
1291+
pub fn err_ref(&self) -> Option<&GovernanceError> {
1292+
match &self.command {
1293+
Some(manage_neuron_response::Command::Error(err)) => Some(err),
1294+
_ => None,
1295+
}
1296+
}
1297+
1298+
pub fn err(self) -> Option<GovernanceError> {
1299+
match self.command {
1300+
Some(manage_neuron_response::Command::Error(err)) => Some(err),
1301+
_ => None,
1302+
}
1303+
}
1304+
1305+
pub fn is_ok(&self) -> bool {
1306+
!self.is_err()
1307+
}
1308+
1309+
pub fn panic_if_error(self, msg: &str) -> Self {
1310+
if let Some(manage_neuron_response::Command::Error(err)) = &self.command {
1311+
panic!("{}: {:?}", msg, err);
1312+
}
1313+
self
1314+
}
1315+
1316+
// This is generic so that callers can pass either GovernanceError from
1317+
// the ic_nns_governance crate (notice the lack of "_api" at the end of
1318+
// the name!), in addition to GovernanceError from this crate.
1319+
pub fn error<E>(err: E) -> Self
1320+
where
1321+
GovernanceError: From<E>,
1322+
{
1323+
ManageNeuronResponse {
1324+
command: Some(Command::Error(GovernanceError::from(err))),
1325+
}
1326+
}
1327+
1328+
pub fn configure_response() -> Self {
1329+
ManageNeuronResponse {
1330+
command: Some(manage_neuron_response::Command::Configure(
1331+
manage_neuron_response::ConfigureResponse {},
1332+
)),
1333+
}
1334+
}
1335+
1336+
pub fn disburse_response(transfer_block_height: u64) -> Self {
1337+
ManageNeuronResponse {
1338+
command: Some(manage_neuron_response::Command::Disburse(
1339+
manage_neuron_response::DisburseResponse {
1340+
transfer_block_height,
1341+
},
1342+
)),
1343+
}
1344+
}
1345+
1346+
pub fn spawn_response(created_neuron_id: NeuronId) -> Self {
1347+
let created_neuron_id = Some(created_neuron_id);
1348+
ManageNeuronResponse {
1349+
command: Some(manage_neuron_response::Command::Spawn(
1350+
manage_neuron_response::SpawnResponse { created_neuron_id },
1351+
)),
1352+
}
1353+
}
1354+
1355+
pub fn merge_maturity_response(response: MergeMaturityResponse) -> Self {
1356+
ManageNeuronResponse {
1357+
command: Some(manage_neuron_response::Command::MergeMaturity(response)),
1358+
}
1359+
}
1360+
1361+
pub fn stake_maturity_response(response: StakeMaturityResponse) -> Self {
1362+
ManageNeuronResponse {
1363+
command: Some(manage_neuron_response::Command::StakeMaturity(response)),
1364+
}
1365+
}
1366+
1367+
pub fn follow_response() -> Self {
1368+
ManageNeuronResponse {
1369+
command: Some(manage_neuron_response::Command::Follow(
1370+
manage_neuron_response::FollowResponse {},
1371+
)),
1372+
}
1373+
}
1374+
1375+
pub fn make_proposal_response(proposal_id: ProposalId, message: String) -> Self {
1376+
let proposal_id = Some(proposal_id);
1377+
let message = Some(message);
1378+
ManageNeuronResponse {
1379+
command: Some(manage_neuron_response::Command::MakeProposal(
1380+
manage_neuron_response::MakeProposalResponse {
1381+
proposal_id,
1382+
message,
1383+
},
1384+
)),
1385+
}
1386+
}
1387+
1388+
pub fn register_vote_response() -> Self {
1389+
ManageNeuronResponse {
1390+
command: Some(manage_neuron_response::Command::RegisterVote(
1391+
manage_neuron_response::RegisterVoteResponse {},
1392+
)),
1393+
}
1394+
}
1395+
1396+
pub fn split_response(created_neuron_id: NeuronId) -> Self {
1397+
let created_neuron_id = Some(created_neuron_id);
1398+
ManageNeuronResponse {
1399+
command: Some(manage_neuron_response::Command::Split(
1400+
manage_neuron_response::SplitResponse { created_neuron_id },
1401+
)),
1402+
}
1403+
}
1404+
1405+
pub fn merge_response(merge_response: manage_neuron_response::MergeResponse) -> Self {
1406+
ManageNeuronResponse {
1407+
command: Some(manage_neuron_response::Command::Merge(merge_response)),
1408+
}
1409+
}
1410+
1411+
pub fn disburse_to_neuron_response(created_neuron_id: NeuronId) -> Self {
1412+
let created_neuron_id = Some(created_neuron_id);
1413+
ManageNeuronResponse {
1414+
command: Some(manage_neuron_response::Command::DisburseToNeuron(
1415+
manage_neuron_response::DisburseToNeuronResponse { created_neuron_id },
1416+
)),
1417+
}
1418+
}
1419+
1420+
pub fn claim_or_refresh_neuron_response(refreshed_neuron_id: NeuronId) -> Self {
1421+
let refreshed_neuron_id = Some(refreshed_neuron_id);
1422+
ManageNeuronResponse {
1423+
command: Some(manage_neuron_response::Command::ClaimOrRefresh(
1424+
manage_neuron_response::ClaimOrRefreshResponse {
1425+
refreshed_neuron_id,
1426+
},
1427+
)),
1428+
}
1429+
}
1430+
1431+
pub fn refresh_voting_power_response(_: ()) -> Self {
1432+
ManageNeuronResponse {
1433+
command: Some(manage_neuron_response::Command::RefreshVotingPower(
1434+
manage_neuron_response::RefreshVotingPowerResponse {},
1435+
)),
1436+
}
1437+
}
1438+
}
12791439
}
12801440

12811441
#[derive(candid::CandidType, candid::Deserialize, serde::Serialize, comparable::Comparable)]

rs/nns/governance/api/src/pb.rs

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
use crate::pb::v1::{
2-
governance::migration::MigrationStatus, governance_error::ErrorType, manage_neuron_response,
3-
neuron::DissolveState, CreateServiceNervousSystem, GovernanceError, ManageNeuronResponse,
4-
NetworkEconomics, Neuron, NeuronState, NeuronsFundEconomics,
5-
NeuronsFundMatchedFundingCurveCoefficients, VotingPowerEconomics, XdrConversionRate,
2+
governance::migration::MigrationStatus, governance_error::ErrorType, neuron::DissolveState,
3+
CreateServiceNervousSystem, GovernanceError, NetworkEconomics, Neuron, NeuronState,
4+
NeuronsFundEconomics, NeuronsFundMatchedFundingCurveCoefficients, VotingPowerEconomics,
5+
XdrConversionRate,
66
};
77
use ic_nervous_system_common::{ONE_DAY_SECONDS, ONE_MONTH_SECONDS};
88
use ic_nervous_system_proto::pb::v1::{Decimal, Duration, GlobalTimeOfDay, Percentage};
@@ -16,15 +16,6 @@ pub mod v1;
1616
/// The number of e8s per ICP;
1717
const E8S_PER_ICP: u64 = TOKEN_SUBDIVIDABLE_BY;
1818

19-
impl ManageNeuronResponse {
20-
pub fn panic_if_error(self, msg: &str) -> Self {
21-
if let Some(manage_neuron_response::Command::Error(err)) = &self.command {
22-
panic!("{}: {:?}", msg, err);
23-
}
24-
self
25-
}
26-
}
27-
2819
impl GovernanceError {
2920
pub fn new(error_type: ErrorType) -> Self {
3021
Self {

rs/nns/governance/canbench/canbench_results.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ benches:
5555
scopes: {}
5656
compute_ballots_for_new_proposal_with_stable_neurons:
5757
total:
58-
instructions: 2169168
58+
instructions: 2220000
5959
heap_increase: 0
6060
stable_memory_increase: 0
6161
scopes: {}

rs/nns/governance/canister/canister.rs

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -703,11 +703,9 @@ async fn transfer_gtc_neuron(
703703
#[update]
704704
async fn manage_neuron(_manage_neuron: ManageNeuronRequest) -> ManageNeuronResponse {
705705
debug_log("manage_neuron");
706-
ManageNeuronResponse::from(
707-
governance_mut()
708-
.manage_neuron(&caller(), &(gov_pb::ManageNeuron::from(_manage_neuron)))
709-
.await,
710-
)
706+
governance_mut()
707+
.manage_neuron(&caller(), &(gov_pb::ManageNeuron::from(_manage_neuron)))
708+
.await
711709
}
712710

713711
#[cfg(feature = "test")]
@@ -724,9 +722,7 @@ fn update_neuron(neuron: Neuron) -> Option<GovernanceError> {
724722
#[update]
725723
fn simulate_manage_neuron(manage_neuron: ManageNeuronRequest) -> ManageNeuronResponse {
726724
debug_log("simulate_manage_neuron");
727-
let response =
728-
governance().simulate_manage_neuron(&caller(), gov_pb::ManageNeuron::from(manage_neuron));
729-
ManageNeuronResponse::from(response)
725+
governance().simulate_manage_neuron(&caller(), gov_pb::ManageNeuron::from(manage_neuron))
730726
}
731727

732728
#[query]

rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto

Lines changed: 0 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -1224,88 +1224,6 @@ message ManageNeuron {
12241224
}
12251225
}
12261226

1227-
// The response of the ManageNeuron command
1228-
//
1229-
// There is a dedicated response type for each `ManageNeuron.command` field
1230-
message ManageNeuronResponse {
1231-
message ConfigureResponse {}
1232-
1233-
message DisburseResponse {
1234-
// The block height at which the disburse transfer happened
1235-
uint64 transfer_block_height = 1;
1236-
}
1237-
1238-
message SpawnResponse {
1239-
// The ID of the Neuron created from spawning a Neuron
1240-
ic_nns_common.pb.v1.NeuronId created_neuron_id = 1;
1241-
}
1242-
1243-
message MergeMaturityResponse {
1244-
uint64 merged_maturity_e8s = 1;
1245-
uint64 new_stake_e8s = 2;
1246-
}
1247-
1248-
message StakeMaturityResponse {
1249-
uint64 maturity_e8s = 1;
1250-
uint64 staked_maturity_e8s = 2;
1251-
}
1252-
1253-
message FollowResponse {}
1254-
1255-
message MakeProposalResponse {
1256-
// The ID of the created proposal
1257-
ic_nns_common.pb.v1.ProposalId proposal_id = 1;
1258-
optional string message = 2;
1259-
}
1260-
1261-
message RegisterVoteResponse {}
1262-
1263-
message SplitResponse {
1264-
// The ID of the Neuron created from splitting another Neuron
1265-
ic_nns_common.pb.v1.NeuronId created_neuron_id = 1;
1266-
}
1267-
1268-
// A response for merging or simulating merge neurons
1269-
message MergeResponse {
1270-
// The resulting state of the source neuron
1271-
Neuron source_neuron = 1;
1272-
// The resulting state of the target neuron
1273-
Neuron target_neuron = 2;
1274-
// The NeuronInfo of the source neuron
1275-
NeuronInfo source_neuron_info = 3;
1276-
// The NeuronInfo of the target neuron
1277-
NeuronInfo target_neuron_info = 4;
1278-
}
1279-
1280-
message DisburseToNeuronResponse {
1281-
// The ID of the Neuron created from disbursing a Neuron
1282-
ic_nns_common.pb.v1.NeuronId created_neuron_id = 1;
1283-
}
1284-
1285-
message ClaimOrRefreshResponse {
1286-
ic_nns_common.pb.v1.NeuronId refreshed_neuron_id = 1;
1287-
}
1288-
1289-
message RefreshVotingPowerResponse {}
1290-
1291-
oneof command {
1292-
GovernanceError error = 1;
1293-
ConfigureResponse configure = 2;
1294-
DisburseResponse disburse = 3;
1295-
SpawnResponse spawn = 4;
1296-
FollowResponse follow = 5;
1297-
MakeProposalResponse make_proposal = 6;
1298-
RegisterVoteResponse register_vote = 7;
1299-
SplitResponse split = 8;
1300-
DisburseToNeuronResponse disburse_to_neuron = 9;
1301-
ClaimOrRefreshResponse claim_or_refresh = 10;
1302-
MergeMaturityResponse merge_maturity = 11;
1303-
MergeResponse merge = 12;
1304-
StakeMaturityResponse stake_maturity = 13;
1305-
RefreshVotingPowerResponse refresh_voting_power = 14;
1306-
}
1307-
}
1308-
13091227
message GovernanceError {
13101228
enum ErrorType {
13111229
ERROR_TYPE_UNSPECIFIED = 0;

0 commit comments

Comments
 (0)