Merge branch 'develop' into fix/stackerdb-audit-coinfabrik

jcnelson · web-flow · commit f1d57a1b797f · 2024-12-04T15:09:08.000Z
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,6 +15,13 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
 
 ### Changed
 
+## [3.0.0.0.4]
+
+### Added
+
+### Changed
+
+- Use the same burn view loader in both block validation and block processing
 
 ## [3.0.0.0.3]
 
diff --git a/stacks-signer/CHANGELOG.md b/stacks-signer/CHANGELOG.md
@@ -11,6 +11,14 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
 
 ### Changed
 
+## [3.0.0.0.4.0]
+
+### Added
+
+### Changed
+
+- Use the same burn view loader in both block validation and block processing
+
 ## [3.0.0.0.3.0]
 
 ### Added
diff --git a/stackslib/src/chainstate/nakamoto/mod.rs b/stackslib/src/chainstate/nakamoto/mod.rs
@@ -1788,6 +1788,73 @@ impl NakamotoChainState {
         }
     }
 
+    /// Get the current burnchain view
+    /// This is either:
+    ///    (1)  set by the tenure change tx if one exists
+    ///    (2)  the same as parent block id
+    pub fn get_block_burn_view(
+        sort_db: &SortitionDB,
+        next_ready_block: &NakamotoBlock,
+        parent_header_info: &StacksHeaderInfo,
+    ) -> Result<ConsensusHash, ChainstateError> {
+        let burnchain_view = if let Some(tenure_change) = next_ready_block.get_tenure_tx_payload() {
+            if let Some(ref parent_burn_view) = parent_header_info.burn_view {
+                // check that the tenure_change's burn view descends from the parent
+                let parent_burn_view_sn = SortitionDB::get_block_snapshot_consensus(
+                    sort_db.conn(),
+                    parent_burn_view,
+                )?
+                .ok_or_else(|| {
+                    warn!(
+                        "Cannot process Nakamoto block: could not find parent block's burnchain view";
+                        "consensus_hash" => %next_ready_block.header.consensus_hash,
+                        "stacks_block_hash" => %next_ready_block.header.block_hash(),
+                        "stacks_block_id" => %next_ready_block.header.block_id(),
+                        "parent_block_id" => %next_ready_block.header.parent_block_id
+                    );
+                    ChainstateError::InvalidStacksBlock("Failed to load burn view of parent block ID".into())
+                })?;
+                let handle = sort_db.index_handle_at_ch(&tenure_change.burn_view_consensus_hash)?;
+                let connected_sort_id = get_ancestor_sort_id(&handle, parent_burn_view_sn.block_height, &handle.context.chain_tip)?
+                    .ok_or_else(|| {
+                        warn!(
+                            "Cannot process Nakamoto block: could not find parent block's burnchain view";
+                            "consensus_hash" => %next_ready_block.header.consensus_hash,
+                            "stacks_block_hash" => %next_ready_block.header.block_hash(),
+                            "stacks_block_id" => %next_ready_block.header.block_id(),
+                            "parent_block_id" => %next_ready_block.header.parent_block_id
+                        );
+                        ChainstateError::InvalidStacksBlock("Failed to load burn view of parent block ID".into())
+                    })?;
+                if connected_sort_id != parent_burn_view_sn.sortition_id {
+                    warn!(
+                        "Cannot process Nakamoto block: parent block's burnchain view does not connect to own burn view";
+                        "consensus_hash" => %next_ready_block.header.consensus_hash,
+                        "stacks_block_hash" => %next_ready_block.header.block_hash(),
+                        "stacks_block_id" => %next_ready_block.header.block_id(),
+                        "parent_block_id" => %next_ready_block.header.parent_block_id
+                    );
+                    return Err(ChainstateError::InvalidStacksBlock(
+                        "Does not connect to burn view of parent block ID".into(),
+                    ));
+                }
+            }
+            tenure_change.burn_view_consensus_hash
+        } else {
+            parent_header_info.burn_view.clone().ok_or_else(|| {
+                warn!(
+                    "Cannot process Nakamoto block: parent block does not have a burnchain view and current block has no tenure tx";
+                    "consensus_hash" => %next_ready_block.header.consensus_hash,
+                    "stacks_block_hash" => %next_ready_block.header.block_hash(),
+                    "stacks_block_id" => %next_ready_block.header.block_id(),
+                    "parent_block_id" => %next_ready_block.header.parent_block_id
+                );
+                ChainstateError::InvalidStacksBlock("Failed to load burn view of parent block ID".into())
+            })?
+        };
+        Ok(burnchain_view)
+    }
+
     /// Process the next ready block.
     /// If there exists a ready Nakamoto block, then this method returns Ok(Some(..)) with the
     /// receipt.  Otherwise, it returns Ok(None).
@@ -1920,62 +1987,8 @@ impl NakamotoChainState {
         //   this is either:
         //    (1)  set by the tenure change tx if one exists
         //    (2)  the same as parent block id
-
-        let burnchain_view = if let Some(tenure_change) = next_ready_block.get_tenure_tx_payload() {
-            if let Some(ref parent_burn_view) = parent_header_info.burn_view {
-                // check that the tenure_change's burn view descends from the parent
-                let parent_burn_view_sn = SortitionDB::get_block_snapshot_consensus(
-                    sort_db.conn(),
-                    parent_burn_view,
-                )?
-                .ok_or_else(|| {
-                    warn!(
-                        "Cannot process Nakamoto block: could not find parent block's burnchain view";
-                        "consensus_hash" => %next_ready_block.header.consensus_hash,
-                        "stacks_block_hash" => %next_ready_block.header.block_hash(),
-                        "stacks_block_id" => %next_ready_block.header.block_id(),
-                        "parent_block_id" => %next_ready_block.header.parent_block_id
-                    );
-                    ChainstateError::InvalidStacksBlock("Failed to load burn view of parent block ID".into())
-                })?;
-                let handle = sort_db.index_handle_at_ch(&tenure_change.burn_view_consensus_hash)?;
-                let connected_sort_id = get_ancestor_sort_id(&handle, parent_burn_view_sn.block_height, &handle.context.chain_tip)?
-                    .ok_or_else(|| {
-                        warn!(
-                            "Cannot process Nakamoto block: could not find parent block's burnchain view";
-                            "consensus_hash" => %next_ready_block.header.consensus_hash,
-                            "stacks_block_hash" => %next_ready_block.header.block_hash(),
-                            "stacks_block_id" => %next_ready_block.header.block_id(),
-                            "parent_block_id" => %next_ready_block.header.parent_block_id
-                        );
-                        ChainstateError::InvalidStacksBlock("Failed to load burn view of parent block ID".into())
-                    })?;
-                if connected_sort_id != parent_burn_view_sn.sortition_id {
-                    warn!(
-                        "Cannot process Nakamoto block: parent block's burnchain view does not connect to own burn view";
-                        "consensus_hash" => %next_ready_block.header.consensus_hash,
-                        "stacks_block_hash" => %next_ready_block.header.block_hash(),
-                        "stacks_block_id" => %next_ready_block.header.block_id(),
-                        "parent_block_id" => %next_ready_block.header.parent_block_id
-                    );
-                    return Err(ChainstateError::InvalidStacksBlock(
-                        "Does not connect to burn view of parent block ID".into(),
-                    ));
-                }
-            }
-            tenure_change.burn_view_consensus_hash
-        } else {
-            parent_header_info.burn_view.clone().ok_or_else(|| {
-                warn!(
-                    "Cannot process Nakamoto block: parent block does not have a burnchain view and current block has no tenure tx";
-                    "consensus_hash" => %next_ready_block.header.consensus_hash,
-                    "stacks_block_hash" => %next_ready_block.header.block_hash(),
-                    "stacks_block_id" => %next_ready_block.header.block_id(),
-                    "parent_block_id" => %next_ready_block.header.parent_block_id
-                );
-                ChainstateError::InvalidStacksBlock("Failed to load burn view of parent block ID".into())
-            })?
-        };
+        let burnchain_view =
+            Self::get_block_burn_view(sort_db, &next_ready_block, &parent_header_info)?;
         let Some(burnchain_view_sn) =
             SortitionDB::get_block_snapshot_consensus(sort_db.conn(), &burnchain_view)?
         else {
diff --git a/stackslib/src/net/api/postblock_proposal.rs b/stackslib/src/net/api/postblock_proposal.rs
@@ -374,9 +374,30 @@ impl NakamotoBlockProposal {
             });
         }
 
-        let sort_tip = SortitionDB::get_canonical_sortition_tip(sortdb.conn())?;
-        let burn_dbconn: SortitionHandleConn = sortdb.index_handle(&sort_tip);
-        let mut db_handle = sortdb.index_handle(&sort_tip);
+        // open sortition view to the current burn view.
+        // If the block has a TenureChange with an Extend cause, then the burn view is whatever is
+        // indicated in the TenureChange.
+        // Otherwise, it's the same as the block's parent's burn view.
+        let parent_stacks_header = NakamotoChainState::get_block_header(
+            chainstate.db(),
+            &self.block.header.parent_block_id,
+        )?
+        .ok_or_else(|| BlockValidateRejectReason {
+            reason_code: ValidateRejectCode::InvalidBlock,
+            reason: "Invalid parent block".into(),
+        })?;
+
+        let burn_view_consensus_hash =
+            NakamotoChainState::get_block_burn_view(sortdb, &self.block, &parent_stacks_header)?;
+        let sort_tip =
+            SortitionDB::get_block_snapshot_consensus(sortdb.conn(), &burn_view_consensus_hash)?
+                .ok_or_else(|| BlockValidateRejectReason {
+                    reason_code: ValidateRejectCode::NoSuchTenure,
+                    reason: "Failed to find sortition for block tenure".to_string(),
+                })?;
+
+        let burn_dbconn: SortitionHandleConn = sortdb.index_handle(&sort_tip.sortition_id);
+        let mut db_handle = sortdb.index_handle(&sort_tip.sortition_id);
 
         // (For the signer)
         // Verify that the block's tenure is on the canonical sortition history
@@ -413,14 +434,6 @@ impl NakamotoBlockProposal {
         )?;
 
         // Validate txs against chainstate
-        let parent_stacks_header = NakamotoChainState::get_block_header(
-            chainstate.db(),
-            &self.block.header.parent_block_id,
-        )?
-        .ok_or_else(|| BlockValidateRejectReason {
-            reason_code: ValidateRejectCode::InvalidBlock,
-            reason: "Invalid parent block".into(),
-        })?;
 
         // Validate the block's timestamp. It must be:
         // - Greater than the parent block's timestamp
diff --git a/stackslib/src/net/api/postblock_v3.rs b/stackslib/src/net/api/postblock_v3.rs
@@ -70,7 +70,7 @@ impl HttpRequest for RPCPostBlockRequestHandler {
     }
 
     fn path_regex(&self) -> Regex {
-        Regex::new(&format!("^{PATH}$")).unwrap()
+        Regex::new(&format!("^{}(/)?$", PATH.trim_end_matches('/'))).unwrap()
     }
 
     fn metrics_identifier(&self) -> &str {
diff --git a/stackslib/src/net/api/tests/postblock_v3.rs b/stackslib/src/net/api/tests/postblock_v3.rs
@@ -214,6 +214,66 @@ fn handle_req_accepted() {
     assert_eq!(resp.stacks_block_id, next_block_id);
 }
 
+#[test]
+fn handle_req_without_trailing_accepted() {
+    let addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 33333);
+    let path_without_slash: &str = "/v3/blocks/upload";
+    let observer = TestEventObserver::new();
+    let mut rpc_test = TestRPC::setup_nakamoto(function_name!(), &observer);
+    let (next_block, ..) = rpc_test.peer_1.single_block_tenure(
+        &rpc_test.privk1,
+        |_| {},
+        |burn_ops| {
+            rpc_test.peer_2.next_burnchain_block(burn_ops.clone());
+        },
+        |_| true,
+    );
+    let next_block_id = next_block.block_id();
+    let mut requests = vec![];
+
+    // post the block
+    requests.push(
+        StacksHttpRequest::new_for_peer(
+            addr.into(),
+            "POST".into(),
+            path_without_slash.into(),
+            HttpRequestContents::new().payload_stacks(&next_block),
+        )
+        .unwrap(),
+    );
+
+    // idempotent
+    requests.push(
+        StacksHttpRequest::new_for_peer(
+            addr.into(),
+            "POST".into(),
+            path_without_slash.into(),
+            HttpRequestContents::new().payload_stacks(&next_block),
+        )
+        .unwrap(),
+    );
+    let mut responses = rpc_test.run(requests);
+
+    let response = responses.remove(0);
+    info!(
+        "Response for the request that has the path without the last '/': {}",
+        std::str::from_utf8(&response.try_serialize().unwrap()).unwrap()
+    );
+
+    let resp = response.decode_stacks_block_accepted().unwrap();
+    assert_eq!(resp.accepted, true);
+    assert_eq!(resp.stacks_block_id, next_block_id);
+
+    let response = responses.remove(0);
+    info!(
+        "Response for the request that has the path without the last '/': {}",
+        std::str::from_utf8(&response.try_serialize().unwrap()).unwrap()
+    );
+    let resp = response.decode_stacks_block_accepted().unwrap();
+    assert_eq!(resp.accepted, false);
+    assert_eq!(resp.stacks_block_id, next_block_id);
+}
+
 #[test]
 fn handle_req_unknown_burn_block() {
     let addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 33333);
diff --git a/testnet/stacks-node/src/config.rs b/testnet/stacks-node/src/config.rs
@@ -53,6 +53,7 @@ use stacks::net::connection::ConnectionOptions;
 use stacks::net::{Neighbor, NeighborAddress, NeighborKey};
 use stacks::types::chainstate::BurnchainHeaderHash;
 use stacks::types::EpochList;
+use stacks::util::hash::to_hex;
 use stacks::util_lib::boot::boot_code_id;
 use stacks::util_lib::db::Error as DBError;
 use stacks_common::consts::SIGNER_SLOTS_PER_USER;
@@ -833,7 +834,12 @@ impl Config {
         }
 
         let miner = match config_file.miner {
-            Some(miner) => miner.into_config_default(miner_default_config)?,
+            Some(mut miner) => {
+                if miner.mining_key.is_none() && !node.seed.is_empty() {
+                    miner.mining_key = Some(to_hex(&node.seed));
+                }
+                miner.into_config_default(miner_default_config)?
+            }
             None => miner_default_config,
         };
 
@@ -2560,6 +2566,13 @@ pub struct MinerConfigFile {
 
 impl MinerConfigFile {
     fn into_config_default(self, miner_default_config: MinerConfig) -> Result<MinerConfig, String> {
+        match &self.mining_key {
+            Some(_) => {}
+            None => {
+                panic!("mining key not set");
+            }
+        }
+
         let mining_key = self
             .mining_key
             .as_ref()
diff --git a/testnet/stacks-node/src/nakamoto_node/miner.rs b/testnet/stacks-node/src/nakamoto_node/miner.rs
@@ -143,7 +143,11 @@ pub struct BlockMinerThread {
     registered_key: RegisteredKey,
     /// Burnchain block snapshot which elected this miner
     burn_election_block: BlockSnapshot,
-    /// Current burnchain tip
+    /// Current burnchain tip as of the last TenureChange
+    /// * if the last tenure-change was a BlockFound, then this is the same as the
+    /// `burn_election_block`.
+    /// * otherwise, if the last tenure-change is an Extend, then this is the sortition of the burn
+    /// view consensus hash in the TenureChange
     burn_block: BlockSnapshot,
     /// The start of the parent tenure for this tenure
     parent_tenure_id: StacksBlockId,
diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs
@@ -506,7 +506,7 @@ fn block_proposal_rejection() {
         signer_test.wait_for_validate_reject_response(short_timeout, block_signer_signature_hash_2);
     assert!(matches!(
         reject.reason_code,
-        ValidateRejectCode::UnknownParent
+        ValidateRejectCode::InvalidBlock
     ));
 
     let start_polling = Instant::now();
@@ -532,7 +532,10 @@ fn block_proposal_rejection() {
                     assert!(matches!(reason_code, RejectCode::SortitionViewMismatch));
                 } else if signer_signature_hash == block_signer_signature_hash_2 {
                     found_signer_signature_hash_2 = true;
-                    assert!(matches!(reason_code, RejectCode::ValidationFailed(_)));
+                    assert!(matches!(
+                        reason_code,
+                        RejectCode::ValidationFailed(ValidateRejectCode::InvalidBlock)
+                    ));
                 } else {
                     continue;
                 }

Original file line number	Diff line number	Diff line change
`@@ -70,7 +70,7 @@ impl HttpRequest for RPCPostBlockRequestHandler {`
`70`	`70`	`}`
`71`	`71`
`72`	`72`	`fn path_regex(&self) -> Regex {`
`73`		`- Regex::new(&format!("^{PATH}$")).unwrap()`
	`73`	`+ Regex::new(&format!("^{}(/)?$", PATH.trim_end_matches('/'))).unwrap()`
`74`	`74`	`}`
`75`	`75`
`76`	`76`	`fn metrics_identifier(&self) -> &str {`