Sometimes only the "Send coverage" step of a CI job will fail. This
commit turns this step into a "best effort" step instead so that it
does not block a CI job from passing.
It can, for example, often happen that a single job is re-run from the
GH UI, it then passes but the "Send coverage" step fails due to the
"Can't add a job to a build that is already closed." error meaning that
the only way to get the CI step to pass is to re-push and retrigger a
full CI run.
With the previous commit, the AddNode method was removed and since that
was the only method making use of the ForEachChannel on the
GraphCacheNode interface, we can remove that method. Since the only two
methods left just expose the node's pub key and features, it really is
not required anymore and so the entire thing can be removed along with
the implementation of it.
The AddNode method on the GraphCache calls `AddNodeFeatures` underneath
and then iterates through all the node's persisted channels and adds
them to the cache too via `AddChannel`.
This is, however, not required since at the time the cache is populated
in `NewChannelGraph`, the cache is populated will all persisted nodes
and all persisted channels. Then, once any new channels come in, via
`AddChannelEdge`, they are added to the cache via AddChannel. If any new
nodes come in via `AddLightningNode`, then currently the cache's AddNode
method is called which the both adds the node and again iterates through
all persisted channels and re-adds them to the cache. This is definitely
redundent since the initial cache population and updates via
AddChannelEdge should keep the cache fresh in terms of channels.
So we remove this for 2 reasons: 1) to remove the redundent DB calls and
2) this requires a kvdb.RTx to be passed in to the GraphCache calls
which will make it hard to extract the cache out of the CRUD layer
and be used more generally.
The AddNode method made sense when the cache was first added in the
code-base
[here](369c09be61 (diff-ae36bdb6670644d20c4e43f3a0ed47f71886c2bcdf3cc2937de24315da5dc072R213))
since then during graph cache population, nodes and channels would be
added to the cache in a single DB transaction. This was, however,
changed [later
on](352008a0c2)
to be done in 2 separate DB calls for efficiency reasons.
Define a new GraphSource interface that describes the access required by
the autopilot server. Let its constructor take this interface instead of
a raw pointer to the graphdb.ChannelGraph.
In this commit, a new NodeRTx interface is added which represents
consistent access to a persisted models.LightningNode. The
ForEachChannel method of the interface gives the caller access to the
node's channels under the same read transaction (if any) that was used
to fetch the node in the first place. The FetchNode method returns
another NodeRTx which again will have the same underlying read
transaction.
The main point of this interface is to provide this consistent access
without needing to expose the `kvdb.RTx` type as a method parameter.
This will then make it much easier in future to add new implementations
of this interface that are backed by other databases (or RPC
connections) where the `kvdb.RTx` type does not apply.
We will make use of the new interface in the `autopilot` package in
upcoming commits in order to remove the `autopilot`'s dependence on the
pointer to the `*graphdb.ChannelGraph` which it has today.
Introduce a new type for testing code so the main databaseChannelGraph
type does not need to make various write calls to the
`graphdb.ChannelGraph` but the new testDBGraph type still can for tests.
This is a pure code move commit where we move any code that is only ever
used by tests to test files. Many of the calls to the
graphdb.ChannelGraph pointer are only coming from tests code.
We need to make sure the links are recreated before calling the
interceptor, otherwise we'd get the following error, causing the test to
fail.
```
2025-02-07 15:01:38.991 [ERR] RPCS interceptor.go:624: [/routerrpc.Router/HtlcInterceptor]: fwd (Chan ID=487:3:0, HTLC ID=0) not found
```
This commit refactors `collectResultAsync` such that this method is now
only responsible for collecting results from the switch. The method
`decideNextStep` is expanded to process these results in the same
goroutine where we fetch the payment from db, to make sure the lifecycle
loop always have a consistent view of a given payment.
In preparation for adding more modifiers. We want to later add a
modifier that will tweak the errors returned by the mock chain once
funding transaction validation has been moved to the gossiper.
This is in preparation for moving the funding transaction validation
code to the gossiper from the graph.Builder since then the gossiper will
start making GetBlockHash/GetBlock and GetUtxo calls.
Convert a bunch of the helper functions to instead be methods on the
testCtx type. This is in preparation for adding a mockChain to the
testCtx that these helpers can then use to add blocks and utxos to.
See `notifications_test.go` for an idea of what we are trying to emulate
here. Once the funding tx code has moved to the gossiper, then the logic
in `notifications_test.go` will be removed.
In preparation for moving funding transaction validiation from the
Builder to the Gossiper in later commit, we first convert these graph
Error Codes to normal error variables. This will help make the later
commit a pure code move.
The point of the `graph.Builder`'s `networkHandler` goroutine is to
ensure that certain requests are handled in a synchronous fashion.
However, any requests received on the `networkUpdates` channel, are
currently immediately handled in a goroutine which calls
`handleNetworkUpdate` which calls `processUpdate` before doing topology
notifications. In other words, there is no reason for these
`networkUpdates` to be handled in the `networkHandler` since they are
always handled asynchronously anyways. This design is most likely due to
the fact that originally the gossiper and graph builder code lived in
the same system and so the pattern was copied across.
So in this commit, we just remove the complexity. The only part we need
to spin off in a goroutine is the topology notifications.
In this commit, we remove the `processUpdate` method which handles each
announement type (node, channel, channel update) in a separate switch
case. Each of these cases currently has a non-trivial amount of code.
This commit creates separate methods for each message type we want to
handle instead. This removes a level of indentation and will make things
easier to review when we start editing the code for each handler.
The `netann` package is a more appropriate place for this code to live.
Also, once the funding transaction code is moved out of the
`graph.Builder`, then no `lnwire` validation will occur in the `graph`
package.
