Fix empty cells fn.

Author: aterentic-ethernal

proptest-regressions/data.txt

@@ -0,0 +1,8 @@
+# Seeds for failure cases proptest has generated in the past. It is
+# automatically read and these particular cases re-run before any
+# novel cases are generated.
+#
+# It is recommended to check this file in to source control so that
+# everyone who runs the test benefits from these saved cases.
+cc 8e7c4f7fc5141690d2c174ad161c48f09353b9c00371f477869b046ae509851f # shrinks to rows = 96, cols = 683
+cc 0bf7bc7f3be889957f0f95c40241df0c9a8e05bcb684101765c7a11f187872fa # shrinks to matrix = [[None]]

src/client.rs

@@ -31,7 +31,7 @@ use rocksdb::{DBWithThreadMode, SingleThreaded};
 use crate::{
 	data::{
 		construct_matrix, decode_block_cid_ask_message, decode_block_cid_fact_message, empty_cells,
-		extract_block, extract_cell, extract_links, get_matrix, matrix_cells,
+		extract_block, extract_cell, extract_links, get_matrix, matrix_cells, non_empty_cells_len,
 		prepare_block_cid_ask_message, prepare_block_cid_fact_message, push_matrix,
 	},
 	rpc::get_cells,
@@ -414,17 +414,11 @@ pub async fn run_client(
 					vec![]
 				});
 
-				log::info!(
-					"Fetched {} cells from IPFS",
-					ipfs_cells
-						.iter()
-						.fold(0usize, |sum, val| sum + val.iter().flatten().count())
-				);
-
 				let requested_cells = empty_cells(&ipfs_cells, block.max_cols, block.max_rows);
 
 				log::info!(
-					"Requested {} cells from the full node",
+					"Got {} cells from IPFS, requesting {} from full node",
+					non_empty_cells_len(&ipfs_cells),
 					requested_cells.len()
 				);
 

src/data.rs

@@ -73,11 +73,24 @@ pub fn matrix_cells(rows: u16, cols: u16) -> impl Iterator<Item = (usize, usize)
 }
 
 pub fn empty_cells(matrix: &Matrix, cols: u16, rows: u16) -> Vec<(usize, usize)> {
+	// TODO: Optimal solution is to use proper Matrix abstraction to derive empty cells
 	matrix_cells(rows, cols)
-		.filter(|(row, col)| matrix.get(*col).and_then(|col| col.get(*row)).is_none())
+		.filter(|(row, col)| {
+			matrix
+				.get(*col)
+				.and_then(|col| col.get(*row))
+				.map(|cell| cell.is_none())
+				.unwrap_or(true)
+		})
 		.collect::<Vec<(usize, usize)>>()
 }
 
+pub fn non_empty_cells_len(matrix: &Matrix) -> usize {
+	matrix
+		.iter()
+		.fold(0usize, |sum, val| sum + val.iter().flatten().count())
+}
+
 fn get_cell(ipfs: &Ipfs<DefaultParams>, cid: &Cid) -> Result<Option<Cell>> {
 	ipfs.get(cid)
 		.and_then(|result| result.ipld())
@@ -451,40 +464,35 @@ mod tests {
 	}
 
 	fn matrix_strategy() -> impl Strategy<Value = Vec<Vec<Option<Vec<u8>>>>> {
+		let rows_len = (1..64usize).next().unwrap();
 		collection::vec(
-			collection::vec(any::<Option<Vec<u8>>>(), collection::size_range(1..64)),
+			collection::vec(any::<Option<Vec<u8>>>(), rows_len),
 			collection::size_range(1..64),
 		)
 	}
 
-	fn non_empty_cells(matrix: &Matrix, cols: u16, rows: u16) -> Vec<(usize, usize)> {
-		matrix_cells(rows as usize, cols as usize)
-			.filter(|(row, col)| matrix.get(*col).and_then(|col| col.get(*row)).is_some())
-			.collect::<Vec<(usize, usize)>>()
-	}
-
 	proptest! {
 		#[test]
-		fn matrix_cells_length_is_correct(rows in 0usize..1024, cols in 0usize..1024) {
-			prop_assert_eq!(matrix_cells(rows, cols).count(), rows * cols);
+		fn matrix_cells_length_is_correct(rows in 0u16..1024, cols in 0u16..1024) {
+			prop_assert_eq!(matrix_cells(rows, cols).count(), rows as usize * cols as usize);
 		}
 
 		#[test]
 		fn empty_cells_length_is_correct(matrix in matrix_strategy()) {
-			let cols = matrix.len() as u16;
-			let rows = matrix[0].len() as u16;
-			let empty_cells_len = empty_cells(&matrix, cols, rows).len() as u16;
-			let non_empty_cells_len = non_empty_cells(&matrix, cols, rows).len() as u16;
+			let cols = matrix.len() ;
+			let rows = matrix[0].len() ;
+			let empty_cells_len = empty_cells(&matrix, cols as u16, rows as u16).len();
+			let non_empty_cells_len =  non_empty_cells_len(&matrix);
 
-			prop_assert_eq!(empty_cells_len + non_empty_cells_len, rows * cols);
+			prop_assert_eq!(empty_cells_len + non_empty_cells_len, (rows * cols) as usize);
 		}
 	}
 
 	#[test_case(1, 1 => vec![(0, 0)] ; "one cell")]
 	#[test_case(4, 1 => vec![(0, 0), (1, 0), (2,0), (3,0)] ; "four rows, one column")]
 	#[test_case(1, 4 => vec![(0, 0), (0, 1), (0,2), (0,3)] ; "four columns, one row")]
-	#[test_case(2, 2 => vec![(0, 0), (0, 1), (1,0), (1,1)] ; "square matrix")]
-	fn test_matrix_cells(rows: usize, cols: usize) -> Vec<(usize, usize)> {
+	#[test_case(2, 2 => vec![(0, 0), (1, 0), (0,1), (1,1)] ; "square matrix")]
+	fn test_matrix_cells(rows: u16, cols: u16) -> Vec<(usize, usize)> {
 		matrix_cells(rows, cols).collect::<Vec<(usize, usize)>>()
 	}
 

src/main.rs

@@ -185,6 +185,7 @@ pub async fn do_main() -> Result<()> {
 
 				let begin = SystemTime::now();
 
+				// TODO: Setting max rows * 2 to match extended matrix dimensions
 				let max_rows = header.extrinsics_root.rows * 2;
 				let max_cols = header.extrinsics_root.cols;
 				if max_cols < 3 {

src/sync.rs

@@ -83,6 +83,7 @@ pub async fn sync_block_headers(
 
 					let begin = SystemTime::now();
 
+					// TODO: Setting max rows * 2 to match extended matrix dimensions
 					let max_rows = block_body.header.extrinsics_root.rows * 2;
 					let max_cols = block_body.header.extrinsics_root.cols;
 					let commitment = block_body.header.extrinsics_root.commitment;

src/types.rs

@@ -1,6 +1,5 @@
 extern crate ipfs_embed;
 
-use codec::Decode;
 use ipfs_embed::{Block as IpfsBlock, Cid, DefaultParams, Multiaddr, PeerId, StreamId};
 use serde::{Deserialize, Deserializer, Serialize};