Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle onion creation failure #314

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions src/commonMain/kotlin/fr/acinq/lightning/crypto/sphinx/Sphinx.kt
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ object Sphinx {

fun mac(key: ByteArray, message: ByteArray): ByteVector32 = Digest.sha256().hmac(key, message, 64).toByteVector32()

fun mac(key: ByteVector, message: ByteVector): ByteVector32 = mac(key.toByteArray(), message.toByteArray())
private fun mac(key: ByteVector, message: ByteVector): ByteVector32 = mac(key.toByteArray(), message.toByteArray())

private fun generateKey(keyType: ByteArray, secret: ByteVector32): ByteVector32 = mac(keyType, secret.toByteArray())

Expand All @@ -49,9 +49,9 @@ object Sphinx {

fun generateStream(key: ByteVector32, length: Int): ByteArray = ChaCha20.encrypt(zeroes(length), key.toByteArray(), zeroes(12))

fun computeSharedSecret(pub: PublicKey, secret: PrivateKey): ByteVector32 = Crypto.sha256(pub.times(secret).value).toByteVector32()
private fun computeSharedSecret(pub: PublicKey, secret: PrivateKey): ByteVector32 = Crypto.sha256(pub.times(secret).value).toByteVector32()

fun computeBlindingFactor(pub: PublicKey, secret: ByteVector): ByteVector32 = Crypto.sha256(pub.value + secret).toByteVector32()
private fun computeBlindingFactor(pub: PublicKey, secret: ByteVector): ByteVector32 = Crypto.sha256(pub.value + secret).toByteVector32()

fun blind(pub: PublicKey, blindingFactor: ByteVector32): PublicKey = pub.times(PrivateKey(blindingFactor))

Expand Down Expand Up @@ -256,7 +256,9 @@ object Sphinx {
* @return An onion packet with all shared secrets. The onion packet can be sent to the first node in the list, and
* the shared secrets (one per node) can be used to parse returned failure messages if needed.
*/
fun create(sessionKey: PrivateKey, publicKeys: List<PublicKey>, payloads: List<ByteArray>, associatedData: ByteVector32, packetLength: Int): PacketAndSecrets {
fun create(sessionKey: PrivateKey, publicKeys: List<PublicKey>, payloads: List<ByteArray>, associatedData: ByteVector32, packetLength: Int): Try<PacketAndSecrets> = runTrying {
require(payloads.sumOf { it.size + MacLength } <= packetLength) { "packet per-hop payloads cannot exceed $packetLength bytes" }

val (ephemeralPublicKeys, sharedsecrets) = computeEphemeralPublicKeysAndSharedSecrets(sessionKey, publicKeys)
val filler = generateFiller("rho", sharedsecrets.dropLast(1), payloads.dropLast(1), packetLength)

Expand All @@ -272,7 +274,7 @@ object Sphinx {
}

val packet = loop(payloads.dropLast(1), ephemeralPublicKeys.dropLast(1), sharedsecrets.dropLast(1), lastPacket)
return PacketAndSecrets(packet, SharedSecrets(sharedsecrets.zip(publicKeys)))
PacketAndSecrets(packet, SharedSecrets(sharedsecrets.zip(publicKeys)))
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ sealed class FinalFailure {

// @formatter:off
object AlreadyPaid : FinalFailure() { override fun toString(): String = "this invoice has already been paid" }
object InvoiceTooBig : FinalFailure() { override fun toString(): String = "this invoice contains too much metadata to be paid" }
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dpad85 @robbiehanson what do you think of this error? I believe we need to surface to the user that this problem is caused by this specific invoice that they are trying to pay, and that some of the fields it contains force us to send too much data to the recipient, which we cannot do.

This shouldn't happen often, but when it does, it's good that it's explicitly handled. We currently use an inefficient trampoline scheme where the onion is capped at 400 bytes, we will change that to a variable-size onion which should improve the issue if we see it happening regularly once option_payment_metadata starts being deployed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense. The error message would get surfaced to the user. On iOS, this info gets displayed in the payment details screen. For a developer, or anybody helping to debug, this would be very useful information.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, the UI needs as much info as it can get. We don't have to display everything right away, but this is a good thing to have and we can also reformat those messages easily if needed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, then this should find its way into lightning-kmp soon (I've stacked a few PRs on top of each other)

object InvalidPaymentAmount : FinalFailure() { override fun toString(): String = "payment amount must be positive" }
object InvalidPaymentId : FinalFailure() { override fun toString(): String = "payment ID must be unique" }
object NoAvailableChannels : FinalFailure() { override fun toString(): String = "no channels available to send payment" }
Expand All @@ -33,7 +34,7 @@ data class OutgoingPaymentFailure(val reason: FinalFailure, val failures: List<O
* A detailed summary of the all internal errors.
* This is targeted at users with technical knowledge of the lightning protocol.
*/
fun details(): String = failures.foldIndexed("", { index, msg, problem -> msg + "${index + 1}: ${problem.details}\n" })
fun details(): String = failures.foldIndexed("") { index, msg, problem -> msg + "${index + 1}: ${problem.details}\n" }

companion object {
fun convertFailure(failure: Either<ChannelException, FailureMessage>, completedAt: Long = currentTimestampMillis()): OutgoingPayment.Part.Status.Failed = when (failure) {
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import fr.acinq.lightning.router.ChannelHop
import fr.acinq.lightning.router.Hop
import fr.acinq.lightning.router.NodeHop
import fr.acinq.lightning.utils.Either
import fr.acinq.lightning.utils.Try
import fr.acinq.lightning.utils.UUID
import fr.acinq.lightning.wire.FailureMessage
import fr.acinq.lightning.wire.OnionRoutingPacket
Expand All @@ -27,7 +28,7 @@ object OutgoingPaymentPacket {
/**
* Build an encrypted onion packet from onion payloads and node public keys.
*/
private fun buildOnion(nodes: List<PublicKey>, payloads: List<PaymentOnion.PerHopPayload>, associatedData: ByteVector32, payloadLength: Int): PacketAndSecrets {
private fun buildOnion(nodes: List<PublicKey>, payloads: List<PaymentOnion.PerHopPayload>, associatedData: ByteVector32, payloadLength: Int): Try<PacketAndSecrets> {
require(nodes.size == payloads.size)
val sessionKey = Lightning.randomKey()
val payloadsBin = payloads
Expand Down Expand Up @@ -75,7 +76,7 @@ object OutgoingPaymentPacket {
* - firstExpiry is the cltv expiry for the first trampoline node in the route
* - the trampoline onion to include in final payload of a normal onion
*/
fun buildTrampolineToLegacyPacket(invoice: PaymentRequest, hops: List<NodeHop>, finalPayload: PaymentOnion.FinalPayload): Triple<MilliSatoshi, CltvExpiry, PacketAndSecrets> {
fun buildTrampolineToLegacyPacket(invoice: PaymentRequest, hops: List<NodeHop>, finalPayload: PaymentOnion.FinalPayload): Try<Triple<MilliSatoshi, CltvExpiry, PacketAndSecrets>> {
// NB: the final payload will never reach the recipient, since the next-to-last trampoline hop will convert that to a legacy payment
// We use the smallest final payload possible, otherwise we may overflow the trampoline onion size.
val dummyFinalPayload = PaymentOnion.FinalPayload.createSinglePartPayload(finalPayload.amount, finalPayload.expiry, finalPayload.paymentSecret, null)
Expand All @@ -89,8 +90,9 @@ object OutgoingPaymentPacket {
Triple(amount + hop.fee(amount), expiry + hop.cltvExpiryDelta, listOf(payload) + payloads)
}
val nodes = hops.map { it.nextNodeId }
val onion = buildOnion(nodes, payloads, invoice.paymentHash, OnionRoutingPacket.TrampolinePacketLength)
return Triple(firstAmount, firstExpiry, onion)
return buildOnion(nodes, payloads, invoice.paymentHash, OnionRoutingPacket.TrampolinePacketLength).map { onion ->
Triple(firstAmount, firstExpiry, onion)
}
}

/**
Expand All @@ -103,22 +105,24 @@ object OutgoingPaymentPacket {
* - firstExpiry is the cltv expiry for the first htlc in the route
* - the onion to include in the HTLC
*/
fun buildPacket(paymentHash: ByteVector32, hops: List<Hop>, finalPayload: PaymentOnion.FinalPayload, payloadLength: Int): Triple<MilliSatoshi, CltvExpiry, PacketAndSecrets> {
fun buildPacket(paymentHash: ByteVector32, hops: List<Hop>, finalPayload: PaymentOnion.FinalPayload, payloadLength: Int): Try<Triple<MilliSatoshi, CltvExpiry, PacketAndSecrets>> {
val (firstAmount, firstExpiry, payloads) = buildPayloads(hops.drop(1), finalPayload)
val nodes = hops.map { it.nextNodeId }
// BOLT 2 requires that associatedData == paymentHash
val onion = buildOnion(nodes, payloads, paymentHash, payloadLength)
return Triple(firstAmount, firstExpiry, onion)
return buildOnion(nodes, payloads, paymentHash, payloadLength).map { onion ->
Triple(firstAmount, firstExpiry, onion)
}
}

/**
* Build the command to add an HTLC with the given final payload and using the provided hops.
*
* @return the command and the onion shared secrets (used to decrypt the error in case of payment failure)
*/
fun buildCommand(paymentId: UUID, paymentHash: ByteVector32, hops: List<ChannelHop>, finalPayload: PaymentOnion.FinalPayload): Pair<CMD_ADD_HTLC, SharedSecrets> {
val (firstAmount, firstExpiry, onion) = buildPacket(paymentHash, hops, finalPayload, OnionRoutingPacket.PaymentPacketLength)
return Pair(CMD_ADD_HTLC(firstAmount, paymentHash, firstExpiry, onion.packet, paymentId, commit = true), onion.sharedSecrets)
fun buildCommand(paymentId: UUID, paymentHash: ByteVector32, hops: List<ChannelHop>, finalPayload: PaymentOnion.FinalPayload): Try<Pair<CMD_ADD_HTLC, SharedSecrets>> {
return buildPacket(paymentHash, hops, finalPayload, OnionRoutingPacket.PaymentPacketLength).map { (firstAmount, firstExpiry, onion) ->
Pair(CMD_ADD_HTLC(firstAmount, paymentHash, firstExpiry, onion.packet, paymentId, commit = true), onion.sharedSecrets)
}
}

fun buildHtlcFailure(nodeSecret: PrivateKey, paymentHash: ByteVector32, onion: OnionRoutingPacket, reason: CMD_FAIL_HTLC.Reason): Either<FailureMessage, ByteVector> {
Expand Down
16 changes: 12 additions & 4 deletions src/commonMain/kotlin/fr/acinq/lightning/utils/Either.kt
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,16 @@ sealed class Either<out A, out B> {
}

@Suppress("UNCHECKED_CAST")
fun <L, R, T> Either<L, R>.flatMap(f: (R) -> Either<L, T>): Either<L, T> =
this.fold({ this as Either<L, T> }, f)
fun <L, R, T> Either<L, R>.flatMap(f: (R) -> Either<L, T>): Either<L, T> = this.fold({ this as Either<L, T> }, f)

fun <L, R, T> Either<L, R>.map(f: (R) -> T): Either<L, T> =
flatMap { Either.Right(f(it)) }
fun <L, R, T> Either<L, R>.map(f: (R) -> T): Either<L, T> = flatMap { Either.Right(f(it)) }

fun <L, R> List<Either<L, R>>.toEither(): Either<L, List<R>> = this.fold(Either.Right(listOf())) { current, element ->
when (current) {
is Either.Left -> current
is Either.Right -> when (element) {
is Either.Left -> Either.Left(element.value)
is Either.Right -> Either.Right(current.value + element.value)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ object PaymentOnion {
// NB: we limit the number of routing hints to ensure we don't overflow the onion.
// A better solution is to provide the routing hints outside the onion (in the `update_add_htlc` tlv stream).
val prunedRoutingHints = invoice.routingInfo.shuffled().fold(listOf<PaymentRequest.TaggedField.RoutingInfo>()) { previous, current ->
if (previous.flatMap { it.hints }.size + current.hints.size <= 4) {
if (previous.flatMap { it.hints }.size + current.hints.size <= 3) {
previous + current
} else {
previous
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ object TestsHelper {
val expiry = CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight)
val dummyKey = PrivateKey(ByteVector32("0101010101010101010101010101010101010101010101010101010101010101")).publicKey()
val dummyUpdate = ChannelUpdate(ByteVector64.Zeroes, ByteVector32.Zeroes, ShortChannelId(144, 0, 0), 0, 0, 0, CltvExpiryDelta(1), 0.msat, 0.msat, 0, null)
val cmd = OutgoingPaymentPacket.buildCommand(paymentId, paymentHash, listOf(ChannelHop(dummyKey, destination, dummyUpdate)), PaymentOnion.FinalPayload.createSinglePartPayload(amount, expiry, randomBytes32(), null)).first.copy(commit = false)
val cmd = OutgoingPaymentPacket.buildCommand(paymentId, paymentHash, listOf(ChannelHop(dummyKey, destination, dummyUpdate)), PaymentOnion.FinalPayload.createSinglePartPayload(amount, expiry, randomBytes32(), null)).get().first.copy(commit = false)
return Pair(paymentPreimage, cmd)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import fr.acinq.lightning.wire.*
import fr.acinq.secp256k1.Hex
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFails
import kotlin.test.assertTrue

class SphinxTestsCommon : LightningTestSuite() {
Expand Down Expand Up @@ -183,7 +182,7 @@ class SphinxTestsCommon : LightningTestSuite() {

@Test
fun `create packet with fixed-size payloads (reference test vector)`() {
val packetAndSecrets = Sphinx.create(sessionKey, publicKeys, referenceFixedSizePayloads.map { it.toByteArray() }, associatedData, OnionRoutingPacket.PaymentPacketLength)
val packetAndSecrets = Sphinx.create(sessionKey, publicKeys, referenceFixedSizePayloads.map { it.toByteArray() }, associatedData, OnionRoutingPacket.PaymentPacketLength).get()
val onion = packetAndSecrets.packet
assertEquals(
Hex.encode(OnionRoutingPacketSerializer(OnionRoutingPacket.PaymentPacketLength).write(onion)),
Expand All @@ -209,7 +208,7 @@ class SphinxTestsCommon : LightningTestSuite() {

@Test
fun `create packet with variable-size payloads (reference test vector)`() {
val packetAndSecrets = Sphinx.create(sessionKey, publicKeys, referenceVariableSizePayloads.map { it.toByteArray() }, associatedData, OnionRoutingPacket.PaymentPacketLength)
val packetAndSecrets = Sphinx.create(sessionKey, publicKeys, referenceVariableSizePayloads.map { it.toByteArray() }, associatedData, OnionRoutingPacket.PaymentPacketLength).get()
val onion = packetAndSecrets.packet
assertEquals(
Hex.encode(OnionRoutingPacketSerializer(OnionRoutingPacket.PaymentPacketLength).write(onion)),
Expand All @@ -235,7 +234,7 @@ class SphinxTestsCommon : LightningTestSuite() {

@Test
fun `create packet with variable-size payloads filling the onion`() {
val packetAndSecrets = Sphinx.create(sessionKey, publicKeys, variableSizePayloadsFull.map { it.toByteArray() }, associatedData, OnionRoutingPacket.PaymentPacketLength)
val packetAndSecrets = Sphinx.create(sessionKey, publicKeys, variableSizePayloadsFull.map { it.toByteArray() }, associatedData, OnionRoutingPacket.PaymentPacketLength).get()
val onion = packetAndSecrets.packet
assertEquals(
Hex.encode(OnionRoutingPacketSerializer(OnionRoutingPacket.PaymentPacketLength).write(onion)),
Expand All @@ -261,7 +260,7 @@ class SphinxTestsCommon : LightningTestSuite() {

@Test
fun `create packet with single variable-size payload filling the onion`() {
val packetAndSecrets = Sphinx.create(sessionKey, publicKeys.take(1), variableSizeOneHopPayload.map { it.toByteArray() }, associatedData, OnionRoutingPacket.PaymentPacketLength)
val packetAndSecrets = Sphinx.create(sessionKey, publicKeys.take(1), variableSizeOneHopPayload.map { it.toByteArray() }, associatedData, OnionRoutingPacket.PaymentPacketLength).get()
val onion = packetAndSecrets.packet
assertEquals(
Hex.encode(OnionRoutingPacketSerializer(OnionRoutingPacket.PaymentPacketLength).write(onion)),
Expand All @@ -274,7 +273,7 @@ class SphinxTestsCommon : LightningTestSuite() {

@Test
fun `create trampoline packet`() {
val packetAndSecrets = Sphinx.create(sessionKey, publicKeys, trampolinePayloads.map { it.toByteArray() }, associatedData, OnionRoutingPacket.TrampolinePacketLength)
val packetAndSecrets = Sphinx.create(sessionKey, publicKeys, trampolinePayloads.map { it.toByteArray() }, associatedData, OnionRoutingPacket.TrampolinePacketLength).get()
val onion = packetAndSecrets.packet
assertEquals(
Hex.encode(OnionRoutingPacketSerializer(OnionRoutingPacket.TrampolinePacketLength).write(onion)),
Expand All @@ -298,7 +297,16 @@ class SphinxTestsCommon : LightningTestSuite() {
Hex.decode("fd2a0101234567"),
Hex.decode("000000000000000000000000000000000000000000000000000000000000000000")
)
assertFails { Sphinx.create(sessionKey, publicKeys.take(2), invalidPayloads, associatedData, OnionRoutingPacket.PaymentPacketLength) }
assertTrue(Sphinx.create(sessionKey, publicKeys.take(2), invalidPayloads, associatedData, OnionRoutingPacket.PaymentPacketLength).isFailure)
}

@Test
fun `create packet with payloads too big`() {
val payloadsTooBig = listOf(
Hex.decode("c0010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101"),
Hex.decode("c0020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202"),
)
assertTrue(Sphinx.create(sessionKey, publicKeys.take(2), payloadsTooBig, associatedData, OnionRoutingPacket.TrampolinePacketLength).isFailure)
}

@Test
Expand Down Expand Up @@ -417,7 +425,7 @@ class SphinxTestsCommon : LightningTestSuite() {
// route: origin -> node #0 -> node #1 -> node #2 -> node #3 -> node #4
// origin builds the onion packet
val packetLength = it.first
val packetAndSecrets = Sphinx.create(sessionKey, publicKeys, it.second.map { p -> p.toByteArray() }, associatedData, packetLength)
val packetAndSecrets = Sphinx.create(sessionKey, publicKeys, it.second.map { p -> p.toByteArray() }, associatedData, packetLength).get()

// each node parses and forwards the packet
// node #0
Expand Down Expand Up @@ -477,7 +485,7 @@ class SphinxTestsCommon : LightningTestSuite() {
// route: origin -> node #0 -> node #1 -> node #2 -> node #3 -> node #4
// origin builds the onion packet
val packetLength = it.first
val packetAndSecrets = Sphinx.create(sessionKey, publicKeys, it.second.map { p -> p.toByteArray() }, associatedData, packetLength)
val packetAndSecrets = Sphinx.create(sessionKey, publicKeys, it.second.map { p -> p.toByteArray() }, associatedData, packetLength).get()

// each node parses and forwards the packet
// node #0
Expand Down
Loading