Skip to content

Conversation

@vincenzopalazzo
Copy link
Collaborator

The signer needs to know when the splice operation starts and the splice parameters for each splice transaction candidate.

The channel establishment v2 (dual funding) code path already notifies the signer via the hsmd API hsmd_ready_channel calls However, the splicing code path does not.

Fixes: #6723

Suggested-by: @devrandom
Co-Developed-by: @devrandom
Co-Developed-by: Ken Sedgwick ken@bonsai.com

@vincenzopalazzo vincenzopalazzo added this to the v23.11 milestone Oct 5, 2023
@vincenzopalazzo vincenzopalazzo self-assigned this Oct 5, 2023
@vincenzopalazzo vincenzopalazzo force-pushed the macros/remote-signer-with-splicing branch from cf18fa6 to 555ecf6 Compare October 5, 2023 08:03
@vincenzopalazzo vincenzopalazzo force-pushed the macros/remote-signer-with-splicing branch from 3fe70d0 to 6b79651 Compare October 10, 2023 19:42
@vincenzopalazzo vincenzopalazzo requested review from devrandom and ksedgwic and removed request for ddustin and devrandom October 10, 2023 19:42
@vincenzopalazzo
Copy link
Collaborator Author

We discuss the semantics of the push_value with the VLS team, and we conclude that the push_value may be used as a control value. This mean with splice that the push value is the funded relative amount that the remote node is pushing inside the channel after the tx_complete message exchange.

P.S: This semantics can change in the future iirc @devrandom and @ksedgwic are still thinking about all the design possibility

@devrandom
Copy link
Contributor

devrandom commented Oct 10, 2023

[deleted - attached message to wrong PR]

Copy link
Contributor

@devrandom devrandom left a comment

Choose a reason for hiding this comment

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

this generally looks fine, but I didn't check all the call sites in detail

@vincenzopalazzo
Copy link
Collaborator Author

but I didn't check all the call sites in detail

I follow the @ksedgwic call here, but we call the hsmd function in two places:

  • splice_accepter, after the tx_complete is exchanged
  • splice_initiator_user_finalized: after the tx_complete is exchanged

and we set the push_value as described before and the channel_value to the total channel amount including splice values and inflight update.

Let me know if this needs some more love somewhere :)

@vincenzopalazzo vincenzopalazzo force-pushed the macros/remote-signer-with-splicing branch from 6b79651 to 452dfad Compare October 11, 2023 08:52
@vincenzopalazzo

This comment was marked as outdated.

@vincenzopalazzo vincenzopalazzo force-pushed the macros/remote-signer-with-splicing branch 2 times, most recently from 716d2a5 to cff36f1 Compare October 11, 2023 08:53
@ksedgwic
Copy link
Collaborator

The assertion in libhsmd.c:

	/* Fail fast if any values are uninitialized or obviously wrong. */
	...
	assert(amount_msat_less_eq(push_value, value_msat));
	...

may not make sense if we change the meaning of push_value to be relative, especially since negative values appear as large positive?

I recommend striking that assertion; I don't think there is any useful assertion to replace it with.

Copy link
Collaborator

@ksedgwic ksedgwic left a comment

Choose a reason for hiding this comment

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

This looks good to me.

@ksedgwic
Copy link
Collaborator

I still think we need to strike (remove) the assertion in libhsmd.c as described above #6746 (comment)

@vincenzopalazzo
Copy link
Collaborator Author

I still think we need to strike (remove) the assertion in libhsmd.c as described above #6746 (comment)

Thanks @ksedgwic, I missed this.

I pushed it on d10f33b because I think it deserves a proper commit explanation

ksedgwic and others added 2 commits October 20, 2023 09:01
The signer needs to know when the splice operation starts and the
splice parameters for each splice transaction candidate.

The channel establishment v2 (dual funding) code path already
notifies the signer via the hsmd API hsmd_ready_channel calls
However, the splicing code path does not.

Link: ElementsProject#6723
Suggested-by: @devrandom
Co-Developed-by: @devrandom
Co-Developed-by: Ken Sedgwick <ken@bonsai.com>
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
The assertion may not make sense if we change the
meaning of `push_value` to be relative, especially since
negative values appear as large positive.

Suggested-by: Ken Sedgwick <ken@bonsai.com>
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
@vincenzopalazzo vincenzopalazzo force-pushed the macros/remote-signer-with-splicing branch from cff36f1 to a8e9cd6 Compare October 20, 2023 07:01
@nepet nepet requested a review from ksedgwic October 20, 2023 15:40
Copy link
Collaborator

@ksedgwic ksedgwic left a comment

Choose a reason for hiding this comment

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

ACK a8e9cd6b

Copy link
Contributor

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

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

Ack a8e9cd6

@rustyrussell rustyrussell merged commit a6e1f19 into ElementsProject:master Oct 23, 2023
@vincenzopalazzo vincenzopalazzo deleted the macros/remote-signer-with-splicing branch October 23, 2023 17:08
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 this pull request may close these issues.

splice: signer must be informed of splice parameters

4 participants