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

Update splice messages according to new spec draft #3129

Merged
merged 1 commit into from
Jun 27, 2024

Conversation

optout21
Copy link
Contributor

Some changes to the splice messages according to new spec draft lightning/bolts#1160
The splicing messages are not yet used (on main).

The list of changes (some new fields, some renames):

  • CommitmentSigned: New optional batch parameters, used in case multiple commitment are updated at once. Optional batch field of type CommitmentSignedBatch, containing batch size and funding txid.
  • TxAddInput: New optional field shared_input_txid, to be used when during splicing the previous funding transaction is added as an input (funding_txid).
  • TxSignatures: Rename optional field to shared_input_signature, from funding_outpoint_sig.
  • SpliceInit: Rename msg to SpliceInit, from Splice (msg is now splice_init in spec) (also SendSpliceInit, handle_splice_init)
  • SpliceInit: Rename amount field to funding_contribution_satoshis (from relative_satoshis). Note that his is the part of the initiator only.
  • SpliceInit: New optional bool field require_confirmed_inputs.
  • SpliceInit: The chain_hash field is removed (not needed).
  • SpliceAck: Rename amount field to funding_contribution_satoshis (from relative_satoshis). Note that his is the part of the acceptor only.
  • SpliceAck: New optional bool field require_confirmed_inputs.
  • SpliceAck: The chain_hash field is removed (not needed).
  • SpliceLocked: Added new splice_txid field, with the id of the new locked funding tx
  • Change wire type for SpliceInit and SpliceAck to 80 & 81 (from 75 & 76)

@codecov-commenter
Copy link

codecov-commenter commented Jun 17, 2024

Codecov Report

Attention: Patch coverage is 88.57143% with 4 lines in your changes missing coverage. Please review.

Project coverage is 91.15%. Comparing base (07f3380) to head (43a6ee3).
Report is 31 commits behind head on main.

Files Patch % Lines
lightning/src/ln/peer_handler.rs 0.00% 2 Missing ⚠️
lightning/src/ln/channelmanager.rs 0.00% 1 Missing ⚠️
lightning/src/ln/functional_test_utils.rs 0.00% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3129      +/-   ##
==========================================
+ Coverage   89.92%   91.15%   +1.22%     
==========================================
  Files         121      121              
  Lines       99172   107295    +8123     
  Branches    99172   107295    +8123     
==========================================
+ Hits        89180    97803    +8623     
+ Misses       7391     7009     -382     
+ Partials     2601     2483     -118     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@optout21 optout21 force-pushed the splicing-msgs-update branch 2 times, most recently from 7423566 to 35a08ae Compare June 17, 2024 13:03
TheBlueMatt
TheBlueMatt previously approved these changes Jun 17, 2024
Copy link
Collaborator

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

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

Didn't bother to check if it matches the spec, we'll find out when we try to use it if it doesnt.

@optout21 optout21 force-pushed the splicing-msgs-update branch from 35a08ae to bb84c5f Compare June 18, 2024 10:44
@optout21 optout21 marked this pull request as ready for review June 18, 2024 13:40
@optout21
Copy link
Contributor Author

Pushed some minor fuzz fixes; ready for review.

@jkczyz jkczyz self-requested a review June 18, 2024 14:20
lightning/src/ln/msgs.rs Outdated Show resolved Hide resolved
lightning/src/ln/msgs.rs Outdated Show resolved Hide resolved
lightning/src/ln/msgs.rs Outdated Show resolved Hide resolved
@optout21 optout21 force-pushed the splicing-msgs-update branch from bb84c5f to 43a6ee3 Compare June 24, 2024 11:06
@optout21 optout21 requested a review from jkczyz June 24, 2024 11:08
Copy link
Contributor

@dunxen dunxen left a comment

Choose a reason for hiding this comment

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

LGTM. Bound to be some additional changes coming up anyway, but I've reviewed what's here against the current draft messages.

@jkczyz jkczyz merged commit 3ccf064 into lightningdevkit:main Jun 27, 2024
15 of 16 checks passed
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.

5 participants