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

Misc1 graph #173

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

Misc1 graph #173

wants to merge 30 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.

Copy link

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 21, 2025

Pull Request Test Coverage Report for Build 12948601523

Details

  • 3168 of 4631 (68.41%) changed or added relevant lines in 42 files are covered.
  • 28854 unchanged lines in 442 files lost coverage.
  • Overall coverage decreased (-10.1%) to 48.655%

Changes Missing Coverage Covered Lines Changed/Added Lines %
chainreg/no_chain_backend.go 0 1 0.0%
graph/db/graph_cache.go 25 26 96.15%
lnrpc/invoicesrpc/addinvoice.go 7 8 87.5%
peer/test_utils.go 0 1 0.0%
batch/batch.go 3 5 60.0%
channeldb/codec.go 4 6 66.67%
graph/db/models/edge_point.go 0 3 0.0%
lnrpc/graphrpc/log.go 6 9 66.67%
lnrpc/routerrpc/router_backend.go 7 10 70.0%
lncfg/remote_graph.go 6 10 60.0%
Files with Coverage Reduction New Missed Lines %
graph/errors.go 1 94.74%
channeldb/forwarding_policy.go 2 85.14%
routing/chainview/queue.go 2 94.29%
record/hop.go 2 93.33%
rpcperms/interceptor.go 2 82.91%
lnrpc/invoicesrpc/invoices_server.go 2 74.55%
lnwallet/sigpool.go 2 65.63%
netann/sign.go 2 70.0%
contractcourt/htlc_incoming_contest_resolver.go 2 81.17%
lnwire/stfu.go 2 75.0%
Totals Coverage Status
Change from base Build 12948129598: -10.1%
Covered Lines: 99805
Relevant Lines: 205127

💛 - Coveralls

@ellemouton ellemouton force-pushed the misc1Graph branch 5 times, most recently from 55dfee6 to 5bde390 Compare January 22, 2025 12:21
The authenticated gossiper is already given access to the graph Builder
which has the IsZombieChannel method on it. So this does not need to be
passed in as a separate config value.
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.
In this commit, we remove the graphsession package which added an
uneccessary layer of indirection between the underlying graph and the
interface required by the router. After this commit, the graph "session"
that the router starts, is now fully encapsulated within the
ChannelGraph which means we no longer expose the underlying kvdb.Tx at
all. This simplification/encapsulation will also be important for when
we implement non-kvdb backends for the graph.
In preparation for an external source (over RPC) or a different DB type
implementing this interface, we update the methods to take contexts.
In this commit, we add a bunch of context.TODO()s all of which will be
removed in the following commits.
Since we want to convert this layer to be purely bbolt CRUD. and we will
re-use the name ChannelGraph elsewhere.
When set, LND will not advertise the gossip queries feature bit and it
will not initiate gossip syncing with any peer.
We already set `nobootstrap` in the default node flags for itest nodes,
so we can remove this check now. This will allow us to later test
bootstrapping in an itest.
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