From 844e56bff343976e53dd259c6a9f36f16a22604a Mon Sep 17 00:00:00 2001 From: Lldenaurois Date: Thu, 24 Jun 2021 18:41:16 -0400 Subject: [PATCH] node/approval-voting: Get all module tests to pass. This commit modifies all tests to ensure tests are passing. --- .../approval-voting/src/approval_db/v1/mod.rs | 4 +- .../src/approval_db/v1/tests.rs | 280 +++++++------ node/core/approval-voting/src/backend.rs | 1 + node/core/approval-voting/src/import.rs | 72 +--- node/core/approval-voting/src/lib.rs | 8 +- node/core/approval-voting/src/tests.rs | 373 +++++++++++------- 6 files changed, 413 insertions(+), 325 deletions(-) diff --git a/node/core/approval-voting/src/approval_db/v1/mod.rs b/node/core/approval-voting/src/approval_db/v1/mod.rs index 7c252a3596b5..f09ebd2cc6eb 100644 --- a/node/core/approval-voting/src/approval_db/v1/mod.rs +++ b/node/core/approval-voting/src/approval_db/v1/mod.rs @@ -30,8 +30,8 @@ use bitvec::{vec::BitVec, order::Lsb0 as BitOrderLsb0}; pub(crate) const STORED_BLOCKS_KEY: &[u8] = b"Approvals_StoredBlocks"; -//#[cfg(test)] -//pub mod tests; +#[cfg(test)] +pub mod tests; // slot_duration * 2 + DelayTranche gives the number of delay tranches since the // unix epoch. diff --git a/node/core/approval-voting/src/approval_db/v1/tests.rs b/node/core/approval-voting/src/approval_db/v1/tests.rs index 71c4d3c47e29..c8d666971cb2 100644 --- a/node/core/approval-voting/src/approval_db/v1/tests.rs +++ b/node/core/approval-voting/src/approval_db/v1/tests.rs @@ -17,7 +17,15 @@ //! Tests for the aux-schema of approval voting. use super::*; +use std::sync::Arc; +use std::collections::HashMap; use polkadot_primitives::v1::Id as ParaId; +use kvdb::KeyValueDB; +use crate::backend::{Backend, DbBackend, OverlayedBackend}; +use crate::ops::{ + NewCandidateInfo, StoredBlockRange, load_block_entry, load_candidate_entry, load_stored_blocks, + load_all_blocks, load_blocks_at_height, add_block_entry, force_approve, canonicalize, +}; const DATA_COL: u32 = 0; const NUM_COLUMNS: u32 = 1; @@ -26,36 +34,35 @@ const TEST_CONFIG: Config = Config { col_data: DATA_COL, }; -pub(crate) fn write_stored_blocks(tx: &mut DBTransaction, range: StoredBlockRange) { - tx.put_vec( - DATA_COL, - &STORED_BLOCKS_KEY[..], - range.encode(), - ); +fn make_db() -> (DbBackend, Arc) { + let db_writer: Arc = Arc::new(kvdb_memorydb::create(NUM_COLUMNS)); + (DbBackend::new(db_writer.clone(), TEST_CONFIG), db_writer) } -pub(crate) fn write_blocks_at_height(tx: &mut DBTransaction, height: BlockNumber, blocks: &[Hash]) { - tx.put_vec( - DATA_COL, - &blocks_at_height_key(height)[..], - blocks.encode(), - ); +pub(crate) fn write_stored_blocks(db: &mut OverlayedBackend<'_, impl Backend>, range: StoredBlockRange) { + db.write_stored_block_range(range); } -pub(crate) fn write_block_entry(tx: &mut DBTransaction, block_hash: &Hash, entry: &BlockEntry) { - tx.put_vec( - DATA_COL, - &block_entry_key(block_hash)[..], - entry.encode(), - ); +pub(crate) fn write_blocks_at_height( + db: &mut OverlayedBackend<'_, impl Backend>, + height: BlockNumber, + blocks: &[Hash]) +{ + db.write_blocks_at_height(height, blocks.to_vec()); } -pub(crate) fn write_candidate_entry(tx: &mut DBTransaction, candidate_hash: &CandidateHash, entry: &CandidateEntry) { - tx.put_vec( - DATA_COL, - &candidate_entry_key(candidate_hash)[..], - entry.encode(), - ); +pub(crate) fn write_block_entry( + db: &mut OverlayedBackend<'_, impl Backend>, + entry: &BlockEntry) +{ + db.write_block_entry(entry.clone().into()); +} + +pub(crate) fn write_candidate_entry( + db: &mut OverlayedBackend<'_, impl Backend>, + entry: &CandidateEntry) +{ + db.write_candidate_entry(entry.clone().into()); } fn make_bitvec(len: usize) -> BitVec { @@ -92,11 +99,11 @@ fn make_candidate(para_id: ParaId, relay_parent: Hash) -> CandidateReceipt { #[test] fn read_write() { - let store = kvdb_memorydb::create(NUM_COLUMNS); + let (mut db, store) = make_db(); let hash_a = Hash::repeat_byte(1); let hash_b = Hash::repeat_byte(2); - let candidate_hash = CandidateHash(Hash::repeat_byte(3)); + let candidate_hash = CandidateReceipt::::default().hash(); let range = StoredBlockRange(10, 20); let at_height = vec![hash_a, hash_b]; @@ -124,53 +131,48 @@ fn read_write() { approvals: Default::default(), }; - let mut tx = DBTransaction::new(); - - write_stored_blocks(&mut tx, range.clone()); - write_blocks_at_height(&mut tx, 1, &at_height); - write_block_entry(&mut tx, &hash_a, &block_entry); - write_candidate_entry(&mut tx, &candidate_hash, &candidate_entry); + let mut overlay_db = OverlayedBackend::new(&db); + write_stored_blocks(&mut overlay_db, range.clone()); + write_blocks_at_height(&mut overlay_db, 1, &at_height); + write_block_entry(&mut overlay_db, &block_entry); + write_candidate_entry(&mut overlay_db, &candidate_entry); - store.write(tx).unwrap(); + let write_ops = overlay_db.into_write_ops(); + db.write(write_ops).unwrap(); - assert_eq!(load_stored_blocks(&store, &TEST_CONFIG).unwrap(), Some(range)); - assert_eq!(load_blocks_at_height(&store, &TEST_CONFIG, 1).unwrap(), at_height); - assert_eq!(load_block_entry(&store, &TEST_CONFIG, &hash_a).unwrap(), Some(block_entry)); + assert_eq!(load_stored_blocks(store.as_ref(), &TEST_CONFIG).unwrap(), Some(range)); + assert_eq!(load_blocks_at_height(store.as_ref(), &TEST_CONFIG, &1).unwrap(), at_height); + assert_eq!(load_block_entry(store.as_ref(), &TEST_CONFIG, &hash_a).unwrap(), Some(block_entry.into())); assert_eq!( - load_candidate_entry(&store, &TEST_CONFIG, &candidate_hash).unwrap(), - Some(candidate_entry), + load_candidate_entry(store.as_ref(), &TEST_CONFIG, &candidate_hash).unwrap(), + Some(candidate_entry.into()), ); - let delete_keys = vec![ - STORED_BLOCKS_KEY.to_vec(), - blocks_at_height_key(1).to_vec(), - block_entry_key(&hash_a).to_vec(), - candidate_entry_key(&candidate_hash).to_vec(), - ]; - - let mut tx = DBTransaction::new(); - for key in delete_keys { - tx.delete(DATA_COL, &key[..]); - } - - store.write(tx).unwrap(); + let mut overlay_db = OverlayedBackend::new(&db); + overlay_db.delete_blocks_at_height(1); + overlay_db.delete_block_entry(&hash_a); + overlay_db.delete_candidate_entry(&candidate_hash); + let write_ops = overlay_db.into_write_ops(); + db.write(write_ops).unwrap(); - assert!(load_stored_blocks(&store, &TEST_CONFIG).unwrap().is_none()); - assert!(load_blocks_at_height(&store, &TEST_CONFIG, 1).unwrap().is_empty()); - assert!(load_block_entry(&store, &TEST_CONFIG, &hash_a).unwrap().is_none()); - assert!(load_candidate_entry(&store, &TEST_CONFIG, &candidate_hash).unwrap().is_none()); + assert!(load_blocks_at_height(store.as_ref(), &TEST_CONFIG, &1).unwrap().is_empty()); + assert!(load_block_entry(store.as_ref(), &TEST_CONFIG, &hash_a).unwrap().is_none()); + assert!(load_candidate_entry(store.as_ref(), &TEST_CONFIG, &candidate_hash).unwrap().is_none()); } #[test] fn add_block_entry_works() { - let store = kvdb_memorydb::create(NUM_COLUMNS); + let (mut db, store) = make_db(); let parent_hash = Hash::repeat_byte(1); let block_hash_a = Hash::repeat_byte(2); let block_hash_b = Hash::repeat_byte(69); - let candidate_hash_a = CandidateHash(Hash::repeat_byte(3)); - let candidate_hash_b = CandidateHash(Hash::repeat_byte(4)); + let candidate_receipt_a = make_candidate(1.into(), parent_hash); + let candidate_receipt_b = make_candidate(2.into(), parent_hash); + + let candidate_hash_a = candidate_receipt_a.hash(); + let candidate_hash_b = candidate_receipt_b.hash(); let block_number = 10; @@ -192,48 +194,52 @@ fn add_block_entry_works() { let mut new_candidate_info = HashMap::new(); new_candidate_info.insert(candidate_hash_a, NewCandidateInfo { - candidate: make_candidate(1.into(), parent_hash), + candidate: candidate_receipt_a, backing_group: GroupIndex(0), our_assignment: None, }); + let mut overlay_db = OverlayedBackend::new(&db); add_block_entry( - &store, - &TEST_CONFIG, - block_entry_a.clone(), + &mut overlay_db, + block_entry_a.clone().into(), n_validators, |h| new_candidate_info.get(h).map(|x| x.clone()), ).unwrap(); + let write_ops = overlay_db.into_write_ops(); + db.write(write_ops).unwrap(); new_candidate_info.insert(candidate_hash_b, NewCandidateInfo { - candidate: make_candidate(2.into(), parent_hash), + candidate: candidate_receipt_b, backing_group: GroupIndex(1), our_assignment: None, }); + let mut overlay_db = OverlayedBackend::new(&db); add_block_entry( - &store, - &TEST_CONFIG, - block_entry_b.clone(), + &mut overlay_db, + block_entry_b.clone().into(), n_validators, |h| new_candidate_info.get(h).map(|x| x.clone()), ).unwrap(); + let write_ops = overlay_db.into_write_ops(); + db.write(write_ops).unwrap(); - assert_eq!(load_block_entry(&store, &TEST_CONFIG, &block_hash_a).unwrap(), Some(block_entry_a)); - assert_eq!(load_block_entry(&store, &TEST_CONFIG, &block_hash_b).unwrap(), Some(block_entry_b)); + assert_eq!(load_block_entry(store.as_ref(), &TEST_CONFIG, &block_hash_a).unwrap(), Some(block_entry_a.into())); + assert_eq!(load_block_entry(store.as_ref(), &TEST_CONFIG, &block_hash_b).unwrap(), Some(block_entry_b.into())); - let candidate_entry_a = load_candidate_entry(&store, &TEST_CONFIG, &candidate_hash_a) + let candidate_entry_a = load_candidate_entry(store.as_ref(), &TEST_CONFIG, &candidate_hash_a) .unwrap().unwrap(); assert_eq!(candidate_entry_a.block_assignments.keys().collect::>(), vec![&block_hash_a, &block_hash_b]); - let candidate_entry_b = load_candidate_entry(&store, &TEST_CONFIG, &candidate_hash_b) + let candidate_entry_b = load_candidate_entry(store.as_ref(), &TEST_CONFIG, &candidate_hash_b) .unwrap().unwrap(); assert_eq!(candidate_entry_b.block_assignments.keys().collect::>(), vec![&block_hash_b]); } #[test] fn add_block_entry_adds_child() { - let store = kvdb_memorydb::create(NUM_COLUMNS); + let (mut db, store) = make_db(); let parent_hash = Hash::repeat_byte(1); let block_hash_a = Hash::repeat_byte(2); @@ -255,31 +261,33 @@ fn add_block_entry_adds_child() { let n_validators = 10; + let mut overlay_db = OverlayedBackend::new(&db); add_block_entry( - &store, - &TEST_CONFIG, - block_entry_a.clone(), + &mut overlay_db, + block_entry_a.clone().into(), n_validators, |_| None, ).unwrap(); add_block_entry( - &store, - &TEST_CONFIG, - block_entry_b.clone(), + &mut overlay_db, + block_entry_b.clone().into(), n_validators, |_| None, ).unwrap(); + let write_ops = overlay_db.into_write_ops(); + db.write(write_ops).unwrap(); + block_entry_a.children.push(block_hash_b); - assert_eq!(load_block_entry(&store, &TEST_CONFIG, &block_hash_a).unwrap(), Some(block_entry_a)); - assert_eq!(load_block_entry(&store, &TEST_CONFIG, &block_hash_b).unwrap(), Some(block_entry_b)); + assert_eq!(load_block_entry(store.as_ref(), &TEST_CONFIG, &block_hash_a).unwrap(), Some(block_entry_a.into())); + assert_eq!(load_block_entry(store.as_ref(), &TEST_CONFIG, &block_hash_b).unwrap(), Some(block_entry_b.into())); } #[test] fn canonicalize_works() { - let store = kvdb_memorydb::create(NUM_COLUMNS); + let (mut db, store) = make_db(); // -> B1 -> C1 -> D1 // A -> B2 -> C2 -> D2 @@ -296,9 +304,10 @@ fn canonicalize_works() { let n_validators = 10; - let mut tx = DBTransaction::new(); - write_stored_blocks(&mut tx, StoredBlockRange(1, 5)); - store.write(tx).unwrap(); + let mut overlay_db = OverlayedBackend::new(&db); + write_stored_blocks(&mut overlay_db, StoredBlockRange(1, 5)); + let write_ops = overlay_db.into_write_ops(); + db.write(write_ops).unwrap(); let genesis = Hash::repeat_byte(0); @@ -310,11 +319,17 @@ fn canonicalize_works() { let block_hash_d1 = Hash::repeat_byte(6); let block_hash_d2 = Hash::repeat_byte(7); - let cand_hash_1 = CandidateHash(Hash::repeat_byte(10)); - let cand_hash_2 = CandidateHash(Hash::repeat_byte(11)); - let cand_hash_3 = CandidateHash(Hash::repeat_byte(12)); - let cand_hash_4 = CandidateHash(Hash::repeat_byte(13)); - let cand_hash_5 = CandidateHash(Hash::repeat_byte(15)); + let candidate_receipt_genesis = make_candidate(1.into(), genesis); + let candidate_receipt_a = make_candidate(2.into(), block_hash_a); + let candidate_receipt_b = make_candidate(3.into(), block_hash_a); + let candidate_receipt_b1 = make_candidate(4.into(), block_hash_b1); + let candidate_receipt_c1 = make_candidate(5.into(), block_hash_c1); + + let cand_hash_1 = candidate_receipt_genesis.hash(); + let cand_hash_2 = candidate_receipt_a.hash(); + let cand_hash_3 = candidate_receipt_b.hash(); + let cand_hash_4 = candidate_receipt_b1.hash(); + let cand_hash_5 = candidate_receipt_c1.hash(); let block_entry_a = make_block_entry(block_hash_a, genesis, 1, Vec::new()); let block_entry_b1 = make_block_entry(block_hash_b1, block_hash_a, 2, Vec::new()); @@ -344,35 +359,34 @@ fn canonicalize_works() { vec![(CoreIndex(0), cand_hash_5)], ); - let candidate_info = { let mut candidate_info = HashMap::new(); candidate_info.insert(cand_hash_1, NewCandidateInfo { - candidate: make_candidate(1.into(), genesis), + candidate: candidate_receipt_genesis, backing_group: GroupIndex(1), our_assignment: None, }); candidate_info.insert(cand_hash_2, NewCandidateInfo { - candidate: make_candidate(2.into(), block_hash_a), + candidate: candidate_receipt_a, backing_group: GroupIndex(2), our_assignment: None, }); candidate_info.insert(cand_hash_3, NewCandidateInfo { - candidate: make_candidate(3.into(), block_hash_a), + candidate: candidate_receipt_b, backing_group: GroupIndex(3), our_assignment: None, }); candidate_info.insert(cand_hash_4, NewCandidateInfo { - candidate: make_candidate(4.into(), block_hash_b1), + candidate: candidate_receipt_b1, backing_group: GroupIndex(4), our_assignment: None, }); candidate_info.insert(cand_hash_5, NewCandidateInfo { - candidate: make_candidate(5.into(), block_hash_c1), + candidate: candidate_receipt_c1, backing_group: GroupIndex(5), our_assignment: None, }); @@ -391,25 +405,27 @@ fn canonicalize_works() { block_entry_d2.clone(), ]; + let mut overlay_db = OverlayedBackend::new(&db); for block_entry in blocks { add_block_entry( - &store, - &TEST_CONFIG, - block_entry, + &mut overlay_db, + block_entry.into(), n_validators, |h| candidate_info.get(h).map(|x| x.clone()), ).unwrap(); } + let write_ops = overlay_db.into_write_ops(); + db.write(write_ops).unwrap(); let check_candidates_in_store = |expected: Vec<(CandidateHash, Option>)>| { for (c_hash, in_blocks) in expected { let (entry, in_blocks) = match in_blocks { None => { - assert!(load_candidate_entry(&store, &TEST_CONFIG, &c_hash).unwrap().is_none()); + assert!(load_candidate_entry(store.as_ref(), &TEST_CONFIG, &c_hash).unwrap().is_none()); continue } Some(i) => ( - load_candidate_entry(&store, &TEST_CONFIG, &c_hash).unwrap().unwrap(), + load_candidate_entry(store.as_ref(), &TEST_CONFIG, &c_hash).unwrap().unwrap(), i, ), }; @@ -426,19 +442,19 @@ fn canonicalize_works() { for (hash, with_candidates) in expected { let (entry, with_candidates) = match with_candidates { None => { - assert!(load_block_entry(&store, &TEST_CONFIG, &hash).unwrap().is_none()); + assert!(load_block_entry(store.as_ref(), &TEST_CONFIG, &hash).unwrap().is_none()); continue } Some(i) => ( - load_block_entry(&store, &TEST_CONFIG, &hash).unwrap().unwrap(), + load_block_entry(store.as_ref(), &TEST_CONFIG, &hash).unwrap().unwrap(), i, ), }; - assert_eq!(entry.candidates.len(), with_candidates.len()); + assert_eq!(entry.candidates().len(), with_candidates.len()); for x in with_candidates { - assert!(entry.candidates.iter().position(|&(_, ref c)| c == &x).is_some()); + assert!(entry.candidates().iter().position(|&(_, ref c)| c == &x).is_some()); } } }; @@ -461,9 +477,12 @@ fn canonicalize_works() { (block_hash_d2, Some(vec![cand_hash_5])), ]); - canonicalize(&store, &TEST_CONFIG, 3, block_hash_c1).unwrap(); + let mut overlay_db = OverlayedBackend::new(&db); + canonicalize(&mut overlay_db, 3, block_hash_c1).unwrap(); + let write_ops = overlay_db.into_write_ops(); + db.write(write_ops).unwrap(); - assert_eq!(load_stored_blocks(&store, &TEST_CONFIG).unwrap().unwrap(), StoredBlockRange(4, 5)); + assert_eq!(load_stored_blocks(store.as_ref(), &TEST_CONFIG).unwrap().unwrap(), StoredBlockRange(4, 5)); check_candidates_in_store(vec![ (cand_hash_1, None), @@ -486,12 +505,13 @@ fn canonicalize_works() { #[test] fn force_approve_works() { - let store = kvdb_memorydb::create(NUM_COLUMNS); + let (mut db, store) = make_db(); let n_validators = 10; - let mut tx = DBTransaction::new(); - write_stored_blocks(&mut tx, StoredBlockRange(1, 4)); - store.write(tx).unwrap(); + let mut overlay_db = OverlayedBackend::new(&db); + write_stored_blocks(&mut overlay_db, StoredBlockRange(1, 4)); + let write_ops = overlay_db.into_write_ops(); + db.write(write_ops).unwrap(); let candidate_hash = CandidateHash(Hash::repeat_byte(42)); let single_candidate_vec = vec![(CoreIndex(0), candidate_hash)]; @@ -524,35 +544,36 @@ fn force_approve_works() { block_entry_d.clone(), ]; + let mut overlay_db = OverlayedBackend::new(&db); for block_entry in blocks { add_block_entry( - &store, - &TEST_CONFIG, - block_entry, + &mut overlay_db, + block_entry.into(), n_validators, |h| candidate_info.get(h).map(|x| x.clone()), ).unwrap(); } - - force_approve(&store, TEST_CONFIG, block_hash_d, 2).unwrap(); + force_approve(&mut overlay_db, block_hash_d, 2).unwrap(); + let write_ops = overlay_db.into_write_ops(); + db.write(write_ops).unwrap(); assert!(load_block_entry( - &store, + store.as_ref(), &TEST_CONFIG, &block_hash_a, ).unwrap().unwrap().approved_bitfield.all()); assert!(load_block_entry( - &store, + store.as_ref(), &TEST_CONFIG, &block_hash_b, ).unwrap().unwrap().approved_bitfield.all()); assert!(load_block_entry( - &store, + store.as_ref(), &TEST_CONFIG, &block_hash_c, ).unwrap().unwrap().approved_bitfield.not_any()); assert!(load_block_entry( - &store, + store.as_ref(), &TEST_CONFIG, &block_hash_d, ).unwrap().unwrap().approved_bitfield.not_any()); @@ -560,7 +581,7 @@ fn force_approve_works() { #[test] fn load_all_blocks_works() { - let store = kvdb_memorydb::create(NUM_COLUMNS); + let (mut db, store) = make_db(); let parent_hash = Hash::repeat_byte(1); let block_hash_a = Hash::repeat_byte(2); @@ -592,34 +613,35 @@ fn load_all_blocks_works() { let n_validators = 10; + let mut overlay_db = OverlayedBackend::new(&db); add_block_entry( - &store, - &TEST_CONFIG, - block_entry_a.clone(), + &mut overlay_db, + block_entry_a.clone().into(), n_validators, |_| None ).unwrap(); // add C before B to test sorting. add_block_entry( - &store, - &TEST_CONFIG, - block_entry_c.clone(), + &mut overlay_db, + block_entry_c.clone().into(), n_validators, |_| None ).unwrap(); add_block_entry( - &store, - &TEST_CONFIG, - block_entry_b.clone(), + &mut overlay_db, + block_entry_b.clone().into(), n_validators, |_| None ).unwrap(); + let write_ops = overlay_db.into_write_ops(); + db.write(write_ops).unwrap(); + assert_eq!( load_all_blocks( - &store, + store.as_ref(), &TEST_CONFIG ).unwrap(), vec![block_hash_a, block_hash_b, block_hash_c], diff --git a/node/core/approval-voting/src/backend.rs b/node/core/approval-voting/src/backend.rs index 60c39d0cae68..763a69e8baf5 100644 --- a/node/core/approval-voting/src/backend.rs +++ b/node/core/approval-voting/src/backend.rs @@ -13,6 +13,7 @@ use super::approval_db::v1::{ use super::ops::StoredBlockRange; use super::persisted_entries::{BlockEntry, CandidateEntry}; +#[derive(Debug)] pub(super) enum BackendWriteOp { WriteStoredBlockRange(StoredBlockRange), WriteBlocksAtHeight(BlockNumber, Vec), diff --git a/node/core/approval-voting/src/import.rs b/node/core/approval-voting/src/import.rs index 1899e8b530a5..a6316c167d76 100644 --- a/node/core/approval-voting/src/import.rs +++ b/node/core/approval-voting/src/import.rs @@ -547,8 +547,7 @@ pub(crate) async fn handle_new_head<'a>( #[cfg(test)] mod tests { use super::*; - use crate::backend::BackendWriteOp; - use crate::ops::StoredBlockRange; + use crate::backend::{DbBackend}; use polkadot_node_subsystem_test_helpers::make_subsystem_context; use polkadot_node_primitives::approval::{VRFOutput, VRFProof}; use polkadot_primitives::v1::{SessionInfo, ValidatorIndex}; @@ -563,6 +562,7 @@ mod tests { use assert_matches::assert_matches; use merlin::Transcript; use std::{pin::Pin, sync::Arc}; + use kvdb::KeyValueDB; use crate::{ APPROVAL_SESSIONS, criteria, BlockEntry, @@ -575,54 +575,6 @@ mod tests { const TEST_CONFIG: DatabaseConfig = DatabaseConfig { col_data: DATA_COL, }; - - #[derive(Default)] - struct TestDB { - stored_blocks: Option, - blocks_at_height: HashMap>, - block_entries: HashMap, - candidate_entries: HashMap, - } - - impl Backend for TestDB { - fn load_block_entry( - &self, - block_hash: &Hash, - ) -> SubsystemResult> { - Ok(self.block_entries.get(block_hash).map(|c| c.clone())) - } - - fn load_candidate_entry( - &self, - candidate_hash: &CandidateHash, - ) -> SubsystemResult> { - Ok(self.candidate_entries.get(candidate_hash).map(|c| c.clone())) - } - - fn load_all_blocks(&self) -> SubsystemResult> { - let mut hashes: Vec<_> = self.block_entries.keys().cloned().collect(); - - hashes.sort_by_key(|k| self.block_entries.get(k).unwrap().block_number()); - - Ok(hashes) - } - - fn load_blocks_at_height(&self, block_height: &BlockNumber) -> SubsystemResult> { - let v = self.blocks_at_height.get(block_height).map(|v| v.clone()).unwrap_or_default(); - Ok(v) - } - - fn load_stored_blocks(&self) -> SubsystemResult> { - Ok(self.stored_blocks.clone()) - } - - fn write(&mut self, ops: I) -> SubsystemResult<()> - where I: IntoIterator - { - Ok(()) - } - } - #[derive(Default)] struct MockClock; @@ -1162,7 +1114,10 @@ mod tests { #[test] fn insta_approval_works() { - let mut db = TestDB::default(); + let db_writer: Arc = Arc::new(kvdb_memorydb::create(NUM_COLUMNS)); + let mut db = DbBackend::new(db_writer.clone(), TEST_CONFIG); + let mut overlay_db = OverlayedBackend::new(&db); + let pool = TaskExecutor::new(); let (mut ctx, mut handle) = make_subsystem_context::<(), _>(pool.clone()); @@ -1222,8 +1177,7 @@ mod tests { .collect::>(); let mut state = single_session_state(session, session_info); - db.block_entries.insert( - parent_hash.clone(), + overlay_db.write_block_entry( crate::approval_db::v1::BlockEntry { block_hash: parent_hash.clone(), parent_hash: Default::default(), @@ -1234,14 +1188,15 @@ mod tests { candidates: Vec::new(), approved_bitfield: Default::default(), children: Vec::new(), - }.into(), + }.into() ); - let db_writer = kvdb_memorydb::create(NUM_COLUMNS); + let write_ops = overlay_db.into_write_ops(); + db.write(write_ops).unwrap(); let test_fut = { Box::pin(async move { - let mut overlay_db = OverlayedBackend::new(&db_writer); + let mut overlay_db = OverlayedBackend::new(&db); let result = handle_new_head( &mut ctx, &mut state, @@ -1250,6 +1205,9 @@ mod tests { &Some(1), ).await.unwrap(); + let write_ops = overlay_db.into_write_ops(); + db.write(write_ops).unwrap(); + assert_eq!(result.len(), 1); let candidates = &result[0].imported_candidates; assert_eq!(candidates.len(), 2); @@ -1258,7 +1216,7 @@ mod tests { // the first candidate should be insta-approved // the second should not let entry: BlockEntry = crate::ops::load_block_entry( - &db_writer, + db_writer.as_ref(), &TEST_CONFIG, &hash, ) diff --git a/node/core/approval-voting/src/lib.rs b/node/core/approval-voting/src/lib.rs index 95d64455fb0c..aa9ce209a598 100644 --- a/node/core/approval-voting/src/lib.rs +++ b/node/core/approval-voting/src/lib.rs @@ -82,8 +82,8 @@ mod persisted_entries; use crate::approval_db::v1::{Config as DatabaseConfig}; use crate::backend::{DbBackend, Backend, OverlayedBackend}; -//#[cfg(test)] -//mod tests; +#[cfg(test)] +mod tests; const APPROVAL_SESSIONS: SessionIndex = 6; const APPROVAL_CHECKING_TIMEOUT: Duration = Duration::from_secs(120); @@ -684,7 +684,7 @@ async fn run( let mut last_finalized_height: Option = None; loop { - let mut overlayed_db = OverlayedBackend::new(&backend); + let mut overlayed_db = OverlayedBackend::new(&backend); let actions = futures::select! { (tick, woken_block, woken_candidate) = wakeups.next(&*state.clock).fuse() => { subsystem.metrics.on_wakeup(); @@ -1782,7 +1782,7 @@ fn import_checked_approval<'a>( // // 1. The source is remote, as we don't store anything new in the approval entry. // 2. The candidate is not newly approved, as we haven't altered the approval entry's - // approved flag with `mark_approved` above. + // approved flag with `mark_approved` above. // 3. The source had already approved the candidate, as we haven't altered the bitfield. if !source.is_remote() || newly_approved || !already_approved_by { // In all other cases, we need to write the candidate entry. diff --git a/node/core/approval-voting/src/tests.rs b/node/core/approval-voting/src/tests.rs index 312461db0743..0890e5cdf76b 100644 --- a/node/core/approval-voting/src/tests.rs +++ b/node/core/approval-voting/src/tests.rs @@ -15,8 +15,9 @@ // along with Polkadot. If not, see . use super::*; -use super::backend::Backend; -use polkadot_primitives::v1::{CoreIndex, GroupIndex, ValidatorSignature}; +use super::backend::{Backend, BackendWriteOp}; +use super::ops::StoredBlockRange; +use polkadot_primitives::v1::{CandidateDescriptor, CoreIndex, GroupIndex, ValidatorSignature}; use polkadot_node_primitives::approval::{ AssignmentCert, AssignmentCertKind, VRFOutput, VRFProof, RELAY_VRF_MODULO_CONTEXT, DelayTranche, @@ -188,6 +189,34 @@ impl Backend for TestStore { Ok(hashes) } + + fn load_blocks_at_height(&self, _height: &BlockNumber) -> SubsystemResult> { + unreachable!() + } + + fn load_stored_blocks(&self) -> SubsystemResult> { + unreachable!() + } + + fn write(&mut self, ops: I) -> SubsystemResult<()> + where + I: IntoIterator + { + for op in ops.into_iter() { + match op { + BackendWriteOp::WriteBlockEntry(entry) => { + let hash = entry.block_hash(); + let _ = self.block_entries.insert(hash, entry); + } + BackendWriteOp::WriteCandidateEntry(entry) => { + let hash = entry.candidate.hash(); + let _ = self.candidate_entries.insert(hash, entry); + } + _ => unreachable!(), + } + } + Ok(()) + } } fn blank_state() -> State { @@ -242,6 +271,7 @@ struct StateConfig { validator_groups: Vec>, needed_approvals: u32, no_show_slots: u32, + candidate_hash: Option, } impl Default for StateConfig { @@ -254,6 +284,7 @@ impl Default for StateConfig { validator_groups: vec![vec![ValidatorIndex(0)], vec![ValidatorIndex(1)]], needed_approvals: 1, no_show_slots: 2, + candidate_hash: None, } } } @@ -268,11 +299,12 @@ fn some_state(config: StateConfig, db: &mut TestStore) -> State { validator_groups, needed_approvals, no_show_slots, + candidate_hash, } = config; let n_validators = validators.len(); - let mut state = State { + let state = State { clock: Box::new(MockClock::new(tick)), ..single_session_state(session_index, SessionInfo { validators: validators.iter().map(|v| v.public().into()).collect(), @@ -291,7 +323,7 @@ fn some_state(config: StateConfig, db: &mut TestStore) -> State { let core_index = 0.into(); let block_hash = Hash::repeat_byte(0x01); - let candidate_hash = CandidateHash(Hash::repeat_byte(0xCC)); + let candidate_hash = candidate_hash.unwrap_or_else(|| CandidateHash(Hash::repeat_byte(0xCC))); add_block( db, @@ -307,6 +339,7 @@ fn some_state(config: StateConfig, db: &mut TestStore) -> State { n_validators, core_index, GroupIndex(0), + None, ); state @@ -341,6 +374,7 @@ fn add_candidate_to_block( n_validators: usize, core: CoreIndex, backing_group: GroupIndex, + candidate_receipt: Option, ) { let mut block_entry = db.block_entries.get(&block_hash).unwrap().clone(); @@ -349,7 +383,7 @@ fn add_candidate_to_block( .or_insert_with(|| approval_db::v1::CandidateEntry { session: block_entry.session(), block_assignments: Default::default(), - candidate: CandidateReceipt::default(), + candidate: candidate_receipt.unwrap_or_default(), approvals: bitvec::bitvec![BitOrderLsb0, u8; 0; n_validators], }.into()); @@ -386,14 +420,21 @@ fn rejects_bad_assignment() { let mut state = some_state(Default::default(), &mut db); let candidate_index = 0; + let mut overlay_db = OverlayedBackend::new(&db); let res = check_and_import_assignment( &mut state, + &mut overlay_db, assignment_good.clone(), candidate_index, ).unwrap(); assert_eq!(res.0, AssignmentCheckResult::Accepted); // Check that the assignment's been imported. - assert!(res.1.iter().any(|action| matches!(action, Action::WriteCandidateEntry(..)))); + assert_eq!(res.1.len(), 1); + + let write_ops = overlay_db.into_write_ops().collect::>(); + assert_eq!(write_ops.len(), 1); + assert_matches!(write_ops.get(0).unwrap(), BackendWriteOp::WriteCandidateEntry(..)); + db.write(write_ops).unwrap(); // unknown hash let unknown_hash = Hash::repeat_byte(0x02); @@ -407,12 +448,15 @@ fn rejects_bad_assignment() { ), }; + let mut overlay_db = OverlayedBackend::new(&db); let res = check_and_import_assignment( &mut state, + &mut overlay_db, assignment, candidate_index, ).unwrap(); assert_eq!(res.0, AssignmentCheckResult::Bad(AssignmentCheckError::UnknownBlock(unknown_hash))); + assert_eq!(overlay_db.into_write_ops().count(), 0); let mut state = State { assignment_criteria: Box::new(MockAssignmentCriteria::check_only(|| { @@ -422,12 +466,15 @@ fn rejects_bad_assignment() { }; // same assignment, but this time rejected + let mut overlay_db = OverlayedBackend::new(&db); let res = check_and_import_assignment( &mut state, + &mut overlay_db, assignment_good, candidate_index, ).unwrap(); assert_eq!(res.0, AssignmentCheckResult::Bad(AssignmentCheckError::InvalidCert(ValidatorIndex(0)))); + assert_eq!(overlay_db.into_write_ops().count(), 0); } #[test] @@ -453,13 +500,20 @@ fn rejects_assignment_in_future() { ..some_state(StateConfig { tick, ..Default::default() }, &mut db) }; + let mut overlay_db = OverlayedBackend::new(&db); let res = check_and_import_assignment( &mut state, + &mut overlay_db, assignment.clone(), candidate_index, ).unwrap(); assert_eq!(res.0, AssignmentCheckResult::TooFarInFuture); + let write_ops = overlay_db.into_write_ops().collect::>(); + + assert_eq!(write_ops.len(), 0); + db.write(write_ops).unwrap(); + let mut state = State { assignment_criteria: Box::new(MockAssignmentCriteria::check_only(move || { Ok((tick + 20 - 1) as _) @@ -467,12 +521,22 @@ fn rejects_assignment_in_future() { ..some_state(StateConfig { tick, ..Default::default() }, &mut db) }; + let mut overlay_db = OverlayedBackend::new(&db); let res = check_and_import_assignment( &mut state, + &mut overlay_db, assignment.clone(), candidate_index, ).unwrap(); assert_eq!(res.0, AssignmentCheckResult::Accepted); + + let write_ops = overlay_db.into_write_ops().collect::>(); + assert_eq!(write_ops.len(), 1); + + assert_matches!( + write_ops.get(0).unwrap(), + BackendWriteOp::WriteCandidateEntry(..) + ); } #[test] @@ -492,19 +556,22 @@ fn rejects_assignment_with_unknown_candidate() { let mut state = some_state(Default::default(), &mut db); + let mut overlay_db = OverlayedBackend::new(&db); let res = check_and_import_assignment( &mut state, + &mut overlay_db, assignment.clone(), candidate_index, ).unwrap(); assert_eq!(res.0, AssignmentCheckResult::Bad(AssignmentCheckError::InvalidCandidateIndex(candidate_index))); + assert_eq!(overlay_db.into_write_ops().count(), 0); } #[test] fn assignment_import_updates_candidate_entry_and_schedules_wakeup() { let mut db = TestStore::default(); let block_hash = Hash::repeat_byte(0x01); - let candidate_hash = CandidateHash(Hash::repeat_byte(0xCC)); + let candidate_hash = CandidateReceipt::::default().hash(); let candidate_index = 0; let assignment = IndirectAssignmentCert { @@ -521,17 +588,22 @@ fn assignment_import_updates_candidate_entry_and_schedules_wakeup() { assignment_criteria: Box::new(MockAssignmentCriteria::check_only(|| { Ok(0) })), - ..some_state(Default::default(), &mut db) + ..some_state(StateConfig { + candidate_hash: Some(candidate_hash), + ..Default::default() + }, &mut db) }; + let mut overlay_db = OverlayedBackend::new(&db); let (res, actions) = check_and_import_assignment( &mut state, + &mut overlay_db, assignment.clone(), candidate_index, ).unwrap(); assert_eq!(res, AssignmentCheckResult::Accepted); - assert_eq!(actions.len(), 2); + assert_eq!(actions.len(), 1); assert_matches!( actions.get(0).unwrap(), @@ -547,11 +619,13 @@ fn assignment_import_updates_candidate_entry_and_schedules_wakeup() { } ); + let write_ops = overlay_db.into_write_ops().collect::>(); + assert_eq!(1, write_ops.len()); assert_matches!( - actions.get(1).unwrap(), - Action::WriteCandidateEntry(c, e) => { - assert_eq!(c, &candidate_hash); - assert!(e.approval_entry(&block_hash).unwrap().is_assigned(ValidatorIndex(0))); + write_ops.get(0).unwrap(), + BackendWriteOp::WriteCandidateEntry(ref c_entry) => { + assert_eq!(&c_entry.candidate.hash(), &candidate_hash); + assert!(c_entry.approval_entry(&block_hash).unwrap().is_assigned(ValidatorIndex(0))); } ); } @@ -576,8 +650,10 @@ fn rejects_approval_before_assignment() { signature: sign_approval(Sr25519Keyring::Alice, candidate_hash, 1), }; + let mut overlay_db = OverlayedBackend::new(&db); let (actions, res) = check_and_import_approval( &state, + &mut overlay_db, &Metrics(None), vote, |r| r @@ -585,6 +661,7 @@ fn rejects_approval_before_assignment() { assert_eq!(res, ApprovalCheckResult::Bad(ApprovalCheckError::NoAssignment(ValidatorIndex(0)))); assert!(actions.is_empty()); + assert_eq!(overlay_db.into_write_ops().count(), 0); } #[test] @@ -593,7 +670,7 @@ fn rejects_approval_if_no_candidate_entry() { let block_hash = Hash::repeat_byte(0x01); let candidate_hash = CandidateHash(Hash::repeat_byte(0xCC)); - let mut state = State { + let state = State { assignment_criteria: Box::new(MockAssignmentCriteria::check_only(|| { Ok(0) })), @@ -609,8 +686,10 @@ fn rejects_approval_if_no_candidate_entry() { db.candidate_entries.remove(&candidate_hash); + let mut overlay_db = OverlayedBackend::new(&db); let (actions, res) = check_and_import_approval( &state, + &mut overlay_db, &Metrics(None), vote, |r| r @@ -618,6 +697,7 @@ fn rejects_approval_if_no_candidate_entry() { assert_eq!(res, ApprovalCheckResult::Bad(ApprovalCheckError::InvalidCandidate(0, candidate_hash))); assert!(actions.is_empty()); + assert_eq!(overlay_db.into_write_ops().count(), 0); } #[test] @@ -627,7 +707,7 @@ fn rejects_approval_if_no_block_entry() { let candidate_hash = CandidateHash(Hash::repeat_byte(0xCC)); let validator_index = ValidatorIndex(0); - let mut state = State { + let state = State { assignment_criteria: Box::new(MockAssignmentCriteria::check_only(|| { Ok(0) })), @@ -648,8 +728,10 @@ fn rejects_approval_if_no_block_entry() { db.block_entries.remove(&block_hash); + let mut overlay_db = OverlayedBackend::new(&db); let (actions, res) = check_and_import_approval( &state, + &mut overlay_db, &Metrics(None), vote, |r| r @@ -657,17 +739,18 @@ fn rejects_approval_if_no_block_entry() { assert_eq!(res, ApprovalCheckResult::Bad(ApprovalCheckError::UnknownBlock(block_hash))); assert!(actions.is_empty()); + assert_eq!(overlay_db.into_write_ops().count(), 0); } #[test] fn accepts_and_imports_approval_after_assignment() { let mut db = TestStore::default(); let block_hash = Hash::repeat_byte(0x01); - let candidate_hash = CandidateHash(Hash::repeat_byte(0xCC)); + let candidate_hash = CandidateReceipt::::default().hash(); let validator_index = ValidatorIndex(0); let candidate_index = 0; - let mut state = State { + let state = State { assignment_criteria: Box::new(MockAssignmentCriteria::check_only(|| { Ok(0) })), @@ -675,6 +758,7 @@ fn accepts_and_imports_approval_after_assignment() { validators: vec![Sr25519Keyring::Alice, Sr25519Keyring::Bob, Sr25519Keyring::Charlie], validator_groups: vec![vec![ValidatorIndex(0), ValidatorIndex(1)], vec![ValidatorIndex(2)]], needed_approvals: 2, + candidate_hash: Some(candidate_hash), ..Default::default() }, &mut db) }; @@ -691,8 +775,10 @@ fn accepts_and_imports_approval_after_assignment() { .unwrap() .import_assignment(0, validator_index, 0); + let mut overlay_db = OverlayedBackend::new(&db); let (actions, res) = check_and_import_approval( &state, + &mut overlay_db, &Metrics(None), vote, |r| r @@ -700,12 +786,14 @@ fn accepts_and_imports_approval_after_assignment() { assert_eq!(res, ApprovalCheckResult::Accepted); - assert_eq!(actions.len(), 1); + assert_eq!(actions.len(), 0); + + let write_ops = overlay_db.into_write_ops().collect::>(); + assert_eq!(write_ops.len(), 1); assert_matches!( - actions.get(0).unwrap(), - Action::WriteCandidateEntry(c_hash, c_entry) => { - assert_eq!(c_hash, &candidate_hash); - assert!(c_entry.approvals().get(validator_index.0 as usize).unwrap()); + write_ops.get(0).unwrap(), + BackendWriteOp::WriteCandidateEntry(ref c_entry) => { + assert_eq!(&c_entry.candidate.hash(), &candidate_hash); assert!(!c_entry.approval_entry(&block_hash).unwrap().is_approved()); } ); @@ -720,7 +808,7 @@ fn second_approval_import_only_schedules_wakeups() { let validator_index_b = ValidatorIndex(1); let candidate_index = 0; - let mut state = State { + let state = State { assignment_criteria: Box::new(MockAssignmentCriteria::check_only(|| { Ok(0) })), @@ -749,8 +837,10 @@ fn second_approval_import_only_schedules_wakeups() { // There is only one assignment, so nothing to schedule if we double-import. + let mut overlay_db = OverlayedBackend::new(&db); let (actions, res) = check_and_import_approval( &state, + &mut overlay_db, &Metrics(None), vote.clone(), |r| r @@ -758,6 +848,7 @@ fn second_approval_import_only_schedules_wakeups() { assert_eq!(res, ApprovalCheckResult::Accepted); assert!(actions.is_empty()); + assert_eq!(overlay_db.into_write_ops().count(), 0); // After adding a second assignment, there should be a schedule wakeup action. @@ -766,8 +857,10 @@ fn second_approval_import_only_schedules_wakeups() { .unwrap() .import_assignment(0, validator_index_b, 0); + let mut overlay_db = OverlayedBackend::new(&db); let (actions, res) = check_and_import_approval( &state, + &mut overlay_db, &Metrics(None), vote, |r| r @@ -775,6 +868,7 @@ fn second_approval_import_only_schedules_wakeups() { assert_eq!(res, ApprovalCheckResult::Accepted); assert_eq!(actions.len(), 1); + assert_eq!(overlay_db.into_write_ops().count(), 0); assert_matches!( actions.get(0).unwrap(), @@ -786,11 +880,11 @@ fn second_approval_import_only_schedules_wakeups() { fn import_checked_approval_updates_entries_and_schedules() { let mut db = TestStore::default(); let block_hash = Hash::repeat_byte(0x01); - let candidate_hash = CandidateHash(Hash::repeat_byte(0xCC)); + let candidate_hash = CandidateReceipt::::default().hash(); let validator_index_a = ValidatorIndex(0); let validator_index_b = ValidatorIndex(1); - let mut state = State { + let state = State { assignment_criteria: Box::new(MockAssignmentCriteria::check_only(|| { Ok(0) })), @@ -798,6 +892,7 @@ fn import_checked_approval_updates_entries_and_schedules() { validators: vec![Sr25519Keyring::Alice, Sr25519Keyring::Bob, Sr25519Keyring::Charlie], validator_groups: vec![vec![ValidatorIndex(0), ValidatorIndex(1)], vec![ValidatorIndex(2)]], needed_approvals: 2, + candidate_hash: Some(candidate_hash), ..Default::default() }, &mut db) }; @@ -814,7 +909,7 @@ fn import_checked_approval_updates_entries_and_schedules() { { let mut overlay_db = OverlayedBackend::new(&db); - let mut actions = import_checked_approval( + let actions = import_checked_approval( &state, &mut overlay_db, &Metrics(None), @@ -824,7 +919,7 @@ fn import_checked_approval_updates_entries_and_schedules() { ApprovalSource::Remote(validator_index_a), ); - assert_eq!(actions.len(), 2); + assert_eq!(actions.len(), 1); assert_matches!( actions.get(0).unwrap(), Action::ScheduleWakeup { @@ -836,40 +931,49 @@ fn import_checked_approval_updates_entries_and_schedules() { assert_eq!(c_hash, &candidate_hash); } ); + + let mut write_ops = overlay_db.into_write_ops().collect::>(); + assert_eq!(write_ops.len(), 1); assert_matches!( - actions.get_mut(1).unwrap(), - Action::WriteCandidateEntry(c_hash, ref mut c_entry) => { - assert_eq!(c_hash, &candidate_hash); + write_ops.get_mut(0).unwrap(), + BackendWriteOp::WriteCandidateEntry(ref mut c_entry) => { assert!(!c_entry.approval_entry(&block_hash).unwrap().is_approved()); assert!(c_entry.mark_approval(validator_index_a)); - - db.candidate_entries.insert(candidate_hash, c_entry.clone()); } ); + + db.write(write_ops).unwrap(); } { - let mut actions = import_checked_approval( + let mut overlay_db = OverlayedBackend::new(&db); + let actions = import_checked_approval( &state, + &mut overlay_db, &Metrics(None), db.block_entries.get(&block_hash).unwrap().clone(), candidate_hash, db.candidate_entries.get(&candidate_hash).unwrap().clone(), ApprovalSource::Remote(validator_index_b), ); + assert_eq!(actions.len(), 0); + + let mut write_ops = overlay_db.into_write_ops().collect::>(); + assert_eq!(write_ops.len(), 2); assert_matches!( - actions.get(0).unwrap(), - Action::WriteBlockEntry(b_entry) => { + write_ops.get(0).unwrap(), + BackendWriteOp::WriteBlockEntry(b_entry) => { assert_eq!(b_entry.block_hash(), block_hash); assert!(b_entry.is_fully_approved()); assert!(b_entry.is_candidate_approved(&candidate_hash)); } ); + assert_matches!( - actions.get_mut(1).unwrap(), - Action::WriteCandidateEntry(c_hash, ref mut c_entry) => { - assert_eq!(c_hash, &candidate_hash); + write_ops.get_mut(1).unwrap(), + BackendWriteOp::WriteCandidateEntry(ref mut c_entry) => { + assert_eq!(&c_entry.candidate.hash(), &candidate_hash); assert!(c_entry.approval_entry(&block_hash).unwrap().is_approved()); assert!(c_entry.mark_approval(validator_index_b)); } @@ -879,7 +983,6 @@ fn import_checked_approval_updates_entries_and_schedules() { #[test] fn assignment_triggered_by_all_with_less_than_threshold() { - let mut db = TestStore::default(); let block_hash = Hash::repeat_byte(0x01); let mut candidate_entry: CandidateEntry = { @@ -926,7 +1029,6 @@ fn assignment_triggered_by_all_with_less_than_threshold() { #[test] fn assignment_not_triggered_by_all_with_threshold() { - let mut db = TestStore::default(); let block_hash = Hash::repeat_byte(0x01); let mut candidate_entry: CandidateEntry = { @@ -979,7 +1081,6 @@ fn assignment_not_triggered_by_all_with_threshold() { #[test] fn assignment_not_triggered_if_already_triggered() { - let mut db = TestStore::default(); let block_hash = Hash::repeat_byte(0x01); let candidate_entry: CandidateEntry = { @@ -1018,7 +1119,6 @@ fn assignment_not_triggered_if_already_triggered() { #[test] fn assignment_not_triggered_by_exact() { - let mut db = TestStore::default(); let block_hash = Hash::repeat_byte(0x01); let candidate_entry: CandidateEntry = { @@ -1057,7 +1157,6 @@ fn assignment_not_triggered_by_exact() { #[test] fn assignment_not_triggered_more_than_maximum() { - let mut db = TestStore::default(); let block_hash = Hash::repeat_byte(0x01); let maximum_broadcast = 10; @@ -1102,7 +1201,6 @@ fn assignment_not_triggered_more_than_maximum() { #[test] fn assignment_triggered_if_at_maximum() { - let mut db = TestStore::default(); let block_hash = Hash::repeat_byte(0x01); let maximum_broadcast = 10; @@ -1147,7 +1245,6 @@ fn assignment_triggered_if_at_maximum() { #[test] fn assignment_not_triggered_if_at_maximum_but_clock_is_before() { - let mut db = TestStore::default(); let block_hash = Hash::repeat_byte(0x01); let maximum_broadcast = 10; @@ -1192,7 +1289,6 @@ fn assignment_not_triggered_if_at_maximum_but_clock_is_before() { #[test] fn assignment_not_triggered_if_at_maximum_but_clock_is_before_with_drift() { - let mut db = TestStore::default(); let block_hash = Hash::repeat_byte(0x01); let maximum_broadcast = 10; @@ -1237,7 +1333,6 @@ fn assignment_not_triggered_if_at_maximum_but_clock_is_before_with_drift() { #[test] fn wakeups_next() { - let mut db = TestStore::default(); let mut wakeups = Wakeups::default(); let b_a = Hash::repeat_byte(0); @@ -1296,7 +1391,6 @@ fn wakeups_next() { #[test] fn wakeup_earlier_supersedes_later() { - let mut db = TestStore::default(); let mut wakeups = Wakeups::default(); let b_a = Hash::repeat_byte(0); @@ -1326,13 +1420,17 @@ fn wakeup_earlier_supersedes_later() { fn import_checked_approval_sets_one_block_bit_at_a_time() { let mut db = TestStore::default(); let block_hash = Hash::repeat_byte(0x01); - let candidate_hash = CandidateHash(Hash::repeat_byte(0xCC)); - let candidate_hash_2 = CandidateHash(Hash::repeat_byte(0xDD)); + let candidate_hash = CandidateReceipt::::default().hash(); + let candidate_receipt_2 = CandidateReceipt:: { + descriptor: CandidateDescriptor::default(), + commitments_hash: Hash::repeat_byte(0x02), + }; + let candidate_hash_2 = candidate_receipt_2.hash(); let validator_index_a = ValidatorIndex(0); let validator_index_b = ValidatorIndex(1); - let mut state = State { + let state = State { assignment_criteria: Box::new(MockAssignmentCriteria::check_only(|| { Ok(0) })), @@ -1340,6 +1438,7 @@ fn import_checked_approval_sets_one_block_bit_at_a_time() { validators: vec![Sr25519Keyring::Alice, Sr25519Keyring::Bob, Sr25519Keyring::Charlie], validator_groups: vec![vec![ValidatorIndex(0), ValidatorIndex(1)], vec![ValidatorIndex(2)]], needed_approvals: 2, + candidate_hash: Some(candidate_hash), ..Default::default() }, &mut db) }; @@ -1351,6 +1450,7 @@ fn import_checked_approval_sets_one_block_bit_at_a_time() { 3, CoreIndex(1), GroupIndex(1), + Some(candidate_receipt_2), ); let setup_candidate = |db: &mut TestStore, c_hash| { @@ -1371,8 +1471,10 @@ fn import_checked_approval_sets_one_block_bit_at_a_time() { setup_candidate(&mut db, candidate_hash); setup_candidate(&mut db, candidate_hash_2); + let mut overlay_db = OverlayedBackend::new(&db); let actions = import_checked_approval( &state, + &mut overlay_db, &Metrics(None), db.block_entries.get(&block_hash).unwrap().clone(), candidate_hash, @@ -1380,31 +1482,35 @@ fn import_checked_approval_sets_one_block_bit_at_a_time() { ApprovalSource::Remote(validator_index_b), ); - assert_eq!(actions.len(), 2); + assert_eq!(actions.len(), 0); + + let write_ops = overlay_db.into_write_ops().collect::>(); + assert_eq!(write_ops.len(), 2); + assert_matches!( - actions.get(0).unwrap(), - Action::WriteBlockEntry(b_entry) => { + write_ops.get(0).unwrap(), + BackendWriteOp::WriteBlockEntry(b_entry) => { assert_eq!(b_entry.block_hash(), block_hash); assert!(!b_entry.is_fully_approved()); assert!(b_entry.is_candidate_approved(&candidate_hash)); assert!(!b_entry.is_candidate_approved(&candidate_hash_2)); - - db.block_entries.insert(block_hash, b_entry.clone()); } ); assert_matches!( - actions.get(1).unwrap(), - Action::WriteCandidateEntry(c_h, c_entry) => { - assert_eq!(c_h, &candidate_hash); + write_ops.get(1).unwrap(), + BackendWriteOp::WriteCandidateEntry(c_entry) => { + assert_eq!(&c_entry.candidate.hash(), &candidate_hash); assert!(c_entry.approval_entry(&block_hash).unwrap().is_approved()); - - db.candidate_entries.insert(*c_h, c_entry.clone()); } ); + db.write(write_ops).unwrap(); + + let mut overlay_db = OverlayedBackend::new(&db); let actions = import_checked_approval( &state, + &mut overlay_db, &Metrics(None), db.block_entries.get(&block_hash).unwrap().clone(), candidate_hash_2, @@ -1412,10 +1518,14 @@ fn import_checked_approval_sets_one_block_bit_at_a_time() { ApprovalSource::Remote(validator_index_b), ); - assert_eq!(actions.len(), 2); + assert_eq!(actions.len(), 0); + + let write_ops = overlay_db.into_write_ops().collect::>(); + assert_eq!(write_ops.len(), 2); + assert_matches!( - actions.get(0).unwrap(), - Action::WriteBlockEntry(b_entry) => { + write_ops.get(0).unwrap(), + BackendWriteOp::WriteBlockEntry(b_entry) => { assert_eq!(b_entry.block_hash(), block_hash); assert!(b_entry.is_fully_approved()); assert!(b_entry.is_candidate_approved(&candidate_hash)); @@ -1424,9 +1534,9 @@ fn import_checked_approval_sets_one_block_bit_at_a_time() { ); assert_matches!( - actions.get(1).unwrap(), - Action::WriteCandidateEntry(c_h, c_entry) => { - assert_eq!(c_h, &candidate_hash_2); + write_ops.get(1).unwrap(), + BackendWriteOp::WriteCandidateEntry(c_entry) => { + assert_eq!(&c_entry.candidate.hash(), &candidate_hash_2); assert!(c_entry.approval_entry(&block_hash).unwrap().is_approved()); } ); @@ -1446,20 +1556,6 @@ fn approved_ancestor_all_approved() { let slot = Slot::from(1); let session_index = 1; - let mut state = State { - assignment_criteria: Box::new(MockAssignmentCriteria::check_only(|| { - Ok(0) - })), - ..some_state(StateConfig { - validators: vec![Sr25519Keyring::Alice, Sr25519Keyring::Bob], - validator_groups: vec![vec![ValidatorIndex(0)], vec![ValidatorIndex(1)]], - needed_approvals: 2, - session_index, - slot, - ..Default::default() - }, &mut db) - }; - let add_block = |db: &mut TestStore, block_hash, approved| { add_block( db, @@ -1484,8 +1580,9 @@ fn approved_ancestor_all_approved() { let (mut ctx, mut handle) = make_subsystem_context::<(), _>(pool.clone()); let test_fut = Box::pin(async move { + let overlay_db = OverlayedBackend::new(&db); assert_eq!( - handle_approved_ancestor(&mut ctx, &db, block_hash_4, 0, &Default::default()) + handle_approved_ancestor(&mut ctx, &overlay_db, block_hash_4, 0, &Default::default()) .await.unwrap(), Some((block_hash_4, 4)), ) @@ -1531,20 +1628,6 @@ fn approved_ancestor_missing_approval() { let slot = Slot::from(1); let session_index = 1; - let mut state = State { - assignment_criteria: Box::new(MockAssignmentCriteria::check_only(|| { - Ok(0) - })), - ..some_state(StateConfig { - validators: vec![Sr25519Keyring::Alice, Sr25519Keyring::Bob], - validator_groups: vec![vec![ValidatorIndex(0)], vec![ValidatorIndex(1)]], - needed_approvals: 2, - session_index, - slot, - ..Default::default() - }, &mut db) - }; - let add_block = |db: &mut TestStore, block_hash, approved| { add_block( db, @@ -1569,8 +1652,9 @@ fn approved_ancestor_missing_approval() { let (mut ctx, mut handle) = make_subsystem_context::<(), _>(pool.clone()); let test_fut = Box::pin(async move { + let overlay_db = OverlayedBackend::new(&db); assert_eq!( - handle_approved_ancestor(&mut ctx, &db, block_hash_4, 0, &Default::default()) + handle_approved_ancestor(&mut ctx, &overlay_db, block_hash_4, 0, &Default::default()) .await.unwrap(), Some((block_hash_2, 2)), ) @@ -1607,11 +1691,11 @@ fn process_wakeup_trigger_assignment_launch_approval() { let mut db = TestStore::default(); let block_hash = Hash::repeat_byte(0x01); - let candidate_hash = CandidateHash(Hash::repeat_byte(0xCC)); + let candidate_hash = CandidateReceipt::::default().hash(); let slot = Slot::from(1); let session_index = 1; - let mut state = State { + let state = State { assignment_criteria: Box::new(MockAssignmentCriteria::check_only(|| { Ok(0) })), @@ -1621,18 +1705,22 @@ fn process_wakeup_trigger_assignment_launch_approval() { needed_approvals: 2, session_index, slot, + candidate_hash: Some(candidate_hash), ..Default::default() }, &mut db) }; + let mut overlay_db = OverlayedBackend::new(&db); let actions = process_wakeup( &state, + &mut overlay_db, block_hash, candidate_hash, 1, ).unwrap(); assert!(actions.is_empty()); + assert_eq!(overlay_db.into_write_ops().count(), 0); db.candidate_entries .get_mut(&candidate_hash) @@ -1648,30 +1736,19 @@ fn process_wakeup_trigger_assignment_launch_approval() { triggered: false, }.into()); + let mut overlay_db = OverlayedBackend::new(&db); let actions = process_wakeup( &state, + &mut overlay_db, block_hash, candidate_hash, 1, ).unwrap(); - assert_eq!(actions.len(), 3); - assert_matches!( - actions.get(0).unwrap(), - Action::WriteCandidateEntry(c_hash, c_entry) => { - assert_eq!(c_hash, &candidate_hash); - assert!(c_entry - .approval_entry(&block_hash) - .unwrap() - .our_assignment() - .unwrap() - .triggered() - ); - } - ); + assert_eq!(actions.len(), 2); assert_matches!( - actions.get(1).unwrap(), + actions.get(0).unwrap(), Action::LaunchApproval { candidate_index, .. @@ -1681,14 +1758,31 @@ fn process_wakeup_trigger_assignment_launch_approval() { ); assert_matches!( - actions.get(2).unwrap(), + actions.get(1).unwrap(), Action::ScheduleWakeup { tick, .. } => { assert_eq!(tick, &slot_to_tick(0 + 2)); } - ) + ); + + let write_ops = overlay_db.into_write_ops().collect::>(); + assert_eq!(write_ops.len(), 1); + + assert_matches!( + write_ops.get(0).unwrap(), + BackendWriteOp::WriteCandidateEntry(c_entry) => { + assert_eq!(&c_entry.candidate.hash(), &candidate_hash); + assert!(c_entry + .approval_entry(&block_hash) + .unwrap() + .our_assignment() + .unwrap() + .triggered() + ); + } + ); } #[test] @@ -1700,7 +1794,7 @@ fn process_wakeup_schedules_wakeup() { let slot = Slot::from(1); let session_index = 1; - let mut state = State { + let state = State { assignment_criteria: Box::new(MockAssignmentCriteria::check_only(|| { Ok(10) })), @@ -1728,8 +1822,10 @@ fn process_wakeup_schedules_wakeup() { triggered: false, }.into()); + let mut overlay_db = OverlayedBackend::new(&db); let actions = process_wakeup( &state, + &mut overlay_db, block_hash, candidate_hash, 1, @@ -1744,6 +1840,7 @@ fn process_wakeup_schedules_wakeup() { assert_eq!(tick, &(slot_to_tick(slot) + 10)); } ); + assert_eq!(overlay_db.into_write_ops().count(), 0); } #[test] @@ -1761,17 +1858,18 @@ fn local_approval_import_always_updates_approval_entry() { let mut db = TestStore::default(); let block_hash = Hash::repeat_byte(0x01); let block_hash_2 = Hash::repeat_byte(0x02); - let candidate_hash = CandidateHash(Hash::repeat_byte(0xCC)); + let candidate_hash = CandidateReceipt::::default().hash(); let validator_index = ValidatorIndex(0); let state_config = StateConfig { validators: vec![Sr25519Keyring::Alice, Sr25519Keyring::Bob, Sr25519Keyring::Charlie], validator_groups: vec![vec![ValidatorIndex(0), ValidatorIndex(1)], vec![ValidatorIndex(2)]], needed_approvals: 2, + candidate_hash: Some(candidate_hash), ..Default::default() }; - let mut state = State { + let state = State { assignment_criteria: Box::new(MockAssignmentCriteria::check_only(|| { Ok(0) })), @@ -1792,6 +1890,7 @@ fn local_approval_import_always_updates_approval_entry() { state_config.validators.len(), 1.into(), GroupIndex(1), + None, ); let sig_a = sign_approval(Sr25519Keyring::Alice, candidate_hash, 1); @@ -1821,8 +1920,10 @@ fn local_approval_import_always_updates_approval_entry() { } { - let mut actions = import_checked_approval( + let mut overlay_db = OverlayedBackend::new(&db); + let actions = import_checked_approval( &state, + &mut overlay_db, &Metrics(None), db.block_entries.get(&block_hash).unwrap().clone(), candidate_hash, @@ -1830,26 +1931,31 @@ fn local_approval_import_always_updates_approval_entry() { ApprovalSource::Local(validator_index, sig_a.clone()), ); - assert_eq!(actions.len(), 1); + assert_eq!(actions.len(), 0); + + let mut write_ops = overlay_db.into_write_ops().collect::>(); + assert_eq!(write_ops.len(), 1); assert_matches!( - actions.get_mut(0).unwrap(), - Action::WriteCandidateEntry(c_hash, ref mut c_entry) => { - assert_eq!(c_hash, &candidate_hash); + write_ops.get_mut(0).unwrap(), + BackendWriteOp::WriteCandidateEntry(ref mut c_entry) => { + assert_eq!(&c_entry.candidate.hash(), &candidate_hash); assert_eq!( c_entry.approval_entry(&block_hash).unwrap().local_statements().1, Some(sig_a), ); assert!(c_entry.mark_approval(validator_index)); - - db.candidate_entries.insert(candidate_hash, c_entry.clone()); } ); + + db.write(write_ops).unwrap(); } { - let mut actions = import_checked_approval( + let mut overlay_db = OverlayedBackend::new(&db); + let actions = import_checked_approval( &state, + &mut overlay_db, &Metrics(None), db.block_entries.get(&block_hash_2).unwrap().clone(), candidate_hash, @@ -1857,19 +1963,20 @@ fn local_approval_import_always_updates_approval_entry() { ApprovalSource::Local(validator_index, sig_b.clone()), ); - assert_eq!(actions.len(), 1); + assert_eq!(actions.len(), 0); + + let mut write_ops = overlay_db.into_write_ops().collect::>(); + assert_eq!(write_ops.len(), 1); assert_matches!( - actions.get_mut(0).unwrap(), - Action::WriteCandidateEntry(c_hash, ref mut c_entry) => { - assert_eq!(c_hash, &candidate_hash); + write_ops.get_mut(0).unwrap(), + BackendWriteOp::WriteCandidateEntry(ref mut c_entry) => { + assert_eq!(&c_entry.candidate.hash(), &candidate_hash); assert_eq!( c_entry.approval_entry(&block_hash_2).unwrap().local_statements().1, Some(sig_b), ); assert!(c_entry.mark_approval(validator_index)); - - db.candidate_entries.insert(candidate_hash, c_entry.clone()); } ); }