Combine feature in codec, not type

This allows us to drop the global/local fields from the codebase.
They are handled directly at the wire level.

Author: t-bast

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

@@ -307,8 +307,7 @@ class Channel(val nodeParams: NodeParams, val wallet: EclairWallet, remoteNodeId
             paymentBasepoint = open.paymentBasepoint,
             delayedPaymentBasepoint = open.delayedPaymentBasepoint,
             htlcBasepoint = open.htlcBasepoint,
-            globalFeatures = remoteInit.globalFeatures,
-            localFeatures = remoteInit.localFeatures)
+            features = remoteInit.features)
           log.debug(s"remote params: $remoteParams")
           goto(WAIT_FOR_FUNDING_CREATED) using DATA_WAIT_FOR_FUNDING_CREATED(open.temporaryChannelId, localParams, remoteParams, open.fundingSatoshis, open.pushMsat, open.feeratePerKw, open.firstPerCommitmentPoint, open.channelFlags, channelVersion, accept) sending accept
       }
@@ -340,8 +339,7 @@ class Channel(val nodeParams: NodeParams, val wallet: EclairWallet, remoteNodeId
             paymentBasepoint = accept.paymentBasepoint,
             delayedPaymentBasepoint = accept.delayedPaymentBasepoint,
             htlcBasepoint = accept.htlcBasepoint,
-            globalFeatures = remoteInit.globalFeatures,
-            localFeatures = remoteInit.localFeatures)
+            features = remoteInit.features)
           log.debug(s"remote params: $remoteParams")
           val localFundingPubkey = keyManager.fundingPublicKey(localParams.fundingKeyPath)
           val fundingPubkeyScript = Script.write(Script.pay2wsh(Scripts.multiSig2of2(localFundingPubkey.publicKey, remoteParams.fundingPubKey)))

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

@@ -212,8 +212,7 @@ final case class LocalParams(nodeId: PublicKey,
                              maxAcceptedHtlcs: Int,
                              isFunder: Boolean,
                              defaultFinalScriptPubKey: ByteVector,
-                             globalFeatures: ByteVector,
-                             localFeatures: ByteVector)
+                             features: ByteVector)
 
 final case class RemoteParams(nodeId: PublicKey,
                               dustLimit: Satoshi,
@@ -227,8 +226,7 @@ final case class RemoteParams(nodeId: PublicKey,
                               paymentBasepoint: PublicKey,
                               delayedPaymentBasepoint: PublicKey,
                               htlcBasepoint: PublicKey,
-                              globalFeatures: ByteVector,
-                              localFeatures: ByteVector)
+                              features: ByteVector)
 
 object ChannelFlags {
   val AnnounceChannel = 0x01.toByte

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

@@ -67,8 +67,8 @@ object Helpers {
    */
   def updateFeatures(data: HasCommitments, localInit: Init, remoteInit: Init): HasCommitments = {
     val commitments1 = data.commitments.copy(
-      localParams = data.commitments.localParams.copy(globalFeatures = localInit.globalFeatures, localFeatures = localInit.localFeatures),
-      remoteParams = data.commitments.remoteParams.copy(globalFeatures = remoteInit.globalFeatures, localFeatures = remoteInit.localFeatures))
+      localParams = data.commitments.localParams.copy(features = localInit.features),
+      remoteParams = data.commitments.remoteParams.copy(features = remoteInit.features))
     data match {
       case d: DATA_WAIT_FOR_FUNDING_CONFIRMED => d.copy(commitments = commitments1)
       case d: DATA_WAIT_FOR_FUNDING_LOCKED => d.copy(commitments = commitments1)
@@ -173,25 +173,25 @@ object Helpers {
   }
 
   /**
-    *
-    * @param referenceFeePerKw reference fee rate per kiloweight
-    * @param currentFeePerKw   current fee rate per kiloweight
-    * @return the "normalized" difference between i.e local and remote fee rate: |reference - current| / avg(current, reference)
-    */
+   *
+   * @param referenceFeePerKw reference fee rate per kiloweight
+   * @param currentFeePerKw   current fee rate per kiloweight
+   * @return the "normalized" difference between i.e local and remote fee rate: |reference - current| / avg(current, reference)
+   */
   def feeRateMismatch(referenceFeePerKw: Long, currentFeePerKw: Long): Double =
     Math.abs((2.0 * (referenceFeePerKw - currentFeePerKw)) / (currentFeePerKw + referenceFeePerKw))
 
   def shouldUpdateFee(commitmentFeeratePerKw: Long, networkFeeratePerKw: Long, updateFeeMinDiffRatio: Double): Boolean =
     feeRateMismatch(networkFeeratePerKw, commitmentFeeratePerKw) > updateFeeMinDiffRatio
 
   /**
-    *
-    * @param referenceFeePerKw      reference fee rate per kiloweight
-    * @param currentFeePerKw        current fee rate per kiloweight
-    * @param maxFeerateMismatchRatio maximum fee rate mismatch ratio
-    * @return true if the difference between current and reference fee rates is too high.
-    *         the actual check is |reference - current| / avg(current, reference) > mismatch ratio
-    */
+   *
+   * @param referenceFeePerKw       reference fee rate per kiloweight
+   * @param currentFeePerKw         current fee rate per kiloweight
+   * @param maxFeerateMismatchRatio maximum fee rate mismatch ratio
+   * @return true if the difference between current and reference fee rates is too high.
+   *         the actual check is |reference - current| / avg(current, reference) > mismatch ratio
+   */
   def isFeeDiffTooHigh(referenceFeePerKw: Long, currentFeePerKw: Long, maxFeerateMismatchRatio: Double): Boolean =
     feeRateMismatch(referenceFeePerKw, currentFeePerKw) > maxFeerateMismatchRatio
 
@@ -459,7 +459,7 @@ object Helpers {
       // TODO: check that
       val dustLimitSatoshis = localParams.dustLimit.max(remoteParams.dustLimit)
       val closingTx = Transactions.makeClosingTx(commitInput, localScriptPubkey, remoteScriptPubkey, localParams.isFunder, dustLimitSatoshis, closingFee, localCommit.spec)
-      val localClosingSig = keyManager.sign(closingTx,  keyManager.fundingPublicKey(commitments.localParams.fundingKeyPath))
+      val localClosingSig = keyManager.sign(closingTx, keyManager.fundingPublicKey(commitments.localParams.fundingKeyPath))
       val closingSigned = ClosingSigned(channelId, closingFee, localClosingSig)
       log.info(s"signed closing txid=${closingTx.tx.txid} with closingFeeSatoshis=${closingSigned.feeSatoshis}")
       log.debug(s"closingTxid=${closingTx.tx.txid} closingTx=${closingTx.tx}}")

eclair-core/src/main/scala/fr/acinq/eclair/io/Peer.scala

@@ -101,10 +101,10 @@ class Peer(val nodeParams: NodeParams, remoteNodeId: PublicKey, authenticator: A
       transport ! TransportHandler.Listener(self)
       context watch transport
       val localInit = nodeParams.overrideFeatures.get(remoteNodeId) match {
-        case Some(f) => wire.Init(ByteVector.empty, f)
-        case None => wire.Init(ByteVector.empty, nodeParams.features)
+        case Some(f) => wire.Init(f)
+        case None => wire.Init(nodeParams.features)
       }
-      log.info(s"using globalFeatures=${localInit.globalFeatures.toBin} and localFeatures=${localInit.localFeatures.toBin}")
+      log.info(s"using features=${localInit.features.toBin}")
       transport ! localInit
 
       val address_opt = if (outgoing) {
@@ -134,7 +134,7 @@ class Peer(val nodeParams: NodeParams, remoteNodeId: PublicKey, authenticator: A
     case Event(remoteInit: wire.Init, d: InitializingData) =>
       d.transport ! TransportHandler.ReadAck(remoteInit)
 
-      log.info(s"peer is using globalFeatures=${remoteInit.globalFeatures.toBin} and localFeatures=${remoteInit.localFeatures.toBin}")
+      log.info(s"peer is using features=${remoteInit.features.toBin}")
 
       if (Features.areSupported(remoteInit.features)) {
         d.origin_opt.foreach(origin => origin ! "connected")
@@ -680,8 +680,7 @@ object Peer {
       maxAcceptedHtlcs = nodeParams.maxAcceptedHtlcs,
       defaultFinalScriptPubKey = defaultFinalScriptPubKey,
       isFunder = isFunder,
-      globalFeatures = ByteVector.empty,
-      localFeatures = nodeParams.features)
+      features = nodeParams.features)
   }
 
   /**

eclair-core/src/main/scala/fr/acinq/eclair/wire/ChannelCodecs.scala

@@ -70,8 +70,7 @@ object ChannelCodecs extends Logging {
       ("maxAcceptedHtlcs" | uint16) ::
       ("isFunder" | bool) ::
       ("defaultFinalScriptPubKey" | varsizebinarydata) ::
-      ("globalFeatures" | varsizebinarydata) ::
-      ("localFeatures" | varsizebinarydata)).as[LocalParams]
+      ("features" | combinedFeaturesCodec)).as[LocalParams]
 
   val remoteParamsCodec: Codec[RemoteParams] = (
     ("nodeId" | publicKey) ::
@@ -86,8 +85,7 @@ object ChannelCodecs extends Logging {
       ("paymentBasepoint" | publicKey) ::
       ("delayedPaymentBasepoint" | publicKey) ::
       ("htlcBasepoint" | publicKey) ::
-      ("globalFeatures" | varsizebinarydata) ::
-      ("localFeatures" | varsizebinarydata)).as[RemoteParams]
+      ("features" | combinedFeaturesCodec)).as[RemoteParams]
 
   val directionCodec: Codec[Direction] = Codec[Direction](
     (dir: Direction) => bool.encode(dir == IN),

eclair-core/src/main/scala/fr/acinq/eclair/wire/LightningMessageCodecs.scala

@@ -20,18 +20,34 @@ import fr.acinq.eclair.wire.CommonCodecs._
 import fr.acinq.eclair.{KamonExt, wire}
 import kamon.Kamon
 import kamon.tag.TagSet
-import scodec.bits.BitVector
+import scodec.bits.{BitVector, ByteVector}
 import scodec.codecs._
 import scodec.{Attempt, Codec}
+import shapeless.HNil
 
 /**
  * Created by PM on 15/11/2016.
  */
 object LightningMessageCodecs {
 
-  val initCodec: Codec[Init] = (
-    ("globalFeatures" | varsizebinarydata) ::
-      ("localFeatures" | varsizebinarydata)).as[Init]
+  /** For historical reasons, features are divided into two feature bitmasks. We only send from the second one, but we allow receiving in both. */
+  val combinedFeaturesCodec: Codec[ByteVector] = (("globalFeatures" | varsizebinarydata) :: ("localFeatures" | varsizebinarydata)).exmap[ByteVector](
+    features => {
+      val f1 = features.head
+      val f2 = features.tail.head
+      val combinedFeatures = if (f1.length == f2.length) {
+        f1 | f2
+      } else if (f1.length > f2.length) {
+        f1 | f2.padLeft(f1.length)
+      } else {
+        f1.padLeft(f2.length) | f2
+      }
+      Attempt.Successful(combinedFeatures)
+    },
+    features => Attempt.Successful(ByteVector.empty :: features :: HNil)
+  )
+
+  val initCodec: Codec[Init] = combinedFeaturesCodec.as[Init]
 
   val errorCodec: Codec[Error] = (
     ("channelId" | bytes32) ::

eclair-core/src/main/scala/fr/acinq/eclair/wire/LightningMessageTypes.scala

@@ -45,20 +45,7 @@ sealed trait HasChainHash extends LightningMessage { def chainHash: ByteVector32
 sealed trait UpdateMessage extends HtlcMessage // <- not in the spec
 // @formatter:on
 
-/** For historical reasons, features are divided into two feature bitmasks. We only use the second one in our messages, but we allow receiving in both. */
-case class Init(globalFeatures: ByteVector, localFeatures: ByteVector) extends SetupMessage {
-  val features = combineFeatures(globalFeatures, localFeatures)
-
-  private def combineFeatures(f1: ByteVector, f2: ByteVector): ByteVector = {
-    if (f1.length == f2.length) {
-      f1 | f2
-    } else if (f1.length > f2.length) {
-      f1 | f2.padLeft(f1.length)
-    } else {
-      f1.padLeft(f2.length) | f2
-    }
-  }
-}
+case class Init(features: ByteVector) extends SetupMessage
 
 case class Error(channelId: ByteVector32, data: ByteVector) extends SetupMessage with HasChannelId {
   def toAscii: String = if (fr.acinq.eclair.isAsciiPrintable(data)) new String(data.toArray, StandardCharsets.US_ASCII) else "n/a"

eclair-core/src/test/scala/fr/acinq/eclair/channel/FuzzyPipe.scala

@@ -25,8 +25,8 @@ import scala.concurrent.duration._
 import scala.util.Random
 
 /**
-  * A Fuzzy [[fr.acinq.eclair.Pipe]] which randomly disconnects/reconnects peers.
-  */
+ * A Fuzzy [[fr.acinq.eclair.Pipe]] which randomly disconnects/reconnects peers.
+ */
 class FuzzyPipe(fuzzy: Boolean) extends Actor with Stash with ActorLogging {
 
   import scala.concurrent.ExecutionContext.Implicits.global
@@ -39,7 +39,7 @@ class FuzzyPipe(fuzzy: Boolean) extends Actor with Stash with ActorLogging {
     case _ => stash()
   }
 
-  def stayOrDisconnect(a: ActorRef, b: ActorRef, countdown: Int) = {
+  def stayOrDisconnect(a: ActorRef, b: ActorRef, countdown: Int): Unit = {
     if (!fuzzy) context become connected(a, b, countdown - 1) // fuzzy mode disabled, we never disconnect
     else if (countdown > 1) context become connected(a, b, countdown - 1)
     else {
@@ -71,7 +71,7 @@ class FuzzyPipe(fuzzy: Boolean) extends Actor with Stash with ActorLogging {
       log.debug(f"  X-${msg2String(msg)}%-6s--- B")
     case 'reconnect =>
       log.debug("RECONNECTED")
-      val dummyInit = Init(ByteVector.empty, ByteVector.empty)
+      val dummyInit = Init(ByteVector.empty)
       a ! INPUT_RECONNECTED(self, dummyInit, dummyInit)
       b ! INPUT_RECONNECTED(self, dummyInit, dummyInit)
       context become connected(a, b, Random.nextInt(40))

eclair-core/src/test/scala/fr/acinq/eclair/channel/FuzzySpec.scala

@@ -67,8 +67,8 @@ class FuzzySpec extends TestkitBaseClass with StateTestsHelperMethods with Loggi
     val alice: TestFSMRef[State, Data, Channel] = TestFSMRef(new Channel(Alice.nodeParams, wallet, Bob.nodeParams.nodeId, alice2blockchain.ref, router.ref, relayerA))
     val bob: TestFSMRef[State, Data, Channel] = TestFSMRef(new Channel(Bob.nodeParams, wallet, Alice.nodeParams.nodeId, bob2blockchain.ref, router.ref, relayerB))
     within(30 seconds) {
-      val aliceInit = Init(Alice.channelParams.globalFeatures, Alice.channelParams.localFeatures)
-      val bobInit = Init(Bob.channelParams.globalFeatures, Bob.channelParams.localFeatures)
+      val aliceInit = Init(Alice.channelParams.features)
+      val bobInit = Init(Bob.channelParams.features)
       registerA ! alice
       registerB ! bob
       // no announcements

eclair-core/src/test/scala/fr/acinq/eclair/channel/RecoverySpec.scala

@@ -34,9 +34,9 @@ class RecoverySpec extends TestkitBaseClass with StateTestsHelperMethods {
     }
   }
 
-  def aliceInit = Init(ByteVector.empty, TestConstants.Alice.nodeParams.features)
+  def aliceInit = Init(TestConstants.Alice.nodeParams.features)
 
-  def bobInit = Init(ByteVector.empty, TestConstants.Bob.nodeParams.features)
+  def bobInit = Init(TestConstants.Bob.nodeParams.features)
 
   test("use funding pubkeys from publish commitment to spend our output") { f =>
     import f._