Added significant amount of validation to components

Focus as on preventing lazy errors, where the primary issue is an incorrect configuration, but where the runtime error only occurs once the system is up and running, resulting in difficult to diagnose errors. The two ways this was accomplished by: - Add partyId validation whereever possible. - Add `Objects.requireNonNull` checks in component constructors. Additionally: - Added improved dianostics for failing futures in MASCOT `TestRuntime`.
aicis · Apr 2, 2024 · 83ed851 · 83ed851
1 parent 040477a
commit 83ed851
Show file tree

Hide file tree

Showing 30 changed files with 196 additions and 81 deletions.
diff --git a/core/src/main/java/dk/alexandra/fresco/framework/Party.java b/core/src/main/java/dk/alexandra/fresco/framework/Party.java
@@ -1,5 +1,8 @@
 package dk.alexandra.fresco.framework;
 
+import dk.alexandra.fresco.framework.util.ValidationUtils;
+import java.util.Objects;
+
 /**
  * FRESCO's view of a MPC party.
  */
@@ -17,8 +20,10 @@ public class Party {
    * @param port the tcp port to connect on
    */
   public Party(int id, String host, int port) {
+    ValidationUtils.assertValidId(id);
+
     this.id = id;
-    this.host = host;
+    this.host = Objects.requireNonNull(host);
     this.port = port;
   }
 

diff --git a/core/src/main/java/dk/alexandra/fresco/framework/configuration/NetworkConfigurationImpl.java b/core/src/main/java/dk/alexandra/fresco/framework/configuration/NetworkConfigurationImpl.java
@@ -2,6 +2,7 @@
 
 import dk.alexandra.fresco.framework.Party;
 import dk.alexandra.fresco.framework.util.Pair;
+import dk.alexandra.fresco.framework.util.ValidationUtils;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Objects;
@@ -14,14 +15,22 @@ public class NetworkConfigurationImpl implements NetworkConfiguration {
   private final Map<Integer, Party> parties;
 
   public NetworkConfigurationImpl(int myId, Map<Integer, Party> parties) {
+    // Validation
     Objects.requireNonNull(parties);
     checkAddressesUnique(parties);
+    ValidationUtils.assertValidId(myId);
+    if (parties.get(myId) == null) {
+      throw new RuntimeException(String.format("myId %d must be in the parties map: %s", myId, parties));
+    }
+
+    // Set fields
     this.myId = myId;
     this.parties = parties;
   }
 
   @Override
   public Party getParty(int id) {
+    ValidationUtils.assertValidId(id);
     return parties.get(id);
   }
 

diff --git a/core/src/main/java/dk/alexandra/fresco/framework/network/socket/SocketNetwork.java b/core/src/main/java/dk/alexandra/fresco/framework/network/socket/SocketNetwork.java
@@ -3,6 +3,7 @@
 import dk.alexandra.fresco.framework.configuration.NetworkConfiguration;
 import dk.alexandra.fresco.framework.network.CloseableNetwork;
 import dk.alexandra.fresco.framework.util.ExceptionConverter;
+import dk.alexandra.fresco.framework.util.ValidationUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -136,7 +137,7 @@ public SocketNetwork(NetworkConfiguration conf, SocketFactory socketFactory,
   private void startCommunication(Map<Integer, Socket> sockets) {
     for (Entry<Integer, Socket> entry : sockets.entrySet()) {
       final int id = entry.getKey();
-      inRange(id);
+      assertPartyIdInRange(id);
       Socket socket = entry.getValue();
       Receiver receiver = new Receiver(socket);
       this.receivers.put(id, receiver);
@@ -150,7 +151,7 @@ public void send(int partyId, byte[] data) {
     if (partyId == conf.getMyId()) {
       this.selfQueue.add(data);
     } else {
-      inRange(partyId);
+      assertPartyIdInRange(partyId);
       if (!senders.get(partyId).isRunning()) {
         throw new RuntimeException(
             "P" + conf.getMyId() + ": Unable to send to P" + partyId + ". Sender not running");
@@ -164,7 +165,7 @@ public byte[] receive(final int partyId) {
     if (partyId == conf.getMyId()) {
       return ExceptionConverter.safe(selfQueue::take, "Receiving from self failed");
     }
-    inRange(partyId);
+    assertPartyIdInRange(partyId);
     byte[] data;
     data = receivers.get(partyId).pollMessage(RECEIVE_TIMEOUT);
     while (data == null) {
@@ -182,11 +183,8 @@ public byte[] receive(final int partyId) {
    *
    * @param partyId an ID for a party
    */
-  private void inRange(final int partyId) {
-    if (!(0 < partyId && partyId < getNoOfParties() + 1)) {
-      throw new IllegalArgumentException(
-          "Party id " + partyId + " not in range 1 ... " + getNoOfParties());
-    }
+  private void assertPartyIdInRange(final int partyId) {
+    ValidationUtils.assertValidId(partyId, getNoOfParties());
   }
 
   /**

diff --git a/core/src/main/java/dk/alexandra/fresco/framework/sce/SecureComputationEngineImpl.java b/core/src/main/java/dk/alexandra/fresco/framework/sce/SecureComputationEngineImpl.java
@@ -10,6 +10,7 @@
 import dk.alexandra.fresco.framework.sce.resources.ResourcePool;
 import dk.alexandra.fresco.suite.ProtocolSuite;
 import java.time.Duration;
+import java.util.Objects;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
@@ -39,13 +40,13 @@ public class SecureComputationEngineImpl
   /**
    * Creates a new {@link SecureComputationEngineImpl}.
    *
-   * @param protocolSuite the {@link ProtocolSuite} to use to evaluate the secure computation
-   * @param evaluator the {@link ProtocolEvaluator} to run secure evaluation.
+   * @param protocolSuite {@link ProtocolSuite} to use to evaluate the secure computation. Not nullable.
+   * @param evaluator {@link ProtocolEvaluator} to run secure evaluation. Not nullable.
    */
   public SecureComputationEngineImpl(ProtocolSuite<ResourcePoolT, BuilderT> protocolSuite,
       ProtocolEvaluator<ResourcePoolT> evaluator) {
-    this.protocolSuite = protocolSuite;
-    this.evaluator = evaluator;
+    this.protocolSuite = Objects.requireNonNull(protocolSuite);
+    this.evaluator = Objects.requireNonNull(evaluator);
     this.setup = false;
   }
 

diff --git a/core/src/main/java/dk/alexandra/fresco/framework/sce/evaluator/BatchedProtocolEvaluator.java b/core/src/main/java/dk/alexandra/fresco/framework/sce/evaluator/BatchedProtocolEvaluator.java
@@ -5,6 +5,7 @@
 import dk.alexandra.fresco.framework.network.Network;
 import dk.alexandra.fresco.framework.sce.resources.ResourcePool;
 import dk.alexandra.fresco.suite.ProtocolSuite;
+import java.util.Objects;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -34,9 +35,9 @@ public BatchedProtocolEvaluator(
   public BatchedProtocolEvaluator(
       BatchEvaluationStrategy<ResourcePoolT> batchEvaluator,
       ProtocolSuite<ResourcePoolT, ?> protocolSuite, int maxBatchSize) {
-    this.batchEvaluator = batchEvaluator;
+    this.batchEvaluator = Objects.requireNonNull(batchEvaluator);
     this.maxBatchSize = maxBatchSize;
-    this.protocolSuite = protocolSuite;
+    this.protocolSuite = Objects.requireNonNull(protocolSuite);
   }
 
   @Override

diff --git a/core/src/main/java/dk/alexandra/fresco/framework/sce/resources/ResourcePoolImpl.java b/core/src/main/java/dk/alexandra/fresco/framework/sce/resources/ResourcePoolImpl.java
@@ -1,5 +1,7 @@
 package dk.alexandra.fresco.framework.sce.resources;
 
+import dk.alexandra.fresco.framework.util.ValidationUtils;
+
 /**
  * Container for resources needed by runtimes (protocol suites).
  */
@@ -12,10 +14,13 @@ public class ResourcePoolImpl implements ResourcePool {
    * Creates an instance of the default implementation of a resource pool. This contains the basic
    * resources needed within FRESCO.
    *
-   * @param myId The ID of the MPC party.
+   * @param myId The ID of the MPC party. Must be a valid id, but may lie
+   * outside of {@code noOfPlayers} as some subclasses create sub-networks, with fewer noOfPlayers.
    * @param noOfPlayers The amount of parties within the MPC computation.
    */
   public ResourcePoolImpl(int myId, int noOfPlayers) {
+    ValidationUtils.assertValidId(myId);
+    ValidationUtils.assertValidId(noOfPlayers);
     this.myId = myId;
     this.noOfPlayers = noOfPlayers;
   }

diff --git a/core/src/main/java/dk/alexandra/fresco/framework/util/ArithmeticDummyDataSupplier.java b/core/src/main/java/dk/alexandra/fresco/framework/util/ArithmeticDummyDataSupplier.java
@@ -9,8 +9,9 @@
 
 /**
  * Supplies generic pre-processed material common across arithmetic SPDZ-like suites, including
- * random elements, bits, and multiplication triples. <p>Uses {@link Random} to deterministically
- * generate all material. NOT secure.</p>
+ * random elements, bits, and multiplication triples.
+ *
+ * <p>Uses {@link Random} to deterministically generate all material. This is not secure, and should not be used in production code!
  */
 public class ArithmeticDummyDataSupplier {
 
@@ -21,8 +22,9 @@ public class ArithmeticDummyDataSupplier {
   private final Random random;
   private final SecretSharer<BigInteger> sharer;
   private final ModularReductionAlgorithm reducer;
-  
+
   public ArithmeticDummyDataSupplier(int myId, int noOfParties, BigInteger modulus) {
+    ValidationUtils.assertValidId(myId, noOfParties);
     this.myId = myId;
     this.noOfParties = noOfParties;
     this.modulus = modulus;

diff --git a/core/src/main/java/dk/alexandra/fresco/framework/util/DrngImpl.java b/core/src/main/java/dk/alexandra/fresco/framework/util/DrngImpl.java
@@ -1,6 +1,7 @@
 package dk.alexandra.fresco.framework.util;
 
 import java.math.BigInteger;
+import java.util.Objects;
 
 /**
  * A simple implementation based on a deterministic bit generator.
@@ -14,10 +15,10 @@ public class DrngImpl implements Drng {
 
   /**
    * Creates a number generator from a bit generator.
-   * @param drbg a deterministic random bit generator
+   * @param drbg a deterministic random bit generator. Not nullable.
    */
   public DrngImpl(Drbg drbg) {
-    this.drbg = drbg;
+    this.drbg = Objects.requireNonNull(drbg);
   }
 
   @Override

diff --git a/core/src/main/java/dk/alexandra/fresco/framework/util/ValidationUtils.java b/core/src/main/java/dk/alexandra/fresco/framework/util/ValidationUtils.java
@@ -0,0 +1,34 @@
+package dk.alexandra.fresco.framework.util;
+
+/** Contains methods for validating ids. */
+public final class ValidationUtils {
+
+  private ValidationUtils() {}
+
+  /**
+   * Validates that the given party id is within the valid range of ids, without a known max id.
+   *
+   * @param partyId Id to validate
+   * @exception IllegalArgumentException if id is not valid
+   */
+  public static void assertValidId(int partyId) {
+    if (partyId < 1) {
+      throw new IllegalArgumentException(String.format("Party id %d must be one-indexed", partyId));
+    }
+  }
+
+  /**
+   * Validates that the given party id is within the valid range of ids, with a known max id.
+   *
+   * @param partyId Id to validate
+   * @param numParties Max id
+   * @exception IllegalArgumentException if id is not valid
+   */
+  public static void assertValidId(int partyId, int numParties) {
+    assertValidId(partyId);
+    if (numParties < partyId) {
+      throw new IllegalArgumentException(
+          String.format("Party id %d must be in range [1,%d]", partyId, numParties));
+    }
+  }
+}
diff --git a/core/src/main/java/dk/alexandra/fresco/lib/field/integer/BasicNumericContext.java b/core/src/main/java/dk/alexandra/fresco/lib/field/integer/BasicNumericContext.java
@@ -1,7 +1,9 @@
 package dk.alexandra.fresco.lib.field.integer;
 
 import dk.alexandra.fresco.framework.builder.numeric.field.FieldDefinition;
+import dk.alexandra.fresco.framework.util.ValidationUtils;
 import java.math.BigInteger;
+import java.util.Objects;
 
 /**
  * Holds the most crucial properties about the finite field we are working within.
@@ -24,12 +26,13 @@ public class BasicNumericContext {
    *     have.
    * @param myId my party id
    * @param noOfParties number of parties in computation
-   * @param fieldDefinition the field definition used in the application
+   * @param fieldDefinition the field definition used in the application. Nullable.
    * @param defaultFixedPointPrecision the fixed point precision when using the fixed point library
    * @param statisticalSecurityParam the statistical security parameter
    */
   public BasicNumericContext(int maxBitLength, int myId, int noOfParties,
       FieldDefinition fieldDefinition, int defaultFixedPointPrecision, int statisticalSecurityParam) {
+    ValidationUtils.assertValidId(myId, noOfParties);
     this.maxBitLength = maxBitLength;
     this.myId = myId;
     this.noOfParties = noOfParties;

diff --git a/...main/java/dk/alexandra/fresco/suite/dummy/arithmetic/DummyArithmeticResourcePoolImpl.java b/...main/java/dk/alexandra/fresco/suite/dummy/arithmetic/DummyArithmeticResourcePoolImpl.java
@@ -3,6 +3,7 @@
 import dk.alexandra.fresco.framework.builder.numeric.field.FieldDefinition;
 import dk.alexandra.fresco.framework.sce.resources.ResourcePool;
 import dk.alexandra.fresco.framework.sce.resources.ResourcePoolImpl;
+import dk.alexandra.fresco.framework.util.ValidationUtils;
 
 /**
  * Implements the resource pool needed for the Dummy Arithmetic suite.
@@ -22,6 +23,7 @@ public class DummyArithmeticResourcePoolImpl extends ResourcePoolImpl
   public DummyArithmeticResourcePoolImpl(int myId, int noOfPlayers,
       FieldDefinition fieldDefinition) {
     super(myId, noOfPlayers);
+    ValidationUtils.assertValidId(myId, noOfPlayers);
     this.fieldDefinition = fieldDefinition;
   }
 

diff --git a/core/src/test/java/dk/alexandra/fresco/framework/TestThreadRunner.java b/core/src/test/java/dk/alexandra/fresco/framework/TestThreadRunner.java
@@ -193,15 +193,15 @@ public static <ResourcePoolT extends ResourcePool, Builder extends ProtocolBuild
               iterator.remove();
             }
           } catch (InterruptedException e) {
-            throw new TestFrameworkException("Test was interrupted");
+            throw new TestFrameworkException("Test was interrupted", e);
           }
           if (t.setupException != null) {
-            throw new TestFrameworkException(t + " threw exception in setup (see stderr)");
+            throw new TestFrameworkException(t + " threw exception in setup", t.setupException);
           } else if (t.testException != null) {
-            throw new TestFrameworkException(t + " threw exception in test (see stderr)",
+            throw new TestFrameworkException(t + " threw exception in test",
                 t.testException);
           } else if (t.teardownException != null) {
-            throw new TestFrameworkException(t + " threw exception in teardown (see stderr)");
+            throw new TestFrameworkException(t + " threw exception in teardown", t.setupException);
           }
         }
       }

diff --git a/core/src/test/java/dk/alexandra/fresco/framework/sce/TestSecureComputationEngineImpl.java b/core/src/test/java/dk/alexandra/fresco/framework/sce/TestSecureComputationEngineImpl.java
@@ -57,7 +57,7 @@ public void testRunApplication() {
           return builder.numeric().open(builder.numeric().add(a, b));
         };
     DummyArithmeticResourcePool rp =
-        new DummyArithmeticResourcePoolImpl(0, 1, fieldDefinition);
+        new DummyArithmeticResourcePoolImpl(1, 1, fieldDefinition);
 
     BigInteger b = sce.runApplication(app, rp, null);
     assertThat(b, is(BigInteger.valueOf(20)));
@@ -72,7 +72,7 @@ public void testRunApplicationLong() {
           return builder.numeric().open(builder.numeric().add(a, b));
         };
     DummyArithmeticResourcePool rp =
-        new DummyArithmeticResourcePoolImpl(0, 1, fieldDefinition);
+        new DummyArithmeticResourcePoolImpl(1, 1, fieldDefinition);
 
     BigInteger b = sce.runApplication(app, rp, null);
     assertThat(b, is(BigInteger.valueOf(20)));
@@ -85,7 +85,7 @@ public void testRunApplicationAppThrows() {
           throw new RuntimeException();
         };
     DummyArithmeticResourcePool rp =
-        new DummyArithmeticResourcePoolImpl(0, 1, fieldDefinition);
+        new DummyArithmeticResourcePoolImpl(1, 1, fieldDefinition);
     sce.runApplication(app, rp, null);
     fail("Should not be reachable");
   }
@@ -99,7 +99,7 @@ public void testRunApplicationAppTimesOut() {
           }
         };
     DummyArithmeticResourcePool rp =
-        new DummyArithmeticResourcePoolImpl(0, 1, fieldDefinition);
+        new DummyArithmeticResourcePoolImpl(1, 1, fieldDefinition);
     sce.runApplication(app, rp, null, Duration.ofNanos(1));
     fail("Should not be reachable");
   }

diff --git a/core/src/test/java/dk/alexandra/fresco/framework/sce/resources/TestResourcePoolImpl.java b/core/src/test/java/dk/alexandra/fresco/framework/sce/resources/TestResourcePoolImpl.java
@@ -9,8 +9,8 @@ public class TestResourcePoolImpl {
 
   @Test
   public void testResourcePoolImpl() {
-    ResourcePoolImpl rp = new ResourcePoolImpl(0, 1);
-    assertThat(rp.getMyId(), is(0));
+    ResourcePoolImpl rp = new ResourcePoolImpl(1, 1);
+    assertThat(rp.getMyId(), is(1));
     assertThat(rp.getNoOfParties(), is(1));
   }