From 8f48d8161966a06ed29c50463fb8966de59c38c9 Mon Sep 17 00:00:00 2001 From: max-dfinity <100170574+max-dfinity@users.noreply.github.com> Date: Mon, 2 Dec 2024 15:25:26 -0800 Subject: [PATCH] refactor(nns): move cast_vote_and_cascade_following tests to voting.rs (#2930) Move the tests into a more relevant place. --- rs/nns/governance/src/governance/tests/mod.rs | 238 ------------------ rs/nns/governance/src/voting.rs | 217 +++++++++++++++- 2 files changed, 215 insertions(+), 240 deletions(-) diff --git a/rs/nns/governance/src/governance/tests/mod.rs b/rs/nns/governance/src/governance/tests/mod.rs index c6091e8ccf1..5e7f9a70aed 100644 --- a/rs/nns/governance/src/governance/tests/mod.rs +++ b/rs/nns/governance/src/governance/tests/mod.rs @@ -1134,244 +1134,6 @@ mod neuron_archiving_tests { } // end proptest } -mod cast_vote_and_cascade_follow { - use crate::{ - governance::{Governance, MIN_DISSOLVE_DELAY_FOR_VOTE_ELIGIBILITY_SECONDS}, - neuron::{DissolveStateAndAge, Neuron, NeuronBuilder}, - pb::v1::{neuron::Followees, Ballot, ProposalData, Topic, Vote}, - test_utils::{MockEnvironment, StubCMC, StubIcpLedger}, - }; - use futures::FutureExt; - use ic_base_types::PrincipalId; - use ic_nns_common::pb::v1::{NeuronId, ProposalId}; - use icp_ledger::Subaccount; - use maplit::{btreemap, hashmap}; - use std::collections::{BTreeMap, HashMap}; - - fn make_ballot(voting_power: u64, vote: Vote) -> Ballot { - Ballot { - voting_power, - vote: vote as i32, - } - } - - fn make_test_neuron_with_followees( - id: u64, - topic: Topic, - followees: Vec, - aging_since_timestamp_seconds: u64, - ) -> Neuron { - NeuronBuilder::new( - NeuronId { id }, - Subaccount::try_from(&[0u8; 32] as &[u8]).unwrap(), - PrincipalId::new_user_test_id(1), - DissolveStateAndAge::NotDissolving { - dissolve_delay_seconds: MIN_DISSOLVE_DELAY_FOR_VOTE_ELIGIBILITY_SECONDS, - aging_since_timestamp_seconds, - }, - 123_456_789, - ) - .with_followees(hashmap! { - topic as i32 => Followees { - followees: followees.into_iter().map(|id| NeuronId { id }).collect() - } - }) - .build() - } - - #[test] - fn test_cast_vote_and_cascade_doesnt_cascade_neuron_management() { - let now = 1000; - let topic = Topic::NeuronManagement; - - let make_neuron = |id: u64, followees: Vec| { - make_test_neuron_with_followees(id, topic, followees, now) - }; - - let add_neuron_with_ballot = |neuron_map: &mut BTreeMap, - ballots: &mut HashMap, - id: u64, - followees: Vec, - vote: Vote| { - let neuron = make_neuron(id, followees); - let deciding_voting_power = neuron.deciding_voting_power(now); - neuron_map.insert(id, neuron); - ballots.insert(id, make_ballot(deciding_voting_power, vote)); - }; - - let add_neuron_without_ballot = - |neuron_map: &mut BTreeMap, id: u64, followees: Vec| { - let neuron = make_neuron(id, followees); - neuron_map.insert(id, neuron); - }; - - let mut heap_neurons = BTreeMap::new(); - let mut ballots = HashMap::new(); - for id in 1..=5 { - // Each neuron follows all neurons with a lower id - let followees = (1..id).collect(); - - add_neuron_with_ballot( - &mut heap_neurons, - &mut ballots, - id, - followees, - Vote::Unspecified, - ); - } - // Add another neuron that follows both a neuron with a ballot and without a ballot - add_neuron_with_ballot( - &mut heap_neurons, - &mut ballots, - 6, - vec![1, 7], - Vote::Unspecified, - ); - - // Add a neuron without a ballot for neuron 6 to follow. - add_neuron_without_ballot(&mut heap_neurons, 7, vec![1]); - - let governance_proto = crate::pb::v1::Governance { - neurons: heap_neurons - .into_iter() - .map(|(id, neuron)| (id, neuron.into_proto(now))) - .collect(), - proposals: btreemap! { - 1 => ProposalData { - id: Some(ProposalId {id: 1}), - ballots, - ..Default::default() - } - }, - ..Default::default() - }; - let mut governance = Governance::new( - governance_proto, - Box::new(MockEnvironment::new(Default::default(), 0)), - Box::new(StubIcpLedger {}), - Box::new(StubCMC {}), - ); - - governance - .cast_vote_and_cascade_follow( - ProposalId { id: 1 }, - NeuronId { id: 1 }, - Vote::Yes, - topic, - ) - .now_or_never() - .unwrap(); - - let deciding_voting_power = |neuron_id| { - governance - .neuron_store - .with_neuron(&neuron_id, |n| n.deciding_voting_power(now)) - .unwrap() - }; - assert_eq!( - governance.heap_data.proposals.get(&1).unwrap().ballots, - hashmap! { - 1 => make_ballot(deciding_voting_power(NeuronId { id: 1}), Vote::Yes), - 2 => make_ballot(deciding_voting_power(NeuronId { id: 2}), Vote::Unspecified), - 3 => make_ballot(deciding_voting_power(NeuronId { id: 3}), Vote::Unspecified), - 4 => make_ballot(deciding_voting_power(NeuronId { id: 4}), Vote::Unspecified), - 5 => make_ballot(deciding_voting_power(NeuronId { id: 5}), Vote::Unspecified), - 6 => make_ballot(deciding_voting_power(NeuronId { id: 6}), Vote::Unspecified), - } - ); - } - - #[test] - fn test_cast_vote_and_cascade_works() { - let now = 1000; - let topic = Topic::NetworkCanisterManagement; - - let make_neuron = |id: u64, followees: Vec| { - make_test_neuron_with_followees(id, topic, followees, now) - }; - - let add_neuron_with_ballot = |neuron_map: &mut BTreeMap, - ballots: &mut HashMap, - id: u64, - followees: Vec, - vote: Vote| { - let neuron = make_neuron(id, followees); - let deciding_voting_power = neuron.deciding_voting_power(now); - neuron_map.insert(id, neuron); - ballots.insert(id, make_ballot(deciding_voting_power, vote)); - }; - - let add_neuron_without_ballot = - |neuron_map: &mut BTreeMap, id: u64, followees: Vec| { - let neuron = make_neuron(id, followees); - neuron_map.insert(id, neuron); - }; - - let mut neurons = BTreeMap::new(); - let mut ballots = HashMap::new(); - for id in 1..=5 { - // Each neuron follows all neurons with a lower id - let followees = (1..id).collect(); - - add_neuron_with_ballot(&mut neurons, &mut ballots, id, followees, Vote::Unspecified); - } - // Add another neuron that follows both a neuron with a ballot and without a ballot - add_neuron_with_ballot(&mut neurons, &mut ballots, 6, vec![1, 7], Vote::Unspecified); - - // Add a neuron without a ballot for neuron 6 to follow. - add_neuron_without_ballot(&mut neurons, 7, vec![1]); - - let governance_proto = crate::pb::v1::Governance { - neurons: neurons - .into_iter() - .map(|(id, neuron)| (id, neuron.into_proto(now))) - .collect(), - proposals: btreemap! { - 1 => ProposalData { - id: Some(ProposalId {id: 1}), - ballots, - ..Default::default() - } - }, - ..Default::default() - }; - let mut governance = Governance::new( - governance_proto, - Box::new(MockEnvironment::new(Default::default(), 0)), - Box::new(StubIcpLedger {}), - Box::new(StubCMC {}), - ); - - governance - .cast_vote_and_cascade_follow( - ProposalId { id: 1 }, - NeuronId { id: 1 }, - Vote::Yes, - topic, - ) - .now_or_never() - .unwrap(); - - let deciding_voting_power = |neuron_id| { - governance - .neuron_store - .with_neuron(&neuron_id, |n| n.deciding_voting_power(now)) - .unwrap() - }; - assert_eq!( - governance.heap_data.proposals.get(&1).unwrap().ballots, - hashmap! { - 1 => make_ballot(deciding_voting_power(NeuronId { id: 1 }), Vote::Yes), - 2 => make_ballot(deciding_voting_power(NeuronId { id: 2 }), Vote::Yes), - 3 => make_ballot(deciding_voting_power(NeuronId { id: 3 }), Vote::Yes), - 4 => make_ballot(deciding_voting_power(NeuronId { id: 4 }), Vote::Yes), - 5 => make_ballot(deciding_voting_power(NeuronId { id: 5 }), Vote::Yes), - 6 => make_ballot(deciding_voting_power(NeuronId { id: 6 }), Vote::Unspecified), - } - ); - } -} - #[test] fn test_pre_and_post_upgrade_first_time() { let neuron1 = NeuronProto { diff --git a/rs/nns/governance/src/voting.rs b/rs/nns/governance/src/voting.rs index d9576d8e5ba..a00579c3a49 100644 --- a/rs/nns/governance/src/voting.rs +++ b/rs/nns/governance/src/voting.rs @@ -226,18 +226,27 @@ fn retain_neurons_with_castable_ballots( mod test { use crate::{ - governance::MIN_DISSOLVE_DELAY_FOR_VOTE_ELIGIBILITY_SECONDS, + governance::{Governance, MIN_DISSOLVE_DELAY_FOR_VOTE_ELIGIBILITY_SECONDS}, neuron::{DissolveStateAndAge, Neuron, NeuronBuilder}, neuron_store::NeuronStore, - pb::v1::{neuron::Followees, Ballot, Topic, Vote}, + pb::v1::{neuron::Followees, Ballot, ProposalData, Topic, Vote}, + test_utils::{MockEnvironment, StubCMC, StubIcpLedger}, voting::ProposalVotingStateMachine, }; + use futures::FutureExt; use ic_base_types::PrincipalId; use ic_nns_common::pb::v1::{NeuronId, ProposalId}; use icp_ledger::Subaccount; use maplit::{btreemap, hashmap}; use std::collections::{BTreeMap, BTreeSet, HashMap}; + fn make_ballot(voting_power: u64, vote: Vote) -> Ballot { + Ballot { + voting_power, + vote: vote as i32, + } + } + fn make_neuron( id: u64, cached_neuron_stake_e8s: u64, @@ -267,6 +276,210 @@ mod test { .build() } + #[test] + fn test_cast_vote_and_cascade_doesnt_cascade_neuron_management() { + let now = 1000; + let topic = Topic::NeuronManagement; + + let make_neuron = |id: u64, followees: Vec| { + make_neuron( + id, + 100, + hashmap! {topic.into() => Followees { + followees: followees.into_iter().map(|id| NeuronId { id }).collect(), + }}, + ) + }; + + let add_neuron_with_ballot = |neuron_map: &mut BTreeMap, + ballots: &mut HashMap, + id: u64, + followees: Vec, + vote: Vote| { + let neuron = make_neuron(id, followees); + let deciding_voting_power = neuron.deciding_voting_power(now); + neuron_map.insert(id, neuron); + ballots.insert(id, make_ballot(deciding_voting_power, vote)); + }; + + let add_neuron_without_ballot = + |neuron_map: &mut BTreeMap, id: u64, followees: Vec| { + let neuron = make_neuron(id, followees); + neuron_map.insert(id, neuron); + }; + + let mut heap_neurons = BTreeMap::new(); + let mut ballots = HashMap::new(); + for id in 1..=5 { + // Each neuron follows all neurons with a lower id + let followees = (1..id).collect(); + + add_neuron_with_ballot( + &mut heap_neurons, + &mut ballots, + id, + followees, + Vote::Unspecified, + ); + } + // Add another neuron that follows both a neuron with a ballot and without a ballot + add_neuron_with_ballot( + &mut heap_neurons, + &mut ballots, + 6, + vec![1, 7], + Vote::Unspecified, + ); + + // Add a neuron without a ballot for neuron 6 to follow. + add_neuron_without_ballot(&mut heap_neurons, 7, vec![1]); + + let governance_proto = crate::pb::v1::Governance { + neurons: heap_neurons + .into_iter() + .map(|(id, neuron)| (id, neuron.into_proto(now))) + .collect(), + proposals: btreemap! { + 1 => ProposalData { + id: Some(ProposalId {id: 1}), + ballots, + ..Default::default() + } + }, + ..Default::default() + }; + let mut governance = Governance::new( + governance_proto, + Box::new(MockEnvironment::new(Default::default(), 0)), + Box::new(StubIcpLedger {}), + Box::new(StubCMC {}), + ); + + governance + .cast_vote_and_cascade_follow( + ProposalId { id: 1 }, + NeuronId { id: 1 }, + Vote::Yes, + topic, + ) + .now_or_never() + .unwrap(); + + let deciding_voting_power = |neuron_id| { + governance + .neuron_store + .with_neuron(&neuron_id, |n| n.deciding_voting_power(now)) + .unwrap() + }; + assert_eq!( + governance.heap_data.proposals.get(&1).unwrap().ballots, + hashmap! { + 1 => make_ballot(deciding_voting_power(NeuronId { id: 1}), Vote::Yes), + 2 => make_ballot(deciding_voting_power(NeuronId { id: 2}), Vote::Unspecified), + 3 => make_ballot(deciding_voting_power(NeuronId { id: 3}), Vote::Unspecified), + 4 => make_ballot(deciding_voting_power(NeuronId { id: 4}), Vote::Unspecified), + 5 => make_ballot(deciding_voting_power(NeuronId { id: 5}), Vote::Unspecified), + 6 => make_ballot(deciding_voting_power(NeuronId { id: 6}), Vote::Unspecified), + } + ); + } + + #[test] + fn test_cast_vote_and_cascade_works() { + let now = 1000; + let topic = Topic::NetworkCanisterManagement; + + let make_neuron = |id: u64, followees: Vec| { + make_neuron( + id, + 100, + hashmap! {topic.into() => Followees { + followees: followees.into_iter().map(|id| NeuronId { id }).collect(), + }}, + ) + }; + + let add_neuron_with_ballot = |neuron_map: &mut BTreeMap, + ballots: &mut HashMap, + id: u64, + followees: Vec, + vote: Vote| { + let neuron = make_neuron(id, followees); + let deciding_voting_power = neuron.deciding_voting_power(now); + neuron_map.insert(id, neuron); + ballots.insert(id, make_ballot(deciding_voting_power, vote)); + }; + + let add_neuron_without_ballot = + |neuron_map: &mut BTreeMap, id: u64, followees: Vec| { + let neuron = make_neuron(id, followees); + neuron_map.insert(id, neuron); + }; + + let mut neurons = BTreeMap::new(); + let mut ballots = HashMap::new(); + for id in 1..=5 { + // Each neuron follows all neurons with a lower id + let followees = (1..id).collect(); + + add_neuron_with_ballot(&mut neurons, &mut ballots, id, followees, Vote::Unspecified); + } + // Add another neuron that follows both a neuron with a ballot and without a ballot + add_neuron_with_ballot(&mut neurons, &mut ballots, 6, vec![1, 7], Vote::Unspecified); + + // Add a neuron without a ballot for neuron 6 to follow. + add_neuron_without_ballot(&mut neurons, 7, vec![1]); + + let governance_proto = crate::pb::v1::Governance { + neurons: neurons + .into_iter() + .map(|(id, neuron)| (id, neuron.into_proto(now))) + .collect(), + proposals: btreemap! { + 1 => ProposalData { + id: Some(ProposalId {id: 1}), + ballots, + ..Default::default() + } + }, + ..Default::default() + }; + let mut governance = Governance::new( + governance_proto, + Box::new(MockEnvironment::new(Default::default(), 0)), + Box::new(StubIcpLedger {}), + Box::new(StubCMC {}), + ); + + governance + .cast_vote_and_cascade_follow( + ProposalId { id: 1 }, + NeuronId { id: 1 }, + Vote::Yes, + topic, + ) + .now_or_never() + .unwrap(); + + let deciding_voting_power = |neuron_id| { + governance + .neuron_store + .with_neuron(&neuron_id, |n| n.deciding_voting_power(now)) + .unwrap() + }; + assert_eq!( + governance.heap_data.proposals.get(&1).unwrap().ballots, + hashmap! { + 1 => make_ballot(deciding_voting_power(NeuronId { id: 1 }), Vote::Yes), + 2 => make_ballot(deciding_voting_power(NeuronId { id: 2 }), Vote::Yes), + 3 => make_ballot(deciding_voting_power(NeuronId { id: 3 }), Vote::Yes), + 4 => make_ballot(deciding_voting_power(NeuronId { id: 4 }), Vote::Yes), + 5 => make_ballot(deciding_voting_power(NeuronId { id: 5 }), Vote::Yes), + 6 => make_ballot(deciding_voting_power(NeuronId { id: 6 }), Vote::Unspecified), + } + ); + } + fn add_neuron_with_ballot( neuron_store: &mut NeuronStore, ballots: &mut HashMap,