chore: avoid warning from duplicate block-commit

obycode · obycode · commit 120348ac3817 · 2024-09-11T15:45:22.000-04:00
This change ended up being a lot larger than I'd hoped, but with a recent investigation into some networking failures when communicating with the bitcoin node, these warnings were annoying. I'm just trying to reduce false-positive warning logs. The problematic case shows up like this in the logs before this change: ``` INFO [1725975417.732025] [testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs:1634] [miner-block-http://127.0.0.1:35738] Abort attempt to re-submit identical LeaderBlockCommit WARN [1725975417.732029] [testnet/stacks-node/src/neon_node.rs:2759] [miner-block-http://127.0.0.1:35738] Relayer: Failed to submit Bitcoin transaction ```
diff --git a/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs b/testnet/stacks-node/src/burnchains/bitcoin_regtest_controller.rs
diff --git a/testnet/stacks-node/src/burnchains/mocknet_controller.rs b/testnet/stacks-node/src/burnchains/mocknet_controller.rs
@@ -168,10 +168,10 @@ impl BurnchainController for MocknetController {
         operation: BlockstackOperationType,
         _op_signer: &mut BurnchainOpSigner,
         _attempt: u64,
-    ) -> Option<Txid> {
+    ) -> Result<Txid, BurnchainControllerError> {
         let txid = operation.txid();
         self.queued_operations.push_back(operation);
-        Some(txid)
+        Ok(txid)
     }
 
     fn sync(
diff --git a/testnet/stacks-node/src/burnchains/mod.rs b/testnet/stacks-node/src/burnchains/mod.rs
@@ -19,13 +19,25 @@ use super::operations::BurnchainOpSigner;
 pub enum Error {
     CoordinatorClosed,
     IndexerError(burnchains::Error),
+    BurnchainError,
+    MaxFeeRateExceeded,
+    IdenticalOperation,
+    NoUtxos,
+    TransactionSubmissionFailed,
+    SerializerError,
 }
 
 impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
             Error::CoordinatorClosed => write!(f, "ChainsCoordinator closed"),
             Error::IndexerError(ref e) => write!(f, "Indexer error: {:?}", e),
+            Error::BurnchainError => write!(f, "Burnchain error"),
+            Error::MaxFeeRateExceeded => write!(f, "Max fee rate exceeded"),
+            Error::IdenticalOperation => write!(f, "Identical operation, not submitting"),
+            Error::NoUtxos => write!(f, "No UTXOs available"),
+            Error::TransactionSubmissionFailed => write!(f, "Transaction submission failed"),
+            Error::SerializerError => write!(f, "Serializer error"),
         }
     }
 }
@@ -45,7 +57,7 @@ pub trait BurnchainController {
         operation: BlockstackOperationType,
         op_signer: &mut BurnchainOpSigner,
         attempt: u64,
-    ) -> Option<Txid>;
+    ) -> Result<Txid, Error>;
     fn sync(&mut self, target_block_height_opt: Option<u64>) -> Result<(BurnchainTip, u64), Error>;
     fn sortdb_ref(&self) -> &SortitionDB;
     fn sortdb_mut(&mut self) -> &mut SortitionDB;
diff --git a/testnet/stacks-node/src/nakamoto_node/relayer.rs b/testnet/stacks-node/src/nakamoto_node/relayer.rs
@@ -530,9 +530,9 @@ impl RelayerThread {
         let op = Self::make_key_register_op(vrf_pk, burnchain_tip_consensus_hash, &miner_pkh);
 
         let mut op_signer = self.keychain.generate_op_signer();
-        if let Some(txid) =
-            self.bitcoin_controller
-                .submit_operation(cur_epoch, op, &mut op_signer, 1)
+        if let Ok(txid) = self
+            .bitcoin_controller
+            .submit_operation(cur_epoch, op, &mut op_signer, 1)
         {
             // advance key registration state
             self.last_vrf_key_burn_height = Some(burn_block.block_height);
@@ -1048,8 +1048,8 @@ impl RelayerThread {
                 &mut op_signer,
                 1,
             )
-            .ok_or_else(|| {
-                warn!("Failed to submit block-commit bitcoin transaction");
+            .map_err(|e| {
+                warn!("Failed to submit block-commit bitcoin transaction: {}", e);
                 NakamotoNodeError::BurnchainSubmissionFailed
             })?;
 
diff --git a/testnet/stacks-node/src/neon_node.rs b/testnet/stacks-node/src/neon_node.rs
@@ -213,7 +213,7 @@ use super::{BurnchainController, Config, EventDispatcher, Keychain};
 use crate::burnchains::bitcoin_regtest_controller::{
     addr2str, burnchain_params_from_config, BitcoinRegtestController, OngoingBlockCommit,
 };
-use crate::burnchains::make_bitcoin_indexer;
+use crate::burnchains::{make_bitcoin_indexer, Error as BurnchainControllerError};
 use crate::chain_data::MinerStats;
 use crate::config::NodeConfig;
 use crate::globals::{NeonGlobals as Globals, RelayerDirective};
@@ -2753,16 +2753,21 @@ impl BlockMinerThread {
         } = self.config.get_node_config(false);
 
         let res = bitcoin_controller.submit_operation(target_epoch_id, op, &mut op_signer, attempt);
-        if res.is_none() {
-            self.failed_to_submit_last_attempt = true;
-            if !mock_mining {
-                warn!("Relayer: Failed to submit Bitcoin transaction");
-                return None;
+        self.failed_to_submit_last_attempt = match res {
+            Ok(_) => false,
+            Err(BurnchainControllerError::IdenticalOperation) => {
+                info!("Relayer: Block-commit already submitted");
+                true
             }
-            debug!("Relayer: Mock-mining enabled; not sending Bitcoin transaction");
-        } else {
-            self.failed_to_submit_last_attempt = false;
-        }
+            Err(_) if mock_mining => {
+                debug!("Relayer: Mock-mining enabled; not sending Bitcoin transaction");
+                true
+            }
+            Err(e) => {
+                warn!("Relayer: Failed to submit Bitcoin transaction: {:?}", e);
+                true
+            }
+        };
 
         let assembled_block = AssembledAnchorBlock {
             parent_consensus_hash: parent_block_info.parent_consensus_hash,
@@ -3620,7 +3625,7 @@ impl RelayerThread {
         );
 
         let mut one_off_signer = self.keychain.generate_op_signer();
-        if let Some(txid) =
+        if let Ok(txid) =
             self.bitcoin_controller
                 .submit_operation(cur_epoch, op, &mut one_off_signer, 1)
         {
diff --git a/testnet/stacks-node/src/run_loop/neon.rs b/testnet/stacks-node/src/run_loop/neon.rs
@@ -497,7 +497,7 @@ impl RunLoop {
                             return burnchain_error::ShutdownInitiated;
                         }
                     }
-                    Error::IndexerError(_) => {}
+                    _ => {}
                 }
                 error!("Burnchain controller stopped: {}", e);
                 panic!();
diff --git a/testnet/stacks-node/src/tests/epoch_205.rs b/testnet/stacks-node/src/tests/epoch_205.rs
@@ -629,7 +629,7 @@ fn transition_empty_blocks() {
                 &mut op_signer,
                 1,
             );
-            assert!(res.is_some(), "Failed to submit block-commit");
+            assert!(res.is_ok(), "Failed to submit block-commit");
         }
 
         next_block_and_wait(&mut btc_regtest_controller, &blocks_processed);
diff --git a/testnet/stacks-node/src/tests/epoch_21.rs b/testnet/stacks-node/src/tests/epoch_21.rs
@@ -678,7 +678,7 @@ fn transition_fixes_bitcoin_rigidity() {
                 &mut miner_signer,
                 1
             )
-            .is_some(),
+            .is_ok(),
         "Pre-stx operation should submit successfully"
     );
 
@@ -713,7 +713,7 @@ fn transition_fixes_bitcoin_rigidity() {
                 &mut spender_signer,
                 1
             )
-            .is_some(),
+            .is_ok(),
         "Transfer operation should submit successfully"
     );
 
@@ -835,7 +835,7 @@ fn transition_fixes_bitcoin_rigidity() {
                 &mut miner_signer,
                 1
             )
-            .is_some(),
+            .is_ok(),
         "Pre-stx operation should submit successfully"
     );
 
@@ -866,7 +866,7 @@ fn transition_fixes_bitcoin_rigidity() {
                 &mut spender_signer,
                 1
             )
-            .is_some(),
+            .is_ok(),
         "Transfer operation should submit successfully"
     );
 
@@ -1946,7 +1946,7 @@ fn transition_empty_blocks() {
             let mut op_signer = keychain.generate_op_signer();
             let res =
                 bitcoin_controller.submit_operation(StacksEpochId::Epoch21, op, &mut op_signer, 1);
-            assert!(res.is_some(), "Failed to submit block-commit");
+            assert!(res.is_ok(), "Failed to submit block-commit");
         }
 
         next_block_and_wait(&mut btc_regtest_controller, &blocks_processed);
diff --git a/testnet/stacks-node/src/tests/nakamoto_integrations.rs b/testnet/stacks-node/src/tests/nakamoto_integrations.rs
@@ -2876,7 +2876,7 @@ fn vote_for_aggregate_key_burn_op() {
                 &mut miner_signer,
                 1
             )
-            .is_some(),
+            .is_ok(),
         "Pre-stx operation should submit successfully"
     );
 
@@ -2952,7 +2952,7 @@ fn vote_for_aggregate_key_burn_op() {
                 &mut signer_burnop_signer,
                 1
             )
-            .is_some(),
+            .is_ok(),
         "Vote for aggregate key operation should submit successfully"
     );
 
@@ -3433,7 +3433,7 @@ fn stack_stx_burn_op_integration_test() {
                 &mut miner_signer_1,
                 1
             )
-            .is_some(),
+            .is_ok(),
         "Pre-stx operation should submit successfully"
     );
 
@@ -3463,7 +3463,7 @@ fn stack_stx_burn_op_integration_test() {
                 &mut miner_signer_2,
                 1
             )
-            .is_some(),
+            .is_ok(),
         "Pre-stx operation should submit successfully"
     );
     info!("Submitted 2 pre-stx ops at block {block_height}, mining a few blocks...");
@@ -3604,7 +3604,7 @@ fn stack_stx_burn_op_integration_test() {
                 &mut signer_burnop_signer_1,
                 1
             )
-            .is_some(),
+            .is_ok(),
         "Stack STX operation should submit successfully"
     );
 
@@ -3631,7 +3631,7 @@ fn stack_stx_burn_op_integration_test() {
                 &mut signer_burnop_signer_2,
                 1
             )
-            .is_some(),
+            .is_ok(),
         "Stack STX operation should submit successfully"
     );
 
diff --git a/testnet/stacks-node/src/tests/neon_integrations.rs b/testnet/stacks-node/src/tests/neon_integrations.rs
@@ -1934,7 +1934,7 @@ fn stx_transfer_btc_integration_test() {
                 &mut miner_signer,
                 1
             )
-            .is_some(),
+            .is_ok(),
         "Pre-stx operation should submit successfully"
     );
 
@@ -1964,7 +1964,7 @@ fn stx_transfer_btc_integration_test() {
                 &mut spender_signer,
                 1
             )
-            .is_some(),
+            .is_ok(),
         "Transfer operation should submit successfully"
     );
     // should be elected in the same block as the transfer, so balances should be unchanged.
@@ -2215,7 +2215,7 @@ fn stx_delegate_btc_integration_test() {
                 &mut miner_signer,
                 1
             )
-            .is_some(),
+            .is_ok(),
         "Pre-stx operation should submit successfully"
     );
 
@@ -2244,7 +2244,7 @@ fn stx_delegate_btc_integration_test() {
                 &mut spender_signer,
                 1
             )
-            .is_some(),
+            .is_ok(),
         "Delegate operation should submit successfully"
     );
 
@@ -2507,7 +2507,7 @@ fn stack_stx_burn_op_test() {
                 &mut miner_signer_1,
                 1
             )
-            .is_some(),
+            .is_ok(),
         "Pre-stx operation should submit successfully"
     );
 
@@ -2528,7 +2528,7 @@ fn stack_stx_burn_op_test() {
                 &mut miner_signer_2,
                 1
             )
-            .is_some(),
+            .is_ok(),
         "Pre-stx operation should submit successfully"
     );
     info!("Submitted 2 pre-stx ops at block {block_height}, mining a few blocks...");
@@ -2614,7 +2614,7 @@ fn stack_stx_burn_op_test() {
                 &mut spender_signer_1,
                 1
             )
-            .is_some(),
+            .is_ok(),
         "Stack STX operation with some signer key should submit successfully"
     );
 
@@ -2642,7 +2642,7 @@ fn stack_stx_burn_op_test() {
                 &mut spender_signer_2,
                 1
             )
-            .is_some(),
+            .is_ok(),
         "Stack STX operation with no signer key should submit successfully"
     );
 
@@ -2949,7 +2949,7 @@ fn vote_for_aggregate_key_burn_op_test() {
                 &mut miner_signer,
                 1
             )
-            .is_some(),
+            .is_ok(),
         "Pre-stx operation should submit successfully"
     );
 
@@ -3006,7 +3006,7 @@ fn vote_for_aggregate_key_burn_op_test() {
                 &mut spender_signer,
                 1
             )
-            .is_some(),
+            .is_ok(),
         "Vote for aggregate key operation should submit successfully"
     );
 

Original file line number	Diff line number	Diff line change
`@@ -497,7 +497,7 @@ impl RunLoop {`
`497`	`497`	`return burnchain_error::ShutdownInitiated;`
`498`	`498`	`}`
`499`	`499`	`}`
`500`		`- Error::IndexerError(_) => {}`
	`500`	`+ _ => {}`
`501`	`501`	`}`
`502`	`502`	`error!("Burnchain controller stopped: {}", e);`
`503`	`503`	`panic!();`
Original file line number	Diff line number	Diff line change
`@@ -629,7 +629,7 @@ fn transition_empty_blocks() {`
`629`	`629`	`&mut op_signer,`
`630`	`630`	`1,`
`631`	`631`	`);`
`632`		`- assert!(res.is_some(), "Failed to submit block-commit");`
	`632`	`+ assert!(res.is_ok(), "Failed to submit block-commit");`
`633`	`633`	`}`
`634`	`634`
`635`	`635`	`next_block_and_wait(&mut btc_regtest_controller, &blocks_processed);`
Original file line number	Diff line number	Diff line change
`@@ -678,7 +678,7 @@ fn transition_fixes_bitcoin_rigidity() {`
`678`	`678`	`&mut miner_signer,`
`679`	`679`	`1`
`680`	`680`	`)`
`681`		`- .is_some(),`
	`681`	`+ .is_ok(),`
`682`	`682`	`"Pre-stx operation should submit successfully"`
`683`	`683`	`);`
`684`	`684`
`@@ -713,7 +713,7 @@ fn transition_fixes_bitcoin_rigidity() {`
`713`	`713`	`&mut spender_signer,`
`714`	`714`	`1`
`715`	`715`	`)`
`716`		`- .is_some(),`
	`716`	`+ .is_ok(),`
`717`	`717`	`"Transfer operation should submit successfully"`
`718`	`718`	`);`
`719`	`719`
`@@ -835,7 +835,7 @@ fn transition_fixes_bitcoin_rigidity() {`
`835`	`835`	`&mut miner_signer,`
`836`	`836`	`1`
`837`	`837`	`)`
`838`		`- .is_some(),`
	`838`	`+ .is_ok(),`
`839`	`839`	`"Pre-stx operation should submit successfully"`
`840`	`840`	`);`
`841`	`841`
`@@ -866,7 +866,7 @@ fn transition_fixes_bitcoin_rigidity() {`
`866`	`866`	`&mut spender_signer,`
`867`	`867`	`1`
`868`	`868`	`)`
`869`		`- .is_some(),`
	`869`	`+ .is_ok(),`
`870`	`870`	`"Transfer operation should submit successfully"`
`871`	`871`	`);`
`872`	`872`
`@@ -1946,7 +1946,7 @@ fn transition_empty_blocks() {`
`1946`	`1946`	`let mut op_signer = keychain.generate_op_signer();`
`1947`	`1947`	`let res =`
`1948`	`1948`	`bitcoin_controller.submit_operation(StacksEpochId::Epoch21, op, &mut op_signer, 1);`
`1949`		`- assert!(res.is_some(), "Failed to submit block-commit");`
	`1949`	`+ assert!(res.is_ok(), "Failed to submit block-commit");`
`1950`	`1950`	`}`
`1951`	`1951`
`1952`	`1952`	`next_block_and_wait(&mut btc_regtest_controller, &blocks_processed);`
Original file line number	Diff line number	Diff line change
`@@ -2876,7 +2876,7 @@ fn vote_for_aggregate_key_burn_op() {`
`2876`	`2876`	`&mut miner_signer,`
`2877`	`2877`	`1`
`2878`	`2878`	`)`
`2879`		`- .is_some(),`
	`2879`	`+ .is_ok(),`
`2880`	`2880`	`"Pre-stx operation should submit successfully"`
`2881`	`2881`	`);`
`2882`	`2882`
`@@ -2952,7 +2952,7 @@ fn vote_for_aggregate_key_burn_op() {`
`2952`	`2952`	`&mut signer_burnop_signer,`
`2953`	`2953`	`1`
`2954`	`2954`	`)`
`2955`		`- .is_some(),`
	`2955`	`+ .is_ok(),`
`2956`	`2956`	`"Vote for aggregate key operation should submit successfully"`
`2957`	`2957`	`);`
`2958`	`2958`
`@@ -3433,7 +3433,7 @@ fn stack_stx_burn_op_integration_test() {`
`3433`	`3433`	`&mut miner_signer_1,`
`3434`	`3434`	`1`
`3435`	`3435`	`)`
`3436`		`- .is_some(),`
	`3436`	`+ .is_ok(),`
`3437`	`3437`	`"Pre-stx operation should submit successfully"`
`3438`	`3438`	`);`
`3439`	`3439`
`@@ -3463,7 +3463,7 @@ fn stack_stx_burn_op_integration_test() {`
`3463`	`3463`	`&mut miner_signer_2,`
`3464`	`3464`	`1`
`3465`	`3465`	`)`
`3466`		`- .is_some(),`
	`3466`	`+ .is_ok(),`
`3467`	`3467`	`"Pre-stx operation should submit successfully"`
`3468`	`3468`	`);`
`3469`	`3469`	`info!("Submitted 2 pre-stx ops at block {block_height}, mining a few blocks...");`
`@@ -3604,7 +3604,7 @@ fn stack_stx_burn_op_integration_test() {`
`3604`	`3604`	`&mut signer_burnop_signer_1,`
`3605`	`3605`	`1`
`3606`	`3606`	`)`
`3607`		`- .is_some(),`
	`3607`	`+ .is_ok(),`
`3608`	`3608`	`"Stack STX operation should submit successfully"`
`3609`	`3609`	`);`
`3610`	`3610`
`@@ -3631,7 +3631,7 @@ fn stack_stx_burn_op_integration_test() {`
`3631`	`3631`	`&mut signer_burnop_signer_2,`
`3632`	`3632`	`1`
`3633`	`3633`	`)`
`3634`		`- .is_some(),`
	`3634`	`+ .is_ok(),`
`3635`	`3635`	`"Stack STX operation should submit successfully"`
`3636`	`3636`	`);`
`3637`	`3637`
Original file line number	Diff line number	Diff line change
`@@ -1934,7 +1934,7 @@ fn stx_transfer_btc_integration_test() {`
`1934`	`1934`	`&mut miner_signer,`
`1935`	`1935`	`1`
`1936`	`1936`	`)`
`1937`		`- .is_some(),`
	`1937`	`+ .is_ok(),`
`1938`	`1938`	`"Pre-stx operation should submit successfully"`
`1939`	`1939`	`);`
`1940`	`1940`
`@@ -1964,7 +1964,7 @@ fn stx_transfer_btc_integration_test() {`
`1964`	`1964`	`&mut spender_signer,`
`1965`	`1965`	`1`
`1966`	`1966`	`)`
`1967`		`- .is_some(),`
	`1967`	`+ .is_ok(),`
`1968`	`1968`	`"Transfer operation should submit successfully"`
`1969`	`1969`	`);`
`1970`	`1970`	`// should be elected in the same block as the transfer, so balances should be unchanged.`
`@@ -2215,7 +2215,7 @@ fn stx_delegate_btc_integration_test() {`
`2215`	`2215`	`&mut miner_signer,`
`2216`	`2216`	`1`
`2217`	`2217`	`)`
`2218`		`- .is_some(),`
	`2218`	`+ .is_ok(),`
`2219`	`2219`	`"Pre-stx operation should submit successfully"`
`2220`	`2220`	`);`
`2221`	`2221`
`@@ -2244,7 +2244,7 @@ fn stx_delegate_btc_integration_test() {`
`2244`	`2244`	`&mut spender_signer,`
`2245`	`2245`	`1`
`2246`	`2246`	`)`
`2247`		`- .is_some(),`
	`2247`	`+ .is_ok(),`
`2248`	`2248`	`"Delegate operation should submit successfully"`
`2249`	`2249`	`);`
`2250`	`2250`
`@@ -2507,7 +2507,7 @@ fn stack_stx_burn_op_test() {`
`2507`	`2507`	`&mut miner_signer_1,`
`2508`	`2508`	`1`
`2509`	`2509`	`)`
`2510`		`- .is_some(),`
	`2510`	`+ .is_ok(),`
`2511`	`2511`	`"Pre-stx operation should submit successfully"`
`2512`	`2512`	`);`
`2513`	`2513`
`@@ -2528,7 +2528,7 @@ fn stack_stx_burn_op_test() {`
`2528`	`2528`	`&mut miner_signer_2,`
`2529`	`2529`	`1`
`2530`	`2530`	`)`
`2531`		`- .is_some(),`
	`2531`	`+ .is_ok(),`
`2532`	`2532`	`"Pre-stx operation should submit successfully"`
`2533`	`2533`	`);`
`2534`	`2534`	`info!("Submitted 2 pre-stx ops at block {block_height}, mining a few blocks...");`
`@@ -2614,7 +2614,7 @@ fn stack_stx_burn_op_test() {`
`2614`	`2614`	`&mut spender_signer_1,`
`2615`	`2615`	`1`
`2616`	`2616`	`)`
`2617`		`- .is_some(),`
	`2617`	`+ .is_ok(),`
`2618`	`2618`	`"Stack STX operation with some signer key should submit successfully"`
`2619`	`2619`	`);`
`2620`	`2620`
`@@ -2642,7 +2642,7 @@ fn stack_stx_burn_op_test() {`
`2642`	`2642`	`&mut spender_signer_2,`
`2643`	`2643`	`1`
`2644`	`2644`	`)`
`2645`		`- .is_some(),`
	`2645`	`+ .is_ok(),`
`2646`	`2646`	`"Stack STX operation with no signer key should submit successfully"`
`2647`	`2647`	`);`
`2648`	`2648`
`@@ -2949,7 +2949,7 @@ fn vote_for_aggregate_key_burn_op_test() {`
`2949`	`2949`	`&mut miner_signer,`
`2950`	`2950`	`1`
`2951`	`2951`	`)`
`2952`		`- .is_some(),`
	`2952`	`+ .is_ok(),`
`2953`	`2953`	`"Pre-stx operation should submit successfully"`
`2954`	`2954`	`);`
`2955`	`2955`
`@@ -3006,7 +3006,7 @@ fn vote_for_aggregate_key_burn_op_test() {`
`3006`	`3006`	`&mut spender_signer,`
`3007`	`3007`	`1`
`3008`	`3008`	`)`
`3009`		`- .is_some(),`
	`3009`	`+ .is_ok(),`
`3010`	`3010`	`"Vote for aggregate key operation should submit successfully"`
`3011`	`3011`	`);`
`3012`	`3012`