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

Shuffle #157

Closed
wants to merge 32 commits into from
Closed

Shuffle #157

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

This field is only ever written to and never read from. We remove it so
that later on when we want to convert NetAddress into and from a
protobuf message, we don't need to encode the ChainHash uneccessarily.
This was included in NetAddress when the plan was still to have LND
cater for different blockchains.
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.
Copy link

github-actions bot commented Nov 8, 2024

Pull reviewers stats

Stats of the last 30 days for lnd:

User Total reviews Time to review Total comments

So that we can stop depending on kvdb.RTx directly. The implementation
of this is currently just a kvdb.RTx but eventually this could be a SQL
transaction or even multiple transactions for the case where the query
is being sent to both a local and remote db. The only place where we
want to expose a transaction right now is for pathfinding, so we only do
this for the `graphsession.graph` interface used by pathfinding.
In preparation for a clean graph source interface, we create a new
ForEachNode method that does not pass a DB transaction to the call-back
provided. The existing method is renamed to ForEachNodeCBTx.
Any call that wants to pass in a transaction to the call-back can make
use of ForEachNodeChannelTx instead.
In preparation for adding this method to an interface that can be
satisfied by a different DB or even a backing RPC connection, we let it
take a context.
In preparation for this interface being backed by an RPC client or
different type of DB.
TODO: split up into separate PR...
Define a new GraphSource interface required by the invoicerpc server and
remove its access to the graphdb.ChannelGraph pointer.
When set, LND will not advertise the gossip queries feature bit and it
will not initiate gossip syncing with any peer.
@ellemouton ellemouton force-pushed the shuffle branch 4 times, most recently from 4b6a11d to 9ab9912 Compare November 9, 2024 10:19
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.

1 participant