#73 Hardening of SDK

src/main/java/org/unicitylabs/sdk/api/InclusionCertificate.java

@@ -3,14 +3,11 @@
 import org.unicitylabs.sdk.crypto.hash.DataHash;
 import org.unicitylabs.sdk.crypto.hash.DataHasher;
 import org.unicitylabs.sdk.crypto.hash.HashAlgorithm;
-import org.unicitylabs.sdk.smt.radix.FinalizedBranch;
-import org.unicitylabs.sdk.smt.radix.FinalizedLeafBranch;
-import org.unicitylabs.sdk.smt.radix.FinalizedNodeBranch;
-import org.unicitylabs.sdk.util.BitString;
+import org.unicitylabs.sdk.smt.SparseMerkleTreePathUtils;
+import org.unicitylabs.sdk.smt.radix.SparseMerkleTreeRootNode;
 import org.unicitylabs.sdk.util.HexConverter;
 import org.unicitylabs.sdk.util.LongConverter;
 
-import java.math.BigInteger;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -29,35 +26,23 @@ private InclusionCertificate(byte[] bitmap, List<DataHash> siblings) {
     this.siblings = siblings;
   }
 
-  public static InclusionCertificate create(FinalizedNodeBranch root, byte[] key) {
-    FinalizedBranch node = root;
+  public static InclusionCertificate create(SparseMerkleTreeRootNode root, byte[] key) {
+    Objects.requireNonNull(root, "root cannot be null");
+    Objects.requireNonNull(key, "key cannot be null");
+    if (key.length != 32) {
+      throw new IllegalArgumentException("Key must be 32 bytes long.");
+    }
 
     ArrayList<DataHash> siblings = new ArrayList<>();
     byte[] bitmap = new byte[InclusionCertificate.BITMAP_SIZE];
-    BigInteger keyPath = BitString.fromBytesReversedLSB(key).toBigInteger();
-
-    while (node != null) {
-      if (node instanceof FinalizedLeafBranch) {
-        FinalizedLeafBranch leaf = (FinalizedLeafBranch) node;
-        if (!Arrays.equals(leaf.getKey(), key)) {
-          throw new RuntimeException(String.format("Leaf not found for key: %s", HexConverter.encode(key)));
-        }
-
-        return new InclusionCertificate(bitmap, siblings);
-      }
 
-      FinalizedNodeBranch nodeBranch = (FinalizedNodeBranch) node;
-      boolean isRight = keyPath.testBit(nodeBranch.getDepth());
-      FinalizedBranch sibling = isRight ? nodeBranch.getLeft() : nodeBranch.getRight();
-      if (sibling != null) {
-        bitmap[nodeBranch.getDepth() / 8] |= (byte) (1 << nodeBranch.getDepth() % 8);
-        siblings.add(sibling.getHash());
-      }
-
-      node = isRight ? nodeBranch.getRight() : nodeBranch.getLeft();
+    for (SparseMerkleTreeRootNode.Sibling sibling : root.getPath(key)) {
+      int depth = sibling.getDepth();
+      bitmap[depth >> 3] |= (byte) (0x80 >> (depth & 7));
+      siblings.add(sibling.getHash());
     }
 
-    throw new RuntimeException("Could not construct inclusion certificate: Invalid path");
+    return new InclusionCertificate(bitmap, siblings);
   }
 
   public static InclusionCertificate decode(byte[] bytes) {
@@ -113,20 +98,17 @@ public boolean verify(StateId leafKey, DataHash leafValue, DataHash expectedRoot
             .update(value)
             .digest();
 
-    BigInteger keyPath = BitString.fromBytesReversedLSB(key).toBigInteger();
-    BigInteger bitmapPath = BitString.fromBytesReversedLSB(this.bitmap).toBigInteger();
-
     int position = this.siblings.size();
     for (int depth = InclusionCertificate.MAX_DEPTH; depth >= 0; depth--) {
-      if (!bitmapPath.testBit(depth)) continue;
+      if (SparseMerkleTreePathUtils.getBitAtDepth(this.bitmap, depth) == 0) continue;
 
       position -= 1;
       if (position < 0) return false;
 
       DataHash sibling = this.siblings.get(position);
 
       byte[] left, right;
-      if (keyPath.testBit(depth)) {
+      if (SparseMerkleTreePathUtils.getBitAtDepth(key, depth) == 1) {
         left = sibling.getData();
         right = hash.getData();
       } else {
@@ -137,6 +119,7 @@ public boolean verify(StateId leafKey, DataHash leafValue, DataHash expectedRoot
       hash = new DataHasher(HashAlgorithm.SHA256)
               .update(new byte[]{0x01})
               .update(LongConverter.encode(depth))
+              .update(SparseMerkleTreePathUtils.regionFromKey(key, depth))
               .update(left)
               .update(right)
               .digest();

src/main/java/org/unicitylabs/sdk/api/JsonRpcAggregatorClient.java

@@ -3,8 +3,10 @@
 import org.unicitylabs.sdk.api.jsonrpc.JsonRpcHttpTransport;
 import org.unicitylabs.sdk.util.HexConverter;
 
+import java.net.URI;
 import java.util.HashMap;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
 import java.util.concurrent.CompletableFuture;
@@ -31,17 +33,46 @@ public JsonRpcAggregatorClient(String url) {
 
 
   /**
-   * Create aggregator client for destination url with api key.
+   * Create aggregator client for destination url with api key. When an api key is supplied the
+   * url must be {@code https}, so the key is never sent over plaintext.
    *
    * @param url    destination url
    * @param apiKey api key
    *
+   * @throws IllegalArgumentException if an api key is supplied for a non-https url
    */
   public JsonRpcAggregatorClient(String url, String apiKey) {
-    this.transport = new JsonRpcHttpTransport(Objects.requireNonNull(url, "url cannot be null"));
+    this(url, apiKey, false);
+  }
+
+  /**
+   * Create aggregator client for destination url with api key.
+   *
+   * @param url    destination url
+   * @param apiKey api key
+   * @param allowInsecureTransport when {@code true}, permit sending the api key over a non-https
+   *     url (intended for local development and testing only)
+   *
+   * @throws IllegalArgumentException if an api key is supplied for a non-https url and
+   *     {@code allowInsecureTransport} is {@code false}
+   */
+  public JsonRpcAggregatorClient(String url, String apiKey, boolean allowInsecureTransport) {
+    Objects.requireNonNull(url, "url cannot be null");
+
+    if (apiKey != null && !allowInsecureTransport && !JsonRpcAggregatorClient.isHttps(url)) {
+      throw new IllegalArgumentException(
+              "API key must not be sent over plaintext HTTP; use an https url.");
+    }
+
+    this.transport = new JsonRpcHttpTransport(url);
     this.apiKey = apiKey;
   }
 
+  private static boolean isHttps(String url) {
+    String scheme = URI.create(url).getScheme();
+    return scheme != null && scheme.toLowerCase(Locale.ROOT).equals("https");
+  }
+
   /**
    * Submit a certification request for a transaction state transition.
    *

src/main/java/org/unicitylabs/sdk/api/NetworkId.java

@@ -6,53 +6,58 @@
  */
 public final class NetworkId {
 
-  public static final NetworkId MAINNET = new NetworkId((short) 1, "MAINNET");
-  public static final NetworkId TESTNET = new NetworkId((short) 2, "TESTNET");
-  public static final NetworkId LOCAL = new NetworkId((short) 3, "LOCAL");
+  public static final NetworkId MAINNET = new NetworkId(1, "MAINNET");
+  public static final NetworkId TESTNET = new NetworkId(2, "TESTNET");
+  public static final NetworkId LOCAL = new NetworkId(3, "LOCAL");
 
-  private final short id;
+  private final int id;
   private final String name;
 
-  private NetworkId(short id, String name) {
+  private NetworkId(int id, String name) {
     this.id = id;
     this.name = name;
   }
 
-  private NetworkId(short id) {
+  private NetworkId(int id) {
     this(id, null);
   }
 
   /**
-   * Resolve a NetworkId from its numeric identifier. Returns the registered
-   * singleton for known ids; constructs a new (unnamed) instance for any
-   * other 16-bit value.
+   * Resolve a NetworkId from its raw 16-bit identifier. The whole 16-bit unsigned range
+   * {@code [1, 65535]} is valid; a {@code short} carries every bit pattern, so ids at or above
+   * {@code 0x8000} are supplied as negative shorts and interpreted as their unsigned value.
+   * Returns the registered singleton for known ids; constructs a new (unnamed) instance for any
+   * other value.
    *
-   * @param id numeric network identifier
+   * @param id raw 16-bit network identifier
    * @return NetworkId for the given identifier
    */
   public static NetworkId fromId(short id) {
-    if (id <= 0) {
-      throw new IllegalArgumentException(
-          "Network identifier out of allowed 16-bit unsigned range: " + id + ".");
+    int value = id & 0xFFFF;
+    if (value == 0) {
+      throw new IllegalArgumentException("Network identifier cannot be zero.");
     }
-    if (id == MAINNET.id) {
+
+    if (value == MAINNET.id) {
       return MAINNET;
     }
-    if (id == TESTNET.id) {
+    if (value == TESTNET.id) {
       return TESTNET;
     }
-    if (id == LOCAL.id) {
+    if (value == LOCAL.id) {
       return LOCAL;
     }
-    return new NetworkId(id);
+    return new NetworkId(value);
   }
 
   /**
-   * Get the numeric network identifier.
+   * Get the network identifier as its unsigned 16-bit value in {@code [1, 65535]}. Returned as an
+   * {@code int} so ids at or above {@code 0x8000} are not sign-extended when encoded or written as
+   * a number.
    *
-   * @return numeric identifier
+   * @return unsigned numeric identifier
    */
-  public short getId() {
+  public int getId() {
     return this.id;
   }
 
@@ -69,7 +74,7 @@ public boolean equals(Object o) {
 
   @Override
   public int hashCode() {
-    return Short.hashCode(this.id);
+    return Integer.hashCode(this.id);
   }
 
   @Override

src/main/java/org/unicitylabs/sdk/api/bft/RootTrustBase.java

@@ -8,8 +8,12 @@
 import org.unicitylabs.sdk.serializer.UnicityObjectMapper;
 import org.unicitylabs.sdk.serializer.json.JsonSerializationException;
 import org.unicitylabs.sdk.serializer.json.LongAsStringSerializer;
+import org.unicitylabs.sdk.util.HexConverter;
 
 import java.util.Arrays;
+import java.util.HashSet;
+import java.util.LinkedHashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
@@ -20,11 +24,12 @@
  */
 public class RootTrustBase {
 
-  private final long version;
+  private static final long VERSION = 1;
+
   private final NetworkId networkId;
   private final long epoch;
   private final long epochStartRound;
-  private final Set<NodeInfo> rootNodes;
+  private final Map<String, NodeInfo> rootNodes;
   private final long quorumThreshold;
   private final byte[] stateHash;
   private final byte[] changeRecordHash;
@@ -37,18 +42,46 @@ public class RootTrustBase {
           @JsonProperty("networkId") NetworkId networkId,
           @JsonProperty("epoch") long epoch,
           @JsonProperty("epochStartRound") long epochStartRound,
-          @JsonProperty("rootNodes") Set<NodeInfo> rootNodes,
+          @JsonProperty("rootNodes") List<NodeInfo> rootNodes,
           @JsonProperty("quorumThreshold") long quorumThreshold,
           @JsonProperty("stateHash") byte[] stateHash,
           @JsonProperty("changeRecordHash") byte[] changeRecordHash,
           @JsonProperty("previousEntryHash") byte[] previousEntryHash,
           @JsonProperty("signatures") Map<String, byte[]> signatures
   ) {
-    this.version = version;
+    if (version != RootTrustBase.VERSION) {
+      throw new IllegalArgumentException(
+              String.format("Unsupported RootTrustBase version: %s", version));
+    }
+
+    Objects.requireNonNull(rootNodes, "rootNodes cannot be null");
+
+    Map<String, NodeInfo> nodes = new LinkedHashMap<>();
+    Set<String> signingKeys = new HashSet<>();
+    for (NodeInfo node : rootNodes) {
+      Objects.requireNonNull(node, "Trust base node cannot be null");
+      if (nodes.putIfAbsent(node.getNodeId(), node) != null) {
+        throw new IllegalArgumentException(
+                String.format("Duplicate trust base node id: %s", node.getNodeId()));
+      }
+      if (!signingKeys.add(HexConverter.encode(node.getSigningKey()))) {
+        throw new IllegalArgumentException("Duplicate trust base signing key.");
+      }
+    }
+
+    if (nodes.isEmpty()) {
+      throw new IllegalArgumentException("Trust base must contain at least one root node.");
+    }
+
+    if (quorumThreshold < 1 || quorumThreshold > nodes.size()) {
+      throw new IllegalArgumentException(
+              "Trust base quorum threshold must be between 1 and the root node count.");
+    }
+
     this.networkId = networkId;
     this.epoch = epoch;
     this.epochStartRound = epochStartRound;
-    this.rootNodes = Set.copyOf(rootNodes);
+    this.rootNodes = nodes;
     this.quorumThreshold = quorumThreshold;
     this.stateHash = Arrays.copyOf(stateHash, stateHash.length);
     this.changeRecordHash = changeRecordHash == null
@@ -71,7 +104,7 @@ public class RootTrustBase {
    */
   @JsonSerialize(using = LongAsStringSerializer.class)
   public long getVersion() {
-    return this.version;
+    return RootTrustBase.VERSION;
   }
 
   /**
@@ -109,7 +142,17 @@ public long getEpochStartRound() {
    * @return root nodes
    */
   public Set<NodeInfo> getRootNodes() {
-    return this.rootNodes;
+    return Set.copyOf(this.rootNodes.values());
+  }
+
+  /**
+   * Get root node by node id.
+   *
+   * @param nodeId node id
+   * @return node info, or {@code null} when no node with the given id exists
+   */
+  public NodeInfo getRootNode(String nodeId) {
+    return this.rootNodes.get(nodeId);
   }
 
   /**
@@ -210,6 +253,12 @@ public static class NodeInfo {
             @JsonProperty("sigKey") byte[] signingKey,
             @JsonProperty("stake") long stakedAmount
     ) {
+      Objects.requireNonNull(nodeId, "Node id cannot be null");
+      Objects.requireNonNull(signingKey, "Node signing key cannot be null");
+      if (stakedAmount <= 0) {
+        throw new IllegalArgumentException("Each trust base root node must have positive stake.");
+      }
+
       this.nodeId = nodeId;
       this.signingKey = Arrays.copyOf(signingKey, signingKey.length);
       this.stakedAmount = stakedAmount;