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 #2934

Merged
merged 1 commit into from
Oct 24, 2024
Merged

Conversation

t-bast
Copy link
Member

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

This is clearer that way and yields the same result: we only look at txOut amounts when computing each node's contributions.

See a related lightning-kmp PR: ACINQ/lightning-kmp#724

This is clearer that way and yields the same result: we only look at
`txOut` amounts when computing each node's contributions.
@t-bast t-bast requested review from pm47 and remyers October 24, 2024 04:23
@remyers
Copy link
Contributor

remyers commented Oct 24, 2024

When I double checked the spec I did not see that the shared input should be considered at all. What is the rational for counting the total shared input towards towards the amount of the initiating node?

Based on the current wording of the spec it seems we should only count the tx_add_input values and ignore the shared input:

The sending node:
  - if it has the lowest total satoshis contributed, as defined by total
    `tx_add_input` values, or both peers have contributed equal amounts
    but it has the lowest `node_id` (sorted lexicographically):
    - MUST transmit their `tx_signatures` first

@t-bast
Copy link
Member Author

t-bast commented Oct 24, 2024

The shared input is sent to the other node with tx_add_input, by the splice initiator:

The sending node:
  - If it is the splice initiator:
    - MUST add the current channel input to the splice transaction by
      sending `tx_add_input` with `shared_input_txid` containing the
      `txid` of the latest funding transaction.

Since there's no reason to explicitly exclude this tx_add_input (I don't see why we'd want to exclude it?), we thus need to make its amount count for the splice initiator.

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! thanks for the explanation of why the initiator should also count the sharedInput towards their total.

@t-bast t-bast merged commit 4ca8ea0 into master Oct 24, 2024
1 check passed
@t-bast t-bast deleted the clarify-should-sign-first branch October 24, 2024 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants