Skip to content

Commit

Permalink
Weight 0 is still a valid weight in this contract
Browse files Browse the repository at this point in the history
  • Loading branch information
ueco-jb committed Nov 11, 2021
1 parent bb86101 commit 3f89079
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 36 deletions.
23 changes: 12 additions & 11 deletions contracts/cw3-flex-multisig/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -541,11 +541,11 @@ mod tests {
) -> (Addr, Addr) {
// 1. Instantiate group contract with members (and OWNER as admin)
let members = vec![
member(OWNER, 1),
member(OWNER, 0),
member(VOTER1, 1),
member(VOTER2, 2),
member(VOTER3, 3),
member(VOTER4, 13), // so that he alone can pass a 50 / 52% threshold proposal
member(VOTER4, 12), // so that he alone can pass a 50 / 52% threshold proposal
member(VOTER5, 5),
];
let group_addr = instantiate_group(app, members);
Expand Down Expand Up @@ -808,7 +808,7 @@ mod tests {
let voting_period = Duration::Time(2000000);
let threshold = Threshold::ThresholdQuorum {
threshold: Decimal::percent(80),
quorum: Decimal::percent(1),
quorum: Decimal::percent(20),
};
let (flex_addr, _) = setup_test_case(&mut app, threshold, voting_period, init_funds, false);

Expand Down Expand Up @@ -886,9 +886,10 @@ mod tests {
msgs,
expires: voting_period.after(&proposed_at),
status: Status::Open,
threshold: ThresholdResponse::AbsoluteCount {
weight: 3,
total_weight: 24,
threshold: ThresholdResponse::ThresholdQuorum {
total_weight: 23,
threshold: Decimal::percent(80),
quorum: Decimal::percent(20),
},
};
assert_eq!(&expected, &res.proposals[0]);
Expand Down Expand Up @@ -948,7 +949,7 @@ mod tests {
// No/Veto votes have no effect on the tally
// Compute the current tally
let tally = get_tally(&app, flex_addr.as_ref(), proposal_id);
assert_eq!(tally, 2);
assert_eq!(tally, 1);

// Cast a No vote
let no_vote = ExecuteMsg::Vote {
Expand Down Expand Up @@ -1005,7 +1006,7 @@ mod tests {
assert_eq!(ContractError::NotOpen {}, err.downcast().unwrap());

// query individual votes
// initial (with 1 weight)
// initial (with 0 weight)
let voter = OWNER.into();
let vote: VoteResponse = app
.wrap()
Expand All @@ -1016,7 +1017,7 @@ mod tests {
VoteInfo {
voter: OWNER.into(),
vote: Vote::Yes,
weight: 1
weight: 0
}
);

Expand Down Expand Up @@ -1224,7 +1225,7 @@ mod tests {
.query_wasm_smart(&flex_addr, &QueryMsg::Threshold {})
.unwrap();
let expected_thresh = ThresholdResponse::ThresholdQuorum {
total_weight: 25,
total_weight: 23,
threshold: Decimal::percent(51),
quorum: Decimal::percent(1),
};
Expand Down Expand Up @@ -1303,7 +1304,7 @@ mod tests {
.query_wasm_smart(&flex_addr, &QueryMsg::Threshold {})
.unwrap();
let expected_thresh = ThresholdResponse::ThresholdQuorum {
total_weight: 43,
total_weight: 41,
threshold: Decimal::percent(51),
quorum: Decimal::percent(1),
};
Expand Down
6 changes: 6 additions & 0 deletions contracts/cw3-flex-multisig/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ pub enum ContractError {
#[error("Required threshold must be higher then 50% and smaller then 100%")]
InvalidThreshold {},

#[error("Required quorum threshold cannot be zero")]
ZeroQuorumThreshold {},

#[error("Not possible to reach required quorum threshold")]
UnreachableQuorumThreshold {},

#[error("Required weight cannot be zero")]
ZeroWeight {},

Expand Down
56 changes: 39 additions & 17 deletions contracts/cw3-flex-multisig/src/msg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,13 @@ impl Threshold {
}
Threshold::AbsolutePercentage {
percentage: percentage_needed,
} => valid_percentage(percentage_needed),
} => valid_threshold(percentage_needed),
Threshold::ThresholdQuorum {
threshold,
quorum: quroum,
} => {
valid_percentage(threshold)?;
valid_percentage(quroum)
valid_threshold(threshold)?;
valid_quorum(quroum)
}
}
}
Expand Down Expand Up @@ -92,14 +92,25 @@ impl Threshold {
}

/// Asserts that the 0.5 < percent <= 1.0
fn valid_percentage(percent: &Decimal) -> Result<(), ContractError> {
fn valid_threshold(percent: &Decimal) -> Result<(), ContractError> {
if *percent > Decimal::percent(100) || *percent < Decimal::percent(50) {
Err(ContractError::InvalidThreshold {})
} else {
Ok(())
}
}

/// Asserts that the 0.5 < percent <= 1.0
fn valid_quorum(percent: &Decimal) -> Result<(), ContractError> {
if percent.is_zero() {
Err(ContractError::ZeroQuorumThreshold {})
} else if *percent > Decimal::one() {
Err(ContractError::UnreachableQuorumThreshold {})
} else {
Ok(())
}
}

// TODO: add some T variants? Maybe good enough as fixed Empty for now
#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
#[serde(rename_all = "snake_case")]
Expand Down Expand Up @@ -165,36 +176,44 @@ mod tests {
use super::*;

#[test]
fn validate_percentage() {
fn validate_quorum_percentage() {
// TODO: test the error messages

// 0 is never a valid percentage
let err = valid_percentage(&Decimal::zero()).unwrap_err();
let err = valid_quorum(&Decimal::zero()).unwrap_err();
assert_eq!(
err.to_string(),
ContractError::InvalidThreshold {}.to_string()
ContractError::ZeroQuorumThreshold {}.to_string()
);

// 100% is
valid_percentage(&Decimal::one()).unwrap();
valid_quorum(&Decimal::one()).unwrap();

// 101% is not
let err = valid_percentage(&Decimal::percent(101)).unwrap_err();
let err = valid_quorum(&Decimal::percent(101)).unwrap_err();
assert_eq!(
err.to_string(),
ContractError::InvalidThreshold {}.to_string()
ContractError::UnreachableQuorumThreshold {}.to_string()
);
// not 100.1%
let err = valid_percentage(&Decimal::permille(1001)).unwrap_err();
let err = valid_quorum(&Decimal::permille(1001)).unwrap_err();
assert_eq!(
err.to_string(),
ContractError::InvalidThreshold {}.to_string()
ContractError::UnreachableQuorumThreshold {}.to_string()
);
}

#[test]
fn validate_threshold_percentage() {
// other values in between 0.5 and 1 are valid
valid_percentage(&Decimal::percent(51)).unwrap();
valid_percentage(&Decimal::percent(67)).unwrap();
valid_percentage(&Decimal::percent(99)).unwrap();
valid_threshold(&Decimal::percent(51)).unwrap();
valid_threshold(&Decimal::percent(67)).unwrap();
valid_threshold(&Decimal::percent(99)).unwrap();
let err = valid_threshold(&Decimal::percent(101)).unwrap_err();
assert_eq!(
err.to_string(),
ContractError::InvalidThreshold {}.to_string()
);
}

#[test]
Expand Down Expand Up @@ -247,15 +266,18 @@ mod tests {
.unwrap_err();
assert_eq!(
err.to_string(),
ContractError::UnreachableWeight {}.to_string()
ContractError::InvalidThreshold {}.to_string()
);
let err = Threshold::ThresholdQuorum {
threshold: Decimal::percent(51),
quorum: Decimal::percent(0),
}
.validate(5)
.unwrap_err();
assert_eq!(err.to_string(), ContractError::ZeroWeight {}.to_string());
assert_eq!(
err.to_string(),
ContractError::ZeroQuorumThreshold {}.to_string()
);
}

#[test]
Expand Down
10 changes: 2 additions & 8 deletions packages/cw4/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,14 +88,8 @@ impl Cw4Contract {
height: u64,
) -> StdResult<u64> {
self.member_at_height(querier, member, height)?.map_or(
Err(StdError::generic_err("Unauthorized")),
|member_weight| {
if member_weight < 1 {
Err(StdError::generic_err("Unauthorized"))
} else {
Ok(member_weight)
}
},
Err(StdError::generic_err("Not a member")),
|member_weight| Ok(member_weight),
)
}

Expand Down

0 comments on commit 3f89079

Please sign in to comment.