diff --git a/.github/workflows/run-tests-on-push-to-main.yml b/.github/workflows/run-tests-on-push-to-main.yml index 7bd62060..5cd34a6e 100644 --- a/.github/workflows/run-tests-on-push-to-main.yml +++ b/.github/workflows/run-tests-on-push-to-main.yml @@ -38,6 +38,7 @@ jobs: --package acropolis_module_drep_state \ --package acropolis_module_epochs_state \ --package acropolis_module_genesis_bootstrapper \ + --package acropolis_module_governance_state \ --package acropolis_module_mithril_snapshot_fetcher \ --package acropolis_module_snapshot_bootstrapper \ --package acropolis_module_spdd_state \ diff --git a/modules/governance_state/src/alonzo_babbage_voting.rs b/modules/governance_state/src/alonzo_babbage_voting.rs index 46c9a4f7..c0d1b5c9 100644 --- a/modules/governance_state/src/alonzo_babbage_voting.rs +++ b/modules/governance_state/src/alonzo_babbage_voting.rs @@ -5,12 +5,15 @@ use acropolis_common::{ use anyhow::{bail, Result}; use std::collections::{HashMap, HashSet}; +// (vote epoch, vote slot, proposal) +type VoteData = (u64, u64, Box); + #[derive(Default)] pub struct AlonzoBabbageVoting { /// map "enact epoch" (proposal enacts at this epoch end) to voting - /// "voting": map voter (genesis key) => (vote epoch, vote slot, proposal) + /// "voting": map voter (genesis key) => votedata /// "vote epoch/slot" --- moment, when the vote was cast for the proposal - proposals: HashMap)>>, + proposals: HashMap>, slots_per_epoch: u32, update_quorum: u32, } @@ -31,7 +34,7 @@ impl AlonzoBabbageVoting { pub fn process_update_proposals( &mut self, block_info: &BlockInfo, - updates: &Vec, + updates: &[AlonzoBabbageUpdateProposal], ) -> Result<()> { if updates.is_empty() { return Ok(()); @@ -46,7 +49,7 @@ impl AlonzoBabbageVoting { } for pp in updates.iter() { - let entry = self.proposals.entry(pp.enactment_epoch + 1).or_insert(HashMap::new()); + let entry = self.proposals.entry(pp.enactment_epoch + 1).or_default(); for (k, p) in &pp.proposals { // A new proposal for key k always replaces the old one entry.insert(k.clone(), (block_info.epoch, block_info.slot, p.clone())); @@ -81,7 +84,8 @@ impl AlonzoBabbageVoting { let votes: Vec<_> = proposals .iter() - .filter_map(|(k, v)| (v == parameter_update).then(|| k.clone())) + .filter(|&(_, v)| v == parameter_update) + .map(|(k, _)| k.clone()) .collect(); for v in &votes { @@ -220,7 +224,7 @@ mod tests { fn extract_mainnet_parameter( f: impl Fn(&ProtocolParamUpdate) -> Option, ) -> Result> { - extract_parameter(5, 432_000, &MAINNET_PROPOSALS_JSON, f) + extract_parameter(5, 432_000, MAINNET_PROPOSALS_JSON, f) } const DECENTRALISATION: [(u64, f32); 39] = [ @@ -282,9 +286,9 @@ mod tests { let dcu = extract_mainnet_parameter(|p| p.decentralisation_constant)?; assert_eq!(DECENTRALISATION.len(), dcu.len()); - for i in 0..dcu.len() { - let rat = rational_number_from_f32(DECENTRALISATION[i].1)?; - assert_eq!((DECENTRALISATION[i].0, rat), *dcu.get(i).unwrap()); + for (decent, param) in DECENTRALISATION.iter().zip(dcu) { + let rat = rational_number_from_f32(decent.1)?; + assert_eq!((decent.0, rat), param); } Ok(()) @@ -319,7 +323,7 @@ mod tests { fn extract_sanchonet_parameter( f: impl Fn(&ProtocolParamUpdate) -> Option, ) -> Result> { - extract_parameter(3, 86_400, &SANCHONET_PROPOSALS_JSON, f) + extract_parameter(3, 86_400, SANCHONET_PROPOSALS_JSON, f) } const SANCHONET_PROTOCOL_VERSION: [(u64, (u64, u64)); 3] = diff --git a/modules/governance_state/src/conway_voting.rs b/modules/governance_state/src/conway_voting.rs index 84582409..c10c176e 100644 --- a/modules/governance_state/src/conway_voting.rs +++ b/modules/governance_state/src/conway_voting.rs @@ -128,7 +128,7 @@ impl ConwayVoting { ) -> Result<()> { self.votes_count += voter_votes.voting_procedures.len(); for (action_id, procedure) in voter_votes.voting_procedures.iter() { - let votes = self.votes.entry(action_id.clone()).or_insert_with(|| HashMap::new()); + let votes = self.votes.entry(action_id.clone()).or_default(); match self.action_status.get(action_id) { None => { @@ -149,7 +149,7 @@ impl ConwayVoting { } if let Some((prev_trans, prev_vote)) = - votes.insert(voter.clone(), (transaction.clone(), procedure.clone())) + votes.insert(voter.clone(), (*transaction, procedure.clone())) { // Re-voting is allowed; new vote must be treated as the proper one, // older is to be discarded. @@ -202,8 +202,8 @@ impl ConwayVoting { /// Should be called when voting is over fn end_voting(&mut self, action_id: &GovActionId) -> Result<()> { - self.votes.remove(&action_id); - self.proposals.remove(&action_id); + self.votes.remove(action_id); + self.proposals.remove(action_id); Ok(()) } @@ -223,7 +223,7 @@ impl ConwayVoting { pool: VoteCount::zero(), }; - let Some(all_votes) = self.votes.get(&action_id) else { + let Some(all_votes) = self.votes.get(action_id) else { return Ok(votes); }; @@ -334,10 +334,10 @@ impl ConwayVoting { spo_stake: &HashMap, ) -> Result> { let outcome = - self.is_finally_accepted(new_epoch, voting_state, &action_id, drep_stake, spo_stake)?; - let expired = self.is_expired(new_epoch, &action_id)?; + self.is_finally_accepted(new_epoch, voting_state, action_id, drep_stake, spo_stake)?; + let expired = self.is_expired(new_epoch, action_id)?; if outcome.accepted || expired { - self.end_voting(&action_id)?; + self.end_voting(action_id)?; info!( "New epoch {new_epoch}: voting for {action_id} outcome: {}, expired: {expired}", outcome.accepted @@ -362,7 +362,7 @@ impl ConwayVoting { /// Function dumps information about completed (expired, ratified, enacted) governance /// actions in format, close to that of `gov_action_proposal` from `sqldb`. - pub fn print_outcome_to_verify(&self, outcome: &Vec) -> Result<()> { + pub fn print_outcome_to_verify(&self, outcome: &[GovernanceOutcome]) -> Result<()> { let out_file_name = match &self.verification_output_file { Some(o) => o, None => return Ok(()), @@ -416,7 +416,7 @@ impl ConwayVoting { {ratification_info},{cast},{threshold}\n", elem.voting.procedure.gov_action_id ); - if let Err(e) = out_file.write(&res.as_bytes()) { + if let Err(e) = out_file.write(res.as_bytes()) { error!( "Cannot write 'res' to verification output {out_file_name} for writing: {e}" ); @@ -434,7 +434,7 @@ impl ConwayVoting { spo_stake: &HashMap, ) -> Result> { let mut outcome = Vec::::new(); - let actions = self.proposals.keys().map(|a| a.clone()).collect::>(); + let actions = self.proposals.keys().cloned().collect::>(); for action_id in actions.iter() { info!( @@ -443,8 +443,8 @@ impl ConwayVoting { ); let one_outcome = match self.process_one_proposal( new_block.epoch, - &voting_state, - &action_id, + voting_state, + action_id, drep_stake, spo_stake, ) { @@ -508,7 +508,7 @@ impl ConwayVoting { pub fn update_action_status_with_outcomes( &mut self, epoch: u64, - outcomes: &Vec, + outcomes: &[GovernanceOutcome], ) -> Result<()> { for one_outcome in outcomes.iter() { let action_id = &one_outcome.voting.procedure.gov_action_id; @@ -601,8 +601,8 @@ mod tests { }, ); - voting.update_action_status_with_outcomes(0, &vec![])?; - voting.update_action_status_with_outcomes(1, &vec![oc1.clone()])?; + voting.update_action_status_with_outcomes(0, &[])?; + voting.update_action_status_with_outcomes(1, std::slice::from_ref(&oc1))?; assert_eq!( voting .action_status diff --git a/modules/governance_state/src/conway_voting_test.rs b/modules/governance_state/src/conway_voting_test.rs index 8f8679e3..1c03fdd2 100644 --- a/modules/governance_state/src/conway_voting_test.rs +++ b/modules/governance_state/src/conway_voting_test.rs @@ -8,8 +8,7 @@ mod tests { SingleVoterVotes, TxHash, Vote, VoteCount, VoteResult, Voter, VotingProcedure, }; use anyhow::{anyhow, bail, Result}; - use serde; - use serde_json; + use serde_with::{base64::Base64, serde_as}; use std::{ collections::{BTreeMap, HashMap}, @@ -83,7 +82,7 @@ mod tests { .strip_prefix("Some(") .ok_or_else(|| anyhow!("Does not have 'Some(' prefix {}", s))?; let num = wp.strip_suffix(")").ok_or_else(|| anyhow!("Must have ')' suffix {}", s))?; - num.parse().map_err(|e| anyhow!("Cannot parse value {num}, error {e}")).map(|x| Some(x)) + num.parse().map_err(|e| anyhow!("Cannot parse value {num}, error {e}")).map(Some) } } @@ -127,12 +126,14 @@ mod tests { }) .collect::)>>>()?; - let res = HashMap::from_iter(converted.into_iter()); + let res = HashMap::from_iter(converted); Ok(res) } - fn map_voter_list(votes: &Vec, v: Vote) -> Result> { + type VoterList = Vec<(Voter, VotingProcedure)>; + + fn map_voter_list(votes: &[VotingRecord], v: Vote) -> Result { votes .iter() .map(|x| { @@ -145,14 +146,12 @@ mod tests { }, )) }) - .collect::>>() + .collect() } /// Reads list of votes: for each epoch, for each gov-action, three lists of voters: /// ([yes voters], [no voters], [abstain voters]) - fn read_voting_state( - voting_json: &[u8], - ) -> Result>> { + fn read_voting_state(voting_json: &[u8]) -> Result> { let voting = serde_json::from_slice::>)>>(voting_json)?; @@ -161,8 +160,8 @@ mod tests { let action_id = GovActionId::from_bech32(action_id)?; let mut vote_procs = Vec::new(); - if votes.len() > 0 { - vote_procs = map_voter_list(votes.get(0).unwrap(), Vote::Yes)?; + if !votes.is_empty() { + vote_procs = map_voter_list(votes.first().unwrap(), Vote::Yes)?; vote_procs.append(&mut map_voter_list(votes.get(1).unwrap(), Vote::No)?); vote_procs.append(&mut map_voter_list(votes.get(2).unwrap(), Vote::Abstain)?); } @@ -189,13 +188,13 @@ mod tests { // s504645304083669/961815354510517/93516758517300,c0:d1:s1 let records = line?.iter().map(|x| x.to_owned()).collect::>(); - if records.len() == 0 { + if records.is_empty() { continue; } else if records.len() != 17 { bail!("Wrong number of elements in csv line: {:?}", records) } - let action_id = records.get(0).unwrap(); + let action_id = records.first().unwrap(); let start_epoch = records.get(6).unwrap(); let proposal = records.get(9).unwrap(); let ratification_epoch = records.get(11).unwrap(); @@ -220,7 +219,7 @@ mod tests { fn read_protocol_params(configs_json: &[u8]) -> Result> { let configs = serde_json::from_slice::>(configs_json)?; - Ok(BTreeMap::from_iter(configs.into_iter())) + Ok(BTreeMap::from_iter(configs)) } #[test] @@ -309,8 +308,8 @@ mod tests { epoch, voting_state, &record.action_id, - ¤t_drep, - ¤t_pool, + current_drep, + current_pool, )?; let Some(outcome) = outcome else { @@ -319,7 +318,7 @@ mod tests { }; assert_eq!(outcome.accepted, record.ratification_epoch.is_some()); - assert_eq!(outcome.accepted, !record.expiration_epoch.is_some()); + assert_eq!(outcome.accepted, record.expiration_epoch.is_none()); if outcome.accepted { assert_eq!(Some(epoch + 2), record.ratification_epoch) } else { diff --git a/modules/governance_state/src/governance_state.rs b/modules/governance_state/src/governance_state.rs index 7191d3e7..f9034e11 100644 --- a/modules/governance_state/src/governance_state.rs +++ b/modules/governance_state/src/governance_state.rs @@ -74,7 +74,7 @@ impl GovernanceStateConfig { governance_query_topic: Self::conf(config, DEFAULT_GOVERNANCE_QUERY_TOPIC), verification_output_file: config .get_string(VERIFICATION_OUTPUT_FILE) - .map(|x| Some(x)) + .map(Some) .unwrap_or(None), }) } @@ -94,7 +94,7 @@ impl GovernanceState { } } - async fn read_parameters<'a>( + async fn read_parameters( parameters_s: &mut Box>, ) -> Result<(BlockInfo, ProtocolParamsMessage)> { match parameters_s.read().await?.1.as_ref() { @@ -205,7 +205,7 @@ impl GovernanceState { } } GovernanceStateQuery::GetProposalVotes { proposal } => { - match locked.get_proposal_votes(&proposal) { + match locked.get_proposal_votes(proposal) { Ok(votes) => { GovernanceStateQueryResponse::ProposalVotes(ProposalVotes { votes }) } diff --git a/modules/governance_state/src/state.rs b/modules/governance_state/src/state.rs index 3f72c1f2..1a1e6b0a 100644 --- a/modules/governance_state/src/state.rs +++ b/modules/governance_state/src/state.rs @@ -65,7 +65,7 @@ impl State { if !epoch_blk.new_epoch { bail!("Block {epoch_blk:?} must start a new epoch"); } - self.current_era = epoch_blk.era.clone(); // If era is the same -- no problem + self.current_era = epoch_blk.era; // If era is the same -- no problem self.alonzo_babbage_voting.advance_epoch(epoch_blk); Ok(()) } @@ -155,16 +155,14 @@ impl State { } fn recalculate_voting_state(&self) -> Result { - let drep_stake = self.drep_stake.iter().map(|(_dr, lov)| lov).sum(); + let drep_stake = self.drep_stake.values().sum(); let committee_usize = self.conway_voting.get_conway_params()?.committee.members.len(); - let committee = committee_usize.try_into().or_else(|e| { - Err(anyhow!( - "Commitee size: conversion usize -> u64 failed, {e}" - )) - })?; + let committee = committee_usize + .try_into() + .map_err(|e| anyhow!("Commitee size: conversion usize -> u64 failed, {e}"))?; - let spo_stake = self.spo_stake.iter().map(|(_sp, ds)| ds.live).sum(); + let spo_stake = self.spo_stake.values().map(|ds| ds.live).sum(); Ok(VotingRegistrationState::new( spo_stake, @@ -182,15 +180,17 @@ impl State { &mut self, new_block: &BlockInfo, ) -> Result { - let mut output = GovernanceOutcomesMessage::default(); - output.alonzo_babbage_outcomes = self.alonzo_babbage_voting.finalize_voting(new_block)?; + let mut output = GovernanceOutcomesMessage { + alonzo_babbage_outcomes: self.alonzo_babbage_voting.finalize_voting(new_block)?, + ..Default::default() + }; if self.current_era >= Era::Conway { // Last chance to print actual votes; later they'll be cleaned self.conway_voting.log_conway_voting_stats(new_block.epoch); let voting_state = self.recalculate_voting_state()?; let ratified = self.conway_voting.finalize_conway_voting( - &new_block, + new_block, &voting_state, &self.drep_stake, &self.spo_stake, diff --git a/modules/governance_state/src/voting_state.rs b/modules/governance_state/src/voting_state.rs index 2bfd768d..0d40a72e 100644 --- a/modules/governance_state/src/voting_state.rs +++ b/modules/governance_state/src/voting_state.rs @@ -163,46 +163,28 @@ impl VotingRegistrationState { d_th = max(d_th, &d.pp_governance_group); } - VoteResult::new(c.threshold.clone(), d_th.clone(), p_th.clone()) + VoteResult::new(c.threshold, *d_th, *p_th) + } + GovernanceAction::HardForkInitiation(_) => { + VoteResult::new(c.threshold, d.hard_fork_initiation, p.hard_fork_initiation) + } + GovernanceAction::TreasuryWithdrawals(_) => { + VoteResult::new(c.threshold, d.treasury_withdrawal, *zero) + } + GovernanceAction::NoConfidence(_) => { + VoteResult::new(*zero, d.motion_no_confidence, p.motion_no_confidence) } - GovernanceAction::HardForkInitiation(_) => VoteResult::new( - c.threshold.clone(), - d.hard_fork_initiation.clone(), - p.hard_fork_initiation.clone(), - ), - GovernanceAction::TreasuryWithdrawals(_) => VoteResult::new( - c.threshold.clone(), - d.treasury_withdrawal.clone(), - zero.clone(), - ), - GovernanceAction::NoConfidence(_) => VoteResult::new( - zero.clone(), - d.motion_no_confidence.clone(), - p.motion_no_confidence.clone(), - ), GovernanceAction::UpdateCommittee(_) => { if thresholds.committee.is_empty() { - VoteResult::new( - zero.clone(), - d.committee_no_confidence.clone(), - p.committee_no_confidence.clone(), - ) + VoteResult::new(*zero, d.committee_no_confidence, p.committee_no_confidence) } else { - VoteResult::new( - zero.clone(), - d.committee_normal.clone(), - p.committee_normal.clone(), - ) + VoteResult::new(*zero, d.committee_normal, p.committee_normal) } } - GovernanceAction::NewConstitution(_) => VoteResult::new( - c.threshold.clone(), - d.update_constitution.clone(), - zero.clone(), - ), - GovernanceAction::Information => { - VoteResult::new(zero.clone(), one.clone(), one.clone()) + GovernanceAction::NewConstitution(_) => { + VoteResult::new(c.threshold, d.update_constitution, *zero) } + GovernanceAction::Information => VoteResult::new(*zero, *one, *one), } } @@ -212,7 +194,7 @@ impl VotingRegistrationState { } if denom == 0 { - Ok(RationalNumber::ZERO.clone()) + Ok(RationalNumber::ZERO) } else { Ok(RationalNumber::new(nom, denom)) } @@ -369,10 +351,7 @@ mod tests { ); println!("Thresholds: {:?}", th); - assert_eq!( - voting_state.compare_votes(&hard_fork, true, &vr, &th)?, - true - ); + assert!(voting_state.compare_votes(&hard_fork, true, &vr, &th)?); Ok(()) } @@ -436,10 +415,7 @@ mod tests { ); println!("Thresholds: {:?}", th); - assert_eq!( - voting_state.compare_votes(&constitution, false, &vr, &th)?, - true - ); + assert!(voting_state.compare_votes(&constitution, false, &vr, &th)?); Ok(()) }