move tick generation back to banking_stage, add unit tests (#1332)

* move tick generation back to banking_stage, add unit tests

fixes #1217

* remove channel() stuff for synchronous comm; use a mutex

src/banking_stage.rs

@@ -6,10 +6,9 @@ use bank::Bank;
 use bincode::deserialize;
 use counter::Counter;
 use entry::Entry;
-use hash::{Hash, Hasher};
+use hash::Hasher;
 use log::Level;
 use packet::{Packets, SharedPackets};
-use poh::PohEntry;
 use poh_service::PohService;
 use rayon::prelude::*;
 use result::{Error, Result};
@@ -43,26 +42,39 @@ impl BankingStage {
         let thread_hdl = Builder::new()
             .name("solana-banking-stage".to_string())
             .spawn(move || {
-                let (hash_sender, hash_receiver) = channel();
+                let poh_service = PohService::new(bank.last_id(), tick_duration.is_some());
-                let (poh_service, poh_receiver) =
+
-                    PohService::new(bank.last_id(), hash_receiver, tick_duration);
+                let mut last_tick = Instant::now();
+
                 loop {
+                    let timeout =
+                        tick_duration.map(|duration| duration - (Instant::now() - last_tick));
+
                     if let Err(e) = Self::process_packets(
+                        timeout,
                         &bank,
-                        &hash_sender,
+                        &poh_service,
-                        &poh_receiver,
                         &verified_receiver,
                         &entry_sender,
                     ) {
                         match e {
                             Error::RecvTimeoutError(RecvTimeoutError::Disconnected) => break,
                             Error::RecvTimeoutError(RecvTimeoutError::Timeout) => (),
+                            Error::RecvError(_) => break,
                             Error::SendError => break,
-                            _ => error!("{:?}", e),
+                            _ => {
+                                error!("process_packets() {:?}", e);
+                                break;
+                            }
                         }
                     }
+                    if tick_duration.is_some() && last_tick.elapsed() > tick_duration.unwrap() {
+                        if let Err(e) = Self::tick(&poh_service, &entry_sender) {
+                            error!("tick() {:?}", e);
+                        }
+                        last_tick = Instant::now();
+                    }
                 }
-                drop(hash_sender);
                 poh_service.join().unwrap();
             }).unwrap();
         (BankingStage { thread_hdl }, entry_receiver)
@@ -80,26 +92,33 @@ impl BankingStage {
             }).collect()
     }
 
+    fn tick(poh_service: &PohService, entry_sender: &Sender<Vec<Entry>>) -> Result<()> {
+        let poh = poh_service.tick();
+        let entry = Entry {
+            num_hashes: poh.num_hashes,
+            id: poh.id,
+            transactions: vec![],
+        };
+        entry_sender.send(vec![entry])?;
+        Ok(())
+    }
+
     fn process_transactions(
         bank: &Arc<Bank>,
         transactions: &[Transaction],
-        hash_sender: &Sender<Hash>,
+        poh_service: &PohService,
-        poh_receiver: &Receiver<PohEntry>,
+    ) -> Result<Vec<Entry>> {
-        entry_sender: &Sender<Vec<Entry>>,
-    ) -> Result<()> {
         let mut entries = Vec::new();
 
-        debug!("transactions: {}", transactions.len());
+        debug!("processing: {}", transactions.len());
 
         let mut chunk_start = 0;
         while chunk_start != transactions.len() {
             let chunk_end = chunk_start + Entry::num_will_fit(&transactions[chunk_start..]);
 
             let results = bank.process_transactions(&transactions[chunk_start..chunk_end]);
-            debug!("results: {}", results.len());
 
             let mut hasher = Hasher::default();
-
             let processed_transactions: Vec<_> = transactions[chunk_start..chunk_end]
                 .into_iter()
                 .enumerate()
@@ -114,64 +133,45 @@ impl BankingStage {
                     }
                 }).collect();
 
-            debug!("processed ok: {}", processed_transactions.len());
+            debug!("processed: {}", processed_transactions.len());
 
             chunk_start = chunk_end;
 
             let hash = hasher.result();
 
             if !processed_transactions.is_empty() {
-                hash_sender.send(hash)?;
+                let poh = poh_service.record(hash);
 
-                let mut answered = false;
+                bank.register_entry_id(&poh.id);
-                while !answered {
+                entries.push(Entry {
-                    entries.extend(poh_receiver.try_iter().map(|poh| {
-                        if let Some(mixin) = poh.mixin {
-                            answered = true;
-                            assert_eq!(mixin, hash);
-                            bank.register_entry_id(&poh.id);
-                            Entry {
-                                num_hashes: poh.num_hashes,
-                                id: poh.id,
-                                transactions: processed_transactions.clone(),
-                            }
-                        } else {
-                            Entry {
-                                num_hashes: poh.num_hashes,
-                                id: poh.id,
-                                transactions: vec![],
-                            }
-                        }
-                    }));
-                }
-            } else {
-                entries.extend(poh_receiver.try_iter().map(|poh| Entry {
                     num_hashes: poh.num_hashes,
                     id: poh.id,
-                    transactions: vec![],
+                    transactions: processed_transactions,
-                }));
+                });
             }
         }
 
         debug!("done process_transactions, {} entries", entries.len());
 
-        entry_sender.send(entries)?;
+        Ok(entries)
-        Ok(())
     }
 
     /// Process the incoming packets and send output `Signal` messages to `signal_sender`.
     /// Discard packets via `packet_recycler`.
     pub fn process_packets(
+        timeout: Option<Duration>,
         bank: &Arc<Bank>,
-        hash_sender: &Sender<Hash>,
+        poh_service: &PohService,
-        poh_receiver: &Receiver<PohEntry>,
         verified_receiver: &Receiver<Vec<(SharedPackets, Vec<u8>)>>,
         entry_sender: &Sender<Vec<Entry>>,
     ) -> Result<()> {
-        let timer = Duration::new(1, 0);
         let recv_start = Instant::now();
-        let mms = verified_receiver.recv_timeout(timer)?;
+        // TODO pass deadline to recv_deadline() when/if it becomes available?
-        debug!("verified_recevier {:?}", verified_receiver);
+        let mms = if let Some(timeout) = timeout {
+            verified_receiver.recv_timeout(timeout)?
+        } else {
+            verified_receiver.recv()?
+        };
         let now = Instant::now();
         let mut reqs_len = 0;
         let mms_len = mms.len();
@@ -189,6 +189,8 @@ impl BankingStage {
             let transactions = Self::deserialize_transactions(&msgs.read());
             reqs_len += transactions.len();
 
+            debug!("transactions received {}", transactions.len());
+
             let transactions: Vec<_> = transactions
                 .into_iter()
                 .zip(vers)
@@ -200,14 +202,10 @@ impl BankingStage {
                         None
                     },
                 }).collect();
+            debug!("verified transactions {}", transactions.len());
 
-            Self::process_transactions(
+            let entries = Self::process_transactions(bank, &transactions, poh_service)?;
-                bank,
+            entry_sender.send(entries)?;
-                &transactions,
-                hash_sender,
-                poh_receiver,
-                entry_sender,
-            )?;
         }
 
         inc_new_counter_info!(
@@ -241,63 +239,153 @@ impl Service for BankingStage {
     }
 }
 
-// TODO: When banking is pulled out of RequestStage, add this test back in.
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use bank::Bank;
+    use ledger::Block;
+    use mint::Mint;
+    use packet::{to_packets, PacketRecycler};
+    use signature::{Keypair, KeypairUtil};
+    use std::thread::sleep;
+    use transaction::Transaction;
 
-//use bank::Bank;
+    #[test]
-//use entry::Entry;
+    fn test_banking_stage_shutdown() {
-//use hash::Hash;
+        let bank = Bank::new(&Mint::new(2));
-//use record_stage::RecordStage;
+        let (verified_sender, verified_receiver) = channel();
-//use record_stage::Signal;
+        let (banking_stage, _entry_receiver) =
-//use result::Result;
+            BankingStage::new(Arc::new(bank), verified_receiver, None);
-//use std::sync::mpsc::{channel, Sender};
+        drop(verified_sender);
-//use std::sync::{Arc, Mutex};
+        assert_eq!(banking_stage.join().unwrap(), ());
-//use std::time::Duration;
+    }
-//use transaction::Transaction;
+
-//
+    #[test]
-//#[cfg(test)]
+    fn test_banking_stage_tick() {
-//mod tests {
+        let bank = Bank::new(&Mint::new(2));
-//    use bank::Bank;
+        let start_hash = bank.last_id();
-//    use mint::Mint;
+        let (verified_sender, verified_receiver) = channel();
-//    use signature::{KeyPair, KeyPairUtil};
+        let (banking_stage, entry_receiver) = BankingStage::new(
-//    use transaction::Transaction;
+            Arc::new(bank),
-//
+            verified_receiver,
-//    #[test]
+            Some(Duration::from_millis(1)),
-//    // TODO: Move this test banking_stage. Calling process_transactions() directly
+        );
-//    // defeats the purpose of this test.
+        sleep(Duration::from_millis(50));
-//    fn test_banking_sequential_consistency() {
+        drop(verified_sender);
-//        // In this attack we'll demonstrate that a verifier can interpret the ledger
+
-//        // differently if either the server doesn't signal the ledger to add an
+        let entries: Vec<_> = entry_receiver.iter().flat_map(|x| x).collect();
-//        // Entry OR if the verifier tries to parallelize across multiple Entries.
+        assert!(entries.len() != 0);
-//        let mint = Mint::new(2);
+        assert!(entries.verify(&start_hash));
-//        let bank = Bank::new(&mint);
+        assert_eq!(banking_stage.join().unwrap(), ());
-//        let banking_stage = EventProcessor::new(bank, &mint.last_id(), None);
+    }
-//
+
-//        // Process a batch that includes a transaction that receives two tokens.
+    #[test]
-//        let alice = KeyPair::new();
+    fn test_banking_stage_no_tick() {
-//        let tx = Transaction::new(&mint.keypair(), alice.pubkey(), 2, mint.last_id());
+        let bank = Bank::new(&Mint::new(2));
-//        let transactions = vec![tx];
+        let (verified_sender, verified_receiver) = channel();
-//        let entry0 = banking_stage.process_transactions(transactions).unwrap();
+        let (banking_stage, entry_receiver) =
-//
+            BankingStage::new(Arc::new(bank), verified_receiver, None);
-//        // Process a second batch that spends one of those tokens.
+        sleep(Duration::from_millis(1000));
-//        let tx = Transaction::new(&alice, mint.pubkey(), 1, mint.last_id());
+        drop(verified_sender);
-//        let transactions = vec![tx];
+
-//        let entry1 = banking_stage.process_transactions(transactions).unwrap();
+        let entries: Vec<_> = entry_receiver.try_iter().map(|x| x).collect();
-//
+        assert!(entries.len() == 0);
-//        // Collect the ledger and feed it to a new bank.
+        assert_eq!(banking_stage.join().unwrap(), ());
-//        let entries = vec![entry0, entry1];
+    }
-//
+
-//        // Assert the user holds one token, not two. If the server only output one
+    #[test]
-//        // entry, then the second transaction will be rejected, because it drives
+    fn test_banking_stage() {
-//        // the account balance below zero before the credit is added.
+        let mint = Mint::new(2);
-//        let bank = Bank::new(&mint);
+        let bank = Bank::new(&mint);
-//        for entry in entries {
+        let start_hash = bank.last_id();
-//            assert!(
+        let (verified_sender, verified_receiver) = channel();
-//                bank
+        let (banking_stage, entry_receiver) =
-//                    .process_transactions(entry.transactions)
+            BankingStage::new(Arc::new(bank), verified_receiver, None);
-//                    .into_iter()
+
-//                    .all(|x| x.is_ok())
+        // good tx
-//            );
+        let keypair = mint.keypair();
-//        }
+        let tx = Transaction::new(&keypair, keypair.pubkey(), 1, start_hash);
-//        assert_eq!(bank.get_balance(&alice.pubkey()), Some(1));
+
-//    }
+        // good tx, but no verify
-//}
+        let tx_no_ver = Transaction::new(&keypair, keypair.pubkey(), 1, start_hash);
+
+        // bad tx, AccountNotFound
+        let keypair = Keypair::new();
+        let tx_anf = Transaction::new(&keypair, keypair.pubkey(), 1, start_hash);
+
+        // send 'em over
+        let recycler = PacketRecycler::default();
+        let packets = to_packets(&recycler, &[tx, tx_no_ver, tx_anf]);
+
+        // glad they all fit
+        assert_eq!(packets.len(), 1);
+        verified_sender                       // tx, no_ver, anf
+            .send(vec![(packets[0].clone(), vec![1u8, 0u8, 1u8])])
+            .unwrap();
+
+        drop(verified_sender);
+
+        let entries: Vec<_> = entry_receiver.iter().map(|x| x).collect();
+        assert_eq!(entries.len(), 1);
+        let mut last_id = start_hash;
+        entries.iter().for_each(|entries| {
+            assert_eq!(entries.len(), 1);
+            assert!(entries.verify(&last_id));
+            last_id = entries.last().unwrap().id;
+        });
+        assert_eq!(banking_stage.join().unwrap(), ());
+    }
+
+    #[test]
+    fn test_banking_stage_entryfication() {
+        // In this attack we'll demonstrate that a verifier can interpret the ledger
+        // differently if either the server doesn't signal the ledger to add an
+        // Entry OR if the verifier tries to parallelize across multiple Entries.
+        let mint = Mint::new(2);
+        let bank = Bank::new(&mint);
+        let recycler = PacketRecycler::default();
+        let (verified_sender, verified_receiver) = channel();
+        let (banking_stage, entry_receiver) =
+            BankingStage::new(Arc::new(bank), verified_receiver, None);
+
+        // Process a batch that includes a transaction that receives two tokens.
+        let alice = Keypair::new();
+        let tx = Transaction::new(&mint.keypair(), alice.pubkey(), 2, mint.last_id());
+
+        let packets = to_packets(&recycler, &[tx]);
+        verified_sender
+            .send(vec![(packets[0].clone(), vec![1u8])])
+            .unwrap();
+
+        // Process a second batch that spends one of those tokens.
+        let tx = Transaction::new(&alice, mint.pubkey(), 1, mint.last_id());
+        let packets = to_packets(&recycler, &[tx]);
+        verified_sender
+            .send(vec![(packets[0].clone(), vec![1u8])])
+            .unwrap();
+        drop(verified_sender);
+        assert_eq!(banking_stage.join().unwrap(), ());
+
+        // Collect the ledger and feed it to a new bank.
+        let ventries: Vec<_> = entry_receiver.iter().collect();
+
+        // same assertion as below, really...
+        assert_eq!(ventries.len(), 2);
+
+        // Assert the user holds one token, not two. If the stage only outputs one
+        // entry, then the second transaction will be rejected, because it drives
+        // the account balance below zero before the credit is added.
+        let bank = Bank::new(&mint);
+        for entries in ventries {
+            for entry in entries {
+                assert!(
+                    bank.process_transactions(&entry.transactions)
+                        .into_iter()
+                        .all(|x| x.is_ok())
+                );
+            }
+        }
+        assert_eq!(bank.get_balance(&alice.pubkey()), 1);
+    }
+
+}

src/entry.rs

@@ -200,13 +200,17 @@ fn next_hash(start_hash: &Hash, num_hashes: u64, transactions: &[Transaction]) -
         return *start_hash;
     }
 
-    let mut poh = Poh::new(*start_hash, None);
+    let mut poh = Poh::new(*start_hash);
 
     for _ in 1..num_hashes {
         poh.hash();
     }
 
-    poh.record(Transaction::hash(transactions)).id
+    if transactions.is_empty() {
+        poh.tick().id
+    } else {
+        poh.record(Transaction::hash(transactions)).id
+    }
 }
 
 /// Creates the next Tick or Transaction Entry `num_hashes` after `start_hash`.

src/poh.rs

@@ -1,14 +1,10 @@
 //! The `Poh` module provides an object for generating a Proof of History.
 //! It records Hashes items on behalf of its users.
-
 use hash::{hash, hashv, Hash};
-use std::time::{Duration, Instant};
 
 pub struct Poh {
     last_hash: Hash,
     num_hashes: u64,
-    last_tick: Instant,
-    tick_duration: Option<Duration>,
 }
 
 #[derive(Debug)]
@@ -19,13 +15,10 @@ pub struct PohEntry {
 }
 
 impl Poh {
-    pub fn new(last_hash: Hash, tick_duration: Option<Duration>) -> Self {
+    pub fn new(last_hash: Hash) -> Self {
-        let last_tick = Instant::now();
         Poh {
             last_hash,
             num_hashes: 0,
-            last_tick,
-            tick_duration,
         }
     }
 
@@ -36,10 +29,10 @@ impl Poh {
 
     pub fn record(&mut self, mixin: Hash) -> PohEntry {
         let num_hashes = self.num_hashes + 1;
-        self.num_hashes = 0;
-
         self.last_hash = hashv(&[&self.last_hash.as_ref(), &mixin.as_ref()]);
 
+        self.num_hashes = 0;
+
         PohEntry {
             num_hashes,
             id: self.last_hash,
@@ -49,23 +42,21 @@ impl Poh {
 
     // emissions of Ticks (i.e. PohEntries without a mixin) allows
     //  validators to parallelize the work of catching up
-    pub fn tick(&mut self) -> Option<PohEntry> {
+    pub fn tick(&mut self) -> PohEntry {
-        if let Some(tick_duration) = self.tick_duration {
+        self.hash();
-            if self.last_tick.elapsed() >= tick_duration {
+
-                self.last_tick = Instant::now();
+        let num_hashes = self.num_hashes;
-                let entry = PohEntry {
+        self.num_hashes = 0;
-                    num_hashes: self.num_hashes,
+
-                    id: self.last_hash,
+        PohEntry {
-                    mixin: None,
+            num_hashes,
-                };
+            id: self.last_hash,
-                self.num_hashes = 0;
+            mixin: None,
-                return Some(entry);
-            }
         }
-        None
     }
 }
 
+#[cfg(test)]
 pub fn verify(initial: Hash, entries: &[PohEntry]) -> bool {
     let mut last_hash = initial;
 

src/poh_service.rs

@@ -11,12 +11,14 @@
 use hash::Hash;
 use poh::{Poh, PohEntry};
 use service::Service;
-use std::sync::mpsc::{channel, Receiver, RecvError, Sender, TryRecvError};
+use std::sync::atomic::{AtomicBool, Ordering};
+use std::sync::{Arc, Mutex};
 use std::thread::{self, Builder, JoinHandle};
-use std::time::Duration;
 
 pub struct PohService {
+    poh: Arc<Mutex<Poh>>,
     thread_hdl: JoinHandle<()>,
+    run_poh: Arc<AtomicBool>,
 }
 
 impl PohService {
@@ -24,74 +26,83 @@ impl PohService {
     /// sending back Entry messages until either the receiver or sender channel is closed.
     /// if tick_duration is some, service will automatically produce entries every
     ///  `tick_duration`.
-    pub fn new(
+    pub fn new(start_hash: Hash, run_poh: bool) -> Self {
-        start_hash: Hash,
+        let poh = Arc::new(Mutex::new(Poh::new(start_hash)));
-        hash_receiver: Receiver<Hash>,
+        let run_poh = Arc::new(AtomicBool::new(run_poh));
-        tick_duration: Option<Duration>,
-    ) -> (Self, Receiver<PohEntry>) {
-        let (poh_sender, poh_receiver) = channel();
 
+        let thread_poh = poh.clone();
+        let thread_run_poh = run_poh.clone();
         let thread_hdl = Builder::new()
             .name("solana-record-service".to_string())
             .spawn(move || {
-                let mut poh = Poh::new(start_hash, tick_duration);
+                while thread_run_poh.load(Ordering::Relaxed) {
-                if tick_duration.is_some() {
+                    thread_poh.lock().unwrap().hash();
-                    loop {
-                        if Self::try_process_hashes(&mut poh, &hash_receiver, &poh_sender).is_err()
-                        {
-                            return;
-                        }
-                        poh.hash();
-                    }
-                } else {
-                    let _ = Self::process_hashes(&mut poh, &hash_receiver, &poh_sender);
                 }
             }).unwrap();
 
-        (PohService { thread_hdl }, poh_receiver)
+        PohService {
-    }
+            poh,
-
+            run_poh,
-    fn process_hash(hash: Hash, poh: &mut Poh, sender: &Sender<PohEntry>) -> Result<(), ()> {
+            thread_hdl,
-        let resp = poh.record(hash);
-        sender.send(resp).or(Err(()))?;
-        Ok(())
-    }
-
-    fn process_hashes(
-        poh: &mut Poh,
-        receiver: &Receiver<Hash>,
-        sender: &Sender<PohEntry>,
-    ) -> Result<(), ()> {
-        loop {
-            match receiver.recv() {
-                Ok(hash) => Self::process_hash(hash, poh, sender)?,
-                Err(RecvError) => return Err(()),
-            }
         }
     }
 
-    fn try_process_hashes(
+    pub fn tick(&self) -> PohEntry {
-        poh: &mut Poh,
+        let mut poh = self.poh.lock().unwrap();
-        receiver: &Receiver<Hash>,
+        poh.tick()
-        sender: &Sender<PohEntry>,
-    ) -> Result<(), ()> {
-        loop {
-            if let Some(resp) = poh.tick() {
-                sender.send(resp).or(Err(()))?;
-            }
-            match receiver.try_recv() {
-                Ok(hash) => Self::process_hash(hash, poh, sender)?,
-                Err(TryRecvError::Empty) => return Ok(()),
-                Err(TryRecvError::Disconnected) => return Err(()),
-            };
-        }
     }
+
+    pub fn record(&self, mixin: Hash) -> PohEntry {
+        let mut poh = self.poh.lock().unwrap();
+        poh.record(mixin)
+    }
+
+    //    fn process_hash(
+    //        mixin: Option<Hash>,
+    //        poh: &mut Poh,
+    //        sender: &Sender<PohEntry>,
+    //    ) -> Result<(), ()> {
+    //        let resp = match mixin {
+    //            Some(mixin) => poh.record(mixin),
+    //            None => poh.tick(),
+    //        };
+    //        sender.send(resp).or(Err(()))?;
+    //        Ok(())
+    //    }
+    //
+    //    fn process_hashes(
+    //        poh: &mut Poh,
+    //        receiver: &Receiver<Option<Hash>>,
+    //        sender: &Sender<PohEntry>,
+    //    ) -> Result<(), ()> {
+    //        loop {
+    //            match receiver.recv() {
+    //                Ok(hash) => Self::process_hash(hash, poh, sender)?,
+    //                Err(RecvError) => return Err(()),
+    //            }
+    //        }
+    //    }
+    //
+    //    fn try_process_hashes(
+    //        poh: &mut Poh,
+    //        receiver: &Receiver<Option<Hash>>,
+    //        sender: &Sender<PohEntry>,
+    //    ) -> Result<(), ()> {
+    //        loop {
+    //            match receiver.try_recv() {
+    //                Ok(hash) => Self::process_hash(hash, poh, sender)?,
+    //                Err(TryRecvError::Empty) => return Ok(()),
+    //                Err(TryRecvError::Disconnected) => return Err(()),
+    //            };
+    //        }
+    //    }
 }
 
 impl Service for PohService {
     type JoinReturnType = ();
 
     fn join(self) -> thread::Result<()> {
+        self.run_poh.store(false, Ordering::Relaxed);
         self.thread_hdl.join()
     }
 }
@@ -100,57 +111,38 @@ impl Service for PohService {
 mod tests {
     use super::*;
     use poh::verify;
-    use std::sync::mpsc::channel;
     use std::thread::sleep;
+    use std::time::Duration;
 
     #[test]
     fn test_poh() {
-        let (hash_sender, hash_receiver) = channel();
+        let poh_service = PohService::new(Hash::default(), false);
-        let (poh_service, poh_receiver) = PohService::new(Hash::default(), hash_receiver, None);
 
-        hash_sender.send(Hash::default()).unwrap();
+        let entry0 = poh_service.record(Hash::default());
         sleep(Duration::from_millis(1));
-        hash_sender.send(Hash::default()).unwrap();
+        let entry1 = poh_service.record(Hash::default());
         sleep(Duration::from_millis(1));
-        hash_sender.send(Hash::default()).unwrap();
+        let entry2 = poh_service.record(Hash::default());
-
-        let entry0 = poh_receiver.recv().unwrap();
-        let entry1 = poh_receiver.recv().unwrap();
-        let entry2 = poh_receiver.recv().unwrap();
 
         assert_eq!(entry0.num_hashes, 1);
-        assert_eq!(entry0.num_hashes, 1);
+        assert_eq!(entry1.num_hashes, 1);
-        assert_eq!(entry0.num_hashes, 1);
+        assert_eq!(entry2.num_hashes, 1);
 
-        drop(hash_sender);
+        assert_eq!(poh_service.join().unwrap(), ());
-        assert_eq!(poh_service.thread_hdl.join().unwrap(), ());
 
         assert!(verify(Hash::default(), &[entry0, entry1, entry2]));
     }
 
     #[test]
-    fn test_poh_closed_sender() {
+    fn test_do_poh() {
-        let (hash_sender, hash_receiver) = channel();
+        let poh_service = PohService::new(Hash::default(), true);
-        let (poh_service, poh_receiver) = PohService::new(Hash::default(), hash_receiver, None);
-        drop(poh_receiver);
-        hash_sender.send(Hash::default()).unwrap();
-        assert_eq!(poh_service.thread_hdl.join().unwrap(), ());
-    }
-
-    #[test]
-    fn test_poh_clock() {
-        let (hash_sender, hash_receiver) = channel();
-        let (_poh_service, poh_receiver) = PohService::new(
-            Hash::default(),
-            hash_receiver,
-            Some(Duration::from_millis(1)),
-        );
 
         sleep(Duration::from_millis(50));
-        drop(hash_sender);
+        let entry = poh_service.tick();
-        let pohs: Vec<_> = poh_receiver.iter().map(|x| x).collect();
+        assert!(entry.num_hashes > 1);
-        assert!(pohs.len() > 1);
 
-        assert!(verify(Hash::default(), &pohs));
+        assert_eq!(poh_service.join().unwrap(), ());
+
+        assert!(verify(Hash::default(), &vec![entry]));
     }
 }