Validate channel parameters

We apply slightly different validation for zero-fee commitments:

- the commit feerate must be `0 sat/byte`
- the max number of accepted HTLCs must be at most 114
- `update_fee` cannot be used

We verify those requirements during channel creation and add tests
for normal channel operation.

Author: t-bast

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

@@ -21,9 +21,9 @@ import fr.acinq.bitcoin.scalacompat.Crypto.PublicKey
 import fr.acinq.bitcoin.scalacompat.{Block, BlockHash, Crypto, Satoshi, SatoshiLong}
 import fr.acinq.eclair.Setup.Seeds
 import fr.acinq.eclair.blockchain.fee._
-import fr.acinq.eclair.channel.ChannelFlags
 import fr.acinq.eclair.channel.fsm.Channel
 import fr.acinq.eclair.channel.fsm.Channel.{BalanceThreshold, ChannelConf, UnhandledExceptionStrategy}
+import fr.acinq.eclair.channel.{ChannelFlags, ChannelTypes}
 import fr.acinq.eclair.crypto.Noise.KeyPair
 import fr.acinq.eclair.crypto.keymanager.{ChannelKeyManager, NodeKeyManager, OnChainKeyManager}
 import fr.acinq.eclair.db._
@@ -361,7 +361,7 @@ object NodeParams extends Logging {
     require(htlcMinimum > 0.msat, "channel.htlc-minimum-msat must be strictly greater than 0")
 
     val maxAcceptedHtlcs = config.getInt("channel.max-accepted-htlcs")
-    require(maxAcceptedHtlcs <= Channel.MAX_ACCEPTED_HTLCS, s"channel.max-accepted-htlcs must be lower than ${Channel.MAX_ACCEPTED_HTLCS}")
+    require(maxAcceptedHtlcs <= Channel.maxAcceptedHtlcs(ChannelTypes.AnchorOutputsZeroFeeHtlcTx()), s"channel.max-accepted-htlcs must be lower than ${Channel.maxAcceptedHtlcs(ChannelTypes.AnchorOutputsZeroFeeHtlcTx())}")
 
     val maxToLocalCLTV = CltvExpiryDelta(config.getInt("channel.max-to-local-delay-blocks"))
     val offeredCLTV = CltvExpiryDelta(config.getInt("channel.to-remote-delay-blocks"))

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

@@ -114,12 +114,6 @@ object Helpers {
     // BOLT #2: The receiving node MUST fail the channel if: to_self_delay is unreasonably large.
     if (open.toSelfDelay > nodeParams.channelConf.maxToLocalDelay) return Left(ToSelfDelayTooHigh(open.temporaryChannelId, open.toSelfDelay, nodeParams.channelConf.maxToLocalDelay))
 
-    // BOLT #2: The receiving node MUST fail the channel if: max_accepted_htlcs is greater than 483.
-    if (open.maxAcceptedHtlcs > Channel.MAX_ACCEPTED_HTLCS) return Left(InvalidMaxAcceptedHtlcs(open.temporaryChannelId, open.maxAcceptedHtlcs, Channel.MAX_ACCEPTED_HTLCS))
-
-    // BOLT #2: The receiving node MUST fail the channel if: it considers feerate_per_kw too small for timely processing.
-    if (isFeeTooSmall(open.feeratePerKw)) return Left(FeerateTooSmall(open.temporaryChannelId, open.feeratePerKw))
-
     if (open.dustLimitSatoshis > nodeParams.channelConf.maxRemoteDustLimit) return Left(DustLimitTooLarge(open.temporaryChannelId, open.dustLimitSatoshis, nodeParams.channelConf.maxRemoteDustLimit))
 
     // BOLT #2: The receiving node MUST fail the channel if: dust_limit_satoshis is greater than channel_reserve_satoshis.
@@ -143,6 +137,12 @@ object Helpers {
       case _: SegwitV0CommitmentFormat => ()
     }
 
+    // BOLT #2: The receiving node MUST fail the channel if: it considers feerate_per_kw too small for timely processing.
+    if (isFeeTooSmall(open.feeratePerKw, channelType)) return Left(FeerateTooSmall(open.temporaryChannelId, open.feeratePerKw))
+
+    // BOLT #2: The receiving node MUST fail the channel if max_accepted_htlcs is too high.
+    if (open.maxAcceptedHtlcs > Channel.maxAcceptedHtlcs(channelType)) return Left(InvalidMaxAcceptedHtlcs(open.temporaryChannelId, open.maxAcceptedHtlcs, Channel.maxAcceptedHtlcs(channelType)))
+
     // BOLT #2: The receiving node MUST fail the channel if: it considers feerate_per_kw too small for timely processing or unreasonably large.
     val localFeeratePerKw = nodeParams.onChainFeeConf.getCommitmentFeerate(nodeParams.currentBitcoinCoreFeerates, remoteNodeId, channelType.commitmentFormat)
     if (nodeParams.onChainFeeConf.feerateToleranceFor(remoteNodeId).isProposedCommitFeerateTooHigh(localFeeratePerKw, open.feeratePerKw)) return Left(FeerateTooDifferent(open.temporaryChannelId, localFeeratePerKw, open.feeratePerKw))
@@ -178,12 +178,6 @@ object Helpers {
     // BOLT #2: The receiving node MUST fail the channel if: to_self_delay is unreasonably large.
     if (open.toSelfDelay > nodeParams.channelConf.maxToLocalDelay) return Left(ToSelfDelayTooHigh(open.temporaryChannelId, open.toSelfDelay, nodeParams.channelConf.maxToLocalDelay))
 
-    // BOLT #2: The receiving node MUST fail the channel if: max_accepted_htlcs is greater than 483.
-    if (open.maxAcceptedHtlcs > Channel.MAX_ACCEPTED_HTLCS) return Left(InvalidMaxAcceptedHtlcs(open.temporaryChannelId, open.maxAcceptedHtlcs, Channel.MAX_ACCEPTED_HTLCS))
-
-    // BOLT #2: The receiving node MUST fail the channel if: it considers feerate_per_kw too small for timely processing.
-    if (isFeeTooSmall(open.commitmentFeerate)) return Left(FeerateTooSmall(open.temporaryChannelId, open.commitmentFeerate))
-
     if (open.dustLimit < Channel.MIN_DUST_LIMIT) return Left(DustLimitTooSmall(open.temporaryChannelId, open.dustLimit, Channel.MIN_DUST_LIMIT))
     if (open.dustLimit > nodeParams.channelConf.maxRemoteDustLimit) return Left(DustLimitTooLarge(open.temporaryChannelId, open.dustLimit, nodeParams.channelConf.maxRemoteDustLimit))
 
@@ -193,6 +187,12 @@ object Helpers {
     }
     val channelFeatures = ChannelFeatures(channelType, localFeatures, remoteFeatures, open.channelFlags.announceChannel)
 
+    // BOLT #2: The receiving node MUST fail the channel if: it considers feerate_per_kw too small for timely processing.
+    if (isFeeTooSmall(open.commitmentFeerate, channelType)) return Left(FeerateTooSmall(open.temporaryChannelId, open.commitmentFeerate))
+
+    // BOLT #2: The receiving node MUST fail the channel if max_accepted_htlcs is too high.
+    if (open.maxAcceptedHtlcs > Channel.maxAcceptedHtlcs(channelType)) return Left(InvalidMaxAcceptedHtlcs(open.temporaryChannelId, open.maxAcceptedHtlcs, Channel.maxAcceptedHtlcs(channelType)))
+
     // BOLT #2: The receiving node MUST fail the channel if: it considers feerate_per_kw too small for timely processing or unreasonably large.
     val localFeeratePerKw = nodeParams.onChainFeeConf.getCommitmentFeerate(nodeParams.currentBitcoinCoreFeerates, remoteNodeId, channelType.commitmentFormat)
     if (nodeParams.onChainFeeConf.feerateToleranceFor(remoteNodeId).isProposedCommitFeerateTooHigh(localFeeratePerKw, open.commitmentFeerate)) return Left(FeerateTooDifferent(open.temporaryChannelId, localFeeratePerKw, open.commitmentFeerate))
@@ -218,7 +218,7 @@ object Helpers {
       case Right(channelType) => channelType
     }
 
-    if (accept.maxAcceptedHtlcs > Channel.MAX_ACCEPTED_HTLCS) return Left(InvalidMaxAcceptedHtlcs(accept.temporaryChannelId, accept.maxAcceptedHtlcs, Channel.MAX_ACCEPTED_HTLCS))
+    if (accept.maxAcceptedHtlcs > Channel.maxAcceptedHtlcs(channelType)) return Left(InvalidMaxAcceptedHtlcs(accept.temporaryChannelId, accept.maxAcceptedHtlcs, Channel.maxAcceptedHtlcs(channelType)))
 
     if (accept.dustLimitSatoshis > nodeParams.channelConf.maxRemoteDustLimit) return Left(DustLimitTooLarge(open.temporaryChannelId, accept.dustLimitSatoshis, nodeParams.channelConf.maxRemoteDustLimit))
     if (accept.dustLimitSatoshis < Channel.MIN_DUST_LIMIT) return Left(DustLimitTooSmall(accept.temporaryChannelId, accept.dustLimitSatoshis, Channel.MIN_DUST_LIMIT))
@@ -268,7 +268,7 @@ object Helpers {
     // BOLT #2: The receiving node MUST fail the channel if: push_msat is greater than funding_satoshis * 1000.
     if (accept.pushAmount > accept.fundingAmount) return Left(InvalidPushAmount(accept.temporaryChannelId, accept.pushAmount, accept.fundingAmount.toMilliSatoshi))
 
-    if (accept.maxAcceptedHtlcs > Channel.MAX_ACCEPTED_HTLCS) return Left(InvalidMaxAcceptedHtlcs(accept.temporaryChannelId, accept.maxAcceptedHtlcs, Channel.MAX_ACCEPTED_HTLCS))
+    if (accept.maxAcceptedHtlcs > Channel.maxAcceptedHtlcs(channelType)) return Left(InvalidMaxAcceptedHtlcs(accept.temporaryChannelId, accept.maxAcceptedHtlcs, Channel.maxAcceptedHtlcs(channelType)))
 
     if (accept.dustLimit < Channel.MIN_DUST_LIMIT) return Left(DustLimitTooSmall(accept.temporaryChannelId, accept.dustLimit, Channel.MIN_DUST_LIMIT))
     if (accept.dustLimit > nodeParams.channelConf.maxRemoteDustLimit) return Left(DustLimitTooLarge(open.temporaryChannelId, accept.dustLimit, nodeParams.channelConf.maxRemoteDustLimit))
@@ -291,8 +291,12 @@ object Helpers {
    * @param remoteFeeratePerKw remote fee rate per kiloweight
    * @return true if the remote fee rate is too small
    */
-  private def isFeeTooSmall(remoteFeeratePerKw: FeeratePerKw): Boolean = {
-    remoteFeeratePerKw < FeeratePerKw.MinimumFeeratePerKw
+  private def isFeeTooSmall(remoteFeeratePerKw: FeeratePerKw, channelType: SupportedChannelType): Boolean = {
+    channelType match {
+      case _: ChannelTypes.AnchorOutputs | _: ChannelTypes.AnchorOutputsZeroFeeHtlcTx => remoteFeeratePerKw < FeeratePerKw.MinimumFeeratePerKw
+      case _: ChannelTypes.SimpleTaprootChannelsStaging | ChannelTypes.SimpleTaprootChannelsPhoenix => remoteFeeratePerKw < FeeratePerKw.MinimumFeeratePerKw
+      case _: ChannelTypes.ZeroFeeCommitments => false
+    }
   }
 
   /** Compute the temporaryChannelId of a dual-funded channel. */

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

@@ -127,11 +127,11 @@ object Channel {
       }
     }
 
-    def commitParams(fundingAmount: Satoshi, unlimitedMaxHtlcValueInFlight: Boolean): ProposedCommitParams = ProposedCommitParams(
+    def commitParams(fundingAmount: Satoshi, channelType: SupportedChannelType, unlimitedMaxHtlcValueInFlight: Boolean): ProposedCommitParams = ProposedCommitParams(
       localDustLimit = dustLimit,
       localHtlcMinimum = htlcMinimum,
       localMaxHtlcValueInFlight = maxHtlcValueInFlight(fundingAmount, unlimitedMaxHtlcValueInFlight),
-      localMaxAcceptedHtlcs = maxAcceptedHtlcs,
+      localMaxAcceptedHtlcs = maxAcceptedHtlcs.min(Channel.maxAcceptedHtlcs(channelType)),
       toRemoteDelay = toRemoteDelay,
     )
   }
@@ -151,7 +151,14 @@ object Channel {
 
   // https://github.com/lightningnetwork/lightning-rfc/blob/master/02-peer-protocol.md#requirements
   val MAX_FUNDING_WITHOUT_WUMBO: Satoshi = 16777216 sat // = 2^24
-  val MAX_ACCEPTED_HTLCS = 483
+
+  /** We limit the number of HTLCs that can be pending to ensure that the commit tx doesn't exceed standard weight limits. */
+  def maxAcceptedHtlcs(channelType: SupportedChannelType): Int = channelType match {
+    case _: ChannelTypes.AnchorOutputs | _: ChannelTypes.AnchorOutputsZeroFeeHtlcTx => 483
+    case _: ChannelTypes.SimpleTaprootChannelsStaging | ChannelTypes.SimpleTaprootChannelsPhoenix => 483
+    // When using v3 transactions, the maximum package size is more restrictive than v2 transactions.
+    case _: ChannelTypes.ZeroFeeCommitments => 114
+  }
 
   // We may need to rely on our peer's commit tx in certain cases (backup/restore) so we must ensure their transactions
   // can propagate through the bitcoin network (assuming bitcoin core nodes with default policies).

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

@@ -229,7 +229,7 @@ class Peer(val nodeParams: NodeParams,
           requireConfirmedInputs = requireConfirmedInputs,
           requestFunding_opt = c.requestFunding_opt,
           localChannelParams = localParams,
-          proposedCommitParams = nodeParams.channelConf.commitParams(c.fundingAmount, unlimitedMaxHtlcValueInFlight = false),
+          proposedCommitParams = nodeParams.channelConf.commitParams(c.fundingAmount, channelType, unlimitedMaxHtlcValueInFlight = false),
           remote = d.peerConnection,
           remoteInit = d.remoteInit,
           channelFlags = c.channelFlags_opt.getOrElse(nodeParams.channelConf.channelFlags),
@@ -293,7 +293,7 @@ class Peer(val nodeParams: NodeParams,
                     pushAmount_opt = None,
                     requireConfirmedInputs = false,
                     localChannelParams = localParams,
-                    proposedCommitParams = nodeParams.channelConf.commitParams(open.fundingSatoshis, unlimitedMaxHtlcValueInFlight = false),
+                    proposedCommitParams = nodeParams.channelConf.commitParams(open.fundingSatoshis, channelType, unlimitedMaxHtlcValueInFlight = false),
                     remote = d.peerConnection,
                     remoteInit = d.remoteInit,
                     channelConfig = channelConfig,
@@ -308,7 +308,7 @@ class Peer(val nodeParams: NodeParams,
                     pushAmount_opt = None,
                     requireConfirmedInputs = nodeParams.channelConf.requireConfirmedInputsForDualFunding,
                     localChannelParams = localParams,
-                    proposedCommitParams = nodeParams.channelConf.commitParams(open.fundingAmount + addFunding_opt.map(_.fundingAmount).getOrElse(0 sat), unlimitedMaxHtlcValueInFlight = false),
+                    proposedCommitParams = nodeParams.channelConf.commitParams(open.fundingAmount + addFunding_opt.map(_.fundingAmount).getOrElse(0 sat), channelType, unlimitedMaxHtlcValueInFlight = false),
                     remote = d.peerConnection,
                     remoteInit = d.remoteInit,
                     channelConfig = channelConfig,

eclair-core/src/main/scala/fr/acinq/eclair/wire/internal/channel/version5/ChannelCodecs5.scala

@@ -82,6 +82,7 @@ private[channel] object ChannelCodecs5 {
       .typecase(0x02, provide(Transactions.ZeroFeeHtlcTxAnchorOutputsCommitmentFormat))
       .typecase(0x03, provide(Transactions.PhoenixSimpleTaprootChannelCommitmentFormat))
       .typecase(0x04, provide(Transactions.ZeroFeeHtlcTxSimpleTaprootChannelCommitmentFormat))
+      .typecase(0x05, provide(Transactions.ZeroFeeCommitmentFormat))
       // 0x00 was used for pre-anchor channels, which have been deprecated after eclair v0.13.1.
       .typecase(0x00, fail[Transactions.CommitmentFormat](Err("some of your channels are not using anchor outputs: you must restart with your previous eclair version and close those channels before updating to this version of eclair (see the release notes for more details)")))
 

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

@@ -98,6 +98,8 @@ object ChannelStateTestsTags {
   val OptionSimpleTaprootPhoenix = "option_simple_taproot_phoenix"
   /** If set, channels will use taproot. */
   val OptionSimpleTaproot = "option_simple_taproot"
+  /** If set, channel will use zero-fee commitments. */
+  val ZeroFeeCommitments = "zero_fee_commitments"
 }
 
 trait ChannelStateTestsBase extends Assertions with Eventually {
@@ -142,7 +144,8 @@ trait ChannelStateTestsBase extends Assertions with Eventually {
         dualFunded = dualFunded,
         commitTxFeerate = channelType match {
           case _: ChannelTypes.AnchorOutputs | ChannelTypes.SimpleTaprootChannelsPhoenix => TestConstants.phoenixCommitFeeratePerKw
-          case _ => TestConstants.anchorOutputsFeeratePerKw
+          case _: ChannelTypes.AnchorOutputsZeroFeeHtlcTx | _: ChannelTypes.SimpleTaprootChannelsStaging => TestConstants.anchorOutputsFeeratePerKw
+          case _: ChannelTypes.ZeroFeeCommitments => FeeratePerKw(0 sat)
         },
         fundingTxFeerate = TestConstants.feeratePerKw,
         fundingTxFeeBudget_opt = None,
@@ -265,6 +268,7 @@ trait ChannelStateTestsBase extends Assertions with Eventually {
       .modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.AnchorOutputsPhoenix))(_.removed(Features.AnchorOutputsZeroFeeHtlcTx).updated(Features.AnchorOutputs, FeatureSupport.Optional))
       .modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.OptionSimpleTaprootPhoenix))(_.removed(Features.SimpleTaprootChannelsStaging).updated(Features.SimpleTaprootChannelsPhoenix, FeatureSupport.Optional).updated(Features.PhoenixZeroReserve, FeatureSupport.Optional))
       .modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.OptionSimpleTaproot))(_.updated(Features.SimpleTaprootChannelsStaging, FeatureSupport.Optional))
+      .modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.ZeroFeeCommitments))(_.updated(Features.ZeroFeeCommitments, FeatureSupport.Optional))
     )
     val nodeParamsB1 = nodeParamsB.copy(features = nodeParamsB.features
       .modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.DisableWumbo))(_.removed(Features.Wumbo))
@@ -277,6 +281,7 @@ trait ChannelStateTestsBase extends Assertions with Eventually {
       .modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.AnchorOutputsPhoenix))(_.removed(Features.AnchorOutputsZeroFeeHtlcTx).updated(Features.AnchorOutputs, FeatureSupport.Optional))
       .modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.OptionSimpleTaprootPhoenix))(_.removed(Features.SimpleTaprootChannelsStaging).updated(Features.SimpleTaprootChannelsPhoenix, FeatureSupport.Optional).updated(Features.PhoenixZeroReserve, FeatureSupport.Optional))
       .modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.OptionSimpleTaproot))(_.updated(Features.SimpleTaprootChannelsStaging, FeatureSupport.Optional))
+      .modify(_.activated).usingIf(tags.contains(ChannelStateTestsTags.ZeroFeeCommitments))(_.updated(Features.ZeroFeeCommitments, FeatureSupport.Optional))
     )
     (nodeParamsA1, nodeParamsB1)
   }
@@ -287,7 +292,9 @@ trait ChannelStateTestsBase extends Assertions with Eventually {
 
     val scidAlias = canUse(Features.ScidAlias) && !announceChannel // alias feature is incompatible with public channel
     val zeroConf = canUse(Features.ZeroConf)
-    if (canUse(Features.SimpleTaprootChannelsStaging)) {
+    if (canUse(Features.ZeroFeeCommitments)) {
+      ChannelTypes.ZeroFeeCommitments(scidAlias, zeroConf)
+    } else if (canUse(Features.SimpleTaprootChannelsStaging)) {
       ChannelTypes.SimpleTaprootChannelsStaging(scidAlias, zeroConf)
     } else if (canUse(Features.SimpleTaprootChannelsPhoenix)) {
       ChannelTypes.SimpleTaprootChannelsPhoenix

eclair-core/src/test/scala/fr/acinq/eclair/channel/states/a/WaitForAcceptChannelStateSpec.scala

@@ -135,11 +135,20 @@ class WaitForAcceptChannelStateSpec extends TestKitBaseClass with FixtureAnyFunS
   test("recv AcceptChannel (invalid max accepted htlcs)") { f =>
     import f._
     val accept = bob2alice.expectMsgType[AcceptChannel]
-    // spec says max = 483
-    val invalidMaxAcceptedHtlcs = 484
-    alice ! accept.copy(maxAcceptedHtlcs = invalidMaxAcceptedHtlcs)
+    alice ! accept.copy(maxAcceptedHtlcs = 484)
     val error = alice2bob.expectMsgType[Error]
-    assert(error == Error(accept.temporaryChannelId, InvalidMaxAcceptedHtlcs(accept.temporaryChannelId, invalidMaxAcceptedHtlcs, Channel.MAX_ACCEPTED_HTLCS).getMessage))
+    assert(error == Error(accept.temporaryChannelId, InvalidMaxAcceptedHtlcs(accept.temporaryChannelId, maxAcceptedHtlcs = 484, max = 483).getMessage))
+    listener.expectMsgType[ChannelAborted]
+    awaitCond(alice.stateName == CLOSED)
+    aliceOpenReplyTo.expectMsgType[OpenChannelResponse.Rejected]
+  }
+
+  test("recv AcceptChannel (invalid max accepted htlcs, zero-fee commitments)", Tag(ChannelStateTestsTags.ZeroFeeCommitments)) { f =>
+    import f._
+    val accept = bob2alice.expectMsgType[AcceptChannel]
+    alice ! accept.copy(maxAcceptedHtlcs = 115)
+    val error = alice2bob.expectMsgType[Error]
+    assert(error == Error(accept.temporaryChannelId, InvalidMaxAcceptedHtlcs(accept.temporaryChannelId, maxAcceptedHtlcs = 115, max = 114).getMessage))
     listener.expectMsgType[ChannelAborted]
     awaitCond(alice.stateName == CLOSED)
     aliceOpenReplyTo.expectMsgType[OpenChannelResponse.Rejected]