Address PR feedback

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

@@ -70,7 +70,7 @@ object Features {
   }
 
   case object StaticRemoteKey extends Feature {
-    val rfcName = "staticremotekey"
+    val rfcName = "option_static_remotekey"
     val mandatory = 12
   }
 
@@ -132,8 +132,8 @@ object Features {
   def hasFeature(features: ByteVector, feature: Feature, support: Option[FeatureSupport]): Boolean = hasFeature(features.bits, feature, support)
 
   /** returns true if both have at least optional support */
-  def canUseFeature(localFeatures: ByteVector, remoteFeatures: ByteVector, feature: Feature, support: Option[FeatureSupport] = None): Boolean = {
-    hasFeature(localFeatures, feature, support) && hasFeature(remoteFeatures, feature, support)
+  def canUseFeature(localFeatures: ByteVector, remoteFeatures: ByteVector, feature: Feature): Boolean = {
+    hasFeature(localFeatures, feature) && hasFeature(remoteFeatures, feature)
   }
 
   /**

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

@@ -2092,7 +2092,7 @@ class Channel(val nodeParams: NodeParams, val wallet: EclairWallet, remoteNodeId
       case v if v.isSet(USE_STATIC_REMOTEKEY_BIT) =>
         val remoteCommitPublished = RemoteCommitPublished(commitTx, None, List.empty, List.empty, Map.empty)
         val nextData = DATA_CLOSING(d.commitments, fundingTx = None, waitingSince = now, Nil, futureRemoteCommitPublished = Some(remoteCommitPublished))
-        goto(CLOSING) using nextData storing()
+        goto(CLOSING) using nextData storing() // we don't need to claim our main output in the remote commit because it already spends to our wallet address
       case _ =>
         val remotePerCommitmentPoint = d.remoteChannelReestablish.myCurrentPerCommitmentPoint
         val remoteCommitPublished = Helpers.Closing.claimRemoteCommitMainOutput(keyManager, d.commitments, remotePerCommitmentPoint, commitTx, nodeParams.onChainFeeConf.feeEstimator, nodeParams.onChainFeeConf.feeTargets)

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

@@ -629,8 +629,8 @@ object Commitments {
       case _ => keyManager.paymentPoint(channelKeyPath).publicKey
     }
     val localPaymentPubkey = channelVersion match {
-      case v if v.isSet(USE_STATIC_REMOTEKEY_BIT) => localParams.localPaymentBasepoint.get
-      case _ => Generators.derivePubKey(keyManager.paymentPoint(channelKeyPath).publicKey, remotePerCommitmentPoint)
+      case v if v.isSet(USE_STATIC_REMOTEKEY_BIT) => localPaymentBasepoint
+      case _ => Generators.derivePubKey(localPaymentBasepoint, remotePerCommitmentPoint)
     }
     val localHtlcPubkey = Generators.derivePubKey(keyManager.htlcPoint(channelKeyPath).publicKey, remotePerCommitmentPoint)
     val remoteDelayedPaymentPubkey = Generators.derivePubKey(remoteParams.delayedPaymentBasepoint, remotePerCommitmentPoint)

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

@@ -606,7 +606,7 @@ object Helpers {
     /**
      *
      * Claim all the HTLCs that we've received from their current commit tx, if the channel used option_static_remotekey
-     * we don't claim our main output.
+     * we don't need to claim our main output because it directly pays to one of our wallet's p2wpkh addresses.
      *
      * @param commitments  our commitment data, which include payment preimages
      * @param remoteCommit the remote commitment data to use to claim outputs (it can be their current or next commitment)

eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala

@@ -22,7 +22,7 @@ import akka.actor.Status
 import akka.actor.Status.Failure
 import akka.testkit.TestProbe
 import fr.acinq.bitcoin.Crypto.PrivateKey
-import fr.acinq.bitcoin.{Bech32, ByteVector32, ByteVector64, Crypto, Script, ScriptFlags, Transaction}
+import fr.acinq.bitcoin.{ByteVector32, ByteVector64, Crypto, ScriptFlags, Transaction}
 import fr.acinq.eclair.TestConstants.{Alice, Bob}
 import fr.acinq.eclair.UInt64.Conversions._
 import fr.acinq.eclair.blockchain._
@@ -43,7 +43,6 @@ import fr.acinq.eclair.{TestConstants, TestkitBaseClass, randomBytes32, _}
 import org.scalatest.{Outcome, Tag}
 import scodec.bits._
 
-import scala.concurrent.Future
 import scala.concurrent.duration._
 
 /**
@@ -57,19 +56,7 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods {
   implicit val log: akka.event.LoggingAdapter = akka.event.NoLogging
 
   override def withFixture(test: OneArgTest): Outcome = {
-    val testWallet = if(test.tags.contains("static_remotekey")){
-      val randomKey = PrivateKey(randomBytes32).publicKey
-      new TestWallet{
-        override def getReceivePubkey(receiveAddress: Option[String] = None): Future[Crypto.PublicKey] = Future.successful(randomKey)
-        override def getReceiveAddress: Future[String] = Future.successful({
-          val scriptPubKey = Script.write(Script.pay2wpkh(randomKey))
-          Bech32.encodeWitnessAddress("bcrt", 0, scriptPubKey)
-        })
-      }
-    } else {
-      new TestWallet
-    }
-    val setup = init(wallet = testWallet)
+    val setup = init()
     import setup._
     within(30 seconds) {
       reachNormal(setup, test.tags)
@@ -1119,7 +1106,7 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods {
     import f._
     val sender = TestProbe()
 
-      assert(alice.stateData.asInstanceOf[DATA_NORMAL].commitments.localParams.features == hex"2000")
+    assert(alice.stateData.asInstanceOf[DATA_NORMAL].commitments.localParams.features == hex"2000")
     assert(bob.stateData.asInstanceOf[DATA_NORMAL].commitments.localParams.features == hex"2000")
 
     def aliceToRemoteScript() = {
@@ -1169,8 +1156,9 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods {
     peer.expectMsg(Peer.Disconnect(alice.stateData.asInstanceOf[DATA_NORMAL].commitments.remoteParams.nodeId))
   }
 
-  test("recv CMD_FULFILL_HTLC") { f =>
+  private def testReceiveCmdFulfillHtlc(f: FixtureParam): Unit = {
     import f._
+
     val sender = TestProbe()
     val (r, htlc) = addHtlc(50000000 msat, alice, bob, alice2bob, bob2alice)
     crossSign(alice, bob, alice2bob, bob2alice)
@@ -1185,6 +1173,10 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods {
         localChanges = initialState.commitments.localChanges.copy(initialState.commitments.localChanges.proposed :+ fulfill))))
   }
 
+  test("recv CMD_FULFILL_HTLC") { testReceiveCmdFulfillHtlc _ }
+
+  test("recv CMD_FULFILL_HTLC (static_remotekey)", Tag("static_remotekey")) { testReceiveCmdFulfillHtlc _ }
+
   test("recv CMD_FULFILL_HTLC (unknown htlc id)") { f =>
     import f._
     val sender = TestProbe()
@@ -1219,7 +1211,7 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods {
     relayerB.expectMsg(CommandBuffer.CommandAck(initialState.channelId, 42))
   }
 
-  test("recv UpdateFulfillHtlc") { f =>
+  private def testUpdateFulfillHtlc(f: FixtureParam) = {
     import f._
     val sender = TestProbe()
     val (r, htlc) = addHtlc(50000000 msat, alice, bob, alice2bob, bob2alice)
@@ -1239,6 +1231,10 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods {
     assert(forward.htlc === htlc)
   }
 
+  test("recv UpdateFulfillHtlc") { testUpdateFulfillHtlc _ }
+
+  test("recv UpdateFulfillHtlc (static_remotekey)", Tag("(static_remotekey)")) { testUpdateFulfillHtlc _ }
+
   test("recv UpdateFulfillHtlc (sender has not signed htlc)") { f =>
     import f._
     val sender = TestProbe()
@@ -1294,7 +1290,7 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods {
     alice2blockchain.expectMsgType[WatchConfirmed]
   }
 
-  test("recv CMD_FAIL_HTLC") { f =>
+  private def testCmdFailHtlc(f: FixtureParam) = {
     import f._
     val sender = TestProbe()
     val (r, htlc) = addHtlc(50000000 msat, alice, bob, alice2bob, bob2alice)
@@ -1308,8 +1304,13 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods {
     awaitCond(bob.stateData == initialState.copy(
       commitments = initialState.commitments.copy(
         localChanges = initialState.commitments.localChanges.copy(initialState.commitments.localChanges.proposed :+ fail))))
+
   }
 
+  test("recv CMD_FAIL_HTLC") { testCmdFailHtlc _ }
+
+  test("recv CMD_FAIL_HTLC (static_remotekey)", Tag("static_remotekey")) { testCmdFailHtlc _ }
+
   test("recv CMD_FAIL_HTLC (unknown htlc id)") { f =>
     import f._
     val sender = TestProbe()
@@ -1377,7 +1378,7 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods {
     relayerB.expectMsg(CommandBuffer.CommandAck(initialState.channelId, 42))
   }
 
-  test("recv UpdateFailHtlc") { f =>
+  private def testUpdateFailHtlc(f: FixtureParam) = {
     import f._
     val sender = TestProbe()
     val (_, htlc) = addHtlc(50000000 msat, alice, bob, alice2bob, bob2alice)
@@ -1395,6 +1396,9 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods {
     relayerA.expectNoMsg()
   }
 
+  test("recv UpdateFailHtlc") { testUpdateFailHtlc _ }
+  test("recv UpdateFailHtlc (static_remotekey)", Tag("static_remotekey")) { testUpdateFailHtlc _ }
+
   test("recv UpdateFailMalformedHtlc") { f =>
     import f._
     val sender = TestProbe()