Fundchannel: extend fundchannel_cancel to work before funding broadcast#3006
Merged
niftynei merged 9 commits intoSep 11, 2019
Merged
Conversation
0ac2046 to
6cd305a
Compare
ZmnSCPxj
approved these changes
Aug 29, 2019
Contributor
ZmnSCPxj
left a comment
There was a problem hiding this comment.
Looks OK to me, please just add documentation.
Contributor
Author
|
@ZmnSCPxj Thank you! Now I complete the document. |
ZmnSCPxj
suggested changes
Aug 29, 2019
c20d47d to
5c4c68b
Compare
niftynei
reviewed
Sep 4, 2019
f4eb879 to
6d4a303
Compare
Contributor
Author
|
Add the check that the tx broadcast by bitcoind can be catched. |
ea84584 to
81570e8
Compare
For now, we can't fully ensure that the broadcast was catched from a third pary. Only when the transaction (broadcast by a third pary) is onchain, we can catch it.
81570e8 to
67c15a5
Compare
Contributor
Author
|
Rebased and resolved the conflict. |
Collaborator
|
ack 67c15a5 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix #2740
This PR try to make
fundchannel_cancelcan work afterfundchannel_completeand before funding broadcast.Process
The extended process is:
For funder:
funderchannel_complete;funderchannel_cancel:fundchannelprocess completed ==>channeldsend a error message to fundee ==>delete_channel())For fundee:
funderchannel_complete, the channel state isAWAITING_LOCKIN. And the fundee begin to wait for funding locked both locally and remotelyAWAITING_LOCKIN, when fundee receives the error message from funder, fundee will delete(forget) the channel(also delete the peer, because of the limit ofnotify_disconnectwhen remote peer sendserror? #2741 )Limits
But here's also some limits:
This also ask caller to use
fundchannel_cancelcorrectly: must be before funding broadcast.fundchannel_cancelis called afterfundchannel_completeand we cancel the channel successfully. Then we must reconnect with the remote peer beforefundchannel_startagain.Like
notify_disconnectwhen remote peer sendserror? #2741 said, when the peer receive error message or when the peer deletes the channels, the peer will disconnect with the remote peer.The main reason for this is
channel_set_owner(channel, NULL, false)moves responsibility for the connection to connectd not openingd. If we want to keep connection, rolling back fromchanneldtoopeningdis necessary.Open Question
In f77d6c7 (Fix: Store the transaction(broadcast by
txsend) into DB), I add the labelTX_UNKNOWNfor the transaction broadcast by
txsend.I think maybe we can add optional string field for
txsendcommand to specific the label. What do you think about it?Unfinished
Update document
Add CHANGELOG
Note: 9ebb521 is duplicated with a commit of #2964 , just for making travis-ci happyThanks for @ZmnSCPxj and @niftynei's explanation, direction and help!