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

Satoshi amounts must be used for htlc comparison #1804

Closed
pm47 opened this issue May 14, 2021 · 1 comment · Fixed by #1806
Closed

Satoshi amounts must be used for htlc comparison #1804

pm47 opened this issue May 14, 2021 · 1 comment · Fixed by #1806
Labels

Comments

@pm47
Copy link
Member

pm47 commented May 14, 2021

Spec says that the equality is based on outputs, which are in sats:

Lexicographic ordering: see BIP69. In the case of identical HTLC outputs, the outputs are ordered in increasing cltv_expiry order.

We're using the millisatoshi amount:

def sort(a: CommitmentOutputLink[CommitmentOutput], b: CommitmentOutputLink[CommitmentOutput]): Boolean = (a.commitmentOutput, b.commitmentOutput) match {
case (OutHtlc(OutgoingHtlc(htlcA)), OutHtlc(OutgoingHtlc(htlcB))) if htlcA.paymentHash == htlcB.paymentHash && htlcA.amountMsat == htlcB.amountMsat =>
htlcA.cltvExpiry <= htlcB.cltvExpiry
case _ => LexicographicalOrdering.isLessThan(a.output, b.output)
}
}

This causes commitment sig mismatch and force closes.

Thanks @Roasbeef for the debug!

@pm47 pm47 added the bug label May 14, 2021
@pm47
Copy link
Member Author

pm47 commented May 14, 2021

Related to lightning/bolts#872.

pm47 added a commit that referenced this issue May 16, 2021
pm47 added a commit that referenced this issue May 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant