Skip to content

Two simple Channel cleanups #314

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

Merged
merged 2 commits into from
Mar 22, 2019

Conversation

TheBlueMatt
Copy link
Collaborator

No description provided.

This should probably have happened when we moved most state into
the state enums themselves, but specifically forcing awareness of
the removed/not removed state would have prevented me from
introducing a bug in the first version of an upcoming reserve-value
patch.
Copy link

@ariard ariard left a comment

Choose a reason for hiding this comment

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

utACK

/// Remote removed this and sent a commitment_signed (implying we've revoke_and_ack'ed it), but
/// the remote side hasn't yet revoked their previous state, which we need them to do before we
/// can do any backwards failing. Implies AwaitingRemoteRevoke.
/// We have removed this HTLC in our latest commitment_signed and are now just waiting on a
/// revoke_and_ack to drop completely.
AwaitingRemovedRemoteRevoke,
AwaitingRemovedRemoteRevoke(Option<HTLCFailReason>),
Copy link

Choose a reason for hiding this comment

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

Given that a state flag misuse has been and could be a likely source of errors, maybe we should have a more verbose name to dissociate AwaitingRemoteRevokeToRemote and AwaitingRemovedRemoteRevoke, like AwaitingRemoteRevokeToFinalDrop ? Had to read again their definition to get the slightly difference between them.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure! That's somewhat orthogonal, though, so I'll leave that for a later cleanup, see #323.

@TheBlueMatt TheBlueMatt merged commit bb094f1 into lightningdevkit:master Mar 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants