lisa neigut <nifty...@gmail.com> writes:
> Hello fellow Lightning devs!
>
> What follows is a draft for the new dual funding flow. Note that the
> `option_will_fund_for_food` specification has been omitted for this draft.

Hi!

Wow, my mailer really mangled this!  I've liberally demangled below
as I quote.

The proposal is great, but intense, so I've bikeshedded the language.
My only objection is that I'd love to simplify RBF.

> ===== Proposal
>
> Create a new channel open protocol set (v2), with three new message types:
> `funding_puts2`, `commitment_signed2`, and `funding_signed2`, plus two for
> negotiating RBF, `init_rbf` and `accept_rbf`.
>
>
> Quick overview of the message exchange for v2:
>
>    +----+                              +----+
>    |    |--(1)---  open_channel2  ---->|    |
>    |    |<-(2)--  accept_channel2  ----|    |
>    |    |                              |    |
>    |    |--(3)--  funding_puts2  ----->|    |
>    |    |<-(4)--  funding_puts2  ----- |    |
>    |    |                              |    |
>    |    |--(5)-- commitment_signed2 -->|    |
>    |    |<-(6)-- commitment_signed2 ---|    |
>    | A  |                              |  B |
>    |    |--(7)--- funding_signed2 ---->|    |
>    |    |<-(8)--- funding_signed2 -----|    |
>    |    |                              |    |
>    |    |--(a)--- init_rbf ----------->|    |
>    |    |<-(b)--- accept_rbf ----------|    |
>    |    |                              |    |
>    |    |--(9)--- funding_locked2 ---->|    |
>    |    |<-(10)---funding_locked2 -----|    |
>    +----+                              +----+
>   where node A is the ‘initiator’ and node B is the ‘dual-funder’

We currently use the terms funder and fundee, which are now
inaccurate ofc.  Perhaps 'opener' and 'accepter' are not great english,
but they map to the messages well?

> Willingness to use v2 is flagged in init via `option_dual_fund`.
> `init`
>
> local channel feature flag, `option_dual_fund`
>
> == Channel establishment with dual_funding
>
> ____`open_channel2`:
> [32:chain_hash]
> … // unchanged
> [1:channel_flags]
> [?: options_tlv]

Always prefix variable fields by length, even this one.  Otherwise
we can never extend, and you never know...

   [2:tlv_len]
   [tlv_len:opening_tlv]

I think we can remove `funding_satoshis` here; we'll know when they add
their inputs, so it's redundant.

Another subtle point is the feerate_per_kw field; in the old scheme it
applied to the first commitment tx, but here it applies to both the
first commitment tx and the funding tx itself (unless
option_simplified_commitment, though roasbeef has suggested further
splitting that option, in which case we'll want another fee field here).

> options_tlv:

Let's call this `opening_tlv` since there are other TLVs coming?

>    1.
>    Type: 1 `option_upfront_shutdown_script`
>    1.
>
>       [2:len]
>       2.
>
>       Value: `shutdown_scriptpubkey`
> If nodes have negotiated `option_dual_fund`
> The sending node:
>    -
>    MAY begin channel establishment using `open_channel2`

 - MUST NOT send `open_channel`.

> Otherwise the receiving node:
>    -
>    MUST return an error.

This is a requirement for receiving `open_channel`  IIUC?

ie.

The receiving node MUST fail the channel if:
   ...
   - `option_dual_fund` has been negotiated.


> ____ `accept_channel2`:
>
> [32:temporary_channel_id]
> …  // unchanged
> [33:first_per_commitment_point]
> [?: options_tlv]
>

If we call this `opening_tlv` we can just reuse the definition from
before.

> ____`funding_puts2`

We can probably drop the 2 here and call it, um.. `funding_compose`?
(Thanks thesaurus.com).  I get where you're going with 'puts, but it
took me a while :)

> This message exchanges the input and output information necessary to
> compose the funding transaction.
>
> [32:temporary_channel_id]
> [`2`:`num_inputs`]
> [`num_inputs*input_info`]
> [`2`:`num_outputs`]
> [`num_outputs`*ouput_info`]

typo: output_info

> 1. subtype: `input_info`
> 2. data:
>  * [`8`:`satoshis`]
>  * [`32`:`prevtxid`]
>  * [`4`:`prevtxoutnum`]
>  * [`2`:`scriptlen`]
>  * [`scriptlen`:`script`]
>  * [`2`:`max_extra_witness_len`]
>  * [`2`:`wscriptlen`]
>  * [`wscriptlen`:`wscript`]
>
> 1. subtype: `output_info`
> 2. data:
>  * [`8`:`satoshis`]
>  * [`2`:`scriptlen`]
>  * [`scriptlen`:`script`]
>
> Requirements:
>
> The sending node:
>
>    - MUST ensure each `input_info` refers to an existing UTXO
>    - MUST ensure the `output_info`.`script` is a standard script
>    - MUST NOT spend any UTXOs specified in funding_puts2 until/unless the
>      channel establishment has failed


> If is the initiator (A):
> - MUST NOT send an empty message  (`num_inputs` + `num_outputs` = 0)
>
>      If is the dual-funder (B):
>    -
>    consider the `put_limit` the total number of `num_inputs` plus
>    `num_outputs` from `funding_puts2`, with minimum 2.
>    -
>    MUST NOT send a number of `input_data` and/or `output_data` which
>    exceeds the `put_limit`

Side note: I wonder if we should relax this limit when we talk about
`option_will_fund_for_food`?

>    -
>    MAY send an empty message

Be explicit? MAY offer zero `num_inputs` and `num_outputs`.  That's not
quite an empty message...

> The receiving node:
>
>   If is the initiator (A):
>
>    -
>    MUST fail the channel if the `num_inputs` plus `num_outputs` is greater
>    than the `put_limit`

How about MAY?  It's a protection thing, but less to change when we
option_will_fund_for_food.  Unless we set the `put_limit` to min (4) or
something in that case?

Oh, it needs to check max_extra_witness_len is reasonable too, since
that will affect the fees.  Each signature adds 74, and pubkey adds 34,
so I think MUST BE less than 500 is perfectly reasonable (for both
reader and writer).

> If has not yet sent a `fund_puts2` for this channel establishment
>     - SHOULD send their `fund_puts2` message
>
> Otherwise
>    - SHOULD send `commitment_signed2`

Perhaps add '- MUST use the sent and received `input_info` and
`output_info` to create the funding transaction, using
`max_extra_witness_len` for each `input_info` and `feerate_per_kw` from
`open_channel2`.'

Side note: we need to define max_extra_witness_len as the total
marshalled length of the extra witness data which will be supplied
(ie. sizeof(varint) + sizeof(data) for each one).

> Rationale:
>
> Each node must have a complete set of the transaction inputs/outputs, to
> derive the funding transaction hash.
>
> There is a dual_funding protocol that only requires one side to send their
> witness data and inputs. This is more efficient, however it results in
> asymmetry between the nodes, where one is revealing their UTXOs before the
> funding transaction is committed.. We mitigate this asymmetry by asking the
> initiator (A) to send their set of inputs before the dual-funder (B) does.
>
> NB: This is reusing the input/output structures from the Splicing proposal,
> but with a more generalized name.
>
>
> ____`commitment_signed2`

I just realized that `commitment_signed` is the name of the message we
use during HTLC / fee updates.  But since the message is identical to
this one in both form and purpose, I think we can reuse it here.

> This message exchanges the counterparty’s signature for the first
> commitment transaction, so it can broadcast the funding transaction knowing
> that funds can be redeemed.
>
> [32: `channel_id`]
> [`64`: commitment_signature`]
>
> Requirements:
>
> The sending node:
>
>    - MUST derive the `channel_id` from the funding transaction’s id
>    - MUST set signature to the valid signature, using its funding_pubkey for
>    the initial commitment transaction, as defined in BOLT #3

This is good.

>
>   If it has not received a `funding_puts2`
>
>    -
>
>    MUST NOT send their `commitment_signature`

This is implied by the requirement that they generate the funding
transaction.  For better or worse, we don't usually spell out
requirements not to send things out of order.

> The receiving node:
>
>    - MUST verify the commitment signature is valid for the funding
>    transaction -> commitment transaction that it has derived independently
>
>      If this signature is invalid it
>
>    -   MUST fail the channel
>
>
>   If it has not sent a `commitment_signed2` message
>
>    -  MUST send their `commitment_signed2` message
>
>    If this is in a flow initiated from `init_rbf`:
>    -

Perhaps be explicit here: 'If this commitment_signed2 was in response to
an `init_rbf` message:'?

>    MUST save the temporary_channel_id until the channel funding transaction
>    has been locked (this is the channel id of the currently broadcast
>    transaction)

Confused by this, see commentry down later.

> Rationale:
>
> In the previous channel establishment protocol, we were able to compress
> the commitment signature exchange into `funding_created`/`funding_signed`.
> With dual funding, we need interaction to build the funding transaction --
> commitment sig exchange is now a separate step.

> ___`funding_signed2`
>
> This message exchanges the witness data for the inputs that were originally
> sent in the `funding_puts2` message.
>
> [`32`:`channel_id`]
> [`2`:`num_witnesses`]
> [`num_witnesses*witness_stack`]
>
> Requirements:
>
> The sending node:
>     - MUST remember the details of this funding transaction

     - MUST send one `witness_stack` for each `output_info`
       it sent in `funding_puts2`.

>    - MUST NOT send a `witness_stack` whose length exceeds the corresponding
> `max_extra_witness_len`
>
>     If they have NOT received a valid `commitment_signed2` message
>    - MUST not send their `funding_signed2` message

Redundant, but so vital I agree it needs to be stated.

> The receiving node:
>
>    - SHOULD check that the number of witnesses sent matches the number of
>    inputs

"SHOULD check" is a spec anti-pattern :)

    if `num_witnesses` does not equal `num_inputs` received in
    `funding_puts2`:
        - MUST fail the channel.

> If a `witness_stack` length exceeds the corresponding
> `max_extra_witness_len`:
>
>    -
>
>      MAY error.

MUST?

>    If is the `initiator` (A):
>
>    -
>
>    SHOULD apply `witness` to the funding transaction and broadcast the
>    result.

Since this is symmetrical, you can drop "if it is the `initiator`"?

> Rationale:
>
> Exchanging witness data allows either side to broadcast the funding
> transaction. It also maintains the information symmetry between the nodes.
>
> ___`funding_locked2`
>
> // same as v1
>
> Requirements:
>
> A dual-funding node (B):
>
>    -
>
>    SHOULD broadcast their funding transaction if it does not see the
>    transaction broadcast after a reasonable timeout.

Let's just reuse `funding_locked` maybe?

Not sure why this should wait for broadcast?

> == RBF for Channel Establishment v2
>
> _____`init_rbf`
>
> This message is sent by the initiator, after the funding transaction has
> been broadcast but before the `funding_locked2` has been exchanged.
>
> [32: `channel_id`]
> [8: funding_satoshis]
> [8:dust_limit_satoshis]
> [8:channel_reserve_satoshis]
> [4: feerate_per_kw]
> [`2`:`num_inputs`]
> [`num_inputs*input_info`]
> [`2`:`num_outputs`]
> [`num_outputs`*ouput_info`]

Typo again :)

> Requirements
>
> The sending node:
>    - MUST be the initiator (A)
>    - MAY update the amount, fee rate, dust limit, or channel reserve for the
>    channel

 - MAY send init_rbf if it considers the most recent funding tx unlikely
   to be confirmed in reasonable time.
 - MUST set `feerate_per_kw` larger than the most recent funding tx.

Do we really want to negotiate everything again?  It seems like the
funder should be able to maybe add *new* inputs and outputs (though TBH
I think that's going to be unusual enough that we could omit it), but
doing a wholesale replacement means we have to be careful that the all
RBFs end up having at least one input in common.  Yech.

> The receiving node:
>
>    - MAY reject (error) the RBF request if:
>    -
>
>       the fee rate, dust, or channel reserve is unreasonable
>       -
>
>    MUST reject (error) the RBF request if:
>    -
>
>       the `fee_rate` is less than the rate that was originally proposed
>       -
>
>       the `funding_satoshis` amount is less than the previous negotiated
>       `push_mast`
>       -
>
>       It considers the `feerate_per_kw` too small for timely processing or
>       unreasonable
>       -
>
>       the `dust_limit_satoshis` is greater than the
>       `channel_reserve_satoshis`
>       -
>
>       the initiator’s amount for the initial commitment transaction is not
>       sufficient for full fee payment
>       -
>
>       the `inputs`.`satoshis` does not sum to the `funding_satoshis`
>       -
>
>       the `funding_satoshis` is insufficient to create the transaction at
>       the new `fee_rate`
>       -
>
>       there is no overlap in the proposed inputs and the original input set
>       included in the currently published funding transaction

More subtly, there must be a common subset of inputs between every two
funding txs.

>       -
>
>       they have already received or sent a `funding_locked2` message
>       -
>
>    If there are no errors or unreasonable demands:
>    -
>
>       SHOULD send an `accept_rbf`
>
>
> Rationale:
>
> Once an `init_rbf` has been accepted by the dual-funding node, the message
> flow returns to `commitment_signed2` and proceeds as above, with the
> exception that the `temporary_channel_id` remains as the `channel_id` for
> the currently published but unmined transaction.

By this stage, we are no longer using temporary_channel_id though.

But it's an excellent point I had missed.  The channel_id changes on
each renegotiation.  We could either switch channel_id *after*
each accept_rbf, or keep the original channel_id until funding_locked2 (in
which case it should have a "final_channel_id" field, to make sure we're
talking about the same funding tx).

Since we have to handle the "oops, old one got in!" it might be weird to
see `funding_locked2` with an old txid.  Perhaps we stick with the
original channel_id until then, and flip *after* funding_locked2 is sent
and received.

And yeah, no `update_fee`, `announcement_signatures` until that
funding_locked2 exchange is complete, so we don't get those with an
unsettled channel_id.

> The channel id that becomes fixed for this node will be determined by the
> `funding_locked2` message.
>
> ___`accept_rbf`
>
> This message accepts an RBF request, and renegotiates a dual-funder’s
> funds, dust limit, and channel reserve, and sends the dual-funder’s updated
> puts.

I would make this an empty message, simply an ack.  And note that
the channel_id after this is that of the RBFed tx.

The question then becomes what do we do about reconnection.  I suggest:

opener: if we haven't sent funding_signed, consider it cancelled.  If
   we've received funding_signed, it's obviously locked in.  If we sent
   and didn't received, re-xmit.

accepter: must remember rbf if we sent commitment_signed2.  If we
   received funding_signed it's locked in.  If we receive an init_rbf,
   drop the one we remembered.  If we receive funding_signed, continue.

We still need to address the funding_tx construction; BIP69-style seems
like an unnecessary information leak here.  A 128-bit seed in
open_channel2 could be added, with sorting by SHA(seed | <marshal of
input> | <marshal of witness>) and SHA(seed | <marshal of output>)?

Phew!
Rusty.
_______________________________________________
Lightning-dev mailing list
Lightning-dev@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/lightning-dev

Reply via email to