Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

[PAN-2614] Add simple PeerPermissions interface #1446

Merged
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
d818464
Add LimitedSet utility
mbaxter May 2, 2019
885389d
Add a minimal PeerPermissions interface
mbaxter May 3, 2019
5467e30
Break up PeerBlacklist into smaller components using new interface
mbaxter May 13, 2019
69c244d
Dedupe identical functions
mbaxter May 13, 2019
7dc7111
Modify PeerPermissions callback to consume info on type of update
mbaxter May 13, 2019
c252f00
PeerPermissions update supports an optional list of affected peers
mbaxter May 13, 2019
3d8926d
Simplify permissions checks in discovery controller
mbaxter May 13, 2019
83c93e9
Remove peer dropped event handling (dead code)
mbaxter May 13, 2019
0b3b7f4
Add PeerDiscoveryController Builder
mbaxter May 13, 2019
b95a78e
Be more selective when dropping peers from table
mbaxter May 14, 2019
08ca7b3
Validate banned-node-ids as valid node ids
mbaxter May 14, 2019
6d63cd3
Connect to peers that have not been recently contacted
mbaxter May 14, 2019
650c05b
Fix PantheonCommandTest by using valid node id
mbaxter May 14, 2019
0d6d45e
Merge branch 'master' into PAN-2614/add-peer-permissions-interface
mbaxter May 14, 2019
ab94628
Check permissions at discovery layer considering directionality
mbaxter May 14, 2019
13033b2
Rename interface for clarity
mbaxter May 15, 2019
0bfb168
Validate peers before initiating connection
mbaxter May 15, 2019
e1b0045
Merge branch 'master' into PAN-2614/add-peer-permissions-interface
mbaxter May 15, 2019
8740f74
Clean up PeerPermissions utils
mbaxter May 15, 2019
2f5daf7
Merge branch 'master' into PAN-2614/add-peer-permissions-interface
mbaxter May 16, 2019
ff332f4
Merge branch 'master' into PAN-2614/add-peer-permissions-interface
mbaxter May 17, 2019
69f8d0a
Use ImmutableList
mbaxter May 17, 2019
e05836f
Move utility method to EnodeURL
mbaxter May 17, 2019
01a3a1c
Merge branch 'master' into PAN-2614/add-peer-permissions-interface
mbaxter May 17, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import java.io.IOException;
import java.nio.file.Path;
import java.time.Clock;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
Expand Down Expand Up @@ -108,7 +107,6 @@ public void startNode(final PantheonNode node) {
.jsonRpcConfiguration(node.jsonRpcConfiguration())
.webSocketConfiguration(node.webSocketConfiguration())
.dataDir(node.homeDirectory())
.bannedNodeIds(Collections.emptySet())
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We no longer need to configure an empty set of banned nodes

.metricsSystem(noOpMetricsSystem)
.metricsConfiguration(node.metricsConfiguration())
.p2pEnabled(node.isP2pEnabled())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
import tech.pegasys.pantheon.ethereum.p2p.network.DefaultP2PNetwork;
import tech.pegasys.pantheon.ethereum.p2p.peers.DefaultPeer;
import tech.pegasys.pantheon.ethereum.p2p.peers.Peer;
import tech.pegasys.pantheon.ethereum.p2p.peers.PeerBlacklist;
import tech.pegasys.pantheon.ethereum.p2p.wire.messages.DisconnectMessage.DisconnectReason;
import tech.pegasys.pantheon.ethereum.worldstate.WorldStateArchive;
import tech.pegasys.pantheon.metrics.MetricsSystem;
Expand Down Expand Up @@ -126,7 +125,6 @@ public TestNode(
.vertx(vertx)
.keyPair(this.kp)
.config(networkingConfiguration)
.peerBlacklist(new PeerBlacklist())
.metricsSystem(new NoOpMetricsSystem())
.supportedCapabilities(capabilities)
.build())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import tech.pegasys.pantheon.ethereum.p2p.config.NetworkingConfiguration;
import tech.pegasys.pantheon.ethereum.p2p.config.RlpxConfiguration;
import tech.pegasys.pantheon.ethereum.p2p.network.DefaultP2PNetwork;
import tech.pegasys.pantheon.ethereum.p2p.peers.PeerBlacklist;
import tech.pegasys.pantheon.ethereum.p2p.wire.Capability;
import tech.pegasys.pantheon.ethereum.permissioning.AccountLocalConfigPermissioningController;
import tech.pegasys.pantheon.ethereum.permissioning.NodeLocalConfigPermissioningController;
Expand Down Expand Up @@ -243,7 +242,6 @@ private P2PNetwork createP2pNetwork() {
.keyPair(SECP256K1.KeyPair.generate())
.vertx(vertx)
.config(config)
.peerBlacklist(new PeerBlacklist())
.metricsSystem(new NoOpMetricsSystem())
.build();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ public class DiscoveryPeer extends DefaultPeer {
private long firstDiscovered = 0;
private long lastContacted = 0;
private long lastSeen = 0;
private long lastAttemptedConnection = 0;

private DiscoveryPeer(final EnodeURL enode, final Endpoint endpoint) {
super(enode);
Expand Down Expand Up @@ -88,6 +89,14 @@ public void setLastContacted(final long lastContacted) {
this.lastContacted = lastContacted;
}

public long getLastAttemptedConnection() {
return lastAttemptedConnection;
}

public void setLastAttemptedConnection(final long lastAttemptedConnection) {
this.lastAttemptedConnection = lastAttemptedConnection;
}

public long getLastSeen() {
return lastSeen;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,25 +14,19 @@

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static tech.pegasys.pantheon.util.bytes.BytesValue.wrapBuffer;

import tech.pegasys.pantheon.crypto.SECP256K1;
import tech.pegasys.pantheon.ethereum.p2p.api.DisconnectCallback;
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.PeerDiscoveryController.AsyncExecutor;
import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerRequirement;
import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerTable;
import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PingPacketData;
import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.TimerUtil;
import tech.pegasys.pantheon.ethereum.p2p.peers.DefaultPeerId;
import tech.pegasys.pantheon.ethereum.p2p.peers.PeerBlacklist;
import tech.pegasys.pantheon.ethereum.p2p.wire.messages.DisconnectMessage;
import tech.pegasys.pantheon.ethereum.p2p.peers.PeerId;
import tech.pegasys.pantheon.ethereum.p2p.permissions.PeerPermissions;
import tech.pegasys.pantheon.ethereum.permissioning.node.NodePermissioningController;
import tech.pegasys.pantheon.metrics.MetricsSystem;
import tech.pegasys.pantheon.util.NetworkUtility;
Expand All @@ -47,7 +41,6 @@
import java.util.OptionalInt;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.TimeUnit;
import java.util.function.Consumer;
import java.util.stream.Collectors;
import java.util.stream.Stream;
Expand All @@ -61,17 +54,16 @@
* The peer discovery agent is the network component that sends and receives peer discovery messages
* via UDP.
*/
public abstract class PeerDiscoveryAgent implements DisconnectCallback {
public abstract class PeerDiscoveryAgent {
protected static final Logger LOG = LogManager.getLogger();

// The devp2p specification says only accept packets up to 1280, but some
// clients ignore that, so we add in a little extra padding.
private static final int MAX_PACKET_SIZE_BYTES = 1600;
private static final long PEER_REFRESH_INTERVAL_MS = MILLISECONDS.convert(30, TimeUnit.MINUTES);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved this constant into PeerDiscoveryController where it is used


protected final List<DiscoveryPeer> bootstrapPeers;
private final List<PeerRequirement> peerRequirements = new CopyOnWriteArrayList<>();
private final PeerBlacklist peerBlacklist;
private final PeerPermissions peerPermissions;
private final Optional<NodePermissioningController> nodePermissioningController;
private final MetricsSystem metricsSystem;
/* The peer controller, which takes care of the state machine of peers. */
Expand All @@ -80,7 +72,6 @@ public abstract class PeerDiscoveryAgent implements DisconnectCallback {
/* The keypair used to sign messages. */
protected final SECP256K1.KeyPair keyPair;
private final BytesValue id;
private final PeerTable peerTable;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PeerTable isn't used in PeerDiscoveryAgent - moved it into the PeerDiscoveryController where it is used.

protected final DiscoveryConfiguration config;

/* This is the {@link tech.pegasys.pantheon.ethereum.p2p.Peer} object holding who we are. */
Expand All @@ -89,12 +80,11 @@ public abstract class PeerDiscoveryAgent implements DisconnectCallback {
/* Is discovery enabled? */
private boolean isActive = false;
private final Subscribers<Consumer<PeerBondedEvent>> peerBondedObservers = new Subscribers<>();
private final Subscribers<Consumer<PeerDroppedEvent>> peerDroppedObservers = new Subscribers<>();

public PeerDiscoveryAgent(
final SECP256K1.KeyPair keyPair,
final DiscoveryConfiguration config,
final PeerBlacklist peerBlacklist,
final PeerPermissions peerPermissions,
final Optional<NodePermissioningController> nodePermissioningController,
final MetricsSystem metricsSystem) {
this.metricsSystem = metricsSystem;
Expand All @@ -103,14 +93,13 @@ public PeerDiscoveryAgent(

validateConfiguration(config);

this.peerBlacklist = peerBlacklist;
this.peerPermissions = peerPermissions;
this.nodePermissioningController = nodePermissioningController;
this.bootstrapPeers =
config.getBootnodes().stream().map(DiscoveryPeer::fromEnode).collect(Collectors.toList());

this.config = config;
this.keyPair = keyPair;
this.peerTable = new PeerTable(keyPair.getPublicKey().getEncodedBytes(), 16);

id = keyPair.getPublicKey().getEncodedBytes();
}
Expand Down Expand Up @@ -164,21 +153,19 @@ private void startController() {
}

private PeerDiscoveryController createController() {
return new PeerDiscoveryController(
keyPair,
advertisedPeer,
peerTable,
bootstrapPeers,
this::handleOutgoingPacket,
createTimer(),
createWorkerExecutor(),
PEER_REFRESH_INTERVAL_MS,
PeerRequirement.combine(peerRequirements),
peerBlacklist,
nodePermissioningController,
peerBondedObservers,
peerDroppedObservers,
metricsSystem);
return PeerDiscoveryController.builder()
.keypair(keyPair)
.localPeer(advertisedPeer)
.bootstrapNodes(bootstrapPeers)
.outboundMessageHandler(this::handleOutgoingPacket)
.timerUtil(createTimer())
.workerExecutor(createWorkerExecutor())
.peerRequirement(PeerRequirement.combine(peerRequirements))
.peerPermissions(peerPermissions)
.nodePermissioningController(nodePermissioningController)
.peerBondedObservers(peerBondedObservers)
.metricsSystem(metricsSystem)
.build();
}

protected boolean validatePacketSize(final int packetSize) {
Expand Down Expand Up @@ -240,6 +227,10 @@ public Stream<DiscoveryPeer> streamDiscoveredPeers() {
return controller.map(PeerDiscoveryController::streamDiscoveredPeers).orElse(Stream.empty());
}

public void dropPeer(final PeerId peer) {
controller.ifPresent(c -> c.dropPeer(peer));
}

public Optional<DiscoveryPeer> getAdvertisedPeer() {
return Optional.ofNullable(advertisedPeer);
}
Expand Down Expand Up @@ -272,29 +263,6 @@ 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.
*
* <p><i>No guarantees are made about the order in which observers are invoked.</i>
*
* @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<PeerDroppedEvent> observer) {
checkNotNull(observer);
return peerDroppedObservers.subscribe(observer);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was dead code - listeners were never actually notified of dropped peers

}

/**
* 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.
*
Expand All @@ -320,15 +288,6 @@ private static void validateConfiguration(final DiscoveryConfiguration config) {
checkArgument(config.getBucketSize() > 0, "bucket size cannot be negative nor zero");
}

@Override
public void onDisconnect(
final PeerConnection connection,
final DisconnectMessage.DisconnectReason reason,
final boolean initiatedByPeer) {
final BytesValue nodeId = connection.getPeerInfo().getNodeId();
peerTable.tryEvict(new DefaultPeerId(nodeId));
}

/**
* Returns the current state of the PeerDiscoveryAgent.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerDiscoveryController.AsyncExecutor;
import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.TimerUtil;
import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.VertxTimerUtil;
import tech.pegasys.pantheon.ethereum.p2p.peers.PeerBlacklist;
import tech.pegasys.pantheon.ethereum.p2p.permissions.PeerPermissions;
import tech.pegasys.pantheon.ethereum.permissioning.node.NodePermissioningController;
import tech.pegasys.pantheon.metrics.MetricCategory;
import tech.pegasys.pantheon.metrics.MetricsSystem;
Expand Down Expand Up @@ -58,10 +58,10 @@ public VertxPeerDiscoveryAgent(
final Vertx vertx,
final KeyPair keyPair,
final DiscoveryConfiguration config,
final PeerBlacklist peerBlacklist,
final PeerPermissions peerPermissions,
final Optional<NodePermissioningController> nodePermissioningController,
final MetricsSystem metricsSystem) {
super(keyPair, config, peerBlacklist, nodePermissioningController, metricsSystem);
super(keyPair, config, peerPermissions, nodePermissioningController, metricsSystem);
checkArgument(vertx != null, "vertx instance cannot be null");
this.vertx = vertx;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

@FunctionalInterface
public interface OutboundMessageHandler {
public static OutboundMessageHandler NOOP = (peer, packet) -> {};
OutboundMessageHandler NOOP = (peer, packet) -> {};

void send(final DiscoveryPeer toPeer, final Packet packet);
}
Loading