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 to select funding inputs before sending open_channel2 and splice_init #2903

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

remyers
Copy link
Contributor

@remyers remyers commented Sep 5, 2024

When funds are added to a dual funded channel, or spliced into an existing channel, our channel should be credited in their channel balance any excess that is added to the channel over what the user requests. Currently if a changeless solution is found for funding, excess value over what was requested will be treated as waste and used as extra fees.

The motivation for this change is to take advantage of the new Bitcoin channel selection options proposed in PR 30080 to retain excess funding from changeless transactions and add it to our channel balance.

We introduce a new optional funding contributions parameter to InteractiveTxBuilder. When set, we will start by processing the passed in funding inputs/outputs instead of using InteractiveTxFunder to fund the transaction. The call to InteractiveTxFunder is now made before sending open_channel2 or splice_init and the results are then passed to InteractiveTxBuilder.

This PR builds on PR #2887 - Use final spec values for splicing.

t-bast and others added 7 commits August 1, 2024 18:17
We replace our experimental version of `splice_init`, `splice_ack` and
`splice_locked` by their official version. If our peer is using the
experimental feature bit, we convert our outgoing messages to use the
experimental encoding and incoming messages to the official messages.

We also change the TLV fields added to `tx_add_input`, `tx_signatures`
and `splice_locked` to match the spec version. We always write both the
official and experimental TLV to updated nodes (because the experimental
one is odd and will be ignored) but we drop the official TLV if our
peer is using the experimental feature, because it won't understand the
even TLV field.

This guarantees backwards-compatibility with peers who only support the
experimental feature.
We initially supported splicing with a poor man's quiescence, where we
allowed splice messages if the commitments were already quiescent.

We've shipped support for quiescence since then, which means that new
even nodes relying on experimental splicing should support quiescence.
We can thus remove support for the non-quiescent version.
If the latest splice transaction doesn't confirm, we allow exchanging
`tx_init_rbf` and `tx_ack_rbf` to create another splice transaction to
replace it. We use the same funding contribution as the previous splice.

We disallow creating another splice transaction using `splice_init` if
we have several RBF attempts for the latest splice: we cannot know which
one of them will confirm and should be spent by the new splice.
When 0-conf isn't used, we reject `splice_init` while the previous
splice transaction hasn't confirmed. Our peer should either use RBF
instead of creating a new splice, or they should wait for our node
to receive the block that confirmed the previous transaction. This
protects against chains of unconfirmed transactions.
This change passes `maxExcess` to Peer via OpenChannel and from Peer to Channel via INPUT_INIT_CHANNEL_INITIATOR. The `maxExcess` parameter will be used by bitcoind funding calls but is not currently used.
…ion calls

These parameters are only supported in a testing branch of bitcoind for now.
This will be used by non-initiators when funding dual funded channels and splicing
@t-bast
Copy link
Member

t-bast commented Sep 5, 2024

I haven't done a thorough review yet, but the approach is looking good, concept ACK 👍.

This is even less invasive than I thought, I was afraid we would have to do the funding in the Peer, but we can actually insert a step like you did directly in the channel FSM: this should be working fine.

It's a good thing we had already separated the funding step (InteractiveTxFunder) from the actual interactive-tx state machine (InteractiveTxBuilder), it makes it quite easy to implement this change.

@remyers remyers force-pushed the 2409-funder-first-dual-official branch 2 times, most recently from 81ab7d2 to dc102b5 Compare September 10, 2024 14:49
…ce_init

 - If `addExcessToRecipientPosition_opt` is set, excess from funding (if any) will be added to the proposed `fundingAmount`/`fundingContribution` before sending `open_channel2`/`splice_init` respectively.
 - We assume our peer requires confirmed inputs. In the future we could add a heuristic for this, but it's safer to assume they want confirmed inputs.
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