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

routing: move payment session to separate file #2564

Merged
merged 1 commit into from
Feb 5, 2019

Conversation

joostjager
Copy link
Contributor

Preparation to keep better overview for upcoming routing related work.

halseth
halseth previously approved these changes Jan 30, 2019
routing/router.go Outdated Show resolved Hide resolved
@joostjager
Copy link
Contributor Author

@halseth rebased, review dismissed, ptal

Copy link
Contributor

@cfromknecht cfromknecht left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

// With the edge added, we'll now report back to the global prune view,
// with this new piece of information so it can be utilized for new
// payment sessions.
p.mc.Lock()
Copy link
Contributor

Choose a reason for hiding this comment

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

Not related to this PR, tho we should abstract this operation behind a method to prevent us from modifying mission control’s internal state directly and externally acquiring its mutexes. Same for the above method

Better yet, mission control itself deserves its own interface so that access to internal variables is prevented entirely

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the design of mission control definitely needs to be stepped up.

@joostjager joostjager merged commit 077e188 into lightningnetwork:master Feb 5, 2019
@joostjager joostjager deleted the move-paymentsession branch February 5, 2019 07:45
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