Minor refactoring

I think having the read method in the same place as the write method might help preventing some errors
There are some people who are strictly against using static methods. I suppose I'm not experienced enough yet.

demo/src/main/java/ch/dissem/bitmessage/demo/Main.java

@@ -27,6 +27,7 @@ import org.slf4j.LoggerFactory;
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStreamReader;
+import java.util.Scanner;
 
 /**
  * Created by chris on 06.04.15.
@@ -47,9 +48,9 @@ public class Main {
             }
         });
 
-        BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
         System.out.print("Press Enter to exit\n");
-        br.readLine();
+        Scanner scanner = new Scanner(System.in);
+        scanner.nextLine();
         LOG.info("Shutting down client");
         networkNode.stop();
     }

domain/src/main/java/ch/dissem/bitmessage/entity/Version.java

@@ -19,6 +19,7 @@ package ch.dissem.bitmessage.entity;
 import ch.dissem.bitmessage.Context;
 import ch.dissem.bitmessage.entity.valueobject.NetworkAddress;
 import ch.dissem.bitmessage.utils.Encode;
+import ch.dissem.bitmessage.utils.UnixTime;
 
 import java.io.IOException;
 import java.io.OutputStream;
@@ -148,7 +149,7 @@ public class Version implements MessagePayload {
         public Builder defaults() {
             version = Context.CURRENT_VERSION;
             services = 1;
-            timestamp = System.currentTimeMillis() / 1000;
+            timestamp = UnixTime.now();
             nonce = new Random().nextInt();
             userAgent = "/Jabit:0.0.1/";
             streamNumbers = new long[]{1};

domain/src/main/java/ch/dissem/bitmessage/entity/payload/GenericPayload.java

@@ -16,7 +16,10 @@
 
 package ch.dissem.bitmessage.entity.payload;
 
+import ch.dissem.bitmessage.utils.Decode;
+
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.OutputStream;
 
 /**
@@ -27,11 +30,15 @@ public class GenericPayload implements ObjectPayload {
     private long stream;
     private byte[] data;
 
-    public GenericPayload(long stream, byte[] data) {
+    private GenericPayload(long stream, byte[] data) {
-        this.stream=stream;
+        this.stream = stream;
         this.data = data;
     }
 
+    public static GenericPayload read(InputStream is, long stream, int length) throws IOException {
+        return new GenericPayload(stream, Decode.bytes(is, length));
+    }
+
     @Override
     public long getStream() {
         return stream;

domain/src/main/java/ch/dissem/bitmessage/entity/payload/GetPubkey.java

@@ -16,7 +16,10 @@
 
 package ch.dissem.bitmessage.entity.payload;
 
+import ch.dissem.bitmessage.utils.Decode;
+
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.OutputStream;
 
 /**
@@ -27,7 +30,7 @@ public class GetPubkey implements ObjectPayload {
     private byte[] ripe;
     private byte[] tag;
 
-    public GetPubkey(long stream, byte[] ripeOrTag) {
+    private GetPubkey(long stream, byte[] ripeOrTag) {
         this.stream = stream;
         switch (ripeOrTag.length) {
             case 20:
@@ -41,6 +44,10 @@ public class GetPubkey implements ObjectPayload {
         }
     }
 
+    public static GetPubkey read(InputStream is, long stream, int length) throws IOException {
+        return new GetPubkey(stream, Decode.bytes(is, length));
+    }
+
     @Override
     public long getStream() {
         return stream;

domain/src/main/java/ch/dissem/bitmessage/entity/payload/Msg.java

@@ -16,7 +16,10 @@
 
 package ch.dissem.bitmessage.entity.payload;
 
+import ch.dissem.bitmessage.utils.Decode;
+
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.OutputStream;
 
 /**
@@ -27,7 +30,7 @@ public class Msg implements ObjectPayload {
     private byte[] encrypted;
     private UnencryptedMessage unencrypted;
 
-    public Msg(long stream, byte[] encrypted) {
+    private Msg(long stream, byte[] encrypted) {
         this.stream = stream;
         this.encrypted = encrypted;
     }
@@ -37,6 +40,10 @@ public class Msg implements ObjectPayload {
         this.unencrypted = unencrypted;
     }
 
+    public static Msg read(InputStream is, long stream, int length) throws IOException {
+        return new Msg(stream, Decode.bytes(is, length));
+    }
+
     @Override
     public long getStream() {
         return stream;

domain/src/main/java/ch/dissem/bitmessage/entity/payload/V2Pubkey.java

@@ -16,9 +16,11 @@
 
 package ch.dissem.bitmessage.entity.payload;
 
+import ch.dissem.bitmessage.utils.Decode;
 import ch.dissem.bitmessage.utils.Encode;
 
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.OutputStream;
 
 /**
@@ -40,6 +42,15 @@ public class V2Pubkey implements Pubkey {
         publicEncryptionKey = builder.publicEncryptionKey;
     }
 
+    public static V2Pubkey read(InputStream is, long stream) throws IOException {
+        return new V2Pubkey.Builder()
+                .streamNumber(stream)
+                .behaviorBitfield((int) Decode.uint32(is))
+                .publicSigningKey(Decode.bytes(is, 64))
+                .publicEncryptionKey(Decode.bytes(is, 64))
+                .build();
+    }
+
     @Override
     public long getVersion() {
         return 2;

domain/src/main/java/ch/dissem/bitmessage/entity/payload/V3Pubkey.java

@@ -16,9 +16,11 @@
 
 package ch.dissem.bitmessage.entity.payload;
 
+import ch.dissem.bitmessage.utils.Decode;
 import ch.dissem.bitmessage.utils.Encode;
 
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.OutputStream;
 
 /**
@@ -40,6 +42,19 @@ public class V3Pubkey extends V2Pubkey {
         signature = builder.signature;
     }
 
+    public static V3Pubkey read(InputStream is, long stream) throws IOException {
+        V3Pubkey.Builder v3 = new V3Pubkey.Builder()
+                .streamNumber(stream)
+                .behaviorBitfield((int) Decode.uint32(is))
+                .publicSigningKey(Decode.bytes(is, 64))
+                .publicEncryptionKey(Decode.bytes(is, 64))
+                .nonceTrialsPerByte(Decode.varInt(is))
+                .extraBytes(Decode.varInt(is));
+        int sigLength = (int) Decode.varInt(is);
+        v3.signature(Decode.bytes(is, sigLength));
+        return v3.build();
+    }
+
     @Override
     public void write(OutputStream os) throws IOException {
         super.write(os);

domain/src/main/java/ch/dissem/bitmessage/entity/payload/V4Broadcast.java

@@ -16,7 +16,10 @@
 
 package ch.dissem.bitmessage.entity.payload;
 
+import ch.dissem.bitmessage.utils.Decode;
+
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.OutputStream;
 
 /**
@@ -28,11 +31,15 @@ public class V4Broadcast implements Broadcast {
     private byte[] encrypted;
     private UnencryptedMessage unencrypted;
 
-    public V4Broadcast(long stream, byte[] encrypted) {
+    protected V4Broadcast(long stream, byte[] encrypted) {
         this.stream = stream;
         this.encrypted = encrypted;
     }
 
+    public static V4Broadcast read(InputStream is, long stream, int length) throws IOException {
+        return new V4Broadcast(stream, Decode.bytes(is, length));
+    }
+
     @Override
     public long getStream() {
         return stream;

domain/src/main/java/ch/dissem/bitmessage/entity/payload/V4Pubkey.java

@@ -16,7 +16,10 @@
 
 package ch.dissem.bitmessage.entity.payload;
 
+import ch.dissem.bitmessage.utils.Decode;
+
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.OutputStream;
 
 /**
@@ -31,7 +34,7 @@ public class V4Pubkey implements Pubkey {
     private byte[] encrypted;
     private V3Pubkey decrypted;
 
-    public V4Pubkey(long stream, byte[] tag, byte[] encrypted) {
+    private V4Pubkey(long stream, byte[] tag, byte[] encrypted) {
         this.stream = stream;
         this.tag = tag;
         this.encrypted = encrypted;
@@ -44,6 +47,10 @@ public class V4Pubkey implements Pubkey {
         // TODO: this.encrypted
     }
 
+    public static ObjectPayload read(InputStream stream, long streamNumber, int length) throws IOException {
+        return new V4Pubkey(streamNumber, Decode.bytes(stream, 32), Decode.bytes(stream, length - 32));
+    }
+
     @Override
     public void write(OutputStream stream) throws IOException {
         stream.write(tag);

domain/src/main/java/ch/dissem/bitmessage/entity/payload/V5Broadcast.java

@@ -16,7 +16,10 @@
 
 package ch.dissem.bitmessage.entity.payload;
 
+import ch.dissem.bitmessage.utils.Decode;
+
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.OutputStream;
 
 /**
@@ -25,11 +28,15 @@ import java.io.OutputStream;
 public class V5Broadcast extends V4Broadcast {
     private byte[] tag;
 
-    public V5Broadcast(long stream, byte[] tag, byte[] encrypted) {
+    private V5Broadcast(long stream, byte[] tag, byte[] encrypted) {
         super(stream, encrypted);
         this.tag = tag;
     }
 
+    public static V5Broadcast read(InputStream is, long stream, int length) throws IOException {
+        return new V5Broadcast(stream, Decode.bytes(is, 32), Decode.bytes(is, length - 32));
+    }
+
     public byte[] getTag() {
         return tag;
     }

domain/src/main/java/ch/dissem/bitmessage/factory/Factory.java

@@ -35,7 +35,7 @@ public class Factory {
 
     public static NetworkMessage getNetworkMessage(int version, InputStream stream) throws SocketTimeoutException {
         try {
-            return new V3MessageFactory().read(stream);
+            return V3MessageFactory.read(stream);
         } catch (SocketTimeoutException e) {
             throw e;
         } catch (Exception e) {
@@ -46,7 +46,7 @@ public class Factory {
 
     public static ObjectMessage getObjectMessage(int version, InputStream stream, int length) {
         try {
-            return new V3MessageFactory().readObject(stream, length);
+            return V3MessageFactory.readObject(stream, length);
         } catch (IOException e) {
             LOG.error(e.getMessage(), e);
             return null;
@@ -56,13 +56,13 @@ public class Factory {
     static ObjectPayload getObjectPayload(long objectType, long version, long streamNumber, InputStream stream, int length) throws IOException {
         if (objectType < 4) {
             switch ((int) objectType) {
-                case 0: // getpubkey
+                case 0:
                     return parseGetPubkey((int) version, streamNumber, stream, length);
-                case 1: // pubkey
+                case 1:
                     return parsePubkey((int) version, streamNumber, stream, length);
-                case 2: // msg
+                case 2:
                     return parseMsg((int) version, streamNumber, stream, length);
-                case 3: // broadcast
+                case 3:
                     return parseBroadcast((int) version, streamNumber, stream, length);
                 default:
                     LOG.error("This should not happen, someone broke something in the code!");
@@ -70,53 +70,39 @@ public class Factory {
         }
         // fallback: just store the message - we don't really care what it is
         LOG.warn("Unexpected object type: " + objectType);
-        return new GenericPayload(streamNumber, Decode.bytes(stream, length));
+        return GenericPayload.read(stream, streamNumber, length);
     }
 
     private static ObjectPayload parseGetPubkey(int version, long streamNumber, InputStream stream, int length) throws IOException {
-        return new GetPubkey(streamNumber, Decode.bytes(stream, length));
+        return GetPubkey.read(stream, streamNumber, length);
     }
 
     private static ObjectPayload parsePubkey(int version, long streamNumber, InputStream stream, int length) throws IOException {
         switch (version) {
             case 2:
-                return new V2Pubkey.Builder()
+                return V2Pubkey.read(stream, streamNumber);
-                        .streamNumber(streamNumber)
-                        .behaviorBitfield((int) Decode.int64(stream))
-                        .publicSigningKey(Decode.bytes(stream, 64))
-                        .publicEncryptionKey(Decode.bytes(stream, 64))
-                        .build();
             case 3:
-                V3Pubkey.Builder v3 = new V3Pubkey.Builder()
+                return V3Pubkey.read(stream, streamNumber);
-                        .streamNumber(streamNumber)
-                        .behaviorBitfield((int) Decode.int64(stream))
-                        .publicSigningKey(Decode.bytes(stream, 64))
-                        .publicEncryptionKey(Decode.bytes(stream, 64))
-                        .nonceTrialsPerByte(Decode.varInt(stream))
-                        .extraBytes(Decode.varInt(stream));
-                int sigLength = (int) Decode.varInt(stream);
-                v3.signature(Decode.bytes(stream, sigLength));
-                return v3.build();
             case 4:
-                return new V4Pubkey(streamNumber, Decode.bytes(stream, 32), Decode.bytes(stream, length - 32));
+                return V4Pubkey.read(stream, streamNumber, length);
         }
         LOG.debug("Unexpected pubkey version " + version + ", handling as generic payload object");
-        return new GenericPayload(streamNumber, Decode.bytes(stream, length));
+        return GenericPayload.read(stream, streamNumber, length);
     }
 
     private static ObjectPayload parseMsg(int version, long streamNumber, InputStream stream, int length) throws IOException {
-        return new Msg(streamNumber, Decode.bytes(stream, length));
+        return Msg.read(stream, streamNumber, length);
     }
 
     private static ObjectPayload parseBroadcast(int version, long streamNumber, InputStream stream, int length) throws IOException {
         switch (version) {
             case 4:
-                return new V4Broadcast(streamNumber, Decode.bytes(stream, length));
+                return V4Broadcast.read(stream, streamNumber, length);
             case 5:
-                return new V5Broadcast(streamNumber, Decode.bytes(stream, 32), Decode.bytes(stream, length - 32));
+                return V5Broadcast.read(stream, streamNumber, length);
             default:
                 LOG.debug("Encountered unknown broadcast version " + version);
-                return new GenericPayload(streamNumber, Decode.bytes(stream, length));
+                return GenericPayload.read(stream, streamNumber, length);
         }
     }
 }

domain/src/main/java/ch/dissem/bitmessage/factory/V3MessageFactory.java

@@ -34,9 +34,9 @@ import java.io.InputStream;
  * Creates protocol v3 network messages from {@link InputStream InputStreams}
  */
 class V3MessageFactory {
-    private Logger LOG = LoggerFactory.getLogger(V3MessageFactory.class);
+    private static Logger LOG = LoggerFactory.getLogger(V3MessageFactory.class);
 
-    public NetworkMessage read(InputStream stream) throws IOException {
+    public static NetworkMessage read(InputStream stream) throws IOException {
         if (testMagic(stream)) {
             String command = getCommand(stream);
             int length = (int) Decode.uint32(stream);
@@ -56,7 +56,7 @@ class V3MessageFactory {
         }
     }
 
-    private MessagePayload getPayload(String command, InputStream stream, int length) throws IOException {
+    private static MessagePayload getPayload(String command, InputStream stream, int length) throws IOException {
         switch (command) {
             case "version":
                 return parseVersion(stream);
@@ -76,7 +76,7 @@ class V3MessageFactory {
         }
     }
 
-    public ObjectMessage readObject(InputStream stream, int length) throws IOException {
+    public static ObjectMessage readObject(InputStream stream, int length) throws IOException {
         AccessCounter counter = new AccessCounter();
         byte nonce[] = Decode.bytes(stream, 8, counter);
         long expiresTime = Decode.int64(stream, counter);
@@ -96,7 +96,7 @@ class V3MessageFactory {
                 .build();
     }
 
-    private GetData parseGetData(InputStream stream) throws IOException {
+    private static GetData parseGetData(InputStream stream) throws IOException {
         long count = Decode.varInt(stream);
         GetData.Builder builder = new GetData.Builder();
         for (int i = 0; i < count; i++) {
@@ -105,7 +105,7 @@ class V3MessageFactory {
         return builder.build();
     }
 
-    private Inv parseInv(InputStream stream) throws IOException {
+    private static Inv parseInv(InputStream stream) throws IOException {
         long count = Decode.varInt(stream);
         Inv.Builder builder = new Inv.Builder();
         for (int i = 0; i < count; i++) {
@@ -114,7 +114,7 @@ class V3MessageFactory {
         return builder.build();
     }
 
-    private Addr parseAddr(InputStream stream) throws IOException {
+    private static Addr parseAddr(InputStream stream) throws IOException {
         long count = Decode.varInt(stream);
         Addr.Builder builder = new Addr.Builder();
         for (int i = 0; i < count; i++) {
@@ -123,7 +123,7 @@ class V3MessageFactory {
         return builder.build();
     }
 
-    private Version parseVersion(InputStream stream) throws IOException {
+    private static Version parseVersion(InputStream stream) throws IOException {
         int version = Decode.int32(stream);
         long services = Decode.int64(stream);
         long timestamp = Decode.int64(stream);
@@ -143,11 +143,11 @@ class V3MessageFactory {
                 .streams(streamNumbers).build();
     }
 
-    private InventoryVector parseInventoryVector(InputStream stream) throws IOException {
+    private static InventoryVector parseInventoryVector(InputStream stream) throws IOException {
         return new InventoryVector(Decode.bytes(stream, 32));
     }
 
-    private NetworkAddress parseAddress(InputStream stream, boolean light) throws IOException {
+    private static NetworkAddress parseAddress(InputStream stream, boolean light) throws IOException {
         long time;
         long streamNumber;
         if (!light) {
@@ -163,7 +163,7 @@ class V3MessageFactory {
         return new NetworkAddress.Builder().time(time).stream(streamNumber).services(services).ipv6(ipv6).port(port).build();
     }
 
-    private boolean testChecksum(byte[] checksum, byte[] payload) {
+    private static boolean testChecksum(byte[] checksum, byte[] payload) {
         byte[] payloadChecksum = Security.sha512(payload);
         for (int i = 0; i < checksum.length; i++) {
             if (checksum[i] != payloadChecksum[i]) {
@@ -173,7 +173,7 @@ class V3MessageFactory {
         return true;
     }
 
-    private String getCommand(InputStream stream) throws IOException {
+    private static String getCommand(InputStream stream) throws IOException {
         byte[] bytes = new byte[12];
         int end = -1;
         for (int i = 0; i < bytes.length; i++) {
@@ -187,7 +187,7 @@ class V3MessageFactory {
         return new String(bytes, 0, end, "ASCII");
     }
 
-    private boolean testMagic(InputStream stream) throws IOException {
+    private static boolean testMagic(InputStream stream) throws IOException {
         for (byte b : NetworkMessage.MAGIC_BYTES) {
             if (b != (byte) stream.read()) return false;
         }

domain/src/main/java/ch/dissem/bitmessage/utils/Security.java

@@ -92,7 +92,7 @@ public class Security {
     }
 
     private static byte[] getProofOfWorkTarget(ObjectMessage object, long nonceTrialsPerByte, long extraBytes) throws IOException {
-        BigInteger TTL = BigInteger.valueOf(object.getExpiresTime() - (System.currentTimeMillis() / 1000));
+        BigInteger TTL = BigInteger.valueOf(object.getExpiresTime() - UnixTime.now());
         LOG.debug("TTL: " + TTL + "s");
         BigInteger numerator = TWO.pow(64);
         BigInteger powLength = BigInteger.valueOf(object.getPayloadBytesWithoutNonce().length + extraBytes);

domain/src/test/java/ch/dissem/bitmessage/utils/SecurityTest.java

@@ -22,6 +22,7 @@ import ch.dissem.bitmessage.ports.MultiThreadedPOWEngine;
 import org.junit.Test;
 
 import javax.xml.bind.DatatypeConverter;
+import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.security.KeyPairGenerator;
 import java.util.Calendar;
@@ -71,8 +72,8 @@ public class SecurityTest {
     public void testProofOfWorkFails() throws IOException {
         ObjectMessage objectMessage = new ObjectMessage.Builder()
                 .nonce(new byte[8])
-                .expiresTime(300 + (System.currentTimeMillis() / 1000)) // 5 minutes
+                .expiresTime(UnixTime.now() + 300) // 5 minutes
-                .payload(new GenericPayload(1, new byte[0]))
+                .payload(GenericPayload.read(new ByteArrayInputStream(new byte[0]), 1, 0))
                 .build();
         Security.checkProofOfWork(objectMessage, 1000, 1000);
     }
@@ -84,7 +85,7 @@ public class SecurityTest {
         ObjectMessage objectMessage = new ObjectMessage.Builder()
                 .nonce(new byte[8])
                 .expiresTime(expires.getTimeInMillis() / 1000)
-                .payload(new GenericPayload(1, new byte[0]))
+                .payload(GenericPayload.read(new ByteArrayInputStream(new byte[0]), 1, 0))
                 .build();
         Security.doProofOfWork(objectMessage, new MultiThreadedPOWEngine(), 1000, 1000);
         Security.checkProofOfWork(objectMessage, 1000, 1000);

networking/src/test/java/ch/dissem/bitmessage/networking/NetworkNodeTest.java

@@ -21,6 +21,7 @@ import ch.dissem.bitmessage.entity.Version;
 import ch.dissem.bitmessage.entity.payload.ObjectPayload;
 import ch.dissem.bitmessage.entity.valueobject.NetworkAddress;
 import ch.dissem.bitmessage.ports.NetworkHandler;
+import ch.dissem.bitmessage.utils.UnixTime;
 import org.junit.Test;
 
 /**
@@ -44,7 +45,7 @@ public class NetworkNodeTest {
                 new Version.Builder()
                         .version(3)
                         .services(1)
-                        .timestamp(System.currentTimeMillis() / 1000)
+                        .timestamp(UnixTime.now())
                         .addrFrom(localhost)
                         .addrRecv(localhost)
                         .nonce(-1)