From 9554aadf052388745768aa535d5dcf6f17f77393 Mon Sep 17 00:00:00 2001
From: Peter Jankuliak <p.jankuliak@gmail.com>
Date: Tue, 17 Oct 2023 11:17:19 +0200
Subject: [PATCH] Don't accept block by the same client who previously failed

---
 lib/src/missing_parts/tracker.rs | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/lib/src/missing_parts/tracker.rs b/lib/src/missing_parts/tracker.rs
index 5114984db..4181c96e4 100644
--- a/lib/src/missing_parts/tracker.rs
+++ b/lib/src/missing_parts/tracker.rs
@@ -42,7 +42,8 @@ impl Tracker {
             .entry(part_id)
             .or_insert_with(|| MissingPartState {
                 offering_clients: HashSet::default(),
-                accepted_by: None,
+                currently_accepted_by: None,
+                previously_accepted_by: Default::default(),
                 required: false,
                 approved: false,
             });
@@ -149,13 +150,18 @@ impl TrackerClient {
             .entry(part_id)
             .or_insert_with(|| MissingPartState {
                 offering_clients: HashSet::default(),
-                accepted_by: None,
+                currently_accepted_by: None,
+                previously_accepted_by: Default::default(),
                 required: false,
                 approved: false,
             });
 
         missing_part.offering_clients.insert(self.client_id);
 
+        // The peer could have previously lied about having the part (e.g. expired block). Now it's
+        // claiming to have it again so we need to reconsider loading from it again.
+        missing_part.previously_accepted_by.remove(&self.client_id);
+
         match state {
             OfferState::Approved => {
                 missing_part.approved = true;
@@ -184,6 +190,9 @@ impl Drop for TrackerClient {
                 notify = true;
             }
 
+            // When the client reconnects, we allow it to accept the again.
+            missing_part.previously_accepted_by.remove(&self.client_id);
+
             // TODO: if the block hasn't other offers and isn't required, remove it
         }
 
@@ -233,9 +242,15 @@ impl PartPromiseAcceptor {
             // unwrap is ok because of the invariant in `Inner`
             let missing_part = inner.missing_parts.get_mut(part_id).unwrap();
 
-            if missing_part.required && missing_part.approved && missing_part.accepted_by.is_none()
+            if missing_part.required
+                && missing_part.approved
+                && missing_part.currently_accepted_by.is_none()
+                && !missing_part
+                    .previously_accepted_by
+                    .contains(&self.client_id)
             {
-                missing_part.accepted_by = Some(self.client_id);
+                missing_part.currently_accepted_by = Some(self.client_id);
+                missing_part.previously_accepted_by.insert(self.client_id);
 
                 return Some(PartPromise {
                     shared: self.shared.clone(),
@@ -354,16 +369,17 @@ struct Inner {
 #[derive(Debug)]
 struct MissingPartState {
     offering_clients: HashSet<ClientId>,
-    accepted_by: Option<ClientId>,
+    currently_accepted_by: Option<ClientId>,
+    previously_accepted_by: HashSet<ClientId>,
     required: bool,
     approved: bool,
 }
 
 impl MissingPartState {
     fn unaccept_by(&mut self, client_id: ClientId) -> bool {
-        if let Some(accepted_by) = &self.accepted_by {
-            if accepted_by == &client_id {
-                self.accepted_by = None;
+        if let Some(currently_accepted_by) = &self.currently_accepted_by {
+            if currently_accepted_by == &client_id {
+                self.currently_accepted_by = None;
                 return true;
             }
         }