From 1714e5b4eb371b31e2b2bff943d8b0874e490f0a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bart=C5=82omiej=20Kami=C5=84ski?= <bart@casperlabs.io>
Date: Fri, 26 Jun 2020 16:14:39 +0200
Subject: [PATCH 1/2] HWY-90: go back to a single consensus value per block

---
 .../consensus/consensus_protocol.rs           |  4 +-
 .../consensus_protocol/protocol_state.rs      |  2 +-
 .../highway_core/active_validator.rs          | 16 +++----
 .../consensus/highway_core/block.rs           | 12 ++---
 .../highway_core/finality_detector.rs         | 47 +++++--------------
 .../consensus/highway_core/highway.rs         |  2 +-
 .../consensus/highway_core/state.rs           | 14 +++---
 .../consensus/highway_core/test_macros.rs     |  4 +-
 .../consensus/highway_core/vertex.rs          | 10 ++--
 src/components/consensus/highway_core/vote.rs |  8 ++--
 src/components/consensus/protocols/highway.rs |  4 +-
 11 files changed, 51 insertions(+), 72 deletions(-)

diff --git a/src/components/consensus/consensus_protocol.rs b/src/components/consensus/consensus_protocol.rs
index c97ed8c4ac..c047b5036d 100644
--- a/src/components/consensus/consensus_protocol.rs
+++ b/src/components/consensus/consensus_protocol.rs
@@ -121,8 +121,8 @@ mod example {
             VIdU64(self.id)
         }
 
-        fn values(&self) -> Vec<DeployHash> {
-            vec![self.deploy_hash.clone()]
+        fn value(&self) -> Option<DeployHash> {
+            Some(self.deploy_hash.clone())
         }
     }
 
diff --git a/src/components/consensus/consensus_protocol/protocol_state.rs b/src/components/consensus/consensus_protocol/protocol_state.rs
index aed5d5125b..479e123dd3 100644
--- a/src/components/consensus/consensus_protocol/protocol_state.rs
+++ b/src/components/consensus/consensus_protocol/protocol_state.rs
@@ -10,7 +10,7 @@ pub(crate) trait VertexTrait: Clone {
 
     fn id(&self) -> Self::Id;
 
-    fn values(&self) -> Vec<Self::Value>;
+    fn value(&self) -> Option<Self::Value>;
 }
 
 pub(crate) enum AddVertexOk<VId> {
diff --git a/src/components/consensus/highway_core/active_validator.rs b/src/components/consensus/highway_core/active_validator.rs
index 85a630b9dd..41e4b623fa 100644
--- a/src/components/consensus/highway_core/active_validator.rs
+++ b/src/components/consensus/highway_core/active_validator.rs
@@ -38,8 +38,8 @@ impl BlockContext {
 ///
 /// It implements the Highway schedule. The protocol proceeds in rounds, and in each round one
 /// validator is the _leader_.
-/// * In the beginning of the round, the leader sends a _proposal_ vote, containing consensus values
-///   (i.e. a block).
+/// * In the beginning of the round, the leader sends a _proposal_ vote, containing a consensus
+///   value (i.e. a block).
 /// * Upon receiving the proposal, all the other validators send a _confirmation_ vote, citing only
 ///   the proposal, their own previous message, and resulting transitive justifications.
 /// * At a fixed point in time later in the round, everyone unconditionally sends a _witness_ vote,
@@ -119,7 +119,7 @@ impl<C: Context> ActiveValidator<C> {
     /// Proposes a new block with the given consensus value.
     pub(crate) fn propose(
         &self,
-        values: Vec<C::ConsensusValue>,
+        value: C::ConsensusValue,
         block_context: BlockContext,
         state: &State<C>,
     ) -> Vec<Effect<C>> {
@@ -129,7 +129,7 @@ impl<C: Context> ActiveValidator<C> {
         }
         let panorama = state.panorama_cutoff(state.panorama(), block_context.instant);
         let instant = block_context.instant();
-        let proposal_vote = self.new_vote(panorama, instant, Some(values), state);
+        let proposal_vote = self.new_vote(panorama, instant, Some(value), state);
         vec![Effect::NewVertex(Vertex::Vote(proposal_vote))]
     }
 
@@ -176,7 +176,7 @@ impl<C: Context> ActiveValidator<C> {
         &self,
         panorama: Panorama<C>,
         instant: u64,
-        values: Option<Vec<C::ConsensusValue>>,
+        value: Option<C::ConsensusValue>,
         state: &State<C>,
     ) -> SignedWireVote<C> {
         let add1 = |vh: &C::Hash| state.vote(vh).seq_number + 1;
@@ -184,7 +184,7 @@ impl<C: Context> ActiveValidator<C> {
         let wvote = WireVote {
             panorama,
             sender: self.vidx,
-            values,
+            value,
             seq_number,
             instant,
         };
@@ -285,7 +285,7 @@ mod tests {
         assert_eq!(416, bctx.instant());
 
         // She has a pending deploy from Colin who wants to pay for a hot beverage.
-        let effects = alice_av.propose(vec![0xC0FFEE], bctx, &state);
+        let effects = alice_av.propose(0xC0FFEE, bctx, &state);
         let proposal_wvote = unwrap_single(effects).unwrap_vote();
         let prop_hash = proposal_wvote.hash();
         state.add_vote(proposal_wvote)?;
@@ -311,7 +311,7 @@ mod tests {
 
         // Payment finalized! "One Pumpkin Spice Mochaccino for Corbyn!"
         assert_eq!(
-            FinalityResult::Finalized(vec![0xC0FFEE], Vec::new()),
+            FinalityResult::Finalized(0xC0FFEE, Vec::new()),
             fd.run(&state)
         );
         Ok(())
diff --git a/src/components/consensus/highway_core/block.rs b/src/components/consensus/highway_core/block.rs
index 7307f0ef6a..0731389962 100644
--- a/src/components/consensus/highway_core/block.rs
+++ b/src/components/consensus/highway_core/block.rs
@@ -7,7 +7,7 @@ pub(crate) struct Block<C: Context> {
     /// The total number of ancestors, i.e. the height in the blockchain.
     pub(crate) height: u64,
     /// The payload, e.g. a list of transactions.
-    pub(crate) values: Vec<C::ConsensusValue>,
+    pub(crate) value: C::ConsensusValue,
     /// A skip list index of the block's ancestors.
     ///
     /// For every `p = 1 << i` that divides `height`, this contains an `i`-th entry pointing to the
@@ -19,11 +19,11 @@ impl<C: Context> Block<C> {
     /// Creates a new block with the given parent and values. Panics if parent does not exist.
     pub(crate) fn new(
         parent_hash: Option<C::Hash>,
-        values: Vec<C::ConsensusValue>,
+        value: C::ConsensusValue,
         state: &State<C>,
     ) -> Block<C> {
         let (parent, mut skip_idx) = match parent_hash {
-            None => return Block::initial(values),
+            None => return Block::initial(value),
             Some(hash) => (state.block(&hash), vec![hash]),
         };
         let height = parent.height + 1;
@@ -33,7 +33,7 @@ impl<C: Context> Block<C> {
         }
         Block {
             height,
-            values,
+            value,
             skip_idx,
         }
     }
@@ -43,10 +43,10 @@ impl<C: Context> Block<C> {
         self.skip_idx.first()
     }
 
-    fn initial(values: Vec<C::ConsensusValue>) -> Block<C> {
+    fn initial(value: C::ConsensusValue) -> Block<C> {
         Block {
             height: 0,
-            values,
+            value,
             skip_idx: vec![],
         }
     }
diff --git a/src/components/consensus/highway_core/finality_detector.rs b/src/components/consensus/highway_core/finality_detector.rs
index 587e001cc5..da8519245e 100644
--- a/src/components/consensus/highway_core/finality_detector.rs
+++ b/src/components/consensus/highway_core/finality_detector.rs
@@ -110,9 +110,9 @@ impl<'a, C: Context> Section<'a, C> {
 pub(crate) enum FinalityResult<V: ConsensusValueT, VID> {
     /// No new block has been finalized yet.
     None,
-    /// A new block with these consensus values has been finalized.
+    /// A new block with this consensus value has been finalized.
     /// Second vectors is a list of indexes of validators that equivocated.
-    Finalized(Vec<V>, Vec<VID>),
+    Finalized(V, Vec<VID>),
     /// The fault tolerance threshold has been exceeded: The number of observed equivocation
     /// invalidates this finality detector's results.
     FttExceeded,
@@ -138,7 +138,7 @@ impl<C: Context> FinalityDetector<C> {
         }
     }
 
-    /// Returns the next batch of values, if any has been finalized since the last call.
+    /// Returns the next value, if any has been finalized since the last call.
     // TODO: Iterate this and return multiple finalized blocks.
     // TODO: Verify the consensus instance ID?
     pub(crate) fn run(
@@ -167,7 +167,7 @@ impl<C: Context> FinalityDetector<C> {
                     self.last_finalized = Some(candidate.clone());
                     let new_equivocators = state.get_new_equivocators(&candidate);
                     return FinalityResult::Finalized(
-                        state.block(candidate).values.clone(),
+                        state.block(candidate).value.clone(),
                         new_equivocators,
                     );
                 }
@@ -261,33 +261,21 @@ mod tests {
         // `b0`, `a0` are level 0 for `B0`. `a0`, `b1` are level 1.
         // So the fault tolerance of `B0` is 2 * (9 - 10/2) * (1 - 1/2) = 4.
         assert_eq!(FinalityResult::None, fd6.run(&state));
-        assert_eq!(
-            FinalityResult::Finalized(vec![0xB0], vec![]),
-            fd4.run(&state)
-        );
+        assert_eq!(FinalityResult::Finalized(0xB0, vec![]), fd4.run(&state));
         assert_eq!(FinalityResult::None, fd4.run(&state));
 
         // Adding another level to the summit increases `B0`'s fault tolerance to 6.
         add_vote!(state, _a2, ALICE, ALICE_SEC, 2; a1, b1, c1);
         add_vote!(state, _b2, BOB, BOB_SEC, 2; a1, b1, c1);
-        assert_eq!(
-            FinalityResult::Finalized(vec![0xB0], vec![]),
-            fd6.run(&state)
-        );
+        assert_eq!(FinalityResult::Finalized(0xB0, vec![]), fd6.run(&state));
         assert_eq!(FinalityResult::None, fd6.run(&state));
 
         // If Alice equivocates, the FTT 4 is exceeded, but she counts as being part of any summit,
         // so `A0` and `A1` get FTT 6. (Bob voted for `A1` and against `B1` in `b2`.)
         add_vote!(state, _e2, BOB, BOB_SEC, 2; a1, b1, c1);
         assert_eq!(FinalityResult::FttExceeded, fd4.run(&state));
-        assert_eq!(
-            FinalityResult::Finalized(vec![0xA0], vec![]),
-            fd6.run(&state)
-        );
-        assert_eq!(
-            FinalityResult::Finalized(vec![0xA1], vec![]),
-            fd6.run(&state)
-        );
+        assert_eq!(FinalityResult::Finalized(0xA0, vec![]), fd6.run(&state));
+        assert_eq!(FinalityResult::Finalized(0xA1, vec![]), fd6.run(&state));
         assert_eq!(FinalityResult::None, fd6.run(&state));
         Ok(())
     }
@@ -311,29 +299,20 @@ mod tests {
         add_vote!(state, _c1, CAROL, CAROL_SEC, 1; N, b0, c0; 0xC1);
         add_vote!(state, _c1_prime, CAROL, CAROL_SEC, 1; N, b0, c0);
         add_vote!(state, b1, BOB, BOB_SEC, 1; a0, b0, N; 0xB1);
-        assert_eq!(
-            FinalityResult::Finalized(vec![0xB0], vec![]),
-            fd4.run(&state)
-        );
+        assert_eq!(FinalityResult::Finalized(0xB0, vec![]), fd4.run(&state));
         add_vote!(state, a1, ALICE, ALICE_SEC, 1; a0, b0, F; 0xA1);
         add_vote!(state, b2, BOB, BOB_SEC, 2; a1, b1, F);
         add_vote!(state, a2, ALICE, ALICE_SEC, 2; a1, b2, F; 0xA2);
-        assert_eq!(
-            FinalityResult::Finalized(vec![0xA0], vec![]),
-            fd4.run(&state)
-        );
+        assert_eq!(FinalityResult::Finalized(0xA0, vec![]), fd4.run(&state));
         // A1 is the first block that sees CAROL equivocating.
         assert_eq!(
-            FinalityResult::Finalized(vec![0xA1], vec![CAROL]),
+            FinalityResult::Finalized(0xA1, vec![CAROL]),
             fd4.run(&state)
         );
         // Finalize A2. It should not report CAROL as equivocator anymore.
         add_vote!(state, b3, BOB, BOB_SEC, 3; a2, b2, F);
         add_vote!(state, _a3, ALICE, ALICE_SEC, 3; a2, b3, F);
-        assert_eq!(
-            FinalityResult::Finalized(vec![0xA2], vec![]),
-            fd4.run(&state)
-        );
+        assert_eq!(FinalityResult::Finalized(0xA2, vec![]), fd4.run(&state));
 
         // Test that an initial block reports equivocators as well.
         let mut bstate: State<TestContext> = State::new(&[Weight(5), Weight(4), Weight(1)], 0);
@@ -344,7 +323,7 @@ mod tests {
         add_vote!(bstate, b0, BOB, BOB_SEC, 0; a0, N, F);
         add_vote!(bstate, a1, ALICE, ALICE_SEC, 1; a0, b0, F);
         assert_eq!(
-            FinalityResult::Finalized(vec![0xA0], vec![CAROL]),
+            FinalityResult::Finalized(0xA0, vec![CAROL]),
             fde4.run(&bstate)
         );
         Ok(())
diff --git a/src/components/consensus/highway_core/highway.rs b/src/components/consensus/highway_core/highway.rs
index d754c25b3f..31d5dfb5b3 100644
--- a/src/components/consensus/highway_core/highway.rs
+++ b/src/components/consensus/highway_core/highway.rs
@@ -147,7 +147,7 @@ pub(crate) mod tests {
         let wvote = WireVote {
             panorama: Panorama::new(WEIGHTS.len()),
             sender: ALICE,
-            values: Some(vec![]),
+            value: Some(0),
             seq_number: 0,
             instant: 1,
         };
diff --git a/src/components/consensus/highway_core/state.rs b/src/components/consensus/highway_core/state.rs
index 4dd20a9d65..8c6866fd79 100644
--- a/src/components/consensus/highway_core/state.rs
+++ b/src/components/consensus/highway_core/state.rs
@@ -210,9 +210,9 @@ impl<C: Context> State<C> {
         self.update_panorama(&swvote);
         let hash = wvote.hash();
         let fork_choice = self.fork_choice(&wvote.panorama).cloned();
-        let (vote, opt_values) = Vote::new(swvote, fork_choice.as_ref(), self);
-        if let Some(values) = opt_values {
-            let block = Block::new(fork_choice, values, self);
+        let (vote, opt_value) = Vote::new(swvote, fork_choice.as_ref(), self);
+        if let Some(value) = opt_value {
+            let block = Block::new(fork_choice, value, self);
             self.blocks.insert(hash.clone(), block);
         }
         self.votes.insert(hash, vote);
@@ -227,11 +227,11 @@ impl<C: Context> State<C> {
     pub(crate) fn wire_vote(&self, hash: &C::Hash) -> Option<SignedWireVote<C>> {
         let vote = self.opt_vote(hash)?.clone();
         let opt_block = self.opt_block(hash);
-        let values = opt_block.map(|block| block.values.clone());
+        let value = opt_block.map(|block| block.value.clone());
         let wvote = WireVote {
             panorama: vote.panorama.clone(),
             sender: vote.sender,
-            values,
+            value,
             seq_number: vote.seq_number,
             instant: vote.instant,
         };
@@ -324,7 +324,7 @@ impl<C: Context> State<C> {
     fn validate_vote(&self, swvote: &SignedWireVote<C>) -> Result<(), VoteError> {
         let wvote = &swvote.wire_vote;
         let sender = wvote.sender;
-        if (wvote.values.is_none() && wvote.panorama.is_empty())
+        if (wvote.value.is_none() && wvote.panorama.is_empty())
             || !self.is_panorama_valid(&wvote.panorama)
         {
             return Err(VoteError::Panorama);
@@ -610,7 +610,7 @@ pub(crate) mod tests {
     fn find_in_swimlane() -> Result<(), AddVoteError<TestContext>> {
         let mut state = State::new(WEIGHTS, 0);
         let mut a = Vec::new();
-        let vote = vote!(ALICE, ALICE_SEC, 0; N, N, N; Some(vec![0xA]));
+        let vote = vote!(ALICE, ALICE_SEC, 0; N, N, N; Some(0xA));
         a.push(vote.hash());
         state.add_vote(vote)?;
         for i in 1..10 {
diff --git a/src/components/consensus/highway_core/test_macros.rs b/src/components/consensus/highway_core/test_macros.rs
index 92938d167b..dffc98c19a 100644
--- a/src/components/consensus/highway_core/test_macros.rs
+++ b/src/components/consensus/highway_core/test_macros.rs
@@ -15,7 +15,7 @@ macro_rules! vote {
         let wvote = crate::components::consensus::highway_core::vertex::WireVote {
             panorama: panorama!($($obs),*),
             sender: $sender,
-            values: $val,
+            value: $val,
             seq_number: $seq_num,
             instant: 0,
         };
@@ -32,7 +32,7 @@ macro_rules! add_vote {
         $state.add_vote(vote)?;
     };
     ($state: ident, $hash: ident, $sender: expr, $secret: expr, $seq_num: expr; $($obs:expr),*; $val: expr) => {
-        let vote = vote!($sender, $secret, $seq_num; $($obs),*; Some(vec![$val]));
+        let vote = vote!($sender, $secret, $seq_num; $($obs),*; Some($val));
         let $hash = vote.hash();
         $state.add_vote(vote)?;
     };
diff --git a/src/components/consensus/highway_core/vertex.rs b/src/components/consensus/highway_core/vertex.rs
index e404415b63..810e50084e 100644
--- a/src/components/consensus/highway_core/vertex.rs
+++ b/src/components/consensus/highway_core/vertex.rs
@@ -30,16 +30,16 @@ pub(crate) enum Vertex<C: Context> {
 }
 
 impl<C: Context> Vertex<C> {
-    /// Returns an iterator over all consensus values mentioned in this vertex.
+    /// Returns the consensus value mentioned in this vertex, if any.
     ///
     /// These need to be validated before passing the vertex into the protocol state. E.g. if
     /// `C::ConsensusValue` is a transaction, it should be validated first (correct signature,
     /// structure, gas limit, etc.). If it is a hash of a transaction, the transaction should be
     /// obtained _and_ validated. Only after that, the vertex can be considered valid.
-    pub(crate) fn values<'a>(&'a self) -> Box<dyn Iterator<Item = &'a C::ConsensusValue> + 'a> {
+    pub(crate) fn value(&self) -> Option<&C::ConsensusValue> {
         match self {
-            Vertex::Vote(swvote) => Box::new(swvote.wire_vote.values.iter().flat_map(|v| v.iter())),
-            Vertex::Evidence(_) => Box::new(iter::empty()),
+            Vertex::Vote(swvote) => swvote.wire_vote.value.as_ref(),
+            Vertex::Evidence(_) => None,
         }
     }
 
@@ -84,7 +84,7 @@ impl<C: Context> SignedWireVote<C> {
 pub(crate) struct WireVote<C: Context> {
     pub(crate) panorama: Panorama<C>,
     pub(crate) sender: ValidatorIndex,
-    pub(crate) values: Option<Vec<C::ConsensusValue>>,
+    pub(crate) value: Option<C::ConsensusValue>,
     pub(crate) seq_number: u64,
     pub(crate) instant: u64,
 }
diff --git a/src/components/consensus/highway_core/vote.rs b/src/components/consensus/highway_core/vote.rs
index eb2d1de832..46070c96a6 100644
--- a/src/components/consensus/highway_core/vote.rs
+++ b/src/components/consensus/highway_core/vote.rs
@@ -126,14 +126,14 @@ pub(crate) struct Vote<C: Context> {
 }
 
 impl<C: Context> Vote<C> {
-    /// Creates a new `Vote` from the `WireVote`, and returns the values if it contained any.
+    /// Creates a new `Vote` from the `WireVote`, and returns the value if it contained any.
     /// Values must be stored as a block, with the same hash.
     pub(crate) fn new(
         swvote: SignedWireVote<C>,
         fork_choice: Option<&C::Hash>,
         state: &State<C>,
-    ) -> (Vote<C>, Option<Vec<C::ConsensusValue>>) {
-        let block = if swvote.wire_vote.values.is_some() {
+    ) -> (Vote<C>, Option<C::ConsensusValue>) {
+        let block = if swvote.wire_vote.value.is_some() {
             swvote.wire_vote.hash() // A vote with a new block votes for itself.
         } else {
             // If the vote didn't introduce a new block, it votes for the fork choice itself.
@@ -164,7 +164,7 @@ impl<C: Context> Vote<C> {
             instant: swvote.wire_vote.instant,
             signature: swvote.signature,
         };
-        (vote, swvote.wire_vote.values)
+        (vote, swvote.wire_vote.value)
     }
 
     /// Returns the sender's previous message.
diff --git a/src/components/consensus/protocols/highway.rs b/src/components/consensus/protocols/highway.rs
index 88ef81c24e..85e48be9b5 100644
--- a/src/components/consensus/protocols/highway.rs
+++ b/src/components/consensus/protocols/highway.rs
@@ -15,8 +15,8 @@ impl<C: Context> VertexTrait for Vertex<C> {
         self.id()
     }
 
-    fn values(&self) -> Vec<C::ConsensusValue> {
-        self.values().cloned().collect()
+    fn value(&self) -> Option<C::ConsensusValue> {
+        self.value().cloned()
     }
 }
 

From a3fc69ac26972e13cc7fc0ae2632200f180ab66f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bart=C5=82omiej=20Kami=C5=84ski?= <bart@casperlabs.io>
Date: Fri, 26 Jun 2020 19:41:12 +0200
Subject: [PATCH 2/2] HWY-90: Address a review comment

---
 src/components/consensus/consensus_protocol.rs                | 4 ++--
 src/components/consensus/consensus_protocol/protocol_state.rs | 2 +-
 src/components/consensus/protocols/highway.rs                 | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/components/consensus/consensus_protocol.rs b/src/components/consensus/consensus_protocol.rs
index c047b5036d..d5b5c1ac9e 100644
--- a/src/components/consensus/consensus_protocol.rs
+++ b/src/components/consensus/consensus_protocol.rs
@@ -121,8 +121,8 @@ mod example {
             VIdU64(self.id)
         }
 
-        fn value(&self) -> Option<DeployHash> {
-            Some(self.deploy_hash.clone())
+        fn value(&self) -> Option<&DeployHash> {
+            Some(&self.deploy_hash)
         }
     }
 
diff --git a/src/components/consensus/consensus_protocol/protocol_state.rs b/src/components/consensus/consensus_protocol/protocol_state.rs
index 479e123dd3..38e78f2bd1 100644
--- a/src/components/consensus/consensus_protocol/protocol_state.rs
+++ b/src/components/consensus/consensus_protocol/protocol_state.rs
@@ -10,7 +10,7 @@ pub(crate) trait VertexTrait: Clone {
 
     fn id(&self) -> Self::Id;
 
-    fn value(&self) -> Option<Self::Value>;
+    fn value(&self) -> Option<&Self::Value>;
 }
 
 pub(crate) enum AddVertexOk<VId> {
diff --git a/src/components/consensus/protocols/highway.rs b/src/components/consensus/protocols/highway.rs
index 85e48be9b5..c16f7c10a8 100644
--- a/src/components/consensus/protocols/highway.rs
+++ b/src/components/consensus/protocols/highway.rs
@@ -15,8 +15,8 @@ impl<C: Context> VertexTrait for Vertex<C> {
         self.id()
     }
 
-    fn value(&self) -> Option<C::ConsensusValue> {
-        self.value().cloned()
+    fn value(&self) -> Option<&C::ConsensusValue> {
+        self.value()
     }
 }