-
Notifications
You must be signed in to change notification settings - Fork 18
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
followups: Configurable Forwarding Limit and Stub Improvements #80
followups: Configurable Forwarding Limit and Stub Improvements #80
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again for these fixes!
5373c1b
to
87b5c05
Compare
Rebased this on #82 because the fix is required for tests in this PR + addressed comments. |
87b5c05
to
0f4a2d3
Compare
Previously if there was an error in process/peercontroller, the stub would continue to run and block on sending new htlcs into channels that have nothing listening on the other end. This change passes in the context used by process so that the stub can shut down cleanly.
0f4a2d3
to
fa9ee24
Compare
Rebased on #82 and addressed final comments! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, once more!
require.Len(t, fwds, 1) | ||
|
||
// Modify the db to have a zero limit on forwarding history. We don't recreate | ||
// the test db because it would re-create the file. Run limitHTLCRecords once |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe not the most beautiful way to cover, but good enough I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah :| didn't think it was worth updating the test db setup for this one case, but I agree.. ugly
Some of the follow ups mentioned for #79. Happy to split these up into multiple PRs, but though one would be okay since the changes are fairly minimal.
Depends on #82.