From 030fa6434b55b49b1e4bfa31470647ce6ab3a095 Mon Sep 17 00:00:00 2001 From: janniks Date: Fri, 26 Jul 2024 17:09:04 +0200 Subject: [PATCH 1/5] fix: ensure minimum non-dust amount as change output on regtest --- .../burnchains/bitcoin_regtest_controller.rs | 23 +++++++++++++------ 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs b/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs index 30f088a96f..0ef63b6dc0 100644 --- a/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs +++ b/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs @@ -1715,6 +1715,7 @@ impl BitcoinRegtestController { spent_in_outputs + min_tx_size * fee_rate + estimated_rbf, &mut utxos_cloned, signer, + true, ); let serialized_tx = SerializedTx::new(tx_cloned); cmp::max(min_tx_size, serialized_tx.bytes.len() as u64) @@ -1731,6 +1732,7 @@ impl BitcoinRegtestController { spent_in_outputs + tx_size * fee_rate + rbf_fee, utxos_set, signer, + true, ); signer.dispose(); Some(()) @@ -1744,38 +1746,45 @@ impl BitcoinRegtestController { &mut self, epoch_id: StacksEpochId, tx: &mut Transaction, - total_to_spend: u64, + tx_cost: u64, utxos_set: &mut UTXOSet, signer: &mut BurnchainOpSigner, + force_change_output: bool, ) -> bool { let mut public_key = signer.get_public_key(); - let mut total_consumed = 0; + + let total_target = if force_change_output { + tx_cost + DUST_UTXO_LIMIT + } else { + tx_cost + }; // select UTXOs until we have enough to cover the cost + let mut total_consumed = 0; let mut available_utxos = vec![]; available_utxos.append(&mut utxos_set.utxos); for utxo in available_utxos.into_iter() { total_consumed += utxo.amount; utxos_set.utxos.push(utxo); - if total_consumed >= total_to_spend { + if total_consumed >= total_target { break; } } - if total_consumed < total_to_spend { + if total_consumed < total_target { warn!( "Consumed total {} is less than intended spend: {}", - total_consumed, total_to_spend + total_consumed, total_target ); return false; } // Append the change output - let value = total_consumed - total_to_spend; + let value = total_consumed - tx_cost; debug!( "Payments value: {:?}, total_consumed: {:?}, total_spent: {:?}", - value, total_consumed, total_to_spend + value, total_consumed, total_target ); if value >= DUST_UTXO_LIMIT { let change_output = if self.config.miner.segwit && epoch_id >= StacksEpochId::Epoch21 { From df7e9e165718e103766d0c0d3515375356bc04cd Mon Sep 17 00:00:00 2001 From: janniks Date: Tue, 30 Jul 2024 17:35:11 +0200 Subject: [PATCH 2/5] fix: update max_tx_sizes for regtest ops --- .../burnchains/bitcoin_regtest_controller.rs | 20 ++++++++++++------- testnet/stacks-node/src/config.rs | 14 +++++++++---- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs b/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs index d4aa528fbe..8e1fafcdd3 100644 --- a/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs +++ b/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs @@ -57,7 +57,11 @@ use stacks_common::util::sleep_ms; use super::super::operations::BurnchainOpSigner; use super::super::Config; use super::{BurnchainController, BurnchainTip, Error as BurnchainControllerError}; -use crate::config::BurnchainConfig; +use crate::config::{ + BurnchainConfig, OP_TX_BLOCK_COMMIT_ESTIM_SIZE, OP_TX_DELEGATE_STACKS_ESTIM_SIZE, + OP_TX_PRE_STACKS_ESTIM_SIZE, OP_TX_STACK_STX_ESTIM_SIZE, OP_TX_TRANSFER_STACKS_ESTIM_SIZE, + OP_TX_VOTE_AGG_ESTIM_SIZE, +}; /// The number of bitcoin blocks that can have /// passed since the UTXO cache was last refreshed before @@ -950,7 +954,7 @@ impl BitcoinRegtestController { utxo_to_use: Option, ) -> Option { let public_key = signer.get_public_key(); - let max_tx_size = 230; + let max_tx_size = OP_TX_TRANSFER_STACKS_ESTIM_SIZE; let (mut tx, mut utxos) = if let Some(utxo) = utxo_to_use { ( Transaction { @@ -1032,7 +1036,7 @@ impl BitcoinRegtestController { utxo_to_use: Option, ) -> Option { let public_key = signer.get_public_key(); - let max_tx_size = 230; + let max_tx_size = OP_TX_DELEGATE_STACKS_ESTIM_SIZE; let (mut tx, mut utxos) = if let Some(utxo) = utxo_to_use { ( @@ -1110,7 +1114,7 @@ impl BitcoinRegtestController { utxo_to_use: Option, ) -> Option { let public_key = signer.get_public_key(); - let max_tx_size = 230; + let max_tx_size = OP_TX_VOTE_AGG_ESTIM_SIZE; let (mut tx, mut utxos) = if let Some(utxo) = utxo_to_use { ( @@ -1204,9 +1208,11 @@ impl BitcoinRegtestController { signer: &mut BurnchainOpSigner, ) -> Option { let public_key = signer.get_public_key(); - let max_tx_size = 280; + let max_tx_size = OP_TX_PRE_STACKS_ESTIM_SIZE; + + let max_tx_size_any_op = 380; + let output_amt = DUST_UTXO_LIMIT + max_tx_size_any_op * get_satoshis_per_byte(&self.config); - let output_amt = DUST_UTXO_LIMIT + max_tx_size * get_satoshis_per_byte(&self.config); let (mut tx, mut utxos) = self.prepare_tx(epoch_id, &public_key, output_amt, None, None, 0)?; @@ -1271,7 +1277,7 @@ impl BitcoinRegtestController { utxo_to_use: Option, ) -> Option { let public_key = signer.get_public_key(); - let max_tx_size = 250; + let max_tx_size = OP_TX_STACK_STX_ESTIM_SIZE; let (mut tx, mut utxos) = if let Some(utxo) = utxo_to_use { ( diff --git a/testnet/stacks-node/src/config.rs b/testnet/stacks-node/src/config.rs index f52c5d6ba9..2bfef69e90 100644 --- a/testnet/stacks-node/src/config.rs +++ b/testnet/stacks-node/src/config.rs @@ -49,10 +49,16 @@ use stacks_common::util::secp256k1::{Secp256k1PrivateKey, Secp256k1PublicKey}; use crate::chain_data::MinerStats; pub const DEFAULT_SATS_PER_VB: u64 = 50; +pub const OP_TX_LEADER_KEY_ESTIM_SIZE: u64 = 290; +pub const OP_TX_BLOCK_COMMIT_ESTIM_SIZE: u64 = 350; +pub const OP_TX_TRANSFER_STACKS_ESTIM_SIZE: u64 = 230; +pub const OP_TX_DELEGATE_STACKS_ESTIM_SIZE: u64 = 230; +pub const OP_TX_VOTE_AGG_ESTIM_SIZE: u64 = 230; +pub const OP_TX_PRE_STACKS_ESTIM_SIZE: u64 = 280; +pub const OP_TX_STACK_STX_ESTIM_SIZE: u64 = 250; + const DEFAULT_MAX_RBF_RATE: u64 = 150; // 1.5x const DEFAULT_RBF_FEE_RATE_INCREMENT: u64 = 5; -const LEADER_KEY_TX_ESTIM_SIZE: u64 = 290; -const BLOCK_COMMIT_TX_ESTIM_SIZE: u64 = 350; const INV_REWARD_CYCLES_TESTNET: u64 = 6; #[derive(Clone, Deserialize, Default, Debug)] @@ -1427,8 +1433,8 @@ impl BurnchainConfig { poll_time_secs: 10, // TODO: this is a testnet specific value. satoshis_per_byte: DEFAULT_SATS_PER_VB, max_rbf: DEFAULT_MAX_RBF_RATE, - leader_key_tx_estimated_size: LEADER_KEY_TX_ESTIM_SIZE, - block_commit_tx_estimated_size: BLOCK_COMMIT_TX_ESTIM_SIZE, + leader_key_tx_estimated_size: OP_TX_LEADER_KEY_ESTIM_SIZE, + block_commit_tx_estimated_size: OP_TX_BLOCK_COMMIT_ESTIM_SIZE, rbf_fee_increment: DEFAULT_RBF_FEE_RATE_INCREMENT, first_burn_block_height: None, first_burn_block_timestamp: None, From eb80166aa8e62af6f0f24f8395f24e0e8730723c Mon Sep 17 00:00:00 2001 From: janniks Date: Tue, 30 Jul 2024 17:47:24 +0200 Subject: [PATCH 3/5] refactor: update estimated tx size consts --- .../burnchains/bitcoin_regtest_controller.rs | 8 ++++---- testnet/stacks-node/src/config.rs | 18 ++++++++++++++---- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs b/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs index 8e1fafcdd3..7de1d09dce 100644 --- a/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs +++ b/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs @@ -58,9 +58,9 @@ use super::super::operations::BurnchainOpSigner; use super::super::Config; use super::{BurnchainController, BurnchainTip, Error as BurnchainControllerError}; use crate::config::{ - BurnchainConfig, OP_TX_BLOCK_COMMIT_ESTIM_SIZE, OP_TX_DELEGATE_STACKS_ESTIM_SIZE, - OP_TX_PRE_STACKS_ESTIM_SIZE, OP_TX_STACK_STX_ESTIM_SIZE, OP_TX_TRANSFER_STACKS_ESTIM_SIZE, - OP_TX_VOTE_AGG_ESTIM_SIZE, + BurnchainConfig, OP_TX_ANY_ESTIM_SIZE, OP_TX_BLOCK_COMMIT_ESTIM_SIZE, + OP_TX_DELEGATE_STACKS_ESTIM_SIZE, OP_TX_PRE_STACKS_ESTIM_SIZE, OP_TX_STACK_STX_ESTIM_SIZE, + OP_TX_TRANSFER_STACKS_ESTIM_SIZE, OP_TX_VOTE_AGG_ESTIM_SIZE, }; /// The number of bitcoin blocks that can have @@ -1210,7 +1210,7 @@ impl BitcoinRegtestController { let public_key = signer.get_public_key(); let max_tx_size = OP_TX_PRE_STACKS_ESTIM_SIZE; - let max_tx_size_any_op = 380; + let max_tx_size_any_op = OP_TX_ANY_ESTIM_SIZE; let output_amt = DUST_UTXO_LIMIT + max_tx_size_any_op * get_satoshis_per_byte(&self.config); let (mut tx, mut utxos) = diff --git a/testnet/stacks-node/src/config.rs b/testnet/stacks-node/src/config.rs index 2bfef69e90..4eef0bbdd0 100644 --- a/testnet/stacks-node/src/config.rs +++ b/testnet/stacks-node/src/config.rs @@ -49,13 +49,23 @@ use stacks_common::util::secp256k1::{Secp256k1PrivateKey, Secp256k1PublicKey}; use crate::chain_data::MinerStats; pub const DEFAULT_SATS_PER_VB: u64 = 50; -pub const OP_TX_LEADER_KEY_ESTIM_SIZE: u64 = 290; -pub const OP_TX_BLOCK_COMMIT_ESTIM_SIZE: u64 = 350; -pub const OP_TX_TRANSFER_STACKS_ESTIM_SIZE: u64 = 230; +pub const OP_TX_BLOCK_COMMIT_ESTIM_SIZE: u64 = 380; pub const OP_TX_DELEGATE_STACKS_ESTIM_SIZE: u64 = 230; -pub const OP_TX_VOTE_AGG_ESTIM_SIZE: u64 = 230; +pub const OP_TX_LEADER_KEY_ESTIM_SIZE: u64 = 290; pub const OP_TX_PRE_STACKS_ESTIM_SIZE: u64 = 280; pub const OP_TX_STACK_STX_ESTIM_SIZE: u64 = 250; +pub const OP_TX_TRANSFER_STACKS_ESTIM_SIZE: u64 = 230; +pub const OP_TX_VOTE_AGG_ESTIM_SIZE: u64 = 230; + +pub const OP_TX_ANY_ESTIM_SIZE: u64 = fmax!( + OP_TX_BLOCK_COMMIT_ESTIM_SIZE, + OP_TX_DELEGATE_STACKS_ESTIM_SIZE, + OP_TX_LEADER_KEY_ESTIM_SIZE, + OP_TX_PRE_STACKS_ESTIM_SIZE, + OP_TX_STACK_STX_ESTIM_SIZE, + OP_TX_TRANSFER_STACKS_ESTIM_SIZE, + OP_TX_VOTE_AGG_ESTIM_SIZE +); const DEFAULT_MAX_RBF_RATE: u64 = 150; // 1.5x const DEFAULT_RBF_FEE_RATE_INCREMENT: u64 = 5; From 0468efd7d63ac7e72af95e4ff88196bf6c4b2692 Mon Sep 17 00:00:00 2001 From: janniks Date: Tue, 30 Jul 2024 17:51:43 +0200 Subject: [PATCH 4/5] chore: remove unused import --- .../src/burnchains/bitcoin_regtest_controller.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs b/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs index 7de1d09dce..b09a71e5cf 100644 --- a/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs +++ b/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs @@ -58,9 +58,9 @@ use super::super::operations::BurnchainOpSigner; use super::super::Config; use super::{BurnchainController, BurnchainTip, Error as BurnchainControllerError}; use crate::config::{ - BurnchainConfig, OP_TX_ANY_ESTIM_SIZE, OP_TX_BLOCK_COMMIT_ESTIM_SIZE, - OP_TX_DELEGATE_STACKS_ESTIM_SIZE, OP_TX_PRE_STACKS_ESTIM_SIZE, OP_TX_STACK_STX_ESTIM_SIZE, - OP_TX_TRANSFER_STACKS_ESTIM_SIZE, OP_TX_VOTE_AGG_ESTIM_SIZE, + BurnchainConfig, OP_TX_ANY_ESTIM_SIZE, OP_TX_DELEGATE_STACKS_ESTIM_SIZE, + OP_TX_PRE_STACKS_ESTIM_SIZE, OP_TX_STACK_STX_ESTIM_SIZE, OP_TX_TRANSFER_STACKS_ESTIM_SIZE, + OP_TX_VOTE_AGG_ESTIM_SIZE, }; /// The number of bitcoin blocks that can have From 9e9aa77752703acd330479b83a2daf662778541d Mon Sep 17 00:00:00 2001 From: janniks Date: Wed, 31 Jul 2024 00:13:29 +0200 Subject: [PATCH 5/5] fix: only force change output for block commits --- .../src/burnchains/bitcoin_regtest_controller.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs b/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs index b09a71e5cf..39ef40490b 100644 --- a/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs +++ b/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs @@ -872,6 +872,7 @@ impl BitcoinRegtestController { fee_rate, &mut utxos, signer, + false, )?; increment_btc_ops_sent_counter(); @@ -1009,6 +1010,7 @@ impl BitcoinRegtestController { get_satoshis_per_byte(&self.config), &mut utxos, signer, + false, )?; increment_btc_ops_sent_counter(); @@ -1092,6 +1094,7 @@ impl BitcoinRegtestController { get_satoshis_per_byte(&self.config), &mut utxos, signer, + false, )?; increment_btc_ops_sent_counter(); @@ -1166,6 +1169,7 @@ impl BitcoinRegtestController { get_satoshis_per_byte(&self.config), &mut utxos, signer, + false, )?; increment_btc_ops_sent_counter(); @@ -1244,6 +1248,7 @@ impl BitcoinRegtestController { get_satoshis_per_byte(&self.config), &mut utxos, signer, + false, )?; increment_btc_ops_sent_counter(); @@ -1331,6 +1336,7 @@ impl BitcoinRegtestController { get_satoshis_per_byte(&self.config), &mut utxos, signer, + false, )?; increment_btc_ops_sent_counter(); @@ -1421,6 +1427,7 @@ impl BitcoinRegtestController { fee_rate, &mut utxos, signer, + true, // only block commit op requires change output to exist )?; let serialized_tx = SerializedTx::new(tx.clone()); @@ -1691,6 +1698,7 @@ impl BitcoinRegtestController { fee_rate: u64, utxos_set: &mut UTXOSet, signer: &mut BurnchainOpSigner, + force_change_output: bool, ) -> Option<()> { // spend UTXOs in order by confirmations. Spend the least-confirmed UTXO first, and in the // event of a tie, spend the smallest-value UTXO first. @@ -1721,7 +1729,7 @@ impl BitcoinRegtestController { spent_in_outputs + min_tx_size * fee_rate + estimated_rbf, &mut utxos_cloned, signer, - true, + force_change_output, ); let serialized_tx = SerializedTx::new(tx_cloned); cmp::max(min_tx_size, serialized_tx.bytes.len() as u64) @@ -1738,7 +1746,7 @@ impl BitcoinRegtestController { spent_in_outputs + tx_size * fee_rate + rbf_fee, utxos_set, signer, - true, + force_change_output, ); signer.dispose(); Some(())