diff --git a/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/permissioning/NodesWhitelistAcceptanceTest.java b/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/permissioning/NodesWhitelistAcceptanceTest.java index de347cdce2..f30e831abf 100644 --- a/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/permissioning/NodesWhitelistAcceptanceTest.java +++ b/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/permissioning/NodesWhitelistAcceptanceTest.java @@ -22,6 +22,7 @@ import java.net.URI; import java.util.Collections; +import com.google.common.collect.Lists; import org.junit.Before; import org.junit.Test; @@ -53,6 +54,18 @@ public void permissionedNodeShouldDiscoverOnlyAllowedNode() { permissionedNode.verify(net.awaitPeerCount(1)); } + @Test + public void permissionedNodeShouldDisconnectFromNodeRemovedFromWhitelist() { + permissionedNode.verify(net.awaitPeerCount(1)); + + // remove allowed node from the whitelist + permissionedNode.verify( + perm.removeNodesFromWhitelist(Lists.newArrayList(((PantheonNode) allowedNode).enodeUrl()))); + + // node should not be connected to any peers + permissionedNode.verify(net.awaitPeerCount(0)); + } + private URI getEnodeURI(final Node node) { return URI.create(((PantheonNode) node).getConfiguration().enodeUrl()); } diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryAgent.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryAgent.java index bd18cc0b56..6731f9d971 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryAgent.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryAgent.java @@ -23,6 +23,7 @@ import tech.pegasys.pantheon.ethereum.p2p.api.PeerConnection; import tech.pegasys.pantheon.ethereum.p2p.config.DiscoveryConfiguration; import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryEvent.PeerBondedEvent; +import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryEvent.PeerDroppedEvent; import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.Packet; import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerDiscoveryController; import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerRequirement; @@ -86,6 +87,7 @@ public abstract class PeerDiscoveryAgent implements DisconnectCallback { /* Is discovery enabled? */ private boolean isActive = false; private final Subscribers> peerBondedObservers = new Subscribers<>(); + private final Subscribers> peerDroppedObservers = new Subscribers<>(); public PeerDiscoveryAgent( final SECP256K1.KeyPair keyPair, @@ -164,7 +166,8 @@ private PeerDiscoveryController createController() { peerRequirement, peerBlacklist, nodeWhitelistController, - peerBondedObservers); + peerBondedObservers, + peerDroppedObservers); } protected boolean validatePacketSize(final int packetSize) { @@ -261,6 +264,29 @@ public boolean removePeerBondedObserver(final long observerId) { return peerBondedObservers.unsubscribe(observerId); } + /** + * Adds an observer that will get called when a peer is dropped from the peer table. + * + *

No guarantees are made about the order in which observers are invoked. + * + * @param observer The observer to call. + * @return A unique ID identifying this observer, to that it can be removed later. + */ + public long observePeerDroppedEvents(final Consumer observer) { + checkNotNull(observer); + return peerDroppedObservers.subscribe(observer); + } + + /** + * Removes an previously added peer dropped observer. + * + * @param observerId The unique ID identifying the observer to remove. + * @return Whether the observer was located and removed. + */ + public boolean removePeerDroppedObserver(final long observerId) { + return peerDroppedObservers.unsubscribe(observerId); + } + /** * Returns the count of observers that are registered on this controller. * @@ -292,7 +318,7 @@ public void onDisconnect( final DisconnectMessage.DisconnectReason reason, final boolean initiatedByPeer) { final BytesValue nodeId = connection.getPeer().getNodeId(); - peerTable.evict(new DefaultPeerId(nodeId)); + peerTable.tryEvict(new DefaultPeerId(nodeId)); } /** diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/Bucket.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/Bucket.java index 7870f42feb..ca67ddd351 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/Bucket.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/Bucket.java @@ -118,7 +118,8 @@ synchronized boolean evict(final PeerId peer) { } // If found, shift all subsequent elements to the left, and decrement tailIndex. for (int i = 0; i <= tailIndex; i++) { - if (peer.equals(kBucket[i])) { + // Peer comparison here must be done by peer id + if (peer.getId().equals(kBucket[i].getId())) { arraycopy(kBucket, i + 1, kBucket, i, tailIndex - i); kBucket[tailIndex--] = null; return true; diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/DiscoveryProtocolLogger.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/DiscoveryProtocolLogger.java new file mode 100644 index 0000000000..0a57c6bda9 --- /dev/null +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/DiscoveryProtocolLogger.java @@ -0,0 +1,55 @@ +/* + * Copyright 2019 ConsenSys AG. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ +package tech.pegasys.pantheon.ethereum.p2p.discovery.internal; + +import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +public class DiscoveryProtocolLogger { + + private static final Logger LOG = LogManager.getLogger(); + + static void logSendingPacket(final Peer peer, final Packet packet) { + LOG.trace( + "<<< Sending {} packet from peer {} ({}): {}", + shortenPacketType(packet), + peer.getId().slice(0, 16), + peer.getEndpoint(), + packet); + } + + static void logReceivedPacket(final Peer peer, final Packet packet) { + LOG.trace( + ">>> Received {} packet from peer {} ({}): {}", + shortenPacketType(packet), + peer.getId().slice(0, 16), + peer.getEndpoint(), + packet); + } + + private static String shortenPacketType(final Packet packet) { + switch (packet.getType()) { + case PING: + return "PING "; + case PONG: + return "PONG "; + case FIND_NEIGHBORS: + return "FINDN"; + case NEIGHBORS: + return "NEIGH"; + } + return null; + } +} diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryController.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryController.java index 768fead1c3..7b53f389fc 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryController.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryController.java @@ -14,17 +14,23 @@ import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.concurrent.TimeUnit.SECONDS; -import static tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerTable.AddResult.Outcome; +import static tech.pegasys.pantheon.ethereum.p2p.discovery.internal.DiscoveryProtocolLogger.logReceivedPacket; +import static tech.pegasys.pantheon.ethereum.p2p.discovery.internal.DiscoveryProtocolLogger.logSendingPacket; +import static tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerTable.AddResult.AddOutcome; import tech.pegasys.pantheon.crypto.SECP256K1; import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair; import tech.pegasys.pantheon.ethereum.p2p.discovery.DiscoveryPeer; import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryEvent; import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryEvent.PeerBondedEvent; +import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryEvent.PeerDroppedEvent; import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryStatus; +import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerTable.EvictResult; +import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerTable.EvictResult.EvictOutcome; import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; import tech.pegasys.pantheon.ethereum.p2p.peers.PeerBlacklist; import tech.pegasys.pantheon.ethereum.permissioning.NodeWhitelistController; +import tech.pegasys.pantheon.ethereum.permissioning.node.NodeWhitelistUpdatedEvent; import tech.pegasys.pantheon.util.Subscribers; import tech.pegasys.pantheon.util.bytes.BytesValue; @@ -123,6 +129,7 @@ public class PeerDiscoveryController { // Observers for "peer bonded" discovery events. private final Subscribers> peerBondedObservers; + private final Subscribers> peerDroppedObservers; private RecursivePeerRefreshState recursivePeerRefreshState; @@ -137,7 +144,8 @@ public PeerDiscoveryController( final PeerRequirement peerRequirement, final PeerBlacklist peerBlacklist, final Optional nodeWhitelistController, - final Subscribers> peerBondedObservers) { + final Subscribers> peerBondedObservers, + final Subscribers> peerDroppedObservers) { this.timerUtil = timerUtil; this.keypair = keypair; this.localPeer = localPeer; @@ -149,6 +157,7 @@ public PeerDiscoveryController( this.nodeWhitelistController = nodeWhitelistController; this.outboundMessageHandler = outboundMessageHandler; this.peerBondedObservers = peerBondedObservers; + this.peerDroppedObservers = peerDroppedObservers; } public CompletableFuture start() { @@ -184,6 +193,9 @@ public CompletableFuture start() { this::refreshTableIfRequired); tableRefreshTimerId = OptionalLong.of(timerId); + nodeWhitelistController.ifPresent( + c -> c.subscribeToListUpdatedEvent(this::handleNodeWhitelistUpdatedEvent)); + return CompletableFuture.completedFuture(null); } @@ -217,12 +229,7 @@ private boolean whitelistIfPresentIsNodePermitted(final DiscoveryPeer sender) { * @param sender The sender. */ public void onMessage(final Packet packet, final DiscoveryPeer sender) { - LOG.trace( - "<<< Received {} discovery packet from {} ({}): {}", - packet.getType(), - sender.getEndpoint(), - sender.getId().slice(0, 16), - packet); + logReceivedPacket(sender, packet); // Message from self. This should not happen. if (sender.getId().equals(localPeer.getId())) { @@ -242,14 +249,12 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) { switch (packet.getType()) { case PING: - LOG.trace("Received PING packet from {}", sender.getEnodeURI()); if (!peerBlacklisted && addToPeerTable(peer)) { final PingPacketData ping = packet.getPacketData(PingPacketData.class).get(); respondToPing(ping, packet.getHash(), peer); } break; case PONG: - LOG.trace("Received PONG packet from {}", sender.getEnodeURI()); matchInteraction(packet) .ifPresent( interaction -> { @@ -261,7 +266,6 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) { }); break; case NEIGHBORS: - LOG.trace("Received NEIGHBORS packet from {}", sender.getEnodeURI()); matchInteraction(packet) .ifPresent( interaction -> @@ -269,7 +273,6 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) { peer, packet.getPacketData(NeighborsPacketData.class).orElse(null))); break; case FIND_NEIGHBORS: - LOG.trace("Received FIND_NEIGHBORS packet from {}", sender.getEnodeURI()); if (!peerKnown || peerBlacklisted) { break; } @@ -282,7 +285,7 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) { private boolean addToPeerTable(final DiscoveryPeer peer) { final PeerTable.AddResult result = peerTable.tryAdd(peer); - if (result.getOutcome() == Outcome.SELF) { + if (result.getOutcome() == AddOutcome.SELF) { return false; } @@ -298,23 +301,45 @@ private boolean addToPeerTable(final DiscoveryPeer peer) { notifyPeerBonded(peer, now); } - if (result.getOutcome() == Outcome.ALREADY_EXISTED) { + if (result.getOutcome() == AddOutcome.ALREADY_EXISTED) { // Bump peer. - peerTable.evict(peer); + peerTable.tryEvict(peer); peerTable.tryAdd(peer); - } else if (result.getOutcome() == Outcome.BUCKET_FULL) { - peerTable.evict(result.getEvictionCandidate()); + } else if (result.getOutcome() == AddOutcome.BUCKET_FULL) { + peerTable.tryEvict(result.getEvictionCandidate()); peerTable.tryAdd(peer); } return true; } + private void handleNodeWhitelistUpdatedEvent(final NodeWhitelistUpdatedEvent event) { + event.getRemovedNodes().stream() + .map(e -> new DiscoveryPeer(DiscoveryPeer.fromURI(e.toURI()))) + .forEach(this::dropFromPeerTable); + } + + @VisibleForTesting + boolean dropFromPeerTable(final DiscoveryPeer peer) { + final EvictResult evictResult = peerTable.tryEvict(peer); + if (evictResult.getOutcome() == EvictOutcome.EVICTED) { + notifyPeerDropped(peer, System.currentTimeMillis()); + return true; + } else { + return false; + } + } + private void notifyPeerBonded(final DiscoveryPeer peer, final long now) { final PeerBondedEvent event = new PeerBondedEvent(peer, now); dispatchEvent(peerBondedObservers, event); } + private void notifyPeerDropped(final DiscoveryPeer peer, final long now) { + final PeerDroppedEvent event = new PeerDroppedEvent(peer, now); + dispatchEvent(peerDroppedObservers, event); + } + private Optional matchInteraction(final Packet packet) { final PeerInteractionState interaction = inflightInteractions.get(packet.getNodeId()); if (interaction == null || !interaction.test(packet)) { @@ -389,10 +414,12 @@ void bond(final DiscoveryPeer peer) { private void sendPacket(final DiscoveryPeer peer, final PacketType type, final PacketData data) { Packet packet = createPacket(type, data); + logSendingPacket(peer, packet); outboundMessageHandler.send(peer, packet); } private void sendPacket(final DiscoveryPeer peer, final Packet packet) { + logSendingPacket(peer, packet); outboundMessageHandler.send(peer, packet); } diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerTable.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerTable.java index 384f2a6004..daf346185c 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerTable.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerTable.java @@ -20,6 +20,7 @@ import tech.pegasys.pantheon.crypto.Hash; import tech.pegasys.pantheon.ethereum.p2p.discovery.DiscoveryPeer; import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryStatus; +import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerTable.AddResult.AddOutcome; import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; import tech.pegasys.pantheon.ethereum.p2p.peers.PeerId; import tech.pegasys.pantheon.util.bytes.BytesValue; @@ -103,7 +104,7 @@ public Optional get(final PeerId peer) { *

  • the operation failed because the peer already existed. * * - * @see AddResult.Outcome + * @see AddOutcome * @param peer The peer to add. * @return An object indicating the outcome of the operation. */ @@ -145,20 +146,33 @@ public AddResult tryAdd(final DiscoveryPeer peer) { * @param peer The peer to evict. * @return Whether the peer existed, and hence the eviction took place. */ - public boolean evict(final PeerId peer) { + public EvictResult tryEvict(final PeerId peer) { final BytesValue id = peer.getId(); final int distance = distanceFrom(peer); + + if (distance == 0) { + return EvictResult.self(); + } + distanceCache.remove(id); + if (table[distance].peers().isEmpty()) { + return EvictResult.absent(); + } + final boolean evicted = table[distance].evict(peer); - evictionCnt += evicted ? 1 : 0; + if (evicted) { + evictionCnt++; + } else { + return EvictResult.absent(); + } // Trigger the bloom filter regeneration if needed. if (evictionCnt >= BLOOM_FILTER_REGENERATION_THRESHOLD) { ForkJoinPool.commonPool().execute(this::buildBloomFilter); } - return evicted; + return EvictResult.evicted(); } private void buildBloomFilter() { @@ -206,7 +220,7 @@ private int distanceFrom(final PeerId peer) { /** A class that encapsulates the result of a peer addition to the table. */ public static class AddResult { /** The outcome of the operation. */ - public enum Outcome { + public enum AddOutcome { /** The peer was added successfully to its corresponding k-bucket. */ ADDED, @@ -221,31 +235,31 @@ public enum Outcome { SELF } - private final Outcome outcome; + private final AddOutcome outcome; private final Peer evictionCandidate; - private AddResult(final Outcome outcome, final Peer evictionCandidate) { + private AddResult(final AddOutcome outcome, final Peer evictionCandidate) { this.outcome = outcome; this.evictionCandidate = evictionCandidate; } static AddResult added() { - return new AddResult(Outcome.ADDED, null); + return new AddResult(AddOutcome.ADDED, null); } static AddResult bucketFull(final Peer evictionCandidate) { - return new AddResult(Outcome.BUCKET_FULL, evictionCandidate); + return new AddResult(AddOutcome.BUCKET_FULL, evictionCandidate); } static AddResult existed() { - return new AddResult(Outcome.ALREADY_EXISTED, null); + return new AddResult(AddOutcome.ALREADY_EXISTED, null); } static AddResult self() { - return new AddResult(Outcome.SELF, null); + return new AddResult(AddOutcome.SELF, null); } - public Outcome getOutcome() { + public AddOutcome getOutcome() { return outcome; } @@ -253,4 +267,34 @@ public Peer getEvictionCandidate() { return evictionCandidate; } } + + static class EvictResult { + public enum EvictOutcome { + EVICTED, + ABSENT, + SELF + } + + private final EvictOutcome outcome; + + private EvictResult(final EvictOutcome outcome) { + this.outcome = outcome; + } + + static EvictResult evicted() { + return new EvictResult(EvictOutcome.EVICTED); + } + + static EvictResult absent() { + return new EvictResult(EvictOutcome.ABSENT); + } + + static EvictResult self() { + return new EvictResult(EvictOutcome.SELF); + } + + EvictOutcome getOutcome() { + return outcome; + } + } } diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java index 266b5be8e3..67276106ea 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java @@ -22,6 +22,8 @@ import tech.pegasys.pantheon.ethereum.p2p.config.NetworkingConfiguration; import tech.pegasys.pantheon.ethereum.p2p.discovery.DiscoveryPeer; import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryAgent; +import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryEvent.PeerBondedEvent; +import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryEvent.PeerDroppedEvent; import tech.pegasys.pantheon.ethereum.p2p.discovery.VertxPeerDiscoveryAgent; import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerRequirement; import tech.pegasys.pantheon.ethereum.p2p.peers.DefaultPeer; @@ -125,6 +127,7 @@ public class NettyP2PNetwork implements P2PNetwork { private final PeerDiscoveryAgent peerDiscoveryAgent; private final PeerBlacklist peerBlacklist; private OptionalLong peerBondedObserverId = OptionalLong.empty(); + private OptionalLong peerDroppedObserverId = OptionalLong.empty(); @VisibleForTesting public final Collection peerMaintainConnectionList; @@ -432,23 +435,37 @@ public void subscribeDisconnect(final DisconnectCallback callback) { public void run() { try { peerDiscoveryAgent.start().join(); - final long observerId = - peerDiscoveryAgent.observePeerBondedEvents( - peerBondedEvent -> { - final Peer peer = peerBondedEvent.getPeer(); - if (connectionCount() < maxPeers - && peer.getEndpoint().getTcpPort().isPresent() - && !isConnecting(peer) - && !isConnected(peer)) { - connect(peer); - } - }); - peerBondedObserverId = OptionalLong.of(observerId); + peerBondedObserverId = + OptionalLong.of(peerDiscoveryAgent.observePeerBondedEvents(handlePeerBondedEvent())); + peerDroppedObserverId = + OptionalLong.of(peerDiscoveryAgent.observePeerDroppedEvents(handlePeerDroppedEvents())); } catch (final Exception ex) { throw new IllegalStateException(ex); } } + private Consumer handlePeerBondedEvent() { + return event -> { + final Peer peer = event.getPeer(); + if (connectionCount() < maxPeers + && peer.getEndpoint().getTcpPort().isPresent() + && !isConnecting(peer) + && !isConnected(peer)) { + connect(peer); + } + }; + } + + private Consumer handlePeerDroppedEvents() { + return event -> { + final Peer peer = event.getPeer(); + getPeers().stream() + .filter(p -> p.getPeer().getNodeId().equals(peer.getId())) + .findFirst() + .ifPresent(p -> p.disconnect(DisconnectReason.REQUESTED)); + }; + } + private boolean isConnecting(final Peer peer) { return pendingConnections.containsKey(peer); } @@ -463,6 +480,8 @@ public void stop() { peerDiscoveryAgent.stop().join(); peerBondedObserverId.ifPresent(peerDiscoveryAgent::removePeerBondedObserver); peerBondedObserverId = OptionalLong.empty(); + peerDroppedObserverId.ifPresent(peerDiscoveryAgent::removePeerDroppedObserver); + peerDroppedObserverId = OptionalLong.empty(); peerDiscoveryAgent.stop().join(); workers.shutdownGracefully(); boss.shutdownGracefully(); diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryTimestampsTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryTimestampsTest.java index b805784890..fd65d3e84f 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryTimestampsTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryTimestampsTest.java @@ -62,6 +62,7 @@ public void lastSeenAndFirstDiscoveredTimestampsUpdatedOnMessage() { () -> true, new PeerBlacklist(), Optional.empty(), + new Subscribers<>(), new Subscribers<>()); controller.start(); diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java index 94e23aad50..e024d28551 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java @@ -25,13 +25,17 @@ import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; import tech.pegasys.pantheon.crypto.SECP256K1; import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair; import tech.pegasys.pantheon.ethereum.p2p.discovery.DiscoveryPeer; +import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryEvent.PeerBondedEvent; +import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryEvent.PeerDroppedEvent; import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryStatus; import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryTestHelper; +import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerTable.EvictResult; import tech.pegasys.pantheon.ethereum.p2p.peers.Endpoint; import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; import tech.pegasys.pantheon.ethereum.p2p.peers.PeerBlacklist; @@ -45,6 +49,7 @@ import tech.pegasys.pantheon.util.uint.UInt256Value; import java.io.IOException; +import java.net.URI; import java.nio.file.Files; import java.util.ArrayList; import java.util.Arrays; @@ -57,8 +62,10 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Consumer; import java.util.stream.Collectors; +import com.google.common.collect.Lists; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -1027,6 +1034,84 @@ public void shouldNotRespondToPingFromNonWhitelistedDiscoveryPeer() throws IOExc assertThat(controller.getPeers()).doesNotContain(peers.get(0)); } + @Test + public void whenObservingNodeWhitelistAndNodeIsRemovedShouldEvictPeerFromPeerTable() + throws IOException { + final PeerTable peerTableSpy = spy(peerTable); + final List peers = createPeersInLastBucket(localPeer, 1); + final DiscoveryPeer peer = peers.get(0); + peerTableSpy.tryAdd(peer); + + final PermissioningConfiguration config = permissioningConfigurationWithTempFile(); + final URI peerURI = URI.create(peer.getEnodeURI()); + config.setNodeWhitelist(Lists.newArrayList(peerURI)); + final NodeWhitelistController nodeWhitelistController = new NodeWhitelistController(config); + + controller = + getControllerBuilder().whitelist(nodeWhitelistController).peerTable(peerTableSpy).build(); + + controller.start(); + nodeWhitelistController.removeNodes(Lists.newArrayList(peerURI.toString())); + + verify(peerTableSpy).tryEvict(eq(DiscoveryPeer.fromURI(peerURI))); + } + + @Test + @SuppressWarnings({"unchecked", "rawtypes"}) + public void whenObservingNodeWhitelistAndNodeIsRemovedShouldNotifyPeerDroppedObservers() + throws IOException { + final PeerTable peerTableSpy = spy(peerTable); + final List peers = createPeersInLastBucket(localPeer, 1); + final DiscoveryPeer peer = peers.get(0); + peerTableSpy.tryAdd(peer); + + final PermissioningConfiguration config = permissioningConfigurationWithTempFile(); + final URI peerURI = URI.create(peer.getEnodeURI()); + config.setNodeWhitelist(Lists.newArrayList(peerURI)); + final NodeWhitelistController nodeWhitelistController = new NodeWhitelistController(config); + + final Consumer peerDroppedEventConsumer = mock(Consumer.class); + final Subscribers> peerDroppedSubscribers = new Subscribers(); + peerDroppedSubscribers.subscribe(peerDroppedEventConsumer); + + doReturn(EvictResult.evicted()).when(peerTableSpy).tryEvict(any()); + + controller = + getControllerBuilder() + .whitelist(nodeWhitelistController) + .peerTable(peerTableSpy) + .peerDroppedObservers(peerDroppedSubscribers) + .build(); + + controller.start(); + nodeWhitelistController.removeNodes(Lists.newArrayList(peerURI.toString())); + + ArgumentCaptor captor = ArgumentCaptor.forClass(PeerDroppedEvent.class); + verify(peerDroppedEventConsumer).accept(captor.capture()); + assertThat(captor.getValue().getPeer()).isEqualTo(DiscoveryPeer.fromURI(peer.getEnodeURI())); + } + + @Test + @SuppressWarnings({"unchecked", "rawtypes"}) + public void whenPeerIsNotEvictedDropFromTableShouldReturnFalseAndNotifyZeroObservers() { + final List peers = createPeersInLastBucket(localPeer, 1); + final DiscoveryPeer peer = peers.get(0); + final PeerTable peerTableSpy = spy(peerTable); + final Consumer peerDroppedEventConsumer = mock(Consumer.class); + final Subscribers> peerDroppedSubscribers = new Subscribers(); + peerDroppedSubscribers.subscribe(peerDroppedEventConsumer); + + doReturn(EvictResult.absent()).when(peerTableSpy).tryEvict(any()); + + controller = getControllerBuilder().peerDroppedObservers(peerDroppedSubscribers).build(); + + controller.start(); + boolean dropped = controller.dropFromPeerTable(peer); + + assertThat(dropped).isFalse(); + verifyZeroInteractions(peerDroppedEventConsumer); + } + private static Packet mockPingPacket(final Peer from, final Peer to) { final Packet packet = mock(Packet.class); @@ -1103,6 +1188,8 @@ static class ControllerBuilder { private PeerTable peerTable; private OutboundMessageHandler outboundMessageHandler = OutboundMessageHandler.NOOP; private static final PeerDiscoveryTestHelper helper = new PeerDiscoveryTestHelper(); + private Subscribers> peerBondedObservers = new Subscribers<>(); + private Subscribers> peerDroppedObservers = new Subscribers<>(); public static ControllerBuilder create() { return new ControllerBuilder(); @@ -1153,6 +1240,17 @@ ControllerBuilder outboundMessageHandler(final OutboundMessageHandler outboundMe return this; } + ControllerBuilder peerBondedObservers(final Subscribers> observers) { + this.peerBondedObservers = observers; + return this; + } + + ControllerBuilder peerDroppedObservers( + final Subscribers> observers) { + this.peerDroppedObservers = observers; + return this; + } + PeerDiscoveryController build() { checkNotNull(keypair); if (localPeer == null) { @@ -1173,7 +1271,8 @@ PeerDiscoveryController build() { PEER_REQUIREMENT, blacklist, whitelist, - new Subscribers<>())); + peerBondedObservers, + peerDroppedObservers)); } } } diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryTableRefreshTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryTableRefreshTest.java index 18e71e6599..c5c9794dab 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryTableRefreshTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryTableRefreshTest.java @@ -64,6 +64,7 @@ public void tableRefreshSingleNode() { () -> true, new PeerBlacklist(), Optional.empty(), + new Subscribers<>(), new Subscribers<>())); controller.start(); diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerTableTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerTableTest.java index fbd9dd13b1..f76c17c812 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerTableTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerTableTest.java @@ -16,7 +16,9 @@ import tech.pegasys.pantheon.ethereum.p2p.discovery.DiscoveryPeer; import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryTestHelper; -import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerTable.AddResult.Outcome; +import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerTable.AddResult.AddOutcome; +import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerTable.EvictResult; +import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerTable.EvictResult.EvictOutcome; import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; import java.util.List; @@ -33,7 +35,7 @@ public void addPeer() { for (final DiscoveryPeer peer : peers) { final PeerTable.AddResult result = table.tryAdd(peer); - assertThat(result.getOutcome()).isEqualTo(Outcome.ADDED); + assertThat(result.getOutcome()).isEqualTo(AddOutcome.ADDED); } assertThat(table.getAllPeers()).hasSize(5); @@ -45,7 +47,7 @@ public void addSelf() { final PeerTable table = new PeerTable(localPeer.getId(), 16); final PeerTable.AddResult result = table.tryAdd(localPeer); - assertThat(result.getOutcome()).isEqualTo(Outcome.SELF); + assertThat(result.getOutcome()).isEqualTo(AddOutcome.SELF); assertThat(table.getAllPeers()).hasSize(0); } @@ -54,13 +56,53 @@ public void peerExists() { final PeerTable table = new PeerTable(Peer.randomId(), 16); final DiscoveryPeer peer = helper.createDiscoveryPeer(); - assertThat(table.tryAdd(peer).getOutcome()).isEqualTo(Outcome.ADDED); + assertThat(table.tryAdd(peer).getOutcome()).isEqualTo(AddOutcome.ADDED); assertThat(table.tryAdd(peer)) .satisfies( result -> { - assertThat(result.getOutcome()).isEqualTo(Outcome.ALREADY_EXISTED); + assertThat(result.getOutcome()).isEqualTo(AddOutcome.ALREADY_EXISTED); assertThat(result.getEvictionCandidate()).isNull(); }); } + + @Test + public void evictExistingPeerShouldEvict() { + final PeerTable table = new PeerTable(Peer.randomId(), 16); + final DiscoveryPeer peer = helper.createDiscoveryPeer(); + + table.tryAdd(peer); + + EvictResult evictResult = table.tryEvict(peer); + assertThat(evictResult.getOutcome()).isEqualTo(EvictOutcome.EVICTED); + } + + @Test + public void evictPeerFromEmptyTableShouldNotEvict() { + final PeerTable table = new PeerTable(Peer.randomId(), 16); + final DiscoveryPeer peer = helper.createDiscoveryPeer(); + + EvictResult evictResult = table.tryEvict(peer); + assertThat(evictResult.getOutcome()).isEqualTo(EvictOutcome.ABSENT); + } + + @Test + public void evictAbsentPeerShouldNotEvict() { + final PeerTable table = new PeerTable(Peer.randomId(), 16); + final DiscoveryPeer peer = helper.createDiscoveryPeer(); + final List otherPeers = helper.createDiscoveryPeers(5); + otherPeers.forEach(table::tryAdd); + + EvictResult evictResult = table.tryEvict(peer); + assertThat(evictResult.getOutcome()).isEqualTo(EvictOutcome.ABSENT); + } + + @Test + public void evictSelfPeerShouldReturnSelfOutcome() { + final DiscoveryPeer peer = helper.createDiscoveryPeer(); + final PeerTable table = new PeerTable(peer.getId(), 16); + + EvictResult evictResult = table.tryEvict(peer); + assertThat(evictResult.getOutcome()).isEqualTo(EvictOutcome.SELF); + } } diff --git a/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/NodeWhitelistController.java b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/NodeWhitelistController.java index 7c515f4ccd..63604e9de5 100644 --- a/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/NodeWhitelistController.java +++ b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/NodeWhitelistController.java @@ -12,15 +12,19 @@ */ package tech.pegasys.pantheon.ethereum.permissioning; +import tech.pegasys.pantheon.ethereum.permissioning.node.NodeWhitelistUpdatedEvent; +import tech.pegasys.pantheon.util.Subscribers; import tech.pegasys.pantheon.util.enode.EnodeURL; import java.io.IOException; import java.net.URI; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Optional; +import java.util.function.Consumer; import java.util.stream.Collectors; import com.google.common.annotations.VisibleForTesting; @@ -34,6 +38,8 @@ public class NodeWhitelistController { private PermissioningConfiguration configuration; private List nodesWhitelist = new ArrayList<>(); private final WhitelistPersistor whitelistPersistor; + private final Subscribers> nodeWhitelistUpdatedObservers = + new Subscribers<>(); public NodeWhitelistController(final PermissioningConfiguration permissioningConfiguration) { this( @@ -73,6 +79,7 @@ public NodesWhitelistResult addNodes(final List enodeURLs) { final List oldWhitelist = new ArrayList<>(this.nodesWhitelist); peers.forEach(this::addNode); + notifyListUpdatedSubscribers(new NodeWhitelistUpdatedEvent(peers, Collections.emptyList())); final NodesWhitelistResult updateConfigFileResult = updateWhitelistInConfigFile(oldWhitelist); if (updateConfigFileResult.result() != WhitelistOperationResult.SUCCESS) { @@ -103,6 +110,7 @@ public NodesWhitelistResult removeNodes(final List enodeURLs) { final List oldWhitelist = new ArrayList<>(this.nodesWhitelist); peers.forEach(this::removeNode); + notifyListUpdatedSubscribers(new NodeWhitelistUpdatedEvent(Collections.emptyList(), peers)); final NodesWhitelistResult updateConfigFileResult = updateWhitelistInConfigFile(oldWhitelist); if (updateConfigFileResult.result() != WhitelistOperationResult.SUCCESS) { @@ -206,6 +214,8 @@ public synchronized void reload() throws RuntimeException { readNodesFromConfig(updatedConfig); configuration = updatedConfig; + + createNodeWhitelistModifiedEventAfterReload(currentAccountsList, nodesWhitelist); } catch (Exception e) { LOG.warn( "Error reloading permissions file. In-memory whitelisted nodes will be reverted to previous valid configuration. " @@ -217,6 +227,36 @@ public synchronized void reload() throws RuntimeException { } } + private void createNodeWhitelistModifiedEventAfterReload( + final List previousNodeWhitelist, final List currentNodesList) { + final List removedNodes = + previousNodeWhitelist.stream() + .filter(n -> !currentNodesList.contains(n)) + .collect(Collectors.toList()); + + final List addedNodes = + currentNodesList.stream() + .filter(n -> !previousNodeWhitelist.contains(n)) + .collect(Collectors.toList()); + + if (!removedNodes.isEmpty() || !addedNodes.isEmpty()) { + notifyListUpdatedSubscribers(new NodeWhitelistUpdatedEvent(addedNodes, removedNodes)); + } + } + + public long subscribeToListUpdatedEvent(final Consumer subscriber) { + return nodeWhitelistUpdatedObservers.subscribe(subscriber); + } + + private void notifyListUpdatedSubscribers(final NodeWhitelistUpdatedEvent event) { + LOG.trace( + "Sending NodeWhitelistUpdatedEvent (added: {}, removed {})", + event.getAddedNodes().size(), + event.getRemovedNodes().size()); + + nodeWhitelistUpdatedObservers.forEach(c -> c.accept(event)); + } + public static class NodesWhitelistResult { private final WhitelistOperationResult result; private final Optional message; diff --git a/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/node/NodeWhitelistUpdatedEvent.java b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/node/NodeWhitelistUpdatedEvent.java new file mode 100644 index 0000000000..4654c5a189 --- /dev/null +++ b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/node/NodeWhitelistUpdatedEvent.java @@ -0,0 +1,58 @@ +/* + * Copyright 2019 ConsenSys AG. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ +package tech.pegasys.pantheon.ethereum.permissioning.node; + +import tech.pegasys.pantheon.util.enode.EnodeURL; + +import java.util.Collections; +import java.util.List; + +import com.google.common.base.Objects; + +public class NodeWhitelistUpdatedEvent { + + private final List addedNodes; + private final List removedNodes; + + public NodeWhitelistUpdatedEvent( + final List addedNodes, final List removedNodes) { + this.addedNodes = addedNodes != null ? addedNodes : Collections.emptyList(); + this.removedNodes = removedNodes != null ? removedNodes : Collections.emptyList(); + } + + public List getAddedNodes() { + return addedNodes; + } + + public List getRemovedNodes() { + return removedNodes; + } + + @Override + public boolean equals(final Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + NodeWhitelistUpdatedEvent that = (NodeWhitelistUpdatedEvent) o; + return Objects.equal(addedNodes, that.addedNodes) + && Objects.equal(removedNodes, that.removedNodes); + } + + @Override + public int hashCode() { + return Objects.hashCode(addedNodes, removedNodes); + } +} diff --git a/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/NodeWhitelistControllerTest.java b/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/NodeWhitelistControllerTest.java index c02dfdf5cb..ee3599d3b2 100644 --- a/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/NodeWhitelistControllerTest.java +++ b/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/NodeWhitelistControllerTest.java @@ -16,14 +16,17 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.catchThrowable; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; import static tech.pegasys.pantheon.ethereum.permissioning.NodeWhitelistController.NodesWhitelistResult; +import tech.pegasys.pantheon.ethereum.permissioning.node.NodeWhitelistUpdatedEvent; import tech.pegasys.pantheon.util.enode.EnodeURL; import java.io.IOException; @@ -33,7 +36,9 @@ import java.nio.file.Path; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; +import java.util.function.Consumer; import com.google.common.collect.Lists; import org.junit.Before; @@ -59,6 +64,15 @@ public void setUp() { new NodeWhitelistController(PermissioningConfiguration.createDefault(), whitelistPersistor); } + @Test + public void whenAddNodesWithValidInputShouldReturnSuccess() { + NodesWhitelistResult expected = new NodesWhitelistResult(WhitelistOperationResult.SUCCESS); + NodesWhitelistResult actualResult = controller.addNodes(Lists.newArrayList(enode1)); + + assertThat(actualResult).isEqualToComparingOnlyGivenFields(expected, "result"); + assertThat(controller.getNodesWhitelist()).containsExactly(enode1); + } + @Test public void whenAddNodesInputHasExistingNodeShouldReturnAddErrorExistingEntry() { controller.addNodes(Arrays.asList(enode1)); @@ -256,6 +270,105 @@ public void reloadNodeWhitelistWithErrorReadingConfigFileShouldKeepOldWhitelist( assertThat(controller.getNodesWhitelist()).containsExactly(expectedEnodeURI); } + @Test + @SuppressWarnings("unchecked") + public void whenAddingNodeShouldNotifyWhitelistModifiedSubscribers() { + final Consumer consumer = mock(Consumer.class); + final NodeWhitelistUpdatedEvent expectedEvent = + new NodeWhitelistUpdatedEvent( + Lists.newArrayList(new EnodeURL(enode1)), Collections.emptyList()); + + controller.subscribeToListUpdatedEvent(consumer); + controller.addNodes(Lists.newArrayList(enode1)); + + verify(consumer).accept(eq(expectedEvent)); + } + + @Test + @SuppressWarnings("unchecked") + public void whenAddingNodeDoesNotAddShouldNotNotifyWhitelistModifiedSubscribers() { + // adding node before subscribing to whitelist modified events + controller.addNodes(Lists.newArrayList(enode1)); + final Consumer consumer = mock(Consumer.class); + + controller.subscribeToListUpdatedEvent(consumer); + // won't add duplicate node + controller.addNodes(Lists.newArrayList(enode1)); + + verifyZeroInteractions(consumer); + } + + @Test + @SuppressWarnings("unchecked") + public void whenRemovingNodeShouldNotifyWhitelistModifiedSubscribers() { + // adding node before subscribing to whitelist modified events + controller.addNodes(Lists.newArrayList(enode1)); + + final Consumer consumer = mock(Consumer.class); + final NodeWhitelistUpdatedEvent expectedEvent = + new NodeWhitelistUpdatedEvent( + Collections.emptyList(), Lists.newArrayList(new EnodeURL(enode1))); + + controller.subscribeToListUpdatedEvent(consumer); + controller.removeNodes(Lists.newArrayList(enode1)); + + verify(consumer).accept(eq(expectedEvent)); + } + + @Test + @SuppressWarnings("unchecked") + public void whenRemovingNodeDoesNotRemoveShouldNotifyWhitelistModifiedSubscribers() { + final Consumer consumer = mock(Consumer.class); + + controller.subscribeToListUpdatedEvent(consumer); + // won't remove absent node + controller.removeNodes(Lists.newArrayList(enode1)); + + verifyZeroInteractions(consumer); + } + + @Test + @SuppressWarnings("unchecked") + public void whenReloadingWhitelistShouldNotifyWhitelistModifiedSubscribers() throws Exception { + final Path permissionsFile = createPermissionsFileWithNode(enode2); + final PermissioningConfiguration permissioningConfig = mock(PermissioningConfiguration.class); + final Consumer consumer = mock(Consumer.class); + final NodeWhitelistUpdatedEvent expectedEvent = + new NodeWhitelistUpdatedEvent( + Lists.newArrayList(new EnodeURL(enode2)), Lists.newArrayList(new EnodeURL(enode1))); + + when(permissioningConfig.getConfigurationFilePath()) + .thenReturn(permissionsFile.toAbsolutePath().toString()); + when(permissioningConfig.isNodeWhitelistEnabled()).thenReturn(true); + when(permissioningConfig.getNodeWhitelist()).thenReturn(Arrays.asList(URI.create(enode1))); + controller = new NodeWhitelistController(permissioningConfig); + controller.subscribeToListUpdatedEvent(consumer); + + controller.reload(); + + verify(consumer).accept(eq(expectedEvent)); + } + + @Test + @SuppressWarnings("unchecked") + public void whenReloadingWhitelistAndNothingChangesShouldNotNotifyWhitelistModifiedSubscribers() + throws Exception { + final Path permissionsFile = createPermissionsFileWithNode(enode1); + final PermissioningConfiguration permissioningConfig = mock(PermissioningConfiguration.class); + final Consumer consumer = mock(Consumer.class); + + when(permissioningConfig.getConfigurationFilePath()) + .thenReturn(permissionsFile.toAbsolutePath().toString()); + when(permissioningConfig.isNodeWhitelistEnabled()).thenReturn(true); + when(permissioningConfig.getNodeWhitelist()).thenReturn(Arrays.asList(URI.create(enode1))); + controller = new NodeWhitelistController(permissioningConfig); + controller.subscribeToListUpdatedEvent(consumer); + + controller.reload(); + + verifyZeroInteractions(consumer); + } + private Path createPermissionsFileWithNode(final String node) throws IOException { final String nodePermissionsFileContent = "nodes-whitelist=[\"" + node + "\"]"; final Path permissionsFile = Files.createTempFile("node_permissions", "");