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

Funding Timeout Recovery proposal #854

Closed
wants to merge 2 commits into from
Closed

Funding Timeout Recovery proposal #854

wants to merge 2 commits into from

Conversation

cdecker
Copy link
Collaborator

@cdecker cdecker commented Mar 15, 2021

Since we recently strengthened the "you're allowed to forget" rule I though it might be nice to revisit the state that this leaves the funder in. When a fundee forgets about a channel, the funder is forced to either double-spend the funding transaction (if possible), or use the negotiated commitment transaction, which usually has much higher fees attached than necessary, and results in the funds being unavailable until the to_us timeout expires.

This proposal describes a minimal set of information a fundee might want to keep around, to assist the funder to recover its funds. It basically boils down to writing a blank check to the funder and the funder can then implement arbitrary recovery logic. The blank check is limited to the channel in question via the funding_privkey which is channel specific.

In addition. it adds a small facility, based on the same ground-work, to recover funds from malleated funding transactions, which we've had one recently due to the user importing the private key into an external wallet and manipulating the funding transaction there before broadcasting. It might be a rare case, but I think it's a nice escape hatch to use if anything goes bad. In that case it was re-negotiating a close on a funding transaction alias.

Update: #866 was filed as a competing variant based on exchanging the keys. Please continue discussing trade-offs here, so we don't spread the discussion over two PRs.

Closes #866

@cdecker cdecker marked this pull request as draft March 15, 2021 15:15
Copy link

@ariard ariard left a comment

Choose a reason for hiding this comment

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

I agree with the problem description, a funding transaction might not confirm in the imparted delay, especially if the funder opening strategy is to start with a "slow confirmation" feerate in an optimistic scenario to save on fees or its fee-bumping logic enforces some upper bound.

If the funding expiry timeout has been reach out, and your transaction isn't confirmed, I think a funder has 3 options to consider :
a) wait for the funding_tx to confirm with its current feerate, thought you're losing the timevalue of funding inputs while they're stuck in the mempool
b) CPFP bump the funding_tx through a change output, timevalue cost is higher than what you're going to bleed in fees
c) double-spend the funding inputs with a RBF, though bip125 replacement penalty might cost you more than confirming the funding tx

I think a node should always consider evaluate c) as it's likely the cheapest option compare to a) OR b) + commitment+CPFP OR "blank_check_tx". It also spares you the commitment+CPFP path, no csv delay, or the blank_check_tx path, counterparty cooperation.

That said, after observing this caveat, I'm all in for a "blank_check_tx" proposal. More mechanisms, easier for a node to build a suiting fee/timevalue strategy :)

proposals/001-funding-timeout-recovery.md Show resolved Hide resolved
This information is required in order to construct a valid closing
transaction without any outputs. The outputs will later be modifiable
by the funder, and committed to by their own signature. The fundee
signs the closing transaction with `sighash_none` to allow these
Copy link

Choose a reason for hiding this comment

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

sighash_none is clearly a privacy-harming watermark but compare to the alternative scenario of broadcasting a commitment with its own 0x20/0x80 watermark it sounds fine to me.

when attempting to re-establish a connection, allowing them to build a
closing transaction, with arbitrary outputs, e.g., not encumbering the
funder's return output with a timeout.

Copy link

Choose a reason for hiding this comment

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

I think this proposal should underscore that's a "gentlemen-agreement"-style model, like data_loss_protection. Game-theory wise, your counterparty could ask a small fee back, based on your fee/timevalue savings from the alternative path.

malleated, and the funding outpoint is no longer the same as
advertised during the negotiation. In this case any commitment
transaction negotiated during the opening will also be invalid, and
the funds are stuck in limbo.
Copy link

Choose a reason for hiding this comment

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

I'm a bit dubious about this specific extension. I think that's a hidden transfer of risk from the funder to the fundee. The funder has a risk of funding_tx malleation freezing its coins unless counterparty cooperates. The fundee has a risk of reusing the fundee_publickey beyond its context usage, and thus opening the way to some sighash_noinput-style replay attack from the funder, even worsen due to sighash_none usage.

For a first version of this mechanism we might pass on this extension ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yea, what's the exact purpose of this - is it just to allow RBF? In that case isn't it simpler to open a fresh channel?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Its main purpose is to prevent some users from losing funds because they decided to RBF-bump the funding transaction using external tools. We had one user that took the funding transaction, added it into electrum and used it to bump the funding transaction, without knowing that this would make the channel completely unusable, and unable to recover the funds, since the commitment transaction was now detached from the funding.

We know that other implementations have had similar issues in the past, so we'd like to have a builtin way of fixing this if the counterparty is collaborative. This is a gentleman's agreement to help recover funds in this case, but it is not a guarantee that it works, since a malicious peer may just as well try to extort the funder. It should be clear that the funder is at fault here, since he malleated the funding tx, but having a mechanism of helping them recover is nice :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be described in the rational for the Alias


- [ ] The specification appears to not require `funding_pubkey` to be
unique to the channel. Is anyone reusing their it? Can we
require it to be unique?
Copy link

Choose a reason for hiding this comment

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

Our signer API already require per-channel secrets to be "unique". I think we should just precise further unique semantic as "unique-even-beyond lightning-usage"

@devrandom what do you think ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We also use unique per-channel secrets in eclair, it makes sense to require it to be unique.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looks like KEYOLO is also an option then, I added #866 as a counter-proposal using the funding_privkey exchange.

Choose a reason for hiding this comment

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

Sounds good. I'm adding an explicit doc in lightning-signer that the channel nonce should not be reused if supplied, and we do generate a unique one if it is not supplied. The generated keys are not in the BIP32 hierarchy, so should not collide with layer-1 keys.

`fundee_privkey`.
4. The signature is sent as part of the `error` message (see Error
Details change below), as TLV type XXX.
5. The funder receives the signature and verifies that it matches the
Copy link

Choose a reason for hiding this comment

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

The funder should run a timeout and fallback to commitment broadcast if your counterparty is definitively non-cooperative or offline ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point, but I guess that's pretty much implied, isn't it?

- [ ] The specification appears to not require `funding_pubkey` to be
unique to the channel. Is anyone reusing their it? Can we
require it to be unique?
- [ ] Do we want to piggyback the signature on the error message or
Copy link

Choose a reason for hiding this comment

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

I would prefer a new message if we add extensions to this protocol. E.g the counterparty could reply with a sighash-single-signed output to price its cooperation. Or a splicing proposal.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We can always extend this using TLVs, to include a collaboration fee (do we want to formalize extortion???) and whatever else we want, having that flexibility and re-using the message we already have are not mutually exclusive imho.

What I like about the error message is that it is atomic, either we get an error along with all the information required to react to it, or we don't. If we split across multiple messages we might get the error, but not the extra information, or we might get the extra information first, not know what to do with it, and only then the error.

Copy link
Collaborator

@t-bast t-bast left a comment

Choose a reason for hiding this comment

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

I have a simpler counter-proposal: what about having the fundee simply give away its funding_privkey? If we require funding_pubkeys to be unique and mark the channel as closed, there shouldn't be any risk involved, right?

This way it defers all the work to the funder to use this private_key however it needs depending on how he messed up his channel. This trivially solves people wrongly sending funds to the multisig 2-of-2 of the channel.

proposals/001-funding-timeout-recovery.md Show resolved Hide resolved

- [ ] The specification appears to not require `funding_pubkey` to be
unique to the channel. Is anyone reusing their it? Can we
require it to be unique?
Copy link
Collaborator

Choose a reason for hiding this comment

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

We also use unique per-channel secrets in eclair, it makes sense to require it to be unique.

@rustyrussell
Copy link
Collaborator

I also proposed the "give the funding_key" to @cdecker but I wasn't sure anyone would be comfortable with it....

@Roasbeef
Copy link
Collaborator

Roasbeef commented Apr 7, 2021

I also proposed the "give the funding_key" to @cdecker but I wasn't sure anyone would be comfortable with it....

This came up in the last IRC meeting with many attendees preferring that route "just send the multi-sig private key" instead of the blank check out. Although this route is much simpler, IMO it's to be avoided (in favor of the original proposal) as:

  • We make no requirements w.r.t the method of derivation used for the private keys. Depending on the implementation, leaking those keys could compromise other keys if non hardened derivation is used.
  • It creates a new foot-gun where a user could possibly be manipulated into sending the keys while the actually is still active, thereby forfeiting all their funds.
  • Just even the thought of "users just sending their private keys of the wire" should make us all shudder...

cdecker added a commit that referenced this pull request Apr 26, 2021
This is the counter-proposal to #854, using the more flexible, but
also potentially more dangerous direct private key exchange, rather
than sending over a `sighash_none` signature for a close transaction.

It was proposed due to the observation that, while not yet required by
todays spec, all implementations decided to use hardened derivation
for the channel `funding_privkey`, making this a safe option.
@cdecker
Copy link
Collaborator Author

cdecker commented Apr 26, 2021

I added the KEYOLO variant as #866 as promised during the last spec
meeting, however I find the more restricted variant exchanging just
the sighash_none signature a bit more palatable, as it clearly has
fewer requirements on the way the fundee funding_privkey is
derived. I added the requirement for hardened derivation multiple
times, but some implementers might eventually forget about it.

To summarize the differences:

  • KEYOLO is less fingerprintable on-chain, given that it doesn't
    require a non-default sighash-flag.
  • KEYOLO also covers the case of a user sending funds to a funding
    output script, outside of a channel funding.
    • Should we consider this part of our spec at all? We can't prevent
      users from doing dumb things, neither should we. And this only
      applies when they happen to send to a channel that timed out
      anyway, i.e., if they send to a channel script that was used the
      funds are unrecoverable even with KEYOLO.
  • It creates a new foot-gun where a user could possibly be
    manipulated into sending the keys while the actually is still
    active, thereby forfeiting all their funds.

I'm afraid this is even the case with the more restrictive #854
proposal: if you send out a sighash_none signature as per this
proposal, and then fail to mark the channel as inactive, then whatever
you do going forward could race with a close transaction that the
funder can now create using that signature. All the invalidation logic
just went out the window. Hence my insistence on marking the channel
as unusable before exchanging signatures. Ideally we'd mark the
channel as forgotten/abandoned on a timeout, and generate the
signature only as a reaction to getting a re-establish for a channel
that is already marked abandoned in the committed state to disk. This
avoids having to deal with some edge-cases like marking in a DB
transaction while queuing the message, and then rolling back or
abandoning the DB transaction, but not clearing the message queue.

Copy link
Contributor

@lightning-developer lightning-developer left a comment

Choose a reason for hiding this comment

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

I like the proposal as I think the use cases mentioned are valid from a users perspective. In particular the Alias of the Funding Output if a user happened to interfere with the funding process.

I have added a few questions to make sure I understood everything correctly while reviewing.

It seems there was no major disagreement from other reviewers. Should we start integrating this to the relevant places in the spec?

sign any future commitment transactions. Thus the fundee may provide a
blanked signature for a commitment transaction using `sighash_none`,
signing off whatever the funder intends to do with the funding
outpoint. This frees the parties from having to negotiate a closing
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel a little bit dumb asking this but would this idea not also work for a regular mutual close? The fees are paid by the funder and this would remove the necessity to have the negotiation of closing fees via closing_signed

Copy link
Contributor

Choose a reason for hiding this comment

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

sighash none would allow the other party to only include an output for himself, it think you mean a sighash single, but that would be clearly visible on chain that a lightning channel was closed, this doesn't matter with announced ones (just search the txid on an LN explorer) but it does with private channels

2016, however it may chose to retain a minimal stub of the channel to
help the funder recover the funds gracefully in case the funding
transaction eventually confirms. In order to do so it must retain the
following information:
Copy link
Contributor

Choose a reason for hiding this comment

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

Of course this is less information than a regular channel but it still wont forget the channel opening attempt completely. So How is that an improvement?

malleated, and the funding outpoint is no longer the same as
advertised during the negotiation. In this case any commitment
transaction negotiated during the opening will also be invalid, and
the funds are stuck in limbo.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be described in the rational for the Alias

unique to the channel. Is anyone reusing their it? Can we
require it to be unique?
- [ ] Do we want to piggyback the signature on the error message or
shall we add a new message that preceeds the error message?
Copy link
Contributor

Choose a reason for hiding this comment

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

The only use case I am seeing is to request a Funding Transaction Alias but that seems a bit complicated from a communication flow perspective.

@cdecker cdecker closed this Dec 21, 2023
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.

9 participants