diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/router/Graph.scala b/eclair-core/src/main/scala/fr/acinq/eclair/router/Graph.scala index 20a2a8e00e..82551ae3c8 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/router/Graph.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/router/Graph.scala @@ -310,13 +310,21 @@ object Graph { } /** + * This forces channel_update(s) with fees=0 to have a minimum of 1msat for the baseFee. Note that + * the update is not being modified and the result of the route computation will still have the update + * with fees=0 which is what will be used to build the onion. * * @param edge the edge for which we want to compute the weight * @param amountWithFees the value that this edge will have to carry along * @return the new amount updated with the necessary fees for this edge */ private def edgeFeeCost(edge: GraphEdge, amountWithFees: Long): Long = { - amountWithFees + nodeFee(edge.update.feeBaseMsat, edge.update.feeProportionalMillionths, amountWithFees) + if(edgeHasZeroFee(edge)) amountWithFees + nodeFee(baseMsat = 1, proportional = 0, amountWithFees) + else amountWithFees + nodeFee(edge.update.feeBaseMsat, edge.update.feeProportionalMillionths, amountWithFees) + } + + private def edgeHasZeroFee(edge: GraphEdge): Boolean = { + edge.update.feeBaseMsat == 0 && edge.update.feeProportionalMillionths == 0 } // Calculates the total cost of a path (amount + fees), direct channels with the source will have a cost of 0 (pay no fees) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentLifecycleSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentLifecycleSpec.scala index ecfe1378a5..45f8597bbf 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentLifecycleSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentLifecycleSpec.scala @@ -19,16 +19,20 @@ package fr.acinq.eclair.payment import akka.actor.FSM.{CurrentState, SubscribeTransitionCallBack, Transition} import akka.actor.Status import akka.testkit.{TestFSMRef, TestProbe} -import fr.acinq.bitcoin.{Block, ByteVector32, MilliSatoshi} +import fr.acinq.bitcoin.Script.{pay2wsh, write} +import fr.acinq.bitcoin.{Block, ByteVector32, MilliSatoshi, Satoshi, Transaction, TxOut} +import fr.acinq.eclair.blockchain.{UtxoStatus, ValidateRequest, ValidateResult, WatchSpentBasic} import fr.acinq.eclair.channel.Register.ForwardShortId import fr.acinq.eclair.channel.{AddHtlcFailed, ChannelUnavailable} import fr.acinq.eclair.crypto.Sphinx import fr.acinq.eclair.crypto.Sphinx.ErrorPacket +import fr.acinq.eclair.io.Peer.PeerRoutingMessage import fr.acinq.eclair.payment.PaymentLifecycle._ -import fr.acinq.eclair.router.Announcements.makeChannelUpdate +import fr.acinq.eclair.router.Announcements.{makeChannelUpdate, makeNodeAnnouncement} import fr.acinq.eclair.router._ +import fr.acinq.eclair.transactions.Scripts import fr.acinq.eclair.wire._ -import fr.acinq.eclair.{Globals, randomBytes32} +import fr.acinq.eclair.{Globals, ShortChannelId, randomBytes32} /** * Created by PM on 29/08/2016. @@ -320,6 +324,60 @@ class PaymentLifecycleSpec extends BaseRouterSpec { assert(fee === MilliSatoshi(paymentOK.amountMsat - request.amountMsat)) } + test("payment succeeded to a channel with fees=0") { fixture => + import fixture._ + import fr.acinq.eclair.randomKey + + // the network will be a --(1)--> b ---(2)--> c --(3)--> d and e --(4)--> f (we are a) and b -> g has fees=0 + // \ + // \--(5)--> g + + val (priv_g, priv_funding_g) = (randomKey, randomKey) + val (g, funding_g) = (priv_g.publicKey, priv_funding_g.publicKey) + val ann_g = makeNodeAnnouncement(priv_g, "node-G", Color(-30, 10, -50), Nil) + val channelId_bg = ShortChannelId(420000, 5, 0) + val chan_bg = channelAnnouncement(channelId_bg, priv_b, priv_g, priv_funding_b, priv_funding_g) + val channelUpdate_bg = makeChannelUpdate(Block.RegtestGenesisBlock.hash, priv_b, g, channelId_bg, cltvExpiryDelta = 9, htlcMinimumMsat = 0, feeBaseMsat = 0, feeProportionalMillionths = 0, htlcMaximumMsat = 500000000L) + val channelUpdate_gb = makeChannelUpdate(Block.RegtestGenesisBlock.hash, priv_g, b, channelId_bg, cltvExpiryDelta = 9, htlcMinimumMsat = 0, feeBaseMsat = 10, feeProportionalMillionths = 8, htlcMaximumMsat = 500000000L) + assert(Router.getDesc(channelUpdate_bg, chan_bg) === ChannelDesc(chan_bg.shortChannelId, priv_b.publicKey, priv_g.publicKey)) + router ! PeerRoutingMessage(null, remoteNodeId, chan_bg) + router ! PeerRoutingMessage(null, remoteNodeId, ann_g) + router ! PeerRoutingMessage(null, remoteNodeId, channelUpdate_bg) + router ! PeerRoutingMessage(null, remoteNodeId, channelUpdate_gb) + watcher.expectMsg(ValidateRequest(chan_bg)) + watcher.send(router, ValidateResult(chan_bg, Right((Transaction(version = 0, txIn = Nil, txOut = TxOut(Satoshi(1000000), write(pay2wsh(Scripts.multiSig2of2(funding_b, funding_g)))) :: Nil, lockTime = 0), UtxoStatus.Unspent)))) + watcher.expectMsgType[WatchSpentBasic] + + // actual test begins + val paymentFSM = system.actorOf(PaymentLifecycle.props(a, router, TestProbe().ref)) + val monitor = TestProbe() + val sender = TestProbe() + val eventListener = TestProbe() + system.eventStream.subscribe(eventListener.ref, classOf[PaymentEvent]) + + paymentFSM ! SubscribeTransitionCallBack(monitor.ref) + val CurrentState(_, WAITING_FOR_REQUEST) = monitor.expectMsgClass(classOf[CurrentState[_]]) + + // we send a payment to G which is just after the + val request = SendPayment(defaultAmountMsat, defaultPaymentHash, g) + sender.send(paymentFSM, request) + + // the route will be A -> B -> G where B -> G has a channel_update with fees=0 + val Transition(_, WAITING_FOR_REQUEST, WAITING_FOR_ROUTE) = monitor.expectMsgClass(classOf[Transition[_]]) + val Transition(_, WAITING_FOR_ROUTE, WAITING_FOR_PAYMENT_COMPLETE) = monitor.expectMsgClass(classOf[Transition[_]]) + + sender.send(paymentFSM, UpdateFulfillHtlc(ByteVector32.Zeroes, 0, defaultPaymentHash)) + + val paymentOK = sender.expectMsgType[PaymentSucceeded] + val PaymentSent(MilliSatoshi(request.amountMsat), fee, request.paymentHash, paymentOK.paymentPreimage, _, _) = eventListener.expectMsgType[PaymentSent] + + // during the route computation the fees were treated as if they were 1msat but when sending the onion we actually put zero + // NB: A -> B doesn't pay fees because it's our direct neighbor + // NB: B -> G doesn't asks for fees at all + assert(fee === MilliSatoshi(0)) + assert(fee === MilliSatoshi(paymentOK.amountMsat - request.amountMsat)) + } + test("filter errors properly") { fixture => val failures = LocalFailure(RouteNotFound) :: RemoteFailure(Hop(a, b, channelUpdate_ab) :: Nil, ErrorPacket(a, TemporaryNodeFailure)) :: LocalFailure(AddHtlcFailed(ByteVector32.Zeroes, ByteVector32.Zeroes, ChannelUnavailable(ByteVector32.Zeroes), Local(None), None, None)) :: LocalFailure(RouteNotFound) :: Nil val filtered = PaymentLifecycle.transformForUser(failures) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/router/RouteCalculationSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/router/RouteCalculationSpec.scala index 2169a9062a..4261faa1ab 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/router/RouteCalculationSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/router/RouteCalculationSpec.scala @@ -590,6 +590,24 @@ class RouteCalculationSpec extends FunSuite { assert(route1.map(hops2Ids) === Success(1 :: 2 :: 4 :: 5 :: Nil)) } + test("ensure the route calculation terminates correctly when selecting 0-fees edges") { + + // the graph contains a possible 0-cost path that goes back on its steps ( e -> f, f -> e ) + val updates = List( + makeUpdate(1L, a, b, 10, 10), // a -> b + makeUpdate(2L, b, c, 10, 10), + makeUpdate(4L, c, d, 10, 10), + makeUpdate(3L, b, e, 0, 0), // b -> e + makeUpdate(6L, e, f, 0, 0), // e -> f + makeUpdate(6L, f, e, 0, 0), // e <- f + makeUpdate(5L, e, d, 0, 0) // e -> d + ).toMap + + val g = makeGraph(updates) + + val route1 = Router.findRoute(g, a, d, DEFAULT_AMOUNT_MSAT, numRoutes = 1, routeParams = DEFAULT_ROUTE_PARAMS) + assert(route1.map(hops2Ids) === Success(1 :: 3 :: 5 :: Nil)) + } /** *