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

Mega graph clean #171

Open
wants to merge 29 commits into
base: master
Choose a base branch
from
Open

Mega graph clean #171

wants to merge 29 commits into from

Conversation

ellemouton
Copy link
Owner

Change Description

Description of change / link to associated issue.

Steps to Test

Steps for reviewers to follow to test the change.

Pull Request Checklist

Testing

  • Your PR passes all CI checks.
  • Tests covering the positive and negative (error paths) are included.
  • Bug fixes contain tests triggering the bug to prevent regressions.

Code Style and Documentation

📝 Please see our Contribution Guidelines for further guidance.

Simplify the ChannelGraphSource interface by removing this unused
method.
For consistency in the graphsessoin.graph interface, we let the
FetchNodeFeatures method take a read transaction just like the
ForEachNodeDirectedChannel. This is nice because then all calls in the
same pathfinding transaction use the same read transaction.
The package name is kept the same. So this is just for less stuttering
at a path level.
This commit adds a new GraphSources interface that LND requires for
graph related read-only queries. As of this commit, the interface is
empty but it will be populated over the next couple of commits. We add
an implementation of this interface backed by a pointer to a
graphdb.ChannelGraph.

The infrustructure is put into place so that the GraphSoure provided to
LND can be overridden by a caller of the lnd.Main function. By default,
LND will satisfy the interface itself via the new `graphsource.DBSource`
struct.
In this commit, we take the existing graphsession.ReadyOnlyGraph
interface and remove its usage of a kvdb.RTx and replace it with a more
abstract `RTx` interface type.

The new GraphSource interface is expanded to include the
graphsession.ReadOnlyGraph interface and the implementation of it,
DBSource, is expanded to include the new methods. It converts the
given RTx to the underlying kvdb read transaction where needed.
Since the GraphSource interface may be satisfied by an RPC connection,
it is best practice to pass a context through to any call in the
interface.

The ChanGraphSource implementation, which uses a kvdb backend, does not
make use of the ctx. Any call-sites are for now given a `context.TODO()`
which will all be addressed in follow up commits.
Define a new GraphSource interface required by the invoicerpc server and
remove its access to the graphdb.ChannelGraph pointer. Add the new
invoicesrpc.GraphSource interface to the GraphSource interface
and let DBSource implement it.
In this commit, we add a bunch of graph methods to the GraphSource, let
DBSource implement it and then we use the graph source for these
methods for the GetNodeInfo, VerifyMessage, DescribeGraph,
GetNodeMetrics, GetChanInfo and GetNodeInfo RPC calls along with peer
alias lookup.
We will later implement this interface with a backing RPC connection and
so it makes sense to pass a context through for cancellation.
So that LND can use a different GraphSource for network bootstrapping
and does not need to rely on its local graph db.
so that the external graph source can be used to query network
information rather than depending on the local graph DB.
So that the calcuation is abstracted behind the interface and not
necessarily dependent on LND's local channel graph.
Remove four context.TODO()s
Copy link

github-actions bot commented Jan 8, 2025

Pull reviewers stats

Stats of the last 30 days for lnd:

User Total reviews Time to review Total comments

@coveralls
Copy link

coveralls commented Jan 8, 2025

Pull Request Test Coverage Report for Build 12690012727

Details

  • 357 of 505 (70.69%) changed or added relevant lines in 45 files are covered.
  • 27933 unchanged lines in 442 files lost coverage.
  • Overall coverage decreased (-9.8%) to 48.869%

Changes Missing Coverage Covered Lines Changed/Added Lines %
autopilot/combinedattach.go 0 1 0.0%
chanbackup/recover.go 2 3 66.67%
graph/db/graph.go 2 3 66.67%
lnrpc/autopilotrpc/autopilot_server.go 0 1 0.0%
peer/test_utils.go 0 1 0.0%
autopilot/top_centrality.go 0 2 0.0%
lnd.go 6 8 75.0%
pilot.go 0 2 0.0%
autopilot/betweenness_centrality.go 0 3 0.0%
autopilot/simple_graph.go 0 3 0.0%
Files with Coverage Reduction New Missed Lines %
graph/errors.go 1 94.74%
discovery/bootstrapper.go 1 1.03%
lnwire/typed_fee.go 2 66.67%
channeldb/forwarding_policy.go 2 85.14%
routing/chainview/queue.go 2 94.29%
record/hop.go 2 93.33%
lnwire/short_channel_id.go 2 89.74%
lnwallet/sigpool.go 2 65.63%
netann/sign.go 2 70.0%
contractcourt/htlc_incoming_contest_resolver.go 2 81.17%
Totals Coverage Status
Change from base Build 12442824783: -9.8%
Covered Lines: 99494
Relevant Lines: 203592

💛 - Coveralls

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