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

WIP: Dual Funding (v2 Channel Establishment protocol) #524

Open
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@niftynei
Copy link
Contributor

niftynei commented Dec 5, 2018

This is a first draft proposal for adding a second open channel version. This will empower either side of the channel to contribute to the funding transaction.

Broadly speaking, the largest change between this and version one of open_channel is that both sides now construct and broadcast the funding transaction. This adds a few more communication rounds, as the commitment transaction signatures can't be exchanged until both sides have the info necessary to construct the funding transaction.

Errata

@rustyrussell
Copy link
Collaborator

rustyrussell left a comment

This is generally good, and I like the rbf flow. @Roasbeef suggested we keep it more minimal (ie. no change of inputs), but that doesn't gain very much compared to this approach, AFAICT.

I've bikeshedded a few things, because that's what we do :)

- MUST fail the channel if:
- `change_satoshis` would be negative.
- if has not yet sent a `funding_compose`:
- MUST send their `funding_compose` message.

This comment has been minimized.

@rustyrussell

rustyrussell Dec 10, 2018

Collaborator

s/their/its/

- MUST send their `funding_compose` message.
- otherwise:
- MUST use the sent and received `input_info` and `output_info`
to create the funding transaction, using `max_extra_witness_len`

This comment has been minimized.

@rustyrussell

rustyrussell Dec 10, 2018

Collaborator

It's now called 'max_witness_len'.

the `put_limit`.
- otherwise:
- MUST fail the channel if:
- `change_satoshis` would be negative.

This comment has been minimized.

@rustyrussell

rustyrussell Dec 10, 2018

Collaborator

It may not be able to figure this out yet (fees). So, perhaps this should be 'sum of input amounts < sum of output amounts'? And add the change_satoshis test only once it has calculated the tx?

This comment has been minimized.

@rustyrussell

rustyrussell Dec 10, 2018

Collaborator

Hmm, actually, I think this being negative should be allowed: in that case, funding_satoshis should be reduced. In effect, this allows you to say "I don't want change, take the fee out of my initial balance". Or perhaps we should make the change output optional for this case?

This comment has been minimized.

@niftynei

niftynei Jan 30, 2019

Author Contributor

This seems like a valid use case. Will remove the 'change_satoshis must be positive' requirement.

Allowing negative change amounts means the resulting channel balance will be less than the indicated funding_satoshis, which ostensibly came from the user. Implementations would probably want to add a way for users to indicate where to take the fees out -- either the provided channel fund amount or in addition to that. (You could do this with the existing flow too, since it's just UX.)

My hesitation with making the change_address optional is that 1) it's an edge case (depends on available UTXO set) 2) it has a small chance of introducing bugs (i.e. broken implementation forgets to include a change address so you end up with a huge channel balance by accident). Leaving out the change address doesn't save you much in calculation overhead when picking out the input UTXO set either. The only thing it does save on is wire bytes, in the case where your funding amt works out exactly right.

to create the funding transaction, using `max_extra_witness_len`
for each `input_info` and `feerate_per_kw_funding` as specified in
`open_channel2`.
- MUST send `commitment_signed2`.

This comment has been minimized.

@rustyrussell

rustyrussell Dec 10, 2018

Collaborator

Can we just use commitment_signed. It's identical, and performs the identical function, so I really think it deserves the same name...

This comment has been minimized.

@niftynei

niftynei Jan 26, 2019

Author Contributor

Right. The rationale was that it might be nice to decouple the two flows as that would allow future modification without needing to worry much about backwards compatibility.


`satoshis` is the value of the input or output. The opener must provide
one change address for the transaction, which is flagged with a
`satoshis` value of zero.

This comment has been minimized.

@rustyrussell

rustyrussell Dec 10, 2018

Collaborator

You said this below as well; delete one? We try to avoid spec-sounding language (i.e. "must") in Rationales to avoid confusion; I would say "The opener provides one output with satoshis equal zero, as a change address once the amount of change can be calculated".

#### Requirements
The sending node:
- MUST set `witness` to the marshalled witness data for each of its
inputs, in funding transaction order. FIXME: add reference

This comment has been minimized.

@rustyrussell

rustyrussell Dec 10, 2018

Collaborator

I realize this is quoting me, but the term I should have used is 'serialized' which IIRC is what the BIPs use.

Exchanging witness data allows both sides to broadcast the funding
transaction and maintains information symmetry.

### The `funding_locked2` Message

This comment has been minimized.

@rustyrussell

rustyrussell Dec 10, 2018

Collaborator

Again, prefer direct 'funding_locked' reuse.

This comment has been minimized.

@niftynei

niftynei Feb 13, 2019

Author Contributor

inclusion of the funding_tx_id means we need a v2 for this

* [`4`:`feerate_per_kw`]
* [`4`:`feerate_per_kw_funding`]
* [`2`:`num_inputs`]
* [`num_inputs`\*`input_info`]

This comment has been minimized.

@rustyrussell

rustyrussell Dec 10, 2018

Collaborator

num_additional_inputs might be clearer?

e.g. an `output_info` entry with `satoshis` set to zero.
- the `feerate_per_kw_funding` is not greater than the previously
negotiated rate.
- `change_satoshis` is negative.

This comment has been minimized.

@rustyrussell

rustyrussell Dec 10, 2018

Collaborator

I think we should allow "extract from channel" in this case, as well.

- MAY set the `num_inputs` to zero.
- MUST transmit all outputs.
- MUST include at least one output with the `satoshis` set to zero,
to be used as the change output.

This comment has been minimized.

@rustyrussell

rustyrussell Dec 10, 2018

Collaborator

Maybe review this depending on decision about non-change channels above...

niftynei added some commits Dec 3, 2018

peer: add v2 for open_channel
First draft of flow for v2 of channel establishment.
transactions: add calculations for dual funded funding_tx
Add calculations for fees and change outputs for funding transactions
that are created via the v2 channel establishment (ie dual funded)
protocol.

@niftynei niftynei force-pushed the niftynei:dual-funding branch from 00c6e6f to 8bacfe4 Feb 6, 2019

@niftynei niftynei changed the title WIP: dual funding first draft WIP: Dual Funding (v2 Channel Establishment protocol) Feb 6, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment