Merge pull request #1985 from blockstack/fix/1984

Fix 1984

Author: jcnelson

sip/sip-007-stacking-consensus.md

@@ -191,10 +191,10 @@ Address validity is determined according to two different rules:
    descendant of the anchor block*, all of the miner's commitment
    funds must be burnt.
 2. If a miner is building off a descendant of the anchor block, the
-   miner must send commitment funds to 5 addresses from the reward
+   miner must send commitment funds to 2 addresses from the reward
    set, chosen as follows:
     * Use the verifiable random function (also used by sortition) to
-      choose 5 addresses from the reward set. These 5 addresses are
+      choose 2 addresses from the reward set. These 2 addresses are
       the reward addresses for this block.
     * Once addresses have been chosen for a block, these addresses are
       removed from the reward set, so that future blocks in the reward
@@ -217,24 +217,25 @@ addresses for a reward cycle, then each miner commitment would have
 
 ## Adjusting Reward Threshold Based on Participation
 
-Each reward cycle may transfer miner funds to up to 5000 Bitcoin
-addresses. To ensure that this number of addresses is sufficient to
-cover the pool of participants (given 100% participation of liquid
-STX), the threshold for participation must be 0.02% (1/5000th) of the
-liquid supply of STX. However, if participation is _lower_ than 100%,
-the reward pool could admit lower STX holders. The Stacking protocol
-specifies **2 operating levels**:
+Each reward cycle may transfer miner funds to up to 4000 Bitcoin
+addresses (2 addresses in a 2000 burn block cycle). To ensure that
+this number of addresses is sufficient to cover the pool of
+participants (given 100% participation of liquid STX), the threshold
+for participation must be 0.025% (1/4000th) of the liquid supply of
+STX. However, if participation is _lower_ than 100%, the reward pool
+could admit lower STX holders. The Stacking protocol specifies **2
+operating levels**:
 
 * **25%** If fewer than `0.25 * STX_LIQUID_SUPPLY` STX participate in
   a reward cycle, participant wallets controlling `x` STX may include
-  `floor(x / (0.00005*STX_LIQUID_SUPPLY))` addresses in the reward set.
-  That is, the minimum participation threshold is 1/20,000th of the liquid
+  `floor(x / (0.0000625*STX_LIQUID_SUPPLY))` addresses in the reward set.
+  That is, the minimum participation threshold is 1/16,000th of the liquid
   supply.
 * **25%-100%** If between `0.25 * STX_LIQUID_SUPPLY` and `1.0 *
   STX_LIQUID_SUPPLY` STX participate in a reward cycle, the reward
   threshold is optimized in order to maximize the number of slots that
   are filled. That is, the minimum threshold `T` for participation will be
-  roughly 1/5,000th of the participating STX (adjusted in increments
+  roughly 1/4,000th of the participating STX (adjusted in increments
   of 10,000 STX). Participant wallets controlling `x` STX may
   include `floor(x / T)` addresses in the
   reward set.
@@ -517,10 +518,6 @@ the second through nth outputs:
        The order of these addresses does not matter. Each of these outputs must receive the
        same amount of BTC.
     c. If the number of remaining addresses in the reward set N is less than M, then the leader
-       block commit transaction must burn BTC:
-          i. If N > 0, then the (N+2)nd output must be a burn output, and it must burn
-             (M-N) * (the amount of BTC transfered to each of the first N outputs)
-         ii. If N == 0, then the 2nd output must be a burn output, and the amount burned
-             by this output will be counted as the amount committed to by the block commit.
-2. Otherwise, the 2nd output must be a burn output, and the amount burned by this output will be
-   counted as the amount committed to by the block commit.
+       block commit transaction must burn BTC by including (M-N) burn outputs.
+2. Otherwise, the second through (M+1)th output must be burn addresses, and the amount burned by
+   these outputs will be counted as the amount committed to by the block commit.

src/burnchains/mod.rs

@@ -51,6 +51,7 @@ use chainstate::stacks::StacksPublicKey;
 
 use chainstate::burn::db::sortdb::PoxId;
 use chainstate::burn::distribution::BurnSamplePoint;
+use chainstate::burn::operations::leader_block_commit::OUTPUTS_PER_COMMIT;
 use chainstate::burn::operations::BlockstackOperationType;
 use chainstate::burn::operations::Error as op_error;
 use chainstate::burn::operations::LeaderKeyRegisterOp;
@@ -311,7 +312,7 @@ impl PoxConstants {
     }
 
     pub fn reward_slots(&self) -> u32 {
-        self.reward_cycle_length
+        self.reward_cycle_length * (OUTPUTS_PER_COMMIT as u32)
     }
 
     /// is participating_ustx enough to engage in PoX in the next reward cycle?

src/chainstate/burn/db/sortdb.rs

@@ -973,7 +973,7 @@ impl<'a> SortitionHandleTx<'a> {
     ///   * The reward cycle had an anchor block, but it isn't known by this node.
     ///   * The reward cycle did not have anchor block
     ///   * The Stacking recipient set is empty (either because this reward cycle has already exhausted the set of addresses or because no one ever Stacked).
-    fn pick_recipient(
+    fn pick_recipients(
         &mut self,
         reward_set_vrf_seed: &SortitionHash,
         next_pox_info: Option<&RewardCycleInfo>,
@@ -986,20 +986,26 @@ impl<'a> SortitionHandleTx<'a> {
                     return Ok(None);
                 }
 
-                let chosen_recipient = reward_set_vrf_seed.choose(
+                if OUTPUTS_PER_COMMIT != 2 {
+                    unreachable!("BUG: PoX reward address selection only implemented for OUTPUTS_PER_COMMIT = 2");
+                }
+
+                let chosen_recipients = reward_set_vrf_seed.choose_two(
                     reward_set
                         .len()
                         .try_into()
                         .expect("BUG: u32 overflow in PoX outputs per commit"),
                 );
 
-                let recipient = (
-                    reward_set[chosen_recipient as usize],
-                    u16::try_from(chosen_recipient).unwrap(),
-                );
                 Ok(Some(RewardSetInfo {
                     anchor_block: anchor_block.clone(),
-                    recipient,
+                    recipients: chosen_recipients
+                        .into_iter()
+                        .map(|ix| {
+                            let recipient = reward_set[ix as usize].clone();
+                            (recipient, u16::try_from(ix).unwrap())
+                        })
+                        .collect(),
                 }))
             } else {
                 Ok(None)
@@ -1013,12 +1019,16 @@ impl<'a> SortitionHandleTx<'a> {
                 if reward_set_size == 0 {
                     Ok(None)
                 } else {
-                    let chosen_recipient = reward_set_vrf_seed.choose(reward_set_size as u32);
-                    let ix = u16::try_from(chosen_recipient).unwrap();
-                    let recipient = (self.get_reward_set_entry(ix)?, ix);
+                    let chosen_recipients = reward_set_vrf_seed.choose_two(reward_set_size as u32);
+                    let mut recipients = vec![];
+                    for ix in chosen_recipients.into_iter() {
+                        let ix = u16::try_from(ix).unwrap();
+                        let recipient = self.get_reward_set_entry(ix)?;
+                        recipients.push((recipient, ix));
+                    }
                     Ok(Some(RewardSetInfo {
                         anchor_block,
-                        recipient,
+                        recipients,
                     }))
                 }
             } else {
@@ -2336,7 +2346,7 @@ impl SortitionDB {
             .mix_burn_header(&parent_snapshot.burn_header_hash);
 
         let reward_set_info =
-            sortition_db_handle.pick_recipient(&reward_set_vrf_hash, next_pox_info.as_ref())?;
+            sortition_db_handle.pick_recipients(&reward_set_vrf_hash, next_pox_info.as_ref())?;
 
         let new_snapshot = sortition_db_handle.process_block_txs(
             &parent_snapshot,
@@ -2375,7 +2385,7 @@ impl SortitionDB {
 
         let mut sortition_db_handle =
             SortitionHandleTx::begin(self, &parent_snapshot.sortition_id)?;
-        sortition_db_handle.pick_recipient(&reward_set_vrf_hash, next_pox_info)
+        sortition_db_handle.pick_recipients(&reward_set_vrf_hash, next_pox_info)
     }
 
     pub fn is_stacks_block_in_sortition_set(
@@ -3229,9 +3239,18 @@ impl<'a> SortitionHandleTx<'a> {
                     if reward_set.len() > 0 {
                         // if we have a reward set, then we must also have produced a recipient
                         //   info for this block
-                        let (addr, ix) = recipient_info.unwrap().recipient.clone();
-                        assert_eq!(&reward_set.remove(ix as usize), &addr,
-                            "BUG: Attempted to remove used address from reward set, but failed to do so safely");
+                        let mut recipients_to_remove: Vec<_> = recipient_info
+                            .unwrap()
+                            .recipients
+                            .iter()
+                            .map(|(addr, ix)| (addr.clone(), *ix))
+                            .collect();
+                        recipients_to_remove.sort_unstable_by(|(_, a), (_, b)| b.cmp(a));
+                        // remove from the reward set any consumed addresses in this first reward block
+                        for (addr, ix) in recipients_to_remove.iter() {
+                            assert_eq!(&reward_set.remove(*ix as usize), addr,
+                                       "BUG: Attempted to remove used address from reward set, but failed to do so safely");
+                        }
                     }
 
                     keys.push(db_keys::pox_reward_set_size().to_string());
@@ -3254,22 +3273,43 @@ impl<'a> SortitionHandleTx<'a> {
                 //   update the reward set
                 if let Some(reward_info) = recipient_info {
                     let mut current_len = self.get_reward_set_size()?;
-                    let (_, recipient_index) = reward_info.recipient;
-
-                    if recipient_index >= current_len {
-                        unreachable!(
-                            "Supplied index should never be greater than recipient set size"
-                        );
+                    let mut recipient_indexes: Vec<_> =
+                        reward_info.recipients.iter().map(|(_, x)| *x).collect();
+                    let mut remapped_entries = HashMap::new();
+                    // sort in decrementing order
+                    recipient_indexes.sort_unstable_by(|a, b| b.cmp(a));
+                    for index in recipient_indexes.into_iter() {
+                        // sanity check
+                        if index >= current_len {
+                            unreachable!(
+                                "Supplied index should never be greater than recipient set size"
+                            );
+                        } else if index + 1 == current_len {
+                            // selected index is the last element: no need to swap, just decrement len
+                            current_len -= 1;
+                        } else {
+                            let replacement = current_len - 1; // if current_len were 0, we would already have panicked.
+                            let replace_with = if let Some((_prior_ix, replace_with)) =
+                                remapped_entries.remove_entry(&replacement)
+                            {
+                                // the entry to swap in was itself swapped, so let's use the new value instead
+                                replace_with
+                            } else {
+                                self.get_reward_set_entry(replacement)?
+                            };
+
+                            // swap and decrement to remove from set
+                            remapped_entries.insert(index, replace_with);
+                            current_len -= 1;
+                        }
                     }
-
-                    current_len -= 1;
-                    let recipient = self.get_reward_set_entry(current_len)?;
-
                     // store the changes in the new trie
                     keys.push(db_keys::pox_reward_set_size().to_string());
                     values.push(db_keys::reward_set_size_to_string(current_len as usize));
-                    keys.push(db_keys::pox_reward_set_entry(recipient_index));
-                    values.push(recipient.to_string())
+                    for (recipient_index, replace_with) in remapped_entries.into_iter() {
+                        keys.push(db_keys::pox_reward_set_entry(recipient_index));
+                        values.push(replace_with.to_string())
+                    }
                 }
             }
         } else {

src/chainstate/burn/mod.rs

@@ -34,6 +34,7 @@ use burnchains::Txid;
 use util::hash::{to_hex, Hash160};
 use util::vrf::VRFProof;
 
+use rand::seq::index::sample;
 use rand::Rng;
 use rand::SeedableRng;
 use rand_chacha::ChaCha20Rng;
@@ -179,12 +180,22 @@ impl SortitionHash {
         SortitionHash(ret)
     }
 
-    /// Choose 1 index from the range [0, max).
-    pub fn choose(&self, max: u32) -> u32 {
+    /// Choose two indices (without replacement) from the range [0, max).
+    pub fn choose_two(&self, max: u32) -> Vec<u32> {
         let mut rng = ChaCha20Rng::from_seed(self.0.clone());
-        let index: u32 = rng.gen_range(0, max);
-        assert!(index < max);
-        index
+        if max < 2 {
+            return (0..max).collect();
+        }
+        let first = rng.gen_range(0, max);
+        let try_second = rng.gen_range(0, max - 1);
+        let second = if first == try_second {
+            // "swap" try_second with max
+            max - 1
+        } else {
+            try_second
+        };
+
+        vec![first, second]
     }
 
     /// Convert a SortitionHash into a (little-endian) uint256