expand first nakamoto-neon test, update block commit logic to issue c…

…ommits at tenure_id changes, cargo fmt-stacks

clarity/src/vm/functions/mod.rs

@@ -68,9 +68,8 @@ macro_rules! switch_on_global_epoch {
     };
 }
 
-use crate::vm::ClarityVersion;
-
 use super::errors::InterpreterError;
+use crate::vm::ClarityVersion;
 
 mod arithmetic;
 mod assets;

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

@@ -8,7 +8,8 @@ use async_h1::client;
 use async_std::io::ReadExt;
 use async_std::net::TcpStream;
 use base64::encode;
-
+#[cfg(test)]
+use clarity::vm::types::PrincipalData;
 use http_types::{Method, Request, Url};
 use serde::Serialize;
 use serde_json::json;
@@ -51,15 +52,12 @@ use stacks_common::deps_common::bitcoin::network::serialize::deserialize as btc_
 use stacks_common::deps_common::bitcoin::network::serialize::RawEncoder;
 use stacks_common::deps_common::bitcoin::util::hash::Sha256dHash;
 use stacks_common::types::chainstate::BurnchainHeaderHash;
+#[cfg(test)]
+use stacks_common::types::chainstate::StacksAddress;
 use stacks_common::util::hash::{hex_bytes, Hash160};
 use stacks_common::util::secp256k1::Secp256k1PublicKey;
 use stacks_common::util::sleep_ms;
 
-#[cfg(test)]
-use clarity::vm::types::PrincipalData;
-#[cfg(test)]
-use stacks_common::types::chainstate::StacksAddress;
-
 use super::super::operations::BurnchainOpSigner;
 use super::super::Config;
 use super::{BurnchainController, BurnchainTip, Error as BurnchainControllerError};

testnet/stacks-node/src/globals.rs

@@ -1,8 +1,6 @@
-use std::sync::atomic::AtomicBool;
-use std::sync::atomic::Ordering;
+use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::mpsc::SyncSender;
-use std::sync::Arc;
-use std::sync::Mutex;
+use std::sync::{Arc, Mutex};
 
 use stacks::burnchains::Txid;
 use stacks::chainstate::burn::operations::LeaderKeyRegisterOp;
@@ -12,16 +10,13 @@ use stacks::chainstate::stacks::db::unconfirmed::UnconfirmedTxMap;
 use stacks::chainstate::stacks::db::StacksChainState;
 use stacks::chainstate::stacks::miner::MinerStatus;
 use stacks::net::NetworkResult;
-use stacks_common::types::chainstate::BlockHeaderHash;
-use stacks_common::types::chainstate::BurnchainHeaderHash;
-use stacks_common::types::chainstate::ConsensusHash;
+use stacks_common::types::chainstate::{BlockHeaderHash, BurnchainHeaderHash, ConsensusHash};
 
 use crate::neon::Counters;
+use crate::neon_node::LeaderKeyRegistrationState;
 use crate::run_loop::RegisteredKey;
 use crate::syncctl::PoxSyncWatchdogComms;
 
-use crate::neon_node::LeaderKeyRegistrationState;
-
 /// Command types for the relayer thread, issued to it by other threads
 pub enum RelayerDirective {
     /// Handle some new data that arrived on the network (such as blocks, transactions, and

testnet/stacks-node/src/nakamoto_node.rs

@@ -20,13 +20,6 @@ use std::sync::mpsc::Receiver;
 use std::thread;
 use std::thread::JoinHandle;
 
-use super::{Config, EventDispatcher, Keychain};
-use crate::burnchains::bitcoin_regtest_controller::addr2str;
-use crate::globals::Globals;
-use crate::globals::RelayerDirective;
-use crate::neon_node::LeaderKeyRegistrationState;
-use crate::run_loop::nakamoto::RunLoop;
-use crate::run_loop::RegisteredKey;
 use clarity::vm::ast::ASTRules;
 use clarity::vm::types::QualifiedContractIdentifier;
 use stacks::burnchains::{Burnchain, BurnchainSigner, Txid};
@@ -52,6 +45,13 @@ use stacks_common::types::StacksEpochId;
 use stacks_common::util::get_epoch_time_secs;
 use stacks_common::util::secp256k1::Secp256k1PrivateKey;
 
+use super::{Config, EventDispatcher, Keychain};
+use crate::burnchains::bitcoin_regtest_controller::addr2str;
+use crate::globals::{Globals, RelayerDirective};
+use crate::neon_node::LeaderKeyRegistrationState;
+use crate::run_loop::nakamoto::RunLoop;
+use crate::run_loop::RegisteredKey;
+
 pub mod miner;
 pub mod peer;
 pub mod relayer;

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

@@ -18,39 +18,35 @@ use std::thread;
 use std::thread::JoinHandle;
 use std::time::Instant;
 
-use super::relayer::RelayerThread;
-use super::Error as NakamotoNodeError;
-use super::{Config, EventDispatcher, Keychain};
-use crate::globals::Globals;
-use crate::mockamoto::signer::SelfSigner;
-use crate::nakamoto_node::VRF_MOCK_MINER_KEY;
-use crate::run_loop::RegisteredKey;
-use crate::ChainTip;
 use clarity::vm::types::PrincipalData;
 use stacks::burnchains::{Burnchain, BurnchainParameters};
 use stacks::chainstate::burn::db::sortdb::SortitionDB;
 use stacks::chainstate::burn::{BlockSnapshot, ConsensusHash};
 use stacks::chainstate::nakamoto::miner::{NakamotoBlockBuilder, NakamotoTenureStart};
 use stacks::chainstate::nakamoto::{NakamotoBlock, NakamotoChainState};
 use stacks::chainstate::stacks::db::{StacksChainState, StacksHeaderInfo};
-use stacks::chainstate::stacks::Error as ChainstateError;
-use stacks::chainstate::stacks::TenureChangeCause;
-use stacks::chainstate::stacks::TenureChangePayload;
-use stacks::chainstate::stacks::ThresholdSignature;
 use stacks::chainstate::stacks::{
-    CoinbasePayload, StacksTransaction, StacksTransactionSigner, TransactionAnchorMode,
+    CoinbasePayload, Error as ChainstateError, StacksTransaction, StacksTransactionSigner,
+    TenureChangeCause, TenureChangePayload, ThresholdSignature, TransactionAnchorMode,
     TransactionPayload, TransactionVersion,
 };
 use stacks::core::mempool::MemPoolDB;
 use stacks::core::FIRST_BURNCHAIN_CONSENSUS_HASH;
 use stacks::cost_estimates::metrics::UnitMetric;
 use stacks::cost_estimates::UnitEstimator;
 use stacks_common::types::chainstate::{StacksAddress, StacksBlockId};
-use stacks_common::types::PrivateKey;
-use stacks_common::types::StacksEpochId;
+use stacks_common::types::{PrivateKey, StacksEpochId};
 use stacks_common::util::hash::Hash160;
 use stacks_common::util::vrf::VRFProof;
 
+use super::relayer::RelayerThread;
+use super::{Config, Error as NakamotoNodeError, EventDispatcher, Keychain};
+use crate::globals::Globals;
+use crate::mockamoto::signer::SelfSigner;
+use crate::nakamoto_node::VRF_MOCK_MINER_KEY;
+use crate::run_loop::RegisteredKey;
+use crate::ChainTip;
+
 pub enum MinerDirective {
     /// The miner won sortition so they should begin a new tenure
     BeginTenure {

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

@@ -13,45 +13,32 @@
 //
 // 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 std::cmp;
 use std::collections::VecDeque;
-
 use std::default::Default;
 use std::net::SocketAddr;
 use std::sync::mpsc::TrySendError;
-
-use std::thread;
 use std::time::Duration;
+use std::{cmp, thread};
 
 use stacks::burnchains::db::BurnchainHeaderReader;
 use stacks::burnchains::PoxConstants;
 use stacks::chainstate::burn::db::sortdb::SortitionDB;
-
 use stacks::chainstate::stacks::db::StacksChainState;
 use stacks::chainstate::stacks::miner::signal_mining_blocked;
-
 use stacks::core::mempool::MemPoolDB;
-
 use stacks::cost_estimates::metrics::{CostMetric, UnitMetric};
 use stacks::cost_estimates::{CostEstimator, FeeEstimator, UnitEstimator};
-
 use stacks::net::dns::{DNSClient, DNSResolver};
 use stacks::net::p2p::PeerNetwork;
-
 use stacks::net::RPCHandlerArgs;
-
 use stacks_common::util::hash::Sha256Sum;
 
+use super::open_chainstate_with_faults;
 use crate::burnchains::make_bitcoin_indexer;
-use crate::globals::Globals;
-use crate::globals::RelayerDirective;
-
+use crate::globals::{Globals, RelayerDirective};
 use crate::run_loop::nakamoto::RunLoop;
-
 use crate::{Config, EventDispatcher};
 
-use super::open_chainstate_with_faults;
-
 /// Thread that runs the network state machine, handling both p2p and http requests.
 pub struct PeerThread {
     /// Node config

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

@@ -13,6 +13,11 @@
 //
 // 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 std::collections::HashMap;
+use std::sync::mpsc::{Receiver, RecvTimeoutError};
+use std::thread::JoinHandle;
+use std::time::{Duration, Instant};
+
 use stacks::burnchains::{Burnchain, Txid};
 use stacks::chainstate::burn::db::sortdb::SortitionDB;
 use stacks::chainstate::burn::operations::leader_block_commit::{
@@ -30,9 +35,9 @@ use stacks::chainstate::stacks::miner::{
     get_mining_spend_amount, signal_mining_blocked, signal_mining_ready,
 };
 use stacks::core::mempool::MemPoolDB;
-use stacks::core::FIRST_BURNCHAIN_CONSENSUS_HASH;
-use stacks::core::FIRST_STACKS_BLOCK_HASH;
-use stacks::core::STACKS_EPOCH_3_0_MARKER;
+use stacks::core::{
+    FIRST_BURNCHAIN_CONSENSUS_HASH, FIRST_STACKS_BLOCK_HASH, STACKS_EPOCH_3_0_MARKER,
+};
 use stacks::cost_estimates::metrics::UnitMetric;
 use stacks::cost_estimates::UnitEstimator;
 use stacks::monitoring::increment_stx_blocks_mined_counter;
@@ -46,21 +51,13 @@ use stacks_common::types::StacksEpochId;
 use stacks_common::util::get_epoch_time_ms;
 use stacks_common::util::hash::Hash160;
 use stacks_common::util::vrf::{VRFProof, VRFPublicKey};
-use std::collections::HashMap;
-use std::sync::mpsc::Receiver;
-use std::sync::mpsc::RecvTimeoutError;
-use std::thread::JoinHandle;
-use std::time::Duration;
-use std::time::Instant;
 
-use super::Error as NakamotoNodeError;
 use super::{
     fault_injection_skip_mining, open_chainstate_with_faults, BlockCommits, Config,
-    EventDispatcher, Keychain, BLOCK_PROCESSOR_STACK_SIZE,
+    Error as NakamotoNodeError, EventDispatcher, Keychain, BLOCK_PROCESSOR_STACK_SIZE,
 };
 use crate::burnchains::BurnchainController;
-use crate::globals::Globals;
-use crate::globals::RelayerDirective;
+use crate::globals::{Globals, RelayerDirective};
 use crate::nakamoto_node::miner::{BlockMinerThread, MinerDirective};
 use crate::neon_node::LeaderKeyRegistrationState;
 use crate::run_loop::nakamoto::RunLoop;
@@ -127,8 +124,9 @@ pub struct RelayerThread {
     ///  to check if it should issue a block commit or try to register a VRF key
     next_initiative: Instant,
     is_miner: bool,
-    /// This is the last snapshot in which the relayer committed
-    last_committed_at: Option<BlockSnapshot>,
+    /// This is the last snapshot in which the relayer committed, and the parent_tenure_id
+    ///  which was committed to
+    last_committed: Option<(BlockSnapshot, StacksBlockId)>,
 }
 
 impl RelayerThread {
@@ -193,7 +191,7 @@ impl RelayerThread {
             miner_thread: None,
             is_miner,
             next_initiative: Instant::now() + Duration::from_secs(10),
-            last_committed_at: None,
+            last_committed: None,
         }
     }
 
@@ -759,7 +757,10 @@ impl RelayerThread {
         );
 
         self.last_commits.insert(txid, ());
-        self.last_committed_at = Some(last_committed_at);
+        self.last_committed = Some((
+            last_committed_at,
+            StacksBlockId::new(&tenure_start_ch, &tenure_start_bh),
+        ));
         self.globals.counters.bump_naka_submitted_commits();
 
         Ok(())
@@ -800,7 +801,10 @@ impl RelayerThread {
             return None;
         };
 
-        let should_commit = if let Some(last_committed_at) = self.last_committed_at.as_ref() {
+        // check if the burnchain changed, if so, we should issue a commit.
+        //  if not, we may still want to update a commit if we've received a new tenure start block
+        let burnchain_changed = if let Some((last_committed_at, ..)) = self.last_committed.as_ref()
+        {
             // if the new sortition tip has a different consesus hash than the last commit,
             //  issue a new commit
             sort_tip.consensus_hash != last_committed_at.consensus_hash
@@ -820,37 +824,38 @@ impl RelayerThread {
             ));
         };
 
-        if should_commit {
-            // TODO: just use `get_block_header_by_consensus_hash`?
-            let first_block_hash = if chain_tip_header
-                .anchored_header
-                .as_stacks_nakamoto()
-                .is_some()
-            {
-                // if the parent block is a nakamoto block, find the starting block of its tenure
-                let Ok(Some(first_block)) =
-                    NakamotoChainState::get_nakamoto_tenure_start_block_header(
-                        self.chainstate_ref().db(),
-                        &chain_tip_header.consensus_hash,
-                    )
-                else {
-                    warn!("Failure getting the first block of tenure in order to assemble block commit";
-                          "tenure_consensus_hash" => %chain_tip_header.consensus_hash,
-                          "tip_block_hash" => %chain_tip_header.anchored_header.block_hash());
-                    return None;
-                };
-                first_block.anchored_header.block_hash()
+        // get the starting block of the chain tip's tenure
+        let Ok(Some(chain_tip_tenure_start)) =
+            NakamotoChainState::get_block_header_by_consensus_hash(
+                self.chainstate_ref().db(),
+                &chain_tip_header.consensus_hash,
+            )
+        else {
+            warn!("Failure getting the first block of tenure in order to assemble block commit";
+                  "tenure_consensus_hash" => %chain_tip_header.consensus_hash,
+                  "tip_block_hash" => %chain_tip_header.anchored_header.block_hash());
+            return None;
+        };
+
+        let chain_tip_tenure_id = chain_tip_tenure_start.index_block_hash();
+        let should_commit = burnchain_changed
+            || if let Some((_, last_committed_tenure_id)) = self.last_committed.as_ref() {
+                // if the tenure ID of the chain tip has changed, issue a new commit
+                last_committed_tenure_id != &chain_tip_tenure_id
             } else {
-                // otherwise the parent block is a epoch2 block, just return its hash directly
-                chain_tip_header.anchored_header.block_hash()
+                // should be unreachable, but either way, if
+                //  `self.last_committed` is None, we should issue a commit
+                true
             };
-            return Some(RelayerDirective::NakamotoTenureStartProcessed(
+
+        if should_commit {
+            Some(RelayerDirective::NakamotoTenureStartProcessed(
                 chain_tip_header.consensus_hash,
-                first_block_hash,
-            ));
+                chain_tip_header.anchored_header.block_hash(),
+            ))
+        } else {
+            None
         }
-
-        return None;
     }
 
     /// Main loop of the relayer.

testnet/stacks-node/src/neon_node.rs

@@ -208,8 +208,7 @@ use crate::burnchains::bitcoin_regtest_controller::{
     addr2str, BitcoinRegtestController, OngoingBlockCommit,
 };
 use crate::burnchains::make_bitcoin_indexer;
-use crate::globals::Globals;
-use crate::globals::RelayerDirective;
+use crate::globals::{Globals, RelayerDirective};
 use crate::run_loop::neon::RunLoop;
 use crate::run_loop::RegisteredKey;
 use crate::ChainTip;