From 51c59c9dd1f772c110a0844b7514e4f01b6f0e0a Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Tue, 13 Oct 2020 09:59:20 -0500 Subject: [PATCH 01/11] Revert "Strip dead code" This reverts commit 11b5457bf95a3bffc018ffbc871cb668c64cec36. Revert "Update tests" This reverts commit 8170d0f185016d46e68b481f6f91c68267b79d26. Revert "Fix integration test" This reverts commit 4377ea7f8b1cfd2c1197fc348c236e0c07c06e29. Revert "From 5 PoX recipients to 1" This reverts commit e29e17bbb2c2dd2940289c5a26597dc816d1513a. --- src/chainstate/burn/db/sortdb.rs | 78 ++++--- src/chainstate/burn/mod.rs | 17 +- .../burn/operations/leader_block_commit.rs | 220 ++++++++++++++++-- src/chainstate/coordinator/tests.rs | 33 ++- testnet/stacks-node/src/neon_node.rs | 4 +- 5 files changed, 274 insertions(+), 78 deletions(-) diff --git a/src/chainstate/burn/db/sortdb.rs b/src/chainstate/burn/db/sortdb.rs index 0930392d49d..32f5c594911 100644 --- a/src/chainstate/burn/db/sortdb.rs +++ b/src/chainstate/burn/db/sortdb.rs @@ -976,7 +976,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>, @@ -989,20 +989,20 @@ impl<'a> SortitionHandleTx<'a> { return Ok(None); } - let chosen_recipient = reward_set_vrf_seed.choose( + let chosen_recipients = reward_set_vrf_seed.choose( + OUTPUTS_PER_COMMIT.try_into().expect("BUG: u32 overflow in PoX outputs per commit"), 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) @@ -1016,13 +1016,17 @@ 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(OUTPUTS_PER_COMMIT.try_into().expect("BUG: u32 overflow in PoX outputs per commit"), + 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 { // no anchor block selected @@ -2339,7 +2343,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, @@ -2378,7 +2382,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( @@ -3233,9 +3237,14 @@ 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()); @@ -3258,17 +3267,32 @@ 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)); diff --git a/src/chainstate/burn/mod.rs b/src/chainstate/burn/mod.rs index e389b8e6112..ccda9771c96 100644 --- a/src/chainstate/burn/mod.rs +++ b/src/chainstate/burn/mod.rs @@ -44,6 +44,9 @@ use ripemd160::Ripemd160; use rusqlite::Connection; use rusqlite::Transaction; use sha2::Sha256; +use rand::SeedableRng; +use rand::seq::index::sample; +use rand_chacha::ChaCha20Rng; use chainstate::burn::db::sortdb::{PoxId, SortitionHandleTx, SortitionId}; @@ -182,12 +185,16 @@ impl SortitionHash { SortitionHash(ret) } - /// Choose 1 index from the range [0, max). - pub fn choose(&self, max: u32) -> u32 { + /// Choose n indices (without replacement) from the range [0, max). + pub fn choose(&self, n: u32, max: u32) -> Vec { let mut rng = ChaCha20Rng::from_seed(self.0.clone()); - let index: u32 = rng.gen_range(0, max); - assert!(index < max); - index + if n > max { + return (0..max).collect(); + } + sample(&mut rng, max as usize, n as usize) + // returned samples should always be u32, because max is u32. + .into_iter().map(|ix| ix.try_into().expect("CORRUPTION: u32-overflow in PoX recipient sample")) + .collect() } /// Convert a SortitionHash into a (little-endian) uint256 diff --git a/src/chainstate/burn/operations/leader_block_commit.rs b/src/chainstate/burn/operations/leader_block_commit.rs index 1ac2adf3f8b..e1636751b9d 100644 --- a/src/chainstate/burn/operations/leader_block_commit.rs +++ b/src/chainstate/burn/operations/leader_block_commit.rs @@ -63,7 +63,7 @@ struct ParsedData { memo: Vec, } -pub static OUTPUTS_PER_COMMIT: usize = 1; +pub static OUTPUTS_PER_COMMIT: usize = 5; impl LeaderBlockCommitOp { #[cfg(test)] @@ -365,7 +365,7 @@ impl BlockstackOperation for LeaderBlockCommitOp { pub struct RewardSetInfo { pub anchor_block: BlockHeaderHash, - pub recipient: (StacksAddress, u16), + pub recipients: Vec<(StacksAddress, u16)> } impl LeaderBlockCommitOp { @@ -405,19 +405,17 @@ impl LeaderBlockCommitOp { let expect_pox_descendant = if self.commit_outs.len() == 0 { false } else { - if self.commit_outs.len() != 1 { - warn!( - "Invalid block commit: expected {} PoX transfers, but commit has {}", - 1, - self.commit_outs.len() - ); - return Err(op_error::BlockCommitBadOutputs); + if self.commit_outs.len() != reward_set_info.recipients.len() { + warn!("Invalid block commit: expected {} PoX transfers, but commit has {}", + reward_set_info.recipients.len(), self.commit_outs.len()); + return Err(op_error::BlockCommitBadOutputs) } - let (expected_commit, _) = reward_set_info.recipient; - if !self.commit_outs.contains(&expected_commit) { - warn!("Invalid block commit: expected to send funds to {}, but that address is not in the committed output set", - expected_commit); - return Err(op_error::BlockCommitBadOutputs); + for (expected_commit, _) in reward_set_info.recipients.iter() { + if !self.commit_outs.contains(expected_commit) { + warn!("Invalid block commit: expected to send funds to {}, but that address is not in the committed output set", + expected_commit); + return Err(op_error::BlockCommitBadOutputs) + } } true }; @@ -607,15 +605,65 @@ mod tests { network_id: BitcoinNetworkType::Mainnet, bytes: Hash160([1; 20]), }, - }], + }, + BitcoinTxOutput { + units: 10, + address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([2; 20]) } + }, + BitcoinTxOutput { + units: 30, + address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([0; 20]) } + }, + ], }); let op = LeaderBlockCommitOp::parse_from_tx(16843019, &BurnchainHeaderHash([0; 32]), &tx) .unwrap(); - // should have 1 commit outputs, and a burn - assert_eq!(op.commit_outs.len(), 1); - assert_eq!(op.burn_fee, 10); + // should have 2 commit outputs, and a burn + assert_eq!(op.commit_outs.len(), 2); + assert_eq!(op.burn_fee, 50); + + let tx = BurnchainTransaction::Bitcoin(BitcoinTransaction { + txid: Txid([0; 32]), vtxindex: 0, + opcode: Opcodes::LeaderBlockCommit as u8, + data: vec![1; 80], + inputs: vec![BitcoinTxInput { + keys: vec![], + num_required: 0, + in_type: BitcoinInputType::Standard + }], + outputs: vec![ + BitcoinTxOutput { + units: 10, + address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([1; 20]) } + }, + BitcoinTxOutput { + units: 10, + address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([2; 20]) } + }, + BitcoinTxOutput { + units: 29, + address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([0; 20]) } + }, + ], + }); + + // burn amount should have been 30, not 29 + match LeaderBlockCommitOp::parse_from_tx(16843019, &BurnchainHeaderHash([0; 32]), &tx).unwrap_err() { + op_error::ParseError => {}, + _ => unreachable!(), + }; let tx = BurnchainTransaction::Bitcoin(BitcoinTransaction { txid: Txid([0; 32]), @@ -634,15 +682,40 @@ mod tests { network_id: BitcoinNetworkType::Mainnet, bytes: Hash160([1; 20]), }, - }], + }, + BitcoinTxOutput { + units: 13, + address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([2; 20]) } + }, + BitcoinTxOutput { + units: 13, + address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([2; 20]) } + }, + BitcoinTxOutput { + units: 13, + address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([2; 20]) } + }, + BitcoinTxOutput { + units: 13, + address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([2; 20]) } + }, + ], }); let op = LeaderBlockCommitOp::parse_from_tx(16843019, &BurnchainHeaderHash([0; 32]), &tx) .unwrap(); - // should have 1 commit outputs - assert_eq!(op.commit_outs.len(), 1); - assert_eq!(op.burn_fee, 13); + // should have 5 commit outputs + assert_eq!(op.commit_outs.len(), 5); + assert_eq!(op.burn_fee, 65); let tx = BurnchainTransaction::Bitcoin(BitcoinTransaction { txid: Txid([0; 32]), @@ -654,7 +727,32 @@ mod tests { num_required: 0, in_type: BitcoinInputType::Standard, }], - outputs: vec![], + outputs: vec![ + BitcoinTxOutput { + units: 13, + address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([1; 20]) } + }, + BitcoinTxOutput { + units: 13, + address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([2; 20]) } + }, + BitcoinTxOutput { + units: 13, + address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([2; 20]) } + }, + BitcoinTxOutput { + units: 13, + address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([2; 20]) } + }, + ], }); // not enough PoX outputs @@ -665,6 +763,55 @@ mod tests { _ => unreachable!(), }; + let tx = BurnchainTransaction::Bitcoin(BitcoinTransaction { + txid: Txid([0; 32]), vtxindex: 0, + opcode: Opcodes::LeaderBlockCommit as u8, + data: vec![1; 80], + inputs: vec![BitcoinTxInput { + keys: vec![], + num_required: 0, + in_type: BitcoinInputType::Standard + }], + outputs: vec![ + BitcoinTxOutput { + units: 13, + address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([1; 20]) } + }, + BitcoinTxOutput { + units: 13, + address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([2; 20]) } + }, + BitcoinTxOutput { + units: 13, + address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([2; 20]) } + }, + BitcoinTxOutput { + units: 10, + address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([2; 20]) } + }, + BitcoinTxOutput { + units: 13, + address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([2; 20]) } + }, + ], + }); + + // unequal PoX outputs + match LeaderBlockCommitOp::parse_from_tx(16843019, &BurnchainHeaderHash([0; 32]), &tx).unwrap_err() { + op_error::ParseError => {}, + _ => unreachable!(), + }; + let tx = BurnchainTransaction::Bitcoin(BitcoinTransaction { txid: Txid([0; 32]), vtxindex: 0, @@ -682,7 +829,32 @@ mod tests { network_id: BitcoinNetworkType::Mainnet, bytes: Hash160([1; 20]), }, - }], + }, + BitcoinTxOutput { + units: 0, + address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([2; 20]) } + }, + BitcoinTxOutput { + units: 0, + address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([2; 20]) } + }, + BitcoinTxOutput { + units: 0, + address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([2; 20]) } + }, + BitcoinTxOutput { + units: 0, + address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([2; 20]) } + }, + ], }); // 0 total burn diff --git a/src/chainstate/coordinator/tests.rs b/src/chainstate/coordinator/tests.rs index d6347a1b459..d18577d1c71 100644 --- a/src/chainstate/coordinator/tests.rs +++ b/src/chainstate/coordinator/tests.rs @@ -350,8 +350,7 @@ fn make_genesis_block_with_recipients( builder.epoch_finish(epoch_tx); let commit_outs = if let Some(recipients) = recipients { - let (addr, _) = recipients.recipient; - vec![addr] + recipients.recipients.iter().map(|(a, _)| a.clone()).collect() } else { vec![] }; @@ -474,8 +473,7 @@ fn make_stacks_block_with_recipients( builder.epoch_finish(epoch_tx); let commit_outs = if let Some(recipients) = recipients { - let (addr, _) = recipients.recipient; - vec![addr] + recipients.recipients.iter().map(|(a, _)| a.clone()).collect() } else { vec![] }; @@ -708,10 +706,8 @@ fn test_sortition_with_reward_set() { let mut vrf_keys: Vec<_> = (0..150).map(|_| VRFPrivateKey::new()).collect(); let mut committers: Vec<_> = (0..150).map(|_| StacksPrivateKey::new()).collect(); - let reward_set_size = 5; - let reward_set: Vec<_> = (0..reward_set_size) - .map(|_| p2pkh_from(&StacksPrivateKey::new())) - .collect(); + let reward_set_size = 15; + let reward_set: Vec<_> = (0..reward_set_size).map(|_| p2pkh_from(&StacksPrivateKey::new())).collect(); setup_states(&[path], &vrf_keys, &committers); @@ -797,14 +793,11 @@ fn test_sortition_with_reward_set() { .test_get_next_block_recipients(reward_cycle_info.as_ref()) .unwrap(); if let Some(ref next_block_recipients) = next_block_recipients { - let (addr, _) = next_block_recipients.recipient; - assert!( - !reward_recipients.contains(&addr), - "Reward set should not already contain address {}", - addr - ); - eprintln!("At iteration: {}, inserting address ... {}", ix, addr); - reward_recipients.insert(addr.clone()); + for (addr, _) in next_block_recipients.recipients.iter() { + assert!(!reward_recipients.contains(addr), "Reward set should not already contain address {}", addr); + eprintln!("At iteration: {}, inserting address ... {}", ix, addr); + reward_recipients.insert(addr.clone()); + } } let (good_op, mut block) = if ix == 0 { @@ -860,14 +853,14 @@ fn test_sortition_with_reward_set() { // sometime have the wrong _number_ of recipients, // other times just have the wrong set of recipients - let recipient = if ix % 2 == 0 { - (p2pkh_from(miner_wrong_out), 0) + let recipients = if ix % 2 == 0 { + vec![(p2pkh_from(miner_wrong_out), 0)] } else { - (p2pkh_from(&StacksPrivateKey::new()), 0) + (0..OUTPUTS_PER_COMMIT).map(|ix| (p2pkh_from(&StacksPrivateKey::new()), ix as u16)).collect() }; let bad_block_recipipients = Some(RewardSetInfo { anchor_block: BlockHeaderHash([0; 32]), - recipient, + recipients }); let (bad_outs_op, _) = make_stacks_block_with_recipients( &sort_db, diff --git a/testnet/stacks-node/src/neon_node.rs b/testnet/stacks-node/src/neon_node.rs index b6151eb9442..357ca8ad76c 100644 --- a/testnet/stacks-node/src/neon_node.rs +++ b/testnet/stacks-node/src/neon_node.rs @@ -210,8 +210,8 @@ fn inner_generate_block_commit_op( let (parent_block_ptr, parent_vtxindex) = (parent_burnchain_height, parent_winning_vtx); let commit_outs = if let Some(recipient_set) = recipients { - let (addr, _) = recipient_set.recipient; - vec![addr] + recipient_set.recipients.into_iter() + .map(|(recipient, _)| recipient).collect() } else { vec![] }; From 825afe0aa7785d6200908c6b1f7045f60e077dfb Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Wed, 14 Oct 2020 17:14:36 -0500 Subject: [PATCH 02/11] most of the work of porting to multiple commit_outs --- src/burnchains/burnchain.rs | 2 +- src/burnchains/mod.rs | 3 +- src/chainstate/burn/db/sortdb.rs | 56 ++- src/chainstate/burn/mod.rs | 25 +- .../burn/operations/leader_block_commit.rs | 452 ++++++++++-------- src/chainstate/coordinator/tests.rs | 30 +- src/chainstate/stacks/boot/mod.rs | 2 +- src/net/mod.rs | 2 +- .../burnchains/bitcoin_regtest_controller.rs | 22 +- testnet/stacks-node/src/neon_node.rs | 7 +- 10 files changed, 333 insertions(+), 268 deletions(-) diff --git a/src/burnchains/burnchain.rs b/src/burnchains/burnchain.rs index 24d132f9546..8c10539e5c5 100644 --- a/src/burnchains/burnchain.rs +++ b/src/burnchains/burnchain.rs @@ -570,7 +570,7 @@ impl Burnchain { } } } - x if x == Opcodes::LeaderBlockCommit as u8 => { + x if x == Opcodes::LeaderBlockCommit as u8 || x == Opcodes::LeaderBlockCommitTransfer as u8 => { match LeaderBlockCommitOp::from_tx(block_header, burn_tx) { Ok(op) => Some(BlockstackOperationType::LeaderBlockCommit(op)), Err(e) => { diff --git a/src/burnchains/mod.rs b/src/burnchains/mod.rs index 07daad27949..bc4d52081e3 100644 --- a/src/burnchains/mod.rs +++ b/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? diff --git a/src/chainstate/burn/db/sortdb.rs b/src/chainstate/burn/db/sortdb.rs index e05d9e615b0..0f23c875bf1 100644 --- a/src/chainstate/burn/db/sortdb.rs +++ b/src/chainstate/burn/db/sortdb.rs @@ -986,8 +986,7 @@ impl<'a> SortitionHandleTx<'a> { return Ok(None); } - let chosen_recipients = reward_set_vrf_seed.choose( - OUTPUTS_PER_COMMIT.try_into().expect("BUG: u32 overflow in PoX outputs per commit"), + let chosen_recipients = reward_set_vrf_seed.choose_two( reward_set .len() .try_into() @@ -996,10 +995,13 @@ impl<'a> SortitionHandleTx<'a> { Ok(Some(RewardSetInfo { anchor_block: anchor_block.clone(), - recipients: chosen_recipients.into_iter().map(|ix| { - let recipient = reward_set[ix as usize].clone(); - (recipient, u16::try_from(ix).unwrap()) - }).collect() + 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,8 +1015,7 @@ impl<'a> SortitionHandleTx<'a> { if reward_set_size == 0 { Ok(None) } else { - let chosen_recipients = reward_set_vrf_seed.choose(OUTPUTS_PER_COMMIT.try_into().expect("BUG: u32 overflow in PoX outputs per commit"), - reward_set_size as u32); + 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(); @@ -1023,7 +1024,8 @@ impl<'a> SortitionHandleTx<'a> { } Ok(Some(RewardSetInfo { anchor_block, - recipients })) + recipients, + })) } } else { // no anchor block selected @@ -3234,8 +3236,12 @@ 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 mut recipients_to_remove: Vec<_> = recipient_info.unwrap().recipients.iter().map( - |(addr, ix)| (addr.clone(), *ix)).collect(); + 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() { @@ -3264,26 +3270,30 @@ 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 mut recipient_indexes: Vec<_> = reward_info.recipients.iter().map(|(_, x)| *x).collect(); + 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"); + 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)? - }; + 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); @@ -3293,8 +3303,10 @@ impl<'a> SortitionHandleTx<'a> { // 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 { diff --git a/src/chainstate/burn/mod.rs b/src/chainstate/burn/mod.rs index 939f7a8e12e..b2ad52ed783 100644 --- a/src/chainstate/burn/mod.rs +++ b/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; @@ -41,9 +42,6 @@ use ripemd160::Ripemd160; use rusqlite::Connection; use rusqlite::Transaction; use sha2::Sha256; -use rand::SeedableRng; -use rand::seq::index::sample; -use rand_chacha::ChaCha20Rng; use chainstate::burn::db::sortdb::{PoxId, SortitionHandleTx, SortitionId}; @@ -111,6 +109,7 @@ impl_byte_array_newtype!(SortitionHash, u8, 32); #[repr(u8)] pub enum Opcodes { LeaderBlockCommit = '[' as u8, + LeaderBlockCommitTransfer = ']' as u8, LeaderKeyRegister = '^' as u8, UserBurnSupport = '_' as u8, } @@ -182,16 +181,22 @@ impl SortitionHash { SortitionHash(ret) } - /// Choose n indices (without replacement) from the range [0, max). - pub fn choose(&self, n: u32, max: u32) -> Vec { + /// Choose two indices (without replacement) from the range [0, max). + pub fn choose_two(&self, max: u32) -> Vec { let mut rng = ChaCha20Rng::from_seed(self.0.clone()); - if n > max { + if max < 2 { return (0..max).collect(); } - sample(&mut rng, max as usize, n as usize) - // returned samples should always be u32, because max is u32. - .into_iter().map(|ix| ix.try_into().expect("CORRUPTION: u32-overflow in PoX recipient sample")) - .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 diff --git a/src/chainstate/burn/operations/leader_block_commit.rs b/src/chainstate/burn/operations/leader_block_commit.rs index c4a04d40a0a..6cc0fcc1f80 100644 --- a/src/chainstate/burn/operations/leader_block_commit.rs +++ b/src/chainstate/burn/operations/leader_block_commit.rs @@ -60,7 +60,7 @@ struct ParsedData { memo: Vec, } -pub static OUTPUTS_PER_COMMIT: usize = 5; +pub static OUTPUTS_PER_COMMIT: usize = 2; impl LeaderBlockCommitOp { #[cfg(test)] @@ -193,10 +193,14 @@ impl LeaderBlockCommitOp { return Err(op_error::InvalidInput); } - if tx.opcode() != (Opcodes::LeaderBlockCommit as u8) { + let expects_pox_outputs = if tx.opcode() == Opcodes::LeaderBlockCommit as u8 { + false + } else if tx.opcode() == Opcodes::LeaderBlockCommitTransfer as u8 { + true + } else { warn!("Invalid tx: invalid opcode {}", tx.opcode()); return Err(op_error::InvalidInput); - } + }; let data = LeaderBlockCommitOp::parse_data(&tx.data()).ok_or_else(|| { warn!("Invalid tx data"); @@ -234,20 +238,26 @@ impl LeaderBlockCommitOp { return Err(op_error::ParseError); } - let mut commit_outs = vec![]; - let mut pox_fee = None; - let mut burn_fee = None; - - for (ix, output) in outputs.into_iter().enumerate() { - // only look at the first OUTPUTS_PER_COMMIT outputs - // or until first _burn_ output - if ix >= OUTPUTS_PER_COMMIT { - break; + let (commit_outs, burn_fee) = if !expects_pox_outputs { + // this is a single output _burn_ commitment. + if !outputs[0].address.is_burn() { + warn!("Invalid tx: should be a burn commitment, but address is not the burn address"); + return Err(op_error::InvalidInput); } - if output.address.is_burn() { - burn_fee.replace(output.amount); - break; - } else { + // mock the commit_outs to burn outputs. + let commit_outs = (0..OUTPUTS_PER_COMMIT) + .map(|_| outputs[0].address.clone()) + .collect(); + let burn_fee = outputs[0].amount; + (commit_outs, burn_fee) + } else { + let mut commit_outs = vec![]; + let mut pox_fee = None; + for (ix, output) in outputs.into_iter().enumerate() { + // only look at the first OUTPUTS_PER_COMMIT outputs + if ix >= OUTPUTS_PER_COMMIT { + break; + } // all pox outputs must have the same fee if let Some(pox_fee) = pox_fee { if output.amount != pox_fee { @@ -259,38 +269,19 @@ impl LeaderBlockCommitOp { } commit_outs.push(output.address); } - } - - // EITHER there was an amount burned _or_ there were OUTPUTS_PER_COMMIT pox outputs - if burn_fee.is_none() && commit_outs.len() != OUTPUTS_PER_COMMIT { - warn!("Invalid commit tx: if fewer than {} PoX addresses are committed to, remainder must be burnt", OUTPUTS_PER_COMMIT); - return Err(op_error::ParseError); - } - // compute the total amount transfered/burned, and check that the burn amount - // is expected given the amount transfered. - let burn_fee = match (burn_fee, pox_fee) { - (Some(burned_amount), Some(pox_amount)) => { - // burned amount must be equal to the "missing" - // PoX slots - let expected_burn_amount = pox_amount - .checked_mul((OUTPUTS_PER_COMMIT - commit_outs.len()) as u64) - .ok_or_else(|| op_error::ParseError)?; - if expected_burn_amount != burned_amount { - warn!("Invalid commit tx: burned output different from PoX reward output"); - return Err(op_error::ParseError); - } - pox_amount - .checked_mul(OUTPUTS_PER_COMMIT as u64) - .ok_or_else(|| op_error::ParseError)? - } - (Some(burned_amount), None) => burned_amount, - (None, Some(pox_amount)) => pox_amount - .checked_mul(OUTPUTS_PER_COMMIT as u64) - .ok_or_else(|| op_error::ParseError)?, - (None, None) => { - unreachable!("A 0-len output should have already errored"); + if commit_outs.len() != OUTPUTS_PER_COMMIT { + warn!("Invalid commit tx: {} commit addresses, but {} PoX addresses should be committed to", commit_outs.len(), OUTPUTS_PER_COMMIT); + return Err(op_error::InvalidInput); } + + // compute the total amount transfered/burned, and check that the burn amount + // is expected given the amount transfered. + let burn_fee = pox_fee + .expect("A 0-len output should have already errored") + .checked_mul(OUTPUTS_PER_COMMIT as u64) // total commitment is the pox_amount * outputs + .ok_or_else(|| op_error::ParseError)?; + (commit_outs, burn_fee) }; if burn_fee == 0 { @@ -317,6 +308,13 @@ impl LeaderBlockCommitOp { burn_header_hash: block_hash.clone(), }) } + + /// are all the outputs for this block commit burns? + pub fn all_outputs_burn(&self) -> bool { + self.commit_outs.iter().fold(true, |previous_is_burn, output_addr| { + previous_is_burn && output_addr.is_burn() + }) + } } impl StacksMessageCodec for LeaderBlockCommitOp { @@ -329,7 +327,11 @@ impl StacksMessageCodec for LeaderBlockCommitOp { block txoff block txoff */ fn consensus_serialize(&self, fd: &mut W) -> Result<(), net_error> { - write_next(fd, &(Opcodes::LeaderBlockCommit as u8))?; + if self.all_outputs_burn() { + write_next(fd, &(Opcodes::LeaderBlockCommit as u8))? + } else { + write_next(fd, &(Opcodes::LeaderBlockCommitTransfer as u8))? + }; write_next(fd, &self.block_header_hash)?; fd.write_all(&self.new_seed.as_bytes()[..]) .map_err(net_error::WriteError)?; @@ -362,7 +364,29 @@ impl BlockstackOperation for LeaderBlockCommitOp { pub struct RewardSetInfo { pub anchor_block: BlockHeaderHash, - pub recipients: Vec<(StacksAddress, u16)> + pub recipients: Vec<(StacksAddress, u16)>, +} + +impl RewardSetInfo { + /// Takes an Option and produces the commit_outs + /// for a corresponding LeaderBlockCommitOp. If RewardSetInfo is none, + /// the LeaderBlockCommitOp will use burn addresses. + pub fn into_commit_outs(from: Option, mainnet: bool) -> Vec { + if let Some(recipient_set) = from { + let mut outs: Vec<_> = recipient_set + .recipients + .into_iter() + .map(|(recipient, _)| recipient) + .collect(); + while outs.len() < OUTPUTS_PER_COMMIT { + outs.push(StacksAddress::burn_address(mainnet)); + } + outs + } else { + (0..OUTPUTS_PER_COMMIT) + .map(|_| StacksAddress::burn_address(mainnet)).collect() + } + } } impl LeaderBlockCommitOp { @@ -399,42 +423,56 @@ impl LeaderBlockCommitOp { // we do this because the descended_from check isn't particularly cheap, so // we want to make sure that any TX that forces us to perform the check // has either burned BTC or sent BTC to the PoX recipients - let expect_pox_descendant = if self.commit_outs.len() == 0 { - false + + // first, handle a corner case: + // all of the commitment outputs are _burns_ + // _and_ the reward set chose two burn addresses as reward addresses. + let recipient_set_all_burns = reward_set_info.recipients.iter() + .fold(true, |prior_is_burn, (addr, _)| prior_is_burn && addr.is_burn()); + + if self.all_outputs_burn() && recipient_set_all_burns { + // pass } else { - if self.commit_outs.len() != reward_set_info.recipients.len() { - warn!("Invalid block commit: expected {} PoX transfers, but commit has {}", - reward_set_info.recipients.len(), self.commit_outs.len()); - return Err(op_error::BlockCommitBadOutputs) - } - for (expected_commit, _) in reward_set_info.recipients.iter() { - if !self.commit_outs.contains(expected_commit) { - warn!("Invalid block commit: expected to send funds to {}, but that address is not in the committed output set", - expected_commit); - return Err(op_error::BlockCommitBadOutputs) + let expect_pox_descendant = if self.all_outputs_burn() { + false + } else { + if self.commit_outs.len() != reward_set_info.recipients.len() { + warn!( + "Invalid block commit: expected {} PoX transfers, but commit has {}", + reward_set_info.recipients.len(), + self.commit_outs.len() + ); + return Err(op_error::BlockCommitBadOutputs); } - } - true - }; + for (expected_commit, _) in reward_set_info.recipients.iter() { + if !self.commit_outs.contains(expected_commit) { + warn!("Invalid block commit: expected to send funds to {}, but that address is not in the committed output set", + expected_commit); + return Err(op_error::BlockCommitBadOutputs); + } + } + true + }; - let descended_from_anchor = tx.descended_from(parent_block_height, &reward_set_info.anchor_block) - .map_err(|e| { - error!("Failed to check whether parent (height={}) is descendent of anchor block={}: {}", - parent_block_height, &reward_set_info.anchor_block, e); - op_error::BlockCommitAnchorCheck})?; - if descended_from_anchor != expect_pox_descendant { - if descended_from_anchor { - warn!("Invalid block commit: descended from PoX anchor, but used burn outputs"); - } else { - warn!( - "Invalid block commit: not descended from PoX anchor, but used PoX outputs" - ); + let descended_from_anchor = tx.descended_from(parent_block_height, &reward_set_info.anchor_block) + .map_err(|e| { + error!("Failed to check whether parent (height={}) is descendent of anchor block={}: {}", + parent_block_height, &reward_set_info.anchor_block, e); + op_error::BlockCommitAnchorCheck})?; + if descended_from_anchor != expect_pox_descendant { + if descended_from_anchor { + warn!("Invalid block commit: descended from PoX anchor, but used burn outputs"); + } else { + warn!( + "Invalid block commit: not descended from PoX anchor, but used PoX outputs" + ); + } + return Err(op_error::BlockCommitBadOutputs); } - return Err(op_error::BlockCommitBadOutputs); } } else { // no recipient info for this sortition, so expect all burns - if self.commit_outs.len() != 0 { + if !self.all_outputs_burn() { warn!("Invalid block commit: this transaction should only have burn outputs."); return Err(op_error::BlockCommitBadOutputs); } @@ -588,32 +626,37 @@ mod tests { let tx = BurnchainTransaction::Bitcoin(BitcoinTransaction { txid: Txid([0; 32]), vtxindex: 0, - opcode: Opcodes::LeaderBlockCommit as u8, + opcode: Opcodes::LeaderBlockCommitTransfer as u8, data: vec![1; 80], inputs: vec![BitcoinTxInput { keys: vec![], num_required: 0, in_type: BitcoinInputType::Standard, }], - outputs: vec![BitcoinTxOutput { - units: 10, - address: BitcoinAddress { - addrtype: BitcoinAddressType::PublicKeyHash, - network_id: BitcoinNetworkType::Mainnet, - bytes: Hash160([1; 20]), + outputs: vec![ + BitcoinTxOutput { + units: 10, + address: BitcoinAddress { + addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([1; 20]), + }, }, - }, BitcoinTxOutput { units: 10, - address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, - network_id: BitcoinNetworkType::Mainnet, - bytes: Hash160([2; 20]) } + address: BitcoinAddress { + addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([2; 20]), + }, }, BitcoinTxOutput { units: 30, - address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, - network_id: BitcoinNetworkType::Mainnet, - bytes: Hash160([0; 20]) } + address: BitcoinAddress { + addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([0; 20]), + }, }, ], }); @@ -621,88 +664,98 @@ mod tests { let op = LeaderBlockCommitOp::parse_from_tx(16843019, &BurnchainHeaderHash([0; 32]), &tx) .unwrap(); - // should have 2 commit outputs, and a burn + // should have 2 commit outputs, summing to 20 burned units assert_eq!(op.commit_outs.len(), 2); - assert_eq!(op.burn_fee, 50); + assert_eq!(op.burn_fee, 20); let tx = BurnchainTransaction::Bitcoin(BitcoinTransaction { - txid: Txid([0; 32]), vtxindex: 0, - opcode: Opcodes::LeaderBlockCommit as u8, + txid: Txid([0; 32]), + vtxindex: 0, + opcode: Opcodes::LeaderBlockCommitTransfer as u8, data: vec![1; 80], inputs: vec![BitcoinTxInput { keys: vec![], num_required: 0, - in_type: BitcoinInputType::Standard + in_type: BitcoinInputType::Standard, }], outputs: vec![ BitcoinTxOutput { units: 10, - address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, - network_id: BitcoinNetworkType::Mainnet, - bytes: Hash160([1; 20]) } - }, - BitcoinTxOutput { - units: 10, - address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, - network_id: BitcoinNetworkType::Mainnet, - bytes: Hash160([2; 20]) } + address: BitcoinAddress { + addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([1; 20]), + }, }, BitcoinTxOutput { - units: 29, - address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, - network_id: BitcoinNetworkType::Mainnet, - bytes: Hash160([0; 20]) } + units: 9, + address: BitcoinAddress { + addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([0; 20]), + }, }, ], }); - // burn amount should have been 30, not 29 - match LeaderBlockCommitOp::parse_from_tx(16843019, &BurnchainHeaderHash([0; 32]), &tx).unwrap_err() { - op_error::ParseError => {}, + // burn amount should have been 10, not 9 + match LeaderBlockCommitOp::parse_from_tx(16843019, &BurnchainHeaderHash([0; 32]), &tx) + .unwrap_err() + { + op_error::ParseError => {} _ => unreachable!(), }; let tx = BurnchainTransaction::Bitcoin(BitcoinTransaction { txid: Txid([0; 32]), vtxindex: 0, - opcode: Opcodes::LeaderBlockCommit as u8, + opcode: Opcodes::LeaderBlockCommitTransfer as u8, data: vec![1; 80], inputs: vec![BitcoinTxInput { keys: vec![], num_required: 0, in_type: BitcoinInputType::Standard, }], - outputs: vec![BitcoinTxOutput { - units: 13, - address: BitcoinAddress { - addrtype: BitcoinAddressType::PublicKeyHash, - network_id: BitcoinNetworkType::Mainnet, - bytes: Hash160([1; 20]), + outputs: vec![ + BitcoinTxOutput { + units: 13, + address: BitcoinAddress { + addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([1; 20]), + }, }, - }, BitcoinTxOutput { units: 13, - address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, - network_id: BitcoinNetworkType::Mainnet, - bytes: Hash160([2; 20]) } + address: BitcoinAddress { + addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([2; 20]), + }, }, BitcoinTxOutput { units: 13, - address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, - network_id: BitcoinNetworkType::Mainnet, - bytes: Hash160([2; 20]) } + address: BitcoinAddress { + addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([2; 20]), + }, }, BitcoinTxOutput { units: 13, - address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, - network_id: BitcoinNetworkType::Mainnet, - bytes: Hash160([2; 20]) } + address: BitcoinAddress { + addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([2; 20]), + }, }, BitcoinTxOutput { units: 13, - address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, - network_id: BitcoinNetworkType::Mainnet, - bytes: Hash160([2; 20]) } + address: BitcoinAddress { + addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([2; 20]), + }, }, ], }); @@ -710,14 +763,14 @@ mod tests { let op = LeaderBlockCommitOp::parse_from_tx(16843019, &BurnchainHeaderHash([0; 32]), &tx) .unwrap(); - // should have 5 commit outputs - assert_eq!(op.commit_outs.len(), 5); - assert_eq!(op.burn_fee, 65); + // should have 2 commit outputs + assert_eq!(op.commit_outs.len(), 2); + assert_eq!(op.burn_fee, 26); let tx = BurnchainTransaction::Bitcoin(BitcoinTransaction { txid: Txid([0; 32]), vtxindex: 0, - opcode: Opcodes::LeaderBlockCommit as u8, + opcode: Opcodes::LeaderBlockCommitTransfer as u8, data: vec![1; 80], inputs: vec![BitcoinTxInput { keys: vec![], @@ -727,27 +780,11 @@ mod tests { outputs: vec![ BitcoinTxOutput { units: 13, - address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, - network_id: BitcoinNetworkType::Mainnet, - bytes: Hash160([1; 20]) } - }, - BitcoinTxOutput { - units: 13, - address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, - network_id: BitcoinNetworkType::Mainnet, - bytes: Hash160([2; 20]) } - }, - BitcoinTxOutput { - units: 13, - address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, - network_id: BitcoinNetworkType::Mainnet, - bytes: Hash160([2; 20]) } - }, - BitcoinTxOutput { - units: 13, - address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, - network_id: BitcoinNetworkType::Mainnet, - bytes: Hash160([2; 20]) } + address: BitcoinAddress { + addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([1; 20]), + }, }, ], }); @@ -761,95 +798,93 @@ mod tests { }; let tx = BurnchainTransaction::Bitcoin(BitcoinTransaction { - txid: Txid([0; 32]), vtxindex: 0, - opcode: Opcodes::LeaderBlockCommit as u8, + txid: Txid([0; 32]), + vtxindex: 0, + opcode: Opcodes::LeaderBlockCommitTransfer as u8, data: vec![1; 80], inputs: vec![BitcoinTxInput { keys: vec![], num_required: 0, - in_type: BitcoinInputType::Standard + in_type: BitcoinInputType::Standard, }], outputs: vec![ BitcoinTxOutput { units: 13, - address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, - network_id: BitcoinNetworkType::Mainnet, - bytes: Hash160([1; 20]) } - }, - BitcoinTxOutput { - units: 13, - address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, - network_id: BitcoinNetworkType::Mainnet, - bytes: Hash160([2; 20]) } - }, - BitcoinTxOutput { - units: 13, - address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, - network_id: BitcoinNetworkType::Mainnet, - bytes: Hash160([2; 20]) } + address: BitcoinAddress { + addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([1; 20]), + }, }, BitcoinTxOutput { units: 10, - address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, - network_id: BitcoinNetworkType::Mainnet, - bytes: Hash160([2; 20]) } - }, - BitcoinTxOutput { - units: 13, - address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, - network_id: BitcoinNetworkType::Mainnet, - bytes: Hash160([2; 20]) } + address: BitcoinAddress { + addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([2; 20]), + }, }, ], }); // unequal PoX outputs - match LeaderBlockCommitOp::parse_from_tx(16843019, &BurnchainHeaderHash([0; 32]), &tx).unwrap_err() { - op_error::ParseError => {}, + match LeaderBlockCommitOp::parse_from_tx(16843019, &BurnchainHeaderHash([0; 32]), &tx) + .unwrap_err() + { + op_error::ParseError => {} _ => unreachable!(), }; let tx = BurnchainTransaction::Bitcoin(BitcoinTransaction { txid: Txid([0; 32]), vtxindex: 0, - opcode: Opcodes::LeaderBlockCommit as u8, + opcode: Opcodes::LeaderBlockCommitTransfer as u8, data: vec![1; 80], inputs: vec![BitcoinTxInput { keys: vec![], num_required: 0, in_type: BitcoinInputType::Standard, }], - outputs: vec![BitcoinTxOutput { - units: 0, - address: BitcoinAddress { - addrtype: BitcoinAddressType::PublicKeyHash, - network_id: BitcoinNetworkType::Mainnet, - bytes: Hash160([1; 20]), + outputs: vec![ + BitcoinTxOutput { + units: 0, + address: BitcoinAddress { + addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([1; 20]), + }, }, - }, BitcoinTxOutput { units: 0, - address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, - network_id: BitcoinNetworkType::Mainnet, - bytes: Hash160([2; 20]) } + address: BitcoinAddress { + addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([2; 20]), + }, }, BitcoinTxOutput { units: 0, - address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, - network_id: BitcoinNetworkType::Mainnet, - bytes: Hash160([2; 20]) } + address: BitcoinAddress { + addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([2; 20]), + }, }, BitcoinTxOutput { units: 0, - address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, - network_id: BitcoinNetworkType::Mainnet, - bytes: Hash160([2; 20]) } + address: BitcoinAddress { + addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([2; 20]), + }, }, BitcoinTxOutput { units: 0, - address: BitcoinAddress { addrtype: BitcoinAddressType::PublicKeyHash, - network_id: BitcoinNetworkType::Mainnet, - bytes: Hash160([2; 20]) } + address: BitcoinAddress { + addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([2; 20]), + }, }, ], }); @@ -886,7 +921,10 @@ mod tests { key_vtxindex: 0x7071, memo: vec![0x80], - commit_outs: vec![], + commit_outs: vec![ + StacksAddress { version: 26, bytes: Hash160::empty() }, + StacksAddress { version: 26, bytes: Hash160::empty() } + ], burn_fee: 12345, input: BurnchainSigner { diff --git a/src/chainstate/coordinator/tests.rs b/src/chainstate/coordinator/tests.rs index c956b731907..afeb5c4544c 100644 --- a/src/chainstate/coordinator/tests.rs +++ b/src/chainstate/coordinator/tests.rs @@ -366,7 +366,11 @@ fn make_genesis_block_with_recipients( builder.epoch_finish(epoch_tx); let commit_outs = if let Some(recipients) = recipients { - recipients.recipients.iter().map(|(a, _)| a.clone()).collect() + recipients + .recipients + .iter() + .map(|(a, _)| a.clone()) + .collect() } else { vec![] }; @@ -489,7 +493,11 @@ fn make_stacks_block_with_recipients( builder.epoch_finish(epoch_tx); let commit_outs = if let Some(recipients) = recipients { - recipients.recipients.iter().map(|(a, _)| a.clone()).collect() + recipients + .recipients + .iter() + .map(|(a, _)| a.clone()) + .collect() } else { vec![] }; @@ -722,8 +730,10 @@ fn test_sortition_with_reward_set() { let mut vrf_keys: Vec<_> = (0..150).map(|_| VRFPrivateKey::new()).collect(); let mut committers: Vec<_> = (0..150).map(|_| StacksPrivateKey::new()).collect(); - let reward_set_size = 15; - let reward_set: Vec<_> = (0..reward_set_size).map(|_| p2pkh_from(&StacksPrivateKey::new())).collect(); + let reward_set_size = 10; + let reward_set: Vec<_> = (0..reward_set_size) + .map(|_| p2pkh_from(&StacksPrivateKey::new())) + .collect(); setup_states(&[path], &vrf_keys, &committers); @@ -810,7 +820,11 @@ fn test_sortition_with_reward_set() { .unwrap(); if let Some(ref next_block_recipients) = next_block_recipients { for (addr, _) in next_block_recipients.recipients.iter() { - assert!(!reward_recipients.contains(addr), "Reward set should not already contain address {}", addr); + assert!( + !reward_recipients.contains(addr), + "Reward set should not already contain address {}", + addr + ); eprintln!("At iteration: {}, inserting address ... {}", ix, addr); reward_recipients.insert(addr.clone()); } @@ -872,11 +886,13 @@ fn test_sortition_with_reward_set() { let recipients = if ix % 2 == 0 { vec![(p2pkh_from(miner_wrong_out), 0)] } else { - (0..OUTPUTS_PER_COMMIT).map(|ix| (p2pkh_from(&StacksPrivateKey::new()), ix as u16)).collect() + (0..OUTPUTS_PER_COMMIT) + .map(|ix| (p2pkh_from(&StacksPrivateKey::new()), ix as u16)) + .collect() }; let bad_block_recipipients = Some(RewardSetInfo { anchor_block: BlockHeaderHash([0; 32]), - recipients + recipients, }); let (bad_outs_op, _) = make_stacks_block_with_recipients( &sort_db, diff --git a/src/chainstate/stacks/boot/mod.rs b/src/chainstate/stacks/boot/mod.rs index da4873e2560..99585005df0 100644 --- a/src/chainstate/stacks/boot/mod.rs +++ b/src/chainstate/stacks/boot/mod.rs @@ -388,7 +388,7 @@ pub mod test { #[test] fn get_reward_threshold_units() { - let test_pox_constants = PoxConstants::new(1000, 1, 1, 1, 5); + let test_pox_constants = PoxConstants::new(500, 1, 1, 1, 5); // when the liquid amount = the threshold step, // the threshold should always be the step size. let liquid = POX_THRESHOLD_STEPS_USTX; diff --git a/src/net/mod.rs b/src/net/mod.rs index abda8df964b..26148dcd9d8 100644 --- a/src/net/mod.rs +++ b/src/net/mod.rs @@ -2927,7 +2927,7 @@ pub mod test { ) { Ok(recipients) => { block_commit_op.commit_outs = match recipients { - Some(info) => vec![info.recipient.0], + Some(info) => info.recipients.into_iter().map(|x| x.0).collect(), None => vec![], }; } diff --git a/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs b/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs index 1c64200b68a..4da0b6af9e0 100644 --- a/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs +++ b/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs @@ -576,9 +576,16 @@ impl BitcoinRegtestController { return None; } - let burned = if payload.commit_outs.len() > 0 { - let pox_transfers = payload.commit_outs.len() as u64; - let burn_remainder = (OUTPUTS_PER_COMMIT as u64) - pox_transfers; + if payload.all_outputs_burn() { + // this a whole burn op, so we just use one burn output + let burn_address_hash = Hash160([0u8; 20]); + let burn_output = BitcoinAddress::to_p2pkh_tx_out(&burn_address_hash, payload.burn_fee); + tx.output.push(burn_output) + } else { + if OUTPUTS_PER_COMMIT != payload.commit_outs.len() { + error!("Generated block commit with wrong OUTPUTS_PER_COMMIT"); + return None; + } let value_per_transfer = payload.burn_fee / (OUTPUTS_PER_COMMIT as u64); if value_per_transfer < 5500 { error!("Total burn fee not enough for number of outputs"); @@ -588,15 +595,6 @@ impl BitcoinRegtestController { tx.output .push(commit_to.to_bitcoin_tx_out(value_per_transfer)); } - value_per_transfer * burn_remainder - } else { - payload.burn_fee - }; - - if burned > 0 { - let burn_address_hash = Hash160([0u8; 20]); - let burn_output = BitcoinAddress::to_p2pkh_tx_out(&burn_address_hash, burned); - tx.output.push(burn_output); } self.finalize_tx(&mut tx, payload.burn_fee, utxos, signer)?; diff --git a/testnet/stacks-node/src/neon_node.rs b/testnet/stacks-node/src/neon_node.rs index 357ca8ad76c..04475f3e6e9 100644 --- a/testnet/stacks-node/src/neon_node.rs +++ b/testnet/stacks-node/src/neon_node.rs @@ -209,12 +209,7 @@ fn inner_generate_block_commit_op( ) -> BlockstackOperationType { let (parent_block_ptr, parent_vtxindex) = (parent_burnchain_height, parent_winning_vtx); - let commit_outs = if let Some(recipient_set) = recipients { - recipient_set.recipients.into_iter() - .map(|(recipient, _)| recipient).collect() - } else { - vec![] - }; + let commit_outs = RewardSetInfo::into_commit_outs(recipients, false); BlockstackOperationType::LeaderBlockCommit(LeaderBlockCommitOp { block_header_hash, From 9dc8b443e3337c159cc8d61f8b1f0237a218c006 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Thu, 15 Oct 2020 09:58:16 -0500 Subject: [PATCH 03/11] update integration tests for more outputs --- testnet/stacks-node/src/tests/neon_integrations.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/testnet/stacks-node/src/tests/neon_integrations.rs b/testnet/stacks-node/src/tests/neon_integrations.rs index fab30cf8a81..c3daddfcb6f 100644 --- a/testnet/stacks-node/src/tests/neon_integrations.rs +++ b/testnet/stacks-node/src/tests/neon_integrations.rs @@ -36,6 +36,7 @@ fn neon_integration_test_conf() -> (Config, StacksAddress) { conf.node.miner = true; conf.node.wait_time_for_microblocks = 500; + conf.burnchain.burn_fee_cap = 20000; conf.burnchain.mode = "neon".into(); conf.burnchain.username = Some("neon-tester".into()); @@ -994,7 +995,7 @@ fn pox_integration_test() { eprintln!("Got UTXOs: {}", utxos.len()); assert_eq!( utxos.len(), - 3, + 7, "Should have received three outputs during PoX reward cycle" ); @@ -1006,7 +1007,7 @@ fn pox_integration_test() { eprintln!("Got UTXOs: {}", utxos.len()); assert_eq!( utxos.len(), - 3, + 7, "Should have received three outputs during PoX reward cycle" ); From bb48228794767367e0360c0f5b58a5d484d1c2ae Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Thu, 15 Oct 2020 10:10:21 -0500 Subject: [PATCH 04/11] fix mut warning --- src/burnchains/burnchain.rs | 4 +- .../burn/operations/leader_block_commit.rs | 39 +++++++++++-------- .../src/tests/neon_integrations.rs | 2 +- 3 files changed, 27 insertions(+), 18 deletions(-) diff --git a/src/burnchains/burnchain.rs b/src/burnchains/burnchain.rs index 8c10539e5c5..31c7529eb92 100644 --- a/src/burnchains/burnchain.rs +++ b/src/burnchains/burnchain.rs @@ -570,7 +570,9 @@ impl Burnchain { } } } - x if x == Opcodes::LeaderBlockCommit as u8 || x == Opcodes::LeaderBlockCommitTransfer as u8 => { + x if x == Opcodes::LeaderBlockCommit as u8 + || x == Opcodes::LeaderBlockCommitTransfer as u8 => + { match LeaderBlockCommitOp::from_tx(block_header, burn_tx) { Ok(op) => Some(BlockstackOperationType::LeaderBlockCommit(op)), Err(e) => { diff --git a/src/chainstate/burn/operations/leader_block_commit.rs b/src/chainstate/burn/operations/leader_block_commit.rs index 6cc0fcc1f80..a551d6cc655 100644 --- a/src/chainstate/burn/operations/leader_block_commit.rs +++ b/src/chainstate/burn/operations/leader_block_commit.rs @@ -241,7 +241,9 @@ impl LeaderBlockCommitOp { let (commit_outs, burn_fee) = if !expects_pox_outputs { // this is a single output _burn_ commitment. if !outputs[0].address.is_burn() { - warn!("Invalid tx: should be a burn commitment, but address is not the burn address"); + warn!( + "Invalid tx: should be a burn commitment, but address is not the burn address" + ); return Err(op_error::InvalidInput); } // mock the commit_outs to burn outputs. @@ -311,9 +313,11 @@ impl LeaderBlockCommitOp { /// are all the outputs for this block commit burns? pub fn all_outputs_burn(&self) -> bool { - self.commit_outs.iter().fold(true, |previous_is_burn, output_addr| { - previous_is_burn && output_addr.is_burn() - }) + self.commit_outs + .iter() + .fold(true, |previous_is_burn, output_addr| { + previous_is_burn && output_addr.is_burn() + }) } } @@ -384,7 +388,8 @@ impl RewardSetInfo { outs } else { (0..OUTPUTS_PER_COMMIT) - .map(|_| StacksAddress::burn_address(mainnet)).collect() + .map(|_| StacksAddress::burn_address(mainnet)) + .collect() } } } @@ -427,8 +432,12 @@ impl LeaderBlockCommitOp { // first, handle a corner case: // all of the commitment outputs are _burns_ // _and_ the reward set chose two burn addresses as reward addresses. - let recipient_set_all_burns = reward_set_info.recipients.iter() - .fold(true, |prior_is_burn, (addr, _)| prior_is_burn && addr.is_burn()); + let recipient_set_all_burns = reward_set_info + .recipients + .iter() + .fold(true, |prior_is_burn, (addr, _)| { + prior_is_burn && addr.is_burn() + }); if self.all_outputs_burn() && recipient_set_all_burns { // pass @@ -777,16 +786,14 @@ mod tests { num_required: 0, in_type: BitcoinInputType::Standard, }], - outputs: vec![ - BitcoinTxOutput { - units: 13, - address: BitcoinAddress { - addrtype: BitcoinAddressType::PublicKeyHash, - network_id: BitcoinNetworkType::Mainnet, - bytes: Hash160([1; 20]), - }, + outputs: vec![BitcoinTxOutput { + units: 13, + address: BitcoinAddress { + addrtype: BitcoinAddressType::PublicKeyHash, + network_id: BitcoinNetworkType::Mainnet, + bytes: Hash160([1; 20]), }, - ], + }], }); // not enough PoX outputs diff --git a/testnet/stacks-node/src/tests/neon_integrations.rs b/testnet/stacks-node/src/tests/neon_integrations.rs index c3daddfcb6f..eb96e7f92d2 100644 --- a/testnet/stacks-node/src/tests/neon_integrations.rs +++ b/testnet/stacks-node/src/tests/neon_integrations.rs @@ -761,7 +761,7 @@ fn pox_integration_test() { let http_origin = format!("http://{}", &conf.node.rpc_bind); let mut burnchain_config = btc_regtest_controller.get_burnchain(); - let mut pox_constants = PoxConstants::new(10, 5, 4, 5, 15); + let pox_constants = PoxConstants::new(10, 5, 4, 5, 15); burnchain_config.pox_constants = pox_constants; btc_regtest_controller.bootstrap_chain(201); From c7db22929c74bee91f53f6ed185b54f5fce49ea5 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Thu, 15 Oct 2020 13:28:23 -0500 Subject: [PATCH 05/11] tests for burner reward set --- src/chainstate/burn/db/sortdb.rs | 4 + .../burn/operations/leader_block_commit.rs | 7 +- src/chainstate/coordinator/tests.rs | 223 ++++++++++++++++++ 3 files changed, 232 insertions(+), 2 deletions(-) diff --git a/src/chainstate/burn/db/sortdb.rs b/src/chainstate/burn/db/sortdb.rs index 0f23c875bf1..42ba48f5f3c 100644 --- a/src/chainstate/burn/db/sortdb.rs +++ b/src/chainstate/burn/db/sortdb.rs @@ -986,6 +986,10 @@ impl<'a> SortitionHandleTx<'a> { return Ok(None); } + 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() diff --git a/src/chainstate/burn/operations/leader_block_commit.rs b/src/chainstate/burn/operations/leader_block_commit.rs index a551d6cc655..c3e53c36330 100644 --- a/src/chainstate/burn/operations/leader_block_commit.rs +++ b/src/chainstate/burn/operations/leader_block_commit.rs @@ -439,8 +439,11 @@ impl LeaderBlockCommitOp { prior_is_burn && addr.is_burn() }); - if self.all_outputs_burn() && recipient_set_all_burns { - // pass + if recipient_set_all_burns { + if !self.all_outputs_burn() { + warn!("Invalid block commit: recipient set should be all burns"); + return Err(op_error::BlockCommitBadOutputs); + } } else { let expect_pox_descendant = if self.all_outputs_burn() { false diff --git a/src/chainstate/coordinator/tests.rs b/src/chainstate/coordinator/tests.rs index afeb5c4544c..98db3ad0f76 100644 --- a/src/chainstate/coordinator/tests.rs +++ b/src/chainstate/coordinator/tests.rs @@ -969,6 +969,229 @@ fn test_sortition_with_reward_set() { } } +#[test] +fn test_sortition_with_burner_reward_set() { + let path = "/tmp/stacks-blockchain-burner-reward-set"; + let _r = std::fs::remove_dir_all(path); + + let mut vrf_keys: Vec<_> = (0..150).map(|_| VRFPrivateKey::new()).collect(); + let mut committers: Vec<_> = (0..150).map(|_| StacksPrivateKey::new()).collect(); + + let reward_set_size = 10; + let mut reward_set: Vec<_> = (0..reward_set_size - 1) + .map(|_| StacksAddress::burn_address(false)) + .collect(); + reward_set.push(p2pkh_from(&StacksPrivateKey::new())); + + setup_states(&[path], &vrf_keys, &committers); + + let mut coord = make_reward_set_coordinator(path, reward_set); + + coord.handle_new_burnchain_block().unwrap(); + + let sort_db = get_sortition_db(path); + + let tip = SortitionDB::get_canonical_burn_chain_tip(sort_db.conn()).unwrap(); + assert_eq!(tip.block_height, 1); + assert_eq!(tip.sortition, false); + let (_, ops) = sort_db + .get_sortition_result(&tip.sortition_id) + .unwrap() + .unwrap(); + + // we should have all the VRF registrations accepted + assert_eq!(ops.accepted_ops.len(), vrf_keys.len()); + assert_eq!(ops.consumed_leader_keys.len(), 0); + + let mut started_first_reward_cycle = false; + // process sequential blocks, and their sortitions... + let mut stacks_blocks: Vec<(SortitionId, StacksBlock)> = vec![]; + let mut anchor_blocks = vec![]; + + // split up the vrf keys and committers so that we have some that will be mining "correctly" + // and some that will be producing bad outputs + + let BURNER_OFFSET = 50; + let mut vrf_key_burners = vrf_keys.split_off(50); + let mut miner_burners = committers.split_off(50); + + let WRONG_OUTS_OFFSET = 100; + let vrf_key_wrong_outs = vrf_key_burners.split_off(50); + let miner_wrong_outs = miner_burners.split_off(50); + + // track the reward set consumption + let mut reward_recipients = HashSet::new(); + for ix in 0..vrf_keys.len() { + let vrf_key = &vrf_keys[ix]; + let miner = &committers[ix]; + + let vrf_burner = &vrf_key_burners[ix]; + let miner_burner = &miner_burners[ix]; + + let vrf_wrong_out = &vrf_key_wrong_outs[ix]; + let miner_wrong_out = &miner_wrong_outs[ix]; + + let mut burnchain = get_burnchain_db(path); + let mut chainstate = get_chainstate(path); + + let parent = if ix == 0 { + BlockHeaderHash([0; 32]) + } else { + stacks_blocks[ix - 1].1.header.block_hash() + }; + + let burnchain_tip = burnchain.get_canonical_chain_tip().unwrap(); + let next_mock_header = BurnchainBlockHeader { + block_height: burnchain_tip.block_height + 1, + block_hash: BurnchainHeaderHash([0; 32]), + parent_block_hash: burnchain_tip.block_hash, + num_txs: 0, + timestamp: 1, + }; + + let reward_cycle_info = coord.get_reward_cycle_info(&next_mock_header).unwrap(); + if reward_cycle_info.is_some() { + // did we process a reward set last cycle? check if the + // recipient set size matches our expectation + if started_first_reward_cycle { + assert_eq!(reward_recipients.len(), 2); + } + // clear the reward recipients tracker, since those + // recipients are now eligible again in the new reward cycle + reward_recipients.clear(); + } + let next_block_recipients = get_rw_sortdb(path) + .test_get_next_block_recipients(reward_cycle_info.as_ref()) + .unwrap(); + if let Some(ref next_block_recipients) = next_block_recipients { + for (addr, _) in next_block_recipients.recipients.iter() { + if !addr.is_burn() { + assert!( + !reward_recipients.contains(addr), + "Reward set should not already contain address {}", + addr + ); + } + eprintln!("At iteration: {}, inserting address ... {}", ix, addr); + reward_recipients.insert(addr.clone()); + } + } + + let (good_op, block) = if ix == 0 { + make_genesis_block_with_recipients( + &sort_db, + &mut chainstate, + &parent, + miner, + 10000, + vrf_key, + ix as u32, + next_block_recipients.as_ref(), + ) + } else { + make_stacks_block_with_recipients( + &sort_db, + &mut chainstate, + &parent, + miner, + 10000, + vrf_key, + ix as u32, + next_block_recipients.as_ref(), + ) + }; + + let expected_winner = good_op.txid(); + let mut ops = vec![good_op]; + + if started_first_reward_cycle { + // sometime have the wrong _number_ of recipients, + // other times just have the wrong set of recipients + let recipients = if ix % 2 == 0 { + vec![(p2pkh_from(miner_wrong_out), 0)] + } else { + (0..OUTPUTS_PER_COMMIT) + .map(|ix| (p2pkh_from(&StacksPrivateKey::new()), ix as u16)) + .collect() + }; + let bad_block_recipipients = Some(RewardSetInfo { + anchor_block: BlockHeaderHash([0; 32]), + recipients, + }); + let (bad_outs_op, _) = make_stacks_block_with_recipients( + &sort_db, + &mut chainstate, + &parent, + miner_wrong_out, + 10000, + vrf_burner, + (ix + WRONG_OUTS_OFFSET) as u32, + bad_block_recipipients.as_ref(), + ); + ops.push(bad_outs_op); + } + + let burnchain_tip = burnchain.get_canonical_chain_tip().unwrap(); + produce_burn_block( + &mut burnchain, + &burnchain_tip.block_hash, + ops, + vec![].iter_mut(), + ); + // handle the sortition + coord.handle_new_burnchain_block().unwrap(); + + let b = get_burnchain(path); + let new_burnchain_tip = burnchain.get_canonical_chain_tip().unwrap(); + if b.is_reward_cycle_start(new_burnchain_tip.block_height) { + started_first_reward_cycle = true; + // store the anchor block for this sortition for later checking + let ic = sort_db.index_handle_at_tip(); + let bhh = ic.get_last_anchor_block_hash().unwrap().unwrap(); + anchor_blocks.push(bhh); + } + + let tip = SortitionDB::get_canonical_burn_chain_tip(sort_db.conn()).unwrap(); + assert_eq!(&tip.winning_block_txid, &expected_winner); + + // load the block into staging + let block_hash = block.header.block_hash(); + + assert_eq!(&tip.winning_stacks_block_hash, &block_hash); + stacks_blocks.push((tip.sortition_id.clone(), block.clone())); + + preprocess_block(&mut chainstate, &sort_db, &tip, block); + + // handle the stacks block + coord.handle_new_stacks_block().unwrap(); + } + + let stacks_tip = SortitionDB::get_canonical_stacks_chain_tip_hash(sort_db.conn()).unwrap(); + let mut chainstate = get_chainstate(path); + assert_eq!( + chainstate.with_read_only_clarity_tx( + &sort_db.index_conn(), + &StacksBlockId::new(&stacks_tip.0, &stacks_tip.1), + |conn| conn + .with_readonly_clarity_env( + PrincipalData::parse("SP3Q4A5WWZ80REGBN0ZXNE540ECJ9JZ4A765Q5K2Q").unwrap(), + LimitedCostTracker::new_max_limit(), + |env| env.eval_raw("block-height") + ) + .unwrap() + ), + Value::UInt(50) + ); + + { + let ic = sort_db.index_handle_at_tip(); + let pox_id = ic.get_pox_id().unwrap(); + assert_eq!(&pox_id.to_string(), + "11111111111", + "PoX ID should reflect the 10 reward cycles _with_ a known anchor block, plus the 'initial' known reward cycle at genesis"); + } +} + #[test] // This test should panic until the MARF stability issue // https://github.com/blockstack/stacks-blockchain/issues/1805 From cce4d5eee595a2c47574a97322cadd789ca03364 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Thu, 15 Oct 2020 17:00:28 -0500 Subject: [PATCH 06/11] require all block commits to use 2 output addresses (burn or not) --- sip/sip-007-stacking-consensus.md | 35 ++--- src/burnchains/burnchain.rs | 4 +- src/chainstate/burn/mod.rs | 1 - .../burn/operations/leader_block_commit.rs | 138 +++++++++--------- src/chainstate/stacks/address.rs | 19 ++- .../burnchains/bitcoin_regtest_controller.rs | 31 ++-- testnet/stacks-node/src/node.rs | 7 +- .../stacks-node/src/tests/bitcoin_regtest.rs | 58 ++------ 8 files changed, 135 insertions(+), 158 deletions(-) diff --git a/sip/sip-007-stacking-consensus.md b/sip/sip-007-stacking-consensus.md index cbc08bef450..08ab9df680a 100644 --- a/sip/sip-007-stacking-consensus.md +++ b/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 1000 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. diff --git a/src/burnchains/burnchain.rs b/src/burnchains/burnchain.rs index 31c7529eb92..24d132f9546 100644 --- a/src/burnchains/burnchain.rs +++ b/src/burnchains/burnchain.rs @@ -570,9 +570,7 @@ impl Burnchain { } } } - x if x == Opcodes::LeaderBlockCommit as u8 - || x == Opcodes::LeaderBlockCommitTransfer as u8 => - { + x if x == Opcodes::LeaderBlockCommit as u8 => { match LeaderBlockCommitOp::from_tx(block_header, burn_tx) { Ok(op) => Some(BlockstackOperationType::LeaderBlockCommit(op)), Err(e) => { diff --git a/src/chainstate/burn/mod.rs b/src/chainstate/burn/mod.rs index b2ad52ed783..25c6158b252 100644 --- a/src/chainstate/burn/mod.rs +++ b/src/chainstate/burn/mod.rs @@ -109,7 +109,6 @@ impl_byte_array_newtype!(SortitionHash, u8, 32); #[repr(u8)] pub enum Opcodes { LeaderBlockCommit = '[' as u8, - LeaderBlockCommitTransfer = ']' as u8, LeaderKeyRegister = '^' as u8, UserBurnSupport = '_' as u8, } diff --git a/src/chainstate/burn/operations/leader_block_commit.rs b/src/chainstate/burn/operations/leader_block_commit.rs index c3e53c36330..0d149a04268 100644 --- a/src/chainstate/burn/operations/leader_block_commit.rs +++ b/src/chainstate/burn/operations/leader_block_commit.rs @@ -193,11 +193,7 @@ impl LeaderBlockCommitOp { return Err(op_error::InvalidInput); } - let expects_pox_outputs = if tx.opcode() == Opcodes::LeaderBlockCommit as u8 { - false - } else if tx.opcode() == Opcodes::LeaderBlockCommitTransfer as u8 { - true - } else { + if tx.opcode() != Opcodes::LeaderBlockCommit as u8 { warn!("Invalid tx: invalid opcode {}", tx.opcode()); return Err(op_error::InvalidInput); }; @@ -238,53 +234,36 @@ impl LeaderBlockCommitOp { return Err(op_error::ParseError); } - let (commit_outs, burn_fee) = if !expects_pox_outputs { - // this is a single output _burn_ commitment. - if !outputs[0].address.is_burn() { - warn!( - "Invalid tx: should be a burn commitment, but address is not the burn address" - ); - return Err(op_error::InvalidInput); + let mut commit_outs = vec![]; + let mut pox_fee = None; + for (ix, output) in outputs.into_iter().enumerate() { + // only look at the first OUTPUTS_PER_COMMIT outputs + if ix >= OUTPUTS_PER_COMMIT { + break; } - // mock the commit_outs to burn outputs. - let commit_outs = (0..OUTPUTS_PER_COMMIT) - .map(|_| outputs[0].address.clone()) - .collect(); - let burn_fee = outputs[0].amount; - (commit_outs, burn_fee) - } else { - let mut commit_outs = vec![]; - let mut pox_fee = None; - for (ix, output) in outputs.into_iter().enumerate() { - // only look at the first OUTPUTS_PER_COMMIT outputs - if ix >= OUTPUTS_PER_COMMIT { - break; + // all pox outputs must have the same fee + if let Some(pox_fee) = pox_fee { + if output.amount != pox_fee { + warn!("Invalid commit tx: different output amounts for different PoX reward addresses"); + return Err(op_error::ParseError); } - // all pox outputs must have the same fee - if let Some(pox_fee) = pox_fee { - if output.amount != pox_fee { - warn!("Invalid commit tx: different output amounts for different PoX reward addresses"); - return Err(op_error::ParseError); - } - } else { - pox_fee.replace(output.amount); - } - commit_outs.push(output.address); + } else { + pox_fee.replace(output.amount); } + commit_outs.push(output.address); + } - if commit_outs.len() != OUTPUTS_PER_COMMIT { - warn!("Invalid commit tx: {} commit addresses, but {} PoX addresses should be committed to", commit_outs.len(), OUTPUTS_PER_COMMIT); - return Err(op_error::InvalidInput); - } + if commit_outs.len() != OUTPUTS_PER_COMMIT { + warn!("Invalid commit tx: {} commit addresses, but {} PoX addresses should be committed to", commit_outs.len(), OUTPUTS_PER_COMMIT); + return Err(op_error::InvalidInput); + } - // compute the total amount transfered/burned, and check that the burn amount - // is expected given the amount transfered. - let burn_fee = pox_fee - .expect("A 0-len output should have already errored") - .checked_mul(OUTPUTS_PER_COMMIT as u64) // total commitment is the pox_amount * outputs - .ok_or_else(|| op_error::ParseError)?; - (commit_outs, burn_fee) - }; + // compute the total amount transfered/burned, and check that the burn amount + // is expected given the amount transfered. + let burn_fee = pox_fee + .expect("A 0-len output should have already errored") + .checked_mul(OUTPUTS_PER_COMMIT as u64) // total commitment is the pox_amount * outputs + .ok_or_else(|| op_error::ParseError)?; if burn_fee == 0 { warn!("Invalid commit tx: burn/transfer amount is 0"); @@ -331,11 +310,7 @@ impl StacksMessageCodec for LeaderBlockCommitOp { block txoff block txoff */ fn consensus_serialize(&self, fd: &mut W) -> Result<(), net_error> { - if self.all_outputs_burn() { - write_next(fd, &(Opcodes::LeaderBlockCommit as u8))? - } else { - write_next(fd, &(Opcodes::LeaderBlockCommitTransfer as u8))? - }; + write_next(fd, &(Opcodes::LeaderBlockCommit as u8))?; write_next(fd, &self.block_header_hash)?; fd.write_all(&self.new_seed.as_bytes()[..]) .map_err(net_error::WriteError)?; @@ -432,6 +407,7 @@ impl LeaderBlockCommitOp { // first, handle a corner case: // all of the commitment outputs are _burns_ // _and_ the reward set chose two burn addresses as reward addresses. + // then, don't need to do a pox descendant check. let recipient_set_all_burns = reward_set_info .recipients .iter() @@ -456,10 +432,22 @@ impl LeaderBlockCommitOp { ); return Err(op_error::BlockCommitBadOutputs); } - for (expected_commit, _) in reward_set_info.recipients.iter() { - if !self.commit_outs.contains(expected_commit) { - warn!("Invalid block commit: expected to send funds to {}, but that address is not in the committed output set", - expected_commit); + + // sort check_recipients and commit_outs so that we can perform an + // iterative equality check + let mut check_recipients: Vec<_> = reward_set_info + .recipients + .iter() + .map(|(addr, _)| addr.clone()) + .collect(); + check_recipients.sort(); + let mut commit_outs = self.commit_outs.clone(); + commit_outs.sort(); + for (expected_commit, found_commit) in commit_outs.iter().zip(check_recipients) + { + if expected_commit != &found_commit { + warn!("Invalid block commit: committed output {} does not match expected {}", + found_commit, expected_commit); return Err(op_error::BlockCommitBadOutputs); } } @@ -599,7 +587,7 @@ mod tests { use address::AddressHashMode; use deps::bitcoin::blockdata::transaction::Transaction; - use deps::bitcoin::network::serialize::deserialize; + use deps::bitcoin::network::serialize::{deserialize, serialize_hex}; use chainstate::burn::{BlockHeaderHash, ConsensusHash, VRFSeed}; @@ -638,7 +626,7 @@ mod tests { let tx = BurnchainTransaction::Bitcoin(BitcoinTransaction { txid: Txid([0; 32]), vtxindex: 0, - opcode: Opcodes::LeaderBlockCommitTransfer as u8, + opcode: Opcodes::LeaderBlockCommit as u8, data: vec![1; 80], inputs: vec![BitcoinTxInput { keys: vec![], @@ -683,7 +671,7 @@ mod tests { let tx = BurnchainTransaction::Bitcoin(BitcoinTransaction { txid: Txid([0; 32]), vtxindex: 0, - opcode: Opcodes::LeaderBlockCommitTransfer as u8, + opcode: Opcodes::LeaderBlockCommit as u8, data: vec![1; 80], inputs: vec![BitcoinTxInput { keys: vec![], @@ -721,7 +709,7 @@ mod tests { let tx = BurnchainTransaction::Bitcoin(BitcoinTransaction { txid: Txid([0; 32]), vtxindex: 0, - opcode: Opcodes::LeaderBlockCommitTransfer as u8, + opcode: Opcodes::LeaderBlockCommit as u8, data: vec![1; 80], inputs: vec![BitcoinTxInput { keys: vec![], @@ -782,7 +770,7 @@ mod tests { let tx = BurnchainTransaction::Bitcoin(BitcoinTransaction { txid: Txid([0; 32]), vtxindex: 0, - opcode: Opcodes::LeaderBlockCommitTransfer as u8, + opcode: Opcodes::LeaderBlockCommit as u8, data: vec![1; 80], inputs: vec![BitcoinTxInput { keys: vec![], @@ -810,7 +798,7 @@ mod tests { let tx = BurnchainTransaction::Bitcoin(BitcoinTransaction { txid: Txid([0; 32]), vtxindex: 0, - opcode: Opcodes::LeaderBlockCommitTransfer as u8, + opcode: Opcodes::LeaderBlockCommit as u8, data: vec![1; 80], inputs: vec![BitcoinTxInput { keys: vec![], @@ -848,7 +836,7 @@ mod tests { let tx = BurnchainTransaction::Bitcoin(BitcoinTransaction { txid: Txid([0; 32]), vtxindex: 0, - opcode: Opcodes::LeaderBlockCommitTransfer as u8, + opcode: Opcodes::LeaderBlockCommit as u8, data: vec![1; 80], inputs: vec![BitcoinTxInput { keys: vec![], @@ -920,7 +908,7 @@ mod tests { let tx_fixtures = vec![ OpFixture { // valid - txstr: "01000000011111111111111111111111111111111111111111111111111111111111111111000000006b483045022100eba8c0a57c1eb71cdfba0874de63cf37b3aace1e56dcbd61701548194a79af34022041dd191256f3f8a45562e5d60956bb871421ba69db605716250554b23b08277b012102d8015134d9db8178ac93acbc43170a2f20febba5087a5b0437058765ad5133d000000000030000000000000000536a4c5069645b222222222222222222222222222222222222222222222222222222222222222233333333333333333333333333333333333333333333333333333333333333334041424350516061626370718039300000000000001976a914000000000000000000000000000000000000000088aca05b0000000000001976a9140be3e286a15ea85882761618e366586b5574100d88ac00000000".to_string(), + txstr: "01000000011111111111111111111111111111111111111111111111111111111111111111000000006b483045022100eba8c0a57c1eb71cdfba0874de63cf37b3aace1e56dcbd61701548194a79af34022041dd191256f3f8a45562e5d60956bb871421ba69db605716250554b23b08277b012102d8015134d9db8178ac93acbc43170a2f20febba5087a5b0437058765ad5133d000000000040000000000000000536a4c5069645b222222222222222222222222222222222222222222222222222222222222222233333333333333333333333333333333333333333333333333333333333333334041424350516061626370718039300000000000001976a914000000000000000000000000000000000000000088ac39300000000000001976a914000000000000000000000000000000000000000088aca05b0000000000001976a9140be3e286a15ea85882761618e366586b5574100d88ac00000000".into(), opstr: "69645b2222222222222222222222222222222222222222222222222222222222222222333333333333333333333333333333333333333333333333333333333333333340414243505160616263707180".to_string(), result: Some(LeaderBlockCommitOp { block_header_hash: BlockHeaderHash::from_bytes(&hex_bytes("2222222222222222222222222222222222222222222222222222222222222222").unwrap()).unwrap(), @@ -936,7 +924,7 @@ mod tests { StacksAddress { version: 26, bytes: Hash160::empty() } ], - burn_fee: 12345, + burn_fee: 24690, input: BurnchainSigner { public_keys: vec![ StacksPublicKey::from_hex("02d8015134d9db8178ac93acbc43170a2f20febba5087a5b0437058765ad5133d0").unwrap(), @@ -945,7 +933,7 @@ mod tests { hash_mode: AddressHashMode::SerializeP2PKH }, - txid: Txid::from_bytes_be(&hex_bytes("3c07a0a93360bc85047bbaadd49e30c8af770f73a37e10fec400174d2e5f27cf").unwrap()).unwrap(), + txid: Txid::from_hex("b08d5d1bc81049a3957e9ff9a5882463811735fd5de985e6d894e9b3d5c49501").unwrap(), vtxindex: vtxindex, block_height: block_height, burn_header_hash: burn_header_hash, @@ -973,8 +961,20 @@ mod tests { let parser = BitcoinBlockParser::new(BitcoinNetworkType::Testnet, BLOCKSTACK_MAGIC_MAINNET); + let mut is_first = false; for tx_fixture in tx_fixtures { - let tx = make_tx(&tx_fixture.txstr).unwrap(); + let mut tx = make_tx(&tx_fixture.txstr).unwrap(); + if is_first { + eprintln!("TX outputs: {}", tx.output.len()); + tx.output.insert( + 2, + StacksAddress::burn_address(false).to_bitcoin_tx_out(12345), + ); + is_first = false; + eprintln!("Updated txstr = {}", serialize_hex(&tx).unwrap()); + assert!(false); + } + let header = match tx_fixture.result { Some(ref op) => BurnchainBlockHeader { block_height: op.block_height, @@ -1012,11 +1012,11 @@ mod tests { } (Err(_e), None) => {} (Ok(_parsed_tx), None) => { - test_debug!("Parsed a tx when we should not have"); + eprintln!("Parsed a tx when we should not have"); assert!(false); } (Err(_e), Some(_result)) => { - test_debug!("Did not parse a tx when we should have"); + eprintln!("Did not parse a tx when we should have"); assert!(false); } }; diff --git a/src/chainstate/stacks/address.rs b/src/chainstate/stacks/address.rs index 56da69f3c39..35453c34a72 100644 --- a/src/chainstate/stacks/address.rs +++ b/src/chainstate/stacks/address.rs @@ -38,10 +38,10 @@ use burnchains::PublicKey; use address::c32::c32_address_decode; -use deps::bitcoin::blockdata::script::Builder as BtcScriptBuilder; - use deps::bitcoin::blockdata::opcodes::All as BtcOp; +use deps::bitcoin::blockdata::script::Builder as BtcScriptBuilder; use deps::bitcoin::blockdata::transaction::TxOut; +use std::cmp::{Ord, Ordering}; use burnchains::bitcoin::address::{ address_type_to_version_byte, to_b52_version_byte, to_c32_version_byte, @@ -79,6 +79,21 @@ impl From for StacksAddress { } } +impl PartialOrd for StacksAddress { + fn partial_cmp(&self, other: &StacksAddress) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for StacksAddress { + fn cmp(&self, other: &StacksAddress) -> Ordering { + match self.version.cmp(&other.version) { + Ordering::Equal => self.bytes.cmp(&other.bytes), + inequality => inequality, + } + } +} + impl StacksAddress { pub fn new(version: u8, hash: Hash160) -> StacksAddress { StacksAddress { diff --git a/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs b/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs index 4da0b6af9e0..19ca309fbd3 100644 --- a/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs +++ b/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs @@ -576,25 +576,18 @@ impl BitcoinRegtestController { return None; } - if payload.all_outputs_burn() { - // this a whole burn op, so we just use one burn output - let burn_address_hash = Hash160([0u8; 20]); - let burn_output = BitcoinAddress::to_p2pkh_tx_out(&burn_address_hash, payload.burn_fee); - tx.output.push(burn_output) - } else { - if OUTPUTS_PER_COMMIT != payload.commit_outs.len() { - error!("Generated block commit with wrong OUTPUTS_PER_COMMIT"); - return None; - } - let value_per_transfer = payload.burn_fee / (OUTPUTS_PER_COMMIT as u64); - if value_per_transfer < 5500 { - error!("Total burn fee not enough for number of outputs"); - return None; - } - for commit_to in payload.commit_outs.iter() { - tx.output - .push(commit_to.to_bitcoin_tx_out(value_per_transfer)); - } + if OUTPUTS_PER_COMMIT != payload.commit_outs.len() { + error!("Generated block commit with wrong OUTPUTS_PER_COMMIT"); + return None; + } + let value_per_transfer = payload.burn_fee / (OUTPUTS_PER_COMMIT as u64); + if value_per_transfer < 5500 { + error!("Total burn fee not enough for number of outputs"); + return None; + } + for commit_to in payload.commit_outs.iter() { + tx.output + .push(commit_to.to_bitcoin_tx_out(value_per_transfer)); } self.finalize_tx(&mut tx, payload.burn_fee, utxos, signer)?; diff --git a/testnet/stacks-node/src/node.rs b/testnet/stacks-node/src/node.rs index 8a00ca667e4..e2438ae2cf9 100644 --- a/testnet/stacks-node/src/node.rs +++ b/testnet/stacks-node/src/node.rs @@ -9,7 +9,8 @@ use std::{thread, thread::JoinHandle, time}; use stacks::burnchains::{Burnchain, BurnchainHeaderHash, Txid}; use stacks::chainstate::burn::db::sortdb::SortitionDB; use stacks::chainstate::burn::operations::{ - BlockstackOperationType, LeaderBlockCommitOp, LeaderKeyRegisterOp, + leader_block_commit::RewardSetInfo, BlockstackOperationType, LeaderBlockCommitOp, + LeaderKeyRegisterOp, }; use stacks::chainstate::burn::{BlockHeaderHash, ConsensusHash, VRFSeed}; use stacks::chainstate::stacks::db::{ClarityTx, StacksChainState, StacksHeaderInfo}; @@ -692,6 +693,8 @@ impl Node { ), }; + let commit_outs = RewardSetInfo::into_commit_outs(None, false); + BlockstackOperationType::LeaderBlockCommit(LeaderBlockCommitOp { block_header_hash, burn_fee, @@ -704,7 +707,7 @@ impl Node { parent_vtxindex, vtxindex: 0, txid: Txid([0u8; 32]), - commit_outs: vec![], + commit_outs, block_height: 0, burn_header_hash: BurnchainHeaderHash([0u8; 32]), }) diff --git a/testnet/stacks-node/src/tests/bitcoin_regtest.rs b/testnet/stacks-node/src/tests/bitcoin_regtest.rs index 9050fae70e2..11f59dda66e 100644 --- a/testnet/stacks-node/src/tests/bitcoin_regtest.rs +++ b/testnet/stacks-node/src/tests/bitcoin_regtest.rs @@ -103,6 +103,8 @@ impl Drop for BitcoinCoreController { } } +const BITCOIND_INT_TEST_COMMITS: u64 = 11000; + #[test] #[ignore] fn bitcoind_integration_test() { @@ -112,7 +114,7 @@ fn bitcoind_integration_test() { let mut conf = super::new_test_conf(); conf.burnchain.commit_anchor_block_within = 2000; - conf.burnchain.burn_fee_cap = 5000; + conf.burnchain.burn_fee_cap = BITCOIND_INT_TEST_COMMITS; conf.burnchain.mode = "helium".to_string(); conf.burnchain.peer_host = "127.0.0.1".to_string(); conf.burnchain.rpc_port = 18443; @@ -136,14 +138,14 @@ fn bitcoind_integration_test() { // In this serie of tests, the callback is fired post-burnchain-sync, pre-stacks-sync run_loop.callbacks.on_new_burn_chain_state(|round, burnchain_tip, chain_tip| { + let block = &burnchain_tip.block_snapshot; + let expected_total_burn = BITCOIND_INT_TEST_COMMITS * (round as u64 + 1); + assert_eq!(block.total_burn, expected_total_burn); + assert_eq!(block.sortition, true); + assert_eq!(block.num_sortitions, round as u64 + 1); + assert_eq!(block.block_height, round as u64 + 203); match round { 0 => { - let block = &burnchain_tip.block_snapshot; - assert!(block.block_height == 203); - assert!(block.total_burn == 5000); - assert!(block.num_sortitions == 1); - assert!(block.sortition == true); - let state_transition = &burnchain_tip.state_transition; assert!(state_transition.accepted_ops.len() == 1); assert!(state_transition.consumed_leader_keys.len() == 1); @@ -157,19 +159,13 @@ fn bitcoind_integration_test() { assert!(burnchain_tip.state_transition.consumed_leader_keys[0].public_key.to_hex() == "ff80684f3a5912662adbae013fb6521f10fb6ba7e4e60ccba8671b765cef8a34"); assert!(op.parent_block_ptr == 0); assert!(op.parent_vtxindex == 0); - assert!(op.burn_fee == 5000); + assert_eq!(op.burn_fee, BITCOIND_INT_TEST_COMMITS); } _ => assert!(false) } } }, 1 => { - let block = &burnchain_tip.block_snapshot; - assert!(block.block_height == 204); - assert!(block.total_burn == 10000); - assert!(block.num_sortitions == 2); - assert!(block.sortition == true); - let state_transition = &burnchain_tip.state_transition; assert!(state_transition.accepted_ops.len() == 1); assert!(state_transition.consumed_leader_keys.len() == 1); @@ -182,7 +178,7 @@ fn bitcoind_integration_test() { LeaderBlockCommit(op) => { assert!(burnchain_tip.state_transition.consumed_leader_keys[0].public_key.to_hex() == "ff80684f3a5912662adbae013fb6521f10fb6ba7e4e60ccba8671b765cef8a34"); assert!(op.parent_block_ptr == 203); - assert!(op.burn_fee == 5000); + assert_eq!(op.burn_fee, BITCOIND_INT_TEST_COMMITS); } _ => assert!(false) } @@ -191,12 +187,6 @@ fn bitcoind_integration_test() { assert!(burnchain_tip.block_snapshot.parent_burn_header_hash == chain_tip.metadata.burn_header_hash); }, 2 => { - let block = &burnchain_tip.block_snapshot; - assert!(block.block_height == 205); - assert!(block.total_burn == 15000); - assert!(block.num_sortitions == 3); - assert!(block.sortition == true); - let state_transition = &burnchain_tip.state_transition; assert!(state_transition.accepted_ops.len() == 1); assert!(state_transition.consumed_leader_keys.len() == 1); @@ -209,7 +199,7 @@ fn bitcoind_integration_test() { LeaderBlockCommit(op) => { assert!(burnchain_tip.state_transition.consumed_leader_keys[0].public_key.to_hex() == "ff80684f3a5912662adbae013fb6521f10fb6ba7e4e60ccba8671b765cef8a34"); assert!(op.parent_block_ptr == 204); - assert!(op.burn_fee == 5000); + assert_eq!(op.burn_fee, BITCOIND_INT_TEST_COMMITS); } _ => assert!(false) } @@ -218,12 +208,6 @@ fn bitcoind_integration_test() { assert!(burnchain_tip.block_snapshot.parent_burn_header_hash == chain_tip.metadata.burn_header_hash); }, 3 => { - let block = &burnchain_tip.block_snapshot; - assert!(block.block_height == 206); - assert!(block.total_burn == 20000); - assert!(block.num_sortitions == 4); - assert!(block.sortition == true); - let state_transition = &burnchain_tip.state_transition; assert!(state_transition.accepted_ops.len() == 1); assert!(state_transition.consumed_leader_keys.len() == 1); @@ -236,7 +220,7 @@ fn bitcoind_integration_test() { LeaderBlockCommit(op) => { assert!(burnchain_tip.state_transition.consumed_leader_keys[0].public_key.to_hex() == "ff80684f3a5912662adbae013fb6521f10fb6ba7e4e60ccba8671b765cef8a34"); assert!(op.parent_block_ptr == 205); - assert!(op.burn_fee == 5000); + assert_eq!(op.burn_fee, BITCOIND_INT_TEST_COMMITS); } _ => assert!(false) } @@ -245,12 +229,6 @@ fn bitcoind_integration_test() { assert!(burnchain_tip.block_snapshot.parent_burn_header_hash == chain_tip.metadata.burn_header_hash); }, 4 => { - let block = &burnchain_tip.block_snapshot; - assert!(block.block_height == 207); - assert!(block.total_burn == 25000); - assert!(block.num_sortitions == 5); - assert!(block.sortition == true); - let state_transition = &burnchain_tip.state_transition; assert!(state_transition.accepted_ops.len() == 1); assert!(state_transition.consumed_leader_keys.len() == 1); @@ -263,7 +241,7 @@ fn bitcoind_integration_test() { LeaderBlockCommit(op) => { assert!(burnchain_tip.state_transition.consumed_leader_keys[0].public_key.to_hex() == "ff80684f3a5912662adbae013fb6521f10fb6ba7e4e60ccba8671b765cef8a34"); assert!(op.parent_block_ptr == 206); - assert!(op.burn_fee == 5000); + assert_eq!(op.burn_fee, BITCOIND_INT_TEST_COMMITS); } _ => assert!(false) } @@ -272,12 +250,6 @@ fn bitcoind_integration_test() { assert!(burnchain_tip.block_snapshot.parent_burn_header_hash == chain_tip.metadata.burn_header_hash); }, 5 => { - let block = &burnchain_tip.block_snapshot; - assert!(block.block_height == 208); - assert!(block.total_burn == 30000); - assert!(block.num_sortitions == 6); - assert!(block.sortition == true); - let state_transition = &burnchain_tip.state_transition; assert!(state_transition.accepted_ops.len() == 1); assert!(state_transition.consumed_leader_keys.len() == 1); @@ -290,7 +262,7 @@ fn bitcoind_integration_test() { LeaderBlockCommit(op) => { assert!(burnchain_tip.state_transition.consumed_leader_keys[0].public_key.to_hex() == "ff80684f3a5912662adbae013fb6521f10fb6ba7e4e60ccba8671b765cef8a34"); assert!(op.parent_block_ptr == 207); - assert!(op.burn_fee == 5000); + assert_eq!(op.burn_fee, BITCOIND_INT_TEST_COMMITS); } _ => assert!(false) } From 00ec45dc01fe761a65f7684bb33521e024141192 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Mon, 19 Oct 2020 10:16:47 -0500 Subject: [PATCH 07/11] use uneven number of reward addrs in test --- src/chainstate/coordinator/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/chainstate/coordinator/tests.rs b/src/chainstate/coordinator/tests.rs index 98db3ad0f76..756d76f2eb2 100644 --- a/src/chainstate/coordinator/tests.rs +++ b/src/chainstate/coordinator/tests.rs @@ -977,7 +977,7 @@ fn test_sortition_with_burner_reward_set() { let mut vrf_keys: Vec<_> = (0..150).map(|_| VRFPrivateKey::new()).collect(); let mut committers: Vec<_> = (0..150).map(|_| StacksPrivateKey::new()).collect(); - let reward_set_size = 10; + let reward_set_size = 9; let mut reward_set: Vec<_> = (0..reward_set_size - 1) .map(|_| StacksAddress::burn_address(false)) .collect(); From a9bfb3bd603a27ec59ba93b84230678d19cba7e1 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Tue, 20 Oct 2020 08:14:09 -0500 Subject: [PATCH 08/11] fix typo --- sip/sip-007-stacking-consensus.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sip/sip-007-stacking-consensus.md b/sip/sip-007-stacking-consensus.md index 08ab9df680a..c914368183e 100644 --- a/sip/sip-007-stacking-consensus.md +++ b/sip/sip-007-stacking-consensus.md @@ -218,7 +218,7 @@ 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 4000 Bitcoin -addresses (2 addresses in a 1000 burn block cycle). To ensure that +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 From d942f93f86f42425a23084ce344f41ae230eb540 Mon Sep 17 00:00:00 2001 From: Jude Nelson Date: Tue, 20 Oct 2020 23:41:53 -0400 Subject: [PATCH 09/11] fix 1984 -- expect_buff(x) should work on buffers smaller than x, and should fill in missing bytes with 0 --- src/vm/types/mod.rs | 63 +++++++++++++++++++++++++++++++++------------ 1 file changed, 47 insertions(+), 16 deletions(-) diff --git a/src/vm/types/mod.rs b/src/vm/types/mod.rs index f448e793585..f55feb8d27a 100644 --- a/src/vm/types/mod.rs +++ b/src/vm/types/mod.rs @@ -726,7 +726,8 @@ impl Value { if let Value::UInt(inner) = self { inner } else { - panic!(format!("Value '{:?}' is not a u128", &self)); + error!("Value '{:?}' is not a u128", &self); + panic!(); } } @@ -734,23 +735,29 @@ impl Value { if let Value::Int(inner) = self { inner } else { - panic!(format!("Value '{:?}' is not an i128", &self)); + error!("Value '{:?}' is not an i128", &self); + panic!(); } } pub fn expect_buff(self, sz: usize) -> Vec { - if let Value::Sequence(SequenceData::Buffer(buffdata)) = self { - if buffdata.data.len() == sz { + if let Value::Sequence(SequenceData::Buffer(mut buffdata)) = self { + if buffdata.data.len() <= sz { + for _ in buffdata.data.len()..sz { + buffdata.data.push(0u8) + } buffdata.data } else { - panic!(format!( + error!( "Value buffer has len {}, expected {}", buffdata.data.len(), sz - )); + ); + panic!(); } } else { - panic!(format!("Value '{:?}' is not a buff", &self)); + error!("Value '{:?}' is not a buff", &self); + panic!(); } } @@ -758,7 +765,8 @@ impl Value { if let Value::Bool(b) = self { b } else { - panic!(format!("Value '{:?}' is not a bool", &self)); + error!("Value '{:?}' is not a bool", &self); + panic!(); } } @@ -766,7 +774,8 @@ impl Value { if let Value::Tuple(data) = self { data } else { - panic!(format!("Value '{:?}' is not a tuple", &self)); + error!("Value '{:?}' is not a tuple", &self); + panic!(); } } @@ -777,7 +786,8 @@ impl Value { None => None, } } else { - panic!(format!("Value '{:?}' is not an optional", &self)); + error!("Value '{:?}' is not an optional", &self); + panic!(); } } @@ -785,7 +795,8 @@ impl Value { if let Value::Principal(p) = self { p } else { - panic!(format!("Value '{:?}' is not a principal", &self)); + error!("Value '{:?}' is not a principal", &self); + panic!(); } } @@ -797,7 +808,8 @@ impl Value { Err(*res_data.data) } } else { - panic!("FATAL: not a response"); + error!("Value '{:?}' is not a response", &self); + panic!(); } } @@ -806,10 +818,12 @@ impl Value { if res_data.committed { *res_data.data } else { - panic!("FATAL: not a (ok ..)"); + error!("Value is not a (ok ..)"); + panic!(); } } else { - panic!("FATAL: not a response"); + error!("Value '{:?}' is not a response", &self); + panic!(); } } @@ -818,10 +832,12 @@ impl Value { if !res_data.committed { *res_data.data } else { - panic!("FATAL: not a (err ..)"); + error!("Value is not a (err ..)"); + panic!(); } } else { - panic!("FATAL: not a response"); + error!("Value '{:?}' is not a response", &self); + panic!(); } } } @@ -1269,4 +1285,19 @@ mod test { "(tuple (a 2))" ); } + + #[test] + fn expect_buff() { + let buff = Value::Sequence(SequenceData::Buffer(BuffData { data: vec![1,2,3,4,5] })); + assert_eq!(buff.clone().expect_buff(5), vec![1,2,3,4,5]); + assert_eq!(buff.clone().expect_buff(6), vec![1,2,3,4,5,0]); + assert_eq!(buff.clone().expect_buff(10), vec![1,2,3,4,5,0,0,0,0,0]); + } + + #[test] + #[should_panic] + fn expect_buff_too_small() { + let buff = Value::Sequence(SequenceData::Buffer(BuffData { data: vec![1,2,3,4,5] })); + let _ = buff.expect_buff(4); + } } From f19ea19127595007b3f7c2fe8427b1515e39ea09 Mon Sep 17 00:00:00 2001 From: Jude Nelson Date: Tue, 20 Oct 2020 23:44:15 -0400 Subject: [PATCH 10/11] cargo fmt --- src/vm/types/mod.rs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/vm/types/mod.rs b/src/vm/types/mod.rs index f55feb8d27a..de1a8b7d696 100644 --- a/src/vm/types/mod.rs +++ b/src/vm/types/mod.rs @@ -1288,16 +1288,23 @@ mod test { #[test] fn expect_buff() { - let buff = Value::Sequence(SequenceData::Buffer(BuffData { data: vec![1,2,3,4,5] })); - assert_eq!(buff.clone().expect_buff(5), vec![1,2,3,4,5]); - assert_eq!(buff.clone().expect_buff(6), vec![1,2,3,4,5,0]); - assert_eq!(buff.clone().expect_buff(10), vec![1,2,3,4,5,0,0,0,0,0]); + let buff = Value::Sequence(SequenceData::Buffer(BuffData { + data: vec![1, 2, 3, 4, 5], + })); + assert_eq!(buff.clone().expect_buff(5), vec![1, 2, 3, 4, 5]); + assert_eq!(buff.clone().expect_buff(6), vec![1, 2, 3, 4, 5, 0]); + assert_eq!( + buff.clone().expect_buff(10), + vec![1, 2, 3, 4, 5, 0, 0, 0, 0, 0] + ); } - + #[test] #[should_panic] fn expect_buff_too_small() { - let buff = Value::Sequence(SequenceData::Buffer(BuffData { data: vec![1,2,3,4,5] })); + let buff = Value::Sequence(SequenceData::Buffer(BuffData { + data: vec![1, 2, 3, 4, 5], + })); let _ = buff.expect_buff(4); } } From 80e6871e61c1ec33b1bea89745702da6b0684f03 Mon Sep 17 00:00:00 2001 From: Jude Nelson Date: Wed, 21 Oct 2020 16:04:06 -0400 Subject: [PATCH 11/11] add expect_buff_padded() which will fill in any missing bytes with a pad value --- src/chainstate/stacks/boot/mod.rs | 4 ++-- src/vm/types/mod.rs | 26 +++++++++++++++++++------- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/chainstate/stacks/boot/mod.rs b/src/chainstate/stacks/boot/mod.rs index 2732840cf71..7bb3e9db213 100644 --- a/src/chainstate/stacks/boot/mod.rs +++ b/src/chainstate/stacks/boot/mod.rs @@ -95,12 +95,12 @@ fn tuple_to_pox_addr(tuple_data: TupleData) -> (AddressHashMode, Hash160) { .expect("FATAL: no 'hashbytes' field in pox-addr") .to_owned(); - let version_u8 = version_value.expect_buff(1)[0]; + let version_u8 = version_value.expect_buff_padded(1, 0)[0]; let version: AddressHashMode = version_u8 .try_into() .expect("FATAL: PoX version is not a supported version byte"); - let hashbytes_vec = hashbytes_value.expect_buff(20); + let hashbytes_vec = hashbytes_value.expect_buff_padded(20, 0); let mut hashbytes_20 = [0u8; 20]; hashbytes_20.copy_from_slice(&hashbytes_vec[0..20]); diff --git a/src/vm/types/mod.rs b/src/vm/types/mod.rs index de1a8b7d696..b74c9ae92d8 100644 --- a/src/vm/types/mod.rs +++ b/src/vm/types/mod.rs @@ -741,11 +741,8 @@ impl Value { } pub fn expect_buff(self, sz: usize) -> Vec { - if let Value::Sequence(SequenceData::Buffer(mut buffdata)) = self { + if let Value::Sequence(SequenceData::Buffer(buffdata)) = self { if buffdata.data.len() <= sz { - for _ in buffdata.data.len()..sz { - buffdata.data.push(0u8) - } buffdata.data } else { error!( @@ -761,6 +758,16 @@ impl Value { } } + pub fn expect_buff_padded(self, sz: usize, pad: u8) -> Vec { + let mut data = self.expect_buff(sz); + if sz > data.len() { + for _ in data.len()..sz { + data.push(pad) + } + } + data + } + pub fn expect_bool(self) -> bool { if let Value::Bool(b) = self { b @@ -1292,10 +1299,15 @@ mod test { data: vec![1, 2, 3, 4, 5], })); assert_eq!(buff.clone().expect_buff(5), vec![1, 2, 3, 4, 5]); - assert_eq!(buff.clone().expect_buff(6), vec![1, 2, 3, 4, 5, 0]); + assert_eq!(buff.clone().expect_buff(6), vec![1, 2, 3, 4, 5]); + assert_eq!( + buff.clone().expect_buff_padded(6, 0), + vec![1, 2, 3, 4, 5, 0] + ); + assert_eq!(buff.clone().expect_buff(10), vec![1, 2, 3, 4, 5]); assert_eq!( - buff.clone().expect_buff(10), - vec![1, 2, 3, 4, 5, 0, 0, 0, 0, 0] + buff.clone().expect_buff_padded(10, 1), + vec![1, 2, 3, 4, 5, 1, 1, 1, 1, 1] ); }