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.

Sign up for GitHub

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

Jump to bottom

onUpdatedDataReceived must be called in the correct order. #5141

Merged
merged 1 commit into from Feb 1, 2021
Merged

onUpdatedDataReceived must be called in the correct order. #5141

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 6 additions & 1 deletion p2p/src/main/java/bisq/network/p2p/P2PService.java
View file
Open in desktop
Expand Up @@ -147,7 +147,7 @@ public P2PService(NetworkNode networkNode,

this.networkNode.addConnectionListener(this);
this.networkNode.addMessageListener(this);
this.requestDataManager.addListener(this);
this.requestDataManager.setListener(this);

// We need to have both the initial data delivered and the hidden service published
networkReadyBinding = EasyBind.combine(hiddenServicePublished, preliminaryDataReceived,
Expand Down Expand Up @@ -317,6 +317,11 @@ public void onPreliminaryDataReceived() {
public void onUpdatedDataReceived() {
if (!isBootstrapped) {
isBootstrapped = true;
// We don't use a listener at mailboxMessageService as we require the correct
// order of execution. The p2pServiceListeners must be called after
// mailboxMessageService.onUpdatedDataReceived.
mailboxMessageService.onUpdatedDataReceived();

p2pServiceListeners.forEach(P2PServiceListener::onUpdatedDataReceived);
p2PDataStorage.onBootstrapComplete();
}
Expand Down
43 changes: 16 additions & 27 deletions p2p/src/main/java/bisq/network/p2p/mailbox/MailboxMessageService.java
View file
Open in desktop
Expand Up @@ -112,7 +112,7 @@
*/
@Singleton
@Slf4j
public class MailboxMessageService implements SetupListener, RequestDataManager.Listener, HashMapChangedListener,
public class MailboxMessageService implements SetupListener, HashMapChangedListener,
PersistedDataHost {
private static final long REPUBLISH_DELAY_SEC = TimeUnit.MINUTES.toSeconds(2);

Expand Down Expand Up @@ -155,7 +155,6 @@ public MailboxMessageService(NetworkNode networkNode,
this.clock = clock;
this.republishMailboxEntries = republishMailboxEntries;

this.requestDataManager.addListener(this);
this.networkNode.addSetupListener(this);

this.persistenceManager.initialize(mailboxMessageList, PersistenceManager.Source.PRIVATE_LOW_PRIO);
Expand Down Expand Up @@ -224,6 +223,19 @@ public void readPersisted(Runnable completeHandler) {
// API
///////////////////////////////////////////////////////////////////////////////////////////

// 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.
public void onUpdatedDataReceived() {
if (!isBootstrapped) {
isBootstrapped = true;
// Only now we start listening and processing. The p2PDataStorage is our cache for data we have received
// after the hidden service was ready.
addHashMapChangedListenerAndApply();
maybeRepublishMailBoxMessages();
}
}

public void sendEncryptedMailboxMessage(NodeAddress peer,
PubKeyRing peersPubKeyRing,
MailboxMessage mailboxMessage,
Expand All @@ -238,8 +250,9 @@ public void sendEncryptedMailboxMessage(NodeAddress peer,
"My node address must not be null at sendEncryptedMailboxMessage");
checkArgument(!keyRing.getPubKeyRing().equals(peersPubKeyRing), "We got own keyring instead of that from peer");

if (!isBootstrapped)
if (!isBootstrapped) {
throw new NetworkNotReadyException();
}

if (networkNode.getAllConnections().isEmpty()) {
sendMailboxMessageListener.onFault("There are no P2P network nodes connected. " +
Expand Down Expand Up @@ -349,30 +362,6 @@ public void onHiddenServicePublished() {
}


///////////////////////////////////////////////////////////////////////////////////////////
// RequestDataManager.Listener implementation
///////////////////////////////////////////////////////////////////////////////////////////

@Override
public void onPreliminaryDataReceived() {
}

@Override
public void onUpdatedDataReceived() {
if (!isBootstrapped) {
isBootstrapped = true;
// Only now we start listening and processing. The p2PDataStorage is our cache for data we have received
// after the hidden service was ready.
addHashMapChangedListenerAndApply();
maybeRepublishMailBoxMessages();
}
}

@Override
public void onDataReceived() {
}


///////////////////////////////////////////////////////////////////////////////////////////
// HashMapChangedListener implementation for ProtectedStorageEntry items
///////////////////////////////////////////////////////////////////////////////////////////
Expand Down
24 changes: 14 additions & 10 deletions p2p/src/main/java/bisq/network/p2p/peers/getdata/RequestDataManager.java
View file
Open in desktop
Expand Up @@ -40,11 +40,9 @@
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;

Expand All @@ -53,6 +51,7 @@
import org.jetbrains.annotations.Nullable;

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;

@Slf4j
public class RequestDataManager implements MessageListener, ConnectionListener, PeerManager.Listener {
Expand Down Expand Up @@ -91,7 +90,10 @@ default void onNoSeedNodeAvailable() {
private final P2PDataStorage dataStorage;
private final PeerManager peerManager;
private final List<NodeAddress> seedNodeAddresses;
private final Set<Listener> listeners = new HashSet<>();

// As we use Guice injection we cannot set the listener in our constructor but the P2PService calls the setListener
// in it's constructor so we can guarantee it is not null.
private Listener listener;

private final Map<NodeAddress, RequestDataHandler> handlerMap = new HashMap<>();
private final Map<String, GetDataRequestHandler> getDataRequestHandlers = new HashMap<>();
Expand Down Expand Up @@ -147,8 +149,10 @@ public void shutDown() {
// API
///////////////////////////////////////////////////////////////////////////////////////////

public void addListener(Listener listener) {
listeners.add(listener);
// We only support one listener as P2PService will manage calls on other clients in the correct order of execution.
// The listener is set from the P2PService constructor so we can guarantee it is not null.
public void setListener(Listener listener) {
this.listener = listener;
}

public boolean requestPreliminaryData() {
Expand Down Expand Up @@ -334,16 +338,16 @@ public void onComplete() {
// We delay because it can be that we get the HS published before we receive the
// preliminary data and the onPreliminaryDataReceived call triggers the
// dataUpdateRequested set to true, so we would also call the onUpdatedDataReceived.
UserThread.runAfter(() -> listeners.forEach(Listener::onPreliminaryDataReceived), 100, TimeUnit.MILLISECONDS);
UserThread.runAfter(checkNotNull(listener)::onPreliminaryDataReceived, 100, TimeUnit.MILLISECONDS);
}

// 2. Later we get a response from requestUpdatesData
if (dataUpdateRequested) {
dataUpdateRequested = false;
listeners.forEach(Listener::onUpdatedDataReceived);
checkNotNull(listener).onUpdatedDataReceived();
}

listeners.forEach(Listener::onDataReceived);
checkNotNull(listener).onDataReceived();
}

@Override
Expand Down Expand Up @@ -371,9 +375,9 @@ public void onFault(String errorMessage, @Nullable Connection connection) {
// Notify listeners
if (!nodeAddressOfPreliminaryDataRequest.isPresent()) {
if (peerManager.isSeedNode(nodeAddress)) {
listeners.forEach(Listener::onNoSeedNodeAvailable);
checkNotNull(listener).onNoSeedNodeAvailable();
} else {
listeners.forEach(Listener::onNoPeersAvailable);
checkNotNull(listener).onNoPeersAvailable();
}
}

Expand Down