[bisq-network/bisq] periodical UpdateDataReq for Seednodes (#3202)

[chimp1984]

chimp1984 commented on this pull request.

Would need a bit more time for testing PR... On first look it looks ok.

> @@ -337,11 +340,13 @@ public void onPreliminaryDataReceived() {
 
     @Override
     public void onUpdatedDataReceived() {
-        isBootstrapped = true;
-        maybeProcessAllMailboxEntries();
-        p2pServiceListeners.stream().forEach(P2PServiceListener::onUpdatedDataReceived);
-        if(!isBootstrapped)
-            p2PDataStorage.onBootstrapComplete();
+        if (!isBootstrapped() || Capabilities.app.containsAll(Capability.SEED_NODE)) {
+            isBootstrapped = true;
+            maybeProcessAllMailboxEntries();
+            p2pServiceListeners.stream().forEach(P2PServiceListener::onUpdatedDataReceived);
+            if (!isBootstrapped)
+                p2PDataStorage.onBootstrapComplete();
+        }

In your new code it is not executed is isBootStapped=true and not a seed node. Without having looked to a wider context that seems wrong or at lease a behaviour change.

> @@ -337,11 +340,13 @@ public void onPreliminaryDataReceived() {
 
     @Override
     public void onUpdatedDataReceived() {
-        isBootstrapped = true;
-        maybeProcessAllMailboxEntries();
-        p2pServiceListeners.stream().forEach(P2PServiceListener::onUpdatedDataReceived);
-        if(!isBootstrapped)
-            p2PDataStorage.onBootstrapComplete();
+        if (!isBootstrapped() || Capabilities.app.containsAll(Capability.SEED_NODE)) {
+            isBootstrapped = true;
+            maybeProcessAllMailboxEntries();
+            p2pServiceListeners.stream().forEach(P2PServiceListener::onUpdatedDataReceived);
+            if (!isBootstrapped)
+                p2PDataStorage.onBootstrapComplete();
+        }

Ah ignore previous comment... See in the first commit the original code...

> @@ -337,11 +340,13 @@ public void onPreliminaryDataReceived() {
 
     @Override
     public void onUpdatedDataReceived() {
-        isBootstrapped = true;
-        maybeProcessAllMailboxEntries();
-        p2pServiceListeners.stream().forEach(P2PServiceListener::onUpdatedDataReceived);
-        if(!isBootstrapped)
-            p2PDataStorage.onBootstrapComplete();
+        if (!isBootstrapped() || Capabilities.app.containsAll(Capability.SEED_NODE)) {
+            isBootstrapped = true;
+            maybeProcessAllMailboxEntries();
+            p2pServiceListeners.stream().forEach(P2PServiceListener::onUpdatedDataReceived);
+            if (!isBootstrapped)
+                p2PDataStorage.onBootstrapComplete();
+        }

Maybe worth to squash all commits, makes review easier

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/bisq-network/bisq/pull/3202#pullrequestreview-283775603
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20190904/de87a28b/attachment.html>