Fix attestation duties bug

sigp · Sep 8, 2020 · dea298e · dea298e
1 parent b8f4d72
commit dea298e
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 3 deletions.
diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs
@@ -857,6 +857,16 @@ pub fn serve<T: BeaconChainTypes>(
         .and_then(
             |epoch: Epoch, query: api_types::ValidatorDutiesQuery, chain: Arc<BeaconChain<T>>| {
                 blocking_json_task(move || {
+                    let current_epoch = chain.epoch().map_err(crate::reject::beacon_chain_error)?;
+
+                    // Taking advantage of saturating addition on epoch.
+                    if epoch + 1 < current_epoch {
+                        return Err(crate::reject::custom_bad_request(format!(
+                            "request epoch {} is more than one epoch prior to current epoch {}",
+                            epoch, current_epoch
+                        )));
+                    }
+
                     let indices = query
                         .index
                         .clone()

diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs
@@ -1020,6 +1020,20 @@ impl ApiTester {
             for indices in self.interesting_validator_indices() {
                 let epoch = Epoch::from(epoch);
 
+                // The endpoint does not allow getting duties older than the previous epoch.
+                if epoch + 1 < current_epoch {
+                    assert_eq!(
+                        self.client
+                            .get_validator_duties_attester(epoch, Some(&indices))
+                            .await
+                            .unwrap_err()
+                            .status()
+                            .map(Into::into),
+                        Some(400)
+                    );
+                    continue;
+                }
+
                 let results = self
                     .client
                     .get_validator_duties_attester(epoch, Some(&indices))

diff --git a/common/eth2/src/lib.rs b/common/eth2/src/lib.rs
@@ -15,7 +15,7 @@ pub enum Error {
 }
 
 impl Error {
-    fn status(&self) -> Option<StatusCode> {
+    pub fn status(&self) -> Option<StatusCode> {
         match self {
             Error::Reqwest(error) => error.status(),
             Error::ServerMessage(msg) => StatusCode::try_from(msg.code).ok(),

diff --git a/common/eth2/src/types.rs b/common/eth2/src/types.rs
@@ -317,14 +317,18 @@ pub struct ChainHeadData {
     pub root: Hash256,
 }
 
-#[derive(Clone, Deserialize)]
+#[derive(Clone, PartialEq, Debug, Deserialize)]
 #[serde(try_from = "String", bound = "T: FromStr")]
 pub struct QueryVec<T: FromStr>(pub Vec<T>);
 
 impl<T: FromStr> TryFrom<String> for QueryVec<T> {
     type Error = String;
 
     fn try_from(string: String) -> Result<Self, Self::Error> {
+        if string == "" {
+            return Ok(Self(vec![]));
+        }
+
         string
             .split(",")
             .map(|s| s.parse().map_err(|_| "unable to parse".to_string()))
@@ -362,6 +366,9 @@ mod tests {
 
     #[test]
     fn query_vec() {
-        assert_eq!("0,1,2".parse::<QueryVec<u64>>().unwrap().0, vec![0, 1, 3]);
+        assert_eq!(
+            QueryVec::try_from("0,1,2".to_string()).unwrap(),
+            QueryVec(vec![0_u64, 1, 2])
+        );
     }
 }