Address PR comments

* ChannelType contains a set of features
* TLV doesn't expose the feature bits directly
* An unsupported channel type class is explicitly added

Author: t-bast

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

@@ -90,7 +90,7 @@ trait Eclair {
 
   def disconnect(nodeId: PublicKey)(implicit timeout: Timeout): Future[String]
 
-  def open(nodeId: PublicKey, fundingAmount: Satoshi, pushAmount_opt: Option[MilliSatoshi], channelType_opt: Option[ChannelType], fundingFeeratePerByte_opt: Option[FeeratePerByte], flags_opt: Option[Int], openTimeout_opt: Option[Timeout])(implicit timeout: Timeout): Future[ChannelOpenResponse]
+  def open(nodeId: PublicKey, fundingAmount: Satoshi, pushAmount_opt: Option[MilliSatoshi], channelType_opt: Option[SupportedChannelType], fundingFeeratePerByte_opt: Option[FeeratePerByte], flags_opt: Option[Int], openTimeout_opt: Option[Timeout])(implicit timeout: Timeout): Future[ChannelOpenResponse]
 
   def close(channels: List[ApiTypes.ChannelIdentifier], scriptPubKey_opt: Option[ByteVector])(implicit timeout: Timeout): Future[Map[ApiTypes.ChannelIdentifier, Either[Throwable, CommandResponse[CMD_CLOSE]]]]
 
@@ -177,7 +177,7 @@ class EclairImpl(appKit: Kit) extends Eclair with Logging {
     (appKit.switchboard ? Peer.Disconnect(nodeId)).mapTo[String]
   }
 
-  override def open(nodeId: PublicKey, fundingAmount: Satoshi, pushAmount_opt: Option[MilliSatoshi], channelType_opt: Option[ChannelType], fundingFeeratePerByte_opt: Option[FeeratePerByte], flags_opt: Option[Int], openTimeout_opt: Option[Timeout])(implicit timeout: Timeout): Future[ChannelOpenResponse] = {
+  override def open(nodeId: PublicKey, fundingAmount: Satoshi, pushAmount_opt: Option[MilliSatoshi], channelType_opt: Option[SupportedChannelType], fundingFeeratePerByte_opt: Option[FeeratePerByte], flags_opt: Option[Int], openTimeout_opt: Option[Timeout])(implicit timeout: Timeout): Future[ChannelOpenResponse] = {
     // we want the open timeout to expire *before* the default ask timeout, otherwise user won't get a generic response
     val openTimeout = openTimeout_opt.getOrElse(Timeout(10 seconds))
     (appKit.switchboard ? Peer.OpenChannel(

eclair-core/src/main/scala/fr/acinq/eclair/blockchain/fee/FeeEstimator.scala

@@ -19,7 +19,7 @@ package fr.acinq.eclair.blockchain.fee
 import fr.acinq.bitcoin.Crypto.PublicKey
 import fr.acinq.bitcoin.Satoshi
 import fr.acinq.eclair.blockchain.CurrentFeerates
-import fr.acinq.eclair.channel.{ChannelType, ChannelTypes}
+import fr.acinq.eclair.channel.{ChannelType, ChannelTypes, SupportedChannelType}
 
 trait FeeEstimator {
   // @formatter:off
@@ -37,7 +37,7 @@ case class FeerateTolerance(ratioLow: Double, ratioHigh: Double, anchorOutputMax
    * @param proposedFeerate fee rate proposed (new proposal through update_fee or previous proposal used in our current commit tx)
    * @return true if the difference between proposed and reference fee rates is too high.
    */
-  def isFeeDiffTooHigh(channelType: ChannelType, networkFeerate: FeeratePerKw, proposedFeerate: FeeratePerKw): Boolean = {
+  def isFeeDiffTooHigh(channelType: SupportedChannelType, networkFeerate: FeeratePerKw, proposedFeerate: FeeratePerKw): Boolean = {
     channelType match {
       case ChannelTypes.Standard =>
         proposedFeerate < networkFeerate * ratioLow || networkFeerate * ratioHigh < proposedFeerate

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

@@ -222,7 +222,10 @@ class Channel(val nodeParams: NodeParams, val wallet: EclairWallet, remoteNodeId
         htlcBasepoint = keyManager.htlcPoint(channelKeyPath).publicKey,
         firstPerCommitmentPoint = keyManager.commitmentPoint(channelKeyPath, 0),
         channelFlags = channelFlags,
-        tlvStream = TlvStream(ChannelTlv.UpfrontShutdownScript(localShutdownScript), ChannelTlv.ChannelType(channelType.features)))
+        tlvStream = TlvStream(
+          ChannelTlv.UpfrontShutdownScriptTlv(localShutdownScript),
+          ChannelTlv.ChannelTypeTlv(channelType)
+        ))
       goto(WAIT_FOR_ACCEPT_CHANNEL) using DATA_WAIT_FOR_ACCEPT_CHANNEL(initFunder, open) sending open
 
     case Event(inputFundee@INPUT_INIT_FUNDEE(_, localParams, remote, _, _, _), Nothing) if !localParams.isFunder =>
@@ -338,7 +341,7 @@ class Channel(val nodeParams: NodeParams, val wallet: EclairWallet, remoteNodeId
 
   when(WAIT_FOR_OPEN_CHANNEL)(handleExceptions {
     case Event(open: OpenChannel, d@DATA_WAIT_FOR_OPEN_CHANNEL(INPUT_INIT_FUNDEE(_, localParams, _, remoteInit, channelConfig, channelType))) =>
-      Helpers.validateParamsFundee(nodeParams, localParams.initFeatures, channelType, open, remoteNodeId, remoteInit.features) match {
+      Helpers.validateParamsFundee(nodeParams, channelType, localParams.initFeatures, open, remoteNodeId, remoteInit.features) match {
         case Left(t) => handleLocalError(t, d, Some(open))
         case Right((channelFeatures, remoteShutdownScript)) =>
           context.system.eventStream.publish(ChannelCreated(self, peer, remoteNodeId, isFunder = false, open.temporaryChannelId, open.feeratePerKw, None))
@@ -362,7 +365,10 @@ class Channel(val nodeParams: NodeParams, val wallet: EclairWallet, remoteNodeId
             delayedPaymentBasepoint = keyManager.delayedPaymentPoint(channelKeyPath).publicKey,
             htlcBasepoint = keyManager.htlcPoint(channelKeyPath).publicKey,
             firstPerCommitmentPoint = keyManager.commitmentPoint(channelKeyPath, 0),
-            tlvStream = TlvStream(ChannelTlv.UpfrontShutdownScript(localShutdownScript), ChannelTlv.ChannelType(channelType.features)))
+            tlvStream = TlvStream(
+              ChannelTlv.UpfrontShutdownScriptTlv(localShutdownScript),
+              ChannelTlv.ChannelTypeTlv(channelType)
+            ))
           val remoteParams = RemoteParams(
             nodeId = remoteNodeId,
             dustLimit = open.dustLimitSatoshis,
@@ -887,7 +893,7 @@ class Channel(val nodeParams: NodeParams, val wallet: EclairWallet, remoteNodeId
     case Event(remoteShutdown@Shutdown(_, remoteScriptPubKey, _), d: DATA_NORMAL) =>
       d.commitments.getRemoteShutdownScript(remoteScriptPubKey) match {
         case Left(e) =>
-          log.warning("they sent an invalid closing script")
+          log.warning(s"they sent an invalid closing script: ${e.getMessage}")
           context.system.scheduler.scheduleOnce(2 second, peer, Peer.Disconnect(remoteNodeId))
           stay() sending Warning(d.channelId, "invalid closing script")
         case Right(remoteShutdownScript) =>

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

@@ -87,13 +87,13 @@ case class INPUT_INIT_FUNDER(temporaryChannelId: ByteVector32,
                              remoteInit: Init,
                              channelFlags: Byte,
                              channelConfig: ChannelConfig,
-                             channelType: ChannelType)
+                             channelType: SupportedChannelType)
 case class INPUT_INIT_FUNDEE(temporaryChannelId: ByteVector32,
                              localParams: LocalParams,
                              remote: ActorRef,
                              remoteInit: Init,
                              channelConfig: ChannelConfig,
-                             channelType: ChannelType)
+                             channelType: SupportedChannelType)
 case object INPUT_CLOSE_COMPLETE_TIMEOUT // when requesting a mutual close, we wait for as much as this timeout, then unilateral close
 case object INPUT_DISCONNECTED
 case class INPUT_RECONNECTED(remote: ActorRef, localInit: Init, remoteInit: Init)

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

@@ -20,7 +20,7 @@ import fr.acinq.bitcoin.Crypto.PrivateKey
 import fr.acinq.bitcoin.{ByteVector32, Satoshi, Transaction}
 import fr.acinq.eclair.blockchain.fee.FeeratePerKw
 import fr.acinq.eclair.wire.protocol.{AnnouncementSignatures, Error, UpdateAddHtlc}
-import fr.acinq.eclair.{CltvExpiry, CltvExpiryDelta, Features, MilliSatoshi, UInt64}
+import fr.acinq.eclair.{CltvExpiry, CltvExpiryDelta, MilliSatoshi, UInt64}
 
 /**
  * Created by PM on 11/04/2017.
@@ -40,7 +40,7 @@ case class InvalidChainHash                        (override val channelId: Byte
 case class InvalidFundingAmount                    (override val channelId: ByteVector32, fundingAmount: Satoshi, min: Satoshi, max: Satoshi) extends ChannelException(channelId, s"invalid funding_satoshis=$fundingAmount (min=$min max=$max)")
 case class InvalidPushAmount                       (override val channelId: ByteVector32, pushAmount: MilliSatoshi, max: MilliSatoshi) extends ChannelException(channelId, s"invalid pushAmount=$pushAmount (max=$max)")
 case class InvalidMaxAcceptedHtlcs                 (override val channelId: ByteVector32, maxAcceptedHtlcs: Int, max: Int) extends ChannelException(channelId, s"invalid max_accepted_htlcs=$maxAcceptedHtlcs (max=$max)")
-case class InvalidChannelType                      (override val channelId: ByteVector32, ourChannelType: Features, theirChannelType: Features) extends ChannelException(channelId, s"invalid channel_type=0x${theirChannelType.toByteVector.toHex}, expected 0x${ourChannelType.toByteVector.toHex}")
+case class InvalidChannelType                      (override val channelId: ByteVector32, ourChannelType: ChannelType, theirChannelType: ChannelType) extends ChannelException(channelId, s"invalid channel_type=$theirChannelType, expected channel_type=$ourChannelType")
 case class DustLimitTooSmall                       (override val channelId: ByteVector32, dustLimit: Satoshi, min: Satoshi) extends ChannelException(channelId, s"dustLimit=$dustLimit is too small (min=$min)")
 case class DustLimitTooLarge                       (override val channelId: ByteVector32, dustLimit: Satoshi, max: Satoshi) extends ChannelException(channelId, s"dustLimit=$dustLimit is too large (max=$max)")
 case class DustLimitAboveOurChannelReserve         (override val channelId: ByteVector32, dustLimit: Satoshi, channelReserve: Satoshi) extends ChannelException(channelId, s"dustLimit=$dustLimit is above our channelReserve=$channelReserve")

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

@@ -40,7 +40,7 @@ case class ChannelFeatures(activated: Set[Feature]) {
     }
   }
 
-  val channelType: ChannelType = {
+  val channelType: SupportedChannelType = {
     if (hasFeature(AnchorOutputs)) {
       ChannelTypes.AnchorOutputs
     } else if (hasFeature(StaticRemoteKey)) {
@@ -67,52 +67,57 @@ object ChannelFeatures {
     // NB: we don't include features that can be safely activated/deactivated without impacting the channel's operation,
     // such as option_dataloss_protect or option_shutdown_anysegwit.
     val availableFeatures: Seq[Feature] = Seq(Wumbo, OptionUpfrontShutdownScript).filter(f => Features.canUseFeature(localFeatures, remoteFeatures, f))
-    val allFeatures = channelType.features.activated.keys.toSeq ++ availableFeatures
+    val allFeatures = channelType.features.toSeq ++ availableFeatures
     ChannelFeatures(allFeatures: _*)
   }
 
 }
 
 /** A channel type is a specific set of even feature bits that represent persistent channel features as defined in Bolt 2. */
 sealed trait ChannelType {
-  // @formatter:off
   /** Features representing that channel type. */
-  def features: Features
+  def features: Set[Feature]
+}
+
+sealed trait SupportedChannelType extends ChannelType {
   /** True if our main output in the remote commitment is directly sent (without any delay) to one of our wallet addresses. */
   def paysDirectlyToWallet: Boolean
-  // @formatter:on
 }
 
 object ChannelTypes {
 
   // @formatter:off
-  case object Standard extends ChannelType {
-    override def features: Features = Features.empty
+  case object Standard extends SupportedChannelType {
+    override def features: Set[Feature] = Set.empty
     override def paysDirectlyToWallet: Boolean = false
     override def toString: String = "standard"
   }
-  case object StaticRemoteKey extends ChannelType {
-    override def features: Features = Features(Features.StaticRemoteKey -> FeatureSupport.Mandatory)
+  case object StaticRemoteKey extends SupportedChannelType {
+    override def features: Set[Feature] = Set(Features.StaticRemoteKey)
     override def paysDirectlyToWallet: Boolean = true
     override def toString: String = "static_remotekey"
   }
-  case object AnchorOutputs extends ChannelType {
-    override def features: Features = Features(Features.StaticRemoteKey -> FeatureSupport.Mandatory, Features.AnchorOutputs -> FeatureSupport.Mandatory)
+  case object AnchorOutputs extends SupportedChannelType {
+    override def features: Set[Feature] = Set(Features.StaticRemoteKey, Features.AnchorOutputs)
     override def paysDirectlyToWallet: Boolean = false
     override def toString: String = "anchor_outputs"
   }
+  case class UnsupportedChannelType(featureBits: Features) extends ChannelType {
+    override def features: Set[Feature] = featureBits.activated.keySet
+    override def toString: String = s"0x${featureBits.toByteVector.toHex}"
+  }
   // @formatter:on
 
   // NB: Bolt 2: features must exactly match in order to identify a channel type.
-  def fromFeatures(features: Features): Option[ChannelType] = features match {
-    case f if f == AnchorOutputs.features => Some(AnchorOutputs)
-    case f if f == StaticRemoteKey.features => Some(StaticRemoteKey)
-    case f if f == Standard.features => Some(Standard)
-    case _ => None
+  def fromFeatures(features: Features): ChannelType = features match {
+    case f if f == Features(Features.StaticRemoteKey -> FeatureSupport.Mandatory, Features.AnchorOutputs -> FeatureSupport.Mandatory) => AnchorOutputs
+    case f if f == Features(Features.StaticRemoteKey -> FeatureSupport.Mandatory) => StaticRemoteKey
+    case f if f == Features.empty => Standard
+    case _ => UnsupportedChannelType(features)
   }
 
   /** Pick the channel type based on local and remote feature bits. */
-  def pickChannelType(localFeatures: Features, remoteFeatures: Features): ChannelType = {
+  def pickChannelType(localFeatures: Features, remoteFeatures: Features): SupportedChannelType = {
     if (Features.canUseFeature(localFeatures, remoteFeatures, Features.AnchorOutputs)) {
       AnchorOutputs
     } else if (Features.canUseFeature(localFeatures, remoteFeatures, Features.StaticRemoteKey)) {

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

@@ -199,7 +199,7 @@ case class Commitments(channelId: ByteVector32,
 
   val commitmentFormat: CommitmentFormat = channelFeatures.commitmentFormat
 
-  val channelType: ChannelType = channelFeatures.channelType
+  val channelType: SupportedChannelType = channelFeatures.channelType
 
   val localNodeId: PublicKey = localParams.nodeId
 

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

@@ -99,15 +99,15 @@ object Helpers {
   /**
    * Called by the fundee
    */
-  def validateParamsFundee(nodeParams: NodeParams, initFeatures: Features, channelType: ChannelType, open: OpenChannel, remoteNodeId: PublicKey, remoteFeatures: Features): Either[ChannelException, (ChannelFeatures, Option[ByteVector])] = {
+  def validateParamsFundee(nodeParams: NodeParams, channelType: SupportedChannelType, localFeatures: Features, open: OpenChannel, remoteNodeId: PublicKey, remoteFeatures: Features): Either[ChannelException, (ChannelFeatures, Option[ByteVector])] = {
     // BOLT #2: if the chain_hash value, within the open_channel, message is set to a hash of a chain that is unknown to the receiver:
     // MUST reject the channel.
     if (nodeParams.chainHash != open.chainHash) return Left(InvalidChainHash(open.temporaryChannelId, local = nodeParams.chainHash, remote = open.chainHash))
 
     if (open.fundingSatoshis < nodeParams.minFundingSatoshis || open.fundingSatoshis > nodeParams.maxFundingSatoshis) return Left(InvalidFundingAmount(open.temporaryChannelId, open.fundingSatoshis, nodeParams.minFundingSatoshis, nodeParams.maxFundingSatoshis))
 
     // BOLT #2: Channel funding limits
-    if (open.fundingSatoshis >= Channel.MAX_FUNDING && !initFeatures.hasFeature(Features.Wumbo)) return Left(InvalidFundingAmount(open.temporaryChannelId, open.fundingSatoshis, nodeParams.minFundingSatoshis, Channel.MAX_FUNDING))
+    if (open.fundingSatoshis >= Channel.MAX_FUNDING && !localFeatures.hasFeature(Features.Wumbo)) return Left(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) return Left(InvalidPushAmount(open.temporaryChannelId, open.pushMsat, open.fundingSatoshis.toMilliSatoshi))
@@ -147,22 +147,22 @@ object Helpers {
     val reserveToFundingRatio = open.channelReserveSatoshis.toLong.toDouble / Math.max(open.fundingSatoshis.toLong, 1)
     if (reserveToFundingRatio > nodeParams.maxReserveToFundingRatio) return Left(ChannelReserveTooHigh(open.temporaryChannelId, open.channelReserveSatoshis, reserveToFundingRatio, nodeParams.maxReserveToFundingRatio))
 
-    val channelFeatures = ChannelFeatures(channelType, initFeatures, remoteFeatures)
-    extractShutdownScript(open.temporaryChannelId, initFeatures, remoteFeatures, open.upfrontShutdownScript_opt).map(script_opt => (channelFeatures, script_opt))
+    val channelFeatures = ChannelFeatures(channelType, localFeatures, remoteFeatures)
+    extractShutdownScript(open.temporaryChannelId, localFeatures, remoteFeatures, open.upfrontShutdownScript_opt).map(script_opt => (channelFeatures, script_opt))
   }
 
   /**
    * Called by the funder
    */
-  def validateParamsFunder(nodeParams: NodeParams, channelType: ChannelType, initFeatures: Features, remoteFeatures: Features, open: OpenChannel, accept: AcceptChannel): Either[ChannelException, (ChannelFeatures, Option[ByteVector])] = {
+  def validateParamsFunder(nodeParams: NodeParams, channelType: SupportedChannelType, localFeatures: Features, remoteFeatures: Features, open: OpenChannel, accept: AcceptChannel): Either[ChannelException, (ChannelFeatures, Option[ByteVector])] = {
     accept.channelType_opt match {
-      case None if channelType != ChannelTypes.pickChannelType(initFeatures, remoteFeatures) =>
+      case None if channelType != ChannelTypes.pickChannelType(localFeatures, remoteFeatures) =>
         // If we have overridden the default channel type, but they didn't support explicit channel type negotiation,
         // we need to abort because they expect a different channel type than what we offered.
-        return Left(InvalidChannelType(open.temporaryChannelId, channelType.features, ChannelTypes.pickChannelType(initFeatures, remoteFeatures).features))
+        return Left(InvalidChannelType(open.temporaryChannelId, channelType, ChannelTypes.pickChannelType(localFeatures, remoteFeatures)))
       case Some(theirChannelType) if accept.channelType_opt != open.channelType_opt =>
         // if channel_type is set, and channel_type was set in open_channel, and they are not equal types: MUST reject the channel.
-        return Left(InvalidChannelType(open.temporaryChannelId, channelType.features, theirChannelType))
+        return Left(InvalidChannelType(open.temporaryChannelId, channelType, theirChannelType))
       case _ => // we agree on channel type
     }
 
@@ -192,8 +192,8 @@ object Helpers {
     val reserveToFundingRatio = accept.channelReserveSatoshis.toLong.toDouble / Math.max(open.fundingSatoshis.toLong, 1)
     if (reserveToFundingRatio > nodeParams.maxReserveToFundingRatio) return Left(ChannelReserveTooHigh(open.temporaryChannelId, accept.channelReserveSatoshis, reserveToFundingRatio, nodeParams.maxReserveToFundingRatio))
 
-    val channelFeatures = ChannelFeatures(channelType, initFeatures, remoteFeatures)
-    extractShutdownScript(accept.temporaryChannelId, initFeatures, remoteFeatures, accept.upfrontShutdownScript_opt).map(script_opt => (channelFeatures, script_opt))
+    val channelFeatures = ChannelFeatures(channelType, localFeatures, remoteFeatures)
+    extractShutdownScript(accept.temporaryChannelId, localFeatures, remoteFeatures, accept.upfrontShutdownScript_opt).map(script_opt => (channelFeatures, script_opt))
   }
 
   /**