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

Maintain a staticnodes.json #1106

Merged
merged 14 commits into from
Mar 20, 2019
Merged

Maintain a staticnodes.json #1106

merged 14 commits into from
Mar 20, 2019

Conversation

rain-on
Copy link
Contributor

@rain-on rain-on commented Mar 15, 2019

The staticnodes.json file resides in the pantheon data directory,
is parsed at startup to determine which peers can be connected
to (aside from bootnodes), and is updated whenever addPeer/removePeer
RPC is invoked.

This file is not updated when finding neighbour peers (ONLY on RPC
call).

The staticnodes.json file resides in the pantheon data directory,
is parsed at startup to determine which peers can be connected
to (aside from bootnodes), and is updated whenever addPeer/removePeer
RPC is invoked.

This file is not updated when finding neighbour peers (ONLY on RPC
call).
public class PersistentJsonPeerCache extends PersistentPeerCache {

private static final Logger LOG = LogManager.getLogger();
private static final Charset UTF_8 = Charset.forName("UTF-8");
Copy link
Contributor

Choose a reason for hiding this comment

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

JDK constant of StandardCharsets.UTF_8 would be a nicer fit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

final Set<EnodeURL> enodes = StaticNodesParser.fromPath(path);

assertThat(enodes.size()).isEqualTo(4);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

What about asserting the contents of the endoes list? ...as not just any old four enode IDs will do

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

import org.junit.Test;
import org.junit.rules.TemporaryFolder;

public class StaticeNodesParserTest {
Copy link
Contributor

Choose a reason for hiding this comment

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

typo in class name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@@ -1161,4 +1164,11 @@ public MetricsSystem getMetricsSystem() {
public PantheonExceptionHandler exceptionHandler() {
return exceptionHandlerSupplier.get();
}

private Set<EnodeURL> loadStaticNodes() throws IOException {
final String STATIC_NODES_FILENAME = "static-nodes.json";
Copy link
Contributor

Choose a reason for hiding this comment

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

can this be made a constant

Copy link
Contributor Author

Choose a reason for hiding this comment

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

made is a local variable.


try {
final JsonArray enodeJsonArray = new JsonArray(new String(staticNodesContent, UTF_8));
for (Object jsonObj : enodeJsonArray.getList()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

be nicer to use the jsonArray.stream instead of populating a set

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

}

try {
final JsonArray enodeJsonArray = new JsonArray(new String(staticNodesContent, UTF_8));
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: thinking the code would be cleaner to do something similar to what IbftExtraDataCLIAdapter has done and just use objectMapper and do a objectMapper.readValue(path.toFile(), typedef..) to read it straight to Set and then transform to enodes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done - sort of.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@rain-on rain-on merged commit 6ca87e2 into PegaSysEng:master Mar 20, 2019
@rain-on rain-on deleted the static_nodes branch March 20, 2019 00:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants