Skip to content

Stricter batching of commit_sig messages on the wire #3083

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
merged 3 commits into from
May 23, 2025
Merged

Stricter batching of commit_sig messages on the wire #3083

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
17c9eb9
Clean-up `TransportHandler`
t-bast Apr 9, 2025
5dbd63a
Send splice `commit_sig` as a batch
t-bast May 13, 2025
71fb4fa
Batch incoming `commit_sig` in `PeerConnection`
t-bast May 15, 2025
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
17 changes: 10 additions & 7 deletions eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -993,7 +993,7 @@ case class Commitments(params: ChannelParams,
}
}

def sendCommit(channelKeys: ChannelKeys)(implicit log: LoggingAdapter): Either[ChannelException, (Commitments, Seq[CommitSig])] = {
def sendCommit(channelKeys: ChannelKeys)(implicit log: LoggingAdapter): Either[ChannelException, (Commitments, CommitSigs)] = {
remoteNextCommitInfo match {
case Right(_) if !changes.localHasChanges => Left(CannotSignWithoutChanges(channelId))
case Right(remoteNextPerCommitmentPoint) =>
Expand All @@ -1007,21 +1007,24 @@ case class Commitments(params: ChannelParams,
active = active1,
remoteNextCommitInfo = Left(WaitForRev(localCommitIndex))
)
Right(commitments1, sigs)
Right(commitments1, CommitSigs(sigs))
case Left(_) => Left(CannotSignBeforeRevocation(channelId))
}
}

def receiveCommit(commits: Seq[CommitSig], channelKeys: ChannelKeys)(implicit log: LoggingAdapter): Either[ChannelException, (Commitments, RevokeAndAck)] = {
def receiveCommit(commitSigs: CommitSigs, channelKeys: ChannelKeys)(implicit log: LoggingAdapter): Either[ChannelException, (Commitments, RevokeAndAck)] = {
// We may receive more commit_sig than the number of active commitments, because there can be a race where we send
// splice_locked while our peer is sending us a batch of commit_sig. When that happens, we simply need to discard
// the commit_sig that belong to commitments we deactivated.
if (commits.size < active.size) {
return Left(CommitSigCountMismatch(channelId, active.size, commits.size))
val sigs = commitSigs match {
case batch: CommitSigBatch if batch.batchSize < active.size => return Left(CommitSigCountMismatch(channelId, active.size, batch.batchSize))
case batch: CommitSigBatch => batch.messages
case _: CommitSig if active.size > 1 => return Left(CommitSigCountMismatch(channelId, active.size, 1))
case commitSig: CommitSig => Seq(commitSig)
}
val commitKeys = LocalCommitmentKeys(params, channelKeys, localCommitIndex + 1)
// Signatures are sent in order (most recent first), calling `zip` will drop trailing sigs that are for deactivated/pruned commitments.
val active1 = active.zip(commits).map { case (commitment, commit) =>
val active1 = active.zip(sigs).map { case (commitment, commit) =>
commitment.receiveCommit(params, channelKeys, commitKeys, changes, commit) match {
case Left(f) => return Left(f)
case Right(commitment1) => commitment1
Expand Down Expand Up @@ -1156,7 +1159,7 @@ case class Commitments(params: ChannelParams,
case ChannelSpendSignature.IndividualSignature(latestRemoteSig) => latestRemoteSig == commitSig.signature
case ChannelSpendSignature.PartialSignatureWithNonce(_, _) => ???
}
params.channelFeatures.hasFeature(Features.DualFunding) && commitSig.batchSize == 1 && isLatestSig
params.channelFeatures.hasFeature(Features.DualFunding) && isLatestSig
}

def localFundingSigs(fundingTxId: TxId): Option[TxSignatures] = {
Expand Down
8 changes: 4 additions & 4 deletions eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -478,14 +478,14 @@ object Helpers {
// we just sent a new commit_sig but they didn't receive it
// we resend the same updates and the same sig, and preserve the same ordering
val signedUpdates = commitments.changes.localChanges.signed
val commitSigs = commitments.active.flatMap(_.nextRemoteCommit_opt).map(_.sig)
val commitSigs = CommitSigs(commitments.active.flatMap(_.nextRemoteCommit_opt).map(_.sig))
retransmitRevocation_opt match {
case None =>
SyncResult.Success(retransmit = signedUpdates ++ commitSigs)
SyncResult.Success(retransmit = signedUpdates :+ commitSigs)
case Some(revocation) if commitments.localCommitIndex > waitingForRevocation.sentAfterLocalCommitIndex =>
SyncResult.Success(retransmit = signedUpdates ++ commitSigs ++ Seq(revocation))
SyncResult.Success(retransmit = signedUpdates :+ commitSigs :+ revocation)
case Some(revocation) =>
SyncResult.Success(retransmit = Seq(revocation) ++ signedUpdates ++ commitSigs)
SyncResult.Success(retransmit = revocation +: signedUpdates :+ commitSigs)
}
case Left(_) if remoteChannelReestablish.nextLocalCommitmentNumber == (commitments.nextRemoteCommitIndex + 1) =>
// we just sent a new commit_sig, they have received it but we haven't received their revocation
Expand Down
Loading