Use asserts in hashToG2, capitalise class HashToCurve

Consensys · Jan 8, 2020 · 670b282 · 670b282
1 parent c566823
commit 670b282
Show file tree

Hide file tree

Showing 7 changed files with 77 additions and 18 deletions.
diff --git a/util/src/jmh/java/tech/pegasys/artemis/util/bench/BenchBLS.java b/util/src/jmh/java/tech/pegasys/artemis/util/bench/BenchBLS.java
@@ -26,7 +26,7 @@
 import org.openjdk.jmh.annotations.State;
 import org.openjdk.jmh.annotations.Warmup;
 import org.openjdk.jmh.infra.Blackhole;
-import tech.pegasys.artemis.util.hashToG2.hashToCurve;
+import tech.pegasys.artemis.util.hashToG2.HashToCurve;
 import tech.pegasys.artemis.util.mikuli.G2Point;
 
 @BenchmarkMode(Mode.AverageTime)
@@ -47,7 +47,7 @@ public void setup() {
 
   @Benchmark
   public void hashToCurve(Blackhole blackhole) {
-    G2Point result = new G2Point(hashToCurve.hashToG2(message, suite));
+    G2Point result = new G2Point(HashToCurve.hashToG2(message, suite));
     blackhole.consume(result);
   }
 }
diff --git a/...ys/artemis/util/hashToG2/hashToCurve.java → ...ys/artemis/util/hashToG2/HashToCurve.java b/...ys/artemis/util/hashToG2/hashToCurve.java → ...ys/artemis/util/hashToG2/HashToCurve.java
@@ -15,9 +15,10 @@
 
 import static tech.pegasys.artemis.util.hashToG2.Helper.clear_h2;
 import static tech.pegasys.artemis.util.hashToG2.Helper.hashToBase;
+import static tech.pegasys.artemis.util.hashToG2.Helper.isInG2;
 import static tech.pegasys.artemis.util.hashToG2.Helper.iso3;
 import static tech.pegasys.artemis.util.hashToG2.Helper.mapToCurve;
-import static tech.pegasys.artemis.util.hashToG2.Helper.onCurveG2;
+import static tech.pegasys.artemis.util.hashToG2.Helper.isOnCurve;
 
 import java.nio.charset.StandardCharsets;
 import org.apache.milagro.amcl.BLS381.ECP2;
@@ -42,7 +43,7 @@
  * purposes of hashing to G2 within Ethereum I believe that this is of no consequence, since all the
  * input information is publicly known.
  */
-public class hashToCurve {
+public class HashToCurve {
 
   // The cipher suite is defined in the Eth2 specs
   private static final Bytes CIPHER_SUITE =
@@ -65,12 +66,14 @@ public static ECP2 hashToG2(Bytes message, Bytes cipherSuite) {
 
     JacobianPoint p = iso3(q0.add(q1));
 
-    if (!onCurveG2(p)) {
-      throw new RuntimeException("hashToCurve failed for unknown reasons.");
-    }
+    // This should never fail, and the check is non-trivial, so we use an assert
+    assert(isOnCurve(p));
 
     JacobianPoint q = clear_h2(p);
 
+    // This should never fail, and the check is very expensive, so we use an assert
+    assert(isInG2(q));
+
     return q.toECP2();
   }
 

diff --git a/util/src/main/java/tech/pegasys/artemis/util/hashToG2/Helper.java b/util/src/main/java/tech/pegasys/artemis/util/hashToG2/Helper.java
@@ -16,6 +16,7 @@
 import static tech.pegasys.artemis.util.hashToG2.Chains.expChain;
 import static tech.pegasys.artemis.util.hashToG2.Chains.h2Chain;
 import static tech.pegasys.artemis.util.hashToG2.Chains.mxChain;
+import static tech.pegasys.artemis.util.hashToG2.Chains.qChain;
 import static tech.pegasys.artemis.util.hashToG2.FP2Immutable.ONE;
 import static tech.pegasys.artemis.util.hashToG2.IetfTools.HKDF_Expand;
 import static tech.pegasys.artemis.util.hashToG2.IetfTools.HKDF_Extract;
@@ -33,7 +34,7 @@ class Helper {
    * @param p a JacobianPoint
    * @return true if the point is on the curve, false otherwise
    */
-  static boolean onCurveG2(JacobianPoint p) {
+  static boolean isOnCurve(JacobianPoint p) {
     if (p.isInfinity()) {
       return true;
     }
@@ -50,6 +51,16 @@ static boolean onCurveG2(JacobianPoint p) {
     return y2.equals(x3.add(z6.mul(four)));
   }
 
+  /**
+   * Tests whether the given point lies in the G2 group.
+   *
+   * @param p a JacobianPoint
+   * @return true if the point is in G2, false otherwise
+   */
+  static boolean isInG2(JacobianPoint p) {
+    return isOnCurve(p) && qChain(p).isInfinity();
+  }
+
   /**
    * Hashes a string msg of any length into an element of the FP2 field.
    *

diff --git a/util/src/test/java/tech/pegasys/artemis/util/hashToG2/HelperTest.java b/util/src/test/java/tech/pegasys/artemis/util/hashToG2/HelperTest.java
@@ -15,21 +15,22 @@
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static tech.pegasys.artemis.util.hashToG2.Helper.clear_h2;
 import static tech.pegasys.artemis.util.hashToG2.Helper.hashToBase;
 import static tech.pegasys.artemis.util.hashToG2.Helper.iso3;
 import static tech.pegasys.artemis.util.hashToG2.Helper.mapToCurve;
-import static tech.pegasys.artemis.util.hashToG2.Helper.onCurveG2;
 import static tech.pegasys.artemis.util.hashToG2.Util.bigFromHex;
 
+import org.apache.milagro.amcl.BLS381.FP;
 import org.apache.tuweni.bytes.Bytes;
 import org.junit.jupiter.api.Test;
 
 class HelperTest {
 
   /*
-   * These tests use data generated from the reference implementation at
+   * Most of these tests use data generated from the reference implementation at
    * https://github.com/algorand/bls_sigs_ref/tree/master/python-impl
    */
 
@@ -47,7 +48,18 @@ void isOnCurve() {
                 "0x0f2fd428b0be2b652deb7d922412bc082171420e723c93c43d1efe6b22acb0f7fff928f441fd09d3aab9c97acec9f579",
                 "0x117318fc47ee37c48a854cac7b5e83a5f3a10693e200aa696d99dbd3d8721e081bf662149a67b81aa71f08c4f030959d"));
 
-    assertTrue(onCurveG2(p));
+    assertTrue(Helper.isOnCurve(p));
+  }
+
+  @Test
+  void isNotOnCurve() {
+    JacobianPoint p =
+        new JacobianPoint(
+            new FP2Immutable(new FP(1), new FP(2)),
+            new FP2Immutable(new FP(3), new FP(4)),
+            new FP2Immutable(new FP(5), new FP(6)));
+
+    assertFalse(Helper.isOnCurve(p));
   }
 
   @Test
@@ -192,4 +204,36 @@ void clearH2Test() {
                 "0x00a0ede04aa5881555e0dc51a8db295b393cb349ea7be5547829b2f102191b2d06118e71a8a756db2316cf15a8378b72"));
     assertEquals(expected, clear_h2(a));
   }
+
+  @Test
+  void pointIsInG2() {
+    JacobianPoint a =
+        new JacobianPoint(
+            new FP2Immutable(
+                "0x16e8fe7af2c3366b444bf79673f540016ae13df03367f409e6e218b30db675f7186ab56809583f33e672edf61f812627",
+                "0x03920e2e7294fcd8a40dd98f1252061d72ec9f82b16732a2f573b509c53271fcef2eab796d58ba443def766269085fb4"),
+            new FP2Immutable(
+                "0x033fe28c591de8a46d2bbe8cd381c0b6a13bae7e0a8ca6a87c6a727c9cd52dbf7b882a4bba9123ee6ee480e34368ba1f",
+                "0x050a1ef21da10a01b75d187a5a6931c7e4c33794d7dc20d2c619ce00f9bfb0380a40d3cff0101ffac993e742798f9fff"),
+            new FP2Immutable(
+                "0x0f008a6b5637c0f7e5957c3c56ba616c5477f1dce08d6805f42dc495faed05ad046304ec7e2c7229694b82f6c886c6f8",
+                "0x00a0ede04aa5881555e0dc51a8db295b393cb349ea7be5547829b2f102191b2d06118e71a8a756db2316cf15a8378b72"));
+    assertTrue(Helper.isInG2(a));
+  }
+
+  @Test
+  void pointIsNotInG2() {
+    JacobianPoint a =
+        new JacobianPoint(
+            new FP2Immutable(
+                "0x0c8977fab5175ac2f09e5f39e29d016f11c094ef10f237d2a5e23f482d0bfb4466688527cd31685bfe481725c31462cc",
+                "0x0b305838069012861bb63501841c91bd5bc7e1359d44cd196681fb14c03e544c22205bced326d490eb886aaa3ed52918"),
+            new FP2Immutable(
+                "0x172cf997b3501882861c07e852fadbf5753eb8a3e1d2ce375e6aed07cf9c1b5ff1cbf1124c6e3b0cf4607c683eafd1a4",
+                "0x0d9dacf241a753d55cff6d45b568b716a2ad68ba29d23f92dea6e7cf6ed54e96cdac4a2b95213f93439b946ebc63349c"),
+            new FP2Immutable(
+                "0x05594bb289f0ebfd8fa3f020c6e1eaf4c49b97d8ccaf3470a3a02da4b3e7104778105bd6c7e0caf97206c77a8b501d4d",
+                "0x0625151f905fad40eb0e2b9b0a46d9afe531256c6d5e39897a27d94700f037a761a741d11275180bd18e620289e02a16"));
+    assertFalse(Helper.isInG2(a));
+  }
 }
diff --git a/util/src/test/java/tech/pegasys/artemis/util/hashToG2/JacobianPointTest.java b/util/src/test/java/tech/pegasys/artemis/util/hashToG2/JacobianPointTest.java
@@ -17,7 +17,7 @@
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
-import static tech.pegasys.artemis.util.hashToG2.Helper.onCurveG2;
+import static tech.pegasys.artemis.util.hashToG2.Helper.isOnCurve;
 import static tech.pegasys.artemis.util.hashToG2.JacobianPoint.INFINITY;
 import static tech.pegasys.artemis.util.hashToG2.Util.bigFromHex;
 
@@ -126,7 +126,7 @@ void toAffineTest() {
                 "0x0374fe17a6875101c7172d962c383f065326c8e8c3f4971456224efe18619f404d7cfab6b3d7df460f44f55f72176eb2",
                 "0x0d15e558d16fe21b72637e9f0d53684cd52fe9dc41dc3dba522370048239b9b687003c8d3e928c2d17ed7451f9c91676"),
             FP2Immutable.ONE);
-    assertTrue(onCurveG2(a));
+    assertTrue(isOnCurve(a));
     assertEquals(expected.toString(), a.toAffine().toString());
   }
 
@@ -157,8 +157,8 @@ void equalsTest() {
             new FP2Immutable(
                 "0x05594bb289f0ebfd8fa3f020c6e1eaf4c49b97d8ccaf3470a3a02da4b3e7104778105bd6c7e0caf97206c77a8b501d4d",
                 "0x0625151f905fad40eb0e2b9b0a46d9afe531256c6d5e39897a27d94700f037a761a741d11275180bd18e620289e02a16"));
-    assertTrue(onCurveG2(a));
-    assertTrue(onCurveG2(b));
+    assertTrue(isOnCurve(a));
+    assertTrue(isOnCurve(b));
     assertNotEquals(a.add(b).getZ(), b.add(a).getZ());
     assertEquals(a.add(b), b.add(a));
   }

diff --git a/util/src/test/java/tech/pegasys/artemis/util/hashToG2/ReferenceTests.java b/util/src/test/java/tech/pegasys/artemis/util/hashToG2/ReferenceTests.java
@@ -61,7 +61,7 @@ class ReferenceTests {
   })
   void referenceTest(
       String fileName, int testNumber, Bytes message, Bytes suite, G2Point expected) {
-    G2Point actual = new G2Point(hashToCurve.hashToG2(message, suite));
+    G2Point actual = new G2Point(HashToCurve.hashToG2(message, suite));
     assertEquals(expected, actual);
   }
 

diff --git a/util/src/test/java/tech/pegasys/artemis/util/hashToG2/hashToCurveTest.java b/util/src/test/java/tech/pegasys/artemis/util/hashToG2/hashToCurveTest.java
@@ -17,7 +17,8 @@
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static tech.pegasys.artemis.util.hashToG2.Chains.qChain;
-import static tech.pegasys.artemis.util.hashToG2.hashToCurve.hashToG2;
+import static tech.pegasys.artemis.util.hashToG2.HashToCurve.hashToG2;
+import static tech.pegasys.artemis.util.hashToG2.Helper.isInG2;
 
 import java.util.ArrayList;
 import java.util.stream.Stream;
@@ -39,7 +40,7 @@ void hashToG2Test2(Integer i) {
         hashToG2(
             Bytes.concatenate(Bytes.wrap("Hello, world!".getBytes(UTF_8)), Bytes.ofUnsignedInt(i)));
     assertFalse(point.is_infinity());
-    assertTrue(qChain(new JacobianPoint(point)).isInfinity());
+    assertTrue(isInG2(new JacobianPoint(point)));
   }
 
   public static Stream<Arguments> getIndices() {