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

Commit

Permalink
Normalize EnodeURLs (#1264)
Browse files Browse the repository at this point in the history
Only specify discovery port explicitly when the discovery port differs
from the listening port.
  • Loading branch information
mbaxter authored Apr 12, 2019
1 parent 381829d commit a6b9b5f
Show file tree
Hide file tree
Showing 21 changed files with 128 additions and 96 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ private String signedTransactionData() {
final String enodeURL = ((RunnableNode) node).enodeUrl().toASCIIString();
final BytesValue payload =
SmartContractPermissioningController.createPayload(
ADD_ENODE_SIGNATURE, new EnodeURL(enodeURL));
ADD_ENODE_SIGNATURE, EnodeURL.fromString(enodeURL));

RawTransaction transaction =
RawTransaction.createTransaction(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ private org.web3j.protocol.core.methods.request.Transaction payload() {
final BytesValue payload =
SmartContractPermissioningController.createPayload(
IS_CONNECTION_ALLOWED_SIGNATURE,
new EnodeURL(sourceEnodeURL),
new EnodeURL(targetEnodeURL));
EnodeURL.fromString(sourceEnodeURL),
EnodeURL.fromString(targetEnodeURL));

return org.web3j.protocol.core.methods.request.Transaction.createFunctionCallTransaction(
null, null, null, null, contractAddress.toString(), payload.toString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ private String signedTransactionData() {
final String enodeURL = ((RunnableNode) node).enodeUrl().toASCIIString();
final BytesValue payload =
SmartContractPermissioningController.createPayload(
REMOVE_ENODE_SIGNATURE, new EnodeURL(enodeURL));
REMOVE_ENODE_SIGNATURE, EnodeURL.fromString(enodeURL));

RawTransaction transaction =
RawTransaction.createTransaction(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ private org.web3j.protocol.core.methods.request.Transaction payload() {
final String sourceEnodeURL = ((RunnableNode) node).enodeUrl().toASCIIString();
final BytesValue payload =
SmartContractPermissioningController.createPayload(
IS_NODE_ALLOWED_SIGNATURE, new EnodeURL(sourceEnodeURL));
IS_NODE_ALLOWED_SIGNATURE, EnodeURL.fromString(sourceEnodeURL));

return org.web3j.protocol.core.methods.request.Transaction.createFunctionCallTransaction(
null, null, null, null, contractAddress.toString(), payload.toString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public String getName() {
protected JsonRpcResponse performOperation(final Object id, final String enode) {
try {
LOG.debug("Adding ({}) to peers", enode);
final EnodeURL enodeURL = new EnodeURL(enode);
final EnodeURL enodeURL = EnodeURL.fromString(enode);
final Peer peer = DefaultPeer.fromEnodeURL(enodeURL);
boolean addedToNetwork = peerNetwork.addMaintainConnectionPeer(peer);
return new JsonRpcSuccessResponse(id, addedToNetwork);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public String getName() {
@Override
protected JsonRpcResponse performOperation(final Object id, final String enode) {
LOG.debug("Remove ({}) to peer cache", enode);
final EnodeURL enodeURL = new EnodeURL(enode);
final EnodeURL enodeURL = EnodeURL.fromString(enode);
final boolean result =
peerNetwork.removeMaintainedConnectionPeer(DefaultPeer.fromEnodeURL(enodeURL));
return new JsonRpcSuccessResponse(id, result);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ private static Set<EnodeURL> readEnodesFromPath(final Path path) throws IOExcept

private static EnodeURL decodeString(final String input) {
try {
return new EnodeURL(input);
return EnodeURL.fromString(input);
} catch (IllegalArgumentException ex) {
LOG.info("Illegally constructed enode supplied ({})", input);
throw ex;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,19 +41,19 @@
public class InsufficientPeersPermissioningProviderTest {
@Mock private P2PNetwork p2pNetwork;
private final EnodeURL SELF_ENODE =
new EnodeURL(
EnodeURL.fromString(
"enode://00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001@192.168.0.1:30303");
private final EnodeURL ENODE_2 =
new EnodeURL(
EnodeURL.fromString(
"enode://00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000002@192.168.0.2:30303");
private final EnodeURL ENODE_3 =
new EnodeURL(
EnodeURL.fromString(
"enode://00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000003@192.168.0.3:30303");
private final EnodeURL ENODE_4 =
new EnodeURL(
EnodeURL.fromString(
"enode://00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004@192.168.0.4:30303");
private final EnodeURL ENODE_5 =
new EnodeURL(
EnodeURL.fromString(
"enode://00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000005@192.168.0.5:30303");

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public class PeerDiscoveryControllerTest {
private final PeerDiscoveryTestHelper helper = new PeerDiscoveryTestHelper();
private final String selfEnodeString =
"enode://5f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.10:1111";
private final EnodeURL selfEnode = new EnodeURL(selfEnodeString);
private final EnodeURL selfEnode = EnodeURL.fromString(selfEnodeString);

@Before
public void initializeMocks() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public final class NettyP2PNetworkTest {

private final String selfEnodeString =
"enode://5f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.10:1111";
private final EnodeURL selfEnode = new EnodeURL(selfEnodeString);
private final EnodeURL selfEnode = EnodeURL.fromString(selfEnodeString);

@Before
public void before() {
Expand Down Expand Up @@ -723,8 +723,8 @@ public void onBlockAddedAndPeerNotPermittedShouldDisconnect() {
final PeerConnection notPermittedPeerConnection =
mockPeerConnection(localPeer, notPermittedPeer);

final EnodeURL permittedEnodeURL = new EnodeURL(permittedPeer.getEnodeURLString());
final EnodeURL notPermittedEnodeURL = new EnodeURL(notPermittedPeer.getEnodeURLString());
final EnodeURL permittedEnodeURL = EnodeURL.fromString(permittedPeer.getEnodeURLString());
final EnodeURL notPermittedEnodeURL = EnodeURL.fromString(notPermittedPeer.getEnodeURLString());

nettyP2PNetwork.start();
nettyP2PNetwork.connect(permittedPeer).complete(permittedPeerConnection);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public NodeLocalConfigPermissioningController(
private void readNodesFromConfig(final LocalPermissioningConfiguration configuration) {
if (configuration.isNodeWhitelistEnabled() && configuration.getNodeWhitelist() != null) {
for (URI uri : configuration.getNodeWhitelist()) {
addNode(new EnodeURL(uri.toString()));
addNode(EnodeURL.fromString(uri.toString()));
}
}
}
Expand All @@ -80,7 +80,8 @@ public NodesWhitelistResult addNodes(final List<String> enodeURLs) {
if (inputValidationResult.result() != WhitelistOperationResult.SUCCESS) {
return inputValidationResult;
}
final List<EnodeURL> peers = enodeURLs.stream().map(EnodeURL::new).collect(Collectors.toList());
final List<EnodeURL> peers =
enodeURLs.stream().map(EnodeURL::fromString).collect(Collectors.toList());

for (EnodeURL peer : peers) {
if (nodesWhitelist.contains(peer)) {
Expand Down Expand Up @@ -111,7 +112,8 @@ public NodesWhitelistResult removeNodes(final List<String> enodeURLs) {
if (inputValidationResult.result() != WhitelistOperationResult.SUCCESS) {
return inputValidationResult;
}
final List<EnodeURL> peers = enodeURLs.stream().map(EnodeURL::new).collect(Collectors.toList());
final List<EnodeURL> peers =
enodeURLs.stream().map(EnodeURL::fromString).collect(Collectors.toList());

boolean anyBootnode = peers.stream().anyMatch(bootnodes::contains);
if (anyBootnode) {
Expand Down Expand Up @@ -213,7 +215,7 @@ private boolean compareEnodes(final EnodeURL nodeA, final EnodeURL nodeB) {
}

public boolean isPermitted(final String enodeURL) {
return isPermitted(new EnodeURL(enodeURL));
return isPermitted(EnodeURL.fromString(enodeURL));
}

public boolean isPermitted(final EnodeURL node) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public void setUp() {
new NodeLocalConfigPermissioningController(
LocalPermissioningConfiguration.createDefault(),
bootnodesList,
new EnodeURL(selfEnode),
EnodeURL.fromString(selfEnode),
whitelistPersistor);
}

Expand Down Expand Up @@ -219,15 +219,17 @@ public void whenCheckingIfNodeIsPermittedDiscoveryPortShouldBeConsideredIfPresen
@Test
public void whenCheckingIfNodeIsPermittedOrderDoesNotMatter() {
controller.addNodes(Arrays.asList(enode1));
assertThat(controller.isPermitted(new EnodeURL(enode1), new EnodeURL(selfEnode))).isTrue();
assertThat(controller.isPermitted(new EnodeURL(selfEnode), new EnodeURL(enode1))).isTrue();
assertThat(controller.isPermitted(EnodeURL.fromString(enode1), EnodeURL.fromString(selfEnode)))
.isTrue();
assertThat(controller.isPermitted(EnodeURL.fromString(selfEnode), EnodeURL.fromString(enode1)))
.isTrue();
}

@Test
public void stateShouldRevertIfWhitelistPersistFails()
throws IOException, WhitelistFileSyncException {
List<String> newNode1 = singletonList(new EnodeURL(enode1).toString());
List<String> newNode2 = singletonList(new EnodeURL(enode2).toString());
List<String> newNode1 = singletonList(EnodeURL.fromString(enode1).toString());
List<String> newNode2 = singletonList(EnodeURL.fromString(enode2).toString());

assertThat(controller.getNodesWhitelist().size()).isEqualTo(0);

Expand Down Expand Up @@ -260,7 +262,7 @@ public void reloadNodeWhitelistWithValidConfigFileShouldUpdateWhitelist() throws
.thenReturn(Arrays.asList(URI.create(expectedEnodeURL)));
controller =
new NodeLocalConfigPermissioningController(
permissioningConfig, bootnodesList, new EnodeURL(selfEnode));
permissioningConfig, bootnodesList, EnodeURL.fromString(selfEnode));

controller.reload();

Expand All @@ -280,7 +282,7 @@ public void reloadNodeWhitelistWithErrorReadingConfigFileShouldKeepOldWhitelist(
.thenReturn(Arrays.asList(URI.create(expectedEnodeURI)));
controller =
new NodeLocalConfigPermissioningController(
permissioningConfig, bootnodesList, new EnodeURL(selfEnode));
permissioningConfig, bootnodesList, EnodeURL.fromString(selfEnode));

final Throwable thrown = catchThrowable(() -> controller.reload());

Expand All @@ -297,7 +299,7 @@ public void whenAddingNodeShouldNotifyWhitelistModifiedSubscribers() {
final Consumer<NodeWhitelistUpdatedEvent> consumer = mock(Consumer.class);
final NodeWhitelistUpdatedEvent expectedEvent =
new NodeWhitelistUpdatedEvent(
Lists.newArrayList(new EnodeURL(enode1)), Collections.emptyList());
Lists.newArrayList(EnodeURL.fromString(enode1)), Collections.emptyList());

controller.subscribeToListUpdatedEvent(consumer);
controller.addNodes(Lists.newArrayList(enode1));
Expand Down Expand Up @@ -328,7 +330,7 @@ public void whenRemovingNodeShouldNotifyWhitelistModifiedSubscribers() {
final Consumer<NodeWhitelistUpdatedEvent> consumer = mock(Consumer.class);
final NodeWhitelistUpdatedEvent expectedEvent =
new NodeWhitelistUpdatedEvent(
Collections.emptyList(), Lists.newArrayList(new EnodeURL(enode1)));
Collections.emptyList(), Lists.newArrayList(EnodeURL.fromString(enode1)));

controller.subscribeToListUpdatedEvent(consumer);
controller.removeNodes(Lists.newArrayList(enode1));
Expand All @@ -352,7 +354,7 @@ public void whenRemovingNodeDoesNotRemoveShouldNotifyWhitelistModifiedSubscriber
public void whenRemovingBootnodeShouldReturnRemoveBootnodeError() {
NodesWhitelistResult expected =
new NodesWhitelistResult(WhitelistOperationResult.ERROR_BOOTNODE_CANNOT_BE_REMOVED);
bootnodesList.add(new EnodeURL(enode1));
bootnodesList.add(EnodeURL.fromString(enode1));
controller.addNodes(Lists.newArrayList(enode1, enode2));

NodesWhitelistResult actualResult = controller.removeNodes(Lists.newArrayList(enode1));
Expand All @@ -370,15 +372,16 @@ public void whenReloadingWhitelistShouldNotifyWhitelistModifiedSubscribers() thr
final Consumer<NodeWhitelistUpdatedEvent> consumer = mock(Consumer.class);
final NodeWhitelistUpdatedEvent expectedEvent =
new NodeWhitelistUpdatedEvent(
Lists.newArrayList(new EnodeURL(enode2)), Lists.newArrayList(new EnodeURL(enode1)));
Lists.newArrayList(EnodeURL.fromString(enode2)),
Lists.newArrayList(EnodeURL.fromString(enode1)));

when(permissioningConfig.getNodePermissioningConfigFilePath())
.thenReturn(permissionsFile.toAbsolutePath().toString());
when(permissioningConfig.isNodeWhitelistEnabled()).thenReturn(true);
when(permissioningConfig.getNodeWhitelist()).thenReturn(Arrays.asList(URI.create(enode1)));
controller =
new NodeLocalConfigPermissioningController(
permissioningConfig, bootnodesList, new EnodeURL(selfEnode));
permissioningConfig, bootnodesList, EnodeURL.fromString(selfEnode));
controller.subscribeToListUpdatedEvent(consumer);

controller.reload();
Expand All @@ -401,7 +404,7 @@ public void whenReloadingWhitelistAndNothingChangesShouldNotNotifyWhitelistModif
when(permissioningConfig.getNodeWhitelist()).thenReturn(Arrays.asList(URI.create(enode1)));
controller =
new NodeLocalConfigPermissioningController(
permissioningConfig, bootnodesList, new EnodeURL(selfEnode));
permissioningConfig, bootnodesList, EnodeURL.fromString(selfEnode));
controller.subscribeToListUpdatedEvent(consumer);

controller.reload();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,9 @@ public void testIpv4Included() throws IOException {

assertThat(
controller.isPermitted(
new EnodeURL(
EnodeURL.fromString(
"enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.1:30303"),
new EnodeURL(
EnodeURL.fromString(
"enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.1:30304")))
.isTrue();
}
Expand All @@ -79,9 +79,9 @@ public void testIpv4DestinationMissing() throws IOException {

assertThat(
controller.isPermitted(
new EnodeURL(
EnodeURL.fromString(
"enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.1:30303"),
new EnodeURL(
EnodeURL.fromString(
"enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.1:30305")))
.isFalse();
}
Expand All @@ -95,9 +95,9 @@ public void testIpv4SourceMissing() throws IOException {

assertThat(
controller.isPermitted(
new EnodeURL(
EnodeURL.fromString(
"enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.1:30302"),
new EnodeURL(
EnodeURL.fromString(
"enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.1:30304")))
.isFalse();
}
Expand All @@ -111,9 +111,9 @@ public void testIpv6Included() throws IOException {

assertThat(
controller.isPermitted(
new EnodeURL(
EnodeURL.fromString(
"enode://1234000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000ab61@[1:2:3:4:5:6:7:8]:30303"),
new EnodeURL(
EnodeURL.fromString(
"enode://1234000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000ab62@[1:2:3:4:5:6:7:8]:30304")))
.isTrue();
}
Expand All @@ -127,9 +127,9 @@ public void testIpv6SourceMissing() throws IOException {

assertThat(
controller.isPermitted(
new EnodeURL(
EnodeURL.fromString(
"enode://1234000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000ab63@[1:2:3:4:5:6:7:8]:30303"),
new EnodeURL(
EnodeURL.fromString(
"enode://1234000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000ab62@[1:2:3:4:5:6:7:8]:30304")))
.isFalse();
}
Expand All @@ -143,9 +143,9 @@ public void testIpv6DestinationMissing() throws IOException {

assertThat(
controller.isPermitted(
new EnodeURL(
EnodeURL.fromString(
"enode://1234000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000ab61@[1:2:3:4:5:6:7:8]:30303"),
new EnodeURL(
EnodeURL.fromString(
"enode://1234000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000ab63@[1:2:3:4:5:6:7:8]:30304")))
.isFalse();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public class NodePermissioningControllerFactoryTest {
private final String enode =
"enode://5f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.10:1111";
Collection<EnodeURL> bootnodes = Collections.emptyList();
EnodeURL selfEnode = new EnodeURL(enode);
EnodeURL selfEnode = EnodeURL.fromString(enode);
LocalPermissioningConfiguration localPermissioningConfig;
SmartContractPermissioningConfiguration smartContractPermissioningConfiguration;
PermissioningConfiguration config;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@
public class NodePermissioningControllerTest {

private static final EnodeURL enode1 =
new EnodeURL(
EnodeURL.fromString(
"enode://94c15d1b9e2fe7ce56e458b9a3b672ef11894ddedd0c6f247e0f1d3487f52b66208fb4aeb8179fce6e3a749ea93ed147c37976d67af557508d199d9594c35f09@192.168.0.2:1234");
private static final EnodeURL enode2 =
new EnodeURL(
EnodeURL.fromString(
"enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.3:5678");

@Mock private SyncStatusNodePermissioningProvider syncStatusNodePermissioningProvider;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,13 @@
public class SyncStatusNodePermissioningProviderTest {

private static final EnodeURL bootnode =
new EnodeURL(
EnodeURL.fromString(
"enode://6332792c4a00e3e4ee0926ed89e0d27ef985424d97b6a45bf0f23e51f0dcb5e66b875777506458aea7af6f9e4ffb69f43f3778ee73c81ed9d34c51c4b16b0b0f@192.168.0.1:9999");
private static final EnodeURL enode1 =
new EnodeURL(
EnodeURL.fromString(
"enode://94c15d1b9e2fe7ce56e458b9a3b672ef11894ddedd0c6f247e0f1d3487f52b66208fb4aeb8179fce6e3a749ea93ed147c37976d67af557508d199d9594c35f09@192.168.0.2:1234");
private static final EnodeURL enode2 =
new EnodeURL(
EnodeURL.fromString(
"enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.3:5678");

@Mock private Synchronizer synchronizer;
Expand Down
Loading

0 comments on commit a6b9b5f

Please sign in to comment.