From b81bf20d752bdf3c473841890d7dc55a22d5d0ff Mon Sep 17 00:00:00 2001 From: Anton Kumaigorodski Date: Thu, 16 Jan 2020 14:16:34 +0200 Subject: [PATCH 1/5] Find htlc by id method (#1266) --- .../fr/acinq/eclair/channel/Commitments.scala | 18 ++++++++---------- .../eclair/transactions/CommitmentSpec.scala | 7 +++++-- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala index b5677dfd6..dfd055864 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala @@ -251,14 +251,12 @@ object Commitments { commitments1 } - def getHtlcCrossSigned(commitments: Commitments, directionRelativeToLocal: Direction, htlcId: Long): Option[UpdateAddHtlc] = { - val remoteSigned = commitments.localCommit.spec.htlcs.find(htlc => htlc.direction == directionRelativeToLocal && htlc.add.id == htlcId) - val localSigned = commitments.remoteNextCommitInfo.left.toOption.map(_.nextRemoteCommit).getOrElse(commitments.remoteCommit) - .spec.htlcs.find(htlc => htlc.direction == directionRelativeToLocal.opposite && htlc.add.id == htlcId) - for { - htlc_out <- remoteSigned - htlc_in <- localSigned - } yield htlc_in.add + def getHtlcCrossSigned(commitments: Commitments, directionRelativeToLocal: Direction, htlcId: Long): Option[UpdateAddHtlc] = for { + localSigned <- commitments.remoteNextCommitInfo.left.toOption.map(_.nextRemoteCommit).getOrElse(commitments.remoteCommit).spec.findHtlcById(htlcId, directionRelativeToLocal.opposite) + remoteSigned <- commitments.localCommit.spec.findHtlcById(htlcId, directionRelativeToLocal) + } yield { + require(localSigned.add == remoteSigned.add) + localSigned.add } def sendFulfill(commitments: Commitments, cmd: CMD_FULFILL_HTLC): (Commitments, UpdateFulfillHtlc) = @@ -532,12 +530,12 @@ object Commitments { // same for fails: we need to make sure that they are in neither commitment before propagating the fail upstream case fail: UpdateFailHtlc => val origin = commitments.originChannels(fail.id) - val add = commitments.remoteCommit.spec.htlcs.find(p => p.direction == IN && p.add.id == fail.id).map(_.add).get + val add = commitments.remoteCommit.spec.findHtlcById(fail.id, IN).map(_.add).get Relayer.ForwardFail(fail, origin, add) // same as above case fail: UpdateFailMalformedHtlc => val origin = commitments.originChannels(fail.id) - val add = commitments.remoteCommit.spec.htlcs.find(p => p.direction == IN && p.add.id == fail.id).map(_.add).get + val add = commitments.remoteCommit.spec.findHtlcById(fail.id, IN).map(_.add).get Relayer.ForwardFailMalformed(fail, origin, add) } // the outgoing following htlcs have been completed (fulfilled or failed) when we received this revocation diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/transactions/CommitmentSpec.scala b/eclair-core/src/main/scala/fr/acinq/eclair/transactions/CommitmentSpec.scala index f1d47cea3..cbd4a9f37 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/transactions/CommitmentSpec.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/transactions/CommitmentSpec.scala @@ -32,6 +32,9 @@ case object OUT extends Direction { def opposite = IN } case class DirectedHtlc(direction: Direction, add: UpdateAddHtlc) final case class CommitmentSpec(htlcs: Set[DirectedHtlc], feeratePerKw: Long, toLocal: MilliSatoshi, toRemote: MilliSatoshi) { + + def findHtlcById(id: Long, direction: Direction): Option[DirectedHtlc] = htlcs.find(htlc => htlc.add.id == id && htlc.direction == direction) + val totalFunds = toLocal + toRemote + htlcs.toSeq.map(_.add.amountMsat).sum } @@ -51,7 +54,7 @@ object CommitmentSpec { // OUT means we are sending an UpdateFulfillHtlc message which means that we are fulfilling an HTLC that they sent def fulfillHtlc(spec: CommitmentSpec, direction: Direction, htlcId: Long): CommitmentSpec = { - spec.htlcs.find(htlc => htlc.direction != direction && htlc.add.id == htlcId) match { + spec.findHtlcById(htlcId, direction.opposite) match { case Some(htlc) if direction == OUT => spec.copy(toLocal = spec.toLocal + htlc.add.amountMsat, htlcs = spec.htlcs - htlc) case Some(htlc) if direction == IN => spec.copy(toRemote = spec.toRemote + htlc.add.amountMsat, htlcs = spec.htlcs - htlc) case None => throw new RuntimeException(s"cannot find htlc id=$htlcId") @@ -60,7 +63,7 @@ object CommitmentSpec { // OUT means we are sending an UpdateFailHtlc message which means that we are failing an HTLC that they sent def failHtlc(spec: CommitmentSpec, direction: Direction, htlcId: Long): CommitmentSpec = { - spec.htlcs.find(htlc => htlc.direction != direction && htlc.add.id == htlcId) match { + spec.findHtlcById(htlcId, direction.opposite) match { case Some(htlc) if direction == OUT => spec.copy(toRemote = spec.toRemote + htlc.add.amountMsat, htlcs = spec.htlcs - htlc) case Some(htlc) if direction == IN => spec.copy(toLocal = spec.toLocal + htlc.add.amountMsat, htlcs = spec.htlcs - htlc) case None => throw new RuntimeException(s"cannot find htlc id=$htlcId") From cb3ed7cde6e3de2e089535b25245ecd4f23d6bec Mon Sep 17 00:00:00 2001 From: Bastien Teinturier <31281497+t-bast@users.noreply.github.com> Date: Fri, 17 Jan 2020 13:39:53 +0100 Subject: [PATCH 2/5] Improve error message when invalid funding tx (#1282) Closes #1281 --- .../src/main/scala/fr/acinq/eclair/channel/Channel.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Channel.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Channel.scala index 2dff44edf..416f46cb7 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Channel.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Channel.scala @@ -547,7 +547,7 @@ class Channel(val nodeParams: NodeParams, val wallet: EclairWallet, remoteNodeId val shortChannelId = ShortChannelId(blockHeight, txIndex, commitments.commitInput.outPoint.index.toInt) goto(WAIT_FOR_FUNDING_LOCKED) using DATA_WAIT_FOR_FUNDING_LOCKED(commitments, shortChannelId, fundingLocked) storing() sending fundingLocked case Failure(t) => - log.error(t, "") + log.error(t, s"rejecting channel with invalid funding tx: ${fundingTx.bin}") goto(CLOSED) } From aa137b7da6c694443455a368273d8684355c478d Mon Sep 17 00:00:00 2001 From: Bastien Teinturier <31281497+t-bast@users.noreply.github.com> Date: Fri, 17 Jan 2020 15:11:26 +0100 Subject: [PATCH 3/5] MPP: allow using unannounced channels (#1283) Otherwise eclair-mobile can't pay using MPP. This heuristic was only here to help Trampoline nodes with a lot of channels relay using MPP, but we disabled that in #1271 anyway. We will reactivate Trampoline-MPP once split is done inside the router. --- .../send/MultiPartPaymentLifecycle.scala | 5 ++--- .../payment/MultiPartPaymentLifecycleSpec.scala | 17 ----------------- 2 files changed, 2 insertions(+), 20 deletions(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/payment/send/MultiPartPaymentLifecycle.scala b/eclair-core/src/main/scala/fr/acinq/eclair/payment/send/MultiPartPaymentLifecycle.scala index b3f1f7028..514ce37d1 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/payment/send/MultiPartPaymentLifecycle.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/payment/send/MultiPartPaymentLifecycle.scala @@ -509,12 +509,11 @@ object MultiPartPaymentLifecycle { }) // Otherwise we need to split the amount based on network statistics and pessimistic fees estimates. - // We filter out unannounced channels: they are very likely leading to a non-routing node. // Note that this will be handled more gracefully once this logic is migrated inside the router. val channels = if (randomize) { - Random.shuffle(localChannels.filter(p => p.commitments.announceChannel && p.nextNodeId != request.targetNodeId)) + Random.shuffle(localChannels.filter(p => p.nextNodeId != request.targetNodeId)) } else { - localChannels.filter(p => p.commitments.announceChannel && p.nextNodeId != request.targetNodeId).sortBy(_.commitments.availableBalanceForSend) + localChannels.filter(p => p.nextNodeId != request.targetNodeId).sortBy(_.commitments.availableBalanceForSend) } val remotePayments = split(toSend - directPayments.map(_.finalPayload.amount).sum, Seq.empty, channels, (remaining: MilliSatoshi, channel: OutgoingChannel) => { // We re-generate a split threshold for each channel to randomize the amounts. diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/payment/MultiPartPaymentLifecycleSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/payment/MultiPartPaymentLifecycleSpec.scala index a4c6d7d9b..0a314376f 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/payment/MultiPartPaymentLifecycleSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/payment/MultiPartPaymentLifecycleSpec.scala @@ -310,23 +310,6 @@ class MultiPartPaymentLifecycleSpec extends TestKit(ActorSystem("test")) with fi assert(result.amount === payment.totalAmount) } - test("skip unannounced channels when sending to remote node") { f => - import f._ - - // The channels to b are not announced: they should be ignored so the payment should fail. - val channels = OutgoingChannels(Seq( - OutgoingChannel(b, channelUpdate_ab_1.copy(channelFlags = ChannelFlags.Empty), makeCommitments(1000 * 1000 msat, 10, announceChannel = false)), - OutgoingChannel(b, channelUpdate_ab_2.copy(channelFlags = ChannelFlags.Empty), makeCommitments(1500 * 1000 msat, 10, announceChannel = false)), - OutgoingChannel(c, channelUpdate_ac_1, makeCommitments(500 * 1000 msat, 10)) - )) - val payment = SendMultiPartPayment(paymentHash, randomBytes32, e, 1200 * 1000 msat, expiry, 3) - initPayment(f, payment, emptyStats.copy(capacity = Stats(Seq(1000), d => Satoshi(d.toLong))), channels) - - val result = sender.expectMsgType[PaymentFailed] - assert(result.id === paymentId) - assert(result.paymentHash === paymentHash) - } - test("retry after error") { f => import f._ val payment = SendMultiPartPayment(paymentHash, randomBytes32, e, 3000 * 1000 msat, expiry, 3) From 5ef4edb348ae7182ad9aab76ffb8e8982b8a1c24 Mon Sep 17 00:00:00 2001 From: dpad85 <5765435+dpad85@users.noreply.github.com> Date: Fri, 10 Jan 2020 14:56:21 +0100 Subject: [PATCH 4/5] Remove feature graph validation in payment request Due to changes in the features system (#1253), feature graph validation would fail with legacy 1.0.1 Phoenix wallet. This check should be disabled as long as there are 1.0.1 Phoenix wallet in the wild. (cherry picked from commit 9f8f8e43ac70037d909cdfc9e510c253da14a9b1) --- .../scala/fr/acinq/eclair/payment/PaymentRequest.scala | 4 ++-- .../fr/acinq/eclair/payment/PaymentRequestSpec.scala | 8 +++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentRequest.scala b/eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentRequest.scala index 69ad5d337..d77763ac3 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentRequest.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentRequest.scala @@ -45,8 +45,8 @@ case class PaymentRequest(prefix: String, amount: Option[MilliSatoshi], timestam amount.foreach(a => require(a > 0.msat, s"amount is not valid")) require(tags.collect { case _: PaymentRequest.PaymentHash => }.size == 1, "there must be exactly one payment hash tag") require(tags.collect { case PaymentRequest.Description(_) | PaymentRequest.DescriptionHash(_) => }.size == 1, "there must be exactly one description tag or one description hash tag") - private val featuresErr = validateFeatureGraph(features.bitmask) - require(featuresErr.isEmpty, featuresErr.map(_.message)) + // Phoenix special case: we do not check the graph to be able to pay legacy 1.0.1 Phoenix wallets. + // todo: add the check back once most 1.0.1 phoenix wallet have been upgraded if (features.allowPaymentSecret) { require(tags.collect { case _: PaymentRequest.PaymentSecret => }.size == 1, "there must be exactly one payment secret tag when feature bit is set") } diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentRequestSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentRequestSpec.scala index a81009bb3..23d8ccedd 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentRequestSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentRequestSpec.scala @@ -372,10 +372,12 @@ class PaymentRequestSpec extends FunSuite { PaymentRequest.read("lnbc1230p1pwljzn3pp5qyqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqdq52dhk6efqd9h8vmmfvdjs9qypqsqylvwhf7xlpy6xpecsnpcjjuuslmzzgeyv90mh7k7vs88k2dkxgrkt75qyfjv5ckygw206re7spga5zfd4agtdvtktxh5pkjzhn9dq2cqz9upw7") ) + // Phoenix special case: ignore payment secret invoice validation + // todo: reinstate this assert once legacy 1.0.1 Phoenix wallet have been upgraded // A multi-part invoice must use a payment secret. - assertThrows[IllegalArgumentException]( - PaymentRequest(Block.LivenetGenesisBlock.hash, Some(123 msat), ByteVector32.One, priv, "MPP without secrets", features = Some(Features(BasicMultiPartPayment.optional, VariableLengthOnion.optional))) - ) + // assertThrows[IllegalArgumentException]( + // PaymentRequest(Block.LivenetGenesisBlock.hash, Some(123 msat), ByteVector32.One, priv, "MPP without secrets", features = Some(Features(BasicMultiPartPayment.optional, VariableLengthOnion.optional))) + // ) } test("trampoline") { From 25b9505529ac7452b87933e1a7fb886d946f17a7 Mon Sep 17 00:00:00 2001 From: dpad85 <5765435+dpad85@users.noreply.github.com> Date: Thu, 16 Jan 2020 13:30:46 +0100 Subject: [PATCH 5/5] Release v0.3.5-android Note that this release jumps straight from 0.3.3 to 0.3.5 so that the android and android-phoenix branches have similar versions tags, which makes it easier to track. As such there is no 0.3.4-android version. --- eclair-core/pom.xml | 2 +- eclair-node/pom.xml | 2 +- pom.xml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/eclair-core/pom.xml b/eclair-core/pom.xml index 4a25e8768..c4b87e97a 100644 --- a/eclair-core/pom.xml +++ b/eclair-core/pom.xml @@ -21,7 +21,7 @@ fr.acinq.eclair eclair_2.11 - 0.3.4-android-SNAPSHOT + 0.3.5-android eclair-core_2.11 diff --git a/eclair-node/pom.xml b/eclair-node/pom.xml index 16e9cc9b7..9dabb7beb 100644 --- a/eclair-node/pom.xml +++ b/eclair-node/pom.xml @@ -21,7 +21,7 @@ fr.acinq.eclair eclair_2.11 - 0.3.4-android-SNAPSHOT + 0.3.5-android eclair-node_2.11 diff --git a/pom.xml b/pom.xml index 407461626..296f7e9ef 100644 --- a/pom.xml +++ b/pom.xml @@ -20,7 +20,7 @@ fr.acinq.eclair eclair_2.11 - 0.3.4-android-SNAPSHOT + 0.3.5-android pom