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

Use shared input's txOut in shouldSignFirst #724

Merged
merged 1 commit into from
Oct 24, 2024
Merged

Conversation

t-bast
Copy link
Member

@t-bast t-bast commented Oct 24, 2024

When deciding who must send tx_signatures first for a splice, we compute how much each node contributes to the splice transaction by adding the amount of the tx_add_input they have sent.

For the shared input (current channel output), instead of using the txOut amount, we previously summed the balance of each node in the channel. The result is the same when there are no pending HTLCs, but when there are pending HTLCs, it underestimates the amount of this input.

This is hard to reproduce (and test) because it requires both nodes to contribute to the splice while having large pending HTLCs that tilt the balance in favor of the wrong side.

When deciding who must send `tx_signatures` first for a splice, we
compute how much each node contributes to the splice transaction by
adding the amount of the `tx_add_input` they have sent.

For the shared input (current channel output), instead of using the
`txOut` amount, we previously summed the balance of each node in the
channel. The result is the same when there are no pending HTLCs, but
when there are pending HTLCs, it underestimates the amount of this
input.

This is hard to reproduce (and test) because it requires both nodes to
contribute to the splice while having large pending HTLCs that tilt the
balance in favor of the wrong side.
@t-bast t-bast requested review from pm47 and remyers October 24, 2024 03:25
@@ -1147,7 +1147,7 @@ data class InteractiveTxSigningSession(
}

fun shouldSignFirst(isInitiator: Boolean, channelParams: ChannelParams, tx: SharedTransaction): Boolean {
val sharedAmountIn = tx.sharedInput?.let { it.localAmount + it.remoteAmount } ?: 0.msat
val sharedAmountIn = tx.sharedInput?.txOut?.amount?.toMilliSatoshi() ?: 0.msat
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that the definition of txOut in InteractiveTxInput.Shared includes the pending HTLCs.

@t-bast t-bast added the bug Something isn't working label Oct 24, 2024
Copy link
Contributor

@remyers remyers left a comment

Choose a reason for hiding this comment

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

LGTM!

@t-bast t-bast merged commit 0537e61 into master Oct 24, 2024
2 checks passed
@t-bast t-bast deleted the tx-sigs-pending-htlcs branch October 24, 2024 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants