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 transaction format #1484

Merged
merged 7 commits into from
Jul 21, 2020
Merged

Anchor output transaction format #1484

merged 7 commits into from
Jul 21, 2020

Conversation

t-bast
Copy link
Member

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

This PR contains a first step towards the anchor outputs commitment format.
It implements all the transaction format changes and passes official test vectors.

It's currently only raw transaction/script code and will at first be unreachable (until support is added in higher layers to negotiate the feature at channel opening time).
We're implementing this feature from the bottom up to be able to integrate small chunks to master safely.

Some clean-up/refactoring was necessary, so I advise to review commit-by-commit.

NB: there is one area where we currently diverge from lnd, in the case where only one of the anchor outputs is added (pruned to_remote). We are still clarifying what the behavior should be on the spec PR, I'll update here once there's consensus between implementations on the expected behavior).

EDIT: we confirmed during the spec meeting that both anchors should always be deduced for simplicity's sake. This is done in fefd076, which makes this PR fully spec compliant.

This lets us re-use most of our existing transaction utilities for
anchor outputs.

Channels will always use the default commitment format (current spec),
but we will be able to change that by setting the `ChannelVersion` to
something appropriate for anchor outputs (and later other commitment formats).
Remove obsolete claim-htlc tests: they used a different format from what
lightning uses and are redundant with existing tests in TransactionsSpec.

Add missing test cases in TransactionsSpec.
Add support for creating an anchor outputs commitment transaction,
without any HTLC.

Support for the new HTLC format will be added in a separate change.
For anchor outputs, some values will not match the hard-coded ones.
We will instead need to read from the test vector.
Anchor outputs isn't fully implemented yet, so we need to ignore the tests
that are currently not passing.
Add support for HTLC transactions with a 1-block relative delay.
Add missing anchor outputs spec tests.
Add missing tests for some revoked paths.
@t-bast t-bast changed the title Anchor output commit tx Anchor output transaction format Jul 15, 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.

In receiveCommit() and sendCommit() shouldn't we change the SIGHASH flag used when signing HTLC txs if anchor_output is active ?

@t-bast
Copy link
Member Author

t-bast commented Jul 20, 2020

In receiveCommit() and sendCommit() shouldn't we change the SIGHASH flag used when signing HTLC txs if anchor_output is active ?

Yes we will :)

But this is done in a later PR (currently being worked on) that focuses on the changes to Channel.scala, Helpers.scala and Commitments.scala to avoid a huge feature PR.

The current PR adds all the script/transaction format capabilities, but doesn't exploit them yet.

For simplicity's sake, we always subtract both anchors from the funder's
main output, even if only one anchor materializes.
@t-bast t-bast merged commit 92a094c into master Jul 21, 2020
@t-bast t-bast deleted the anchor-output-commit-tx branch July 21, 2020 09:36
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.

2 participants