Reclaim liquidity from idle channels (#3269)

diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/profit/PeerScorer.scala b/eclair-core/src/main/scala/fr/acinq/eclair/profit/PeerScorer.scala
index 12c4e03..aebffcb 100644
--- a/eclair-core/src/main/scala/fr/acinq/eclair/profit/PeerScorer.scala
+++ b/eclair-core/src/main/scala/fr/acinq/eclair/profit/PeerScorer.scala
@@ -28,7 +28,7 @@ import fr.acinq.eclair.channel.{CMD_CLOSE, CMD_UPDATE_RELAY_FEE, ChannelFlags, R
 import fr.acinq.eclair.io.Peer.OpenChannel
 import fr.acinq.eclair.payment.relay.Relayer.RelayFees
 import fr.acinq.eclair.profit.PeerStatsTracker._
-import fr.acinq.eclair.{MilliSatoshi, MilliSatoshiLong, NodeParams, TimestampMilli}
+import fr.acinq.eclair.{MilliSatoshi, MilliSatoshiLong, NodeParams, TimestampMilli, TimestampSecond}
 
 import scala.concurrent.ExecutionContext
 import scala.concurrent.duration.{DurationInt, FiniteDuration}
@@ -291,8 +291,10 @@ private class PeerScorer(nodeParams: NodeParams, wallet: OnChainBalanceChecker,
     val hasPastData = bestPeersByVolume.exists(_.stats.drop(Bucket.bucketsPerDay).exists(_ != PeerStats.empty))
     if (hasPastData && replyTo_opt.isEmpty) {
       closeUnbalancedChannelsIfNeeded(peers)
-      val history1 = updateRelayFeesIfNeeded(bestPeersByVolume, history)
-      fundChannelsIfNeeded(bestPeersThatNeedLiquidity, goodSmallPeers, peersToRevive, history1)
+      closeIdleChannelsIfNeeded(peers)
+      val (updatedPeers, history1) = updateRelayFeesIfNeeded(bestPeersByVolume, history)
+      val history2 = decreaseIdleChannelsRelayFeesIfNeeded(peers.filterNot(p => updatedPeers.contains(p.remoteNodeId)), history1)
+      fundChannelsIfNeeded(bestPeersThatNeedLiquidity, goodSmallPeers, peersToRevive, history2)
     } else {
       replyTo_opt.foreach(_ ! bestPeersThatNeedLiquidity.map(_.peer))
       run(history)
@@ -334,6 +336,26 @@ private class PeerScorer(nodeParams: NodeParams, wallet: OnChainBalanceChecker,
       })
   }
 
+  /** We close channels where liquidity has been idle for too long with minimal relay fees. */
+  private def closeIdleChannelsIfNeeded(peers: Seq[PeerInfo]): Unit = {
+    peers
+      // We only close channels when we have more than one.
+      .filter(_.channels.size > 1)
+      // We only close channels for which liquidity is idle.
+      .filter(p => p.stats.map(_.totalAmountOut).sum <= p.capacity * 0.05 && p.stats.map(_.totalAmountIn).sum <= p.capacity * 0.05)
+      // And relay fees have been minimal for long enough to give a chance for routing to catch up.
+      .filter(p => p.latestUpdate_opt.exists(u => u.relayFees.feeProportionalMillionths <= config.relayFees.minRelayFees.feeProportionalMillionths && u.timestamp <= TimestampSecond.now() - 5.days))
+      .foreach(p => {
+        // We keep the best channel and close the others.
+        val toClose = sortChannelsToClose(p.channels).tail
+          // We only close channels where we have a high enough balance.
+          .filter(c => c.canSend >= config.liquidity.localBalanceClosingThreshold)
+          // We don't close channels that have been opened in the last 3 days.
+          .filter(c => c.shortChannelId_opt.forall(scid => scid.blockHeight <= nodeParams.currentBlockHeight - 6 * 24 * 3))
+        closeChannels(p.remoteNodeId, toClose)
+      })
+  }
+
   private def sortChannelsToClose(channels: Seq[ChannelInfo]): Seq[ChannelInfo] = {
     channels.sortWith {
       // We want to keep a public channel over a private channel.
@@ -359,7 +381,7 @@ private class PeerScorer(nodeParams: NodeParams, wallet: OnChainBalanceChecker,
   }
 
   /** We update the relay fees of our top peers to shape volume based on our available liquidity. */
-  private def updateRelayFeesIfNeeded(peers: Seq[PeerInfo], history: DecisionHistory): DecisionHistory = {
+  private def updateRelayFeesIfNeeded(peers: Seq[PeerInfo], history: DecisionHistory): (Set[PublicKey], DecisionHistory) = {
     // We configure *daily* absolute and proportional payment volume targets. We look at events from the current period
     // and the previous period, so we need to get the right ratio to convert those daily amounts.
     val now = TimestampMilli.now()
@@ -455,7 +477,35 @@ private class PeerScorer(nodeParams: NodeParams, wallet: OnChainBalanceChecker,
     updateRelayFeesIfEnabled(peers, feeIncreases ++ feeDecreases ++ feeReverts)
     // Note that in order to avoid oscillating between reverts (reverting a revert), we remove the previous records when
     // reverting a change: this way, the normal algorithm resumes during the next run.
-    history.addFeeChanges(feeIncreases ++ feeDecreases).revertFeeChanges(feeReverts.keySet)
+    val history1 = history.addFeeChanges(feeIncreases ++ feeDecreases).revertFeeChanges(feeReverts.keySet)
+    val updatedPeers = feeIncreases.keySet ++ feeDecreases.keySet ++ feeReverts.keySet
+    (updatedPeers, history1)
+  }
+
+  /**
+   * When channels have been idle for too long, we decrease their relay fees to boost payment volume.
+   * If that doesn't work, we will close these channels with [[closeIdleChannelsIfNeeded]].
+   */
+  private def decreaseIdleChannelsRelayFeesIfNeeded(peers: Seq[PeerInfo], history: DecisionHistory): DecisionHistory = {
+    val feeDecreases = peers
+      // We're only interested in channels for which liquidity is idle.
+      // We ignore peers for which more than 75% of the funds are on their side: they have a higher incentive than us to
+      // close those channels if they aren't useful, so we'll wait for them to do so.
+      .filter(p => p.stats.map(_.totalAmountOut).sum <= p.capacity * 0.05 && p.stats.map(_.totalAmountIn).sum <= p.capacity * 0.05 && p.canSend >= p.capacity * 0.25)
+      // And relay fees aren't already minimal.
+      .filter(p => p.latestUpdate_opt.exists(u => u.relayFees.feeProportionalMillionths > config.relayFees.minRelayFees.feeProportionalMillionths))
+      // And relay fees haven't been updated recently.
+      .filter(p => p.latestUpdate_opt.exists(u => u.timestamp <= TimestampSecond.now() - 1.day))
+      .flatMap(p => {
+        p.latestUpdate_opt match {
+          case Some(u) =>
+            val next = u.relayFees.copy(feeProportionalMillionths = (u.feeProportionalMillionths - 500).max(config.relayFees.minRelayFees.feeProportionalMillionths))
+            Some(p.remoteNodeId -> FeeChangeDecision(FeeDecrease, u.relayFees, next, p.dailyVolumeOut))
+          case None => None
+        }
+      }).toMap
+    updateRelayFeesIfEnabled(peers, feeDecreases)
+    history.addFeeChanges(feeDecreases)
   }
 
   private def updateRelayFeesIfEnabled(peers: Seq[PeerInfo], decisions: Map[PublicKey, FeeChangeDecision]): Unit = {
diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/profit/PeerScorerSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/profit/PeerScorerSpec.scala
index f0dfdec..4f0fc96 100644
--- a/eclair-core/src/test/scala/fr/acinq/eclair/profit/PeerScorerSpec.scala
+++ b/eclair-core/src/test/scala/fr/acinq/eclair/profit/PeerScorerSpec.scala
@@ -598,6 +598,61 @@ class PeerScorerSpec extends ScalaTestWithActorTestKit(ConfigFactory.load("appli
     }
   }
 
+  test("lower fees from idle channels and then close") {
+    val config = defaultConfig.copy(relayFees = defaultConfig.relayFees.copy(minRelayFees = RelayFees(feeBase = 1 msat, feeProportionalMillionths = 600)))
+    withFixture(config = config) { f =>
+      import f._
+
+      val c1a = channelInfo(canSend = 1 btc, canReceive = 1 btc)
+      val c1b = channelInfo(canSend = 0.5 btc, canReceive = 0.5 btc)
+      val c2a = channelInfo(canSend = 1 btc, canReceive = 1 btc)
+      val c2b = channelInfo(canSend = 0.5 btc, canReceive = 0.5 btc)
+      val c3a = channelInfo(canSend = 0.2 btc, canReceive = 0.8 btc)
+      val c3b = channelInfo(canSend = 0.2 btc, canReceive = 0.8 btc)
+
+      // Our first peer's channels have very low volume and the last channel update already used our minimum fees: we should close it.
+      val peerInfo1 = PeerInfo(
+        remoteNodeId = remoteNodeId1,
+        stats = Seq.fill(weeklyBuckets)(peerStats(totalAmountOut = 1_000 sat, totalAmountIn = 1_000 sat, relayFeeEarned = 10 sat)),
+        channels = Seq(c1a, c1b),
+        latestUpdate_opt = Some(channelUpdate(c1a.capacity, RelayFees(1 msat, 600), TimestampSecond.now() - 6.days)),
+        hasPendingChannel = false
+      )
+      // Our second peer's channels have very low volume, but we're not yet using our minimum fees: we should decrease them.
+      val peerInfo2 = PeerInfo(
+        remoteNodeId = remoteNodeId2,
+        stats = Seq.fill(weeklyBuckets)(peerStats(totalAmountOut = 1_000 sat, totalAmountIn = 1_000 sat, relayFeeEarned = 10 sat)),
+        channels = Seq(c2a, c2b),
+        latestUpdate_opt = Some(channelUpdate(c2a.capacity, RelayFees(1 msat, 750), TimestampSecond.now() - 4.days)),
+        hasPendingChannel = false
+      )
+      // Our last peer's channels have very low volume, but we have less than 25% of the funds: we shouldn't do anything yet.
+      val peerInfo3 = PeerInfo(
+        remoteNodeId = remoteNodeId3,
+        stats = Seq.fill(weeklyBuckets)(peerStats(totalAmountOut = 1_000 sat, totalAmountIn = 1_000 sat, relayFeeEarned = 10 sat)),
+        channels = Seq(c3a, c3b),
+        latestUpdate_opt = Some(channelUpdate(c3a.capacity, RelayFees(1 msat, 750), TimestampSecond.now() - 4.days)),
+        hasPendingChannel = false
+      )
+
+      scorer ! ScorePeers(None)
+      inside(tracker.expectMessageType[GetLatestStats]) { msg =>
+        msg.replyTo ! LatestStats(Seq(peerInfo1, peerInfo2, peerInfo3))
+      }
+      // We start by closing one of our channels with our first peer.
+      assert(register.expectMessageType[Register.Forward[CMD_CLOSE]].channelId == c1b.channelId)
+      // Then we lower our relay fees with our second peer.
+      val feeUpdates = Seq(
+        register.expectMessageType[Register.Forward[CMD_UPDATE_RELAY_FEE]],
+        register.expectMessageType[Register.Forward[CMD_UPDATE_RELAY_FEE]],
+      )
+      assert(feeUpdates.map(_.channelId).toSet == Set(c2a.channelId, c2b.channelId))
+      feeUpdates.foreach(cmd => assert(cmd.message.feeProportionalMillionths == 600))
+      // And we're done!
+      register.expectNoMessage(100 millis)
+    }
+  }
+
   test("don't fund the same peer within cooldown period") {
     withFixture(onChainBalance = 10 btc) { f =>
       import f._