[Splicing] Tx negotiation during splicing #3736
Conversation
|
👋 Thanks for assigning @wpaulino as a reviewer! |
optout21
force-pushed
the
splice-dual-tx4
branch
from
7f6dfbd to
c3778bc
Compare
1 similar comment
1 similar comment
1 similar comment
1 similar comment
1 similar comment
1 similar comment
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3736 +/- ##
==========================================
+ Coverage 89.73% 90.28% +0.55%
==========================================
Files 159 160 +1
Lines 128910 132032 +3122
Branches 128910 132032 +3122
==========================================
+ Hits 115676 119207 +3531
+ Misses 10536 10172 -364
+ Partials 2698 2653 -45 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
optout21
force-pushed
the
splice-dual-tx4
branch
3 times, most recently
from
88d2e83 to
866368d
Compare
|
Ready for a new round of review. I have addressed the comments, applied most of them. There is still one to-do (update channel reserve values), that I will do, but the rest is ready for review. |
|
Ready for a new round of review. All pending and newly raised comments addressed. |
optout21
force-pushed
the
splice-dual-tx4
branch
from
59e9b23 to
3577418
Compare
| fn begin_interactive_funding_tx_construction<ES: Deref>( | ||
| &mut self, signer_provider: &SP, entropy_source: &ES, holder_node_id: PublicKey, | ||
| change_destination_opt: Option<ScriptBuf>, | ||
| is_initiator: bool, change_destination_opt: Option<ScriptBuf>, | ||
| prev_funding_input: Option<(TxIn, TransactionU16LenLimited)>, |
There was a problem hiding this comment.
Should we just drop this commit and revisit once #3842 lands? Or we can just drop the prev_funding_input part for now.
|
🔔 1st Reminder Hey @jkczyz! This PR has been waiting for your review. |
|
🔔 2nd Reminder Hey @jkczyz! This PR has been waiting for your review. |
optout21
force-pushed
the
splice-dual-tx4
branch
from
3577418 to
586a094
Compare
|
Comments addressed, minor fixes. Ready for review. |
| fn begin_interactive_funding_tx_construction<ES: Deref>( | ||
| &mut self, signer_provider: &SP, entropy_source: &ES, holder_node_id: PublicKey, | ||
| change_destination_opt: Option<ScriptBuf>, | ||
| is_initiator: bool, change_destination_opt: Option<ScriptBuf>, |
There was a problem hiding this comment.
If we make is_initiator part of FundingNegotiationContext, it seems like we could just provide a reference of it to calculate_change_output_value to clean things up a bit.
There was a problem hiding this comment.
FundingNegotiationContext is defined in channel.rs, and using it in interactivetxs.rs doesn't seem right.
| self.funding.value_to_self_msat, | ||
| our_funding_contribution_satoshis, | ||
| ); | ||
| // TODO: Early check for reserve requirement |
There was a problem hiding this comment.
You still have a PR open for this that needs to be updated
There was a problem hiding this comment.
Correct, #3641, I've just done some fixes on that as well. It's not a hard dependency.
|
🔔 4th Reminder Hey @jkczyz! This PR has been waiting for your review. |
This is a simple rename, DualFundingContext to FundingNegotiationContext, to suggest that this is use not only in dual-funded channel open. Also rename the field dual_funding_context to funding_negotiation_context.
The begin_interactive_funding_tx_construction() method is extended with `is_initiator` parameter (splice initiator), and `prev_funding_input` optional parameter, containing the previous funding transaction, which will be added to the negotiation as an input by the initiator.
Introduce struct NegotiatingChannelView to perform transaction negotiation logic, on top of either PendingV2Channel (dual-funded channel open) or FundedChannel (splicing).
optout21
force-pushed
the
splice-dual-tx4
branch
from
0afb374 to
79b8083
Compare
|
Rebased, some new comments addressed ( |
Fill the logic for including transaction negotiation during splicing, implement the functions: splice_channel, splice_init, splice_ack, funding_tx_constructed. Also extend the test case test_v1_splice_in with the steps for funding negotiation during splicing.
Thanks for the heads up, I did miss some comments that are visible in the Files Changed, but not in the main Conversation tab. I will address those. |
optout21
force-pushed
the
splice-dual-tx4
branch
from
79b8083 to
38e773a
Compare
optout21
force-pushed
the
splice-dual-tx4
branch
from
38e773a to
596e052
Compare
|
Addressed some pending comments, I've rechecked all outstanding (some only show up in Files Changed view), and currently I don't see any items I could address or need to do changes for.
|
Implementation of transaction negotiation during splicing.
Builds on 3407 and 3443.
No new phase,Funded(FundedChannel)is used throughout splicingFundedChannelandPendingV2Channelcan act as a transaction constructorPendingV2Channellogic is put behind a trait --FundingTxConstructorV2RenegotiatingScopeis used to store extra state during splicingFundingChannelcan act as aFundingTxConstructorV2, using the state fromRenegotiatingScope(if present)Since bothFundedChannelandFundingTxConstructorhas context(), context accessors are extracted into a common base trait,ChannelContextProvider(it is also shared byInitialRemoteCommitmentReceiver).(Also relevant: #3444)