From 5e8f958977e5f448d3695f74a8822f6fc7084f32 Mon Sep 17 00:00:00 2001
From: Michael Sproul <michael@sigmaprime.io>
Date: Thu, 5 Dec 2019 10:56:37 +1100
Subject: [PATCH] Allow multiple proposals per validator per epoch (#662)

Closes #658
---
 beacon_node/rest_api/src/validator.rs  | 15 +++++++-------
 beacon_node/rest_api/tests/test.rs     | 28 ++++++++++++++++++--------
 validator_client/src/duties_service.rs | 24 +++++++++-------------
 3 files changed, 37 insertions(+), 30 deletions(-)

diff --git a/beacon_node/rest_api/src/validator.rs b/beacon_node/rest_api/src/validator.rs
index 9eb852623..b9a8f9c4e 100644
--- a/beacon_node/rest_api/src/validator.rs
+++ b/beacon_node/rest_api/src/validator.rs
@@ -28,8 +28,8 @@ pub struct ValidatorDuty {
     pub attestation_committee_index: Option<CommitteeIndex>,
     /// The position of the validator in the committee.
     pub attestation_committee_position: Option<usize>,
-    /// The slot in which a validator must propose a block, or `null` if block production is not required.
-    pub block_proposal_slot: Option<Slot>,
+    /// The slots in which a validator must propose a block (can be empty).
+    pub block_proposal_slots: Vec<Slot>,
 }
 
 #[derive(PartialEq, Debug, Serialize, Deserialize, Clone, Encode, Decode)]
@@ -161,17 +161,18 @@ fn return_validator_duties<T: BeaconChainTypes>(
                         ))
                     })?;
 
-                let block_proposal_slot = validator_proposers
+                let block_proposal_slots = validator_proposers
                     .iter()
-                    .find(|(i, _slot)| validator_index == *i)
-                    .map(|(_i, slot)| *slot);
+                    .filter(|(i, _slot)| validator_index == *i)
+                    .map(|(_i, slot)| *slot)
+                    .collect();
 
                 Ok(ValidatorDuty {
                     validator_pubkey,
                     attestation_slot: duties.map(|d| d.slot),
                     attestation_committee_index: duties.map(|d| d.index),
                     attestation_committee_position: duties.map(|d| d.committee_position),
-                    block_proposal_slot,
+                    block_proposal_slots,
                 })
             } else {
                 Ok(ValidatorDuty {
@@ -179,7 +180,7 @@ fn return_validator_duties<T: BeaconChainTypes>(
                     attestation_slot: None,
                     attestation_committee_index: None,
                     attestation_committee_position: None,
-                    block_proposal_slot: None,
+                    block_proposal_slots: vec![],
                 })
             }
         })
diff --git a/beacon_node/rest_api/tests/test.rs b/beacon_node/rest_api/tests/test.rs
index 7937c9bc3..a810d5acd 100644
--- a/beacon_node/rest_api/tests/test.rs
+++ b/beacon_node/rest_api/tests/test.rs
@@ -294,14 +294,16 @@ fn check_duties<T: BeaconChainTypes>(
                 "attestation index should match"
             );
 
-            if let Some(slot) = duty.block_proposal_slot {
-                let expected_proposer = state
-                    .get_beacon_proposer_index(slot, spec)
-                    .expect("should know proposer");
-                assert_eq!(
-                    expected_proposer, validator_index,
-                    "should get correct proposal slot"
-                );
+            if !duty.block_proposal_slots.is_empty() {
+                for slot in &duty.block_proposal_slots {
+                    let expected_proposer = state
+                        .get_beacon_proposer_index(*slot, spec)
+                        .expect("should know proposer");
+                    assert_eq!(
+                        expected_proposer, validator_index,
+                        "should get correct proposal slot"
+                    );
+                }
             } else {
                 epoch.slot_iter(E::slots_per_epoch()).for_each(|slot| {
                     let slot_proposer = state
@@ -314,6 +316,16 @@ fn check_duties<T: BeaconChainTypes>(
                 })
             }
         });
+
+    // Validator duties should include a proposer for every slot of the epoch.
+    let mut all_proposer_slots: Vec<Slot> = duties
+        .iter()
+        .flat_map(|duty| duty.block_proposal_slots.clone())
+        .collect();
+    all_proposer_slots.sort();
+
+    let all_slots: Vec<Slot> = epoch.slot_iter(E::slots_per_epoch()).collect();
+    assert_eq!(all_proposer_slots, all_slots);
 }
 
 #[test]
diff --git a/validator_client/src/duties_service.rs b/validator_client/src/duties_service.rs
index c83ebef8b..b375eb341 100644
--- a/validator_client/src/duties_service.rs
+++ b/validator_client/src/duties_service.rs
@@ -52,7 +52,7 @@ impl DutiesStore {
                 let epoch = slot.epoch(slots_per_epoch);
 
                 validator_map.get(&epoch).and_then(|duties| {
-                    if duties.block_proposal_slot == Some(slot) {
+                    if duties.block_proposal_slots.contains(&slot) {
                         Some(duties.validator_pubkey.clone())
                     } else {
                         None
@@ -363,7 +363,7 @@ impl<T: SlotClock + 'static, E: EthSpec> DutiesService<T, E> {
                             info!(
                                 log,
                                 "First duty assignment for validator";
-                                "proposal_slot" => format!("{:?}", &duties.block_proposal_slot),
+                                "proposal_slots" => format!("{:?}", &duties.block_proposal_slots),
                                 "attestation_slot" => format!("{:?}", &duties.attestation_slot),
                                 "validator" => format!("{:?}", &duties.validator_pubkey)
                             );
@@ -408,17 +408,11 @@ impl<T: SlotClock + 'static, E: EthSpec> DutiesService<T, E> {
 
 /// Returns `true` if the slots in the `duties` are from the given `epoch`
 fn duties_match_epoch(duties: &ValidatorDuty, epoch: Epoch, slots_per_epoch: u64) -> bool {
-    if let Some(attestation_slot) = duties.attestation_slot {
-        if attestation_slot.epoch(slots_per_epoch) != epoch {
-            return false;
-        }
-    }
-
-    if let Some(block_proposal_slot) = duties.block_proposal_slot {
-        if block_proposal_slot.epoch(slots_per_epoch) != epoch {
-            return false;
-        }
-    }
-
-    true
+    duties
+        .attestation_slot
+        .map_or(true, |slot| slot.epoch(slots_per_epoch) == epoch)
+        && duties
+            .block_proposal_slots
+            .iter()
+            .all(|slot| slot.epoch(slots_per_epoch) == epoch)
 }