Fix bug in check_spend_remote_htlc and let csv delays being user configurable #355
Conversation
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
|
If you need the changes in #348 for testing, just rebase on top of it? Its close to going in anyway, just need one or two more tweaks. |
We were previously using their_to_self_delay to regenerate scripts for spending remote revoked htlc transactions, and that's a bug. Their_to_self_delay is delay enforced by peer upon outputs returning funds back to us. Our_to_self_delay is delay enforced by us upon outputs returning funds back to peer.
|
Okay added a test for testing user configurable csv delay against hard bounds in channel constructors at fe77f3e, ready for review |
|
Looks like it needs rebase to pass travis? |
|
Checked, I was already rebased on master? Dunno pushed it again |
|
Well it may not be a rebase error, just a build failure. |
|
Oh sorry build error due to rebase error of mine! Should be good now at a949d64 |
TheBlueMatt
reviewed
Jul 24, 2019
added 3 commits
July 24, 2019 17:53
Let these values being used as default ones in UserConfig. Also, reduce them to something more reasonable, for BREAKDOWN_TIMEOUT from 1 week to 1 day, for MAX_LOCAL_BREAKDOWN_TIMEOUT from 2 weeks to 1.
within reasonable lower or upper bound Add our_to_self_delay in Channel, to cache user config field at channel construction.
Extend test_justice_tx with user-set csv delay to test that we are able to claim revokeable outputs with different csv delay between both peers.
|
Corrected on e78c25b |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Spotted while reviewing #336.
TODO: tweak a justice test on revoked HTLC outputs to check bug fix. Need test framework changes in #348 to go first.