Skip to content

Commit d8b2cbe

Browse files
pm47pm47
committed
handle sync errors in SHUTDOWN and refactor
1 parent 6d396ce commit d8b2cbe

3 files changed

Lines changed: 60 additions & 47 deletions

File tree

‎eclair-core/src/main/scala/fr/acinq/eclair/channel/Channel.scala‎

Lines changed: 33 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1703,25 +1703,9 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder, remo
17031703

17041704
case Event(channelReestablish: ChannelReestablish, d: DATA_NORMAL) =>
17051705
Syncing.checkSync(keyManager, d, channelReestablish) match {
1706-
case res: SyncResult.LocalLateProven =>
1707-
log.warning(s"counterparty proved that we have an outdated (revoked) local commitment!!! ourLocalCommitmentNumber=${res.ourLocalCommitmentNumber} theirRemoteCommitmentNumber=${res.theirRemoteCommitmentNumber}")
1708-
// their data checks out, we indeed seem to be using an old revoked commitment, and must absolutely *NOT* publish it, because that would be a cheating attempt and they
1709-
// would punish us by taking all the funds in the channel
1710-
handleOutdatedCommitment(channelReestablish, d)
1711-
case res: Syncing.SyncResult.LocalLateUnproven =>
1712-
log.warning(s"our local commitment is in sync, but counterparty says that they have a more recent remote commitment than the one we know of (they could be lying)!!! ourRemoteCommitmentNumber=${res.ourRemoteCommitmentNumber} theirCommitmentNumber=${res.theirLocalCommitmentNumber}")
1713-
// there is no way to make sure that they are saying the truth, the best thing to do is ask them to publish their commitment right now
1714-
// maybe they will publish their commitment, in that case we need to remember their commitment point in order to be able to claim our outputs
1715-
// note that if they don't comply, we could publish our own commitment (it is not stale, otherwise we would be in the case above)
1716-
handleOutdatedCommitment(channelReestablish, d)
1717-
case res: Syncing.SyncResult.RemoteLying =>
1718-
// they are deliberately trying to fool us into thinking we have a late commitment
1719-
log.warning(s"counterparty is lying about us having an outdated commitment!!! ourLocalCommitmentNumber=${res.ourLocalCommitmentNumber} theirRemoteCommitmentNumber=${res.theirRemoteCommitmentNumber}")
1720-
handleLocalError(InvalidRevokedCommitProof(d.channelId, res.ourLocalCommitmentNumber, res.theirRemoteCommitmentNumber, res.invalidPerCommitmentSecret), d, Some(channelReestablish))
1721-
case SyncResult.RemoteLate =>
1722-
log.warning("counterparty appears to be using an outdated commitment, they may request a force-close, standing by...")
1723-
stay()
1724-
case res: SyncResult.InSync =>
1706+
case syncFailure: SyncResult.Failure =>
1707+
handleSyncFailure(channelReestablish, syncFailure, d)
1708+
case syncSuccess: SyncResult.Success =>
17251709
var sendQueue = Queue.empty[LightningMessage]
17261710
// normal case, our data is up-to-date
17271711

@@ -1735,18 +1719,10 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder, remo
17351719
}
17361720

17371721
// we may need to retransmit updates and/or commit_sig and/or revocation
1738-
sendQueue = sendQueue ++ res.retransmit
1722+
sendQueue = sendQueue ++ syncSuccess.retransmit
17391723

17401724
// then we clean up unacknowledged updates
1741-
log.debug("discarding proposed OUT: {}", d.commitments.localChanges.proposed.map(Commitments.msg2String(_)).mkString(","))
1742-
log.debug("discarding proposed IN: {}", d.commitments.remoteChanges.proposed.map(Commitments.msg2String(_)).mkString(","))
1743-
val commitments1 = d.commitments.copy(
1744-
localChanges = d.commitments.localChanges.copy(proposed = Nil),
1745-
remoteChanges = d.commitments.remoteChanges.copy(proposed = Nil),
1746-
localNextHtlcId = d.commitments.localNextHtlcId - d.commitments.localChanges.proposed.collect { case u: UpdateAddHtlc => u }.size,
1747-
remoteNextHtlcId = d.commitments.remoteNextHtlcId - d.commitments.remoteChanges.proposed.collect { case u: UpdateAddHtlc => u }.size)
1748-
log.debug(s"localNextHtlcId=${d.commitments.localNextHtlcId}->${commitments1.localNextHtlcId}")
1749-
log.debug(s"remoteNextHtlcId=${d.commitments.remoteNextHtlcId}->${commitments1.remoteNextHtlcId}")
1725+
val commitments1 = Commitments.discardUnsignedUpdates(d.commitments)
17501726

17511727
commitments1.remoteNextCommitInfo match {
17521728
case Left(_) =>
@@ -1816,16 +1792,13 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder, remo
18161792

18171793
case Event(channelReestablish: ChannelReestablish, d: DATA_SHUTDOWN) =>
18181794
Syncing.checkSync(keyManager, d, channelReestablish) match {
1819-
case res: SyncResult.InSync =>
1820-
val commitments1 = d.commitments.copy(
1821-
localChanges = d.commitments.localChanges.copy(proposed = Nil),
1822-
remoteChanges = d.commitments.remoteChanges.copy(proposed = Nil),
1823-
localNextHtlcId = d.commitments.localNextHtlcId - d.commitments.localChanges.proposed.collect { case u: UpdateAddHtlc => u }.size,
1824-
remoteNextHtlcId = d.commitments.remoteNextHtlcId - d.commitments.remoteChanges.proposed.collect { case u: UpdateAddHtlc => u }.size)
1825-
val sendQueue = Queue.empty[LightningMessage] ++ res.retransmit :+ d.localShutdown
1795+
case syncFailure: SyncResult.Failure =>
1796+
handleSyncFailure(channelReestablish, syncFailure, d)
1797+
case syncSuccess: SyncResult.Success =>
1798+
val commitments1 = Commitments.discardUnsignedUpdates(d.commitments)
1799+
val sendQueue = Queue.empty[LightningMessage] ++ syncSuccess.retransmit :+ d.localShutdown
18261800
// BOLT 2: A node if it has sent a previous shutdown MUST retransmit shutdown.
18271801
goto(SHUTDOWN) using d.copy(commitments = commitments1) sending sendQueue
1828-
case _ => ??? // TODO
18291802
}
18301803

18311804
case Event(_: ChannelReestablish, d: DATA_NEGOTIATING) =>
@@ -2295,6 +2268,29 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder, remo
22952268
stay() using d.copy(channelUpdate = channelUpdate1) storing()
22962269
}
22972270

2271+
private def handleSyncFailure(channelReestablish: ChannelReestablish, syncFailure: SyncResult.Failure, d: HasCommitments) = {
2272+
syncFailure match {
2273+
case res: SyncResult.LocalLateProven =>
2274+
log.warning(s"counterparty proved that we have an outdated (revoked) local commitment!!! ourLocalCommitmentNumber=${res.ourLocalCommitmentNumber} theirRemoteCommitmentNumber=${res.theirRemoteCommitmentNumber}")
2275+
// their data checks out, we indeed seem to be using an old revoked commitment, and must absolutely *NOT* publish it, because that would be a cheating attempt and they
2276+
// would punish us by taking all the funds in the channel
2277+
handleOutdatedCommitment(channelReestablish, d)
2278+
case res: Syncing.SyncResult.LocalLateUnproven =>
2279+
log.warning(s"our local commitment is in sync, but counterparty says that they have a more recent remote commitment than the one we know of (they could be lying)!!! ourRemoteCommitmentNumber=${res.ourRemoteCommitmentNumber} theirCommitmentNumber=${res.theirLocalCommitmentNumber}")
2280+
// there is no way to make sure that they are saying the truth, the best thing to do is ask them to publish their commitment right now
2281+
// maybe they will publish their commitment, in that case we need to remember their commitment point in order to be able to claim our outputs
2282+
// note that if they don't comply, we could publish our own commitment (it is not stale, otherwise we would be in the case above)
2283+
handleOutdatedCommitment(channelReestablish, d)
2284+
case res: Syncing.SyncResult.RemoteLying =>
2285+
// they are deliberately trying to fool us into thinking we have a late commitment
2286+
log.warning(s"counterparty is lying about us having an outdated commitment!!! ourLocalCommitmentNumber=${res.ourLocalCommitmentNumber} theirRemoteCommitmentNumber=${res.theirRemoteCommitmentNumber}")
2287+
handleLocalError(InvalidRevokedCommitProof(d.channelId, res.ourLocalCommitmentNumber, res.theirRemoteCommitmentNumber, res.invalidPerCommitmentSecret), d, Some(channelReestablish))
2288+
case SyncResult.RemoteLate =>
2289+
log.warning("counterparty appears to be using an outdated commitment, they may request a force-close, standing by...")
2290+
stay()
2291+
}
2292+
}
2293+
22982294
private def maybeEmitChannelUpdateChangedEvent(newUpdate: ChannelUpdate, oldUpdate_opt: Option[ChannelUpdate], d: DATA_NORMAL): Unit = {
22992295
if (oldUpdate_opt.isEmpty || !Announcements.areSameIgnoreFlags(newUpdate, oldUpdate_opt.get)) {
23002296
context.system.eventStream.publish(ChannelUpdateParametersChanged(self, d.channelId, newUpdate.shortChannelId, d.commitments.remoteNodeId, newUpdate))

‎eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -875,6 +875,22 @@ object Commitments {
875875
(commitTx, htlcTxs)
876876
}
877877

878+
/**
879+
* When reconnecting, we drop all outgoing unsigned changes.
880+
*/
881+
def discardUnsignedUpdates(commitments: Commitments)(implicit log: LoggingAdapter): Commitments = {
882+
log.debug("discarding proposed OUT: {}", commitments.localChanges.proposed.map(msg2String(_)).mkString(","))
883+
log.debug("discarding proposed IN: {}", commitments.remoteChanges.proposed.map(msg2String(_)).mkString(","))
884+
val commitments1 = commitments.copy(
885+
localChanges = commitments.localChanges.copy(proposed = Nil),
886+
remoteChanges = commitments.remoteChanges.copy(proposed = Nil),
887+
localNextHtlcId = commitments.localNextHtlcId - commitments.localChanges.proposed.collect { case u: UpdateAddHtlc => u }.size,
888+
remoteNextHtlcId = commitments.remoteNextHtlcId - commitments.remoteChanges.proposed.collect { case u: UpdateAddHtlc => u }.size)
889+
log.debug(s"localNextHtlcId=${commitments.localNextHtlcId}->${commitments1.localNextHtlcId}")
890+
log.debug(s"remoteNextHtlcId=${commitments.remoteNextHtlcId}->${commitments1.remoteNextHtlcId}")
891+
commitments1
892+
}
893+
878894
def msg2String(msg: LightningMessage): String = msg match {
879895
case u: UpdateAddHtlc => s"add-${u.id}"
880896
case u: UpdateFulfillHtlc => s"ful-${u.id}"

‎eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -318,11 +318,12 @@ object Helpers {
318318
// @formatter:off
319319
sealed trait SyncResult
320320
object SyncResult {
321-
case class InSync(retransmit: Seq[LightningMessage]) extends SyncResult
322-
case class LocalLateProven(ourLocalCommitmentNumber: Long, theirRemoteCommitmentNumber: Long) extends SyncResult
323-
case class LocalLateUnproven(ourRemoteCommitmentNumber: Long, theirLocalCommitmentNumber: Long) extends SyncResult
324-
case class RemoteLying(ourLocalCommitmentNumber: Long, theirRemoteCommitmentNumber: Long, invalidPerCommitmentSecret: PrivateKey) extends SyncResult
325-
case object RemoteLate extends SyncResult
321+
case class Success(retransmit: Seq[LightningMessage]) extends SyncResult
322+
sealed trait Failure extends SyncResult
323+
case class LocalLateProven(ourLocalCommitmentNumber: Long, theirRemoteCommitmentNumber: Long) extends Failure
324+
case class LocalLateUnproven(ourRemoteCommitmentNumber: Long, theirLocalCommitmentNumber: Long) extends Failure
325+
case class RemoteLying(ourLocalCommitmentNumber: Long, theirRemoteCommitmentNumber: Long, invalidPerCommitmentSecret: PrivateKey) extends Failure
326+
case object RemoteLate extends Failure
326327
}
327328
// @formatter:on
328329

@@ -346,15 +347,15 @@ object Helpers {
346347
val commitSig = waitingForRevocation.sent
347348
retransmitRevocation_opt match {
348349
case None =>
349-
SyncResult.InSync(retransmit = signedUpdates :+ commitSig)
350+
SyncResult.Success(retransmit = signedUpdates :+ commitSig)
350351
case Some(revocation) if d.commitments.localCommit.index > waitingForRevocation.sentAfterLocalCommitIndex =>
351-
SyncResult.InSync(retransmit = signedUpdates :+ commitSig :+ revocation)
352+
SyncResult.Success(retransmit = signedUpdates :+ commitSig :+ revocation)
352353
case Some(revocation) =>
353-
SyncResult.InSync(retransmit = revocation +: signedUpdates :+ commitSig)
354+
SyncResult.Success(retransmit = revocation +: signedUpdates :+ commitSig)
354355
}
355356
case Left(waitingForRevocation) if remoteChannelReestablish.nextLocalCommitmentNumber == (waitingForRevocation.nextRemoteCommit.index + 1) =>
356357
// we just sent a new commit_sig, they have received it but we haven't received their revocation
357-
SyncResult.InSync(retransmit = retransmitRevocation_opt.toSeq)
358+
SyncResult.Success(retransmit = retransmitRevocation_opt.toSeq)
358359
case Left(waitingForRevocation) if remoteChannelReestablish.nextLocalCommitmentNumber < waitingForRevocation.nextRemoteCommit.index =>
359360
// they are behind
360361
SyncResult.RemoteLate
@@ -366,7 +367,7 @@ object Helpers {
366367
)
367368
case Right(_) if remoteChannelReestablish.nextLocalCommitmentNumber == (d.commitments.remoteCommit.index + 1) =>
368369
// they have acknowledged the last commit_sig we sent
369-
SyncResult.InSync(retransmit = retransmitRevocation_opt.toSeq)
370+
SyncResult.Success(retransmit = retransmitRevocation_opt.toSeq)
370371
case Right(_) if remoteChannelReestablish.nextLocalCommitmentNumber < (d.commitments.remoteCommit.index + 1) =>
371372
// they are behind
372373
SyncResult.RemoteLate

0 commit comments

Comments
 (0)