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

Anchor output: handle unilateral close #1501

Merged
merged 2 commits into from
Sep 22, 2020
Merged

Conversation

t-bast
Copy link
Member

@t-bast t-bast commented Jul 31, 2020

This PR wraps up experimental support for anchor outputs by correctly handling all unilateral close scenarios.
Anchor outputs can now be activated and should work end-to-end (but we lack support for actually using the anchors and sighash changes to bump the fees - coming next).

It has been tested end-to-end against lnd master:

  • local commit, no HTLC
  • local commit, pending HTLCs
  • remote commit, no HTLC
  • remote commit, pending HTLCs
  • revoked commit, no HTLC
  • revoked commit, pending HTLCs

I had to change the static_remotekey behavior, @sstone let me know if that sounds good.
We used a bitcoind wallet address which made sense for static_remotekey without anchors, because our main output in the remote commitment was directly a p2wpkh to that address.
But with anchor outputs, our main output in the remote commitment is a p2wsh with a CSV-1 delay (OP_PUSHDATA(paymentPubkey) :: OP_CHECKSIGVERIFY :: OP_1 :: OP_CHECKSEQUENCEVERIFY :: Nil) so it doesn't make sense to use a key owned by the bitcoind wallet, we want to be able to claim it from our internal wallet in cases of unilateral close.

Change the static_remotekey behavior to use a wallet basepoint only when
using "standard" commitments; with anchor outputs we need to claim this
output (it doesn't directly spend to our wallet) so we must use
key-derivation with our lightning keys.

Correctly claim all outputs in unilateral cases, and add corresponding
test cases.

Anchor output commitments should now work end-to-end (but there's no support
for fee bumping yet).
Copy link
Member

@pm47 pm47 left a comment

Choose a reason for hiding this comment

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

This change looks fine to me, but I'm no expert of anchor outputs. A review by @sstone would definitely be nice.

sstone
sstone previously approved these changes Sep 21, 2020
Copy link
Member

@sstone sstone 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 that it is better like this when anchor outputs are used. Thanks for hardening existing tests and adding new ones !

@t-bast t-bast merged commit 662e0c4 into master Sep 22, 2020
@t-bast t-bast deleted the anchor-output-force-close branch September 22, 2020 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants