mirror of
https://github.com/ACINQ/eclair.git
synced 2025-02-24 06:47:46 +01:00
Avoid incoherent payment request features. (#1222)
MPP implies payment secret. Avoid raising exceptions in PaymentInitiator: validate invoice instead of using a require. This way senders always get a response.
This commit is contained in:
Bastien Teinturier
GitHub
parent
631336ed84
commit
d1342eb072
6 changed files with 85 additions and 54 deletions
|
|
@ -18,6 +18,7 @@ package fr.acinq.eclair.payment
|
|||
|
||||
import fr.acinq.bitcoin.Crypto.{PrivateKey, PublicKey}
|
||||
import fr.acinq.bitcoin.{Base58, Base58Check, Bech32, Block, ByteVector32, ByteVector64, Crypto}
|
||||
import fr.acinq.eclair.Features._
|
||||
import fr.acinq.eclair.payment.PaymentRequest._
|
||||
import fr.acinq.eclair.{CltvExpiryDelta, LongToBtcAmount, MilliSatoshi, ShortChannelId, randomBytes32}
|
||||
import scodec.Codec
|
||||
|
|
@ -43,8 +44,13 @@ 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.PaymentSecret => }.size <= 1, "there must be at most one payment secret tag")
|
||||
require(tags.collect { case PaymentRequest.Description(_) | PaymentRequest.DescriptionHash(_) => }.size == 1, "there must be exactly one description tag or one description hash tag")
|
||||
if (features.allowMultiPart) {
|
||||
require(features.allowPaymentSecret, "there must be a payment secret when using multi-part")
|
||||
}
|
||||
if (features.allowPaymentSecret) {
|
||||
require(tags.collect { case _: PaymentRequest.PaymentSecret => }.size == 1, "there must be exactly one payment secret tag when feature bit is set")
|
||||
}
|
||||
|
||||
/**
|
||||
* @return the payment hash
|
||||
|
|
@ -114,8 +120,6 @@ case class PaymentRequest(prefix: String, amount: Option[MilliSatoshi], timestam
|
|||
|
||||
object PaymentRequest {
|
||||
|
||||
import fr.acinq.eclair.Features.PAYMENT_SECRET_OPTIONAL
|
||||
|
||||
val DEFAULT_EXPIRY_SECONDS = 3600
|
||||
|
||||
val prefixes = Map(
|
||||
|
|
@ -129,20 +133,24 @@ object PaymentRequest {
|
|||
features: Option[Features] = Some(Features(PAYMENT_SECRET_OPTIONAL))): PaymentRequest = {
|
||||
|
||||
val prefix = prefixes(chainHash)
|
||||
val tags = {
|
||||
val defaultTags = List(
|
||||
Some(PaymentHash(paymentHash)),
|
||||
Some(Description(description)),
|
||||
fallbackAddress.map(FallbackAddress(_)),
|
||||
expirySeconds.map(Expiry(_)),
|
||||
features).flatten
|
||||
val paymentSecretTag = if (features.exists(_.allowPaymentSecret)) PaymentSecret(randomBytes32) :: Nil else Nil
|
||||
val routingInfoTags = extraHops.map(RoutingInfo)
|
||||
defaultTags ++ paymentSecretTag ++ routingInfoTags
|
||||
}
|
||||
|
||||
PaymentRequest(
|
||||
prefix = prefix,
|
||||
amount = amount,
|
||||
timestamp = timestamp,
|
||||
nodeId = privateKey.publicKey,
|
||||
tags = List(
|
||||
Some(PaymentHash(paymentHash)),
|
||||
Some(PaymentSecret(randomBytes32)),
|
||||
Some(Description(description)),
|
||||
fallbackAddress.map(FallbackAddress(_)),
|
||||
expirySeconds.map(Expiry(_)),
|
||||
features
|
||||
).flatten ++ extraHops.map(RoutingInfo),
|
||||
tags = tags,
|
||||
signature = ByteVector.empty)
|
||||
.sign(privateKey)
|
||||
}
|
||||
|
|
@ -321,11 +329,9 @@ object PaymentRequest {
|
|||
* Features supported or required for receiving this payment.
|
||||
*/
|
||||
case class Features(bitmask: BitVector) extends TaggedField {
|
||||
|
||||
import fr.acinq.eclair.Features._
|
||||
|
||||
lazy val supported: Boolean = areSupported(bitmask)
|
||||
lazy val allowMultiPart: Boolean = hasFeature(bitmask, BASIC_MULTI_PART_PAYMENT_MANDATORY) || hasFeature(bitmask, BASIC_MULTI_PART_PAYMENT_OPTIONAL)
|
||||
lazy val allowPaymentSecret: Boolean = hasFeature(bitmask, PAYMENT_SECRET_MANDATORY) || hasFeature(bitmask, PAYMENT_SECRET_OPTIONAL)
|
||||
lazy val requirePaymentSecret: Boolean = hasFeature(bitmask, PAYMENT_SECRET_MANDATORY)
|
||||
lazy val allowTrampoline: Boolean = hasFeature(bitmask, TRAMPOLINE_PAYMENT_MANDATORY) || hasFeature(bitmask, TRAMPOLINE_PAYMENT_OPTIONAL)
|
||||
|
||||
|
|
|
|||
|
|
@ -47,12 +47,14 @@ class PaymentInitiator(nodeParams: NodeParams, router: ActorRef, relayer: ActorR
|
|||
val finalExpiry = r.finalExpiry(nodeParams.currentBlockHeight)
|
||||
r.paymentRequest match {
|
||||
case Some(invoice) if !invoice.features.supported =>
|
||||
sender ! PaymentFailed(paymentId, r.paymentHash, LocalFailure(new IllegalArgumentException(s"can't send payment: unknown invoice features (${r.paymentRequest.get.features})")) :: Nil)
|
||||
case Some(invoice) if invoice.features.allowMultiPart =>
|
||||
r.predefinedRoute match {
|
||||
case Nil => spawnMultiPartPaymentFsm(paymentCfg) forward SendMultiPartPayment(r.paymentHash, invoice.paymentSecret.get, r.targetNodeId, r.amount, finalExpiry, r.maxAttempts, r.assistedRoutes, r.routeParams)
|
||||
case hops => spawnPaymentFsm(paymentCfg) forward SendPaymentToRoute(r.paymentHash, hops, Onion.createMultiPartPayload(r.amount, invoice.amount.getOrElse(r.amount), finalExpiry, invoice.paymentSecret.get))
|
||||
sender ! PaymentFailed(paymentId, r.paymentHash, LocalFailure(new IllegalArgumentException(s"can't send payment: unknown invoice features (${invoice.features})")) :: Nil)
|
||||
case Some(invoice) if invoice.features.allowMultiPart => invoice.paymentSecret match {
|
||||
case Some(paymentSecret) => r.predefinedRoute match {
|
||||
case Nil => spawnMultiPartPaymentFsm(paymentCfg) forward SendMultiPartPayment(r.paymentHash, paymentSecret, r.targetNodeId, r.amount, finalExpiry, r.maxAttempts, r.assistedRoutes, r.routeParams)
|
||||
case hops => spawnPaymentFsm(paymentCfg) forward SendPaymentToRoute(r.paymentHash, hops, Onion.createMultiPartPayload(r.amount, invoice.amount.getOrElse(r.amount), finalExpiry, paymentSecret))
|
||||
}
|
||||
case None => sender ! PaymentFailed(paymentId, r.paymentHash, LocalFailure(new IllegalArgumentException("can't send payment: multi-part invoice is missing a payment secret")) :: Nil)
|
||||
}
|
||||
case _ =>
|
||||
val payFsm = spawnPaymentFsm(paymentCfg)
|
||||
// NB: we only generate legacy payment onions for now for maximum compatibility.
|
||||
|
|
@ -64,26 +66,30 @@ class PaymentInitiator(nodeParams: NodeParams, router: ActorRef, relayer: ActorR
|
|||
|
||||
case r: SendTrampolinePaymentRequest =>
|
||||
val paymentId = UUID.randomUUID()
|
||||
val paymentCfg = SendPaymentConfig(paymentId, paymentId, None, r.paymentRequest.paymentHash, r.trampolineNodeId, Some(r.paymentRequest), storeInDb = true, publishEvent = true)
|
||||
val trampolineRoute = Seq(
|
||||
NodeHop(nodeParams.nodeId, r.trampolineNodeId, nodeParams.expiryDeltaBlocks, 0 msat),
|
||||
NodeHop(r.trampolineNodeId, r.paymentRequest.nodeId, r.trampolineExpiryDelta, r.trampolineFees) // for now we only use a single trampoline hop
|
||||
)
|
||||
// We generate a random secret for this payment to avoid leaking the invoice secret to the first trampoline node.
|
||||
val trampolineSecret = randomBytes32
|
||||
val finalPayload = if (r.paymentRequest.features.allowMultiPart) {
|
||||
Onion.createMultiPartPayload(r.finalAmount, r.finalAmount, r.finalExpiry(nodeParams.currentBlockHeight), r.paymentRequest.paymentSecret.get)
|
||||
} else {
|
||||
Onion.createSinglePartPayload(r.finalAmount, r.finalExpiry(nodeParams.currentBlockHeight), r.paymentRequest.paymentSecret)
|
||||
}
|
||||
// We assume that the trampoline node supports multi-part payments (it should).
|
||||
val (trampolineAmount, trampolineExpiry, trampolineOnion) = if (r.paymentRequest.features.allowTrampoline) {
|
||||
OutgoingPacket.buildPacket(Sphinx.TrampolinePacket)(r.paymentRequest.paymentHash, trampolineRoute, finalPayload)
|
||||
} else {
|
||||
OutgoingPacket.buildTrampolineToLegacyPacket(r.paymentRequest, trampolineRoute, finalPayload)
|
||||
}
|
||||
spawnMultiPartPaymentFsm(paymentCfg) forward SendMultiPartPayment(r.paymentRequest.paymentHash, trampolineSecret, r.trampolineNodeId, trampolineAmount, trampolineExpiry, 1, r.paymentRequest.routingInfo, r.routeParams, Seq(OnionTlv.TrampolineOnion(trampolineOnion.packet)))
|
||||
sender ! paymentId
|
||||
if (!r.paymentRequest.features.allowTrampoline && r.paymentRequest.amount.isEmpty) {
|
||||
sender ! PaymentFailed(paymentId, r.paymentRequest.paymentHash, LocalFailure(new IllegalArgumentException("cannot pay a 0-value invoice via trampoline-to-legacy (trampoline may steal funds)")) :: Nil)
|
||||
} else {
|
||||
val paymentCfg = SendPaymentConfig(paymentId, paymentId, None, r.paymentRequest.paymentHash, r.trampolineNodeId, Some(r.paymentRequest), storeInDb = true, publishEvent = true)
|
||||
val finalPayload = if (r.paymentRequest.features.allowMultiPart) {
|
||||
Onion.createMultiPartPayload(r.finalAmount, r.finalAmount, r.finalExpiry(nodeParams.currentBlockHeight), r.paymentRequest.paymentSecret.get)
|
||||
} else {
|
||||
Onion.createSinglePartPayload(r.finalAmount, r.finalExpiry(nodeParams.currentBlockHeight), r.paymentRequest.paymentSecret)
|
||||
}
|
||||
val trampolineRoute = Seq(
|
||||
NodeHop(nodeParams.nodeId, r.trampolineNodeId, nodeParams.expiryDeltaBlocks, 0 msat),
|
||||
NodeHop(r.trampolineNodeId, r.paymentRequest.nodeId, r.trampolineExpiryDelta, r.trampolineFees) // for now we only use a single trampoline hop
|
||||
)
|
||||
// We assume that the trampoline node supports multi-part payments (it should).
|
||||
val (trampolineAmount, trampolineExpiry, trampolineOnion) = if (r.paymentRequest.features.allowTrampoline) {
|
||||
OutgoingPacket.buildPacket(Sphinx.TrampolinePacket)(r.paymentRequest.paymentHash, trampolineRoute, finalPayload)
|
||||
} else {
|
||||
OutgoingPacket.buildTrampolineToLegacyPacket(r.paymentRequest, trampolineRoute, finalPayload)
|
||||
}
|
||||
// We generate a random secret for this payment to avoid leaking the invoice secret to the first trampoline node.
|
||||
val trampolineSecret = randomBytes32
|
||||
spawnMultiPartPaymentFsm(paymentCfg) forward SendMultiPartPayment(r.paymentRequest.paymentHash, trampolineSecret, r.trampolineNodeId, trampolineAmount, trampolineExpiry, 1, r.paymentRequest.routingInfo, r.routeParams, Seq(OnionTlv.TrampolineOnion(trampolineOnion.packet)))
|
||||
}
|
||||
}
|
||||
|
||||
def spawnPaymentFsm(paymentCfg: SendPaymentConfig): ActorRef = context.actorOf(PaymentLifecycle.props(nodeParams, paymentCfg, router, register))
|
||||
|
|
|
|||
|
|
@ -271,7 +271,6 @@ object Onion {
|
|||
|
||||
/** Create a trampoline inner payload instructing the trampoline node to relay via a non-trampoline payment. */
|
||||
def createNodeRelayToNonTrampolinePayload(amount: MilliSatoshi, totalAmount: MilliSatoshi, expiry: CltvExpiry, targetNodeId: PublicKey, invoice: PaymentRequest): NodeRelayPayload = {
|
||||
require(invoice.amount.nonEmpty, "cannot pay a 0-value invoice via trampoline-to-legacy (trampoline may steal funds)")
|
||||
val tlvs = Seq[OnionTlv](AmountToForward(amount), OutgoingCltv(expiry), OutgoingNodeId(targetNodeId), InvoiceFeatures(invoice.features.bitmask.bytes), InvoiceRoutingInfo(invoice.routingInfo.toList.map(_.toList)))
|
||||
val tlvs2 = invoice.paymentSecret.map(s => tlvs :+ PaymentData(s, totalAmount)).getOrElse(tlvs)
|
||||
NodeRelayPayload(TlvStream(tlvs2))
|
||||
|
|
|
|||
|
|
@ -36,6 +36,8 @@ import fr.acinq.eclair.wire.{OnionCodecs, OnionTlv}
|
|||
import fr.acinq.eclair.{CltvExpiryDelta, LongToBtcAmount, NodeParams, TestConstants, randomKey}
|
||||
import org.scalatest.{Outcome, fixture}
|
||||
|
||||
import scala.concurrent.duration._
|
||||
|
||||
/**
|
||||
* Created by t-bast on 25/07/2019.
|
||||
*/
|
||||
|
|
@ -100,7 +102,7 @@ class PaymentInitiatorSpec extends TestKit(ActorSystem("test")) with fixture.Fun
|
|||
|
||||
test("forward multi-part payment") { f =>
|
||||
import f._
|
||||
val pr = PaymentRequest(Block.LivenetGenesisBlock.hash, Some(finalAmount), paymentHash, randomKey, "Some invoice", features = Some(Features(BASIC_MULTI_PART_PAYMENT_OPTIONAL)))
|
||||
val pr = PaymentRequest(Block.LivenetGenesisBlock.hash, Some(finalAmount), paymentHash, randomKey, "Some invoice", features = Some(Features(BASIC_MULTI_PART_PAYMENT_OPTIONAL, PAYMENT_SECRET_OPTIONAL)))
|
||||
val req = SendPaymentRequest(finalAmount + 100.msat, paymentHash, c, 1, CltvExpiryDelta(42), Some(pr))
|
||||
sender.send(initiator, req)
|
||||
val id = sender.expectMsgType[UUID]
|
||||
|
|
@ -110,7 +112,7 @@ class PaymentInitiatorSpec extends TestKit(ActorSystem("test")) with fixture.Fun
|
|||
|
||||
test("forward multi-part payment with pre-defined route") { f =>
|
||||
import f._
|
||||
val pr = PaymentRequest(Block.LivenetGenesisBlock.hash, Some(finalAmount), paymentHash, randomKey, "Some invoice", features = Some(Features(BASIC_MULTI_PART_PAYMENT_OPTIONAL)))
|
||||
val pr = PaymentRequest(Block.LivenetGenesisBlock.hash, Some(finalAmount), paymentHash, randomKey, "Some invoice", features = Some(Features(BASIC_MULTI_PART_PAYMENT_OPTIONAL, PAYMENT_SECRET_OPTIONAL)))
|
||||
val req = SendPaymentRequest(finalAmount / 2, paymentHash, c, 1, paymentRequest = Some(pr), predefinedRoute = Seq(a, b, c))
|
||||
sender.send(initiator, req)
|
||||
val id = sender.expectMsgType[UUID]
|
||||
|
|
@ -195,4 +197,22 @@ class PaymentInitiatorSpec extends TestKit(ActorSystem("test")) with fixture.Fun
|
|||
assert(trampolinePayload.invoiceFeatures === Some(pr.features.bitmask.bytes))
|
||||
}
|
||||
|
||||
test("reject trampoline to legacy payment for 0-value invoice") { f =>
|
||||
import f._
|
||||
// This is disabled because it would let the trampoline node steal the whole payment (if malicious).
|
||||
val routingHints = List(List(PaymentRequest.ExtraHop(b, channelUpdate_bc.shortChannelId, 10 msat, 100, CltvExpiryDelta(144))))
|
||||
val features = Features(PAYMENT_SECRET_OPTIONAL, BASIC_MULTI_PART_PAYMENT_OPTIONAL)
|
||||
val pr = PaymentRequest(Block.RegtestGenesisBlock.hash, None, paymentHash, priv_a.privateKey, "#abittooreckless", None, None, routingHints, features = Some(features))
|
||||
val trampolineFees = 21000 msat
|
||||
val req = SendTrampolinePaymentRequest(finalAmount, trampolineFees, pr, b, CltvExpiryDelta(9), CltvExpiryDelta(12))
|
||||
sender.send(initiator, req)
|
||||
val id = sender.expectMsgType[UUID]
|
||||
val fail = sender.expectMsgType[PaymentFailed]
|
||||
assert(fail.id === id)
|
||||
assert(fail.failures.head.isInstanceOf[LocalFailure])
|
||||
|
||||
multiPartPayFsm.expectNoMsg(50 millis)
|
||||
payFsm.expectNoMsg(50 millis)
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -259,16 +259,6 @@ class PaymentPacketSpec extends FunSuite with BeforeAndAfterAll {
|
|||
assert(inner_d.invoiceRoutingInfo === Some(routingHints))
|
||||
}
|
||||
|
||||
test("fail to build a trampoline payment to non-trampoline recipient for 0-value invoice") {
|
||||
// This is disabled because it would let the trampoline node steal the whole payment (if malicious).
|
||||
val routingHints = List(List(PaymentRequest.ExtraHop(randomKey.publicKey, ShortChannelId(42), 10 msat, 100, CltvExpiryDelta(144))))
|
||||
val invoiceFeatures = Features(PAYMENT_SECRET_OPTIONAL, BASIC_MULTI_PART_PAYMENT_OPTIONAL)
|
||||
val invoice = PaymentRequest(Block.RegtestGenesisBlock.hash, None, paymentHash, priv_a.privateKey, "#abittooreckless", None, None, routingHints, features = Some(invoiceFeatures))
|
||||
assertThrows[IllegalArgumentException](
|
||||
buildTrampolineToLegacyPacket(invoice, trampolineHops, FinalLegacyPayload(finalAmount, finalExpiry))
|
||||
)
|
||||
}
|
||||
|
||||
test("fail to build a trampoline payment when too much invoice data is provided") {
|
||||
val routingHintOverflow = List(List.fill(7)(PaymentRequest.ExtraHop(randomKey.publicKey, ShortChannelId(1), 10 msat, 100, CltvExpiryDelta(12))))
|
||||
val invoice = PaymentRequest(Block.RegtestGenesisBlock.hash, Some(finalAmount), paymentHash, priv_a.privateKey, "#reckless", None, None, routingHintOverflow)
|
||||
|
|
|
|||
|
|
@ -312,8 +312,8 @@ class PaymentRequestSpec extends FunSuite {
|
|||
|
||||
val featureBits = Map(
|
||||
Features(bin" 00000000000000000000") -> Result(allowMultiPart = false, requirePaymentSecret = false, areSupported = true),
|
||||
Features(bin" 00010000000000000000") -> Result(allowMultiPart = true, requirePaymentSecret = false, areSupported = true),
|
||||
Features(bin" 00100000000000000000") -> Result(allowMultiPart = true, requirePaymentSecret = false, areSupported = true),
|
||||
Features(bin" 00011000000000000000") -> Result(allowMultiPart = true, requirePaymentSecret = false, areSupported = true),
|
||||
Features(bin" 00101000000000000000") -> Result(allowMultiPart = true, requirePaymentSecret = false, areSupported = true),
|
||||
Features(bin" 00010100000000000000") -> Result(allowMultiPart = true, requirePaymentSecret = true, areSupported = true),
|
||||
Features(bin" 00011000000000000000") -> Result(allowMultiPart = true, requirePaymentSecret = false, areSupported = true),
|
||||
Features(bin" 00101000000000000000") -> Result(allowMultiPart = true, requirePaymentSecret = false, areSupported = true),
|
||||
|
|
@ -349,6 +349,16 @@ class PaymentRequestSpec extends FunSuite {
|
|||
val pr2 = PaymentRequest.read("lnbc40n1pw9qjvwpp5qq3w2ln6krepcslqszkrsfzwy49y0407hvks30ec6pu9s07jur3sdpstfshq5n9v9jzucm0d5s8vmm5v5s8qmmnwssyj3p6yqenwdencqzysxqrrss7ju0s4dwx6w8a95a9p2xc5vudl09gjl0w2n02sjrvffde632nxwh2l4w35nqepj4j5njhh4z65wyfc724yj6dn9wajvajfn5j7em6wsq2elakl")
|
||||
assert(!pr2.features.requirePaymentSecret)
|
||||
assert(pr2.paymentSecret === None)
|
||||
|
||||
// An invoice that sets the payment secret feature bit must provide a payment secret.
|
||||
assertThrows[IllegalArgumentException](
|
||||
PaymentRequest.read("lntb15u1pwahg4kpp56hhnss8tshz2qz539a0u69yjlcq4vpm7776tuqqv53mqn8eeslpsdq4xysyymr0vd4kzcmrd9hx7cqp29qy9qqq6t3lep4wkqeuj4y58fwxj6lqykzqdaa7a7cak4elywptgft0mcfnl0k243870ek8dwnnqww67wrak5kxpfw428rgu58z66er76sh5zsqw0zvns")
|
||||
)
|
||||
|
||||
// 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(BASIC_MULTI_PART_PAYMENT_OPTIONAL)))
|
||||
)
|
||||
}
|
||||
|
||||
test("trampoline") {
|
||||
|
|
@ -359,7 +369,7 @@ class PaymentRequestSpec extends FunSuite {
|
|||
assert(!pr1.features.allowMultiPart)
|
||||
assert(pr1.features.allowTrampoline)
|
||||
|
||||
val pr2 = PaymentRequest(Block.LivenetGenesisBlock.hash, Some(123 msat), ByteVector32.One, priv, "Some invoice", features = Some(Features(TRAMPOLINE_PAYMENT_MANDATORY, BASIC_MULTI_PART_PAYMENT_OPTIONAL)))
|
||||
val pr2 = PaymentRequest(Block.LivenetGenesisBlock.hash, Some(123 msat), ByteVector32.One, priv, "Some invoice", features = Some(Features(TRAMPOLINE_PAYMENT_MANDATORY, BASIC_MULTI_PART_PAYMENT_OPTIONAL, PAYMENT_SECRET_OPTIONAL)))
|
||||
assert(pr2.features.allowMultiPart)
|
||||
assert(pr2.features.allowTrampoline)
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue