Use human readable features in configuration (#1385)

* Parse human readable features from configuration, print features in human readable format.

eclair-core/src/main/resources/reference.conf

@@ -38,11 +38,18 @@ eclair {
   node-color = "49daaa"
 
   trampoline-payments-enable = false // TODO: @t-bast: once spec-ed this should use a global feature flag
-  features = "0a8a" // initial_routing_sync + option_data_loss_protect + option_channel_range_queries + option_channel_range_queries_ex + variable_length_onion
+  // see https://github.com/lightningnetwork/lightning-rfc/blob/master/09-features.md
+  features {
+    initial_routing_sync = optional
+    option_data_loss_protect = optional
+    gossip_queries = optional
+    gossip_queries_ex = optional
+    var_onion_optin = optional
+  }
   override-features = [ // optional per-node features
     #  {
     #    nodeid = "02aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
-    #    features = ""
+    #    features { }
     #  }
   ]
   sync-whitelist = [] // a list of public keys; if non-empty, we will only do the initial sync with those peers

eclair-core/src/main/scala/fr/acinq/eclair/Eclair.scala

@@ -42,7 +42,7 @@ import scodec.bits.ByteVector
 import scala.concurrent.duration._
 import scala.concurrent.{ExecutionContext, Future}
 
-case class GetInfoResponse(version: String, nodeId: PublicKey, alias: String, color: String, features: String, chainHash: ByteVector32, blockHeight: Int, publicAddresses: Seq[NodeAddress])
+case class GetInfoResponse(version: String, nodeId: PublicKey, alias: String, color: String, features: Features, chainHash: ByteVector32, blockHeight: Int, publicAddresses: Seq[NodeAddress])
 
 case class AuditResponse(sent: Seq[PaymentSent], received: Seq[PaymentReceived], relayed: Seq[PaymentRelayed])
 
@@ -317,7 +317,7 @@ class EclairImpl(appKit: Kit) extends Eclair {
     GetInfoResponse(
       version = Kit.getVersionLong,
       color = appKit.nodeParams.color.toString,
-      features = appKit.nodeParams.features.toHex,
+      features = appKit.nodeParams.features,
       nodeId = appKit.nodeParams.nodeId,
       alias = appKit.nodeParams.alias,
       chainHash = appKit.nodeParams.chainHash,

eclair-core/src/main/scala/fr/acinq/eclair/Features.scala

@@ -16,6 +16,9 @@
 
 package fr.acinq.eclair
 
+import com.typesafe.config.Config
+import fr.acinq.eclair.FeatureSupport.{Mandatory, Optional}
+import fr.acinq.eclair.Features.{BasicMultiPartPayment, PaymentSecret}
 import scodec.bits.{BitVector, ByteVector}
 
 /**
@@ -26,23 +29,115 @@ sealed trait FeatureSupport
 
 // @formatter:off
 object FeatureSupport {
-  case object Mandatory extends FeatureSupport
-  case object Optional extends FeatureSupport
+  case object Mandatory extends FeatureSupport { override def toString: String = "mandatory" }
+  case object Optional extends FeatureSupport { override def toString: String = "optional" }
 }
-// @formatter:on
 
 sealed trait Feature {
-  def rfcName: String
 
+  def rfcName: String
   def mandatory: Int
-
   def optional: Int = mandatory + 1
 
+  def supportBit(support: FeatureSupport): Int = support match {
+    case FeatureSupport.Mandatory => mandatory
+    case FeatureSupport.Optional => optional
+  }
+
   override def toString = rfcName
+
+}
+// @formatter:on
+
+case class ActivatedFeature(feature: Feature, support: FeatureSupport)
+
+case class UnknownFeature(bitIndex: Int)
+
+case class Features(activated: Set[ActivatedFeature], unknown: Set[UnknownFeature] = Set.empty) {
+
+  def hasFeature(feature: Feature, support: Option[FeatureSupport] = None): Boolean = support match {
+    case Some(s) => activated.contains(ActivatedFeature(feature, s))
+    case None => hasFeature(feature, Some(Optional)) || hasFeature(feature, Some(Mandatory))
+  }
+
+  def toByteVector: ByteVector = {
+    val activatedFeatureBytes = toByteVectorFromIndex(activated.map { case ActivatedFeature(f, s) => f.supportBit(s) })
+    val unknownFeatureBytes = toByteVectorFromIndex(unknown.map(_.bitIndex))
+    val maxSize = activatedFeatureBytes.size.max(unknownFeatureBytes.size)
+    activatedFeatureBytes.padLeft(maxSize) | unknownFeatureBytes.padLeft(maxSize)
+  }
+
+  private def toByteVectorFromIndex(indexes: Set[Int]): ByteVector = {
+    if (indexes.isEmpty) return ByteVector.empty
+    // When converting from BitVector to ByteVector, scodec pads right instead of left, so we make sure we pad to bytes *before* setting feature bits.
+    var buf = BitVector.fill(indexes.max + 1)(high = false).bytes.bits
+    indexes.foreach { i =>
+      buf = buf.set(i)
+    }
+    buf.reverse.bytes
+  }
+
+  /**
+    * Eclair-mobile thinks feature bit 15 (payment_secret) is gossip_queries_ex which creates issues, so we mask
+    * off basic_mpp and payment_secret. As long as they're provided in the invoice it's not an issue.
+    * We use a long enough mask to account for future features.
+    * TODO: remove that once eclair-mobile is patched.
+    */
+  def maskFeaturesForEclairMobile(): Features = {
+    Features(
+      activated = activated.filter {
+        case ActivatedFeature(PaymentSecret, _) => false
+        case ActivatedFeature(BasicMultiPartPayment, _) => false
+        case _ => true
+      },
+      unknown = unknown
+    )
+  }
+
 }
 
 object Features {
 
+  def empty = Features(Set.empty[ActivatedFeature])
+
+  def apply(features: Set[ActivatedFeature]): Features = Features(activated = features)
+
+  def apply(bytes: ByteVector): Features = apply(bytes.bits)
+
+  def apply(bits: BitVector): Features = {
+    val all = bits.toIndexedSeq.reverse.zipWithIndex.collect {
+      case (true, idx) if knownFeatures.exists(_.optional == idx) => Right(ActivatedFeature(knownFeatures.find(_.optional == idx).get, Optional))
+      case (true, idx) if knownFeatures.exists(_.mandatory == idx) => Right(ActivatedFeature(knownFeatures.find(_.mandatory == idx).get, Mandatory))
+      case (true, idx) => Left(UnknownFeature(idx))
+    }
+    Features(
+      activated = all.collect { case Right(af) => af }.toSet,
+      unknown = all.collect { case Left(inf) => inf }.toSet
+    )
+  }
+
+  /** expects to have a top level config block named "features" */
+  def fromConfiguration(config: Config): Features = Features(
+    knownFeatures.flatMap {
+      feature =>
+        getFeature(config, feature.rfcName) match {
+          case Some(support) => Some(ActivatedFeature(feature, support))
+          case _ => None
+        }
+    })
+
+  /** tries to extract the given feature name from the config, if successful returns its feature support */
+  private def getFeature(config: Config, name: String): Option[FeatureSupport] = {
+    if (!config.hasPath(s"features.$name")) None
+    else {
+      config.getString(s"features.$name") match {
+        case support if support == Mandatory.toString => Some(Mandatory)
+        case support if support == Optional.toString => Some(Optional)
+        case wrongSupport => throw new IllegalArgumentException(s"Wrong support specified ($wrongSupport)")
+      }
+    }
+  }
+
   case object OptionDataLossProtect extends Feature {
     val rfcName = "option_data_loss_protect"
     val mandatory = 0
@@ -80,7 +175,7 @@ object Features {
   }
 
   case object Wumbo extends Feature {
-    val rfcName = "large_channel_support"
+    val rfcName = "option_support_large_channel"
     val mandatory = 18
   }
 
@@ -92,6 +187,28 @@ object Features {
     val mandatory = 50
   }
 
+  val knownFeatures: Set[Feature] = Set(
+    OptionDataLossProtect,
+    InitialRoutingSync,
+    ChannelRangeQueries,
+    VariableLengthOnion,
+    ChannelRangeQueriesExtended,
+    PaymentSecret,
+    BasicMultiPartPayment,
+    Wumbo,
+    TrampolinePayment
+  )
+
+  private val supportedMandatoryFeatures: Set[Feature] = Set(
+    OptionDataLossProtect,
+    ChannelRangeQueries,
+    VariableLengthOnion,
+    ChannelRangeQueriesExtended,
+    PaymentSecret,
+    BasicMultiPartPayment,
+    Wumbo
+  )
+
   // Features may depend on other features, as specified in Bolt 9.
   private val featuresDependency = Map(
     ChannelRangeQueriesExtended -> (ChannelRangeQueries :: Nil),
@@ -104,54 +221,20 @@ object Features {
 
   case class FeatureException(message: String) extends IllegalArgumentException(message)
 
-  def validateFeatureGraph(features: BitVector): Option[FeatureException] = featuresDependency.collectFirst {
-    case (feature, dependencies) if hasFeature(features, feature) && dependencies.exists(d => !hasFeature(features, d)) =>
-      FeatureException(s"${features.toBin} sets $feature but is missing a dependency (${dependencies.filter(d => !hasFeature(features, d)).mkString(" and ")})")
-  }
-
-  def validateFeatureGraph(features: ByteVector): Option[FeatureException] = validateFeatureGraph(features.bits)
-
-  // Note that BitVector indexes from left to right whereas the specification indexes from right to left.
-  // This is why we have to reverse the bits to check if a feature is set.
-
-  private def hasFeature(features: BitVector, bit: Int): Boolean = features.sizeGreaterThan(bit) && features.reverse.get(bit)
-
-  def hasFeature(features: BitVector, feature: Feature, support: Option[FeatureSupport] = None): Boolean = support match {
-    case Some(FeatureSupport.Mandatory) => hasFeature(features, feature.mandatory)
-    case Some(FeatureSupport.Optional) => hasFeature(features, feature.optional)
-    case None => hasFeature(features, feature.optional) || hasFeature(features, feature.mandatory)
-  }
-
-  def hasFeature(features: ByteVector, feature: Feature): Boolean = hasFeature(features.bits, feature)
-
-  def hasFeature(features: ByteVector, feature: Feature, support: Option[FeatureSupport]): Boolean = hasFeature(features.bits, feature, support)
-
-  /**
-   * Check that the features that we understand are correctly specified, and that there are no mandatory features that
-   * we don't understand (even bits).
-   */
-  def areSupported(features: BitVector): Boolean = {
-    val supportedMandatoryFeatures = Set(
-      OptionDataLossProtect,
-      ChannelRangeQueries,
-      VariableLengthOnion,
-      ChannelRangeQueriesExtended,
-      PaymentSecret,
-      BasicMultiPartPayment,
-      Wumbo
-    ).map(_.mandatory.toLong)
-    val reversed = features.reverse
-    for (i <- 0L until reversed.length by 2) {
-      if (reversed.get(i) && !supportedMandatoryFeatures.contains(i)) return false
-    }
-
-    true
+  def validateFeatureGraph(features: Features): Option[FeatureException] = featuresDependency.collectFirst {
+    case (feature, dependencies) if features.hasFeature(feature) && dependencies.exists(d => !features.hasFeature(d)) =>
+      FeatureException(s"$feature is set but is missing a dependency (${dependencies.filter(d => !features.hasFeature(d)).mkString(" and ")})")
   }
 
   /**
    * A feature set is supported if all even bits are supported.
    * We just ignore unknown odd bits.
    */
-  def areSupported(features: ByteVector): Boolean = areSupported(features.bits)
+  def areSupported(features: Features): Boolean = {
+    !features.unknown.exists(_.bitIndex % 2 == 0) && features.activated.forall {
+      case ActivatedFeature(_, Optional) => true
+      case ActivatedFeature(feature, Mandatory) => supportedMandatoryFeatures.contains(feature)
+    }
+  }
 
 }

eclair-core/src/main/scala/fr/acinq/eclair/NodeParams.scala

@@ -22,7 +22,7 @@ import java.nio.file.Files
 import java.util.concurrent.TimeUnit
 import java.util.concurrent.atomic.AtomicLong
 
-import com.typesafe.config.{Config, ConfigFactory}
+import com.typesafe.config.{Config, ConfigFactory, ConfigValueType}
 import fr.acinq.bitcoin.Crypto.PublicKey
 import fr.acinq.bitcoin.{Block, ByteVector32, Satoshi}
 import fr.acinq.eclair.NodeParams.WatcherType
@@ -46,8 +46,8 @@ case class NodeParams(keyManager: KeyManager,
                       alias: String,
                       color: Color,
                       publicAddresses: List[NodeAddress],
-                      features: ByteVector,
-                      overrideFeatures: Map[PublicKey, ByteVector],
+                      features: Features,
+                      overrideFeatures: Map[PublicKey, Features],
                       syncWhitelist: Set[PublicKey],
                       dustLimit: Satoshi,
                       onChainFeeConf: OnChainFeeConf,
@@ -146,6 +146,10 @@ object NodeParams {
       case (old, new_) => require(!config.hasPath(old), s"configuration key '$old' has been replaced by '$new_'")
     }
 
+    // since v0.4.1 features cannot be a byte vector (hex string)
+    val isFeatureByteVector = config.getValue("features").valueType() == ConfigValueType.STRING
+    require(!isFeatureByteVector, "configuration key 'features' have moved from bytevector to human readable (ex: 'feature-name' = optional/mandatory)")
+
     val chain = config.getString("chain")
     val chainHash = makeChainHash(chain)
 
@@ -179,13 +183,13 @@ object NodeParams {
     val nodeAlias = config.getString("node-alias")
     require(nodeAlias.getBytes("UTF-8").length <= 32, "invalid alias, too long (max allowed 32 bytes)")
 
-    val features = ByteVector.fromValidHex(config.getString("features"))
+    val features = Features.fromConfiguration(config)
     val featuresErr = Features.validateFeatureGraph(features)
     require(featuresErr.isEmpty, featuresErr.map(_.message))
 
-    val overrideFeatures: Map[PublicKey, ByteVector] = config.getConfigList("override-features").asScala.map { e =>
+    val overrideFeatures: Map[PublicKey, Features] = config.getConfigList("override-features").asScala.map { e =>
       val p = PublicKey(ByteVector.fromValidHex(e.getString("nodeid")))
-      val f = ByteVector.fromValidHex(e.getString("features"))
+      val f = Features.fromConfiguration(e)
       p -> f
     }.toMap
 

eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelTypes.scala

@@ -24,7 +24,7 @@ import fr.acinq.bitcoin.{ByteVector32, DeterministicWallet, OutPoint, Satoshi, T
 import fr.acinq.eclair.transactions.CommitmentSpec
 import fr.acinq.eclair.transactions.Transactions.CommitTx
 import fr.acinq.eclair.wire.{AcceptChannel, ChannelAnnouncement, ChannelReestablish, ChannelUpdate, ClosingSigned, FailureMessage, FundingCreated, FundingLocked, FundingSigned, Init, OnionRoutingPacket, OpenChannel, Shutdown, UpdateAddHtlc}
-import fr.acinq.eclair.{CltvExpiry, CltvExpiryDelta, MilliSatoshi, ShortChannelId, UInt64}
+import fr.acinq.eclair.{CltvExpiry, CltvExpiryDelta, Features, MilliSatoshi, ShortChannelId, UInt64}
 import scodec.bits.{BitVector, ByteVector}
 
 /**
@@ -230,7 +230,7 @@ final case class LocalParams(nodeId: PublicKey,
                              maxAcceptedHtlcs: Int,
                              isFunder: Boolean,
                              defaultFinalScriptPubKey: ByteVector,
-                             features: ByteVector)
+                             features: Features)
 
 final case class RemoteParams(nodeId: PublicKey,
                               dustLimit: Satoshi,
@@ -244,7 +244,7 @@ final case class RemoteParams(nodeId: PublicKey,
                               paymentBasepoint: PublicKey,
                               delayedPaymentBasepoint: PublicKey,
                               htlcBasepoint: PublicKey,
-                              features: ByteVector)
+                              features: Features)
 
 object ChannelFlags {
   val AnnounceChannel = 0x01.toByte

eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala

@@ -20,7 +20,6 @@ import akka.event.{DiagnosticLoggingAdapter, LoggingAdapter}
 import fr.acinq.bitcoin.Crypto.{PrivateKey, PublicKey, ripemd160, sha256}
 import fr.acinq.bitcoin.Script._
 import fr.acinq.bitcoin._
-import fr.acinq.eclair.Features.{Wumbo, hasFeature}
 import fr.acinq.eclair.blockchain.EclairWallet
 import fr.acinq.eclair.blockchain.fee.{FeeEstimator, FeeTargets}
 import fr.acinq.eclair.channel.Channel.REFRESH_CHANNEL_UPDATE_INTERVAL
@@ -34,7 +33,6 @@ import fr.acinq.eclair.wire._
 import fr.acinq.eclair.{NodeParams, ShortChannelId, addressToPublicKeyScript, _}
 import scodec.bits.ByteVector
 
-import scala.compat.Platform
 import scala.concurrent.Await
 import scala.concurrent.duration._
 import scala.util.{Failure, Success, Try}
@@ -105,7 +103,7 @@ object Helpers {
     if (open.fundingSatoshis < nodeParams.minFundingSatoshis || open.fundingSatoshis > nodeParams.maxFundingSatoshis) throw InvalidFundingAmount(open.temporaryChannelId, open.fundingSatoshis, nodeParams.minFundingSatoshis, nodeParams.maxFundingSatoshis)
 
     // BOLT #2: Channel funding limits
-    if (open.fundingSatoshis >= Channel.MAX_FUNDING && !hasFeature(nodeParams.features, Wumbo)) throw InvalidFundingAmount(open.temporaryChannelId, open.fundingSatoshis, nodeParams.minFundingSatoshis, Channel.MAX_FUNDING)
+    if (open.fundingSatoshis >= Channel.MAX_FUNDING && !nodeParams.features.hasFeature(Features.Wumbo)) throw InvalidFundingAmount(open.temporaryChannelId, open.fundingSatoshis, nodeParams.minFundingSatoshis, Channel.MAX_FUNDING)
 
     // BOLT #2: The receiving node MUST fail the channel if: push_msat is greater than funding_satoshis * 1000.
     if (open.pushMsat > open.fundingSatoshis) throw InvalidPushAmount(open.temporaryChannelId, open.pushMsat, open.fundingSatoshis.toMilliSatoshi)
@@ -219,7 +217,7 @@ object Helpers {
   }
 
   def makeAnnouncementSignatures(nodeParams: NodeParams, commitments: Commitments, shortChannelId: ShortChannelId): AnnouncementSignatures = {
-    val features = ByteVector.empty // empty features for now
+    val features = Features.empty // empty features for now
     val fundingPubKey = nodeParams.keyManager.fundingPublicKey(commitments.localParams.fundingKeyPath)
     val (localNodeSig, localBitcoinSig) = nodeParams.keyManager.signChannelAnnouncement(fundingPubKey.path, nodeParams.chainHash, shortChannelId, commitments.remoteParams.nodeId, commitments.remoteParams.fundingPubKey, features)
     AnnouncementSignatures(commitments.channelId, shortChannelId, localNodeSig, localBitcoinSig)