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

followups: Configurable Forwarding Limit and Stub Improvements #80

Merged

Conversation

carlaKC
Copy link
Collaborator

@carlaKC carlaKC commented Oct 2, 2023

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.

@carlaKC carlaKC marked this pull request as ready for review October 3, 2023 15:57
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.

Thanks again for these fixes!

db.go Outdated Show resolved Hide resolved
process.go Show resolved Hide resolved
stub.go Outdated Show resolved Hide resolved
run.go Show resolved Hide resolved
process.go Show resolved Hide resolved
@carlaKC
Copy link
Collaborator Author

carlaKC commented Oct 11, 2023

Rebased this on #82 because the fix is required for tests in this PR + addressed comments.

db.go Show resolved Hide resolved
lndclient.go Outdated Show resolved Hide resolved
db.go Outdated Show resolved Hide resolved
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.
@carlaKC carlaKC force-pushed the forwarding-history-followups branch from 0f4a2d3 to fa9ee24 Compare October 20, 2023 14:18
@carlaKC
Copy link
Collaborator Author

carlaKC commented Oct 20, 2023

Rebased on #82 and addressed final comments!

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.

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
Copy link
Contributor

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.

Copy link
Collaborator Author

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

@carlaKC carlaKC merged commit 9cb88f2 into lightningequipment:master Oct 20, 2023
3 checks passed
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