Merge pull request #5420 from stacks-network/feat/delay-block-commit

feat: add `block_commit_delay_ms` config option

Author: jcnelson

.github/workflows/bitcoin-tests.yml

@@ -123,6 +123,7 @@ jobs:
           - tests::signer::v0::signing_in_0th_tenure_of_reward_cycle
           - tests::signer::v0::continue_after_tenure_extend
           - tests::signer::v0::multiple_miners_with_custom_chain_id
+          - tests::signer::v0::block_commit_delay
           - tests::nakamoto_integrations::burn_ops_integration_test
           - tests::nakamoto_integrations::check_block_heights
           - tests::nakamoto_integrations::clarity_burn_state

CHANGELOG.md

@@ -10,6 +10,7 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
 ### Changed
 - Add index for StacksBlockId to nakamoto block headers table (improves node performance)
 - Remove the panic for reporting DB deadlocks (just error and continue waiting)
+- Add `block_commit_delay_ms` to the config file to control the time to wait after seeing a new burn block, before submitting a block commit, to allow time for the first Nakamoto block of the new tenure to be mined, allowing this miner to avoid the need to RBF the block commit.
 
 ## [3.0.0.0.1]
 

docs/mining.md

@@ -19,14 +19,26 @@ nakamoto_attempt_time_ms = 20000
 [burnchain]
 # Maximum amount (in sats) of "burn commitment" to broadcast for the next block's leader election
 burn_fee_cap = 20000
-# Amount (in sats) per byte - Used to calculate the transaction fees
-satoshis_per_byte = 25
-# Amount of sats to add when RBF'ing bitcoin tx  (default: 5)
+# Amount in sats per byte used to calculate the Bitcoin transaction fee (default: 50)
+satoshis_per_byte = 50
+# Amount of sats per byte to add when RBF'ing a Bitcoin tx  (default: 5)
 rbf_fee_increment = 5
-# Maximum percentage to RBF bitcoin tx (default: 150% of satsv/B)
+# Maximum percentage of satoshis_per_byte to allow in RBF fee (default: 150)
 max_rbf = 150
 ```
 
+NOTE: Ensuring that your miner can successfully use RBF (Replace-by-Fee) is
+critical for reliable block production. If a miner fails to replace an outdated
+block commit with a higher-fee transaction, it risks committing to an incorrect
+tenure. This would prevent the miner from producing valid blocks during its
+tenure, as it would be building on an invalid chain tip, causing the signers to
+reject its blocks.
+
+To avoid this, configure satoshis_per_byte, rbf_fee_increment, and max_rbf to
+allow for at least three fee increments within the max_rbf limit. This helps
+ensure that your miner can adjust its fees sufficiently to stay on the canonical
+chain.
+
 You can verify that your node is operating as a miner by checking its log output
 to verify that it was able to find its Bitcoin UTXOs:
 

testnet/stacks-node/src/config.rs

@@ -89,6 +89,7 @@ const INV_REWARD_CYCLES_TESTNET: u64 = 6;
 const DEFAULT_MIN_TIME_BETWEEN_BLOCKS_MS: u64 = 1_000;
 const DEFAULT_FIRST_REJECTION_PAUSE_MS: u64 = 5_000;
 const DEFAULT_SUBSEQUENT_REJECTION_PAUSE_MS: u64 = 10_000;
+const DEFAULT_BLOCK_COMMIT_DELAY_MS: u64 = 20_000;
 
 #[derive(Clone, Deserialize, Default, Debug)]
 #[serde(deny_unknown_fields)]
@@ -2167,6 +2168,8 @@ pub struct MinerConfig {
     pub first_rejection_pause_ms: u64,
     /// Time in milliseconds to pause after receiving subsequent threshold rejections, before proposing a new block.
     pub subsequent_rejection_pause_ms: u64,
+    /// Duration to wait for a Nakamoto block after seeing a burnchain block before submitting a block commit.
+    pub block_commit_delay: Duration,
 }
 
 impl Default for MinerConfig {
@@ -2199,6 +2202,7 @@ impl Default for MinerConfig {
             min_time_between_blocks_ms: DEFAULT_MIN_TIME_BETWEEN_BLOCKS_MS,
             first_rejection_pause_ms: DEFAULT_FIRST_REJECTION_PAUSE_MS,
             subsequent_rejection_pause_ms: DEFAULT_SUBSEQUENT_REJECTION_PAUSE_MS,
+            block_commit_delay: Duration::from_millis(DEFAULT_BLOCK_COMMIT_DELAY_MS),
         }
     }
 }
@@ -2568,6 +2572,7 @@ pub struct MinerConfigFile {
     pub min_time_between_blocks_ms: Option<u64>,
     pub first_rejection_pause_ms: Option<u64>,
     pub subsequent_rejection_pause_ms: Option<u64>,
+    pub block_commit_delay_ms: Option<u64>,
 }
 
 impl MinerConfigFile {
@@ -2683,6 +2688,7 @@ impl MinerConfigFile {
             }).unwrap_or(miner_default_config.min_time_between_blocks_ms),
             first_rejection_pause_ms: self.first_rejection_pause_ms.unwrap_or(miner_default_config.first_rejection_pause_ms),
             subsequent_rejection_pause_ms: self.subsequent_rejection_pause_ms.unwrap_or(miner_default_config.subsequent_rejection_pause_ms),
+            block_commit_delay: self.block_commit_delay_ms.map(Duration::from_millis).unwrap_or(miner_default_config.block_commit_delay),
         })
     }
 }

testnet/stacks-node/src/nakamoto_node/relayer.rs

@@ -236,6 +236,8 @@ pub struct RelayerThread {
     /// Information about the last-sent block commit, and the relayer's view of the chain at the
     /// time it was sent.
     last_committed: Option<LastCommit>,
+    /// Timeout for waiting for the first block in a tenure before submitting a block commit
+    new_tenure_timeout: Option<Instant>,
 }
 
 impl RelayerThread {
@@ -293,6 +295,7 @@ impl RelayerThread {
             is_miner,
             next_initiative: Instant::now() + Duration::from_millis(next_initiative_delay),
             last_committed: None,
+            new_tenure_timeout: None,
         }
     }
 
@@ -1176,6 +1179,32 @@ impl RelayerThread {
             return None;
         }
 
+        if !highest_tenure_changed {
+            debug!("Relayer: burnchain view changed, but highest tenure did not");
+            // The burnchain view changed, but the highest tenure did not, so
+            // wait a bit for the first block in the new tenure to arrive. This
+            // is to avoid submitting a block commit that will be immediately
+            // RBFed when the first block arrives.
+            if let Some(new_tenure_timeout) = self.new_tenure_timeout {
+                debug!(
+                    "Relayer: {}s elapsed since burn block arrival",
+                    new_tenure_timeout.elapsed().as_secs(),
+                );
+                if new_tenure_timeout.elapsed() < self.config.miner.block_commit_delay {
+                    return None;
+                }
+            } else {
+                info!(
+                    "Relayer: starting new tenure timeout for {}s",
+                    self.config.miner.block_commit_delay.as_secs()
+                );
+                let timeout = Instant::now() + self.config.miner.block_commit_delay;
+                self.new_tenure_timeout = Some(Instant::now());
+                self.next_initiative = timeout;
+                return None;
+            }
+        }
+
         // burnchain view or highest-tenure view changed, so we need to send (or RBF) a commit
         Some(RelayerDirective::IssueBlockCommit(
             stacks_tip_ch,

testnet/stacks-node/src/tests/nakamoto_integrations.rs

@@ -3721,7 +3721,7 @@ fn follower_bootup_across_multiple_cycles() {
 
     let (mut naka_conf, _miner_account) = naka_neon_integration_conf(None);
     naka_conf.miner.wait_on_interim_blocks = Duration::from_secs(1);
-    naka_conf.node.pox_sync_sample_secs = 30;
+    naka_conf.node.pox_sync_sample_secs = 180;
     naka_conf.burnchain.max_rbf = 10_000_000;
 
     let sender_sk = Secp256k1PrivateKey::new();
@@ -4866,6 +4866,7 @@ fn forked_tenure_is_ignored() {
 
     let (mut naka_conf, _miner_account) = naka_neon_integration_conf(None);
     naka_conf.miner.wait_on_interim_blocks = Duration::from_secs(10);
+    naka_conf.miner.block_commit_delay = Duration::from_secs(0);
     let sender_sk = Secp256k1PrivateKey::new();
     // setup sender + recipient for a test stx transfer
     let sender_addr = tests::to_addr(&sender_sk);

testnet/stacks-node/src/tests/signer/mod.rs

@@ -557,7 +557,7 @@ impl<S: Signer<T> + Send + 'static, T: SignerEventTrait + 'static> SignerTest<Sp
         signer_signature_hash: &Sha512Trunc256Sum,
         expected_signers: &[StacksPublicKey],
     ) -> Result<(), String> {
-        // Make sure that ALL signers accepted the block proposal
+        // Make sure that at least 70% of signers accepted the block proposal
         wait_for(timeout_secs, || {
             let signatures = test_observer::get_stackerdb_chunks()
                 .into_iter()
@@ -585,7 +585,7 @@ impl<S: Signer<T> + Send + 'static, T: SignerEventTrait + 'static> SignerTest<Sp
                     }
                 })
                 .collect::<HashSet<_>>();
-            Ok(signatures.len() == expected_signers.len())
+            Ok(signatures.len() > expected_signers.len() * 7 / 10)
         })
     }