chore: Address Aaron's comments

Author: jbencin

stacks-signer/src/signerdb.rs

@@ -39,15 +39,12 @@ use wsts::net::NonceRequest;
 pub struct BlockInfoV1 {
     /// The associated packet nonce request if we have one
     pub nonce_request: Option<NonceRequest>,
-    /// Whether this block is already being signed over
-    pub signed_over: bool,
 }
 
 impl From<NonceRequest> for BlockInfoV1 {
     fn from(value: NonceRequest) -> Self {
         Self {
             nonce_request: Some(value),
-            signed_over: true,
         }
     }
 }
@@ -65,23 +62,6 @@ pub enum ExtraBlockInfo {
 }
 
 impl ExtraBlockInfo {
-    /// Get `signed_over` if it exists
-    pub fn get_signed_over(&self) -> Option<bool> {
-        match self {
-            ExtraBlockInfo::None | ExtraBlockInfo::V0 => None,
-            ExtraBlockInfo::V1(v1) => Some(v1.signed_over),
-        }
-    }
-    /// Set `signed_over` if it exists
-    pub fn set_signed_over(&mut self, value: bool) -> Result<(), &str> {
-        match self {
-            ExtraBlockInfo::None | ExtraBlockInfo::V0 => Err("Field doesn't exist"),
-            ExtraBlockInfo::V1(v1) => {
-                v1.signed_over = value;
-                Ok(())
-            }
-        }
-    }
     /// Take `nonce_request` if it exists
     pub fn take_nonce_request(&mut self) -> Option<NonceRequest> {
         match self {
@@ -114,6 +94,8 @@ pub struct BlockInfo {
     pub vote: Option<NakamotoBlockVote>,
     /// Whether the block contents are valid
     pub valid: Option<bool>,
+    /// Whether this block is already being signed over
+    pub signed_over: bool,
     /// Time at which the proposal was received by this signer (epoch time in seconds)
     pub proposed_time: u64,
     /// Time at which the proposal was signed by this signer (epoch time in seconds)
@@ -132,6 +114,7 @@ impl From<BlockProposal> for BlockInfo {
             reward_cycle: value.reward_cycle,
             vote: None,
             valid: None,
+            signed_over: false,
             proposed_time: get_epoch_time_secs(),
             signed_self: None,
             signed_group: None,
@@ -144,15 +127,16 @@ impl BlockInfo {
     pub fn new_v1_with_request(block_proposal: BlockProposal, nonce_request: NonceRequest) -> Self {
         let mut block_info = BlockInfo::from(block_proposal);
         block_info.ext = ExtraBlockInfo::V1(BlockInfoV1::from(nonce_request));
+        block_info.signed_over = true;
         block_info
     }
 
     /// Mark this block as valid, signed over, and record a timestamp in the block info if it wasn't
     ///  already set.
     pub fn mark_signed_and_valid(&mut self) {
         self.valid = Some(true);
+        self.signed_over = true;
         self.signed_self.get_or_insert(get_epoch_time_secs());
-        _ = self.ext.set_signed_over(true);
     }
 
     /// Return the block's signer signature hash
@@ -407,7 +391,7 @@ impl SignerDb {
             serde_json::to_string(&block_info).expect("Unable to serialize block info");
         let hash = &block_info.signer_signature_hash();
         let block_id = &block_info.block.block_id();
-        let signed_over = &block_info.ext.get_signed_over().unwrap_or(false);
+        let signed_over = &block_info.signed_over;
         let vote = block_info
             .vote
             .as_ref()
@@ -604,8 +588,6 @@ mod tests {
         let db_path = tmp_db_path();
         let mut db = SignerDb::new(db_path).expect("Failed to create signer db");
         let (mut block_info, block_proposal) = create_block();
-        // We'll need the V1 data fields for this
-        block_info.ext = ExtraBlockInfo::V1(BlockInfoV1::default());
         db.insert_block(&block_info).unwrap();
 
         assert!(db

stacks-signer/src/v1/signer.rs

@@ -586,7 +586,7 @@ impl Signer {
                     .block_lookup(self.reward_cycle, &signer_signature_hash)
                     .unwrap_or_else(|_| Some(BlockInfo::from(block_proposal.clone())))
                     .unwrap_or_else(|| BlockInfo::from(block_proposal.clone()));
-                if block_info.ext.get_signed_over().unwrap_or(false) {
+                if block_info.signed_over {
                     debug!("{self}: Received a sign command for a block we are already signing over. Ignore it.");
                     return;
                 }
@@ -603,9 +603,7 @@ impl Signer {
                     Ok(msg) => {
                         let ack = self.stackerdb_manager.send_message_with_retry(msg.into());
                         debug!("{self}: ACK: {ack:?}",);
-                        block_info.ext.set_signed_over(true).unwrap_or_else(|e| {
-                            error!("{self}: `set_signed_over()` failed: {e:?}");
-                        });
+                        block_info.signed_over = true;
                         self.signer_db
                             .insert_block(&block_info)
                             .unwrap_or_else(|e| {
@@ -709,7 +707,7 @@ impl Signer {
             "{self}: Received a block validate response";
             "block_hash" => block_info.block.header.block_hash(),
             "valid" => block_info.valid,
-            "signed_over" => block_info.ext.get_signed_over(),
+            "signed_over" => block_info.signed_over,
         );
         self.signer_db
             .insert_block(&block_info)

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

@@ -89,7 +89,7 @@ use stacks_common::util::hash::{to_hex, Hash160, Sha512Trunc256Sum};
 use stacks_common::util::secp256k1::{MessageSignature, Secp256k1PrivateKey, Secp256k1PublicKey};
 use stacks_common::util::{get_epoch_time_secs, sleep_ms};
 use stacks_signer::chainstate::{ProposalEvalConfig, SortitionsView};
-use stacks_signer::signerdb::{BlockInfo, BlockInfoV1, ExtraBlockInfo, SignerDb};
+use stacks_signer::signerdb::{BlockInfo, ExtraBlockInfo, SignerDb};
 use wsts::net::Message;
 
 use super::bitcoin_regtest::BitcoinCoreController;
@@ -4714,13 +4714,11 @@ fn signer_chainstate() {
                 reward_cycle,
                 vote: None,
                 valid: Some(true),
+                signed_over: true,
                 proposed_time: get_epoch_time_secs(),
                 signed_self: None,
                 signed_group: None,
-                ext: ExtraBlockInfo::V1(BlockInfoV1 {
-                    nonce_request: None,
-                    signed_over: true,
-                }),
+                ext: ExtraBlockInfo::None,
             })
             .unwrap();
 
@@ -4791,13 +4789,11 @@ fn signer_chainstate() {
                 reward_cycle,
                 vote: None,
                 valid: Some(true),
+                signed_over: true,
                 proposed_time: get_epoch_time_secs(),
                 signed_self: None,
                 signed_group: None,
-                ext: ExtraBlockInfo::V1(BlockInfoV1 {
-                    nonce_request: None,
-                    signed_over: true,
-                }),
+                ext: ExtraBlockInfo::None,
             })
             .unwrap();