feat: backport #4061 to master and make all miner config settings loa…

…dable at runtime

.cargo/config

@@ -1,5 +1,6 @@
 [alias]
 stacks-node = "run --package stacks-node --"
+fmt-stacks = "fmt -- --config group_imports=StdExternalCrate,imports_granularity=Module"
 
 # Needed by perf to generate flamegraphs.
 #[target.x86_64-unknown-linux-gnu]

.github/actions/bitcoin-int-tests/Dockerfile.rustfmt

@@ -4,9 +4,10 @@ WORKDIR /src
 
 COPY ./rust-toolchain .
 COPY ./Cargo.toml .
+COPY ./.cargo .
 
 RUN rustup component add rustfmt
 
 COPY . .
 
-RUN cargo fmt --all -- --check
+RUN cargo fmt-stacks --check

CONTRIBUTING.md

@@ -91,6 +91,24 @@ should reference the issue in the commit message. For example:
 fix: incorporate unlocks in mempool admitter, #3623
 ```
 
+## Recommended developer setup
+### Recommended githooks
+
+It is helpful to set up the pre-commit git hook set up, so that Rust formatting issues are caught before
+you push your code. Follow these instruction to set it up:
+
+1. Rename `.git/hooks/pre-commit.sample` to `.git/hooks/pre-commit`
+2. Change the content of `.git/hooks/pre-commit` to be the following
+```sh
+#!/bin/sh
+git diff --name-only --staged | grep '\.rs$' | xargs -P 8 -I {} rustfmt {} --edition 2021 --check --config group_imports=StdExternalCrate,imports_granularity=Module || (
+  echo 'rustfmt failed: run "cargo fmt-stacks"';
+  exit 1
+)
+```
+3. Make it executable by running `chmod +x .git/hooks/pre-commit`
+   That's it! Now your pre-commit hook should be configured on your local machine.
+
 # Creating and Reviewing PRs
 
 This section describes some best practices on how to create and review PRs in this context.  The target audience is people who have commit access to this repository (reviewers), and people who open PRs (submitters).  This is a living document -- developers can and should document their own additional guidelines here.
@@ -366,19 +384,20 @@ A test should be marked `#[ignore]` if:
 
 ## Formatting
 
-This repository uses the default rustfmt formatting style. PRs will be checked against `rustfmt` and will _fail_ if not
-properly formatted.
+PRs will be checked against `rustfmt` and will _fail_ if not properly formatted.
+Unfortunately, some config options that we require cannot currently be set in `.rustfmt` files, so arguments must be passed via the command line.
+Therefore, we handle `rustfmt` configuration using a Cargo alias: `cargo fmt-stacks`
 
 You can check the formatting locally via:
 
 ```bash
-cargo fmt --all -- --check --config group_imports=StdExternalCrate
+cargo fmt-stacks --check
 ```
 
 You can automatically reformat your commit via:
 
 ```bash
-cargo fmt --all -- --config group_imports=StdExternalCrate
+cargo fmt-stacks
 ```
 
 ## Comments

testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs

@@ -162,6 +162,64 @@ pub fn make_bitcoin_indexer(config: &Config) -> BitcoinIndexer {
     burnchain_indexer
 }
 
+pub fn get_satoshis_per_byte(config: &Config) -> u64 {
+    match config.get_burnchain_config() {
+        Ok(s) => s.satoshis_per_byte,
+        Err(_) => {
+            info!("No config found. Using previous configuration.");
+            config.burnchain.satoshis_per_byte
+        }
+    }
+}
+
+pub fn get_rbf_fee_increment(config: &Config) -> u64 {
+    match config.get_burnchain_config() {
+        Ok(s) => s.rbf_fee_increment,
+        Err(_) => {
+            info!("No config found. Using previous configuration.");
+            config.burnchain.rbf_fee_increment
+        }
+    }
+}
+
+pub fn get_max_rbf(config: &Config) -> u64 {
+    match config.get_burnchain_config() {
+        Ok(s) => s.max_rbf,
+        Err(_) => {
+            info!("No config found. Using previous configuration.");
+            config.burnchain.max_rbf
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::config::DEFAULT_SATS_PER_VB;
+
+    use super::*;
+    use std::env::temp_dir;
+    use std::fs::File;
+    use std::io::Write;
+
+    #[test]
+    fn test_get_satoshis_per_byte() {
+        let dir = temp_dir();
+        let file_path = dir.as_path().join("config.toml");
+
+        let mut config = Config::default();
+
+        let satoshis_per_byte = get_satoshis_per_byte(&config);
+        assert_eq!(satoshis_per_byte, DEFAULT_SATS_PER_VB);
+
+        let mut file = File::create(&file_path).unwrap();
+        writeln!(file, "[burnchain]").unwrap();
+        writeln!(file, "satoshis_per_byte = 51").unwrap();
+        config.config_path = Some(file_path.to_str().unwrap().to_string());
+
+        assert_eq!(get_satoshis_per_byte(&config), 51);
+    }
+}
+
 impl LeaderBlockCommitFees {
     pub fn fees_from_previous_tx(
         &self,
@@ -171,7 +229,7 @@ impl LeaderBlockCommitFees {
         let mut fees = LeaderBlockCommitFees::estimated_fees_from_payload(payload, config);
         fees.spent_in_attempts = cmp::max(1, self.spent_in_attempts);
         fees.final_size = self.final_size;
-        fees.fee_rate = self.fee_rate + config.burnchain.rbf_fee_increment;
+        fees.fee_rate = self.fee_rate + get_rbf_fee_increment(&config);
         fees.is_rbf_enabled = true;
         fees
     }
@@ -190,7 +248,7 @@ impl LeaderBlockCommitFees {
         let value_per_transfer = payload.burn_fee / number_of_transfers;
         let sortition_fee = value_per_transfer * number_of_transfers;
         let spent_in_attempts = 0;
-        let fee_rate = config.burnchain.satoshis_per_byte;
+        let fee_rate = get_satoshis_per_byte(&config);
         let default_tx_size = config.burnchain.block_commit_tx_estimated_size;
 
         LeaderBlockCommitFees {
@@ -802,8 +860,9 @@ impl BitcoinRegtestController {
     ) -> Option<Transaction> {
         let public_key = signer.get_public_key();
 
-        let btc_miner_fee = self.config.burnchain.leader_key_tx_estimated_size
-            * self.config.burnchain.satoshis_per_byte;
+        // reload the config to find satoshis_per_byte changes
+        let satoshis_per_byte = get_satoshis_per_byte(&self.config);
+        let btc_miner_fee = self.config.burnchain.leader_key_tx_estimated_size * satoshis_per_byte;
         let budget_for_outputs = DUST_UTXO_LIMIT;
         let total_required = btc_miner_fee + budget_for_outputs;
 
@@ -831,7 +890,7 @@ impl BitcoinRegtestController {
 
         tx.output = vec![consensus_output];
 
-        let fee_rate = self.config.burnchain.satoshis_per_byte;
+        let fee_rate = get_satoshis_per_byte(&self.config);
 
         self.finalize_tx(
             epoch_id,
@@ -925,7 +984,6 @@ impl BitcoinRegtestController {
     ) -> Option<Transaction> {
         let public_key = signer.get_public_key();
         let max_tx_size = 230;
-
         let (mut tx, mut utxos) = if let Some(utxo) = utxo_to_use {
             (
                 Transaction {
@@ -943,7 +1001,7 @@ impl BitcoinRegtestController {
             self.prepare_tx(
                 epoch_id,
                 &public_key,
-                DUST_UTXO_LIMIT + max_tx_size * self.config.burnchain.satoshis_per_byte,
+                DUST_UTXO_LIMIT + max_tx_size * get_satoshis_per_byte(&self.config),
                 None,
                 None,
                 0,
@@ -977,7 +1035,7 @@ impl BitcoinRegtestController {
             DUST_UTXO_LIMIT,
             0,
             max_tx_size,
-            self.config.burnchain.satoshis_per_byte,
+            get_satoshis_per_byte(&self.config),
             &mut utxos,
             signer,
         )?;
@@ -1026,7 +1084,7 @@ impl BitcoinRegtestController {
             self.prepare_tx(
                 epoch_id,
                 &public_key,
-                DUST_UTXO_LIMIT + max_tx_size * self.config.burnchain.satoshis_per_byte,
+                DUST_UTXO_LIMIT + max_tx_size * get_satoshis_per_byte(&self.config),
                 None,
                 None,
                 0,
@@ -1060,7 +1118,7 @@ impl BitcoinRegtestController {
             DUST_UTXO_LIMIT,
             0,
             max_tx_size,
-            self.config.burnchain.satoshis_per_byte,
+            get_satoshis_per_byte(&self.config),
             &mut utxos,
             signer,
         )?;
@@ -1095,7 +1153,7 @@ impl BitcoinRegtestController {
         let public_key = signer.get_public_key();
         let max_tx_size = 280;
 
-        let output_amt = DUST_UTXO_LIMIT + max_tx_size * self.config.burnchain.satoshis_per_byte;
+        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)?;
 
@@ -1124,7 +1182,7 @@ impl BitcoinRegtestController {
             output_amt,
             0,
             max_tx_size,
-            self.config.burnchain.satoshis_per_byte,
+            get_satoshis_per_byte(&self.config),
             &mut utxos,
             signer,
         )?;
@@ -1322,11 +1380,11 @@ impl BitcoinRegtestController {
 
         // Stop as soon as the fee_rate is ${self.config.burnchain.max_rbf} percent higher, stop RBF
         if ongoing_op.fees.fee_rate
-            > (self.config.burnchain.satoshis_per_byte * self.config.burnchain.max_rbf / 100)
+            > (get_satoshis_per_byte(&self.config) * get_max_rbf(&self.config) / 100)
         {
             warn!(
                 "RBF'd block commits reached {}% satoshi per byte fee rate, not resubmitting",
-                self.config.burnchain.max_rbf
+                get_max_rbf(&self.config)
             );
             self.ongoing_block_commit = Some(ongoing_op);
             return None;

testnet/stacks-node/src/config.rs

@@ -39,7 +39,7 @@ use stacks::util::secp256k1::Secp256k1PublicKey;
 use stacks::vm::costs::ExecutionCost;
 use stacks::vm::types::{AssetIdentifier, PrincipalData, QualifiedContractIdentifier};
 
-const DEFAULT_SATS_PER_VB: u64 = 50;
+pub const DEFAULT_SATS_PER_VB: u64 = 50;
 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;
@@ -48,6 +48,7 @@ const INV_REWARD_CYCLES_TESTNET: u64 = 6;
 
 #[derive(Clone, Deserialize, Default, Debug)]
 pub struct ConfigFile {
+    pub __path: Option<String>,
     pub burnchain: Option<BurnchainConfigFile>,
     pub node: Option<NodeConfigFile>,
     pub ustx_balance: Option<Vec<InitialBalanceFile>>,
@@ -178,7 +179,9 @@ mod tests {
 impl ConfigFile {
     pub fn from_path(path: &str) -> Result<ConfigFile, String> {
         let content = fs::read_to_string(path).map_err(|e| format!("Invalid path: {}", &e))?;
-        Self::from_str(&content)
+        let mut f = Self::from_str(&content)?;
+        f.__path = Some(path.to_string());
+        Ok(f)
     }
 
     pub fn from_str(content: &str) -> Result<ConfigFile, String> {
@@ -353,6 +356,7 @@ impl ConfigFile {
 
 #[derive(Clone, Debug)]
 pub struct Config {
+    pub config_path: Option<String>,
     pub burnchain: BurnchainConfig,
     pub node: NodeConfig,
     pub initial_balances: Vec<InitialBalance>,
@@ -394,6 +398,28 @@ lazy_static! {
 }
 
 impl Config {
+    /// get the up-to-date burnchain from the config
+    pub fn get_burnchain_config(&self) -> Result<BurnchainConfig, String> {
+        if let Some(path) = &self.config_path {
+            let config_file = ConfigFile::from_path(path.as_str())?;
+            let config = Config::from_config_file(config_file)?;
+            Ok(config.burnchain)
+        } else {
+            Ok(self.burnchain.clone())
+        }
+    }
+
+    /// get the up-to-date miner options from the config
+    pub fn get_miner_config(&self) -> Result<MinerConfig, String> {
+        if let Some(path) = &self.config_path {
+            let config_file = ConfigFile::from_path(path.as_str())?;
+            let config = Config::from_config_file(config_file)?;
+            Ok(config.miner)
+        } else {
+            Ok(self.miner.clone())
+        }
+    }
+
     /// Apply any test settings to this burnchain config struct
     fn apply_test_settings(&self, burnchain: &mut Burnchain) {
         if self.burnchain.get_bitcoin_network().1 == BitcoinNetworkType::Mainnet {
@@ -1148,6 +1174,7 @@ impl Config {
         };
 
         Ok(Config {
+            config_path: config_file.__path,
             node,
             burnchain,
             initial_balances,
@@ -1263,30 +1290,36 @@ impl Config {
         microblocks: bool,
         miner_status: Arc<Mutex<MinerStatus>>,
     ) -> BlockBuilderSettings {
+        let miner_config = if let Ok(miner_config) = self.get_miner_config() {
+            miner_config
+        } else {
+            self.miner.clone()
+        };
+
         BlockBuilderSettings {
             max_miner_time_ms: if microblocks {
-                self.miner.microblock_attempt_time_ms
+                miner_config.microblock_attempt_time_ms
             } else if attempt <= 1 {
                 // first attempt to mine a block -- do so right away
-                self.miner.first_attempt_time_ms
+                miner_config.first_attempt_time_ms
             } else {
                 // second or later attempt to mine a block -- give it some time
-                self.miner.subsequent_attempt_time_ms
+                miner_config.subsequent_attempt_time_ms
             },
             mempool_settings: MemPoolWalkSettings {
-                min_tx_fee: self.miner.min_tx_fee,
+                min_tx_fee: miner_config.min_tx_fee,
                 max_walk_time_ms: if microblocks {
-                    self.miner.microblock_attempt_time_ms
+                    miner_config.microblock_attempt_time_ms
                 } else if attempt <= 1 {
                     // first attempt to mine a block -- do so right away
-                    self.miner.first_attempt_time_ms
+                    miner_config.first_attempt_time_ms
                 } else {
                     // second or later attempt to mine a block -- give it some time
-                    self.miner.subsequent_attempt_time_ms
+                    miner_config.subsequent_attempt_time_ms
                 },
-                consider_no_estimate_tx_prob: self.miner.probability_pick_no_estimate_tx,
-                nonce_cache_size: self.miner.nonce_cache_size,
-                candidate_retry_cache_size: self.miner.candidate_retry_cache_size,
+                consider_no_estimate_tx_prob: miner_config.probability_pick_no_estimate_tx,
+                nonce_cache_size: miner_config.nonce_cache_size,
+                candidate_retry_cache_size: miner_config.candidate_retry_cache_size,
             },
             miner_status,
         }
@@ -1308,6 +1341,7 @@ impl std::default::Default for Config {
         let estimation = FeeEstimationConfig::default();
 
         Config {
+            config_path: None,
             burnchain,
             node,
             initial_balances: vec![],