CryptoContext Refactor (#1469)

* Moved logic to disable use of secp256k1 library into crypto project

* Fixed secp doc

core/src/main/scala/org/bitcoins/core/crypto/ExtKey.scala

@@ -1,12 +1,13 @@
 package org.bitcoins.core.crypto
 
-import org.bitcoin.{NativeSecp256k1, Secp256k1Context}
+import org.bitcoin.NativeSecp256k1
 import org.bitcoins.core.hd.{BIP32Node, BIP32Path}
 import org.bitcoins.core.number.{UInt32, UInt8}
 import org.bitcoins.core.util._
 import org.bitcoins.crypto.{
   BaseECKey,
   BouncyCastleUtil,
+  CryptoContext,
   CryptoUtil,
   ECDigitalSignature,
   ECPrivateKey,
@@ -210,11 +211,12 @@ sealed abstract class ExtPrivateKey
     val (il, ir) = hmac.splitAt(32)
     //should be ECGroup addition
     //parse256(IL) + kpar (mod n)
-    val tweak = if (Secp256k1Context.isEnabled) {
-      NativeSecp256k1.privKeyTweakAdd(il.toArray, key.bytes.toArray)
-    } else {
-      val sum = BouncyCastleUtil.addNumbers(key.bytes, il)
-      sum.toByteArray
+    val tweak = CryptoContext.default match {
+      case CryptoContext.LibSecp256k1 =>
+        NativeSecp256k1.privKeyTweakAdd(il.toArray, key.bytes.toArray)
+      case CryptoContext.BouncyCastle =>
+        val sum = BouncyCastleUtil.addNumbers(key.bytes, il)
+        sum.toByteArray
     }
     val childKey = ECPrivateKey(ByteVector(tweak))
     val fp = CryptoUtil.sha256Hash160(key.publicKey.bytes).bytes.take(4)
@@ -368,14 +370,15 @@ sealed abstract class ExtPublicKey extends ExtKey {
       val hmac = CryptoUtil.hmac512(chainCode.bytes, data)
       val (il, ir) = hmac.splitAt(32)
       val priv = ECPrivateKey(il)
-      val childPubKey = if (Secp256k1Context.isEnabled) {
-        val tweaked = NativeSecp256k1.pubKeyTweakAdd(key.bytes.toArray,
-                                                     il.toArray,
-                                                     priv.isCompressed)
-        ECPublicKey(ByteVector(tweaked))
-      } else {
-        val tweak = ECPrivateKey.fromBytes(il).publicKey
-        key.add(tweak)
+      val childPubKey = CryptoContext.default match {
+        case CryptoContext.LibSecp256k1 =>
+          val tweaked = NativeSecp256k1.pubKeyTweakAdd(key.bytes.toArray,
+                                                       il.toArray,
+                                                       priv.isCompressed)
+          ECPublicKey(ByteVector(tweaked))
+        case CryptoContext.BouncyCastle =>
+          val tweak = ECPrivateKey.fromBytes(il).publicKey
+          key.add(tweak)
       }
 
       //we do not handle this case since it is impossible

crypto-test/src/test/scala/org/bitcoins/crypto/BouncyCastleSecp256k1Test.scala

@@ -1,6 +1,7 @@
 package org.bitcoins.crypto
 
-import org.bitcoin.{NativeSecp256k1, Secp256k1Context}
+import org.bitcoin.NativeSecp256k1
+import org.bitcoins.crypto.CryptoContext.{BouncyCastle, LibSecp256k1}
 import org.bitcoins.testkit.core.gen.{CryptoGenerators, NumberGenerator}
 import org.bitcoins.testkit.util.BitcoinSUnitTest
 import org.scalacheck.Gen
@@ -12,11 +13,11 @@ class BouncyCastleSecp256k1Test extends BitcoinSUnitTest {
   behavior of "CryptoLibraries"
 
   override def withFixture(test: NoArgTest): Outcome = {
-    if (Secp256k1Context.isEnabled) {
-      super.withFixture(test)
-    } else {
-      logger.warn(s"Test ${test.name} skipped as Secp256k1 is not available.")
-      Succeeded
+    CryptoContext.default match {
+      case CryptoContext.LibSecp256k1 => super.withFixture(test)
+      case CryptoContext.BouncyCastle =>
+        logger.warn(s"Test ${test.name} skipped as Secp256k1 is not available.")
+        Succeeded
     }
   }
 
@@ -55,33 +56,34 @@ class BouncyCastleSecp256k1Test extends BitcoinSUnitTest {
                                     NumberGenerator.bytevector(33))
     forAll(keyOrGarbageGen) { bytes =>
       assert(
-        ECPublicKey.isFullyValid(bytes, useSecp = false) ==
-          ECPublicKey.isFullyValid(bytes, useSecp = true)
+        ECPublicKey.isFullyValid(bytes, context = BouncyCastle) ==
+          ECPublicKey.isFullyValid(bytes, context = LibSecp256k1)
       )
     }
   }
 
   it must "decompress keys the same" in {
     forAll(CryptoGenerators.publicKey) { pubKey =>
       assert(
-        pubKey.decompressed(useSecp = false) == pubKey.decompressed(
-          useSecp = true))
+        pubKey.decompressed(context = BouncyCastle) == pubKey.decompressed(
+          context = LibSecp256k1))
     }
   }
 
   it must "compute public keys the same" in {
     forAll(CryptoGenerators.privateKey) { privKey =>
       assert(
-        privKey.publicKey(useSecp = false) == privKey.publicKey(useSecp = true))
+        privKey.publicKey(context = BouncyCastle) == privKey.publicKey(
+          context = LibSecp256k1))
     }
   }
 
   it must "compute signatures the same" in {
     forAll(CryptoGenerators.privateKey, NumberGenerator.bytevector(32)) {
       case (privKey, bytes) =>
         assert(
-          privKey.sign(bytes, useSecp = false) == privKey.sign(bytes,
-                                                               useSecp = true))
+          privKey.sign(bytes, context = BouncyCastle) == privKey
+            .sign(bytes, context = LibSecp256k1))
     }
   }
 
@@ -93,11 +95,11 @@ class BouncyCastleSecp256k1Test extends BitcoinSUnitTest {
         val sig = privKey.sign(bytes)
         val pubKey = privKey.publicKey
         assert(
-          pubKey.verify(bytes, sig, useSecp = false) == pubKey
-            .verify(bytes, sig, useSecp = true))
+          pubKey.verify(bytes, sig, context = BouncyCastle) == pubKey
+            .verify(bytes, sig, context = LibSecp256k1))
         assert(
-          pubKey.verify(bytes, badSig, useSecp = false) == pubKey
-            .verify(bytes, badSig, useSecp = true))
+          pubKey.verify(bytes, badSig, context = BouncyCastle) == pubKey
+            .verify(bytes, badSig, context = LibSecp256k1))
     }
   }
 }

crypto/src/main/scala/org/bitcoins/crypto/CryptoContext.scala

@@ -0,0 +1,25 @@
+package org.bitcoins.crypto
+
+import org.bitcoin.Secp256k1Context
+
+sealed trait CryptoContext
+
+object CryptoContext {
+
+  case object LibSecp256k1 extends CryptoContext
+
+  case object BouncyCastle extends CryptoContext
+
+  def default: CryptoContext = {
+    val secpDisabled = System.getenv("DISABLE_SECP256K1")
+    if (secpDisabled != null && (secpDisabled.toLowerCase == "true" || secpDisabled == "1")) {
+      BouncyCastle
+    } else {
+      if (Secp256k1Context.isEnabled) {
+        LibSecp256k1
+      } else {
+        BouncyCastle
+      }
+    }
+  }
+}

crypto/src/main/scala/org/bitcoins/crypto/ECKey.scala

@@ -3,7 +3,7 @@ package org.bitcoins.crypto
 import java.math.BigInteger
 import java.security.SecureRandom
 
-import org.bitcoin.{NativeSecp256k1, Secp256k1Context}
+import org.bitcoin.NativeSecp256k1
 import org.bouncycastle.crypto.AsymmetricCipherKeyPair
 import org.bouncycastle.crypto.generators.ECKeyPairGenerator
 import org.bouncycastle.crypto.params.{
@@ -42,15 +42,16 @@ sealed abstract class ECPrivateKey
     * @return the digital signature
     */
   override def sign(dataToSign: ByteVector): ECDigitalSignature = {
-    sign(dataToSign, Secp256k1Context.isEnabled)
+    sign(dataToSign, CryptoContext.default)
   }
 
-  def sign(dataToSign: ByteVector, useSecp: Boolean): ECDigitalSignature = {
+  def sign(
+      dataToSign: ByteVector,
+      context: CryptoContext): ECDigitalSignature = {
     require(dataToSign.length == 32 && bytes.length <= 32)
-    if (useSecp) {
-      signWithSecp(dataToSign)
-    } else {
-      signWithBouncyCastle(dataToSign)
+    context match {
+      case CryptoContext.LibSecp256k1 => signWithSecp(dataToSign)
+      case CryptoContext.BouncyCastle => signWithBouncyCastle(dataToSign)
     }
   }
 
@@ -73,14 +74,13 @@ sealed abstract class ECPrivateKey
   /** Signifies if the this private key corresponds to a compressed public key */
   def isCompressed: Boolean
 
-  override def publicKey: ECPublicKey = publicKey(Secp256k1Context.isEnabled)
+  override def publicKey: ECPublicKey = publicKey(CryptoContext.default)
 
   /** Derives the public for a the private key */
-  def publicKey(useSecp: Boolean): ECPublicKey = {
-    if (useSecp) {
-      publicKeyWithSecp
-    } else {
-      publicKeyWithBouncyCastle
+  def publicKey(context: CryptoContext): ECPublicKey = {
+    context match {
+      case CryptoContext.LibSecp256k1 => publicKeyWithSecp
+      case CryptoContext.BouncyCastle => publicKeyWithBouncyCastle
     }
   }
 
@@ -109,13 +109,14 @@ object ECPrivateKey extends Factory[ECPrivateKey] {
       isCompressed: Boolean,
       ec: ExecutionContext)
       extends ECPrivateKey {
-    if (Secp256k1Context.isEnabled) {
-      require(NativeSecp256k1.secKeyVerify(bytes.toArray),
-              s"Invalid key according to secp256k1, hex: ${bytes.toHex}")
-    } else {
-      require(CryptoParams.curve.getCurve
-                .isValidFieldElement(new BigInteger(1, bytes.toArray)),
-              s"Invalid key according to Bouncy Castle, hex: ${bytes.toHex}")
+    CryptoContext.default match {
+      case CryptoContext.LibSecp256k1 =>
+        require(NativeSecp256k1.secKeyVerify(bytes.toArray),
+                s"Invalid key according to secp256k1, hex: ${bytes.toHex}")
+      case CryptoContext.BouncyCastle =>
+        require(CryptoParams.curve.getCurve
+                  .isValidFieldElement(new BigInteger(1, bytes.toArray)),
+                s"Invalid key according to Bouncy Castle, hex: ${bytes.toHex}")
     }
   }
 
@@ -182,17 +183,16 @@ sealed abstract class ECPublicKey extends BaseECKey {
     * [[org.bitcoins.crypto.ECPrivateKey ECPrivateKey]]'s corresponding
     * [[org.bitcoins.crypto.ECPublicKey ECPublicKey]]. */
   def verify(data: ByteVector, signature: ECDigitalSignature): Boolean = {
-    verify(data, signature, Secp256k1Context.isEnabled)
+    verify(data, signature, CryptoContext.default)
   }
 
   def verify(
       data: ByteVector,
       signature: ECDigitalSignature,
-      useSecp: Boolean): Boolean = {
-    if (useSecp) {
-      verifyWithSecp(data, signature)
-    } else {
-      verifyWithBouncyCastle(data, signature)
+      context: CryptoContext): Boolean = {
+    context match {
+      case CryptoContext.LibSecp256k1 => verifyWithSecp(data, signature)
+      case CryptoContext.BouncyCastle => verifyWithBouncyCastle(data, signature)
     }
   }
 
@@ -232,13 +232,12 @@ sealed abstract class ECPublicKey extends BaseECKey {
   def isFullyValid: Boolean = ECPublicKey.isFullyValid(bytes)
 
   /** Returns the decompressed version of this [[org.bitcoins.crypto.ECPublicKey ECPublicKey]] */
-  def decompressed: ECPublicKey = decompressed(Secp256k1Context.isEnabled)
+  def decompressed: ECPublicKey = decompressed(CryptoContext.default)
 
-  def decompressed(useSecp: Boolean): ECPublicKey = {
-    if (useSecp) {
-      decompressedWithSecp
-    } else {
-      decompressedWithBouncyCastle
+  def decompressed(context: CryptoContext): ECPublicKey = {
+    context match {
+      case CryptoContext.LibSecp256k1 => decompressedWithSecp
+      case CryptoContext.BouncyCastle => decompressedWithBouncyCastle
     }
   }
 
@@ -307,14 +306,13 @@ object ECPublicKey extends Factory[ECPublicKey] {
     * [[https://github.com/bitcoin/bitcoin/blob/27765b6403cece54320374b37afb01a0cfe571c3/src/pubkey.cpp#L207-L212]]
     */
   def isFullyValid(bytes: ByteVector): Boolean = {
-    isFullyValid(bytes, Secp256k1Context.isEnabled)
+    isFullyValid(bytes, CryptoContext.default)
   }
 
-  def isFullyValid(bytes: ByteVector, useSecp: Boolean): Boolean = {
-    if (useSecp) {
-      isFullyValidWithSecp(bytes)
-    } else {
-      isFullyValidWithBouncyCastle(bytes)
+  def isFullyValid(bytes: ByteVector, context: CryptoContext): Boolean = {
+    context match {
+      case CryptoContext.LibSecp256k1 => isFullyValidWithSecp(bytes)
+      case CryptoContext.BouncyCastle => isFullyValidWithBouncyCastle(bytes)
     }
   }
 

docs/secp256k1/secp256k1.md

@@ -66,7 +66,7 @@ There are two reasons you wouldn't want to use libsecp256k1
 
 There are two ways you can circumvent libsecp256k1
 
-1. Set `DISABLE_SECP256K1=true` in your environment variables. This will force `Secp256k1Context.isEnabled()` to return false
+1. Set `DISABLE_SECP256K1=true` in your environment variables. This will force `CryptoContext.default` to return false which will make Bitcoin-S act like `Secp256k1Context.isEnabled()` has returned false.
 2. Call Bouncy castle methods in `ECKey`. 
 
 Here is an example of calling bouncy castle methods in `ECKey`

secp256k1jni/src/main/java/org/bitcoin/Secp256k1Context.java

@@ -51,14 +51,7 @@ public class Secp256k1Context {
    * to Bouncy Castle implementations in the case of having no libsecp present.
    */
   public static boolean isEnabled() {
-      String secpDisabled = System.getenv("DISABLE_SECP256K1");
-      if (secpDisabled != null &&
-              (secpDisabled.toLowerCase().equals("true") ||
-                      secpDisabled.equals("1"))) {
-          return false;
-      } else {
-          return enabled;
-      }
+      return enabled;
   }
 
   public static long getContext() {