Merge pull request #6376 from jmacxx/fix_issue_6367

Fix loss of mailbox messages during SPV resync
bisq-network · Nov 29, 2022 · e054083 · e054083
2 parents b10881f + 8a94642
commit e054083
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 10 deletions.
diff --git a/core/src/main/java/bisq/core/app/DomainInitialisation.java b/core/src/main/java/bisq/core/app/DomainInitialisation.java
@@ -58,6 +58,7 @@
 import bisq.core.user.User;
 
 import bisq.network.p2p.P2PService;
+import bisq.network.p2p.mailbox.MailboxMessageService;
 
 import bisq.common.ClockWatcher;
 import bisq.common.persistence.PersistenceManager;
@@ -115,6 +116,7 @@ public class DomainInitialisation {
     private final TriggerPriceService triggerPriceService;
     private final MempoolService mempoolService;
     private final OpenBsqSwapOfferService openBsqSwapOfferService;
+    private final MailboxMessageService mailboxMessageService;
 
     @Inject
     public DomainInitialisation(ClockWatcher clockWatcher,
@@ -154,7 +156,8 @@ public DomainInitialisation(ClockWatcher clockWatcher,
                                 DaoStateSnapshotService daoStateSnapshotService,
                                 TriggerPriceService triggerPriceService,
                                 MempoolService mempoolService,
-                                OpenBsqSwapOfferService openBsqSwapOfferService) {
+                                OpenBsqSwapOfferService openBsqSwapOfferService,
+                                MailboxMessageService mailboxMessageService) {
         this.clockWatcher = clockWatcher;
         this.tradeLimits = tradeLimits;
         this.arbitrationManager = arbitrationManager;
@@ -193,6 +196,7 @@ public DomainInitialisation(ClockWatcher clockWatcher,
         this.triggerPriceService = triggerPriceService;
         this.mempoolService = mempoolService;
         this.openBsqSwapOfferService = openBsqSwapOfferService;
+        this.mailboxMessageService = mailboxMessageService;
     }
 
     public void initDomainServices(Consumer<String> rejectedTxErrorMessageHandler,
@@ -277,6 +281,8 @@ public void initDomainServices(Consumer<String> rejectedTxErrorMessageHandler,
         triggerPriceService.onAllServicesInitialized();
         mempoolService.onAllServicesInitialized();
 
+        mailboxMessageService.onAllServicesInitialized();
+
         if (revolutAccountsUpdateHandler != null) {
             revolutAccountsUpdateHandler.accept(user.getPaymentAccountsAsObservable().stream()
                     .filter(paymentAccount -> paymentAccount instanceof RevolutAccount)

diff --git a/desktop/src/main/java/bisq/desktop/main/settings/network/NetworkSettingsView.java b/desktop/src/main/java/bisq/desktop/main/settings/network/NetworkSettingsView.java
@@ -502,9 +502,11 @@ private void updateBitcoinPeersTable() {
         bitcoinNetworkListItems.setAll(walletsSetup.getPeerGroup().getConnectedPeers().stream()
                 .map(BitcoinNetworkListItem::new)
                 .collect(Collectors.toList()));
-        chainHeightTextField.textProperty().setValue(Res.get("settings.net.chainHeight",
-                walletsSetup.chainHeightProperty().get(),
-                PeerGroup.getMostCommonChainHeight(walletsSetup.connectedPeersProperty().get())));
+        if (walletsSetup.connectedPeersProperty().get() != null) {
+            chainHeightTextField.textProperty().setValue(Res.get("settings.net.chainHeight",
+                    walletsSetup.chainHeightProperty().get(),
+                    PeerGroup.getMostCommonChainHeight(walletsSetup.connectedPeersProperty().get())));
+        }
     }
 }
 
diff --git a/p2p/src/main/java/bisq/network/p2p/mailbox/MailboxMessageService.java b/p2p/src/main/java/bisq/network/p2p/mailbox/MailboxMessageService.java
@@ -130,6 +130,8 @@ public class MailboxMessageService implements HashMapChangedListener, PersistedD
     private final Map<String, MailboxItem> mailboxItemsByUid = new HashMap<>();
 
     private boolean isBootstrapped;
+    private boolean allServicesInitialized;
+    private boolean initAfterBootstrapped;
 
     @Inject
     public MailboxMessageService(NetworkNode networkNode,
@@ -236,6 +238,12 @@ public void readPersisted(Runnable completeHandler) {
     // API
     ///////////////////////////////////////////////////////////////////////////////////////////
 
+    // We wait until all services are ready to avoid some edge cases as in https://github.com/bisq-network/bisq/issues/6367
+    public void onAllServicesInitialized() {
+        allServicesInitialized = true;
+        init();
+    }
+
     // We don't listen on requestDataManager directly as we require the correct
     // order of execution. The p2pService is handling the correct order of execution and we get called
     // directly from there.
@@ -247,11 +255,18 @@ public void onBootstrapped() {
 
     // second stage starup for MailboxMessageService ... apply existing messages to their modules
     public void initAfterBootstrapped() {
-        // Only now we start listening and processing. The p2PDataStorage is our cache for data we have received
-        // after the hidden service was ready.
-        addHashMapChangedListener();
-        onAdded(p2PDataStorage.getMap().values());
-        maybeRepublishMailBoxMessages();
+        initAfterBootstrapped = true;
+        init();
+    }
+
+    private void init() {
+        if (allServicesInitialized && initAfterBootstrapped) {
+            // Only now we start listening and processing. The p2PDataStorage is our cache for data we have received
+            // after the hidden service was ready.
+            addHashMapChangedListener();
+            onAdded(p2PDataStorage.getMap().values());
+            maybeRepublishMailBoxMessages();
+        }
     }
 
 
@@ -498,7 +513,7 @@ private void processMyMailboxItem(MailboxItem mailboxItem, String uid) {
                 mailboxMessage.getClass().getSimpleName(), uid, sender);
         decryptedMailboxListeners.forEach(e -> e.onMailboxMessageAdded(decryptedMessageWithPubKey, sender));
 
-        if (isBootstrapped) {
+        if (allServicesInitialized && isBootstrapped) {     // GH ISSUE 6367 only remove after fully initialized
             // After we notified our listeners we remove the data immediately from the network.
             // In case the client has not been ready it need to take it via getMailBoxMessages.
             // We do not remove the data from our local map at that moment. This has to be called explicitely from the