Skip to content

Commit

Permalink
Group ignoreNodes and ignoreChannels
Browse files Browse the repository at this point in the history
  • Loading branch information
@t-bast
t-bast committed May 28, 2020
1 parent 60c8ba4 commit 4065eae
Show file tree
Hide file tree
Showing 8 changed files with 101 additions and 90 deletions.
28 changes: 14 additions & 14 deletions eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentEvents.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import fr.acinq.bitcoin.Crypto.PublicKey
import fr.acinq.eclair.MilliSatoshi
import fr.acinq.eclair.crypto.Sphinx
import fr.acinq.eclair.router.Announcements
import fr.acinq.eclair.router.Router.{ChannelDesc, ChannelHop, Hop}
import fr.acinq.eclair.router.Router.{ChannelDesc, ChannelHop, Hop, Ignore}
import fr.acinq.eclair.wire.Node

/**
Expand Down Expand Up @@ -162,43 +162,43 @@ object PaymentFailure {
.isDefined

/** Update the set of nodes and channels to ignore in retries depending on the failure we received. */
def updateIgnored(failure: PaymentFailure, ignoreNodes: Set[PublicKey], ignoreChannels: Set[ChannelDesc]): (Set[PublicKey], Set[ChannelDesc]) = failure match {
def updateIgnored(failure: PaymentFailure, ignore: Ignore): Ignore = failure match {
case RemoteFailure(hops, Sphinx.DecryptedFailurePacket(nodeId, _)) if nodeId == hops.last.nextNodeId =>
// The failure came from the final recipient: the payment should be aborted without penalizing anyone in the route.
(ignoreNodes, ignoreChannels)
ignore
case RemoteFailure(_, Sphinx.DecryptedFailurePacket(nodeId, _: Node)) =>
(ignoreNodes + nodeId, ignoreChannels)
ignore + nodeId
case RemoteFailure(_, Sphinx.DecryptedFailurePacket(nodeId, failureMessage: Update)) =>
if (Announcements.checkSig(failureMessage.update, nodeId)) {
// We were using an outdated channel update, we should retry with the new one and nobody should be penalized.
(ignoreNodes, ignoreChannels)
ignore
} else {
// This node is fishy, it gave us a bad signature, so let's filter it out.
(ignoreNodes + nodeId, ignoreChannels)
ignore + nodeId
}
case RemoteFailure(hops, Sphinx.DecryptedFailurePacket(nodeId, _)) =>
// Let's ignore the channel outgoing from nodeId.
hops.collectFirst {
case hop: ChannelHop if hop.nodeId == nodeId => ChannelDesc(hop.lastUpdate.shortChannelId, hop.nodeId, hop.nextNodeId)
} match {
case Some(faultyChannel) => (ignoreNodes, ignoreChannels + faultyChannel)
case None => (ignoreNodes, ignoreChannels)
case Some(faultyChannel) => ignore + faultyChannel
case None => ignore
}
case UnreadableRemoteFailure(hops) =>
// We don't know which node is sending garbage, let's blacklist all nodes except the one we are directly connected to and the final recipient.
val blacklist = hops.map(_.nextNodeId).drop(1).dropRight(1)
(ignoreNodes ++ blacklist, ignoreChannels)
val blacklist = hops.map(_.nextNodeId).drop(1).dropRight(1).toSet
ignore ++ blacklist
case LocalFailure(hops, _) => hops.headOption match {
case Some(hop: ChannelHop) =>
val faultyChannel = ChannelDesc(hop.lastUpdate.shortChannelId, hop.nodeId, hop.nextNodeId)
(ignoreNodes, ignoreChannels + faultyChannel)
case _ => (ignoreNodes, ignoreChannels)
ignore + faultyChannel
case _ => ignore
}
}

/** Update the set of nodes and channels to ignore in retries depending on the failures we received. */
def updateIgnored(failures: Seq[PaymentFailure], ignoreNodes: Set[PublicKey], ignoreChannels: Set[ChannelDesc]): (Set[PublicKey], Set[ChannelDesc]) = {
failures.foldLeft((ignoreNodes, ignoreChannels)) { case ((nodes, channels), failure) => updateIgnored(failure, nodes, channels) }
def updateIgnored(failures: Seq[PaymentFailure], ignore: Ignore): Ignore = {
failures.foldLeft(ignore) { case (current, failure) => updateIgnored(failure, current) }
}

}
26 changes: 12 additions & 14 deletions eclair-core/src/main/scala/fr/acinq/eclair/payment/send/MultiPartPaymentLifecycle.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ class MultiPartPaymentLifecycle(nodeParams: NodeParams, cfg: SendPaymentConfig,
val routeParams = r.getRouteParams(nodeParams, randomize = false) // we don't randomize the first attempt, regardless of configuration choices
val maxFee = routeParams.getMaxFee(r.totalAmount)
log.debug("sending {} with maximum fee {}", r.totalAmount, maxFee)
val d = PaymentProgress(sender, r, r.maxAttempts, Map.empty, Set.empty, Set.empty, Nil)
val d = PaymentProgress(sender, r, r.maxAttempts, Map.empty, Ignore.empty, Nil)
router ! createRouteRequest(nodeParams, r.totalAmount, maxFee, routeParams, d)
goto(WAIT_FOR_ROUTES) using d
}
Expand Down Expand Up @@ -99,16 +99,16 @@ class MultiPartPaymentLifecycle(nodeParams: NodeParams, cfg: SendPaymentConfig,

case Event(Status.Failure(t), d: PaymentProgress) =>
log.warning("router error: {}", t.getMessage)
if (d.ignoreChannels.nonEmpty) {
if (d.ignore.channels.nonEmpty) {
// Channels are mostly ignored for temporary reasons, likely because they didn't have enough balance to forward
// the payment. When we're retrying an MPP split, it may make sense to retry those ignored channels because with
// a different split, they may have enough balance to forward the payment.
val (toSend, maxFee) = remainingToSend(nodeParams, d.request, d.pending.values)
retriedFailedChannels = true
log.debug("retry sending {} with maximum fee {} without ignoring channels ({})", toSend, maxFee, d.ignoreChannels.map(_.shortChannelId).mkString(","))
log.debug("retry sending {} with maximum fee {} without ignoring channels ({})", toSend, maxFee, d.ignore.channels.map(_.shortChannelId).mkString(","))
val routeParams = d.request.getRouteParams(nodeParams, randomize = true) // we randomize route selection when we retry
router ! createRouteRequest(nodeParams, toSend, maxFee, routeParams, d).copy(ignoreChannels = Set.empty)
stay using d.copy(remainingAttempts = (d.remainingAttempts - 1).max(0), ignoreChannels = Set.empty)
router ! createRouteRequest(nodeParams, toSend, maxFee, routeParams, d).copy(ignore = d.ignore.emptyChannels())
stay using d.copy(remainingAttempts = (d.remainingAttempts - 1).max(0), ignore = d.ignore.emptyChannels())
} else {
val failure = LocalFailure(Nil, t)
Metrics.PaymentError.withTag(Tags.Failure, Tags.FailureType(failure)).increment()
Expand All @@ -119,8 +119,8 @@ class MultiPartPaymentLifecycle(nodeParams: NodeParams, cfg: SendPaymentConfig,
if (isFinalRecipientFailure(pf, d)) {
goto(PAYMENT_ABORTED) using PaymentAborted(d.sender, d.request, d.failures ++ pf.failures, d.pending.keySet - pf.id)
} else {
val (ignoreNodes, ignoreChannels) = PaymentFailure.updateIgnored(pf.failures, d.ignoreNodes, d.ignoreChannels)
stay using d.copy(pending = d.pending - pf.id, ignoreNodes = ignoreNodes, ignoreChannels = ignoreChannels, failures = d.failures ++ pf.failures)
val ignore1 = PaymentFailure.updateIgnored(pf.failures, d.ignore)
stay using d.copy(pending = d.pending - pf.id, ignore = ignore1, failures = d.failures ++ pf.failures)
}

// The recipient released the preimage without receiving the full payment amount.
Expand All @@ -140,12 +140,12 @@ class MultiPartPaymentLifecycle(nodeParams: NodeParams, cfg: SendPaymentConfig,
Metrics.PaymentError.withTag(Tags.Failure, Tags.FailureType(failure)).increment()
goto(PAYMENT_ABORTED) using PaymentAborted(d.sender, d.request, d.failures ++ pf.failures :+ failure, d.pending.keySet - pf.id)
} else {
val (ignoreNodes, ignoreChannels) = PaymentFailure.updateIgnored(pf.failures, d.ignoreNodes, d.ignoreChannels)
val ignore1 = PaymentFailure.updateIgnored(pf.failures, d.ignore)
val stillPending = d.pending - pf.id
val (toSend, maxFee) = remainingToSend(nodeParams, d.request, stillPending.values)
log.debug("child payment failed, retry sending {} with maximum fee {}", toSend, maxFee)
val routeParams = d.request.getRouteParams(nodeParams, randomize = true) // we randomize route selection when we retry
val d1 = d.copy(pending = stillPending, ignoreNodes = ignoreNodes, ignoreChannels = ignoreChannels, failures = d.failures ++ pf.failures)
val d1 = d.copy(pending = stillPending, ignore = ignore1, failures = d.failures ++ pf.failures)
router ! createRouteRequest(nodeParams, toSend, maxFee, routeParams, d1)
goto(WAIT_FOR_ROUTES) using d1
}
Expand Down Expand Up @@ -331,15 +331,14 @@ object MultiPartPaymentLifecycle {
* @param request payment request containing the total amount to send.
* @param remainingAttempts remaining attempts (after child payments fail).
* @param pending pending child payments (payment sent, we are waiting for a fulfill or a failure).
* @param ignoreChannels channels that should be ignored (previously returned a permanent error).
* @param ignore channels and nodes that should be ignored (previously returned a permanent error).
* @param failures previous child payment failures.
*/
case class PaymentProgress(sender: ActorRef,
request: SendMultiPartPayment,
remainingAttempts: Int,
pending: Map[UUID, Route],
ignoreNodes: Set[PublicKey],
ignoreChannels: Set[ChannelDesc],
ignore: Ignore,
failures: Seq[PaymentFailure]) extends Data

/**
Expand Down Expand Up @@ -373,8 +372,7 @@ object MultiPartPaymentLifecycle {
toSend,
maxFee,
d.request.assistedRoutes,
d.ignoreNodes,
d.ignoreChannels,
d.ignore,
Some(routeParams),
allowMultiPart = true,
d.pending.values.toSeq)
Expand Down
38 changes: 19 additions & 19 deletions eclair-core/src/main/scala/fr/acinq/eclair/payment/send/PaymentLifecycle.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class PaymentLifecycle(nodeParams: NodeParams, cfg: SendPaymentConfig, router: A
if (cfg.storeInDb) {
paymentsDb.addOutgoingPayment(OutgoingPayment(id, cfg.parentId, cfg.externalId, paymentHash, PaymentType.Standard, c.finalPayload.amount, cfg.recipientAmount, cfg.recipientNodeId, System.currentTimeMillis, cfg.paymentRequest, OutgoingPaymentStatus.Pending))
}
goto(WAITING_FOR_ROUTE) using WaitingForRoute(sender, send, Nil, Set.empty, Set.empty)
goto(WAITING_FOR_ROUTE) using WaitingForRoute(sender, send, Nil, Ignore.empty)

case Event(c: SendPayment, WaitingForRequest) =>
span.tag(Tags.TargetNodeId, c.targetNodeId.toString())
Expand All @@ -96,17 +96,17 @@ class PaymentLifecycle(nodeParams: NodeParams, cfg: SendPaymentConfig, router: A
if (cfg.storeInDb) {
paymentsDb.addOutgoingPayment(OutgoingPayment(id, cfg.parentId, cfg.externalId, paymentHash, PaymentType.Standard, c.finalPayload.amount, cfg.recipientAmount, cfg.recipientNodeId, System.currentTimeMillis, cfg.paymentRequest, OutgoingPaymentStatus.Pending))
}
goto(WAITING_FOR_ROUTE) using WaitingForRoute(sender, c, Nil, Set.empty, Set.empty)
goto(WAITING_FOR_ROUTE) using WaitingForRoute(sender, c, Nil, Ignore.empty)
}

when(WAITING_FOR_ROUTE) {
case Event(RouteResponse(route +: _), WaitingForRoute(s, c, failures, ignoreNodes, ignoreChannels)) =>
case Event(RouteResponse(route +: _), WaitingForRoute(s, c, failures, ignore)) =>
log.info(s"route found: attempt=${failures.size + 1}/${c.maxAttempts} route=${route.hops.map(_.nextNodeId).mkString("->")} channels=${route.hops.map(_.lastUpdate.shortChannelId).mkString("->")}")
val (cmd, sharedSecrets) = OutgoingPacket.buildCommand(cfg.upstream, paymentHash, route.hops, c.finalPayload)
register ! Register.ForwardShortId(route.hops.head.lastUpdate.shortChannelId, cmd)
goto(WAITING_FOR_PAYMENT_COMPLETE) using WaitingForComplete(s, c, cmd, failures, sharedSecrets, ignoreNodes, ignoreChannels, route)
goto(WAITING_FOR_PAYMENT_COMPLETE) using WaitingForComplete(s, c, cmd, failures, sharedSecrets, ignore, route)

case Event(Status.Failure(t), WaitingForRoute(s, _, failures, _, _)) =>
case Event(Status.Failure(t), WaitingForRoute(s, _, failures, _)) =>
log.warning("router error: {}", t.getMessage)
Metrics.PaymentError.withTag(Tags.Failure, Tags.FailureType(LocalFailure(Nil, t))).increment()
onFailure(s, PaymentFailed(id, paymentHash, failures :+ LocalFailure(Nil, t)))
Expand All @@ -116,7 +116,7 @@ class PaymentLifecycle(nodeParams: NodeParams, cfg: SendPaymentConfig, router: A
when(WAITING_FOR_PAYMENT_COMPLETE) {
case Event(ChannelCommandResponse.Ok, _) => stay

case Event(fulfill: Relayer.ForwardFulfill, WaitingForComplete(s, c, cmd, failures, _, _, _, route)) =>
case Event(fulfill: Relayer.ForwardFulfill, WaitingForComplete(s, c, cmd, failures, _, _, route)) =>
Metrics.PaymentAttempt.withTag(Tags.MultiPart, value = false).record(failures.size + 1)
val p = PartialPayment(id, c.finalPayload.amount, cmd.amount - c.finalPayload.amount, fulfill.htlc.channelId, Some(cfg.fullRoute(route)))
onSuccess(s, cfg.createPaymentSent(fulfill.paymentPreimage, p :: Nil))
Expand All @@ -130,7 +130,7 @@ class PaymentLifecycle(nodeParams: NodeParams, cfg: SendPaymentConfig, router: A
}
stay

case Event(fail: UpdateFailHtlc, data@WaitingForComplete(s, c, _, failures, sharedSecrets, ignoreNodes, ignoreChannels, route)) =>
case Event(fail: UpdateFailHtlc, data@WaitingForComplete(s, c, _, failures, sharedSecrets, ignore, route)) =>
(Sphinx.FailurePacket.decrypt(fail.reason, sharedSecrets) match {
case success@Success(e) =>
Metrics.PaymentError.withTag(Tags.Failure, Tags.FailureType(RemoteFailure(Nil, e))).increment()
Expand Down Expand Up @@ -167,7 +167,7 @@ class PaymentLifecycle(nodeParams: NodeParams, cfg: SendPaymentConfig, router: A
retry(failure, data)
case Success(e@Sphinx.DecryptedFailurePacket(nodeId, failureMessage: Update)) =>
log.info(s"received 'Update' type error message from nodeId=$nodeId, retrying payment (failure=$failureMessage)")
val ignoreNodes1 = if (Announcements.checkSig(failureMessage.update, nodeId)) {
val ignore1 = if (Announcements.checkSig(failureMessage.update, nodeId)) {
route.getChannelUpdateForNode(nodeId) match {
case Some(u) if u.shortChannelId != failureMessage.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
Expand Down Expand Up @@ -200,15 +200,15 @@ class PaymentLifecycle(nodeParams: NodeParams, cfg: SendPaymentConfig, router: A
case extraHop => extraHop
})
// let's try again, router will have updated its state
router ! RouteRequest(nodeParams.nodeId, c.targetNodeId, c.finalPayload.amount, c.getMaxFee(nodeParams), assistedRoutes1, ignoreNodes, ignoreChannels, c.routeParams)
ignoreNodes
router ! RouteRequest(nodeParams.nodeId, c.targetNodeId, c.finalPayload.amount, c.getMaxFee(nodeParams), assistedRoutes1, ignore, c.routeParams)
ignore
} else {
// this node is fishy, it gave us a bad sig!! let's filter it out
log.warning(s"got bad signature from node=$nodeId update=${failureMessage.update}")
router ! RouteRequest(nodeParams.nodeId, c.targetNodeId, c.finalPayload.amount, c.getMaxFee(nodeParams), c.assistedRoutes, ignoreNodes + nodeId, ignoreChannels, c.routeParams)
ignoreNodes + nodeId
router ! RouteRequest(nodeParams.nodeId, c.targetNodeId, c.finalPayload.amount, c.getMaxFee(nodeParams), c.assistedRoutes, ignore + nodeId, c.routeParams)
ignore + nodeId
}
goto(WAITING_FOR_ROUTE) using WaitingForRoute(s, c, failures :+ RemoteFailure(cfg.fullRoute(route), e), ignoreNodes1, ignoreChannels)
goto(WAITING_FOR_ROUTE) using WaitingForRoute(s, c, failures :+ RemoteFailure(cfg.fullRoute(route), e), ignore1)
case Success(e@Sphinx.DecryptedFailurePacket(nodeId, failureMessage)) =>
log.info(s"received an error message from nodeId=$nodeId, trying to use a different channel (failure=$failureMessage)")
val failure = RemoteFailure(cfg.fullRoute(route), e)
Expand All @@ -224,7 +224,7 @@ class PaymentLifecycle(nodeParams: NodeParams, cfg: SendPaymentConfig, router: A
self ! Status.Failure(new RuntimeException("first hop returned an UpdateFailMalformedHtlc message"))
stay

case Event(Status.Failure(t), data@WaitingForComplete(s, c, _, failures, _, _, _, hops)) =>
case Event(Status.Failure(t), data@WaitingForComplete(s, c, _, failures, _, _, hops)) =>
Metrics.PaymentError.withTag(Tags.Failure, Tags.FailureType(LocalFailure(cfg.fullRoute(hops), t))).increment()
val isFatal = failures.size + 1 >= c.maxAttempts || // retries exhausted
t.isInstanceOf[HtlcsTimedoutDownstream] // htlc timed out so retrying won't help, we need to re-compute cltvs
Expand Down Expand Up @@ -261,9 +261,9 @@ class PaymentLifecycle(nodeParams: NodeParams, cfg: SendPaymentConfig, router: A
}

private def retry(failure: PaymentFailure, data: WaitingForComplete): FSM.State[PaymentLifecycle.State, PaymentLifecycle.Data] = {
val (ignoreNodes1, ignoreChannels1) = PaymentFailure.updateIgnored(failure, data.ignoreNodes, data.ignoreChannels)
router ! RouteRequest(nodeParams.nodeId, data.c.targetNodeId, data.c.finalPayload.amount, data.c.getMaxFee(nodeParams), data.c.assistedRoutes, ignoreNodes1, ignoreChannels1, data.c.routeParams)
goto(WAITING_FOR_ROUTE) using WaitingForRoute(data.sender, data.c, data.failures :+ failure, ignoreNodes1, ignoreChannels1)
val ignore1 = PaymentFailure.updateIgnored(failure, data.ignore)
router ! RouteRequest(nodeParams.nodeId, data.c.targetNodeId, data.c.finalPayload.amount, data.c.getMaxFee(nodeParams), data.c.assistedRoutes, ignore1, data.c.routeParams)
goto(WAITING_FOR_ROUTE) using WaitingForRoute(data.sender, data.c, data.failures :+ failure, ignore1)
}

private def myStop(): State = {
Expand Down Expand Up @@ -343,8 +343,8 @@ object PaymentLifecycle {
// @formatter:off
sealed trait Data
case object WaitingForRequest extends Data
case class WaitingForRoute(sender: ActorRef, c: SendPayment, failures: Seq[PaymentFailure], ignoreNodes: Set[PublicKey], ignoreChannels: Set[ChannelDesc]) extends Data
case class WaitingForComplete(sender: ActorRef, c: SendPayment, cmd: CMD_ADD_HTLC, failures: Seq[PaymentFailure], sharedSecrets: Seq[(ByteVector32, PublicKey)], ignoreNodes: Set[PublicKey], ignoreChannels: Set[ChannelDesc], route: Route) extends Data
case class WaitingForRoute(sender: ActorRef, c: SendPayment, failures: Seq[PaymentFailure], ignore: Ignore) extends Data
case class WaitingForComplete(sender: ActorRef, c: SendPayment, cmd: CMD_ADD_HTLC, failures: Seq[PaymentFailure], sharedSecrets: Seq[(ByteVector32, PublicKey)], ignore: Ignore, route: Route) extends Data

sealed trait State
case object WAITING_FOR_REQUEST extends State
Expand Down
Loading

0 comments on commit 4065eae

Please sign in to comment.