From ccf4b7feabf253684c423eb2c758811530bb76b1 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 19 Dec 2018 14:54:54 +0100 Subject: [PATCH] autopilot: move address lookup from heuristic to agent To avoid having the heuristics deal with (possibly conflicting) address lookups, we let the agent handle them. --- autopilot/agent.go | 15 ++++ autopilot/agent_test.go | 162 +++++++++++++++-------------------- autopilot/prefattach.go | 16 +--- autopilot/prefattach_test.go | 20 ----- 4 files changed, 85 insertions(+), 128 deletions(-) diff --git a/autopilot/agent.go b/autopilot/agent.go index 5a7ca0c0..44d99ec9 100644 --- a/autopilot/agent.go +++ b/autopilot/agent.go @@ -525,6 +525,7 @@ func (a *Agent) openChans(availableFunds btcutil.Amount, numChans uint32, // want to skip. selfPubBytes := a.cfg.Self.SerializeCompressed() nodes := make(map[NodeID]struct{}) + addresses := make(map[NodeID][]net.Addr) if err := a.cfg.Graph.ForEachNode(func(node Node) error { nID := NodeID(node.PubKey()) @@ -535,6 +536,14 @@ func (a *Agent) openChans(availableFunds btcutil.Amount, numChans uint32, return nil } + // If the node has no known addresses, we cannot connect to it, + // so we'll skip it. + addrs := node.Addrs() + if len(addrs) == 0 { + return nil + } + addresses[nID] = addrs + // Additionally, if this node is in the blacklist, then // we'll skip it. if _, ok := nodesToSkip[nID]; ok { @@ -562,6 +571,12 @@ func (a *Agent) openChans(availableFunds btcutil.Amount, numChans uint32, return fmt.Errorf("unable to calculate node scores : %v", err) } + // Add addresses to the candidates. + for nID, c := range scores { + addrs := addresses[nID] + c.Addrs = addrs + } + log.Debugf("Got scores for %d nodes", len(scores)) // Now use the score to make a weighted choice which diff --git a/autopilot/agent_test.go b/autopilot/agent_test.go index e7b46306..964dd144 100644 --- a/autopilot/agent_test.go +++ b/autopilot/agent_test.go @@ -1,7 +1,6 @@ package autopilot import ( - "bytes" "errors" "fmt" "net" @@ -306,6 +305,10 @@ func TestAgentChannelFailureSignal(t *testing.T) { chanController := &mockFailingChanController{} memGraph, _, _ := newMemChanGraph() + node, err := memGraph.addRandNode() + if err != nil { + t.Fatalf("unable to add node: %v", err) + } // With the dependencies we created, we can now create the initial // agent itself. @@ -316,6 +319,7 @@ func TestAgentChannelFailureSignal(t *testing.T) { WalletBalance: func() (btcutil.Amount, error) { return 0, nil }, + // TODO: move address check to agent. ConnectToPeer: func(*btcec.PublicKey, []net.Addr) (bool, error) { return false, nil }, @@ -360,19 +364,14 @@ func TestAgentChannelFailureSignal(t *testing.T) { // request attachment directives, return a fake so the agent will // attempt to open a channel. var fakeDirective = &AttachmentDirective{ - NodeID: NewNodeID(self), + NodeID: NewNodeID(node), ChanAmt: btcutil.SatoshiPerBitcoin, - Addrs: []net.Addr{ - &net.TCPAddr{ - IP: bytes.Repeat([]byte("a"), 16), - }, - }, - Score: 0.5, + Score: 0.5, } select { case heuristic.nodeScoresResps <- map[NodeID]*AttachmentDirective{ - NewNodeID(self): fakeDirective, + NewNodeID(node): fakeDirective, }: case <-time.After(time.Second * 10): t.Fatal("heuristic wasn't queried in time") @@ -707,6 +706,22 @@ func TestAgentImmediateAttach(t *testing.T) { const numChans = 5 + // We'll generate 5 mock directives so it can progress within its loop. + directives := make(map[NodeID]*AttachmentDirective) + nodeKeys := make(map[NodeID]struct{}) + for i := 0; i < numChans; i++ { + pub, err := memGraph.addRandNode() + if err != nil { + t.Fatalf("unable to generate key: %v", err) + } + nodeID := NewNodeID(pub) + directives[nodeID] = &AttachmentDirective{ + NodeID: nodeID, + ChanAmt: btcutil.SatoshiPerBitcoin, + Score: 0.5, + } + nodeKeys[nodeID] = struct{}{} + } // The very first thing the agent should do is query the NeedMoreChans // method on the passed heuristic. So we'll provide it with a response // that will kick off the main loop. @@ -724,31 +739,9 @@ func TestAgentImmediateAttach(t *testing.T) { } // At this point, the agent should now be querying the heuristic to - // requests attachment directives. We'll generate 5 mock directives so - // it can progress within its loop. - directives := make(map[NodeID]*AttachmentDirective) - nodeKeys := make(map[NodeID]struct{}) - for i := 0; i < numChans; i++ { - pub, err := randKey() - if err != nil { - t.Fatalf("unable to generate key: %v", err) - } - nodeID := NewNodeID(pub) - directives[nodeID] = &AttachmentDirective{ - NodeID: nodeID, - ChanAmt: btcutil.SatoshiPerBitcoin, - Addrs: []net.Addr{ - &net.TCPAddr{ - IP: bytes.Repeat([]byte("a"), 16), - }, - }, - Score: 0.5, - } - nodeKeys[nodeID] = struct{}{} - } - - // With our fake directives created, we'll now send then to the agent - // as a return value for the Select function. + // requests attachment directives. With our fake directives created, + // we'll now send then to the agent as a return value for the Select + // function. select { case heuristic.nodeScoresResps <- directives: case <-time.After(time.Second * 10): @@ -853,6 +846,21 @@ func TestAgentPrivateChannels(t *testing.T) { const numChans = 5 + // We'll generate 5 mock directives so the pubkeys will be found in the + // agent's graph, and it can progress within its loop. + directives := make(map[NodeID]*AttachmentDirective) + for i := 0; i < numChans; i++ { + pub, err := memGraph.addRandNode() + if err != nil { + t.Fatalf("unable to generate key: %v", err) + } + directives[NewNodeID(pub)] = &AttachmentDirective{ + NodeID: NewNodeID(pub), + ChanAmt: btcutil.SatoshiPerBitcoin, + Score: 0.5, + } + } + // The very first thing the agent should do is query the NeedMoreChans // method on the passed heuristic. So we'll provide it with a response // that will kick off the main loop. We'll send over a response @@ -867,30 +875,10 @@ func TestAgentPrivateChannels(t *testing.T) { case <-time.After(time.Second * 10): t.Fatalf("heuristic wasn't queried in time") } - // At this point, the agent should now be querying the heuristic to - // requests attachment directives. We'll generate 5 mock directives so - // it can progress within its loop. - directives := make(map[NodeID]*AttachmentDirective) - for i := 0; i < numChans; i++ { - pub, err := randKey() - if err != nil { - t.Fatalf("unable to generate key: %v", err) - } - directives[NewNodeID(pub)] = &AttachmentDirective{ - NodeID: NewNodeID(pub), - ChanAmt: btcutil.SatoshiPerBitcoin, - Addrs: []net.Addr{ - &net.TCPAddr{ - IP: bytes.Repeat([]byte("a"), 16), - }, - }, - Score: 0.5, - } - } - - // With our fake directives created, we'll now send then to the agent - // as a return value for the Select function. + // requests attachment directives. With our fake directives created, + // we'll now send then to the agent as a return value for the Select + // function. select { case heuristic.nodeScoresResps <- directives: case <-time.After(time.Second * 10): @@ -986,6 +974,18 @@ func TestAgentPendingChannelState(t *testing.T) { // exiting. defer close(quit) + // We'll only return a single directive for a pre-chosen node. + nodeKey, err := memGraph.addRandNode() + if err != nil { + t.Fatalf("unable to generate key: %v", err) + } + nodeID := NewNodeID(nodeKey) + nodeDirective := &AttachmentDirective{ + NodeID: nodeID, + ChanAmt: 0.5 * btcutil.SatoshiPerBitcoin, + Score: 0.5, + } + // Once again, we'll start by telling the agent as part of its first // query, that it needs more channels and has 3 BTC available for // attachment. We'll send over a response indicating that it should @@ -1001,25 +1001,6 @@ func TestAgentPendingChannelState(t *testing.T) { constraints.moreChanArgs = make(chan moreChanArg) - // Next, the agent should deliver a query to the Select method of the - // heuristic. We'll only return a single directive for a pre-chosen - // node. - nodeKey, err := randKey() - if err != nil { - t.Fatalf("unable to generate key: %v", err) - } - nodeID := NewNodeID(nodeKey) - nodeDirective := &AttachmentDirective{ - NodeID: nodeID, - ChanAmt: 0.5 * btcutil.SatoshiPerBitcoin, - Addrs: []net.Addr{ - &net.TCPAddr{ - IP: bytes.Repeat([]byte("a"), 16), - }, - }, - Score: 0.5, - } - select { case heuristic.nodeScoresResps <- map[NodeID]*AttachmentDirective{ nodeID: nodeDirective, @@ -1398,6 +1379,17 @@ func TestAgentSkipPendingConns(t *testing.T) { // exiting. defer close(quit) + // We'll only return a single directive for a pre-chosen node. + nodeKey, err := memGraph.addRandNode() + if err != nil { + t.Fatalf("unable to generate key: %v", err) + } + nodeDirective := &AttachmentDirective{ + NodeID: NewNodeID(nodeKey), + ChanAmt: 0.5 * btcutil.SatoshiPerBitcoin, + Score: 0.5, + } + // We'll send an initial "yes" response to advance the agent past its // initial check. This will cause it to try to get directives from the // graph. @@ -1410,24 +1402,6 @@ func TestAgentSkipPendingConns(t *testing.T) { t.Fatalf("heuristic wasn't queried in time") } - // Next, the agent should deliver a query to the Select method of the - // heuristic. We'll only return a single directive for a pre-chosen - // node. - nodeKey, err := randKey() - if err != nil { - t.Fatalf("unable to generate key: %v", err) - } - nodeDirective := &AttachmentDirective{ - NodeID: NewNodeID(nodeKey), - ChanAmt: 0.5 * btcutil.SatoshiPerBitcoin, - Addrs: []net.Addr{ - &net.TCPAddr{ - IP: bytes.Repeat([]byte("a"), 16), - }, - }, - Score: 0.5, - } - select { case heuristic.nodeScoresResps <- map[NodeID]*AttachmentDirective{ NewNodeID(nodeKey): nodeDirective, diff --git a/autopilot/prefattach.go b/autopilot/prefattach.go index b71ca8f9..3c52e231 100644 --- a/autopilot/prefattach.go +++ b/autopilot/prefattach.go @@ -2,7 +2,6 @@ package autopilot import ( prand "math/rand" - "net" "time" "github.com/btcsuite/btcd/btcec" @@ -75,11 +74,9 @@ func (p *ConstrainedPrefAttachment) NodeScores(g ChannelGraph, chans []Channel, map[NodeID]*AttachmentDirective, error) { // Count the number of channels in the graph. We'll also count the - // number of channels as we go for the nodes we are interested in, and - // record their addresses found in the db. + // number of channels as we go for the nodes we are interested in. var graphChans int nodeChanNum := make(map[NodeID]int) - addresses := make(map[NodeID][]net.Addr) if err := g.ForEachNode(func(n Node) error { var nodeChans int err := n.ForEachChannel(func(_ ChannelEdge) error { @@ -98,10 +95,8 @@ func (p *ConstrainedPrefAttachment) NodeScores(g ChannelGraph, chans []Channel, return nil } - // Otherwise we'll record the number of channels, and also - // populate the address in our channel candidates map. + // Otherwise we'll record the number of channels. nodeChanNum[nID] = nodeChans - addresses[nID] = n.Addrs() return nil }); err != nil { @@ -126,7 +121,6 @@ func (p *ConstrainedPrefAttachment) NodeScores(g ChannelGraph, chans []Channel, for nID, nodeChans := range nodeChanNum { _, ok := existingPeers[nID] - addrs := addresses[nID] switch { @@ -135,11 +129,6 @@ func (p *ConstrainedPrefAttachment) NodeScores(g ChannelGraph, chans []Channel, case ok: continue - // If the node has no addresses, we cannot connect to it, so we - // skip it for now, which implicitly gives it a score of 0. - case len(addrs) == 0: - continue - // If the node had no channels, we skip it, since it would have // gotten a zero score anyway. case nodeChans == 0: @@ -152,7 +141,6 @@ func (p *ConstrainedPrefAttachment) NodeScores(g ChannelGraph, chans []Channel, candidates[nID] = &AttachmentDirective{ NodeID: nID, ChanAmt: chanSize, - Addrs: addrs, Score: score, } } diff --git a/autopilot/prefattach_test.go b/autopilot/prefattach_test.go index 2c5f1514..3deda3b5 100644 --- a/autopilot/prefattach_test.go +++ b/autopilot/prefattach_test.go @@ -287,11 +287,6 @@ func TestConstrainedPrefAttachmentSelectTwoVertexes(t *testing.T) { "to be %v, instead was %v", expScore, candidate.Score) } - - if len(candidate.Addrs) == 0 { - t1.Fatalf("expected node to have " + - "available addresses, didn't") - } } }) if !success { @@ -492,11 +487,6 @@ func TestConstrainedPrefAttachmentSelectGreedyAllocation(t *testing.T) { "of %v, instead got %v", maxChanSize, candidate.ChanAmt) } - - if len(candidate.Addrs) == 0 { - t1.Fatalf("expected node to have " + - "available addresses, didn't") - } } // Imagine a few channels are being opened, and there's @@ -527,11 +517,6 @@ func TestConstrainedPrefAttachmentSelectGreedyAllocation(t *testing.T) { "of %v, instead got %v", remBalance, candidate.ChanAmt) } - - if len(candidate.Addrs) == 0 { - t1.Fatalf("expected node to have " + - "available addresses, didn't") - } } }) if !success { @@ -622,11 +607,6 @@ func TestConstrainedPrefAttachmentSelectSkipNodes(t *testing.T) { "of %v, instead got %v", maxChanSize, candidate.ChanAmt) } - - if len(candidate.Addrs) == 0 { - t1.Fatalf("expected node to have " + - "available addresses, didn't") - } } // We'll simulate a channel update by adding the nodes