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

Refactor routing hint failure updates (#2370)

Browse source 
This is a follow up of #2264 where we refactor handling of channel updates
in failures coming from routing hints.

For failures in one of the routing hints, we use the node_id pair (source,
destination) instead of the short_channel_id to identify the edge.
This commit is contained in:
Bastien Teinturier 2022-08-11 11:32:43 +02:00 • committed by GitHub
parent de1ac34d46
commit 2f590a80e2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 47 additions and 38 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
eclair-core/src/main/scala/fr/acinq/eclair/payment/Invoice.scala
View file

@ -55,17 +55,20 @@ trait Invoice {
}
object Invoice {
/** An extra edge that can be used to pay a given invoice and may not be part of the public graph. */
sealed trait ExtraEdge {
// @formatter:off
def sourceNodeId: PublicKey
def feeBase: MilliSatoshi
def feeProportionalMillionths: Long
def cltvExpiryDelta: CltvExpiryDelta
def htlcMinimum: MilliSatoshi
def htlcMaximum_opt: Option[MilliSatoshi]
def relayFees: Relayer.RelayFees = Relayer.RelayFees(feeBase = feeBase, feeProportionalMillionths = feeProportionalMillionths)
// @formatter:on
}
/** A normal graph edge, that should be handled exactly like public graph edges. */
case class BasicEdge(sourceNodeId: PublicKey,
targetNodeId: PublicKey,
shortChannelId: ShortChannelId,

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

@ -262,43 +262,52 @@ class PaymentLifecycle(nodeParams: NodeParams, cfg: SendPaymentConfig, router: A
* @return updated routing hints if applicable.
*/
private def handleUpdate(nodeId: PublicKey, failure: Update, data: WaitingForComplete): Seq[ExtraEdge] = {
// TODO: properly handle updates to channels provided as routing hints in the invoice
data.route.getChannelUpdateForNode(nodeId) match {
case Some(u) if u.shortChannelId != failure.update.shortChannelId =>
// it is possible that nodes in the route prefer using a different channel (to the same N+1 node) than the one we requested, that's fine
log.info(s"received an update for a different channel than the one we asked: requested=${u.shortChannelId} actual=${failure.update.shortChannelId} update=${failure.update}")
case Some(u) if Announcements.areSame(u, failure.update) =>
// node returned the exact same update we used, this can happen e.g. if the channel is imbalanced
// 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), 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), Some(nodeParams.routerConf.channelExcludeDuration))
case Some(u) =>
log.info(s"got a new update for shortChannelId=${u.shortChannelId}: old=$u new=${failure.update}")
val extraEdges1 = data.route.hops.find(_.nodeId == nodeId) match {
case Some(hop) => hop.params match {
case ann: ChannelRelayParams.FromAnnouncement =>
if (ann.channelUpdate.shortChannelId != failure.update.shortChannelId) {
// it is possible that nodes in the route prefer using a different channel (to the same N+1 node) than the one we requested, that's fine
log.info("received an update for a different channel than the one we asked: requested={} actual={} update={}", ann.channelUpdate.shortChannelId, failure.update.shortChannelId, failure.update)
} else if (Announcements.areSame(ann.channelUpdate, failure.update)) {
// node returned the exact same update we used, this can happen e.g. if the channel is imbalanced
// in that case, let's temporarily exclude the channel from future routes, giving it time to recover
log.info("received exact same update from nodeId={}, excluding the channel from futures routes", nodeId)
router ! ExcludeChannel(ChannelDesc(ann.channelUpdate.shortChannelId, nodeId, hop.nextNodeId), Some(nodeParams.routerConf.channelExcludeDuration))
} else 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("it is the second time nodeId={} answers with a new update, excluding it: old={} new={}", nodeId, ann.channelUpdate, failure.update)
router ! ExcludeChannel(ChannelDesc(ann.channelUpdate.shortChannelId, nodeId, hop.nextNodeId), Some(nodeParams.routerConf.channelExcludeDuration))
} else {
log.info("got a new update for shortChannelId={}: old={} new={}", ann.channelUpdate.shortChannelId, ann.channelUpdate, failure.update)
}
data.c.extraEdges
case _: ChannelRelayParams.FromHint =>
log.info("received an update for a routing hint (shortChannelId={} nodeId={} enabled={} update={})", failure.update.shortChannelId, nodeId, failure.update.channelFlags.isEnabled, failure.update)
if (failure.update.channelFlags.isEnabled) {
data.c.extraEdges.map {
case edge: BasicEdge if edge.sourceNodeId == nodeId && edge.targetNodeId == hop.nextNodeId => edge.update(failure.update)
case edge: BasicEdge => edge
}
} else {
// if the channel is disabled, we temporarily exclude it: this is necessary because the routing hint doesn't
// contain channel flags to indicate that it's disabled
// 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
.find { case edge: BasicEdge => edge.sourceNodeId == nodeId && edge.targetNodeId == hop.nextNodeId }
.foreach { case edge: BasicEdge => router ! ExcludeChannel(ChannelDesc(edge.shortChannelId, edge.sourceNodeId, edge.targetNodeId), Some(nodeParams.routerConf.channelExcludeDuration)) }
// we remove this edge for our next payment attempt
data.c.extraEdges.filterNot { case edge: BasicEdge => edge.sourceNodeId == nodeId && edge.targetNodeId == hop.nextNodeId }
}
}
case None =>
log.error(s"couldn't find a channel update for node=$nodeId, this should never happen")
log.error(s"couldn't find node=$nodeId in the route, this should never happen")
data.c.extraEdges
}
// in any case, we forward the update to the router: if the channel is disabled, the router will remove it from its routing table
// in all cases, we forward the update to the router: if the channel is disabled, the router will remove it from its routing table
// if the channel is not announced (e.g. was from a hint), the router will simply ignore the update
router ! failure.update
// we return updated assisted routes: they take precedence over the router's routing table
if (failure.update.channelFlags.isEnabled) {
data.c.extraEdges.map {
case edge: BasicEdge if edge.shortChannelId == failure.update.shortChannelId => edge.update(failure.update)
case edge => edge
}
} else {
// if the channel is disabled, we temporarily exclude it: this is necessary because the routing hint doesn't contain
// 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), 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
}
extraEdges1
}
def myStop(request: SendPayment, result: Either[PaymentFailed, PaymentSent]): State = {

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

@ -545,9 +545,6 @@ object Router {
amountToSend - amount
}
/** This method retrieves the channel update that we used when we built the route. */
def getChannelUpdateForNode(nodeId: PublicKey): Option[ChannelUpdate] = hops.find(_.nodeId == nodeId).map(_.params).collect { case s: ChannelRelayParams.FromAnnouncement => s.channelUpdate }
def printNodes(): String = hops.map(_.nextNodeId).mkString("->")
def printChannels(): String = hops.map(_.shortChannelId).mkString("->")

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

@ -629,8 +629,8 @@ class PaymentLifecycleSpec extends BaseRouterSpec {
sender.send(paymentFSM, addCompleted(HtlcResult.RemoteFail(UpdateFailHtlc(ByteVector32.Zeroes, 0, failureOnion))))
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), Some(nodeParams.routerConf.channelExcludeDuration)))
routerForwarder.expectMsg(channelUpdate_cd_disabled)
}
def testPermanentFailure(router: ActorRef, failure: FailureMessage): Unit = {