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

loopout: compare delta from htlc expiry correctly #89

Merged
merged 3 commits into from
Oct 3, 2019
Merged

loopout: compare delta from htlc expiry correctly #89

merged 3 commits into from
Oct 3, 2019

Conversation

wpaulino
Copy link
Contributor

This addresses an issue where using a sweep confirmation target greater than the default would result in most cases not revealing the preimage due to the default confirmation target yielding a higher fee than the max miner fee backed by the confirmation target provided.

@alexbosworth
Copy link
Member

Should it have had an associated test?

@joostjager
Copy link
Contributor

Change LGTM. But indeed, at least one sanity checking test for this would be useful.

@wpaulino
Copy link
Contributor Author

wpaulino commented Oct 1, 2019

Test added.

loopout_test.go Outdated Show resolved Hide resolved
loopout_test.go Show resolved Hide resolved
Copy link
Contributor

@joostjager joostjager left a comment

Choose a reason for hiding this comment

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

Definitely a better test setup

loopout_test.go Show resolved Hide resolved
loopout_test.go Show resolved Hide resolved
loopout_test.go Outdated Show resolved Hide resolved
This addresses an issue where using a sweep confirmation target greater
than the default would result in most cases not revealing the preimage
due to the default confirmation target yielding a higher fee than the
max miner fee backed by the confirmation target provided.
@alexbosworth alexbosworth merged commit b61807b into lightninglabs:master Oct 3, 2019
@wpaulino wpaulino deleted the loop-out-conf-target branch October 3, 2019 18:33
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.

None yet

3 participants