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

Remove timedOutOutgoingHtlcs from AbstractCommitments #1604

Merged
merged 1 commit into from
Nov 16, 2020
Merged

Remove timedOutOutgoingHtlcs from AbstractCommitments #1604

merged 1 commit into from
Nov 16, 2020

Conversation

akumaigorodski
Copy link
Contributor

@akumaigorodski akumaigorodski commented Nov 16, 2020

It turns out that:

  • def timedOutOutgoingHtlcs(blockHeight: Long): Set[wire.UpdateAddHtlc] defined in AbstractCommitments is currently only used in Commitments object and nowhere else.
  • Logic in HC commitments object can be simplified if timedOutOutgoingHtlcs takes additional arguments besides blockHeight.

So it makes sense to just remove timedOutOutgoingHtlcs from AbstractCommitments and let HC commitments object to do things differently internally without implementing of (useless) timedOutOutgoingHtlcs.

@t-bast
Copy link
Member

t-bast commented Nov 16, 2020

Please spend more time testing those thoroughly.
Changing these abstractions creates issues for our merges to feature branches, so we need some stability on these abstractions.

@akumaigorodski
Copy link
Contributor Author

It's safe to say this is it, the work on HC is done and I don't see a need for any more PRs (besides maybe https://gitter.im/ACINQ/developers?at=5fae6c8bc6fe0131d4f1ea88).

@t-bast t-bast merged commit 407b330 into ACINQ:master Nov 16, 2020
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