mirrors/eclair
1
0
Fork 0
mirror of https://github.com/ACINQ/eclair.git synced 2025-02-23 14:40:34 +01:00
Code Issues Projects Releases Wiki Activity

Add explicit duration to ExcludeChannel (#2368)

Browse source 
This lets callers override the default duration or ban channels for
unlimited durations (until they send a `ListChannelExclusion`).
This commit is contained in:
Bastien Teinturier 2022-08-10 13:26:19 +02:00 • committed by GitHub
parent 0310bf5dc4
commit de1ac34d46
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 22 additions and 25 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
eclair-core/src/main/scala/fr/acinq/eclair/payment/send/PaymentLifecycle.scala
View file

@ -185,7 +185,7 @@ class PaymentLifecycle(nodeParams: NodeParams, cfg: SendPaymentConfig, router: A
failureMessage match {
case TemporaryChannelFailure(update) =>
d.route.hops.find(_.nodeId == nodeId) match {
case Some(failingHop) if ChannelRelayParams.areSame(failingHop.params, ChannelRelayParams.FromAnnouncement(update), true) =>
case Some(failingHop) if ChannelRelayParams.areSame(failingHop.params, ChannelRelayParams.FromAnnouncement(update), ignoreHtlcSize = true) =>
router ! Router.ChannelCouldNotRelay(stoppedRoute.amount, failingHop)
case _ => // otherwise the relay parameters may have changed, so it's not necessarily a liquidity issue
}
@ -272,12 +272,12 @@ class PaymentLifecycle(nodeParams: NodeParams, cfg: SendPaymentConfig, router: A
// in that case, let's temporarily exclude the channel from future routes, giving it time to recover
log.info(s"received exact same update from nodeId=$nodeId, excluding the channel from futures routes")
val nextNodeId = data.route.hops.find(_.nodeId == nodeId).get.nextNodeId
router ! ExcludeChannel(ChannelDesc(u.shortChannelId, nodeId, nextNodeId))
router ! ExcludeChannel(ChannelDesc(u.shortChannelId, nodeId, nextNodeId), Some(nodeParams.routerConf.channelExcludeDuration))
case Some(u) if PaymentFailure.hasAlreadyFailedOnce(nodeId, data.failures) =>
// this node had already given us a new channel update and is still unhappy, it is probably messing with us, let's exclude it
log.warning(s"it is the second time nodeId=$nodeId answers with a new update, excluding it: old=$u new=${failure.update}")
val nextNodeId = data.route.hops.find(_.nodeId == nodeId).get.nextNodeId
router ! ExcludeChannel(ChannelDesc(u.shortChannelId, nodeId, nextNodeId))
router ! ExcludeChannel(ChannelDesc(u.shortChannelId, nodeId, nextNodeId), Some(nodeParams.routerConf.channelExcludeDuration))
case Some(u) =>
log.info(s"got a new update for shortChannelId=${u.shortChannelId}: old=$u new=${failure.update}")
case None =>
@ -296,7 +296,7 @@ class PaymentLifecycle(nodeParams: NodeParams, cfg: SendPaymentConfig, router: A
// channel flags to indicate that it's disabled
data.c.extraEdges
.find { case edge: BasicEdge => edge.shortChannelId == failure.update.shortChannelId }
.foreach { case edge: BasicEdge => router ! ExcludeChannel(ChannelDesc(edge.shortChannelId, edge.sourceNodeId, edge.targetNodeId)) } // we want the exclusion to be router-wide so that sister payments in the case of MPP are aware the channel is faulty
.foreach { case edge: BasicEdge => router ! ExcludeChannel(ChannelDesc(edge.shortChannelId, edge.sourceNodeId, edge.targetNodeId), Some(nodeParams.routerConf.channelExcludeDuration)) } // we want the exclusion to be router-wide so that sister payments in the case of MPP are aware the channel is faulty
data.c.extraEdges
}
}

11
eclair-core/src/main/scala/fr/acinq/eclair/router/Router.scala
View file

@ -32,8 +32,8 @@ import fr.acinq.eclair.crypto.TransportHandler
import fr.acinq.eclair.db.NetworkDb
import fr.acinq.eclair.io.Peer.PeerRoutingMessage
import fr.acinq.eclair.payment.Invoice.ExtraEdge
import fr.acinq.eclair.payment.{Bolt11Invoice, Invoice}
import fr.acinq.eclair.payment.relay.Relayer
import fr.acinq.eclair.payment.{Bolt11Invoice, Invoice}
import fr.acinq.eclair.remote.EclairInternalsSerializer.RemoteTypes
import fr.acinq.eclair.router.Graph.GraphStructure.DirectedGraph
import fr.acinq.eclair.router.Graph.{HeuristicsConstants, WeightRatios}
@ -171,10 +171,9 @@ class Router(val nodeParams: NodeParams, watcher: typed.ActorRef[ZmqWatcher.Comm
case Event(TickPruneStaleChannels, d) =>
stay() using StaleChannels.handlePruneStaleChannels(d, nodeParams.db.network, nodeParams.currentBlockHeight)
case Event(ExcludeChannel(desc@ChannelDesc(shortChannelId, nodeId, _)), d) =>
val banDuration = nodeParams.routerConf.channelExcludeDuration
log.info("excluding shortChannelId={} from nodeId={} for duration={}", shortChannelId, nodeId, banDuration)
context.system.scheduler.scheduleOnce(banDuration, self, LiftChannelExclusion(desc))
case Event(ExcludeChannel(desc, duration_opt), d) =>
log.info("excluding shortChannelId={} from nodeId={} for duration={}", desc.shortChannelId, desc.a, duration_opt.getOrElse("n/a"))
duration_opt.foreach(banDuration => context.system.scheduler.scheduleOnce(banDuration, self, LiftChannelExclusion(desc)))
stay() using d.copy(excludedChannels = d.excludedChannels + desc)
case Event(LiftChannelExclusion(desc@ChannelDesc(shortChannelId, nodeId, _)), d) =>
@ -580,7 +579,7 @@ object Router {
// @formatter:off
/** This is used when we get a TemporaryChannelFailure, to give time for the channel to recover (note that exclusions are directed) */
case class ExcludeChannel(desc: ChannelDesc)
case class ExcludeChannel(desc: ChannelDesc, duration_opt: Option[FiniteDuration])
case class LiftChannelExclusion(desc: ChannelDesc)
// @formatter:on

8
eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentLifecycleSpec.scala
View file

@ -470,7 +470,7 @@ class PaymentLifecycleSpec extends BaseRouterSpec {
// payment lifecycle will ask the router to temporarily exclude this channel from its route calculations
assert(routerForwarder.expectMsgType[ChannelCouldNotRelay].hop.shortChannelId == update_bc.shortChannelId)
routerForwarder.expectMsg(ExcludeChannel(ChannelDesc(update_bc.shortChannelId, b, c)))
routerForwarder.expectMsg(ExcludeChannel(ChannelDesc(update_bc.shortChannelId, b, c), Some(nodeParams.routerConf.channelExcludeDuration)))
routerForwarder.forward(routerFixture.router)
// payment lifecycle forwards the embedded channelUpdate to the router
routerForwarder.expectMsg(update_bc)
@ -521,7 +521,7 @@ class PaymentLifecycleSpec extends BaseRouterSpec {
sender.send(paymentFSM, addCompleted(HtlcResult.RemoteFail(UpdateFailHtlc(ByteVector32.Zeroes, 0, Sphinx.FailurePacket.create(sharedSecrets2.head._1, failure2)))))
// this time the payment lifecycle will ask the router to temporarily exclude this channel from its route calculations
routerForwarder.expectMsg(ExcludeChannel(ChannelDesc(update_bc.shortChannelId, b, c)))
routerForwarder.expectMsg(ExcludeChannel(ChannelDesc(update_bc.shortChannelId, b, c), Some(nodeParams.routerConf.channelExcludeDuration)))
routerForwarder.forward(routerFixture.router)
// but it will still forward the embedded channelUpdate to the router
routerForwarder.expectMsg(channelUpdate_bc_modified_2)
@ -553,7 +553,7 @@ class PaymentLifecycleSpec extends BaseRouterSpec {
sender.send(paymentFSM, addCompleted(HtlcResult.RemoteFail(UpdateFailHtlc(ByteVector32.Zeroes, 0, Sphinx.FailurePacket.create(sharedSecrets1.head._1, failure)))))
// we should temporarily exclude that channel
assert(routerForwarder.expectMsgType[ChannelCouldNotRelay].hop.shortChannelId == update_bc.shortChannelId)
routerForwarder.expectMsg(ExcludeChannel(ChannelDesc(update_bc.shortChannelId, b, c)))
routerForwarder.expectMsg(ExcludeChannel(ChannelDesc(update_bc.shortChannelId, b, c), Some(nodeParams.routerConf.channelExcludeDuration)))
routerForwarder.expectMsg(update_bc)
// this was a single attempt payment
@ -630,7 +630,7 @@ class PaymentLifecycleSpec extends BaseRouterSpec {
assert(routerForwarder.expectMsgType[RouteCouldRelay].route.hops.map(_.shortChannelId) == Seq(update_ab, update_bc).map(_.shortChannelId))
routerForwarder.expectMsg(channelUpdate_cd_disabled)
routerForwarder.expectMsg(ExcludeChannel(ChannelDesc(update_cd.shortChannelId, c, d)))
routerForwarder.expectMsg(ExcludeChannel(ChannelDesc(update_cd.shortChannelId, c, d), Some(nodeParams.routerConf.channelExcludeDuration)))
}
def testPermanentFailure(router: ActorRef, failure: FailureMessage): Unit = {

20
eclair-core/src/test/scala/fr/acinq/eclair/router/RouterSpec.scala
View file

@ -22,22 +22,20 @@ import akka.testkit.TestProbe
import fr.acinq.bitcoin.scalacompat.Crypto.PublicKey
import fr.acinq.bitcoin.scalacompat.Script.{pay2wsh, write}
import fr.acinq.bitcoin.scalacompat.{Block, SatoshiLong, Transaction, TxOut}
import fr.acinq.eclair.RealShortChannelId
import fr.acinq.eclair.blockchain.bitcoind.ZmqWatcher._
import fr.acinq.eclair.channel.{AvailableBalanceChanged, CommitmentsSpec, LocalChannelUpdate, RealScidStatus, ShortIds}
import fr.acinq.eclair.channel.{AvailableBalanceChanged, CommitmentsSpec, LocalChannelUpdate}
import fr.acinq.eclair.crypto.TransportHandler
import fr.acinq.eclair.io.Peer.PeerRoutingMessage
import fr.acinq.eclair.payment.{Bolt11Invoice, Invoice}
import fr.acinq.eclair.payment.Bolt11Invoice.ExtraHop
import fr.acinq.eclair.payment.{Bolt11Invoice, Invoice}
import fr.acinq.eclair.router.Announcements.{makeChannelUpdate, makeNodeAnnouncement}
import fr.acinq.eclair.router.BaseRouterSpec.channelAnnouncement
import fr.acinq.eclair.router.Graph.GraphStructure.GraphEdge
import fr.acinq.eclair.router.Graph.RoutingHeuristics
import fr.acinq.eclair.router.RouteCalculationSpec.{DEFAULT_AMOUNT_MSAT, DEFAULT_MAX_FEE, DEFAULT_ROUTE_PARAMS}
import fr.acinq.eclair.router.Router._
import fr.acinq.eclair.transactions.Scripts
import fr.acinq.eclair.wire.protocol._
import fr.acinq.eclair.{BlockHeight, CltvExpiryDelta, Features, MilliSatoshi, MilliSatoshiLong, RealShortChannelId, ShortChannelId, TestConstants, TimestampSecond, nodeFee, randomKey}
import fr.acinq.eclair.{BlockHeight, CltvExpiryDelta, Features, MilliSatoshi, MilliSatoshiLong, RealShortChannelId, ShortChannelId, TestConstants, TimestampSecond, randomKey}
import scodec.bits._
import scala.concurrent.duration._
@ -431,7 +429,7 @@ class RouterSpec extends BaseRouterSpec {
sender.expectMsgType[RouteResponse]
val bc = ChannelDesc(scid_bc, b, c)
// let's exclude channel b->c
sender.send(router, ExcludeChannel(bc))
sender.send(router, ExcludeChannel(bc, Some(1 hour)))
sender.send(router, RouteRequest(a, d, DEFAULT_AMOUNT_MSAT, DEFAULT_MAX_FEE, routeParams = DEFAULT_ROUTE_PARAMS))
sender.expectMsg(Failure(RouteNotFound))
// note that cb is still available!
@ -620,7 +618,7 @@ class RouterSpec extends BaseRouterSpec {
val sender = TestProbe()
sender.send(router, GetRoutingState)
val channel_ab = sender.expectMsgType[RoutingState].channels.find(_.ann == chan_ab).get
assert(channel_ab.meta_opt == None)
assert(channel_ab.meta_opt.isEmpty)
{
// When the local channel comes back online, it will send a LocalChannelUpdate to the router.
@ -637,7 +635,7 @@ class RouterSpec extends BaseRouterSpec {
val edge_ba = g.getEdge(ChannelDesc(scid_ab, b, a)).get
assert(edge_ab.capacity == channel_ab.capacity && edge_ba.capacity == channel_ab.capacity)
assert(balances.contains(edge_ab.balance_opt))
assert(edge_ba.balance_opt == None)
assert(edge_ba.balance_opt.isEmpty)
}
{
@ -660,7 +658,7 @@ class RouterSpec extends BaseRouterSpec {
val edge_ba = g.getEdge(ChannelDesc(scid_ab, b, a)).get
assert(edge_ab.capacity == channel_ab.capacity && edge_ba.capacity == channel_ab.capacity)
assert(balances.contains(edge_ab.balance_opt))
assert(edge_ba.balance_opt == None)
assert(edge_ba.balance_opt.isEmpty)
}
{
@ -678,7 +676,7 @@ class RouterSpec extends BaseRouterSpec {
val edge_ba = g.getEdge(ChannelDesc(scid_ab, b, a)).get
assert(edge_ab.capacity == channel_ab.capacity && edge_ba.capacity == channel_ab.capacity)
assert(balances.contains(edge_ab.balance_opt))
assert(edge_ba.balance_opt == None)
assert(edge_ba.balance_opt.isEmpty)
}
{
@ -693,7 +691,7 @@ class RouterSpec extends BaseRouterSpec {
// And the graph should be updated too.
val edge_ag = data.graphWithBalances.graph.getEdge(ChannelDesc(alias_ag_private, a, g)).get
assert(edge_ag.capacity == channel_ag.capacity)
assert(edge_ag.balance_opt == Some(33000000 msat))
assert(edge_ag.balance_opt.contains(33000000 msat))
}
}