Merge branch 'develop' of https://github.com/stacks-network/stacks-core into feat/signer-state-conflict-resolution-strategies

Author: jferrant

CHANGELOG.md

@@ -5,7 +5,7 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to the versioning scheme outlined in the [README.md](README.md).
 
-## [Unreleased]
+## [3.1.0.0.8]
 
 ### Added
 

stacks-signer/CHANGELOG.md

@@ -5,7 +5,7 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to the versioning scheme outlined in the [README.md](README.md).
 
-## [Unreleased]
+## [3.1.0.0.8.0]
 
 ### Changed
 

stacks-signer/src/monitoring/mod.rs

@@ -14,12 +14,27 @@
 // You should have received a copy of the GNU General Public License
 // along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+use stacks_common::define_named_enum;
+
 #[cfg(feature = "monitoring_prom")]
 mod prometheus;
 
 #[cfg(feature = "monitoring_prom")]
 mod server;
 
+define_named_enum!(
+/// Represent different state change reason on signer agreement protocol
+SignerAgreementStateChangeReason {
+    /// A new burn block has arrived
+    BurnBlockArrival("burn_block_arrival"),
+    /// A new stacks block has arrived
+    StacksBlockArrival("stacks_block_arrival"),
+    /// A miner is inactive when it should be starting its tenure
+    InactiveMiner("inactive_miner"),
+    /// Signer agreement protocol version has been upgraded
+    ProtocolUpgrade("protocol_upgrade"),
+});
+
 /// Actions for updating metrics
 #[cfg(feature = "monitoring_prom")]
 pub mod actions {
@@ -29,6 +44,7 @@ pub mod actions {
 
     use crate::config::GlobalConfig;
     use crate::monitoring::prometheus::*;
+    use crate::monitoring::SignerAgreementStateChangeReason;
     use crate::v0::signer_state::LocalStateMachine;
 
     /// Update stacks tip height gauge
@@ -108,6 +124,16 @@ pub mod actions {
             .replace(state);
     }
 
+    /// Increment signer agreement state change reason counter
+    pub fn increment_signer_agreement_state_change_reason(
+        reason: SignerAgreementStateChangeReason,
+    ) {
+        let label_value = reason.get_name();
+        SIGNER_AGREEMENT_STATE_CHANGE_REASONS
+            .with_label_values(&[&label_value])
+            .inc();
+    }
+
     /// Start serving monitoring metrics.
     /// This will only serve the metrics if the `monitoring_prom` feature is enabled.
     pub fn start_serving_monitoring_metrics(config: GlobalConfig) -> Result<(), String> {
@@ -131,6 +157,7 @@ pub mod actions {
     use blockstack_lib::chainstate::nakamoto::NakamotoBlock;
     use stacks_common::info;
 
+    use crate::monitoring::SignerAgreementStateChangeReason;
     use crate::v0::signer_state::LocalStateMachine;
     use crate::GlobalConfig;
 
@@ -179,6 +206,12 @@ pub mod actions {
     /// Record the current local state machine
     pub fn record_local_state(_state: LocalStateMachine) {}
 
+    /// Increment signer agreement state change reason counter
+    pub fn increment_signer_agreement_state_change_reason(
+        _reason: SignerAgreementStateChangeReason,
+    ) {
+    }
+
     /// Start serving monitoring metrics.
     /// This will only serve the metrics if the `monitoring_prom` feature is enabled.
     pub fn start_serving_monitoring_metrics(config: GlobalConfig) -> Result<(), String> {

stacks-signer/src/monitoring/prometheus.rs

@@ -79,6 +79,12 @@ lazy_static! {
         vec![0.005, 0.1, 0.25, 0.5, 1.0, 2.5, 5.0, 10.0, 20.0, 30.0, 60.0, 120.0]
     ), &[]).unwrap();
 
+    pub static ref SIGNER_AGREEMENT_STATE_CHANGE_REASONS: IntCounterVec = register_int_counter_vec!(
+        "stacks_signer_agreement_state_change_reasons",
+        "The number of state machine changes in signer agreement protocol. `reason` can be one of: 'burn_block_arrival', 'stacks_block_arrival', 'inactive_miner', 'protocol_upgrade'",
+        &["reason"]
+    ).unwrap();
+
     pub static ref SIGNER_LOCAL_STATE_MACHINE: Mutex<Option<LocalStateMachine>> = Mutex::new(None);
 }
 

stacks-signer/src/v0/signer_state.rs

@@ -487,6 +487,11 @@ impl LocalStateMachine {
                 "inactive_tenure_ch" => %inactive_tenure_ch,
                 "new_active_tenure_ch" => %new_active_tenure_ch
             );
+
+            crate::monitoring::actions::increment_signer_agreement_state_change_reason(
+                crate::monitoring::SignerAgreementStateChangeReason::InactiveMiner,
+            );
+
             Ok(())
         } else {
             warn!("Current miner timed out due to inactivity, but prior miner is not valid. Allowing current miner to continue");
@@ -609,6 +614,11 @@ impl LocalStateMachine {
         *parent_tenure_last_block = *block_id;
         *parent_tenure_last_block_height = height;
         *self = LocalStateMachine::Initialized(prior_state_machine);
+
+        crate::monitoring::actions::increment_signer_agreement_state_change_reason(
+            crate::monitoring::SignerAgreementStateChangeReason::StacksBlockArrival,
+        );
+
         Ok(())
     }
 
@@ -663,7 +673,7 @@ impl LocalStateMachine {
         // set self to uninitialized so that if this function errors,
         //  self is left as uninitialized.
         let prior_state = std::mem::replace(self, Self::Uninitialized);
-        let prior_state_machine = match prior_state {
+        let prior_state_machine = match prior_state.clone() {
             // if the local state machine was uninitialized, just initialize it
             LocalStateMachine::Uninitialized => Self::place_holder(),
             LocalStateMachine::Initialized(signer_state_machine) => signer_state_machine,
@@ -742,6 +752,12 @@ impl LocalStateMachine {
             active_signer_protocol_version: prior_state_machine.active_signer_protocol_version,
         });
 
+        if prior_state != *self {
+            crate::monitoring::actions::increment_signer_agreement_state_change_reason(
+                crate::monitoring::SignerAgreementStateChangeReason::BurnBlockArrival,
+            );
+        }
+
         Ok(())
     }
 

stackslib/src/core/mempool.rs

@@ -2953,7 +2953,18 @@ pub fn try_flush_considered_txs(
     let db_tx = conn.transaction()?;
 
     for txid in considered_txs {
-        db_tx.execute(sql, params![txid])?;
+        match db_tx.execute(sql, params![txid]) {
+            Ok(_) => {}
+            Err(rusqlite::Error::SqliteFailure(err, _))
+                if err.code == rusqlite::ErrorCode::ConstraintViolation =>
+            {
+                // Ignore constraint violations (e.g., foreign key failure)
+                // This can happen if the txid was removed from the mempool DB
+                // before we could flush it to the considered_txs table.
+                continue;
+            }
+            Err(e) => return Err(e.into()),
+        }
     }
 
     db_tx.commit()?;

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

@@ -12183,3 +12183,159 @@ fn v3_transaction_api_endpoint() {
 
     run_loop_thread.join().unwrap();
 }
+
+#[test]
+#[ignore]
+/// This test verifies that the miner can continue even if an insertion into
+/// the `considered_txs` table fails due to a foreign key failure.
+fn handle_considered_txs_foreign_key_failure() {
+    if env::var("BITCOIND_TEST") != Ok("1".into()) {
+        return;
+    }
+
+    let (mut naka_conf, _miner_account) = naka_neon_integration_conf(None);
+    let prom_bind = "127.0.0.1:6000".to_string();
+    naka_conf.node.prometheus_bind = Some(prom_bind);
+    naka_conf.miner.nakamoto_attempt_time_ms = 5_000;
+    naka_conf.miner.tenure_cost_limit_per_block_percentage = None;
+    naka_conf.miner.mempool_walk_strategy = MemPoolWalkStrategy::NextNonceWithHighestFeeRate;
+    // setup senders
+    let send_amt = 1000;
+    let send_fee = 180;
+    let bad_sender_sk = Secp256k1PrivateKey::from_seed(&[30]);
+    let bad_sender_addr = tests::to_addr(&bad_sender_sk);
+    naka_conf.add_initial_balance(
+        PrincipalData::from(bad_sender_addr).to_string(),
+        send_amt + send_fee,
+    );
+    let good_sender_sk = Secp256k1PrivateKey::from_seed(&[31]);
+    let good_sender_addr = tests::to_addr(&good_sender_sk);
+    naka_conf.add_initial_balance(
+        PrincipalData::from(good_sender_addr).to_string(),
+        (send_amt + send_fee) * 2,
+    );
+
+    let sender_signer_sk = Secp256k1PrivateKey::random();
+    let sender_signer_addr = tests::to_addr(&sender_signer_sk);
+    let mut signers = TestSigners::new(vec![sender_signer_sk]);
+    naka_conf.add_initial_balance(PrincipalData::from(sender_signer_addr).to_string(), 100000);
+    let recipient = PrincipalData::from(StacksAddress::burn_address(false));
+    let stacker_sk = setup_stacker(&mut naka_conf);
+    let http_origin = format!("http://{}", &naka_conf.node.rpc_bind);
+
+    test_observer::spawn();
+    test_observer::register_any(&mut naka_conf);
+
+    let mut btcd_controller = BitcoinCoreController::new(naka_conf.clone());
+    btcd_controller
+        .start_bitcoind()
+        .expect("Failed starting bitcoind");
+    let mut btc_regtest_controller = BitcoinRegtestController::new(naka_conf.clone(), None);
+    btc_regtest_controller.bootstrap_chain(201);
+
+    let mut run_loop = boot_nakamoto::BootRunLoop::new(naka_conf.clone()).unwrap();
+    let run_loop_stopper = run_loop.get_termination_switch();
+    let Counters {
+        blocks_processed,
+        naka_submitted_commits: commits_submitted,
+        ..
+    } = run_loop.counters();
+    let counters = run_loop.counters();
+
+    let coord_channel = run_loop.coordinator_channels();
+
+    let run_loop_thread = thread::spawn(move || run_loop.start(None, 0));
+    wait_for_runloop(&blocks_processed);
+    boot_to_epoch_3(
+        &naka_conf,
+        &blocks_processed,
+        &[stacker_sk],
+        &[sender_signer_sk],
+        &mut Some(&mut signers),
+        &mut btc_regtest_controller,
+    );
+
+    info!("Bootstrapped to Epoch-3.0 boundary, starting nakamoto miner");
+
+    info!("Nakamoto miner started...");
+    blind_signer(&naka_conf, &signers, &counters);
+
+    wait_for_first_naka_block_commit(60, &commits_submitted);
+
+    next_block_and_process_new_stacks_block(&mut btc_regtest_controller, 60, &coord_channel)
+        .unwrap();
+
+    let good_transfer_tx = make_stacks_transfer(
+        &good_sender_sk,
+        0,
+        send_fee,
+        naka_conf.burnchain.chain_id,
+        &recipient,
+        send_amt,
+    );
+    submit_tx(&http_origin, &good_transfer_tx);
+
+    wait_for(60, || {
+        let nonce = get_account(&http_origin, &good_sender_addr).nonce;
+        Ok(nonce == 1)
+    })
+    .expect("Timed out waiting for first block");
+
+    let height_before = get_chain_info(&naka_conf).stacks_tip_height;
+
+    // Initiate the transaction stall, then submit transactions.
+    TEST_MINE_STALL.set(true);
+    TEST_TX_STALL.set(true);
+
+    let bad_transfer_tx = make_stacks_transfer(
+        &bad_sender_sk,
+        0,
+        send_fee,
+        naka_conf.burnchain.chain_id,
+        &recipient,
+        send_amt,
+    );
+    let txid = submit_tx(&http_origin, &bad_transfer_tx);
+    info!("Bad transaction submitted: {txid}");
+
+    TEST_MINE_STALL.set(false);
+
+    // Sleep long enough to ensure that the miner has started processing the tx
+    sleep_ms(5_000);
+
+    info!("--------------------- Deleting tx from the mempool ---------------------");
+    // Delete the bad transaction from the mempool.
+    let mempool_db_path = format!(
+        "{}/nakamoto-neon/chainstate/mempool.sqlite",
+        naka_conf.node.working_dir
+    );
+    let conn = Connection::open(&mempool_db_path).unwrap();
+    conn.execute("DELETE FROM mempool WHERE txid = ?", [txid])
+        .unwrap();
+
+    // Unstall the transaction processing, so that the miner will resume.
+    TEST_TX_STALL.set(false);
+
+    info!("--------------------- Waiting for the block ---------------------");
+
+    // Now wait for the next block to be mined.
+    wait_for(30, || {
+        let height = get_chain_info(&naka_conf).stacks_tip_height;
+        Ok(height > height_before)
+    })
+    .expect("Timed out waiting for block");
+
+    let good_sender_nonce = get_account(&http_origin, &good_sender_addr).nonce;
+    let bad_sender_nonce = get_account(&http_origin, &bad_sender_addr).nonce;
+
+    assert_eq!(good_sender_nonce, 1);
+    assert_eq!(bad_sender_nonce, 1);
+
+    coord_channel
+        .lock()
+        .expect("Mutex poisoned")
+        .stop_chains_coordinator();
+    run_loop_stopper.store(false, Ordering::SeqCst);
+
+    run_loop_thread.join().unwrap();
+}