Skip to content

Commit ae677ee

Browse files
sstonesstone
committed
Fixup: improve error handling, address some review comments
1 parent 5bf50ad commit ae677ee

File tree

25 files changed

+405
-188
lines changed

25 files changed

+405
-188
lines changed

‎modules/core/src/commonMain/kotlin/fr/acinq/lightning/channel/ChannelCommand.kt‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,7 @@ sealed class ChannelCommand {
103103
val requestRemoteFunding: LiquidityAds.RequestFunding?,
104104
val currentFeeCredit: MilliSatoshi,
105105
val feerate: FeeratePerKw,
106-
val origins: List<Origin>,
107-
val channelType: ChannelType? = null
106+
val origins: List<Origin>
108107
) : Splice() {
109108
val spliceOutputs: List<TxOut> = spliceOut?.let { listOf(TxOut(it.amount, it.scriptPubKey)) } ?: emptyList()
110109
val liquidityFees: LiquidityAds.Fees? = requestRemoteFunding?.fees(feerate, isChannelCreation = false)

‎modules/core/src/commonMain/kotlin/fr/acinq/lightning/channel/Commitments.kt‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -703,11 +703,7 @@ data class Commitments(
703703
val inactive: List<Commitment>,
704704
val payments: Map<Long, UUID>, // for outgoing htlcs, maps to paymentId
705705
val remoteNextCommitInfo: Either<WaitingForRevocation, PublicKey>, // this one is tricky, it must be kept in sync with Commitment.nextRemoteCommit
706-
val remotePerCommitmentSecrets: ShaChain,
707-
@Transient val remoteCommitNonces: Map<TxId, IndividualNonce>,
708-
@Transient val localCloseeNonce: Transactions.LocalNonce?,
709-
@Transient val remoteCloseeNonce: IndividualNonce?,
710-
@Transient val localCloserNonces: Transactions.CloserNonces?,
706+
val remotePerCommitmentSecrets: ShaChain
711707
) {
712708
init {
713709
require(active.isNotEmpty()) { "there must be at least one active commitment" }
@@ -738,9 +734,9 @@ data class Commitments(
738734
addAll(active)
739735
})
740736

741-
fun addRemoteCommitNonce(fundingTxId: TxId, nonce: IndividualNonce?): Commitments = nonce?.let { this.copy(remoteCommitNonces = this.remoteCommitNonces + (fundingTxId to it)) } ?: this
742-
743-
fun resetNonces(): Commitments = copy(remoteCommitNonces = emptyMap(), localCloseeNonce = null, remoteCloseeNonce = null, localCloserNonces = null)
737+
// fun addRemoteCommitNonce(fundingTxId: TxId, nonce: IndividualNonce?): Commitments = nonce?.let { this.copy(remoteCommitNonces = this.remoteCommitNonces + (fundingTxId to it)) } ?: this
738+
//
739+
// fun resetNonces(): Commitments = copy(remoteCommitNonces = emptyMap(), localCloseeNonce = null, remoteCloseeNonce = null, localCloserNonces = null)
744740

745741
fun channelKeys(keyManager: KeyManager): ChannelKeys = channelParams.localParams.channelKeys(keyManager)
746742

@@ -902,7 +898,7 @@ data class Commitments(
902898
return failure?.let { Either.Left(it) } ?: Either.Right(copy(changes = changes1))
903899
}
904900

905-
fun sendCommit(channelKeys: ChannelKeys, log: MDCLogger): Either<ChannelException, Pair<Commitments, CommitSigs>> {
901+
fun sendCommit(channelKeys: ChannelKeys, remoteCommitNonces: Map<TxId, IndividualNonce>, log: MDCLogger): Either<ChannelException, Pair<Commitments, CommitSigs>> {
906902
val remoteNextPerCommitmentPoint = remoteNextCommitInfo.right ?: return Either.Left(CannotSignBeforeRevocation(channelId))
907903
val commitKeys = channelKeys.remoteCommitmentKeys(channelParams, remoteNextPerCommitmentPoint)
908904
if (!changes.localHasChanges()) return Either.Left(CannotSignWithoutChanges(channelId))
@@ -1016,20 +1012,19 @@ data class Commitments(
10161012
remoteNextCommitInfo = Either.Right(revocation.nextPerCommitmentPoint),
10171013
remotePerCommitmentSecrets = remotePerCommitmentSecrets.addHash(revocation.perCommitmentSecret.value, 0xFFFFFFFFFFFFL - remoteCommitIndex),
10181014
payments = payments1,
1019-
remoteCommitNonces = revocation.nextCommitNonces
10201015
)
10211016
return Either.Right(Pair(commitments1, actions.toList()))
10221017
}
10231018

1024-
fun createShutdown(channelKeys: ChannelKeys, finalScriptPubKey: ByteVector): Pair<Commitments, Shutdown> = when (latest.commitmentFormat) {
1019+
fun createShutdown(channelKeys: ChannelKeys, finalScriptPubKey: ByteVector): Pair<Transactions.LocalNonce?, Shutdown> = when (latest.commitmentFormat) {
10251020
is Transactions.CommitmentFormat.SimpleTaprootChannels -> {
10261021
// We create a fresh local closee nonce every time we send shutdown.
10271022
val localFundingPubKey = channelKeys.fundingKey(latest.fundingTxIndex).publicKey()
10281023
val localCloseeNonce = NonceGenerator.signingNonce(localFundingPubKey, latest.remoteFundingPubkey, latest.fundingTxId)
1029-
this.copy(localCloseeNonce = localCloseeNonce) to Shutdown(channelId, finalScriptPubKey, TlvStream<ShutdownTlv>(ShutdownTlv.ShutdownNonce(localCloseeNonce.publicNonce)))
1024+
localCloseeNonce to Shutdown(channelId, finalScriptPubKey, TlvStream<ShutdownTlv>(ShutdownTlv.ShutdownNonce(localCloseeNonce.publicNonce)))
10301025
}
10311026

1032-
else -> this to Shutdown(channelId, finalScriptPubKey)
1027+
else -> null to Shutdown(channelId, finalScriptPubKey)
10331028
}
10341029

10351030
private fun ChannelContext.updateFundingStatus(fundingTxId: TxId, updateMethod: (Commitment, Long) -> Commitment): Either<Commitments, Pair<Commitments, Commitment>> {

‎modules/core/src/commonMain/kotlin/fr/acinq/lightning/channel/InteractiveTx.kt‎

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -509,7 +509,7 @@ data class SharedTransaction(
509509
.find { txIn.outPoint == it.outPoint }
510510
?.let { input ->
511511
// We generate our secret nonce when sending the corresponding input, we know it exists in the map.
512-
val userNonce = session.secretNonces[input.serialId]!!
512+
val userNonce = session.swapInSecretNonces[input.serialId]!!
513513
val serverNonce = remoteNonces[input.serialId]!!
514514
keyManager.swapInOnChainWallet.signSwapInputUser(unsignedTx, i, previousOutputs, userNonce.first, userNonce.second, serverNonce, input.addressIndex)
515515
.map { TxSignaturesTlv.PartialSignature(it, userNonce.second, serverNonce) }
@@ -536,7 +536,7 @@ data class SharedTransaction(
536536
val serverKey = keyManager.swapInOnChainWallet.localServerPrivateKey(remoteNodeId)
537537
val swapInProtocol = SwapInProtocol(input.userKey, serverKey.publicKey(), input.userRefundKey, input.refundDelay)
538538
// We generate our secret nonce when receiving the corresponding input, we know it exists in the map.
539-
val serverNonce = session.secretNonces[input.serialId]!!
539+
val serverNonce = session.swapInSecretNonces[input.serialId]!!
540540
val userNonce = remoteNonces[input.serialId]!!
541541
swapInProtocol.signSwapInputServer(unsignedTx, i, previousOutputs, serverKey, serverNonce.first, userNonce, serverNonce.second)
542542
.map { TxSignaturesTlv.PartialSignature(it, userNonce, serverNonce.second) }
@@ -694,7 +694,7 @@ data class InteractiveTxSession(
694694
val txCompleteReceived: TxComplete? = null,
695695
val inputsReceivedCount: Int = 0,
696696
val outputsReceivedCount: Int = 0,
697-
val secretNonces: Map<Long, Pair<SecretNonce, IndividualNonce>> = mapOf(),
697+
val swapInSecretNonces: Map<Long, Pair<SecretNonce, IndividualNonce>> = mapOf(),
698698
val commitTxIndex: Long,
699699
val fundingTxIndex: Long,
700700
// README: this is a field because we want to preserve this value when we use .copy() and it would not be the case if it was a val defined in the class body
@@ -760,7 +760,7 @@ data class InteractiveTxSession(
760760
.map { it.serialId }
761761
.sorted()
762762
// We generate secret nonces whenever we send and receive tx_add_input, so we know they exist in the map.
763-
.map { serialId -> secretNonces[serialId]!!.second }
763+
.map { serialId -> swapInSecretNonces[serialId]!!.second }
764764
val commitNonces = when (this.fundingParams.commitmentFormat) {
765765
Transactions.CommitmentFormat.SimpleTaprootChannels -> {
766766
val fundingTxId = runTrying {
@@ -810,16 +810,18 @@ data class InteractiveTxSession(
810810
}
811811
val nextSecretNonces = when (inputOutgoing) {
812812
// Generate a secret nonce for this input if we don't already have one.
813-
is InteractiveTxInput.LocalSwapIn -> when (secretNonces[inputOutgoing.serialId]) {
813+
is InteractiveTxInput.LocalSwapIn -> when (swapInSecretNonces[inputOutgoing.serialId]) {
814814
null -> {
815815
val secretNonce = Musig2.generateNonce(randomBytes32(), Either.Left(swapInKeys.userPrivateKey), listOf(swapInKeys.userPublicKey, swapInKeys.remoteServerPublicKey), null, null)
816-
secretNonces + (inputOutgoing.serialId to secretNonce)
816+
swapInSecretNonces + (inputOutgoing.serialId to secretNonce)
817817
}
818-
else -> secretNonces
818+
819+
else -> swapInSecretNonces
819820
}
820-
else -> secretNonces
821+
822+
else -> swapInSecretNonces
821823
}
822-
val next = copy(toSend = toSend.tail(), localInputs = localInputs + msg.value, txCompleteSent = null, secretNonces = nextSecretNonces)
824+
val next = copy(toSend = toSend.tail(), localInputs = localInputs + msg.value, txCompleteSent = null, swapInSecretNonces = nextSecretNonces)
823825
Pair(next, InteractiveTxSessionAction.SendMessage(txAddInput))
824826
}
825827
is Either.Right -> {
@@ -901,16 +903,18 @@ data class InteractiveTxSession(
901903
}
902904
val secretNonces1 = when (input) {
903905
// Generate a secret nonce for this input if we don't already have one.
904-
is InteractiveTxInput.RemoteSwapIn -> when (secretNonces[input.serialId]) {
906+
is InteractiveTxInput.RemoteSwapIn -> when (swapInSecretNonces[input.serialId]) {
905907
null -> {
906908
val secretNonce = Musig2.generateNonce(randomBytes32(), Either.Right(input.serverKey), listOf(input.userKey, input.serverKey), null, null)
907-
secretNonces + (input.serialId to secretNonce)
909+
swapInSecretNonces + (input.serialId to secretNonce)
908910
}
909-
else -> secretNonces
911+
912+
else -> swapInSecretNonces
910913
}
911-
else -> secretNonces
914+
915+
else -> swapInSecretNonces
912916
}
913-
val session1 = this.copy(remoteInputs = remoteInputs + input, inputsReceivedCount = inputsReceivedCount + 1, txCompleteReceived = null, secretNonces = secretNonces1)
917+
val session1 = this.copy(remoteInputs = remoteInputs + input, inputsReceivedCount = inputsReceivedCount + 1, txCompleteReceived = null, swapInSecretNonces = secretNonces1)
914918
return Either.Right(session1)
915919
}
916920

‎modules/core/src/commonMain/kotlin/fr/acinq/lightning/channel/states/Channel.kt‎

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -392,15 +392,14 @@ sealed class PersistedChannelState : ChannelState() {
392392

393393
sealed class ChannelStateWithCommitments : PersistedChannelState() {
394394
abstract val commitments: Commitments
395+
abstract val remoteCommitNonces: Map<TxId, IndividualNonce>
395396
override val channelId: ByteVector32 get() = commitments.channelId
396397
val isChannelOpener: Boolean get() = commitments.channelParams.localParams.isChannelOpener
397398
val paysCommitTxFees: Boolean get() = commitments.channelParams.localParams.paysCommitTxFees
398399
val remoteNodeId: PublicKey get() = commitments.remoteNodeId
399400

400401
abstract fun updateCommitments(input: Commitments): ChannelStateWithCommitments
401402

402-
fun updateCloseeNonce(remoteCloseeNonce: IndividualNonce?) = updateCommitments(commitments.copy(remoteCloseeNonce = remoteCloseeNonce))
403-
404403
/**
405404
* When a funding transaction confirms, we can prune previous commitments.
406405
* We also watch this funding transaction to be able to detect force-close attempts.
@@ -450,15 +449,19 @@ sealed class ChannelStateWithCommitments : PersistedChannelState() {
450449
commitments: Commitments,
451450
localShutdown: Shutdown,
452451
remoteShutdown: Shutdown,
453-
actions: List<ChannelAction>
452+
actions: List<ChannelAction>,
453+
remoteCommitNonces: Map<TxId, IndividualNonce>,
454+
localCloseeNonce: Transactions.LocalNonce?,
455+
remoteCloseeNonce: IndividualNonce?
454456
): Pair<Negotiating, List<ChannelAction>> {
455457
val localScript = localShutdown.scriptPubKey
456458
val remoteScript = remoteShutdown.scriptPubKey
457459
val currentHeight = currentBlockHeight.toLong()
458460
return when (cmd) {
459461
null -> {
460462
logger.info { "mutual close was initiated by our peer, waiting for remote closing_complete" }
461-
val nextState = Negotiating(commitments, localScript, remoteScript, listOf(), listOf(), currentHeight, cmd)
463+
val nextState =
464+
Negotiating(commitments, remoteCommitNonces, localScript, remoteScript, listOf(), listOf(), currentHeight, cmd, localCloseeNonce = localCloseeNonce, remoteCloseeNonce = remoteCloseeNonce, localCloserNonces = null)
462465
val actions1 = listOf(ChannelAction.Storage.StoreState(nextState))
463466
Pair(nextState, actions + actions1)
464467
}
@@ -467,13 +470,38 @@ sealed class ChannelStateWithCommitments : PersistedChannelState() {
467470
is Either.Left -> {
468471
logger.warning { "cannot create local closing txs, waiting for remote closing_complete: ${closingResult.value.message}" }
469472
cmd.replyTo.complete(ChannelCloseResponse.Failure.Unknown(closingResult.value))
470-
val nextState = Negotiating(commitments, localScript, remoteScript, listOf(), listOf(), currentHeight, cmd)
473+
val nextState = Negotiating(
474+
commitments,
475+
remoteCommitNonces,
476+
localScript,
477+
remoteScript,
478+
listOf(),
479+
listOf(),
480+
currentHeight,
481+
cmd,
482+
localCloseeNonce = localCloseeNonce,
483+
remoteCloseeNonce = remoteCloseeNonce,
484+
localCloserNonces = null
485+
)
471486
val actions1 = listOf(ChannelAction.Storage.StoreState(nextState))
472487
Pair(nextState, actions + actions1)
473488
}
474489
is Either.Right -> {
475490
val (closingTxs, closingComplete, localNonces) = closingResult.value
476-
val nextState = Negotiating(commitments.copy(localCloserNonces = localNonces), localScript, remoteScript, listOf(closingTxs), listOf(), currentHeight, cmd)
491+
val nextState =
492+
Negotiating(
493+
commitments,
494+
remoteCommitNonces,
495+
localScript,
496+
remoteScript,
497+
listOf(closingTxs),
498+
listOf(),
499+
currentHeight,
500+
cmd,
501+
localCloseeNonce = localCloseeNonce,
502+
remoteCloseeNonce = remoteCloseeNonce,
503+
localCloserNonces = localNonces
504+
)
477505
val actions1 = listOf(
478506
ChannelAction.Storage.StoreState(nextState),
479507
ChannelAction.Message.Send(closingComplete),
@@ -685,7 +713,7 @@ sealed class ChannelStateWithCommitments : PersistedChannelState() {
685713
)
686714
val nextState = when (this@ChannelStateWithCommitments) {
687715
is Closing -> this@ChannelStateWithCommitments.copy(remoteCommitPublished = remoteCommitPublished)
688-
is Negotiating -> Closing(commitments, waitingSinceBlock, proposedClosingTxs.flatMap { it.all }, publishedClosingTxs, remoteCommitPublished = remoteCommitPublished)
716+
is Negotiating -> Closing(commitments, remoteCommitNonces = remoteCommitNonces, waitingSinceBlock, proposedClosingTxs.flatMap { it.all }, publishedClosingTxs, remoteCommitPublished = remoteCommitPublished)
689717
else -> Closing(commitments, waitingSinceBlock = currentBlockHeight.toLong(), remoteCommitPublished = remoteCommitPublished)
690718
}
691719
return Pair(nextState, buildList {

‎modules/core/src/commonMain/kotlin/fr/acinq/lightning/channel/states/Closed.kt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package fr.acinq.lightning.channel.states
22

3+
import fr.acinq.bitcoin.TxId
4+
import fr.acinq.bitcoin.crypto.musig2.IndividualNonce
35
import fr.acinq.lightning.channel.ChannelAction
46
import fr.acinq.lightning.channel.ChannelCommand
57
import fr.acinq.lightning.channel.Commitments
@@ -10,6 +12,8 @@ import fr.acinq.lightning.channel.Commitments
1012
data class Closed(val state: Closing) : ChannelStateWithCommitments() {
1113
override val commitments: Commitments get() = state.commitments
1214

15+
override val remoteCommitNonces: Map<TxId, IndividualNonce> get() = state.remoteCommitNonces
16+
1317
override fun updateCommitments(input: Commitments): ChannelStateWithCommitments {
1418
return this.copy(state = state.updateCommitments(input) as Closing)
1519
}

‎modules/core/src/commonMain/kotlin/fr/acinq/lightning/channel/states/Closing.kt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package fr.acinq.lightning.channel.states
22

33
import fr.acinq.bitcoin.Transaction
4+
import fr.acinq.bitcoin.TxId
5+
import fr.acinq.bitcoin.crypto.musig2.IndividualNonce
46
import fr.acinq.bitcoin.updated
57
import fr.acinq.bitcoin.utils.Either
68
import fr.acinq.lightning.blockchain.WatchConfirmed
@@ -33,6 +35,7 @@ data class RevokedClose(val revokedCommitPublished: RevokedCommitPublished) : Cl
3335

3436
data class Closing(
3537
override val commitments: Commitments,
38+
override val remoteCommitNonces: Map<TxId, IndividualNonce> = mapOf(), // TODO: check this
3639
val waitingSinceBlock: Long, // how many blocks since we initiated the closing
3740
val mutualCloseProposed: List<ClosingTx> = emptyList(), // all exchanged closing sigs are flattened, we use this only to keep track of what publishable tx they have
3841
val mutualClosePublished: List<ClosingTx> = emptyList(),

0 commit comments

Comments
 (0)