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

fundchannel_cancel channel_id infelicities #3785

Closed
ZmnSCPxj opened this issue Jun 23, 2020 · 0 comments · Fixed by #3787
Closed

fundchannel_cancel channel_id infelicities #3785

ZmnSCPxj opened this issue Jun 23, 2020 · 0 comments · Fixed by #3787

Comments

@ZmnSCPxj
Copy link
Contributor

ZmnSCPxj commented Jun 23, 2020

Let me describe the current infelicity:

Supposedly fundchannel_cancel has an optional channel_id parameter.

If you have a fundchannel_start but have not yet done fundchannel_complete with a peer, then you can just give the peer ID without giving the channel_id at all. In fact channel_id is ignored, you can literally put valid JSON garbage into it and it will not affect the operation of fundchannel_cancel at all.

---[fundchannel_start $peer]-->[fundchannel_cancel $peer "cat /dev/random"]-->[fundchannel_complete $peer]

On the other hand, after you have done fundchannel_complete, you should give a channel_id:

---[fundchannel_start $peer]-->[fundchannel_complete $peer]-->[fundchannel_cancel $peer $channel]

(actually if you have not recently closed a channel with that peer you do not need the channel ID, because fundchannel_cancel will only fail if there is ambiguity, but if you are trying to make a robust plugin that implements multifundchannel you have to give the channel ID, because you cannot be sure of the previous history with that peer, and it is far simpler (==robuster) to just always give the channel ID all the time than check if we are still remembering a recently-closed channel with that peer)

This translates to a kind of inconsistent use of fundchannel_cancel: you have to give a channel ID after fundchannel_complete, but you cannot get a channel ID from fundchannel_start --- a channel ID is derived from inputs to fundchannel_complete, which is itself derived from outputs of fundchannel_start.

--

So, to get a consistent usage of fundchannel_cancel, we can do a variety of things.

I am not certain what is the best, so let me present the two options I can think of for now.

  • Embrace our one-open-channel-with-each-peer nature. fundchannel_cancel removes its channel_id parameter as unnecessary.
    • If fundchannel_cancel is done after fundchannel_complete, it autosearches for the channel that is currently CHANNELD_AWAITING_LOCKIN; because there can only be one active channel at a time, only up to one channel can be in CHANNELD_AWAITING_LOCKIN.
  • Always require channel_id parameter for fundchannel_cancel. fundchannel_start returns a temporary_channel_id that is used when doing a fundchannel_cancel between fundchannel_start and fundchannel_complete, then fundchannel_complete returns the real channel_id that should be used for fundchannel_cancel afterwards.
    • This is identical to the technique used in BOLT spec anyway, which uses a temporary_channel_id between accept_channel (== the end of fundchannel_start) and funding_created (== the beginning of fundchannel_complete).

I lean more to the former; it seems unlikely we will support more than one channel per peer in the foreseeable future anyway, so fundchannel_cancel should just apply to "whatever channel is being funded with the peer"; previous, already-closed channels should not be considered as one of the possible channels to cancel and thus requiring disambiguation.

On the other hand if we eventually move to multiple channels per peer, then the latter option means that all multifundchannel and fundchannel implementations written today will continue to work in the future with multiple-channels-per-peer (since those implementations will be handling the channel ID anyway).

(It is helpful to point out as well that channel_id parameter in fundchannel_cancel is completely undocumented; not even lightning-cli help fundchannel_cancel hints what channel_id is.)

What does everyone think?

ZmnSCPxj added a commit to ZmnSCPxj/lightning that referenced this issue Jun 24, 2020
… a `channel_id` argument.

Fixes: ElementsProject#3785

Changelog-Changed: `fundchannel_cancel` no longer requires its undocumented `channel_id` argument after `fundchannel_complete`.
ZmnSCPxj added a commit to ZmnSCPxj/lightning that referenced this issue Jun 27, 2020
… a `channel_id` argument.

Fixes: ElementsProject#3785

Changelog-Changed: `fundchannel_cancel` no longer requires its undocumented `channel_id` argument after `fundchannel_complete`.
ZmnSCPxj added a commit to ZmnSCPxj/lightning that referenced this issue Jun 30, 2020
… a `channel_id` argument.

Fixes: ElementsProject#3785

Changelog-Changed: `fundchannel_cancel` no longer requires its undocumented `channel_id` argument after `fundchannel_complete`.
ZmnSCPxj added a commit to ZmnSCPxj/lightning that referenced this issue Jul 1, 2020
… a `channel_id` argument.

Fixes: ElementsProject#3785

Changelog-Changed: `fundchannel_cancel` no longer requires its undocumented `channel_id` argument after `fundchannel_complete`.
ZmnSCPxj added a commit that referenced this issue Jul 2, 2020
… a `channel_id` argument.

Fixes: #3785

Changelog-Changed: `fundchannel_cancel` no longer requires its undocumented `channel_id` argument after `fundchannel_complete`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant