Skip to content

Commit

Permalink
refactor(nns): Refactor voting as precursor for more scalability (#2528)
Browse files Browse the repository at this point in the history 
This refactoring creates a state machine to handle voting, which will
create natural break points in the ongoing calculations that are being
done. This will allow us to scale voting with following to an
arbitrarily large number of neurons.

[Next](#2600)
  • Loading branch information
@max-dfinity
max-dfinity authored Nov 15, 2024
1 parent 4e46b92 commit 139b0f2
Show file tree
Hide file tree
Showing 7 changed files with 472 additions and 195 deletions.
14 changes: 2 additions & 12 deletions rs/nns/common/protobuf_generator/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,28 +15,18 @@ pub fn generate_prost_files(proto: ProtoPaths<'_>, out: &Path) {

config.type_attribute(".", "#[derive(serde::Serialize)]");

for message_name in ["CanisterId", "NeuronId", "PrincipalId", "ProposalId"] {
for message_name in ["NeuronId", "ProposalId"] {
config.type_attribute(
format!("ic_nns_common.pb.v1.{message_name}"),
[
"#[derive(candid::CandidType, candid::Deserialize, comparable::Comparable, Eq)]",
"#[derive(PartialOrd, Ord, std::hash::Hash)]",
"#[self_describing]",
]
.join(" "),
);
}

// Ideally, we'd get rid of these idiosyncracies, and just use the above loop for all our
// decorating needs.
config.type_attribute(
"ic_nns_common.pb.v1.NeuronId",
"#[derive(PartialOrd, Ord, std::hash::Hash)]",
);
config.type_attribute(
"ic_nns_common.pb.v1.PrincipalId",
"#[derive(PartialOrd, Ord, std::hash::Hash)]",
);

std::fs::create_dir_all(out).expect("failed to create output directory");
config.out_dir(out);

Expand Down
24 changes: 21 additions & 3 deletions rs/nns/common/src/gen/ic_nns_common.pb.v1.rs
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,33 @@
// This file is @generated by prost-build.
/// A container for a NeuronId blob, which uniquely identifies
/// a Neuron.
#[derive(serde::Serialize, candid::CandidType, candid::Deserialize, comparable::Comparable, Eq)]
#[derive(
serde::Serialize,
candid::CandidType,
candid::Deserialize,
comparable::Comparable,
Eq,
PartialOrd,
Ord,
std::hash::Hash,
)]
#[self_describing]
#[derive(PartialOrd, Ord, std::hash::Hash, Clone, Copy, PartialEq, ::prost::Message)]
#[derive(Clone, Copy, PartialEq, ::prost::Message)]
pub struct NeuronId {
#[prost(uint64, tag = "2")]
pub id: u64,
}
/// The id of a specific proposal.
#[derive(serde::Serialize, candid::CandidType, candid::Deserialize, comparable::Comparable, Eq)]
#[derive(
serde::Serialize,
candid::CandidType,
candid::Deserialize,
comparable::Comparable,
Eq,
PartialOrd,
Ord,
std::hash::Hash,
)]
#[self_describing]
#[derive(Clone, Copy, PartialEq, ::prost::Message)]
pub struct ProposalId {
Expand Down
180 changes: 4 additions & 176 deletions rs/nns/governance/src/governance.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ use rust_decimal_macros::dec;
use std::{
borrow::Cow,
cmp::{max, Ordering},
collections::{BTreeMap, BTreeSet, HashMap, HashSet},
collections::{HashMap, HashSet},
convert::{TryFrom, TryInto},
fmt,
future::Future,
Expand All @@ -135,6 +135,7 @@ mod benches;
pub mod tla_macros;
#[cfg(feature = "tla")]
pub mod tla;
use crate::voting::cast_vote_and_cascade_follow;
#[cfg(feature = "tla")]
pub use tla::{
claim_neuron_desc, split_neuron_desc, tla_update_method, InstrumentationState, ToTla,
Expand Down Expand Up @@ -5597,7 +5598,7 @@ impl Governance {
.expect("Proposer not found.");

// Cast self-vote, including following.
Governance::cast_vote_and_cascade_follow(
cast_vote_and_cascade_follow(
&proposal_id,
&mut proposal_data.ballots,
proposer_id,
Expand Down Expand Up @@ -5756,179 +5757,6 @@ impl Governance {
}
}

/// Register `voting_neuron_id` voting according to
/// `vote_of_neuron` (which must be `yes` or `no`) in 'ballots' and
/// cascade voting according to the following relationships
/// specified in 'followee_index' (mapping followees to followers for
/// the topic) and 'neurons' (which contains a mapping of followers
/// to followees).
/// Cascading only occurs for proposal topics that support following (i.e.,
/// all topics except Topic::NeuronManagement).
pub(crate) fn cast_vote_and_cascade_follow(
proposal_id: &ProposalId,
ballots: &mut HashMap<u64, Ballot>,
voting_neuron_id: &NeuronId,
vote_of_neuron: Vote,
topic: Topic,
neuron_store: &mut NeuronStore,
) {
assert!(topic != Topic::Unspecified);

// This is the induction variable of the loop: a map from
// neuron ID to the neuron's vote - 'yes' or 'no' (other
// values not allowed).
let mut induction_votes = BTreeMap::new();
induction_votes.insert(*voting_neuron_id, vote_of_neuron);

// Retain only neurons that have a ballot that can still be cast. This excludes
// neurons with no ballots or ballots that have already been cast.
fn retain_neurons_with_castable_ballots(
followers: &mut BTreeSet<NeuronId>,
ballots: &HashMap<u64, Ballot>,
) {
followers.retain(|f| {
ballots
.get(&f.id)
// Only retain neurons with unspecified ballots
.map(|b| b.vote == Vote::Unspecified as i32)
// Neurons without ballots are also dropped
.unwrap_or_default()
});
}

loop {
// First, we cast the specified votes (in the first round,
// this will be a single vote) and collect all neurons
// that follow some of the neurons that are voting.
let mut all_followers = BTreeSet::new();
for (k, v) in induction_votes.iter() {
// The new/induction votes cannot be unspecified.
assert!(*v != Vote::Unspecified);
if let Some(k_ballot) = ballots.get_mut(&k.id) {
// Neuron with ID k is eligible to vote.
if k_ballot.vote == (Vote::Unspecified as i32) {
let register_ballot_result =
neuron_store.with_neuron_mut(&NeuronId { id: k.id }, |k_neuron| {
// Register the neuron's ballot in the
// neuron itself.
k_neuron.register_recent_ballot(topic, proposal_id, *v);
});
match register_ballot_result {
Ok(_) => {
// Only update a vote if it was previously unspecified. Following
// can trigger votes for neurons that have already voted (manually)
// and we don't change these votes.
k_ballot.vote = *v as i32;
// Here k is the followee, i.e., the neuron that has just cast a
// vote that may be followed by other neurons.
//
// Insert followers from 'topic'
all_followers.extend(
neuron_store.get_followers_by_followee_and_topic(*k, topic),
);
// Default following doesn't apply to governance or SNS
// decentralization swap proposals.
if ![Topic::Governance, Topic::SnsAndCommunityFund].contains(&topic)
{
// Insert followers from 'Unspecified' (default followers)
all_followers.extend(
neuron_store.get_followers_by_followee_and_topic(
*k,
Topic::Unspecified,
),
);
}
}
Err(e) => {
// The voting neuron not found in the neurons table. This is a bad
// inconsistency, but there is nothing that can be done about it at
// this place.
eprintln!("error in cast_vote_and_cascade_follow when attempting to cast ballot: {:?}", e);
}
}
}
} else {
// A non-eligible voter was specified in
// new/induction votes. We don't compute the
// followers of this neuron as it didn't actually
// vote.
}
}
// Clear the induction_votes, as we are going to compute a
// new set now.
induction_votes.clear();

// Following is not enabled for neuron management proposals
if topic == Topic::NeuronManagement {
return;
}

// Calling "would_follow_ballots" for neurons that cannot vote is wasteful.
retain_neurons_with_castable_ballots(&mut all_followers, ballots);

for f in all_followers.iter() {
let f_vote = match neuron_store.with_neuron(&NeuronId { id: f.id }, |n| {
n.would_follow_ballots(topic, ballots)
}) {
Ok(vote) => vote,
Err(e) => {
// This is a bad inconsistency, but there is
// nothing that can be done about it at this
// place. We somehow have followers recorded that don't exist.
eprintln!("error in cast_vote_and_cascade_follow when gathering induction votes: {:?}", e);
Vote::Unspecified
}
};
if f_vote != Vote::Unspecified {
// f_vote is yes or no, i.e., f_neuron's
// followee relations indicates that it should
// vote now.
induction_votes.insert(*f, f_vote);
}
}
// If induction_votes is empty, the loop will terminate
// here.
if induction_votes.is_empty() {
return;
}
// We now continue to the next iteration of the loop.
// Because induction_votes is not empty, either at least
// one entry in 'ballots' will change from unspecified to
// yes or no, or all_followers will be empty, whence
// induction_votes will become empty.
//
// Thus, for each iteration of the loop, the number of
// entries in 'ballots' that have an unspecified value
// decreases, or else the loop terminates. As nothing is
// added to 'ballots' (or removed for that matter), the
// loop terminates in at most 'ballots.len()+1' steps.
//
// The worst case is attained if there is a linear
// following graph, like this:
//
// X follows A follows B follows C,
//
// where X is not eligible to vote and nobody has
// voted, i.e.,
//
// ballots = {
// A -> unspecified, B -> unspecified, C -> unspecified
// }
//
// In this case, the subsequent values of
// 'induction_votes' will be {C}, {B}, {A}, {X}.
//
// Note that it does not matter if X has followers. As X
// doesn't vote, its followers are not considered.
//
// The above argument also shows how the algorithm deals
// with cycles in the following graph: votes are
// propagated through the graph in a manner similar to the
// breadth-first search (BFS) algorithm. A node is
// explored when it has voted yes or no.
}
}

fn register_vote(
&mut self,
neuron_id: &NeuronId,
Expand Down Expand Up @@ -5995,7 +5823,7 @@ impl Governance {
));
}

Governance::cast_vote_and_cascade_follow(
cast_vote_and_cascade_follow(
// Actually update the ballot, including following.
proposal_id,
&mut proposal.ballots,
Expand Down
3 changes: 2 additions & 1 deletion rs/nns/governance/src/governance/benches.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use crate::{
temporarily_enable_active_neurons_in_stable_memory,
temporarily_enable_stable_memory_following_index,
test_utils::{MockEnvironment, StubCMC, StubIcpLedger},
voting::cast_vote_and_cascade_follow,
};
use canbench_rs::{bench, bench_fn, BenchResult};
use ic_base_types::PrincipalId;
Expand Down Expand Up @@ -307,7 +308,7 @@ fn cast_vote_cascade_helper(strategy: SetUpStrategy, topic: Topic) -> BenchResul

let proposal_id = ProposalId { id: 1 };
bench_fn(|| {
Governance::cast_vote_and_cascade_follow(
cast_vote_and_cascade_follow(
&proposal_id,
&mut ballots,
&neuron_id.into(),
Expand Down
7 changes: 4 additions & 3 deletions rs/nns/governance/src/governance/tests/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1136,10 +1136,11 @@ mod neuron_archiving_tests {

mod cast_vote_and_cascade_follow {
use crate::{
governance::{Governance, MIN_DISSOLVE_DELAY_FOR_VOTE_ELIGIBILITY_SECONDS},
governance::MIN_DISSOLVE_DELAY_FOR_VOTE_ELIGIBILITY_SECONDS,
neuron::{DissolveStateAndAge, Neuron, NeuronBuilder},
neuron_store::NeuronStore,
pb::v1::{neuron::Followees, Ballot, Topic, Vote},
voting::cast_vote_and_cascade_follow,
};
use ic_base_types::PrincipalId;
use ic_nns_common::pb::v1::{NeuronId, ProposalId};
Expand Down Expand Up @@ -1232,7 +1233,7 @@ mod cast_vote_and_cascade_follow {

let mut neuron_store = NeuronStore::new(heap_neurons);

Governance::cast_vote_and_cascade_follow(
cast_vote_and_cascade_follow(
&ProposalId { id: 1 },
&mut ballots,
&NeuronId { id: 1 },
Expand Down Expand Up @@ -1301,7 +1302,7 @@ mod cast_vote_and_cascade_follow {

let mut neuron_store = NeuronStore::new(neurons);

Governance::cast_vote_and_cascade_follow(
cast_vote_and_cascade_follow(
&ProposalId { id: 1 },
&mut ballots,
&NeuronId { id: 1 },
Expand Down
1 change: 1 addition & 0 deletions rs/nns/governance/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ pub mod proposals;
mod reward;
pub mod storage;
mod subaccount_index;
mod voting;

/// Limit the amount of work for skipping unneeded data on the wire when parsing Candid.
/// The value of 10_000 follows the Candid recommendation.
Expand Down
Loading

0 comments on commit 139b0f2

Please sign in to comment.