From 604e9dd0487500d4dafbcae3fa71ffd5a786a0a3 Mon Sep 17 00:00:00 2001 From: Giacomo Pasini Date: Mon, 17 Jan 2022 17:42:04 +0100 Subject: [PATCH] address comments --- src/bin/cli/rewards/veterans.rs | 35 +++++++++++++-- src/community_advisors/models/de.rs | 65 ++++++++++++++++++++-------- src/community_advisors/models/mod.rs | 4 +- src/rewards/veterans.rs | 34 +++++---------- 4 files changed, 92 insertions(+), 46 deletions(-) diff --git a/src/bin/cli/rewards/veterans.rs b/src/bin/cli/rewards/veterans.rs index 132f9671..eae11df1 100644 --- a/src/bin/cli/rewards/veterans.rs +++ b/src/bin/cli/rewards/veterans.rs @@ -1,7 +1,9 @@ use super::Error; -use catalyst_toolbox::rewards::veterans; +use catalyst_toolbox::community_advisors::models::VeteranRankingRow; +use catalyst_toolbox::rewards::veterans::{self, VcaRewards, VeteranAdvisorIncentive}; use catalyst_toolbox::rewards::Rewards; use catalyst_toolbox::utils::csv; +use serde::Serialize; use std::path::PathBuf; use structopt::StructOpt; @@ -42,15 +44,42 @@ impl VeteransRewards { max_rankings_reputation, max_rankings_rewards, } = self; - let reviews: Vec = csv::load_data_from_csv::<_, b','>(&from)?; + let reviews: Vec = csv::load_data_from_csv::<_, b','>(&from)?; let results = veterans::calculate_veteran_advisors_incentives( &reviews, total_rewards, min_rankings..=max_rankings_rewards, min_rankings..=max_rankings_reputation, ); - csv::dump_data_to_csv(&results.into_iter().collect::>(), &to)?; + + csv::dump_data_to_csv(&rewards_to_csv_data(results), &to).unwrap(); Ok(()) } } + +fn rewards_to_csv_data(rewards: VcaRewards) -> Vec { + #[derive(Serialize)] + struct Entry { + id: String, + rewards: Rewards, + reputation: u64, + } + + rewards + .into_iter() + .map( + |( + id, + VeteranAdvisorIncentive { + rewards, + reputation, + }, + )| Entry { + id, + rewards, + reputation, + }, + ) + .collect() +} diff --git a/src/community_advisors/models/de.rs b/src/community_advisors/models/de.rs index b6b91067..e86f620c 100644 --- a/src/community_advisors/models/de.rs +++ b/src/community_advisors/models/de.rs @@ -1,7 +1,9 @@ use crate::utils::serde::deserialize_truthy_falsy; use serde::Deserialize; +/// (Proposal Id, Assessor Id), an assessor cannot assess the same proposal more than once pub type AdvisorReviewId = (String, String); +pub type VeteranAdvisorId = String; #[derive(Deserialize)] pub struct AdvisorReviewRow { @@ -34,6 +36,24 @@ pub struct AdvisorReviewRow { filtered_out: bool, } +#[derive(Deserialize)] +pub struct VeteranRankingRow { + pub proposal_id: String, + #[serde(alias = "Assessor")] + pub assessor: String, + #[serde(alias = "Excellent", deserialize_with = "deserialize_truthy_falsy")] + excellent: bool, + #[serde(alias = "Good", deserialize_with = "deserialize_truthy_falsy")] + good: bool, + #[serde( + default, + alias = "Filtered Out", + deserialize_with = "deserialize_truthy_falsy" + )] + filtered_out: bool, + pub vca: VeteranAdvisorId, +} + #[derive(Hash, Clone, PartialEq, Eq, Debug)] pub enum ReviewRanking { Excellent, @@ -48,31 +68,40 @@ impl ReviewRanking { } } -impl AdvisorReviewRow { +impl VeteranRankingRow { pub fn score(&self) -> ReviewRanking { - match (self.excellent, self.good, self.filtered_out) { - (true, false, false) => ReviewRanking::Excellent, - (false, true, false) => ReviewRanking::Good, - (false, false, true) => ReviewRanking::FilteredOut, - (false, false, false) => ReviewRanking::NA, - _ => { - // This should never happen, from the source of information a review could be either - // Excellent or Good or not assessed. It cannot be both and it is considered - // a malformed information input. - panic!( - "Invalid combination of scores from assessor {} for proposal {}", - self.assessor, self.proposal_id - ) - } - } + ranking_from_bools(self.excellent, self.good, self.filtered_out) } - /// Returns a unique identifier of this review - pub fn id(&self) -> AdvisorReviewId { + pub fn review_id(&self) -> AdvisorReviewId { (self.proposal_id.clone(), self.assessor.clone()) } } +impl AdvisorReviewRow { + pub fn score(&self) -> ReviewRanking { + ranking_from_bools(self.excellent, self.good, self.filtered_out) + } +} + +fn ranking_from_bools(excellent: bool, good: bool, filtered_out: bool) -> ReviewRanking { + match (excellent, good, filtered_out) { + (true, false, false) => ReviewRanking::Excellent, + (false, true, false) => ReviewRanking::Good, + (false, false, true) => ReviewRanking::FilteredOut, + (false, false, false) => ReviewRanking::NA, + _ => { + // This should never happen, from the source of information a review could be either + // Excellent or Good or not assessed. It cannot be both and it is considered + // a malformed information input. + panic!( + "Invalid combination of scores {} {} {}", + excellent, good, filtered_out + ) + } + } +} + #[cfg(test)] mod tests { use super::ReviewRanking; diff --git a/src/community_advisors/models/mod.rs b/src/community_advisors/models/mod.rs index c0fa0dd2..239a47c8 100644 --- a/src/community_advisors/models/mod.rs +++ b/src/community_advisors/models/mod.rs @@ -2,7 +2,9 @@ mod de; use serde::{Deserialize, Deserializer}; -pub use de::{AdvisorReviewId, AdvisorReviewRow, ReviewRanking}; +pub use de::{ + AdvisorReviewId, AdvisorReviewRow, ReviewRanking, VeteranAdvisorId, VeteranRankingRow, +}; pub enum ProposalStatus { Approved, diff --git a/src/rewards/veterans.rs b/src/rewards/veterans.rs index 76619117..c6db505e 100644 --- a/src/rewards/veterans.rs +++ b/src/rewards/veterans.rs @@ -1,6 +1,6 @@ use crate::community_advisors::models::{ - AdvisorReviewId, AdvisorReviewRow, ReviewRanking::{self, *}, + VeteranAdvisorId, VeteranRankingRow, }; use crate::rewards::Rewards; use itertools::Itertools; @@ -8,9 +8,7 @@ use rust_decimal::{prelude::ToPrimitive, Decimal}; use std::borrow::Borrow; use std::collections::{BTreeMap, HashMap}; -use serde::{Deserialize, Serialize}; - -pub type VeteranAdvisorId = String; +use serde::Serialize; #[derive(Serialize)] pub struct VeteranAdvisorIncentive { @@ -18,6 +16,7 @@ pub struct VeteranAdvisorIncentive { pub reputation: u64, } +pub type VcaRewards = HashMap; pub type EligibilityThresholds = std::ops::RangeInclusive; // TODO: for the sake of clarity, introduce a different naming between ca reviews and vca ranking @@ -25,30 +24,18 @@ pub type EligibilityThresholds = std::ops::RangeInclusive; // Supposing to have a file with all the rankings for each review // e.g. something like an expanded version of a AdvisorReviewRow // [proposal_id, advisor, ratings, ..(other fields from AdvisorReviewRow).., ranking (good/excellent/filtered out), vca] -#[derive(Deserialize)] -pub struct VeteranRankingRow { - #[serde(flatten)] - advisor_review_row: AdvisorReviewRow, - vca: VeteranAdvisorId, -} - -impl VeteranRankingRow { - fn score(&self) -> ReviewRanking { - self.advisor_review_row.score() - } - - fn review_id(&self) -> AdvisorReviewId { - self.advisor_review_row.id() - } -} fn calc_final_ranking_per_review(rankings: &[impl Borrow]) -> ReviewRanking { - let rankings_majority = rankings.len() / 2; + let rankings_majority = Decimal::from(rankings.len()) / Decimal::from(2); let ranks = rankings.iter().counts_by(|r| r.borrow().score()); match (ranks.get(&FilteredOut), ranks.get(&Excellent)) { - (Some(filtered_out), _) if filtered_out > &rankings_majority => ReviewRanking::FilteredOut, - (_, Some(excellent)) if excellent > &rankings_majority => ReviewRanking::Excellent, + (Some(filtered_out), _) if Decimal::from(*filtered_out) >= rankings_majority => { + ReviewRanking::FilteredOut + } + (_, Some(excellent)) if Decimal::from(*excellent) > rankings_majority => { + ReviewRanking::Excellent + } _ => ReviewRanking::Good, } } @@ -69,7 +56,6 @@ fn rewards_disagreement_discount(agreement_rate: Decimal) -> Decimal { } fn reputation_disagreement_discount(agreement_rate: Decimal) -> Decimal { - println!("discount rate {}", agreement_rate); if agreement_rate >= Decimal::new(6, 1) { Decimal::ONE } else {