Skip to content

Commit

Permalink
refactor(nns): More strictly represent neuron visibility. (#3697)
Browse files Browse the repository at this point in the history
This is done by introducing a native Visibility type, and making native
Neuron use it. The most interesting difference between this native
Visibility and pb::Visibility is that native Visibility has no
Unspecified value. Also, got rid of Option.

Without the possibility that visibility can be None, we are able to
restore `#[derive(PartialEq)]` on Neuron. Thus, we ditch the custom
`PartialEq` implementation.

Also, made the `known_neuron_data` field NOT `pub` anymore. That way,
Neuron can prevent it from becoming inconsistent with `visibility`.

# References

Closes [NNS1-3239].

[NNS1-3239]: https://dfinity.atlassian.net/browse/NNS1-3239
  • Loading branch information
daniel-wong-dfinity-org authored Jan 31, 2025
1 parent 4d98696 commit d3a3f07
Show file tree
Hide file tree
Showing 12 changed files with 239 additions and 222 deletions.
6 changes: 3 additions & 3 deletions rs/nns/governance/canbench/canbench_results.yml
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ benches:
scopes: {}
list_active_neurons_fund_neurons_stable:
total:
instructions: 2757139
instructions: 2820000
heap_increase: 0
stable_memory_increase: 0
scopes: {}
Expand All @@ -97,7 +97,7 @@ benches:
scopes: {}
list_neurons_ready_to_unstake_maturity_stable:
total:
instructions: 41457093
instructions: 43300000
heap_increase: 0
stable_memory_increase: 0
scopes: {}
Expand All @@ -121,7 +121,7 @@ benches:
scopes: {}
list_ready_to_spawn_neuron_ids_stable:
total:
instructions: 41428996
instructions: 43270000
heap_increase: 0
stable_memory_increase: 0
scopes: {}
Expand Down
15 changes: 6 additions & 9 deletions rs/nns/governance/src/governance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::{
reassemble_governance_proto, split_governance_proto, HeapGovernanceData, XdrConversionRate,
},
migrations::maybe_run_migrations,
neuron::{DissolveStateAndAge, Neuron, NeuronBuilder},
neuron::{DissolveStateAndAge, Neuron, NeuronBuilder, Visibility},
neuron_data_validation::{NeuronDataValidationSummary, NeuronDataValidator},
neuron_store::{
backfill_some_voting_power_refreshed_timestamps, metrics::NeuronSubsetMetrics,
Expand Down Expand Up @@ -63,7 +63,7 @@ use crate::{
ProposalData, ProposalRewardStatus, ProposalStatus, RestoreAgingSummary, RewardEvent,
RewardNodeProvider, RewardNodeProviders, SettleNeuronsFundParticipationRequest,
SettleNeuronsFundParticipationResponse, StopOrStartCanister, Tally, Topic,
UpdateCanisterSettings, UpdateNodeProvider, Visibility, Vote, VotingPowerEconomics,
UpdateCanisterSettings, UpdateNodeProvider, Vote, VotingPowerEconomics,
WaitForQuietState, XdrConversionRate as XdrConversionRatePb,
},
},
Expand Down Expand Up @@ -2303,7 +2303,7 @@ impl Governance {
// neuron is public, and the caller requested that
// public neurons be included (in full_neurons).
|| (include_public_neurons_in_full_neurons
&& neuron.visibility() == Some(Visibility::Public)
&& neuron.visibility() == Visibility::Public
);
if let_caller_read_full_neuron {
full_neurons.push(neuron.clone().into_api(now, self.voting_power_economics()));
Expand Down Expand Up @@ -2332,7 +2332,7 @@ impl Governance {
self.neuron_store
.with_neuron(&neuron_id, |n| KnownNeuron {
id: Some(n.id()),
known_neuron_data: n.known_neuron_data.clone(),
known_neuron_data: n.known_neuron_data().cloned(),
})
.map_err(|e| {
println!(
Expand Down Expand Up @@ -6259,7 +6259,7 @@ impl Governance {
"No neuron ID specified in the request to register a known neuron.",
)
})?;
let known_neuron_data = known_neuron.known_neuron_data.as_ref().ok_or_else(|| {
let known_neuron_data = known_neuron.known_neuron_data.ok_or_else(|| {
GovernanceError::new_with_message(
ErrorType::NotFound,
"No known neuron data specified in the register neuron request.",
Expand Down Expand Up @@ -6300,10 +6300,7 @@ impl Governance {
}

self.with_neuron_mut(&neuron_id, |neuron| {
neuron
.known_neuron_data
.replace(known_neuron_data.clone())
.map(|old_known_neuron_data| old_known_neuron_data.name)
neuron.set_known_neuron_data(known_neuron_data)
})?;

Ok(())
Expand Down
Loading

0 comments on commit d3a3f07

Please sign in to comment.