Skip to content

Fix restart after alternative feerate remote commit #847

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.

Sign up for GitHub

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

Merged
t-bast merged 1 commit into from
Jan 6, 2026
Merged

Fix restart after alternative feerate remote commit #847

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 7 additions & 1 deletion modules/core/src/commonMain/kotlin/fr/acinq/lightning/channel/Helpers.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -713,7 +713,13 @@ object Helpers {
commitTx: Transaction,
feerates: OnChainFeerates
): Pair<RemoteCommitPublished, RemoteCommitSecondStageTransactions> {
require(remoteCommit.txid == commitTx.txid) { "txid mismatch, provided tx is not the current remote commit tx" }
if (remoteCommit.txid != commitTx.txid) {
// This may only happen when our peer has published an alternative feerate commitment transaction.
// It doesn't matter though, because:
// - alternative feerates are only used for older (non-taproot) channels when there are no HTLCs
// - we will still be able to claim our main output from the published transaction
logger.warning { "txid mismatch (${remoteCommit.txid} != ${commitTx.txid}), provided tx is not the current remote commit tx, is it an alternative feerate commit?" }
}
val commitKeys = channelKeys.remoteCommitmentKeys(commitment.channelParams, remoteCommit.remotePerCommitmentPoint)
val outputs = makeRemoteCommitTxOutputs(channelKeys, commitKeys, commitment, remoteCommit)
val mainTx = claimMainOutput(commitKeys, commitTx, commitment.localCommitParams.dustLimit, commitment.commitmentFormat, commitment.localChannelParams.defaultFinalScriptPubKey, feerates.claimMainFeerate)
Expand Down
6 changes: 3 additions & 3 deletions modules/core/src/commonMain/kotlin/fr/acinq/lightning/channel/states/Channel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,9 @@ sealed class ChannelState {

internal suspend fun ChannelContext.handleLocalError(cmd: ChannelCommand, t: Throwable): Pair<ChannelState, List<ChannelAction>> {
when (cmd) {
is ChannelCommand.MessageReceived -> logger.error(t) { "error on message ${cmd.message::class.simpleName}" }
is ChannelCommand.WatchReceived -> logger.error { "error on watch event ${cmd.watch::class.simpleName}" }
else -> logger.error(t) { "error on command ${cmd::class.simpleName}" }
is ChannelCommand.MessageReceived -> logger.error(t) { "error on message ${cmd.message::class.simpleName}: ${t.message}" }
is ChannelCommand.WatchReceived -> logger.error { "error on watch event ${cmd.watch::class.simpleName}: ${t.message}" }
else -> logger.error(t) { "error on command ${cmd::class.simpleName}: ${t.message}" }
Copy link
Member Author

@t-bast t-bast Jan 6, 2026

Choose a reason for hiding this comment

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

I'm re-adding the exception message directly here because it looks like our logger is swallowing the exception, and thus missing useful data during debugging.

Copy link
Member

@pm47 pm47 Jan 6, 2026

Choose a reason for hiding this comment

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

You mean in our tests? Or in phoenix app logs?

Copy link
Member Author

@t-bast t-bast Jan 6, 2026

Choose a reason for hiding this comment

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

In the Phoenix logs: I'm seeing error on command Restore without details about the exception. The exception is correctly printed in test logs though.

Copy link
Member

@pm47 pm47 Jan 6, 2026

Choose a reason for hiding this comment

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

The bug is in phoenix iOS, the throwable is dropped here: https://github.com/ACINQ/phoenix/blob/5061fd25f45360aa71d232e5b7ae4e6c424eb94d/phoenix-shared/src/iosMain/kotlin/fr/acinq/phoenix/utils/PassthruLogWriter.kt#L15

Copy link
Member Author

@t-bast t-bast Jan 6, 2026

Choose a reason for hiding this comment

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

I remember old discussions about logs containing exceptions being too large, which created issues...isn't that why it was explicitly dropped? Maybe @dpad85 or @robbiehanson remembers?

Copy link
Member

@pm47 pm47 Jan 6, 2026

Choose a reason for hiding this comment

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

Acking the PR in the meantime as this is a separate issue.

}

fun abort(channelId: ByteVector32?, state: ChannelState): Pair<ChannelState, List<ChannelAction>> {
Expand Down
141 changes: 128 additions & 13 deletions modules/core/src/commonTest/kotlin/fr/acinq/lightning/channel/states/ClosingTestsCommon.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -634,18 +634,10 @@ class ClosingTestsCommon : LightningTestSuite() {
val (alice1, bob1) = nodes1
val (alice2, bob2) = crossSign(alice1, bob1)
val (alice3, bob3) = fulfillHtlc(htlc.id, r, alice2, bob2)
val (bob4, actionsBob4) = bob3.process(ChannelCommand.Commitment.Sign)
val commitSigBob = actionsBob4.hasOutgoingMessage<CommitSig>()
val (alice4, actionsAlice4) = alice3.process(ChannelCommand.MessageReceived(commitSigBob))
val revAlice = actionsAlice4.hasOutgoingMessage<RevokeAndAck>()
val (alice5, actionsAlice5) = alice4.process(ChannelCommand.Commitment.Sign)
val commitSigAlice = actionsAlice5.hasOutgoingMessage<CommitSig>()
val (bob5, _) = bob4.process(ChannelCommand.MessageReceived(revAlice))
val (bob6, actionsBob6) = bob5.process(ChannelCommand.MessageReceived(commitSigAlice))
val revBob = actionsBob6.hasOutgoingMessage<RevokeAndAck>()
val (alice6, _) = alice5.process(ChannelCommand.MessageReceived(revBob))
val alternativeCommitTx = useAlternativeCommitSig(alice6, alice6.commitments.active.first(), Commitments.alternativeFeerates.first())
remoteClose(alternativeCommitTx, bob6)
val (bob4, alice4) = crossSign(bob3, alice3)
val alternativeCommitTx = useAlternativeCommitSig(alice4, alice4.commitments.active.first(), Commitments.alternativeFeerates[2])
assertNotEquals(alice4.commitments.active.first().localCommit.txId, alternativeCommitTx.txid)
remoteClose(alternativeCommitTx, bob4)
}

assertNotNull(remoteCommitPublished.localOutput)
Expand All @@ -657,6 +649,66 @@ class ClosingTestsCommon : LightningTestSuite() {
assertTrue(actions.contains(ChannelAction.Storage.StoreState(bobClosed.state)))
}

@Test
fun `recv ClosingTxConfirmed -- remote commit -- alternative feerate -- restart before confirmation`() {
val (alice0, bob0) = reachNormal()
val (bobClosing, remoteCommitPublished, closingTxs) = run {
val (nodes1, r, htlc) = addHtlc(75_000_000.msat, alice0, bob0)
val (alice1, bob1) = nodes1
val (alice2, bob2) = crossSign(alice1, bob1)
val (alice3, bob3) = fulfillHtlc(htlc.id, r, alice2, bob2)
val (bob4, alice4) = crossSign(bob3, alice3)
val alternativeCommitTx = useAlternativeCommitSig(alice4, alice4.commitments.active.first(), Commitments.alternativeFeerates[2])
assertNotEquals(alice4.commitments.active.first().localCommit.txId, alternativeCommitTx.txid)
remoteClose(alternativeCommitTx, bob4)
}

val bobRestart = LNChannel(bobClosing.ctx, WaitForInit)
val (bobClosing1, actions1) = bobRestart.process(ChannelCommand.Init.Restore(bobClosing.state))
assertIs<Closing>(bobClosing1.state)
assertEquals(4, actions1.size)
actions1.hasWatchFundingSpent(bobClosing.commitments.latest.fundingInput.txid)
actions1.hasWatchConfirmed(remoteCommitPublished.commitTx.txid)
actions1.hasPublishTx(closingTxs.mainTx)
assertNotNull(remoteCommitPublished.localOutput)
actions1.hasWatchOutputSpent(remoteCommitPublished.localOutput)

val (bobClosing2, _) = bobClosing.process(ChannelCommand.WatchReceived(WatchConfirmedTriggered(bob0.channelId, WatchConfirmed.ClosingTxConfirmed, 42, 0, remoteCommitPublished.commitTx)))
val (bobClosed, actions) = bobClosing2.process(ChannelCommand.WatchReceived(WatchConfirmedTriggered(bob0.channelId, WatchConfirmed.ClosingTxConfirmed, 43, 0, closingTxs.mainTx)))
assertIs<Closed>(bobClosed.state)
assertTrue(actions.contains(ChannelAction.Storage.StoreState(bobClosed.state)))
}

@Test
fun `recv ClosingTxConfirmed -- remote commit -- alternative feerate -- restart after confirmation`() {
val (alice0, bob0) = reachNormal()
val (bobClosing, remoteCommitPublished, closingTxs) = run {
val (nodes1, r, htlc) = addHtlc(75_000_000.msat, alice0, bob0)
val (alice1, bob1) = nodes1
val (alice2, bob2) = crossSign(alice1, bob1)
val (alice3, bob3) = fulfillHtlc(htlc.id, r, alice2, bob2)
val (bob4, alice4) = crossSign(bob3, alice3)
val alternativeCommitTx = useAlternativeCommitSig(alice4, alice4.commitments.active.first(), Commitments.alternativeFeerates[2])
assertNotEquals(alice4.commitments.active.first().localCommit.txId, alternativeCommitTx.txid)
val (bob5, remoteCommitPublished, closingTxs) = remoteClose(alternativeCommitTx, bob4)
val (bob6, _) = bob5.process(ChannelCommand.WatchReceived(WatchConfirmedTriggered(bob0.channelId, WatchConfirmed.ClosingTxConfirmed, 42, 0, alternativeCommitTx)))
assertIs<LNChannel<Closing>>(bob6)
Triple(bob6, remoteCommitPublished, closingTxs)
}

val bobRestart = LNChannel(bobClosing.ctx, WaitForInit)
val (bobClosing1, actions1) = bobRestart.process(ChannelCommand.Init.Restore(bobClosing.state))
assertIs<Closing>(bobClosing1.state)
assertEquals(2, actions1.size)
actions1.hasPublishTx(closingTxs.mainTx)
assertNotNull(remoteCommitPublished.localOutput)
actions1.hasWatchOutputSpent(remoteCommitPublished.localOutput)

val (bobClosed, actions) = bobClosing1.process(ChannelCommand.WatchReceived(WatchConfirmedTriggered(bob0.channelId, WatchConfirmed.ClosingTxConfirmed, 43, 0, closingTxs.mainTx)))
assertIs<Closed>(bobClosed.state)
assertTrue(actions.contains(ChannelAction.Storage.StoreState(bobClosed.state)))
}

@Test
fun `recv BITCOIN_OUTPUT_SPENT -- remote commit -- extract preimage from HTLC-success tx`() {
val (alice0, bob0) = reachNormal()
Expand Down Expand Up @@ -857,7 +909,8 @@ class ClosingTestsCommon : LightningTestSuite() {
val (bob4, actionsBob4) = bob3.process(ChannelCommand.Commitment.Sign)
val commitSigBob = actionsBob4.hasOutgoingMessage<CommitSig>()
val (alice4, _) = alice3.process(ChannelCommand.MessageReceived(commitSigBob))
val alternativeCommitTx = useAlternativeCommitSig(alice4, alice4.commitments.active.first(), Commitments.alternativeFeerates.first())
val alternativeCommitTx = useAlternativeCommitSig(alice4, alice4.commitments.active.first(), Commitments.alternativeFeerates[1])
assertNotEquals(alice4.commitments.active.first().localCommit.txId, alternativeCommitTx.txid)
remoteClose(alternativeCommitTx, bob4)
}

Expand All @@ -870,6 +923,68 @@ class ClosingTestsCommon : LightningTestSuite() {
assertTrue(actions.contains(ChannelAction.Storage.StoreState(bobClosed.state)))
}

@Test
fun `recv ClosingTxConfirmed -- next remote commit -- alternative feerate -- restart before confirmation`() {
val (alice0, bob0) = reachNormal()
val (bobClosing, remoteCommitPublished, closingTxs) = run {
val (nodes1, r, htlc) = addHtlc(75_000_000.msat, alice0, bob0)
val (alice1, bob1) = nodes1
val (alice2, bob2) = crossSign(alice1, bob1)
val (alice3, bob3) = fulfillHtlc(htlc.id, r, alice2, bob2)
val (bob4, actionsBob4) = bob3.process(ChannelCommand.Commitment.Sign)
val commitSigBob = actionsBob4.hasOutgoingMessage<CommitSig>()
val (alice4, _) = alice3.process(ChannelCommand.MessageReceived(commitSigBob))
val alternativeCommitTx = useAlternativeCommitSig(alice4, alice4.commitments.active.first(), Commitments.alternativeFeerates[1])
assertNotEquals(alice4.commitments.active.first().localCommit.txId, alternativeCommitTx.txid)
remoteClose(alternativeCommitTx, bob4)
}

val bobRestart = LNChannel(bobClosing.ctx, WaitForInit)
val (bobClosing1, actions1) = bobRestart.process(ChannelCommand.Init.Restore(bobClosing.state))
assertIs<Closing>(bobClosing1.state)
actions1.hasWatchFundingSpent(bobClosing.commitments.latest.fundingInput.txid)
actions1.hasWatchConfirmed(remoteCommitPublished.commitTx.txid)
actions1.hasPublishTx(closingTxs.mainTx)
assertNotNull(remoteCommitPublished.localOutput)
actions1.hasWatchOutputSpent(remoteCommitPublished.localOutput)

val (bobClosing2, _) = bobClosing1.process(ChannelCommand.WatchReceived(WatchConfirmedTriggered(bob0.channelId, WatchConfirmed.ClosingTxConfirmed, 42, 0, remoteCommitPublished.commitTx)))
val (bobClosed, actions) = bobClosing2.process(ChannelCommand.WatchReceived(WatchConfirmedTriggered(bob0.channelId, WatchConfirmed.ClosingTxConfirmed, 43, 0, closingTxs.mainTx)))
assertIs<Closed>(bobClosed.state)
assertTrue(actions.contains(ChannelAction.Storage.StoreState(bobClosed.state)))
}

@Test
fun `recv ClosingTxConfirmed -- next remote commit -- alternative feerate -- restart after confirmation`() {
val (alice0, bob0) = reachNormal()
val (bobClosing, remoteCommitPublished, closingTxs) = run {
val (nodes1, r, htlc) = addHtlc(75_000_000.msat, alice0, bob0)
val (alice1, bob1) = nodes1
val (alice2, bob2) = crossSign(alice1, bob1)
val (alice3, bob3) = fulfillHtlc(htlc.id, r, alice2, bob2)
val (bob4, actionsBob4) = bob3.process(ChannelCommand.Commitment.Sign)
val commitSigBob = actionsBob4.hasOutgoingMessage<CommitSig>()
val (alice4, _) = alice3.process(ChannelCommand.MessageReceived(commitSigBob))
val alternativeCommitTx = useAlternativeCommitSig(alice4, alice4.commitments.active.first(), Commitments.alternativeFeerates[1])
assertNotEquals(alice4.commitments.active.first().localCommit.txId, alternativeCommitTx.txid)
val (bob5, remoteCommitPublished, closingTxs) = remoteClose(alternativeCommitTx, bob4)
val (bob6, _) = bob5.process(ChannelCommand.WatchReceived(WatchConfirmedTriggered(bob0.channelId, WatchConfirmed.ClosingTxConfirmed, 42, 0, alternativeCommitTx)))
assertIs<LNChannel<Closing>>(bob6)
Triple(bob6, remoteCommitPublished, closingTxs)
}

val bobRestart = LNChannel(bobClosing.ctx, WaitForInit)
val (bobClosing1, actions1) = bobRestart.process(ChannelCommand.Init.Restore(bobClosing.state))
assertIs<Closing>(bobClosing1.state)
actions1.hasPublishTx(closingTxs.mainTx)
assertNotNull(remoteCommitPublished.localOutput)
actions1.hasWatchOutputSpent(remoteCommitPublished.localOutput)

val (bobClosed, actions) = bobClosing1.process(ChannelCommand.WatchReceived(WatchConfirmedTriggered(bob0.channelId, WatchConfirmed.ClosingTxConfirmed, 43, 0, closingTxs.mainTx)))
assertIs<Closed>(bobClosed.state)
assertTrue(actions.contains(ChannelAction.Storage.StoreState(bobClosed.state)))
}

@Test
fun `recv ClosingTxConfirmed -- next remote commit -- with settled htlcs`() {
val (alice0, bob0) = reachNormal()
Expand Down