Turn check for 0 timestamp in Block::new() into assert

api/src/convert/block.rs

@@ -17,13 +17,7 @@ impl From<&Block> for blockchain::Block {
         block.set_cumulative_txo_count(other.cumulative_txo_count);
         block.set_root_element((&other.root_element).into());
         block.set_contents_hash(blockchain::BlockContentsHash::from(&other.contents_hash));
-
-        // It should be 0 if timestamps are not supported, but being a
-        // publicly accessible field means anyone could set it, so we guard
-        // against that here in the conversion.
-        if block.version.timestamps_are_supported() {
-            block.set_timestamp(other.timestamp);
-        }
+        block.set_timestamp(other.timestamp);
         block
     }
 }
@@ -38,12 +32,6 @@ impl TryFrom<&blockchain::Block> for Block {
         let root_element = TxOutMembershipElement::try_from(value.get_root_element())?;
         let contents_hash = BlockContentsHash::try_from(value.get_contents_hash())?;
 
-        let timestamp = if value.get_version().timestamps_are_supported() {
-            value.get_timestamp()
-        } else {
-            0
-        };
-
         let block = Block {
             id: block_id,
             version: value.version,
@@ -52,7 +40,7 @@ impl TryFrom<&blockchain::Block> for Block {
             cumulative_txo_count: value.cumulative_txo_count,
             root_element,
             contents_hash,
-            timestamp,
+            timestamp: value.timestamp,
         };
         Ok(block)
     }

blockchain/test-utils/src/lib.rs

@@ -111,12 +111,17 @@ pub fn get_blocks_with_recipients<R: RngCore + CryptoRng>(
 
         let block = match &prev_block {
             Some(parent) => {
+                let timestamp = if block_version.timestamps_are_supported() {
+                    parent.timestamp + 1
+                } else {
+                    0
+                };
                 Block::new_with_parent(
                     block_version,
                     parent,
                     &Default::default(),
                     &block_contents,
-                    parent.timestamp + 1,
+                    timestamp,
                 )
             }
             None => Block::new_origin_block(&block_contents.outputs),

blockchain/types/src/block.rs

@@ -102,9 +102,9 @@ impl Block {
     /// * `version` - The block format version
     /// * `parent` - The parent block
     /// * `root_element` - The root element for membership proofs
-    /// * `block_contents - The Contents of the block.
-    /// * `timestamp` - The timestamp of the block in ms since Unix epoch. Only
-    ///    supported for BlockVersion 4 and higher.
+    /// * `block_contents` - The Contents of the block.
+    /// * `timestamp` - The timestamp of the block in ms since Unix epoch.
+    ///   should be 0 for block versions 3 and below.
     pub fn new_with_parent(
         version: BlockVersion,
         parent: &Block,
@@ -119,7 +119,6 @@ impl Block {
             parent.cumulative_txo_count + block_contents.outputs.len() as u64,
             root_element,
             block_contents,
-            // timestamp is normalized in Block::new()
             timestamp,
         )
     }
@@ -136,8 +135,8 @@ impl Block {
     ///   block*
     /// * `root_element` - The root element for membership proofs
     /// * `block_contents` - Contents of the block.
-    /// * `timestamp` - The timestamp of the block in ms since Unix epoch. Only
-    ///    supported for BlockVersion 4 and higher.
+    /// * `timestamp` - The timestamp of the block in ms since Unix epoch.
+    ///   should be 0 for block versions 3 and below.
     pub fn new(
         version: BlockVersion,
         parent_id: &BlockID,
@@ -147,12 +146,7 @@ impl Block {
         block_contents: &BlockContents,
         timestamp: u64,
     ) -> Self {
-
-        let timestamp = if version.timestamps_are_supported() {
-            timestamp
-        } else {
-            0
-        };
+        assert!(timestamp == 0 || version.timestamps_are_supported());
 
         let contents_hash = block_contents.hash();
         let id = compute_block_id(
@@ -499,7 +493,7 @@ mod block_tests {
     }
 
     #[test]
-    fn test_block_version_3_ignores_timestamp() {
+    fn test_block_version_3_ignores_timestamp_in_id() {
         let parent_id = BlockID::try_from(&[1u8; 32][..]).unwrap();
         let index = 1;
         let cumulative_txo_count = 1;
@@ -526,17 +520,6 @@ mod block_tests {
             timestamp_2,
         );
         assert_eq!(id_1, id_2);
-
-        let block = Block::new(
-            BlockVersion::THREE,
-            &parent_id,
-            index,
-            cumulative_txo_count,
-            &root_element,
-            &BlockContents::default(),
-            timestamp_1,
-        );
-        assert_eq!(block.timestamp, 0);
     }
 
     #[test]

ledger/db/src/test_utils/mod.rs

@@ -429,12 +429,18 @@ pub fn add_block_contents_to_ledger(
     let new_block = if num_blocks > 0 {
         let parent = ledger_db.get_block(num_blocks - 1)?;
 
+        let timestamp = if block_version.timestamps_are_supported() {
+            parent.timestamp + 1
+        } else {
+            0
+        };
+
         Block::new_with_parent(
             block_version,
             &parent,
             &Default::default(),
             &block_contents,
-            parent.timestamp + 1,
+            timestamp,
         )
     } else {
         Block::new_origin_block(&block_contents.outputs)