Improvements and fixes for #3217

docs/release-notes/eclair-vnext.md

@@ -9,6 +9,19 @@
 We remove the code used to support legacy channels that don't use anchor outputs or taproot.
 If you still have such channels, eclair won't start: you will need to close those channels, and will only be able to update eclair once they have been successfully closed.
 
+### Channel jamming accountability
+
+We update our channel jamming mitigation to match the latest draft of the [spec](https://github.com/lightning/bolts/pull/1280).
+Note that we use a different architecture for channel bucketing and confidence scoring than what is described in the BOLTs.
+We don't yet fail HTLCs that don't meet these restrictions: we're only collecting data so far to evaluate how the algorithm performs.
+
+If you want to disable this feature entirely, you can set the following values in `eclair.conf`:
+
+```conf
+eclair.relay.peer-reputation.enabled = false
+eclair.relay.reserved-for-accountable = 0.0
+```
+
 ### Configuration changes
 
 <insert changes>

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

@@ -255,8 +255,7 @@ eclair {
     // We assign reputation to our peers to prioritize payments during congestion.
     // The reputation is computed as fees paid divided by what should have been paid if all payments were successful.
     peer-reputation {
-      // Set this parameter to false to disable the reputation algorithm and simply relay the incoming endorsement
-      // value, as described by https://github.com/lightning/blips/blob/master/blip-0004.md,
+      // Set this parameter to false to disable the reputation algorithm and simply relay the incoming accountability.
       enabled = true
       // Reputation decays with the following half life to emphasize recent behavior.
       half-life = 30 days

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

@@ -475,7 +475,7 @@ class EclairImpl(val appKit: Kit) extends Eclair with Logging with SpendFromChan
   override def findRouteBetween(sourceNodeId: PublicKey, targetNodeId: PublicKey, amount: MilliSatoshi, pathFindingExperimentName_opt: Option[String], extraEdges: Seq[Invoice.ExtraEdge] = Seq.empty, includeLocalChannelCost: Boolean = false, ignoreNodeIds: Seq[PublicKey] = Seq.empty, ignoreShortChannelIds: Seq[ShortChannelId] = Seq.empty, maxFee_opt: Option[MilliSatoshi] = None)(implicit timeout: Timeout): Future[RouteResponse] = {
     getRouteParams(pathFindingExperimentName_opt) match {
       case Right(routeParams) =>
-        val target = ClearRecipient(targetNodeId, Features.empty, amount, CltvExpiry(appKit.nodeParams.currentBlockHeight), ByteVector32.Zeroes, extraEdges, upgradeAccountability = true)
+        val target = ClearRecipient(targetNodeId, Features.empty, amount, CltvExpiry(appKit.nodeParams.currentBlockHeight), ByteVector32.Zeroes, upgradeAccountability = true, extraEdges)
         val routeParams1 = routeParams.copy(
           includeLocalChannelCost = includeLocalChannelCost,
           boundaries = routeParams.boundaries.copy(

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

@@ -165,7 +165,7 @@ object Upstream {
     /** Our node is forwarding a payment based on a set of HTLCs from potentially multiple upstream channels. */
     case class Trampoline(received: List[Channel]) extends Hot {
       override val amountIn: MilliSatoshi = received.map(_.add.amountMsat).sum
-      val accountable: Boolean = received.map(_.add.accountable).reduce(_ || _)
+      val accountable: Boolean = received.exists(_.add.accountable)
       // We must use the lowest expiry of the incoming HTLC set.
       val expiryIn: CltvExpiry = received.map(_.add.cltvExpiry).min
       val receivedAt: TimestampMilli = received.map(_.receivedAt).max

eclair-core/src/main/scala/fr/acinq/eclair/payment/Bolt11Invoice.scala

@@ -72,7 +72,7 @@ case class Bolt11Invoice(prefix: String, amount_opt: Option[MilliSatoshi], creat
 
   override lazy val features: Features[InvoiceFeature] = tags.collectFirst { case f: InvoiceFeatures => f.features.invoiceFeatures() }.getOrElse(Features.empty)
 
-  override lazy val accountable: Boolean = tags.contains(Accountable)
+  override lazy val accountable: Boolean = tags.collectFirst { case a: Accountable => a }.nonEmpty
 
   /**
    * @return the hash of this payment invoice
@@ -149,7 +149,7 @@ object Bolt11Invoice {
         fallbackAddress.map(FallbackAddress(_)),
         expirySeconds.map(Expiry(_)),
         Some(MinFinalCltvExpiry(minFinalCltvExpiryDelta.toInt)),
-        Some(Accountable),
+        Some(Accountable()),
         // We want to keep invoices as small as possible, so we explicitly remove unknown features.
         Some(InvoiceFeatures(features.copy(unknown = Set.empty).unscoped()))
       ).flatten
@@ -289,8 +289,7 @@ object Bolt11Invoice {
   /**
    * Present if the recipient is willing to be held accountable for the timely resolution of HTLCs.
    */
-  case object Accountable extends TaggedField
-
+  case class Accountable() extends TaggedField
 
   /**
    * This returns a bitvector with the minimum size necessary to encode the long, left padded to have a length (in bits)
@@ -449,9 +448,9 @@ object Bolt11Invoice {
       .typecase(29, dataCodec(bits).as[UnknownTag29])
       .typecase(30, dataCodec(bits).as[UnknownTag30])
       .\(31) {
-        case Accountable => Accountable
+        case _: Accountable => Accountable()
         case a: InvalidTag31 => a: TaggedField
-      }(choice(dataCodec(provide(Accountable), expectedLength = Some(0)).upcast[TaggedField], dataCodec(bits).as[InvalidTag31].upcast[TaggedField]))
+      }(choice(dataCodec(provide(Accountable()), expectedLength = Some(0)).upcast[TaggedField], dataCodec(bits).as[InvalidTag31].upcast[TaggedField]))
 
     private def fixedSizeTrailingCodec[A](codec: Codec[A], size: Int): Codec[A] = Codec[A](
       (data: A) => codec.encode(data),

eclair-core/src/main/scala/fr/acinq/eclair/payment/Bolt12Invoice.scala

@@ -51,7 +51,7 @@ case class Bolt12Invoice(records: TlvStream[InvoiceTlv]) extends Invoice {
   val blindedPaths: Seq[PaymentBlindedRoute] = records.get[InvoicePaths].get.paths.zip(records.get[InvoiceBlindedPay].get.paymentInfo).map { case (route, info) => PaymentBlindedRoute(route, info) }
   val fallbacks: Option[Seq[FallbackAddress]] = records.get[InvoiceFallbacks].map(_.addresses)
   val signature: ByteVector64 = records.get[Signature].get.signature
-  override val accountable: Boolean = records.records.contains(InvoiceAccountable)
+  override val accountable: Boolean = records.get[InvoiceAccountable].nonEmpty
 
   // It is assumed that the request is valid for this offer.
   def validateFor(request: InvoiceRequest, pathNodeId: PublicKey): Either[String, Unit] = {
@@ -110,7 +110,7 @@ object Bolt12Invoice {
     val amount = request.amount
     val tlvs: Set[InvoiceTlv] = removeSignature(request.records).records ++ Set(
       Some(InvoicePaths(paths.map(_.route))),
-      Some(InvoiceAccountable),
+      Some(InvoiceAccountable()),
       Some(InvoiceBlindedPay(paths.map(_.paymentInfo))),
       Some(InvoiceCreatedAt(TimestampSecond.now())),
       Some(InvoiceRelativeExpiry(invoiceExpiry.toSeconds)),
@@ -170,7 +170,7 @@ case class MinimalBolt12Invoice(records: TlvStream[InvoiceTlv]) extends Invoice
   override val createdAt: TimestampSecond = records.get[InvoiceCreatedAt].get.timestamp
   override val relativeExpiry: FiniteDuration = FiniteDuration(records.get[InvoiceRelativeExpiry].map(_.seconds).getOrElse(Bolt12Invoice.DEFAULT_EXPIRY_SECONDS), TimeUnit.SECONDS)
   override val features: Features[InvoiceFeature] = records.get[InvoiceFeatures].map(f => Features(f.features).invoiceFeatures()).getOrElse(Features[InvoiceFeature](Features.BasicMultiPartPayment -> FeatureSupport.Optional))
-  override def accountable: Boolean = true
+  override val accountable: Boolean = true
 
   override def toString: String = {
     val data = OfferCodecs.invoiceTlvCodec.encode(records).require.bytes

eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentPacket.scala

@@ -144,8 +144,7 @@ object IncomingPaymentPacket {
                 }
             }
           case None if add.pathKey_opt.isDefined => Left(InvalidOnionBlinding(Sphinx.hash(add.onionRoutingPacket)))
-          case None if add.accountable && !payload.records.contains(UpgradeAccountability) =>
-            Left(InvalidOnionPayload(UInt64(19), 0))
+          case None if add.accountable && payload.get[UpgradeAccountability].isEmpty => Left(InvalidOnionPayload(UInt64(19), 0))
           case None =>
             // We are not inside a blinded path: channel relay information is directly available.
             IntermediatePayload.ChannelRelay.Standard.validate(payload).left.map(_.failureMessage).map(payload => ChannelRelayPacket(add, payload, nextPacket, TimestampMilli.now()))
@@ -162,8 +161,7 @@ object IncomingPaymentPacket {
               case DecodedEncryptedRecipientData(blindedPayload, _) => validateBlindedFinalPayload(add, payload, blindedPayload)
             }
           case None if add.pathKey_opt.isDefined => Left(InvalidOnionBlinding(Sphinx.hash(add.onionRoutingPacket)))
-          case None if add.accountable && !payload.records.contains(UpgradeAccountability) =>
-            Left(InvalidOnionPayload(UInt64(19), 0))
+          case None if add.accountable && payload.get[UpgradeAccountability].isEmpty => Left(InvalidOnionPayload(UInt64(19), 0))
           case None =>
             // We check if the payment is using trampoline: if it is, we may not be the final recipient.
             payload.get[OnionPaymentPayloadTlv.TrampolineOnion] match {
@@ -223,7 +221,7 @@ object IncomingPaymentPacket {
       case payload if add.amountMsat < payload.paymentRelayData.paymentConstraints.minAmount => Left(InvalidOnionBlinding(Sphinx.hash(add.onionRoutingPacket)))
       case payload if add.cltvExpiry > payload.paymentRelayData.paymentConstraints.maxCltvExpiry => Left(InvalidOnionBlinding(Sphinx.hash(add.onionRoutingPacket)))
       case payload if !Features.areCompatible(Features.empty, payload.paymentRelayData.allowedFeatures) => Left(InvalidOnionBlinding(Sphinx.hash(add.onionRoutingPacket)))
-      case _ if add.accountable && !blindedPayload.records.contains(RouteBlindingEncryptedDataTlv.UpgradeAccountability) => Left(InvalidOnionBlinding(Sphinx.hash(add.onionRoutingPacket)))
+      case _ if add.accountable && blindedPayload.get[RouteBlindingEncryptedDataTlv.UpgradeAccountability].isEmpty => Left(InvalidOnionBlinding(Sphinx.hash(add.onionRoutingPacket)))
       case payload => Right(ChannelRelayPacket(add, payload, nextPacket, TimestampMilli.now()))
     }
   }
@@ -242,7 +240,7 @@ object IncomingPaymentPacket {
       case payload if payload.paymentConstraints_opt.exists(c => c.maxCltvExpiry < add.cltvExpiry) => Left(InvalidOnionBlinding(Sphinx.hash(add.onionRoutingPacket)))
       case payload if !Features.areCompatible(Features.empty, payload.allowedFeatures) => Left(InvalidOnionBlinding(Sphinx.hash(add.onionRoutingPacket)))
       case payload if add.cltvExpiry < payload.expiry => Left(InvalidOnionBlinding(Sphinx.hash(add.onionRoutingPacket)))
-      case _ if add.accountable && !blindedPayload.records.contains(RouteBlindingEncryptedDataTlv.UpgradeAccountability) => Left(InvalidOnionBlinding(Sphinx.hash(add.onionRoutingPacket)))
+      case _ if add.accountable && blindedPayload.get[RouteBlindingEncryptedDataTlv.UpgradeAccountability].isEmpty => Left(InvalidOnionBlinding(Sphinx.hash(add.onionRoutingPacket)))
       case payload => Right(FinalPacket(add, payload, TimestampMilli.now()))
     }
   }

eclair-core/src/main/scala/fr/acinq/eclair/payment/relay/ChannelRelay.scala

@@ -16,11 +16,11 @@
 
 package fr.acinq.eclair.payment.relay
 
-import akka.actor.{ActorRef, typed}
 import akka.actor.typed.Behavior
 import akka.actor.typed.eventstream.EventStream
 import akka.actor.typed.scaladsl.adapter.TypedActorRefOps
 import akka.actor.typed.scaladsl.{ActorContext, Behaviors}
+import akka.actor.{ActorRef, typed}
 import fr.acinq.bitcoin.scalacompat.ByteVector32
 import fr.acinq.bitcoin.scalacompat.Crypto.PublicKey
 import fr.acinq.eclair.channel._
@@ -77,26 +77,17 @@ object ChannelRelay {
         paymentHash_opt = Some(r.add.paymentHash),
         nodeAlias_opt = Some(nodeParams.alias))) {
         val upstream = Upstream.Hot.Channel(r.add.removeUnknownTlvs(), r.receivedAt, originNode, incomingChannelOccupancy)
-        val accountable = r.add.accountable || incomingChannelOccupancy > 1 - nodeParams.relayParams.reservedBucket
-        if (accountable && !r.payload.upgradeAccountability) {
-          val relay = new ChannelRelay(nodeParams, register, channels, r, upstream, Reputation.Score.min, context, accountable)
-          Metrics.recordPaymentRelayFailed(Tags.FailureType.Jamming, Tags.RelayType.Channel)
-          context.log.info("rejecting htlc: unaccountable HTLC using reserved bucket")
-          relay.safeSendAndStop(r.add.channelId, relay.makeCmdFailHtlc(r.add.id, TemporaryChannelFailure(None)))
-        } else {
-          reputationRecorder_opt match {
-            // TODO: penalize HTLCs that do not use `upgradeAccountability`.
-            //case _ if !r.payload.upgradeAccountability =>
-            //  context.self ! WrappedReputationScore(Reputation.Score.min)
-            case Some(reputationRecorder) =>
-              reputationRecorder ! GetConfidence(context.messageAdapter(WrappedReputationScore(_)), channels.values.headOption.map(_.nextNodeId), r.relayFeeMsat, nodeParams.currentBlockHeight, r.outgoingCltv, accountable)
-            case None =>
-              context.self ! WrappedReputationScore(Reputation.Score.max(accountable))
-          }
-          Behaviors.receiveMessagePartial {
-            case WrappedReputationScore(score) =>
-              new ChannelRelay(nodeParams, register, channels, r, upstream, score, context, accountable).start()
-          }
+        val accountable = r.add.accountable || nodeParams.relayParams.incomingChannelCongested(upstream.incomingChannelOccupancy)
+        val nextNodeId_opt = channels.values.headOption.map(_.nextNodeId)
+        (reputationRecorder_opt, nextNodeId_opt) match {
+          case (Some(reputationRecorder), Some(nextNodeId)) =>
+            reputationRecorder ! GetConfidence(context.messageAdapter(WrappedReputationScore(_)), nextNodeId, r.relayFeeMsat, nodeParams.currentBlockHeight, r.outgoingCltv, accountable)
+          case _ =>
+            context.self ! WrappedReputationScore(Reputation.Score.max(accountable))
+        }
+        Behaviors.receiveMessagePartial {
+          case WrappedReputationScore(score) =>
+            new ChannelRelay(nodeParams, register, channels, r, upstream, score, context).start()
         }
       }
     }
@@ -143,8 +134,7 @@ class ChannelRelay private(nodeParams: NodeParams,
                            r: IncomingPaymentPacket.ChannelRelayPacket,
                            upstream: Upstream.Hot.Channel,
                            reputationScore: Reputation.Score,
-                           context: ActorContext[ChannelRelay.Command],
-                           accountable: Boolean) {
+                           context: ActorContext[ChannelRelay.Command]) {
 
   import ChannelRelay._
 
@@ -176,6 +166,13 @@ class ChannelRelay private(nodeParams: NodeParams,
   private case class PreviouslyTried(channelId: ByteVector32, failure: RES_ADD_FAILED[ChannelException])
 
   def start(): Behavior[Command] = {
+    val accountable = r.add.accountable || nodeParams.relayParams.incomingChannelCongested(upstream.incomingChannelOccupancy)
+    if (accountable && !r.payload.upgradeAccountability) {
+      // We don't yet enforce channel jamming protections: we log and update metrics as if we had failed that payment,
+      // but we currently relay it anyway. This will let us analyze data before actually activating jamming protection.
+      Metrics.recordPaymentRelayFailed(Tags.FailureType.Jamming, Tags.RelayType.Channel)
+      context.log.info("payment would have been rejected if jamming protection was activated")
+    }
     walletNodeId_opt match {
       case Some(walletNodeId) if nodeParams.peerWakeUpConfig.enabled => wakeUp(walletNodeId)
       case _ =>
@@ -282,7 +279,7 @@ class ChannelRelay private(nodeParams: NodeParams,
       }
   }
 
-  def safeSendAndStop(channelId: ByteVector32, cmd: channel.HtlcSettlementCommand): Behavior[Command] = {
+  private def safeSendAndStop(channelId: ByteVector32, cmd: channel.HtlcSettlementCommand): Behavior[Command] = {
     val toSend = cmd match {
       case _: CMD_FULFILL_HTLC => cmd
       case _: CMD_FAIL_HTLC | _: CMD_FAIL_MALFORMED_HTLC => r.payload match {
@@ -345,7 +342,7 @@ class ChannelRelay private(nodeParams: NodeParams,
           makeCmdFailHtlc(r.add.id, UnknownNextPeer())
         }
         walletNodeId_opt match {
-          case Some(walletNodeId) if shouldAttemptOnTheFlyFunding(remoteFeatures_opt, previousFailures) && !accountable => RelayNeedsFunding(walletNodeId, cmdFail)
+          case Some(walletNodeId) if shouldAttemptOnTheFlyFunding(remoteFeatures_opt, previousFailures) => RelayNeedsFunding(walletNodeId, cmdFail)
           case _ => RelayFailure(cmdFail)
         }
     }
@@ -460,7 +457,7 @@ class ChannelRelay private(nodeParams: NodeParams,
     featureOk && liquidityIssue && relayParamsOk
   }
 
-  def makeCmdFailHtlc(originHtlcId: Long, failure: FailureMessage, delay_opt: Option[FiniteDuration] = None): CMD_FAIL_HTLC = {
+  private def makeCmdFailHtlc(originHtlcId: Long, failure: FailureMessage, delay_opt: Option[FiniteDuration] = None): CMD_FAIL_HTLC = {
     val attribution = FailureAttributionData(htlcReceivedAt = upstream.receivedAt, trampolineReceivedAt_opt = None)
     CMD_FAIL_HTLC(originHtlcId, FailureReason.LocalFailure(failure), Some(attribution), delay_opt, commit = true)
   }