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: update route hints when seeing a ChannelUpdate msg #5218

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2,285 changes: 2,285 additions & 0 deletions lntest/itest/lnd_routing_test.go

Large diffs are not rendered by default.

3,989 changes: 940 additions & 3,049 deletions lntest/itest/lnd_test.go

Large diffs are not rendered by default.

5 changes: 5 additions & 0 deletions lntest/itest/lnd_test_list_on_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,11 @@ var allTestCases = []*testCase{
name: "private channels",
test: testPrivateChannels,
},
{
name: "private channel update policy",
test: testUpdateChannelPolicyForPrivateChannel,
},

{
name: "invoice routing hints",
test: testInvoiceRoutingHints,
Expand Down
1 change: 1 addition & 0 deletions lntest/itest/log_error_whitelist.txt
Original file line number Diff line number Diff line change
Expand Up @@ -285,3 +285,4 @@
<time> [ERR] HSWC: ChannelLink(<chan>): failing link: process hodl queue: unable to update commitment: link shutting down with error: internal error
<time> [ERR] INVC: SettleHodlInvoice with preimage <hex>: invoice already canceled
<time> [ERR] RPCS: [/invoicesrpc.Invoices/SettleInvoice]: invoice already canceled
<time> [ERR] HSWC: ChannelLink(<chan>): outgoing htlc(<hex>) has insufficient fee: expected 33000, got 1020
13 changes: 13 additions & 0 deletions routing/mock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"sync"

"github.com/btcsuite/btcd/btcec"
"github.com/go-errors/errors"
"github.com/lightningnetwork/lnd/channeldb"
"github.com/lightningnetwork/lnd/htlcswitch"
Expand Down Expand Up @@ -167,6 +168,18 @@ func (m *mockPaymentSession) RequestRoute(_, _ lnwire.MilliSatoshi,
return r, nil
}

func (m *mockPaymentSession) UpdateAdditionalEdge(_ *lnwire.ChannelUpdate,
_ *btcec.PublicKey, _ *channeldb.ChannelEdgePolicy) bool {

return false
}

func (m *mockPaymentSession) GetAdditionalEdgePolicy(_ *btcec.PublicKey,
_ uint64) *channeldb.ChannelEdgePolicy {

return nil
}

type mockPayer struct {
sendResult chan error
paymentResult chan *htlcswitch.PaymentResult
Expand Down
20 changes: 4 additions & 16 deletions routing/notifications_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,11 +352,8 @@ func (m *mockChainView) Stop() error {
func TestEdgeUpdateNotification(t *testing.T) {
t.Parallel()

ctx, cleanUp, err := createTestCtxSingleNode(0)
ctx, cleanUp := createTestCtxSingleNode(t, 0)
defer cleanUp()
if err != nil {
t.Fatalf("unable to create router: %v", err)
}

// First we'll create the utxo for the channel to be "closed"
const chanValue = 10000
Expand Down Expand Up @@ -546,11 +543,8 @@ func TestNodeUpdateNotification(t *testing.T) {
t.Parallel()

const startingBlockHeight = 101
ctx, cleanUp, err := createTestCtxSingleNode(startingBlockHeight)
ctx, cleanUp := createTestCtxSingleNode(t, startingBlockHeight)
defer cleanUp()
if err != nil {
t.Fatalf("unable to create router: %v", err)
}

// We only accept node announcements from nodes having a known channel,
// so create one now.
Expand Down Expand Up @@ -739,11 +733,8 @@ func TestNotificationCancellation(t *testing.T) {
t.Parallel()

const startingBlockHeight = 101
ctx, cleanUp, err := createTestCtxSingleNode(startingBlockHeight)
ctx, cleanUp := createTestCtxSingleNode(t, startingBlockHeight)
defer cleanUp()
if err != nil {
t.Fatalf("unable to create router: %v", err)
}

// Create a new client to receive notifications.
ntfnClient, err := ctx.router.SubscribeTopology()
Expand Down Expand Up @@ -831,11 +822,8 @@ func TestChannelCloseNotification(t *testing.T) {
t.Parallel()

const startingBlockHeight = 101
ctx, cleanUp, err := createTestCtxSingleNode(startingBlockHeight)
ctx, cleanUp := createTestCtxSingleNode(t, startingBlockHeight)
defer cleanUp()
if err != nil {
t.Fatalf("unable to create router: %v", err)
}

// First we'll create the utxo for the channel to be "closed"
const chanValue = 10000
Expand Down
82 changes: 70 additions & 12 deletions routing/pathfind_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,14 @@ type testGraph struct {

// testNode represents a node within the test graph above. We skip certain
// information such as the node's IP address as that information isn't needed
// for our tests.
// for our tests. Private keys are optional. If set, they should be consistent
Copy link
Member

Choose a reason for hiding this comment

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

FWIW new tests don't really use the old static graph file and instead opt tin create a graph on the fly using some of the newly added test helpers. However I understand this was a revived PR and at the time of writing of this original commit, this was the only/preferred way to do things.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Got it. So are you suggesting we switch these tests to make the graph created on the fly in this PR?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

And unrelated to this PR, we probably want to refactor old tests to get rid of the static graph file someday?

Copy link
Member

Choose a reason for hiding this comment

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

Got it. So are you suggesting we switch these tests to make the graph created on the fly in this PR?

Don't think it's a blocker, as this commit only exists as we salvaged the OG commit from the contributor.

// with the public key. The private key is used to sign error messages
// sent from the node.
type testNode struct {
Source bool `json:"source"`
PubKey string `json:"pubkey"`
Alias string `json:"alias"`
Source bool `json:"source"`
PubKey string `json:"pubkey"`
PrivKey string `json:"privkey"`
Alias string `json:"alias"`
}

// testChan represents the JSON version of a payment channel. This struct
Expand Down Expand Up @@ -200,6 +203,8 @@ func parseTestGraph(path string) (*testGraphInstance, error) {
}

aliasMap := make(map[string]route.Vertex)
privKeyMap := make(map[string]*btcec.PrivateKey)
channelIDs := make(map[route.Vertex]map[route.Vertex]uint64)
var source *channeldb.LightningNode

// First we insert all the nodes within the graph as vertexes.
Expand Down Expand Up @@ -230,6 +235,33 @@ func parseTestGraph(path string) (*testGraphInstance, error) {
// alias map for easy lookup.
aliasMap[node.Alias] = dbNode.PubKeyBytes

// private keys are needed for signing error messages. If set
// check the consistency with the public key.
privBytes, err := hex.DecodeString(node.PrivKey)
if err != nil {
return nil, err
}
if len(privBytes) > 0 {
key, derivedPub := btcec.PrivKeyFromBytes(
btcec.S256(), privBytes,
)

if !bytes.Equal(
pubBytes, derivedPub.SerializeCompressed(),
) {

return nil, fmt.Errorf("%s public key and "+
"private key are inconsistent\n"+
"got %x\nwant %x\n",
node.Alias,
derivedPub.SerializeCompressed(),
pubBytes,
)
}

privKeyMap[node.Alias] = key
}

// If the node is tagged as the source, then we create a
// pointer to is so we can mark the source in the graph
// properly.
Expand All @@ -240,7 +272,8 @@ func parseTestGraph(path string) (*testGraphInstance, error) {
// node can be the source in the graph.
if source != nil {
return nil, errors.New("JSON is invalid " +
"multiple nodes are tagged as the source")
"multiple nodes are tagged as the " +
"source")
}

source = dbNode
Expand Down Expand Up @@ -324,12 +357,35 @@ func parseTestGraph(path string) (*testGraphInstance, error) {
if err := graph.UpdateEdgePolicy(edgePolicy); err != nil {
return nil, err
}

// We also store the channel IDs info for each of the node.
node1Vertex, err := route.NewVertexFromBytes(node1Bytes)
if err != nil {
return nil, err
}

node2Vertex, err := route.NewVertexFromBytes(node2Bytes)
if err != nil {
return nil, err
}

if _, ok := channelIDs[node1Vertex]; !ok {
channelIDs[node1Vertex] = map[route.Vertex]uint64{}
}
channelIDs[node1Vertex][node2Vertex] = edge.ChannelID

if _, ok := channelIDs[node2Vertex]; !ok {
channelIDs[node2Vertex] = map[route.Vertex]uint64{}
}
channelIDs[node2Vertex][node1Vertex] = edge.ChannelID
}

return &testGraphInstance{
graph: graph,
cleanUp: cleanUp,
aliasMap: aliasMap,
graph: graph,
cleanUp: cleanUp,
aliasMap: aliasMap,
privKeyMap: privKeyMap,
channelIDs: channelIDs,
}, nil
}

Expand Down Expand Up @@ -402,6 +458,9 @@ type testGraphInstance struct {
// privKeyMap maps a node alias to its private key. This is used to be
// able to mock a remote node's signing behaviour.
privKeyMap map[string]*btcec.PrivateKey

// channelIDs stores the channel ID for each node.
channelIDs map[route.Vertex]map[route.Vertex]uint64
}

// createTestGraphFromChannels returns a fully populated ChannelGraph based on a set of
Expand Down Expand Up @@ -2052,10 +2111,9 @@ func TestPathFindSpecExample(t *testing.T) {
// we'll pass that in to ensure that the router uses 100 as the current
// height.
const startingHeight = 100
ctx, cleanUp, err := createTestCtxFromFile(startingHeight, specExampleFilePath)
if err != nil {
t.Fatalf("unable to create router: %v", err)
}
ctx, cleanUp := createTestCtxFromFile(
t, startingHeight, specExampleFilePath,
)
defer cleanUp()

// We'll first exercise the scenario of a direct payment from Bob to
Expand Down
Loading