-
Notifications
You must be signed in to change notification settings - Fork 33
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
chantools scbforceclose: extract close tx from SCB and sign it #95
base: master
Are you sure you want to change the base?
Conversation
4530889
to
31b20eb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the two PRs!
I think both need some extra work but are on the right track.
74d418b
to
3e8ea11
Compare
I updated both PRs. Signing now happens in @guggero Please take another look! |
3e8ea11
to
dcc9b3b
Compare
b78f22b
to
4b79ab4
Compare
Rebased for LND 0.18, fixed lint warnings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great PR, looks very good.
Will test once lnd side has been updated.
cmd/chantools/scbforceclose.go
Outdated
|
||
if s.CloseTxInputs == nil { | ||
return nil, errors.New("channel backup does not have data " + | ||
"needed to sign force sloe tx") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: s/sloe/close/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, moved to LND PR (commit chanbackup: add function SignCloseTx
).
cmd/chantools/scbforceclose.go
Outdated
} | ||
|
||
func createTaprootNonceProducer( | ||
s chanbackup.Single, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: formatting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, moved to LND PR (commit chanbackup: add function SignCloseTx
).
4b79ab4
to
521d4c8
Compare
Thanks for the review! I updated LND PR and this PR. The code producing signed transaction was moved to LND to use it in itest. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love how minimal this now turned out 💯
LGTM pending some final manual tests (will attempt to do later today).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tACK, LGTM 🎉
To make things easier to debug, can you please apply the following diff as another commit? That will add the new data to the output of chantools dumpbackup
.
Will merge once the lnd
PR is in.
cmd/chantools/scbforceclose.go
Outdated
return errors.New("failed to read user input") | ||
} | ||
if strings.TrimSpace(userInput) != "YES" { | ||
return errors.New("cancelled by user") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: improve error message slightly, perhaps something like "canceled by user, must type uppercase 'YES'"? Same below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
cmd/chantools/scbforceclose.go
Outdated
s.FundingOutpoint, err) | ||
} | ||
txHex := hex.EncodeToString(buf.Bytes()) | ||
fmt.Println(s.FundingOutpoint) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should prefix these values, otherwise it looks a bit misleading on the command line.
Now:
Type YES to proceed: YES
8821015bb99036728b00f4b1f68ab4e41d8573947f3f94fe0d4ec2f38c1f92d4:0
02000000000101d4921f8cf3c24e0dfe943f7f9473851de4b48af6b1f4008b723690b95b01218800000000000508b280044a010000000000002......
Suggestion:
Type YES to proceed: YES
Channel point: 8821015bb99036728b00f4b1f68ab4e41d8573947f3f94fe0d4ec2f38c1f92d4:0
Raw transaction hex: 02000000000101d4921f8cf3c24e0dfe943f7f9473851de4b48af6b1f4008b723690b95b01218800000000000508b280044a01000000000.....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented!
521d4c8
to
1049f12
Compare
I rebased the PR, updated go replace to the latest lightningnetwork/lnd#8183 version. Also included the suggested patch for dump.go. @guggero I put your name in Author field of the commit. |
9474fe1
to
2f7fc42
Compare
I moved function |
2f7fc42
to
25f7bb7
Compare
Remove MockSigner from Signer. MockSigner has MusigSessionManager is an embedded field, and it is the only part of MockSigner needed in Signer.
It can now be used as keychain.ECDHRing.
It will be reused in scbforceclose.
25f7bb7
to
42b2824
Compare
testvector for custom channels
RootKey Regtest:
Dump:
scbclose:
|
@ziggie1984 Thanks for providing the test vector for custom channel! |
|
It provides function SignCloseTx which produces a signed force close transaction from a channel backup and private key material.
Include fields closetx, commit sig, commit height, tapscript root. That will add the new data to the output of chantools dumpbackup.
42b2824
to
a386bde
Compare
@ziggie1984 Thank you! It works! I updated the testdata for |
I'm going to remove my request for review until |
@starius, remember to re-request review from reviewers when ready |
This is part of lightningnetwork/lnd#7658 (comment) implementation.
This PR depends on lightningnetwork/lnd#8183
(Field chanbackup.Single.CloseTxInputs is needed.)
I added
chantools scbforceclose
command, which extracts closing tx from SCB and signs it and optionally broadcasts.New command:
chantools scbforceclose --help
The command extracts closing transactions from SCB, signs and prints them.
Example
Here is an example of using
chantools scbforceclose
in testnet to sign a force closing transaction and to publish it.chantools --testnet scbforceclose --single_backup xxx --publish --apiurl https://blockstream.info/testnet/api
(To use
--publish
option in testnet, --apiurl should be adjusted. HopefullynewExplorerAPI
from #107 is fixing this.)TODOs