refactor(signer): Add enum for version-specific data to BlockInfo

jbencin · jbencin · commit f6314e6b7190 · 2024-07-24T17:34:23.000-04:00
diff --git a/stacks-signer/src/signerdb.rs b/stacks-signer/src/signerdb.rs
@@ -34,6 +34,73 @@ use stacks_common::util::hash::Sha512Trunc256Sum;
 use stacks_common::{debug, error};
 use wsts::net::NonceRequest;
 
+#[derive(Serialize, Deserialize, Debug, PartialEq, Default)]
+/// Information specific to Signer V1
+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,
+        }
+    }
+}
+
+#[derive(Serialize, Deserialize, Debug, PartialEq, Default)]
+/// Store extra version-specific info in `BlockInfo`
+pub enum ExtraBlockInfo {
+    #[default]
+    /// Don't know what version
+    None,
+    /// Extra data for Signer V0
+    V0,
+    /// Extra data for Signer V1
+    V1(BlockInfoV1),
+}
+
+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 {
+            ExtraBlockInfo::None | ExtraBlockInfo::V0 => None,
+            ExtraBlockInfo::V1(v1) => v1.nonce_request.take(),
+        }
+    }
+    /// Set `nonce_request` if it exists
+    pub fn set_nonce_request(&mut self, value: NonceRequest) -> Result<(), &str> {
+        match self {
+            ExtraBlockInfo::None | ExtraBlockInfo::V0 => Err("Field doesn't exist"),
+            ExtraBlockInfo::V1(v1) => {
+                v1.nonce_request = Some(value);
+                Ok(())
+            }
+        }
+    }
+}
+
 /// Additional Info about a proposed block
 #[derive(Serialize, Deserialize, Debug, PartialEq)]
 pub struct BlockInfo {
@@ -47,16 +114,14 @@ pub struct BlockInfo {
     pub vote: Option<NakamotoBlockVote>,
     /// Whether the block contents are valid
     pub valid: Option<bool>,
-    /// 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,
     /// 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)
     pub signed_self: Option<u64>,
     /// Time at which the proposal was signed by a threshold in the signer set (epoch time in seconds)
     pub signed_group: Option<u64>,
+    /// Extra data specific to v0, v1, etc.
+    pub ext: ExtraBlockInfo,
 }
 
 impl From<BlockProposal> for BlockInfo {
@@ -67,31 +132,27 @@ impl From<BlockProposal> for BlockInfo {
             reward_cycle: value.reward_cycle,
             vote: None,
             valid: None,
-            nonce_request: None,
-            signed_over: false,
             proposed_time: get_epoch_time_secs(),
             signed_self: None,
             signed_group: None,
+            ext: ExtraBlockInfo::default(),
         }
     }
 }
 impl BlockInfo {
     /// Create a new BlockInfo with an associated nonce request packet
-    pub fn new_with_request(block_proposal: BlockProposal, nonce_request: NonceRequest) -> Self {
+    pub fn new_v1_with_request(block_proposal: BlockProposal, nonce_request: NonceRequest) -> Self {
         let mut block_info = BlockInfo::from(block_proposal);
-        block_info.nonce_request = Some(nonce_request);
-        block_info.signed_over = true;
+        block_info.ext = ExtraBlockInfo::V1(BlockInfoV1::from(nonce_request));
         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;
-        if self.signed_self.is_none() {
-            self.signed_self = Some(get_epoch_time_secs());
-        }
+        self.signed_self.get_or_insert(get_epoch_time_secs());
+        _ = self.ext.set_signed_over(true);
     }
 
     /// Return the block's signer signature hash
@@ -115,7 +176,7 @@ CREATE TABLE IF NOT EXISTS blocks (
     block_info TEXT NOT NULL,
     consensus_hash TEXT NOT NULL,
     signed_over INTEGER NOT NULL,
-    stacks_height INTEGER NOT NULL, 
+    stacks_height INTEGER NOT NULL,
     burn_block_height INTEGER NOT NULL,
     PRIMARY KEY (reward_cycle, signer_signature_hash)
 ) STRICT";
@@ -189,7 +250,7 @@ impl SignerDb {
             })
             .optional();
         match result {
-            Ok(x) => Ok(x.unwrap_or_else(|| 0)),
+            Ok(x) => Ok(x.unwrap_or(0)),
             Err(e) => Err(DBError::from(e)),
         }
     }
@@ -346,7 +407,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.signed_over;
+        let signed_over = &block_info.ext.get_signed_over().unwrap_or(false);
         let vote = block_info
             .vote
             .as_ref()
@@ -543,6 +604,8 @@ 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
diff --git a/stacks-signer/src/v1/signer.rs b/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.signed_over {
+                if block_info.ext.get_signed_over().unwrap_or(false) {
                     debug!("{self}: Received a sign command for a block we are already signing over. Ignore it.");
                     return;
                 }
@@ -603,7 +603,9 @@ impl Signer {
                     Ok(msg) => {
                         let ack = self.stackerdb_manager.send_message_with_retry(msg.into());
                         debug!("{self}: ACK: {ack:?}",);
-                        block_info.signed_over = true;
+                        block_info.ext.set_signed_over(true).unwrap_or_else(|e| {
+                            error!("{self}: `set_signed_over()` failed: {e:?}");
+                        });
                         self.signer_db
                             .insert_block(&block_info)
                             .unwrap_or_else(|e| {
@@ -692,7 +694,7 @@ impl Signer {
                 block_info
             }
         };
-        if let Some(mut nonce_request) = block_info.nonce_request.take() {
+        if let Some(mut nonce_request) = block_info.ext.take_nonce_request() {
             debug!("{self}: Received a block validate response from the stacks node for a block we already received a nonce request for. Responding to the nonce request...");
             // We have received validation from the stacks node. Determine our vote and update the request message
             self.determine_vote(&mut block_info, &mut nonce_request);
@@ -707,7 +709,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.signed_over,
+            "signed_over" => block_info.ext.get_signed_over(),
         );
         self.signer_db
             .insert_block(&block_info)
@@ -916,7 +918,7 @@ impl Signer {
                 "{self}: received a nonce request for a new block. Submit block for validation. ";
                 "signer_sighash" => %signer_signature_hash,
             );
-            let block_info = BlockInfo::new_with_request(block_proposal, nonce_request.clone());
+            let block_info = BlockInfo::new_v1_with_request(block_proposal, nonce_request.clone());
             stacks_client
                 .submit_block_for_validation(block_info.block.clone())
                 .unwrap_or_else(|e| {
@@ -928,7 +930,12 @@ impl Signer {
         if block_info.valid.is_none() {
             // We have not yet received validation from the stacks node. Cache the request and wait for validation
             debug!("{self}: We have yet to receive validation from the stacks node for a nonce request. Cache the nonce request and wait for block validation...");
-            block_info.nonce_request = Some(nonce_request.clone());
+            block_info
+                .ext
+                .set_nonce_request(nonce_request.clone())
+                .unwrap_or_else(|e| {
+                    warn!("{self}: Failed to set nonce_request: {e:?}",);
+                });
             return Some(block_info);
         }
 
diff --git a/testnet/stacks-node/src/tests/nakamoto_integrations.rs b/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, SignerDb};
+use stacks_signer::signerdb::{BlockInfo, BlockInfoV1, ExtraBlockInfo, SignerDb};
 use wsts::net::Message;
 
 use super::bitcoin_regtest::BitcoinCoreController;
@@ -4714,11 +4714,13 @@ fn signer_chainstate() {
                 reward_cycle,
                 vote: None,
                 valid: Some(true),
-                nonce_request: None,
-                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,
+                }),
             })
             .unwrap();
 
@@ -4789,11 +4791,13 @@ fn signer_chainstate() {
                 reward_cycle,
                 vote: None,
                 valid: Some(true),
-                nonce_request: None,
-                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,
+                }),
             })
             .unwrap();
 
