Do not allow short lockups to vote on proposals for longer liquidity …

…deployment (#175)

* Do not allow short lockups to vote on proposals for longer liquidity deployment.

* ignore short lockups instead of returning error in vote()

.changelog/unreleased/features/175-short-locks-cant-vote-for-long-proposals.md

@@ -0,0 +1,2 @@
+- Do not allow short lockups to vote on proposals requesting longer liquidity deployment.
+  ([\#175](https://github.com/informalsystems/hydro/pull/175))

artifacts/checksums.txt

@@ -1,2 +1,2 @@
-6f1f80739d9deaba86aba39961841a73b6fb1ae2b52046eff6f9b638777d9b21  hydro.wasm
-eeb3108ad732ca312cf4c5faad508f0c41977a2174bd93c8d06d783012287459  tribute.wasm
+9c08862f95440cf2b132cc6a8f70a5d1d6791dc001a9faf31a9b1322c09fab62  hydro.wasm
+956f646ccbcb493c82b164ab8e4dbcdd80fea97c66483a04fda5971a8b6f4199  tribute.wasm

contracts/hydro/src/contract.rs

@@ -857,6 +857,8 @@ fn vote(
 
     let lock_epoch_length = constants.lock_epoch_length;
     let mut voted_proposals = vec![];
+    let mut locks_voted = vec![];
+    let mut locks_skipped = vec![];
 
     for proposal_to_lockups in proposals_votes {
         let proposal_id = proposal_to_lockups.proposal_id;
@@ -883,17 +885,27 @@ fn vote(
                         ));
 
                     // skip this lock entry, since the locked shares do not belong to a validator that we want to take into account
+                    locks_skipped.push(lock_entry.lock_id);
                     continue;
                 }
             };
 
             let scaled_shares = Decimal::from_ratio(
-                get_lock_time_weighted_shares(round_end, lock_entry, lock_epoch_length),
+                get_lock_time_weighted_shares(round_end, lock_entry.clone(), lock_epoch_length),
                 Uint128::one(),
             );
 
             // skip the lock entries that give zero voting power
             if scaled_shares.is_zero() {
+                locks_skipped.push(lock_entry.lock_id);
+                continue;
+            }
+
+            let proposal = PROPOSAL_MAP.load(deps.storage, (round_id, tranche_id, proposal_id))?;
+
+            // skip lock entries that don't span long enough to be allowed to vote for this proposal
+            if !can_lock_vote_for_proposal(round_id, &constants, &lock_entry, &proposal)? {
+                locks_skipped.push(lock_entry.lock_id);
                 continue;
             }
 
@@ -907,12 +919,7 @@ fn vote(
             )?;
 
             // update the proposal in the proposal map, as well as the props by score map
-            let updated_proposal = update_proposal_and_props_by_score_maps(
-                deps.storage,
-                round_id,
-                tranche_id,
-                proposal_id,
-            )?;
+            update_proposal_and_props_by_score_maps(deps.storage, round_id, tranche_id, &proposal)?;
 
             // Create vote in Votemap
             let vote = Vote {
@@ -925,27 +932,31 @@ fn vote(
                 &vote,
             )?;
 
-            let voting_allowed_round = round_id + updated_proposal.bid_duration;
+            let voting_allowed_round = round_id + proposal.bid_duration;
             VOTING_ALLOWED_ROUND.save(
                 deps.storage,
                 (tranche_id, lock_id),
                 &voting_allowed_round,
             )?;
+
+            locks_voted.push(lock_entry.lock_id);
         }
 
         voted_proposals.push(proposal_id);
     }
 
-    if !voted_proposals.is_empty() {
-        let voted_props_attr = voted_proposals
+    let to_string = |input: &Vec<u64>| {
+        input
             .iter()
-            .map(|proposal_id| proposal_id.to_string())
+            .map(|id| id.to_string())
             .collect::<Vec<String>>()
-            .join(",");
-        Ok(response.add_attribute("proposal_id", voted_props_attr))
-    } else {
-        Ok(response)
-    }
+            .join(",")
+    };
+
+    Ok(response
+        .add_attribute("proposal_id", to_string(&voted_proposals))
+        .add_attribute("locks_voted", to_string(&locks_voted))
+        .add_attribute("locks_skipped", to_string(&locks_skipped)))
 }
 
 // Returns the time-weighted amount of shares locked in the given lock entry in a round with the given end time,
@@ -2088,6 +2099,19 @@ fn update_voting_power_on_proposals(
                 )));
             };
 
+            let proposal =
+                PROPOSAL_MAP.load(deps.storage, (current_round, tranche_id, vote.prop_id))?;
+
+            // Ensure that lock entry spans long enough to be allowed to vote for this proposal.
+            // If not, we will not have this lock vote for the proposal, even if user voted for
+            // only one proposal in the given round and tranche. Note that this condition will
+            // always be satisfied when refreshing the lock entry, since we already checked this
+            // condition when user voted with this lock entry, and refreshing the lock only allows
+            // lock duration to be extended.
+            if !can_lock_vote_for_proposal(current_round, constants, &new_lock_entry, &proposal)? {
+                continue;
+            }
+
             let new_vote_shares = if power_change.is_increased {
                 current_vote_shares.checked_add(power_change.scaled_power_change)?
             } else {
@@ -2128,7 +2152,7 @@ fn update_voting_power_on_proposals(
                 deps.storage,
                 current_round,
                 tranche_id,
-                vote.prop_id,
+                &proposal,
             )?;
         }
     }
@@ -2200,14 +2224,30 @@ pub fn get_vote_for_update(
     })
 }
 
+// Ensure that the lock will have a power greater than 0 at the end of
+// the round preceding the round in which the liquidity will be returned.
+fn can_lock_vote_for_proposal(
+    current_round: u64,
+    constants: &Constants,
+    lock_entry: &LockEntry,
+    proposal: &Proposal,
+) -> Result<bool, ContractError> {
+    let power_required_round_id = current_round + proposal.bid_duration - 1;
+    let power_required_round_end = compute_round_end(constants, power_required_round_id)?;
+
+    Ok(lock_entry.lock_end >= power_required_round_end)
+}
+
+/// This function relies on PROPOSAL_TOTAL_MAP and SCALED_PROPOSAL_SHARES_MAP being
+/// already updated with the new proposal power.
 fn update_proposal_and_props_by_score_maps(
     storage: &mut dyn Storage,
     round_id: u64,
     tranche_id: u64,
-    proposal_id: u64,
-) -> Result<Proposal, ContractError> {
-    // Load the proposal that needs to be updated
-    let mut proposal = PROPOSAL_MAP.load(storage, (round_id, tranche_id, proposal_id))?;
+    proposal: &Proposal,
+) -> Result<(), ContractError> {
+    let mut proposal = proposal.clone();
+    let proposal_id = proposal.proposal_id;
 
     // Delete the proposal's old power in PROPS_BY_SCORE
     PROPS_BY_SCORE.remove(
@@ -2219,7 +2259,7 @@ fn update_proposal_and_props_by_score_maps(
     let total_power = get_total_power_for_proposal(storage, proposal_id)?;
 
     // Save the new power into the proposal
-    proposal.power = total_power.to_uint_ceil(); // TODO: decide whether we need to round or represent as decimals
+    proposal.power = total_power.to_uint_ceil();
 
     // Save the proposal
     PROPOSAL_MAP.save(storage, (round_id, tranche_id, proposal_id), &proposal)?;
@@ -2231,7 +2271,7 @@ fn update_proposal_and_props_by_score_maps(
         &proposal_id,
     )?;
 
-    Ok(proposal)
+    Ok(())
 }
 
 #[allow(clippy::too_many_arguments)] // complex function that needs a lot of arguments

contracts/hydro/src/testing.rs

@@ -363,8 +363,10 @@ fn vote_basic_test() {
     vote_test_with_start_time(mock_env().block.time, 0);
 }
 
-// If user already voted for some proposal, and then locks new tokens or refreshes the existing lock,
-// the voting power on those proposals should get updated accordingly.
+// If user already voted for only one proposal in the given round and tranche, and then locks new tokens or
+// refreshes the existing lock, the voting power on that proposal should get updated accordingly. However,
+// if user voted for proposal that requires liquidity deployment for multiple rounds, but the newly created
+// lock entry doesn't span long enough, then the voting power on such proposal should not be updated.
 #[test]
 fn proposal_power_change_on_lock_and_refresh_test() {
     let user_address = "addr0000";
@@ -449,6 +451,7 @@ fn proposal_power_change_on_lock_and_refresh_test() {
     let second_proposal_id = 1;
     let third_proposal_id = 2;
     let fourth_proposal_id = 3;
+    let fifth_proposal_id = 4;
 
     let first_lockup_id = 0;
     let second_lockup_id = 1;
@@ -739,6 +742,70 @@ fn proposal_power_change_on_lock_and_refresh_test() {
         fourth_proposal_id,
         expected_voting_power,
     );
+
+    // create a new (fifth) proposal that requires liquidity for 3 rounds
+    let msg = ExecuteMsg::CreateProposal {
+        tranche_id: first_tranche_id,
+        title: "proposal title 5".to_string(),
+        description: "proposal description 5".to_string(),
+        bid_duration: 3,
+        minimum_atom_liquidity_request: Uint128::zero(),
+    };
+
+    let res = execute(deps.as_mut(), env.clone(), info.clone(), msg.clone());
+    assert!(res.is_ok());
+
+    // switch vote to the fifth proposal in tranche 1
+    let msg = ExecuteMsg::Vote {
+        tranche_id: first_tranche_id,
+        proposals_votes: vec![ProposalToLockups {
+            proposal_id: fifth_proposal_id,
+            lock_ids: vec![
+                // only the first lockup has some power in second round
+                first_lockup_id,
+            ],
+        }],
+    };
+    let res = execute(deps.as_mut(), env.clone(), info.clone(), msg.clone());
+    assert!(res.is_ok());
+
+    // verify users vote for the fifth proposal in tranche 1
+    expected_voting_power = 1500u128;
+
+    let res = query_user_votes(
+        deps.as_ref(),
+        second_round_id,
+        first_tranche_id,
+        info.sender.to_string(),
+    );
+    assert!(res.is_ok(), "error: {:?}", res);
+    assert_eq!(fifth_proposal_id, res.unwrap().votes[0].prop_id);
+
+    assert_proposal_voting_power(
+        &deps,
+        second_round_id,
+        first_tranche_id,
+        fifth_proposal_id,
+        expected_voting_power,
+    );
+
+    // lock more tokens for one round and verify that the fifth proposal power
+    // didn't change since the lock doesn't span long enough to be allowed to
+    // vote for this proposal.
+    let info = get_message_info(&deps.api, user_address, &[user_token1.clone()]);
+    let msg = ExecuteMsg::LockTokens {
+        lock_duration: TWO_WEEKS_IN_NANO_SECONDS,
+    };
+    let res = execute(deps.as_mut(), env.clone(), info.clone(), msg.clone());
+    assert!(res.is_ok());
+
+    assert_proposal_voting_power(
+        &deps,
+        second_round_id,
+        first_tranche_id,
+        fifth_proposal_id,
+        expected_voting_power,
+    );
 }
 
 #[test]
@@ -930,6 +997,7 @@ fn vote_extended_proposals_test() {
     let info = get_message_info(&deps.api, user_address, &[user_token.clone()]);
     let mut init_params = get_default_instantiate_msg(&deps.api);
     init_params.first_round_start = env.block.time;
+    init_params.round_length = ONE_MONTH_IN_NANO_SECONDS;
 
     let res = instantiate(
         deps.as_mut(),
@@ -941,8 +1009,22 @@ fn vote_extended_proposals_test() {
 
     set_default_validator_for_rounds(deps.as_mut(), 0, 5);
 
+    // advance the env time to simulate ongoing round
+    env.block.time = env.block.time.plus_hours(1);
+
+    // create a lock that will have power long enough to vote for the 'long lasting' proposal
+    let msg = ExecuteMsg::LockTokens {
+        lock_duration: 6 * ONE_MONTH_IN_NANO_SECONDS,
+    };
+
+    let res = execute(deps.as_mut(), env.clone(), info.clone(), msg);
+    assert!(res.is_ok());
+
+    // create one more lock that will not be allowed to vote for the 'long lasting' proposal
+    // since it will have 0 power at the end of the round that precedes the round in which
+    // the liquidity should be returned
     let msg = ExecuteMsg::LockTokens {
-        lock_duration: 3 * ONE_MONTH_IN_NANO_SECONDS,
+        lock_duration: 2 * ONE_MONTH_IN_NANO_SECONDS,
     };
 
     let res = execute(deps.as_mut(), env.clone(), info.clone(), msg);
@@ -952,6 +1034,7 @@ fn vote_extended_proposals_test() {
     let tranche_id = 1;
 
     let first_lock_id = 0;
+    let second_lock_id = 1;
 
     let second_proposal_id = 1;
     let third_proposal_id = 2;
@@ -1022,7 +1105,44 @@ fn vote_extended_proposals_test() {
     // check that users voted for the second proposal
     let res = query_user_votes(deps.as_ref(), round_id, tranche_id, info.sender.to_string());
     assert!(res.is_ok(), "error: {:?}", res);
-    assert_eq!(second_proposal_id, res.unwrap().votes[0].prop_id);
+    let user_vote = res.unwrap().votes[0].clone();
+    assert_eq!(second_proposal_id, user_vote.prop_id);
+
+    // save vote power for future verification
+    let old_vote_power = user_vote.power;
+
+    // vote for second proposal p(2) with lock that doesn't span long enough
+    let msg = ExecuteMsg::Vote {
+        tranche_id,
+        proposals_votes: vec![ProposalToLockups {
+            proposal_id: second_proposal_id,
+            lock_ids: vec![second_lock_id],
+        }],
+    };
+    let res = execute(deps.as_mut(), env.clone(), info.clone(), msg.clone());
+    assert!(res.is_ok());
+
+    let mut second_lock_skipped = false;
+    for attribute in res.unwrap().attributes {
+        if attribute.key.eq("locks_skipped")
+            && attribute.value.contains(&second_lock_id.to_string())
+        {
+            second_lock_skipped = true;
+            break;
+        }
+    }
+    assert!(
+        second_lock_skipped,
+        "lock with ID {} should be skipped, but it wasn't",
+        second_lock_id
+    );
+
+    // verify that user's vote didn't change
+    let res = query_user_votes(deps.as_ref(), round_id, tranche_id, info.sender.to_string());
+    assert!(res.is_ok(), "error: {:?}", res);
+    let user_vote = res.unwrap().votes[0].clone();
+    assert_eq!(second_proposal_id, user_vote.prop_id);
+    assert_eq!(old_vote_power, user_vote.power);
 
     // advance the chain by one round length to move to round 1
     env.block.time = env.block.time.plus_nanos(init_params.round_length);