Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LIBP2P] - Delay sending ResponseMessage::NotFound #3046

Closed
lukaszrzasik opened this issue Apr 26, 2024 · 0 comments · Fixed by #3231
Closed

[LIBP2P] - Delay sending ResponseMessage::NotFound #3046

lukaszrzasik opened this issue Apr 26, 2024 · 0 comments · Fixed by #3231

Comments

@lukaszrzasik
Copy link
Contributor

What is this task and why do we need to work on it?

Currently a node immediately responds with ResponseMessage::NotFound when it doesn't have necessary data to calculate VID. This behaviour might be problematic in situations when the node hasn't yet received DA proposal but it most probably will receive it soon.

What work will need to be done to complete this task?

Delay sending ResponseMessage::NotFound. Wait for a predetermined timeout in hope that the data required to calculate VID arrives and the node can respond with ResponseMessage::Found

Are there any other details to include?

No response

What are the acceptance criteria to close this issue?

This is hard to test because it's not deterministic. The most consistent way to reproduce the issue is to apply the following patch:

diff --git a/crates/hotshot/src/traits/networking/combined_network.rs b/crates/hotshot/src/traits/networking/combined_network.rs
index 3a56909..0ee74e3 100644
--- a/crates/hotshot/src/traits/networking/combined_network.rs
+++ b/crates/hotshot/src/traits/networking/combined_network.rs
@@ -502,7 +502,7 @@ impl<TYPES: NodeType> ConnectedNetwork<Message<TYPES>, TYPES::SignatureKey>
         });
         // View changed, let's start primary again
         self.primary_down.store(false, Ordering::Relaxed);
-        self.primary_fail_counter.store(0, Ordering::Relaxed);
+        // self.primary_fail_counter.store(0, Ordering::Relaxed);
     }
 
     fn is_primary_down(&self) -> bool {

and run the test_combined_network_cdn_crash test.

Branch work will be merged to (if not the default branch)

No response

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant