Fix: ChannelException could be ignored if hidden by a jamming one

Author: thomash-acinq

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

@@ -856,7 +856,7 @@ case class Commitments(params: ChannelParams,
    * @param cmd add HTLC command
    * @return either Left(failure, error message) where failure is a failure message (see BOLT #4 and the Failure Message class) or Right(new commitments, updateAddHtlc)
    */
-  def sendAdd(cmd: CMD_ADD_HTLC, currentHeight: BlockHeight, channelConf: ChannelConf, feerates: FeeratesPerKw, feeConf: OnChainFeeConf): Either[ChannelException, (Commitments, UpdateAddHtlc)] = {
+  def sendAdd(cmd: CMD_ADD_HTLC, currentHeight: BlockHeight, channelConf: ChannelConf, feerates: FeeratesPerKw, feeConf: OnChainFeeConf)(implicit log: LoggingAdapter): Either[ChannelException, (Commitments, UpdateAddHtlc)] = {
     // we must ensure we're not relaying htlcs that are already expired, otherwise the downstream channel will instantly close
     // NB: we add a 3 blocks safety to reduce the probability of running into this when our bitcoin node is slightly outdated
     val minExpiry = CltvExpiry(currentHeight + 3)
@@ -880,20 +880,28 @@ case class Commitments(params: ChannelParams,
     val changes1 = changes.addLocalProposal(add).copy(localNextHtlcId = changes.localNextHtlcId + 1)
     val originChannels1 = originChannels + (add.id -> cmd.origin)
     // we verify that this htlc is allowed in every active commitment
-    active.map(_.canSendAdd(add.amountMsat, params, changes1, feerates, feeConf, cmd.confidence))
-      .collectFirst { case Left(f) =>
+    val canSendAdds = active.map(_.canSendAdd(add.amountMsat, params, changes1, feerates, feeConf, cmd.confidence))
+    // Log only for jamming protection.
+    canSendAdds.collectFirst {
+      case Left(f: TooManySmallHtlcs) =>
+        log.info("TooManySmallHtlcs: {} outgoing HTLCs are below {}}", f.number, f.below)
         Metrics.dropHtlc(f, Tags.Directions.Outgoing)
-        Left(f)
-      }
-      // TODO: Delete these lines to activate jamming protections
-      .flatMap {
+      case Left(f: ConfidenceTooLow) =>
+        log.info("ConfidenceTooLow: confidence is {}% while channel is {}% full", (100 * f.confidence).toInt, (100 * f.occupancy).toInt)
+        Metrics.dropHtlc(f, Tags.Directions.Outgoing)
+    }
+    canSendAdds.flatMap { // TODO: We ignore jamming protection, delete this flatMap to activate jamming protection.
         case Left(_: TooManySmallHtlcs) | Left(_: ConfidenceTooLow) => None
         case x => Some(x)
       }
+      .collectFirst { case Left(f) =>
+        Metrics.dropHtlc(f, Tags.Directions.Outgoing)
+        Left(f)
+      }
       .getOrElse(Right(copy(changes = changes1, originChannels = originChannels1), add))
   }
 
-  def receiveAdd(add: UpdateAddHtlc, feerates: FeeratesPerKw, feeConf: OnChainFeeConf): Either[ChannelException, Commitments] = {
+  def receiveAdd(add: UpdateAddHtlc, feerates: FeeratesPerKw, feeConf: OnChainFeeConf)(implicit log: LoggingAdapter): Either[ChannelException, Commitments] = {
     if (add.id != changes.remoteNextHtlcId) {
       return Left(UnexpectedHtlcId(channelId, expected = changes.remoteNextHtlcId, actual = add.id))
     }
@@ -906,16 +914,21 @@ case class Commitments(params: ChannelParams,
 
     val changes1 = changes.addRemoteProposal(add).copy(remoteNextHtlcId = changes.remoteNextHtlcId + 1)
     // we verify that this htlc is allowed in every active commitment
-    active.map(_.canReceiveAdd(add.amountMsat, params, changes1, feerates, feeConf))
-      .collectFirst { case Left(f) =>
+    val canReceiveAdds = active.map(_.canReceiveAdd(add.amountMsat, params, changes1, feerates, feeConf))
+    // Log only for jamming protection.
+    canReceiveAdds.collectFirst {
+      case Left(f: TooManySmallHtlcs) =>
+        log.info("TooManySmallHtlcs: {} incoming HTLCs are below {}}", f.number, f.below)
         Metrics.dropHtlc(f, Tags.Directions.Incoming)
-        Left(f)
-      }
-      // TODO: Delete these lines to activate jamming protections
-      .flatMap {
+    }
+    canReceiveAdds.flatMap { // TODO: We ignore jamming protection, delete this flatMap to activate jamming protection.
         case Left(_: TooManySmallHtlcs) | Left(_: ConfidenceTooLow) => None
         case x => Some(x)
       }
+      .collectFirst { case Left(f) =>
+        Metrics.dropHtlc(f, Tags.Directions.Incoming)
+        Left(f)
+      }
       .getOrElse(Right(copy(changes = changes1)))
   }