From 1d82e12fcfb4b896fa090d64088c77aee755ea70 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 19 Dec 2018 14:54:53 +0100 Subject: [PATCH 01/10] autopilot: define AgentConstraints To decouple the autopilot heuristic from the constraints, we start by abstracting them behind an interface to make them easier to mock. We also rename them HeuristicConstraints->AgentConstraints to make it clear that they are now constraints the agent must adhere to. --- autopilot/agent.go | 8 +- autopilot/agent_constraints.go | 151 +++++++++++++++++++++++++++++ autopilot/agent_test.go | 101 +++++++++++-------- autopilot/heuristic_constraints.go | 76 --------------- autopilot/prefattach.go | 10 +- autopilot/prefattach_test.go | 79 ++++++++------- pilot.go | 14 +-- 7 files changed, 271 insertions(+), 168 deletions(-) create mode 100644 autopilot/agent_constraints.go delete mode 100644 autopilot/heuristic_constraints.go diff --git a/autopilot/agent.go b/autopilot/agent.go index 56038c016..d384abde1 100644 --- a/autopilot/agent.go +++ b/autopilot/agent.go @@ -57,7 +57,7 @@ type Config struct { // Constraints is the set of constraints the autopilot must adhere to // when opening channels. - Constraints *HeuristicConstraints + Constraints AgentConstraints // TODO(roasbeef): add additional signals from fee rates and revenue of // currently opened channels @@ -573,11 +573,11 @@ func (a *Agent) openChans(availableFunds btcutil.Amount, numChans uint32, // available to future heuristic selections. a.pendingMtx.Lock() defer a.pendingMtx.Unlock() - if uint16(len(a.pendingOpens)) >= a.cfg.Constraints.MaxPendingOpens { + if uint16(len(a.pendingOpens)) >= a.cfg.Constraints.MaxPendingOpens() { log.Debugf("Reached cap of %v pending "+ "channel opens, will retry "+ "after success/failure", - a.cfg.Constraints.MaxPendingOpens) + a.cfg.Constraints.MaxPendingOpens()) return nil } @@ -642,7 +642,7 @@ func (a *Agent) executeDirective(directive AttachmentDirective) { // first. a.pendingMtx.Lock() if uint16(len(a.pendingOpens)) >= - a.cfg.Constraints.MaxPendingOpens { + a.cfg.Constraints.MaxPendingOpens() { // Since we've reached our max number of pending opens, we'll // disconnect this peer and exit. However, if we were // previously connected to them, then we'll make sure to diff --git a/autopilot/agent_constraints.go b/autopilot/agent_constraints.go new file mode 100644 index 000000000..914a16cee --- /dev/null +++ b/autopilot/agent_constraints.go @@ -0,0 +1,151 @@ +package autopilot + +import ( + "github.com/btcsuite/btcutil" +) + +// AgentConstraints is an interface the agent will query to determine what +// limits it will need to stay inside when opening channels. +type AgentConstraints interface { + // ChannelBudget should, given the passed parameters, return whether + // more channels can be be opened while still staying withing the set + // constraints. If the constraints allow us to open more channels, then + // the first return value will represent the amount of additional funds + // available towards creating channels. The second return value is the + // exact *number* of additional channels available. + ChannelBudget(chans []Channel, balance btcutil.Amount) ( + btcutil.Amount, uint32) + + // MaxPendingOpens returns the maximum number of pending channel + // establishment goroutines that can be lingering. We cap this value in + // order to control the level of parallelism caused by the autopilot + // agent. + MaxPendingOpens() uint16 + + // MinChanSize returns the smallest channel that the autopilot agent + // should create. + MinChanSize() btcutil.Amount + + // MaxChanSize returns largest channel that the autopilot agent should + // create. + MaxChanSize() btcutil.Amount +} + +// agenConstraints is an implementation of the AgentConstraints interface that +// indicate the constraints the autopilot agent must adhere to when opening +// channels. +type agentConstraints struct { + // minChanSize is the smallest channel that the autopilot agent should + // create. + minChanSize btcutil.Amount + + // maxChanSize the largest channel that the autopilot agent should + // create. + maxChanSize btcutil.Amount + + // chanLimit the maximum number of channels that should be created. + chanLimit uint16 + + // allocation the percentage of total funds that should be committed to + // automatic channel establishment. + allocation float64 + + // maxPendingOpens is the maximum number of pending channel + // establishment goroutines that can be lingering. We cap this value in + // order to control the level of parallelism caused by the autopilot + // agent. + maxPendingOpens uint16 +} + +// A compile time assertion to ensure agentConstraints satisfies the +// AgentConstraints interface. +var _ AgentConstraints = (*agentConstraints)(nil) + +// NewConstraints returns a new AgentConstraints with the given limits. +func NewConstraints(minChanSize, maxChanSize btcutil.Amount, chanLimit, + maxPendingOpens uint16, allocation float64) AgentConstraints { + + return &agentConstraints{ + minChanSize: minChanSize, + maxChanSize: maxChanSize, + chanLimit: chanLimit, + allocation: allocation, + maxPendingOpens: maxPendingOpens, + } +} + +// ChannelBudget should, given the passed parameters, return whether more +// channels can be be opened while still staying withing the set constraints. +// If the constraints allow us to open more channels, then the first return +// value will represent the amount of additional funds available towards +// creating channels. The second return value is the exact *number* of +// additional channels available. +// +// Note: part of the AgentConstraints interface. +func (h *agentConstraints) ChannelBudget(channels []Channel, + funds btcutil.Amount) (btcutil.Amount, uint32) { + + // If we're already over our maximum allowed number of channels, then + // we'll instruct the controller not to create any more channels. + if len(channels) >= int(h.chanLimit) { + return 0, 0 + } + + // The number of additional channels that should be opened is the + // difference between the channel limit, and the number of channels we + // already have open. + numAdditionalChans := uint32(h.chanLimit) - uint32(len(channels)) + + // First, we'll tally up the total amount of funds that are currently + // present within the set of active channels. + var totalChanAllocation btcutil.Amount + for _, channel := range channels { + totalChanAllocation += channel.Capacity + } + + // With this value known, we'll now compute the total amount of fund + // allocated across regular utxo's and channel utxo's. + totalFunds := funds + totalChanAllocation + + // Once the total amount has been computed, we then calculate the + // fraction of funds currently allocated to channels. + fundsFraction := float64(totalChanAllocation) / float64(totalFunds) + + // If this fraction is below our threshold, then we'll return true, to + // indicate the controller should call Select to obtain a candidate set + // of channels to attempt to open. + needMore := fundsFraction < h.allocation + if !needMore { + return 0, 0 + } + + // Now that we know we need more funds, we'll compute the amount of + // additional funds we should allocate towards channels. + targetAllocation := btcutil.Amount(float64(totalFunds) * h.allocation) + fundsAvailable := targetAllocation - totalChanAllocation + return fundsAvailable, numAdditionalChans +} + +// MaxPendingOpens returns the maximum number of pending channel establishment +// goroutines that can be lingering. We cap this value in order to control the +// level of parallelism caused by the autopilot agent. +// +// Note: part of the AgentConstraints interface. +func (h *agentConstraints) MaxPendingOpens() uint16 { + return h.maxPendingOpens +} + +// MinChanSize returns the smallest channel that the autopilot agent should +// create. +// +// Note: part of the AgentConstraints interface. +func (h *agentConstraints) MinChanSize() btcutil.Amount { + return h.minChanSize +} + +// MaxChanSize returns largest channel that the autopilot agent should create. +// +// Note: part of the AgentConstraints interface. +func (h *agentConstraints) MaxChanSize() btcutil.Amount { + return h.maxChanSize +} diff --git a/autopilot/agent_test.go b/autopilot/agent_test.go index 1a3c8d759..c975326b6 100644 --- a/autopilot/agent_test.go +++ b/autopilot/agent_test.go @@ -25,6 +25,27 @@ type moreChanArg struct { balance btcutil.Amount } +type mockConstraints struct { +} + +func (m *mockConstraints) ChannelBudget(chans []Channel, + balance btcutil.Amount) (btcutil.Amount, uint32) { + return 1e8, 10 +} + +func (m *mockConstraints) MaxPendingOpens() uint16 { + return 10 +} + +func (m *mockConstraints) MinChanSize() btcutil.Amount { + return 0 +} +func (m *mockConstraints) MaxChanSize() btcutil.Amount { + return 1e8 +} + +var _ AgentConstraints = (*mockConstraints)(nil) + type mockHeuristic struct { moreChansResps chan moreChansResp moreChanArgs chan moreChanArg @@ -150,6 +171,8 @@ func TestAgentChannelOpenSignal(t *testing.T) { nodeScoresResps: make(chan map[NodeID]*AttachmentDirective), quit: quit, } + constraints := &mockConstraints{} + chanController := &mockChanController{ openChanSignals: make(chan openChanIntent, 10), } @@ -170,10 +193,8 @@ func TestAgentChannelOpenSignal(t *testing.T) { DisconnectPeer: func(*btcec.PublicKey) error { return nil }, - Graph: memGraph, - Constraints: &HeuristicConstraints{ - MaxPendingOpens: 10, - }, + Graph: memGraph, + Constraints: constraints, } initialChans := []Channel{} agent, err := New(testCfg, initialChans) @@ -283,6 +304,8 @@ func TestAgentChannelFailureSignal(t *testing.T) { nodeScoresResps: make(chan map[NodeID]*AttachmentDirective), quit: quit, } + constraints := &mockConstraints{} + chanController := &mockFailingChanController{} memGraph, _, _ := newMemChanGraph() @@ -301,10 +324,8 @@ func TestAgentChannelFailureSignal(t *testing.T) { DisconnectPeer: func(*btcec.PublicKey) error { return nil }, - Graph: memGraph, - Constraints: &HeuristicConstraints{ - MaxPendingOpens: 10, - }, + Graph: memGraph, + Constraints: constraints, } initialChans := []Channel{} @@ -394,6 +415,8 @@ func TestAgentChannelCloseSignal(t *testing.T) { nodeScoresResps: make(chan map[NodeID]*AttachmentDirective), quit: quit, } + constraints := &mockConstraints{} + chanController := &mockChanController{ openChanSignals: make(chan openChanIntent), } @@ -414,10 +437,8 @@ func TestAgentChannelCloseSignal(t *testing.T) { DisconnectPeer: func(*btcec.PublicKey) error { return nil }, - Graph: memGraph, - Constraints: &HeuristicConstraints{ - MaxPendingOpens: 10, - }, + Graph: memGraph, + Constraints: constraints, } // We'll start the agent with two channels already being active. @@ -512,6 +533,8 @@ func TestAgentBalanceUpdate(t *testing.T) { nodeScoresResps: make(chan map[NodeID]*AttachmentDirective), quit: quit, } + constraints := &mockConstraints{} + chanController := &mockChanController{ openChanSignals: make(chan openChanIntent), } @@ -538,10 +561,8 @@ func TestAgentBalanceUpdate(t *testing.T) { DisconnectPeer: func(*btcec.PublicKey) error { return nil }, - Graph: memGraph, - Constraints: &HeuristicConstraints{ - MaxPendingOpens: 10, - }, + Graph: memGraph, + Constraints: constraints, } initialChans := []Channel{} agent, err := New(testCfg, initialChans) @@ -630,6 +651,8 @@ func TestAgentImmediateAttach(t *testing.T) { nodeScoresResps: make(chan map[NodeID]*AttachmentDirective), quit: quit, } + constraints := &mockConstraints{} + chanController := &mockChanController{ openChanSignals: make(chan openChanIntent), } @@ -653,10 +676,8 @@ func TestAgentImmediateAttach(t *testing.T) { DisconnectPeer: func(*btcec.PublicKey) error { return nil }, - Graph: memGraph, - Constraints: &HeuristicConstraints{ - MaxPendingOpens: 10, - }, + Graph: memGraph, + Constraints: constraints, } initialChans := []Channel{} agent, err := New(testCfg, initialChans) @@ -773,6 +794,8 @@ func TestAgentPrivateChannels(t *testing.T) { nodeScoresResps: make(chan map[NodeID]*AttachmentDirective), quit: quit, } + constraints := &mockConstraints{} + // The chanController should be initialized such that all of its open // channel requests are for private channels. chanController := &mockChanController{ @@ -799,10 +822,8 @@ func TestAgentPrivateChannels(t *testing.T) { DisconnectPeer: func(*btcec.PublicKey) error { return nil }, - Graph: memGraph, - Constraints: &HeuristicConstraints{ - MaxPendingOpens: 10, - }, + Graph: memGraph, + Constraints: constraints, } agent, err := New(cfg, nil) if err != nil { @@ -905,6 +926,8 @@ func TestAgentPendingChannelState(t *testing.T) { nodeScoresResps: make(chan map[NodeID]*AttachmentDirective), quit: quit, } + constraints := &mockConstraints{} + chanController := &mockChanController{ openChanSignals: make(chan openChanIntent), } @@ -932,10 +955,8 @@ func TestAgentPendingChannelState(t *testing.T) { DisconnectPeer: func(*btcec.PublicKey) error { return nil }, - Graph: memGraph, - Constraints: &HeuristicConstraints{ - MaxPendingOpens: 10, - }, + Graph: memGraph, + Constraints: constraints, } initialChans := []Channel{} agent, err := New(testCfg, initialChans) @@ -1097,6 +1118,8 @@ func TestAgentPendingOpenChannel(t *testing.T) { nodeScoresResps: make(chan map[NodeID]*AttachmentDirective), quit: quit, } + constraints := &mockConstraints{} + chanController := &mockChanController{ openChanSignals: make(chan openChanIntent), } @@ -1114,10 +1137,8 @@ func TestAgentPendingOpenChannel(t *testing.T) { WalletBalance: func() (btcutil.Amount, error) { return walletBalance, nil }, - Graph: memGraph, - Constraints: &HeuristicConstraints{ - MaxPendingOpens: 10, - }, + Graph: memGraph, + Constraints: constraints, } agent, err := New(cfg, nil) if err != nil { @@ -1190,6 +1211,8 @@ func TestAgentOnNodeUpdates(t *testing.T) { nodeScoresResps: make(chan map[NodeID]*AttachmentDirective), quit: quit, } + constraints := &mockConstraints{} + chanController := &mockChanController{ openChanSignals: make(chan openChanIntent), } @@ -1207,10 +1230,8 @@ func TestAgentOnNodeUpdates(t *testing.T) { WalletBalance: func() (btcutil.Amount, error) { return walletBalance, nil }, - Graph: memGraph, - Constraints: &HeuristicConstraints{ - MaxPendingOpens: 10, - }, + Graph: memGraph, + Constraints: constraints, } agent, err := New(cfg, nil) if err != nil { @@ -1303,6 +1324,8 @@ func TestAgentSkipPendingConns(t *testing.T) { nodeScoresResps: make(chan map[NodeID]*AttachmentDirective), quit: quit, } + constraints := &mockConstraints{} + chanController := &mockChanController{ openChanSignals: make(chan openChanIntent), } @@ -1341,10 +1364,8 @@ func TestAgentSkipPendingConns(t *testing.T) { DisconnectPeer: func(*btcec.PublicKey) error { return nil }, - Graph: memGraph, - Constraints: &HeuristicConstraints{ - MaxPendingOpens: 10, - }, + Graph: memGraph, + Constraints: constraints, } initialChans := []Channel{} agent, err := New(testCfg, initialChans) diff --git a/autopilot/heuristic_constraints.go b/autopilot/heuristic_constraints.go deleted file mode 100644 index 916f85071..000000000 --- a/autopilot/heuristic_constraints.go +++ /dev/null @@ -1,76 +0,0 @@ -package autopilot - -import ( - "github.com/btcsuite/btcutil" -) - -// HeuristicConstraints is a struct that indicate the constraints an autopilot -// heuristic must adhere to when opening channels. -type HeuristicConstraints struct { - // MinChanSize is the smallest channel that the autopilot agent should - // create. - MinChanSize btcutil.Amount - - // MaxChanSize the largest channel that the autopilot agent should - // create. - MaxChanSize btcutil.Amount - - // ChanLimit the maximum number of channels that should be created. - ChanLimit uint16 - - // Allocation the percentage of total funds that should be committed to - // automatic channel establishment. - Allocation float64 - - // MaxPendingOpens is the maximum number of pending channel - // establishment goroutines that can be lingering. We cap this value in - // order to control the level of parallelism caused by the autopilot - // agent. - MaxPendingOpens uint16 -} - -// availableChans returns the funds and number of channels slots the autopilot -// has available towards new channels, and still be within the set constraints. -func (h *HeuristicConstraints) availableChans(channels []Channel, - funds btcutil.Amount) (btcutil.Amount, uint32) { - - // If we're already over our maximum allowed number of channels, then - // we'll instruct the controller not to create any more channels. - if len(channels) >= int(h.ChanLimit) { - return 0, 0 - } - - // The number of additional channels that should be opened is the - // difference between the channel limit, and the number of channels we - // already have open. - numAdditionalChans := uint32(h.ChanLimit) - uint32(len(channels)) - - // First, we'll tally up the total amount of funds that are currently - // present within the set of active channels. - var totalChanAllocation btcutil.Amount - for _, channel := range channels { - totalChanAllocation += channel.Capacity - } - - // With this value known, we'll now compute the total amount of fund - // allocated across regular utxo's and channel utxo's. - totalFunds := funds + totalChanAllocation - - // Once the total amount has been computed, we then calculate the - // fraction of funds currently allocated to channels. - fundsFraction := float64(totalChanAllocation) / float64(totalFunds) - - // If this fraction is below our threshold, then we'll return true, to - // indicate the controller should call Select to obtain a candidate set - // of channels to attempt to open. - needMore := fundsFraction < h.Allocation - if !needMore { - return 0, 0 - } - - // Now that we know we need more funds, we'll compute the amount of - // additional funds we should allocate towards channels. - targetAllocation := btcutil.Amount(float64(totalFunds) * h.Allocation) - fundsAvailable := targetAllocation - totalChanAllocation - return fundsAvailable, numAdditionalChans -} diff --git a/autopilot/prefattach.go b/autopilot/prefattach.go index 656763072..3b5ee9a74 100644 --- a/autopilot/prefattach.go +++ b/autopilot/prefattach.go @@ -21,7 +21,7 @@ import ( // // TODO(roasbeef): BA, with k=-3 type ConstrainedPrefAttachment struct { - constraints *HeuristicConstraints + constraints AgentConstraints } // NewConstrainedPrefAttachment creates a new instance of a @@ -29,7 +29,7 @@ type ConstrainedPrefAttachment struct { // and an allocation amount which is interpreted as a percentage of funds that // is to be committed to channels at all times. func NewConstrainedPrefAttachment( - cfg *HeuristicConstraints) *ConstrainedPrefAttachment { + cfg AgentConstraints) *ConstrainedPrefAttachment { prand.Seed(time.Now().Unix()) @@ -53,7 +53,7 @@ func (p *ConstrainedPrefAttachment) NeedMoreChans(channels []Channel, funds btcutil.Amount) (btcutil.Amount, uint32, bool) { // We'll try to open more channels as long as the constraints allow it. - availableFunds, availableChans := p.constraints.availableChans( + availableFunds, availableChans := p.constraints.ChannelBudget( channels, funds, ) return availableFunds, availableChans, availableChans > 0 @@ -142,7 +142,7 @@ func (p *ConstrainedPrefAttachment) NodeScores(g ChannelGraph, chans []Channel, candidates := make(map[NodeID]*AttachmentDirective) for nID, nodeChans := range nodeChanNum { // As channel size we'll use the maximum channel size available. - chanSize := p.constraints.MaxChanSize + chanSize := p.constraints.MaxChanSize() if fundsAvailable-chanSize < 0 { chanSize = fundsAvailable } @@ -159,7 +159,7 @@ func (p *ConstrainedPrefAttachment) NodeScores(g ChannelGraph, chans []Channel, // If the amount is too small, we don't want to attempt opening // another channel. - case chanSize == 0 || chanSize < p.constraints.MinChanSize: + case chanSize == 0 || chanSize < p.constraints.MinChanSize(): continue // If the node has no addresses, we cannot connect to it, so we diff --git a/autopilot/prefattach_test.go b/autopilot/prefattach_test.go index f485ce7d5..0fbf68e35 100644 --- a/autopilot/prefattach_test.go +++ b/autopilot/prefattach_test.go @@ -29,12 +29,13 @@ func TestConstrainedPrefAttachmentNeedMoreChan(t *testing.T) { threshold = 0.5 ) - constraints := &HeuristicConstraints{ - MinChanSize: minChanSize, - MaxChanSize: maxChanSize, - ChanLimit: chanLimit, - Allocation: threshold, - } + constraints := NewConstraints( + minChanSize, + maxChanSize, + chanLimit, + 0, + threshold, + ) randChanID := func() lnwire.ShortChannelID { return lnwire.NewShortChanIDFromInt(uint64(prand.Int63())) @@ -242,12 +243,13 @@ func TestConstrainedPrefAttachmentSelectEmptyGraph(t *testing.T) { threshold = 0.5 ) - constraints := &HeuristicConstraints{ - MinChanSize: minChanSize, - MaxChanSize: maxChanSize, - ChanLimit: chanLimit, - Allocation: threshold, - } + constraints := NewConstraints( + minChanSize, + maxChanSize, + chanLimit, + 0, + threshold, + ) prefAttach := NewConstrainedPrefAttachment(constraints) @@ -350,12 +352,14 @@ func TestConstrainedPrefAttachmentSelectTwoVertexes(t *testing.T) { threshold = 0.5 ) - constraints := &HeuristicConstraints{ - MinChanSize: minChanSize, - MaxChanSize: maxChanSize, - ChanLimit: chanLimit, - Allocation: threshold, - } + constraints := NewConstraints( + minChanSize, + maxChanSize, + chanLimit, + 0, + threshold, + ) + for _, graph := range chanGraphs { success := t.Run(graph.name, func(t1 *testing.T) { graph, cleanup, err := graph.genFunc() @@ -474,12 +478,13 @@ func TestConstrainedPrefAttachmentSelectInsufficientFunds(t *testing.T) { threshold = 0.5 ) - constraints := &HeuristicConstraints{ - MinChanSize: minChanSize, - MaxChanSize: maxChanSize, - ChanLimit: chanLimit, - Allocation: threshold, - } + constraints := NewConstraints( + minChanSize, + maxChanSize, + chanLimit, + 0, + threshold, + ) for _, graph := range chanGraphs { success := t.Run(graph.name, func(t1 *testing.T) { @@ -544,12 +549,13 @@ func TestConstrainedPrefAttachmentSelectGreedyAllocation(t *testing.T) { threshold = 0.5 ) - constraints := &HeuristicConstraints{ - MinChanSize: minChanSize, - MaxChanSize: maxChanSize, - ChanLimit: chanLimit, - Allocation: threshold, - } + constraints := NewConstraints( + minChanSize, + maxChanSize, + chanLimit, + 0, + threshold, + ) for _, graph := range chanGraphs { success := t.Run(graph.name, func(t1 *testing.T) { @@ -711,12 +717,13 @@ func TestConstrainedPrefAttachmentSelectSkipNodes(t *testing.T) { threshold = 0.5 ) - constraints := &HeuristicConstraints{ - MinChanSize: minChanSize, - MaxChanSize: maxChanSize, - ChanLimit: chanLimit, - Allocation: threshold, - } + constraints := NewConstraints( + minChanSize, + maxChanSize, + chanLimit, + 0, + threshold, + ) for _, graph := range chanGraphs { success := t.Run(graph.name, func(t1 *testing.T) { diff --git a/pilot.go b/pilot.go index 9f3527421..e68a3d2b0 100644 --- a/pilot.go +++ b/pilot.go @@ -87,13 +87,13 @@ func initAutoPilot(svr *server, cfg *autoPilotConfig) *autopilot.ManagerCfg { atplLog.Infof("Instantiating autopilot with cfg: %v", spew.Sdump(cfg)) // Set up the constraints the autopilot heuristics must adhere to. - atplConstraints := &autopilot.HeuristicConstraints{ - MinChanSize: btcutil.Amount(cfg.MinChannelSize), - MaxChanSize: btcutil.Amount(cfg.MaxChannelSize), - ChanLimit: uint16(cfg.MaxChannels), - Allocation: cfg.Allocation, - MaxPendingOpens: 10, - } + atplConstraints := autopilot.NewConstraints( + btcutil.Amount(cfg.MinChannelSize), + btcutil.Amount(cfg.MaxChannelSize), + uint16(cfg.MaxChannels), + 10, + cfg.Allocation, + ) // First, we'll create the preferential attachment heuristic, // initialized with the passed auto pilot configuration parameters. From 0e1713956b1bd7fc80c1aac401569a02a0792768 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 19 Dec 2018 14:54:53 +0100 Subject: [PATCH 02/10] autopilot/agent: call ChannelBudget on constrainsts We let the agent call ChannelBudget on its constraints directly, and not go through the heuristic. This is needed since when we want to have multiple active heuristics concurrently, it won't make sense anymore to ask each of the heuristics. The mockConstraints are also updated to act as the mockHeuristic did before, by making it possible to control the responses it gives by sending them on the contained channels. --- autopilot/agent.go | 9 +- autopilot/agent_test.go | 195 +++++++++++++++++++++------------------- 2 files changed, 108 insertions(+), 96 deletions(-) diff --git a/autopilot/agent.go b/autopilot/agent.go index d384abde1..bc9009946 100644 --- a/autopilot/agent.go +++ b/autopilot/agent.go @@ -478,12 +478,13 @@ func (a *Agent) controller() { a.pendingMtx.Unlock() // Now that we've updated our internal state, we'll consult our - // channel attachment heuristic to determine if we should open - // up any additional channels or modify existing channels. - availableFunds, numChans, needMore := a.cfg.Heuristic.NeedMoreChans( + // channel attachment heuristic to determine if we can open + // up any additional channels while staying within our + // constraints. + availableFunds, numChans := a.cfg.Constraints.ChannelBudget( totalChans, a.totalBalance, ) - if !needMore { + if numChans == 0 { continue } diff --git a/autopilot/agent_test.go b/autopilot/agent_test.go index c975326b6..f3b70edf1 100644 --- a/autopilot/agent_test.go +++ b/autopilot/agent_test.go @@ -15,9 +15,8 @@ import ( ) type moreChansResp struct { - needMore bool - numMore uint32 - amt btcutil.Amount + numMore uint32 + amt btcutil.Amount } type moreChanArg struct { @@ -26,11 +25,33 @@ type moreChanArg struct { } type mockConstraints struct { + moreChansResps chan moreChansResp + moreChanArgs chan moreChanArg + quit chan struct{} } func (m *mockConstraints) ChannelBudget(chans []Channel, balance btcutil.Amount) (btcutil.Amount, uint32) { - return 1e8, 10 + + if m.moreChanArgs != nil { + moreChan := moreChanArg{ + chans: chans, + balance: balance, + } + + select { + case m.moreChanArgs <- moreChan: + case <-m.quit: + return 0, 0 + } + } + + select { + case resp := <-m.moreChansResps: + return resp.amt, resp.numMore + case <-m.quit: + return 0, 0 + } } func (m *mockConstraints) MaxPendingOpens() uint16 { @@ -47,9 +68,6 @@ func (m *mockConstraints) MaxChanSize() btcutil.Amount { var _ AgentConstraints = (*mockConstraints)(nil) type mockHeuristic struct { - moreChansResps chan moreChansResp - moreChanArgs chan moreChanArg - nodeScoresResps chan map[NodeID]*AttachmentDirective nodeScoresArgs chan directiveArg @@ -58,26 +76,7 @@ type mockHeuristic struct { func (m *mockHeuristic) NeedMoreChans(chans []Channel, balance btcutil.Amount) (btcutil.Amount, uint32, bool) { - - if m.moreChanArgs != nil { - moreChan := moreChanArg{ - chans: chans, - balance: balance, - } - - select { - case m.moreChanArgs <- moreChan: - case <-m.quit: - return 0, 0, false - } - } - - select { - case resp := <-m.moreChansResps: - return resp.amt, resp.numMore, resp.needMore - case <-m.quit: - return 0, 0, false - } + return 0, 0, false } type directiveArg struct { @@ -167,11 +166,13 @@ func TestAgentChannelOpenSignal(t *testing.T) { quit := make(chan struct{}) heuristic := &mockHeuristic{ - moreChansResps: make(chan moreChansResp), nodeScoresResps: make(chan map[NodeID]*AttachmentDirective), quit: quit, } - constraints := &mockConstraints{} + constraints := &mockConstraints{ + moreChansResps: make(chan moreChansResp), + quit: quit, + } chanController := &mockChanController{ openChanSignals: make(chan openChanIntent, 10), @@ -221,7 +222,7 @@ func TestAgentChannelOpenSignal(t *testing.T) { // We'll send an initial "no" response to advance the agent past its // initial check. select { - case heuristic.moreChansResps <- moreChansResp{false, 0, 0}: + case constraints.moreChansResps <- moreChansResp{0, 0}: case <-time.After(time.Second * 10): t.Fatalf("heuristic wasn't queried in time") } @@ -237,7 +238,7 @@ func TestAgentChannelOpenSignal(t *testing.T) { // The agent should now query the heuristic in order to determine its // next action as it local state has now been modified. select { - case heuristic.moreChansResps <- moreChansResp{false, 0, 0}: + case constraints.moreChansResps <- moreChansResp{0, 0}: // At this point, the local state of the agent should // have also been updated to reflect that the LN node // now has an additional channel with one BTC. @@ -300,11 +301,13 @@ func TestAgentChannelFailureSignal(t *testing.T) { quit := make(chan struct{}) heuristic := &mockHeuristic{ - moreChansResps: make(chan moreChansResp), nodeScoresResps: make(chan map[NodeID]*AttachmentDirective), quit: quit, } - constraints := &mockConstraints{} + constraints := &mockConstraints{ + moreChansResps: make(chan moreChansResp), + quit: quit, + } chanController := &mockFailingChanController{} memGraph, _, _ := newMemChanGraph() @@ -353,7 +356,7 @@ func TestAgentChannelFailureSignal(t *testing.T) { // First ensure the agent will attempt to open a new channel. Return // that we need more channels, and have 5BTC to use. select { - case heuristic.moreChansResps <- moreChansResp{true, 1, 5 * btcutil.SatoshiPerBitcoin}: + case constraints.moreChansResps <- moreChansResp{1, 5 * btcutil.SatoshiPerBitcoin}: case <-time.After(time.Second * 10): t.Fatal("heuristic wasn't queried in time") } @@ -384,7 +387,7 @@ func TestAgentChannelFailureSignal(t *testing.T) { // Now ensure that the controller loop is re-executed. select { - case heuristic.moreChansResps <- moreChansResp{true, 1, 5 * btcutil.SatoshiPerBitcoin}: + case constraints.moreChansResps <- moreChansResp{1, 5 * btcutil.SatoshiPerBitcoin}: case <-time.After(time.Second * 10): t.Fatal("heuristic wasn't queried in time") } @@ -411,11 +414,13 @@ func TestAgentChannelCloseSignal(t *testing.T) { quit := make(chan struct{}) heuristic := &mockHeuristic{ - moreChansResps: make(chan moreChansResp), nodeScoresResps: make(chan map[NodeID]*AttachmentDirective), quit: quit, } - constraints := &mockConstraints{} + constraints := &mockConstraints{ + moreChansResps: make(chan moreChansResp), + quit: quit, + } chanController := &mockChanController{ openChanSignals: make(chan openChanIntent), @@ -476,7 +481,7 @@ func TestAgentChannelCloseSignal(t *testing.T) { // We'll send an initial "no" response to advance the agent past its // initial check. select { - case heuristic.moreChansResps <- moreChansResp{false, 0, 0}: + case constraints.moreChansResps <- moreChansResp{0, 0}: case <-time.After(time.Second * 10): t.Fatalf("heuristic wasn't queried in time") } @@ -488,7 +493,7 @@ func TestAgentChannelCloseSignal(t *testing.T) { // The agent should now query the heuristic in order to determine its // next action as it local state has now been modified. select { - case heuristic.moreChansResps <- moreChansResp{false, 0, 0}: + case constraints.moreChansResps <- moreChansResp{0, 0}: // At this point, the local state of the agent should // have also been updated to reflect that the LN node // has no existing open channels. @@ -529,11 +534,13 @@ func TestAgentBalanceUpdate(t *testing.T) { quit := make(chan struct{}) heuristic := &mockHeuristic{ - moreChansResps: make(chan moreChansResp), nodeScoresResps: make(chan map[NodeID]*AttachmentDirective), quit: quit, } - constraints := &mockConstraints{} + constraints := &mockConstraints{ + moreChansResps: make(chan moreChansResp), + quit: quit, + } chanController := &mockChanController{ openChanSignals: make(chan openChanIntent), @@ -589,7 +596,7 @@ func TestAgentBalanceUpdate(t *testing.T) { // We'll send an initial "no" response to advance the agent past its // initial check. select { - case heuristic.moreChansResps <- moreChansResp{false, 0, 0}: + case constraints.moreChansResps <- moreChansResp{0, 0}: case <-time.After(time.Second * 10): t.Fatalf("heuristic wasn't queried in time") } @@ -605,7 +612,7 @@ func TestAgentBalanceUpdate(t *testing.T) { // The agent should now query the heuristic in order to determine its // next action as it local state has now been modified. select { - case heuristic.moreChansResps <- moreChansResp{false, 0, 0}: + case constraints.moreChansResps <- moreChansResp{0, 0}: // At this point, the local state of the agent should // have also been updated to reflect that the LN node // now has an additional 5BTC available. @@ -647,11 +654,13 @@ func TestAgentImmediateAttach(t *testing.T) { quit := make(chan struct{}) heuristic := &mockHeuristic{ - moreChansResps: make(chan moreChansResp), nodeScoresResps: make(chan map[NodeID]*AttachmentDirective), quit: quit, } - constraints := &mockConstraints{} + constraints := &mockConstraints{ + moreChansResps: make(chan moreChansResp), + quit: quit, + } chanController := &mockChanController{ openChanSignals: make(chan openChanIntent), @@ -711,10 +720,9 @@ func TestAgentImmediateAttach(t *testing.T) { // We'll send over a response indicating that it should // establish more channels, and give it a budget of 5 BTC to do // so. - case heuristic.moreChansResps <- moreChansResp{ - needMore: true, - numMore: numChans, - amt: 5 * btcutil.SatoshiPerBitcoin, + case constraints.moreChansResps <- moreChansResp{ + numMore: numChans, + amt: 5 * btcutil.SatoshiPerBitcoin, }: case <-time.After(time.Second * 10): t.Fatalf("heuristic wasn't queried in time") @@ -790,11 +798,13 @@ func TestAgentPrivateChannels(t *testing.T) { quit := make(chan struct{}) heuristic := &mockHeuristic{ - moreChansResps: make(chan moreChansResp), nodeScoresResps: make(chan map[NodeID]*AttachmentDirective), quit: quit, } - constraints := &mockConstraints{} + constraints := &mockConstraints{ + moreChansResps: make(chan moreChansResp), + quit: quit, + } // The chanController should be initialized such that all of its open // channel requests are for private channels. @@ -854,12 +864,11 @@ func TestAgentPrivateChannels(t *testing.T) { // indicating that it should establish more channels, and give it a // budget of 5 BTC to do so. resp := moreChansResp{ - needMore: true, - numMore: numChans, - amt: 5 * btcutil.SatoshiPerBitcoin, + numMore: numChans, + amt: 5 * btcutil.SatoshiPerBitcoin, } select { - case heuristic.moreChansResps <- resp: + case constraints.moreChansResps <- resp: case <-time.After(time.Second * 10): t.Fatalf("heuristic wasn't queried in time") } @@ -922,11 +931,13 @@ func TestAgentPendingChannelState(t *testing.T) { quit := make(chan struct{}) heuristic := &mockHeuristic{ - moreChansResps: make(chan moreChansResp), nodeScoresResps: make(chan map[NodeID]*AttachmentDirective), quit: quit, } - constraints := &mockConstraints{} + constraints := &mockConstraints{ + moreChansResps: make(chan moreChansResp), + quit: quit, + } chanController := &mockChanController{ openChanSignals: make(chan openChanIntent), @@ -985,16 +996,15 @@ func TestAgentPendingChannelState(t *testing.T) { // attachment. We'll send over a response indicating that it should // establish more channels, and give it a budget of 1 BTC to do so. select { - case heuristic.moreChansResps <- moreChansResp{ - needMore: true, - numMore: 1, - amt: btcutil.SatoshiPerBitcoin, + case constraints.moreChansResps <- moreChansResp{ + numMore: 1, + amt: btcutil.SatoshiPerBitcoin, }: case <-time.After(time.Second * 10): t.Fatalf("heuristic wasn't queried in time") } - heuristic.moreChanArgs = make(chan moreChanArg) + 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 @@ -1057,7 +1067,7 @@ func TestAgentPendingChannelState(t *testing.T) { // The request that we get should include a pending channel for the // one that we just created, otherwise the agent isn't properly // updating its internal state. - case req := <-heuristic.moreChanArgs: + case req := <-constraints.moreChanArgs: if len(req.chans) != 1 { t.Fatalf("should include pending chan in current "+ "state, instead have %v chans", len(req.chans)) @@ -1077,7 +1087,7 @@ func TestAgentPendingChannelState(t *testing.T) { // We'll send across a response indicating that it *does* need more // channels. select { - case heuristic.moreChansResps <- moreChansResp{true, 1, btcutil.SatoshiPerBitcoin}: + case constraints.moreChansResps <- moreChansResp{1, btcutil.SatoshiPerBitcoin}: case <-time.After(time.Second * 10): t.Fatalf("need more chans wasn't queried in time") } @@ -1114,11 +1124,13 @@ func TestAgentPendingOpenChannel(t *testing.T) { quit := make(chan struct{}) heuristic := &mockHeuristic{ - moreChansResps: make(chan moreChansResp), nodeScoresResps: make(chan map[NodeID]*AttachmentDirective), quit: quit, } - constraints := &mockConstraints{} + constraints := &mockConstraints{ + moreChansResps: make(chan moreChansResp), + quit: quit, + } chanController := &mockChanController{ openChanSignals: make(chan openChanIntent), @@ -1164,7 +1176,7 @@ func TestAgentPendingOpenChannel(t *testing.T) { // We'll send an initial "no" response to advance the agent past its // initial check. select { - case heuristic.moreChansResps <- moreChansResp{false, 0, 0}: + case constraints.moreChansResps <- moreChansResp{0, 0}: case <-time.After(time.Second * 10): t.Fatalf("heuristic wasn't queried in time") } @@ -1176,7 +1188,7 @@ func TestAgentPendingOpenChannel(t *testing.T) { // The agent should now query the heuristic in order to determine its // next action as its local state has now been modified. select { - case heuristic.moreChansResps <- moreChansResp{false, 0, 0}: + case constraints.moreChansResps <- moreChansResp{0, 0}: case <-time.After(time.Second * 10): t.Fatalf("heuristic wasn't queried in time") } @@ -1207,11 +1219,13 @@ func TestAgentOnNodeUpdates(t *testing.T) { quit := make(chan struct{}) heuristic := &mockHeuristic{ - moreChansResps: make(chan moreChansResp), nodeScoresResps: make(chan map[NodeID]*AttachmentDirective), quit: quit, } - constraints := &mockConstraints{} + constraints := &mockConstraints{ + moreChansResps: make(chan moreChansResp), + quit: quit, + } chanController := &mockChanController{ openChanSignals: make(chan openChanIntent), @@ -1258,10 +1272,9 @@ func TestAgentOnNodeUpdates(t *testing.T) { // initial check. This will cause it to try to get directives from an // empty graph. select { - case heuristic.moreChansResps <- moreChansResp{ - needMore: true, - numMore: 2, - amt: walletBalance, + case constraints.moreChansResps <- moreChansResp{ + numMore: 2, + amt: walletBalance, }: case <-time.After(time.Second * 10): t.Fatalf("heuristic wasn't queried in time") @@ -1283,10 +1296,9 @@ func TestAgentOnNodeUpdates(t *testing.T) { // channels. Since we haven't done anything, we will send the same // response as before since we are still trying to open channels. select { - case heuristic.moreChansResps <- moreChansResp{ - needMore: true, - numMore: 2, - amt: walletBalance, + case constraints.moreChansResps <- moreChansResp{ + numMore: 2, + amt: walletBalance, }: case <-time.After(time.Second * 10): t.Fatalf("heuristic wasn't queried in time") @@ -1320,11 +1332,13 @@ func TestAgentSkipPendingConns(t *testing.T) { quit := make(chan struct{}) heuristic := &mockHeuristic{ - moreChansResps: make(chan moreChansResp), nodeScoresResps: make(chan map[NodeID]*AttachmentDirective), quit: quit, } - constraints := &mockConstraints{} + constraints := &mockConstraints{ + moreChansResps: make(chan moreChansResp), + quit: quit, + } chanController := &mockChanController{ openChanSignals: make(chan openChanIntent), @@ -1393,10 +1407,9 @@ func TestAgentSkipPendingConns(t *testing.T) { // initial check. This will cause it to try to get directives from the // graph. select { - case heuristic.moreChansResps <- moreChansResp{ - needMore: true, - numMore: 1, - amt: walletBalance, + case constraints.moreChansResps <- moreChansResp{ + numMore: 1, + amt: walletBalance, }: case <-time.After(time.Second * 10): t.Fatalf("heuristic wasn't queried in time") @@ -1440,10 +1453,9 @@ func TestAgentSkipPendingConns(t *testing.T) { // The heuristic again informs the agent that we need more channels. select { - case heuristic.moreChansResps <- moreChansResp{ - needMore: true, - numMore: 1, - amt: walletBalance, + case constraints.moreChansResps <- moreChansResp{ + numMore: 1, + amt: walletBalance, }: case <-time.After(time.Second * 10): t.Fatalf("heuristic wasn't queried in time") @@ -1477,10 +1489,9 @@ func TestAgentSkipPendingConns(t *testing.T) { // The agent will now retry since the last connection attempt failed. // The heuristic again informs the agent that we need more channels. select { - case heuristic.moreChansResps <- moreChansResp{ - needMore: true, - numMore: 1, - amt: walletBalance, + case constraints.moreChansResps <- moreChansResp{ + numMore: 1, + amt: walletBalance, }: case <-time.After(time.Second * 10): t.Fatalf("heuristic wasn't queried in time") From fbfc9a53af111c86017600f63cc011f886b35f9c Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 19 Dec 2018 14:54:53 +0100 Subject: [PATCH 03/10] autopilot: TestConstrainedPrefAttachmentNeedMoreChan->TestConstraintsChannelBudget Since the constraints are no longer something the heuristic needs to be aware of, we move the test. --- autopilot/agent_constraints_test.go | 166 ++++++++++++++++++++++++++++ autopilot/prefattach_test.go | 162 --------------------------- 2 files changed, 166 insertions(+), 162 deletions(-) create mode 100644 autopilot/agent_constraints_test.go diff --git a/autopilot/agent_constraints_test.go b/autopilot/agent_constraints_test.go new file mode 100644 index 000000000..4ce78245f --- /dev/null +++ b/autopilot/agent_constraints_test.go @@ -0,0 +1,166 @@ +package autopilot + +import ( + "testing" + "time" + + prand "math/rand" + + "github.com/btcsuite/btcutil" + "github.com/lightningnetwork/lnd/lnwire" +) + +func TestConstraintsChannelBudget(t *testing.T) { + t.Parallel() + + prand.Seed(time.Now().Unix()) + + const ( + minChanSize = 0 + maxChanSize = btcutil.Amount(btcutil.SatoshiPerBitcoin) + + chanLimit = 3 + + threshold = 0.5 + ) + + constraints := NewConstraints( + minChanSize, + maxChanSize, + chanLimit, + 0, + threshold, + ) + + randChanID := func() lnwire.ShortChannelID { + return lnwire.NewShortChanIDFromInt(uint64(prand.Int63())) + } + + testCases := []struct { + channels []Channel + walletAmt btcutil.Amount + + needMore bool + amtAvailable btcutil.Amount + numMore uint32 + }{ + // Many available funds, but already have too many active open + // channels. + { + []Channel{ + { + ChanID: randChanID(), + Capacity: btcutil.Amount(prand.Int31()), + }, + { + ChanID: randChanID(), + Capacity: btcutil.Amount(prand.Int31()), + }, + { + ChanID: randChanID(), + Capacity: btcutil.Amount(prand.Int31()), + }, + }, + btcutil.Amount(btcutil.SatoshiPerBitcoin * 10), + false, + 0, + 0, + }, + + // Ratio of funds in channels and total funds meets the + // threshold. + { + []Channel{ + { + ChanID: randChanID(), + Capacity: btcutil.Amount(btcutil.SatoshiPerBitcoin), + }, + { + ChanID: randChanID(), + Capacity: btcutil.Amount(btcutil.SatoshiPerBitcoin), + }, + }, + btcutil.Amount(btcutil.SatoshiPerBitcoin * 2), + false, + 0, + 0, + }, + + // Ratio of funds in channels and total funds is below the + // threshold. We have 10 BTC allocated amongst channels and + // funds, atm. We're targeting 50%, so 5 BTC should be + // allocated. Only 1 BTC is atm, so 4 BTC should be + // recommended. We should also request 2 more channels as the + // limit is 3. + { + []Channel{ + { + ChanID: randChanID(), + Capacity: btcutil.Amount(btcutil.SatoshiPerBitcoin), + }, + }, + btcutil.Amount(btcutil.SatoshiPerBitcoin * 9), + true, + btcutil.Amount(btcutil.SatoshiPerBitcoin * 4), + 2, + }, + + // Ratio of funds in channels and total funds is below the + // threshold. We have 14 BTC total amongst the wallet's + // balance, and our currently opened channels. Since we're + // targeting a 50% allocation, we should commit 7 BTC. The + // current channels commit 4 BTC, so we should expected 3 BTC + // to be committed. We should only request a single additional + // channel as the limit is 3. + { + []Channel{ + { + ChanID: randChanID(), + Capacity: btcutil.Amount(btcutil.SatoshiPerBitcoin), + }, + { + ChanID: randChanID(), + Capacity: btcutil.Amount(btcutil.SatoshiPerBitcoin * 3), + }, + }, + btcutil.Amount(btcutil.SatoshiPerBitcoin * 10), + true, + btcutil.Amount(btcutil.SatoshiPerBitcoin * 3), + 1, + }, + + // Ratio of funds in channels and total funds is above the + // threshold. + { + []Channel{ + { + ChanID: randChanID(), + Capacity: btcutil.Amount(btcutil.SatoshiPerBitcoin), + }, + { + ChanID: randChanID(), + Capacity: btcutil.Amount(btcutil.SatoshiPerBitcoin), + }, + }, + btcutil.Amount(btcutil.SatoshiPerBitcoin), + false, + 0, + 0, + }, + } + + for i, testCase := range testCases { + amtToAllocate, numMore := constraints.ChannelBudget( + testCase.channels, testCase.walletAmt, + ) + + if amtToAllocate != testCase.amtAvailable { + t.Fatalf("test #%v: expected %v, got %v", + i, testCase.amtAvailable, amtToAllocate) + } + if numMore != testCase.numMore { + t.Fatalf("test #%v: expected %v, got %v", + i, testCase.numMore, numMore) + } + } +} diff --git a/autopilot/prefattach_test.go b/autopilot/prefattach_test.go index 0fbf68e35..94d021238 100644 --- a/autopilot/prefattach_test.go +++ b/autopilot/prefattach_test.go @@ -12,170 +12,8 @@ import ( "github.com/btcsuite/btcd/btcec" "github.com/btcsuite/btcutil" "github.com/lightningnetwork/lnd/channeldb" - "github.com/lightningnetwork/lnd/lnwire" ) -func TestConstrainedPrefAttachmentNeedMoreChan(t *testing.T) { - t.Parallel() - - prand.Seed(time.Now().Unix()) - - const ( - minChanSize = 0 - maxChanSize = btcutil.Amount(btcutil.SatoshiPerBitcoin) - - chanLimit = 3 - - threshold = 0.5 - ) - - constraints := NewConstraints( - minChanSize, - maxChanSize, - chanLimit, - 0, - threshold, - ) - - randChanID := func() lnwire.ShortChannelID { - return lnwire.NewShortChanIDFromInt(uint64(prand.Int63())) - } - - testCases := []struct { - channels []Channel - walletAmt btcutil.Amount - - needMore bool - amtAvailable btcutil.Amount - numMore uint32 - }{ - // Many available funds, but already have too many active open - // channels. - { - []Channel{ - { - ChanID: randChanID(), - Capacity: btcutil.Amount(prand.Int31()), - }, - { - ChanID: randChanID(), - Capacity: btcutil.Amount(prand.Int31()), - }, - { - ChanID: randChanID(), - Capacity: btcutil.Amount(prand.Int31()), - }, - }, - btcutil.Amount(btcutil.SatoshiPerBitcoin * 10), - false, - 0, - 0, - }, - - // Ratio of funds in channels and total funds meets the - // threshold. - { - []Channel{ - { - ChanID: randChanID(), - Capacity: btcutil.Amount(btcutil.SatoshiPerBitcoin), - }, - { - ChanID: randChanID(), - Capacity: btcutil.Amount(btcutil.SatoshiPerBitcoin), - }, - }, - btcutil.Amount(btcutil.SatoshiPerBitcoin * 2), - false, - 0, - 0, - }, - - // Ratio of funds in channels and total funds is below the - // threshold. We have 10 BTC allocated amongst channels and - // funds, atm. We're targeting 50%, so 5 BTC should be - // allocated. Only 1 BTC is atm, so 4 BTC should be - // recommended. We should also request 2 more channels as the - // limit is 3. - { - []Channel{ - { - ChanID: randChanID(), - Capacity: btcutil.Amount(btcutil.SatoshiPerBitcoin), - }, - }, - btcutil.Amount(btcutil.SatoshiPerBitcoin * 9), - true, - btcutil.Amount(btcutil.SatoshiPerBitcoin * 4), - 2, - }, - - // Ratio of funds in channels and total funds is below the - // threshold. We have 14 BTC total amongst the wallet's - // balance, and our currently opened channels. Since we're - // targeting a 50% allocation, we should commit 7 BTC. The - // current channels commit 4 BTC, so we should expected 3 BTC - // to be committed. We should only request a single additional - // channel as the limit is 3. - { - []Channel{ - { - ChanID: randChanID(), - Capacity: btcutil.Amount(btcutil.SatoshiPerBitcoin), - }, - { - ChanID: randChanID(), - Capacity: btcutil.Amount(btcutil.SatoshiPerBitcoin * 3), - }, - }, - btcutil.Amount(btcutil.SatoshiPerBitcoin * 10), - true, - btcutil.Amount(btcutil.SatoshiPerBitcoin * 3), - 1, - }, - - // Ratio of funds in channels and total funds is above the - // threshold. - { - []Channel{ - { - ChanID: randChanID(), - Capacity: btcutil.Amount(btcutil.SatoshiPerBitcoin), - }, - { - ChanID: randChanID(), - Capacity: btcutil.Amount(btcutil.SatoshiPerBitcoin), - }, - }, - btcutil.Amount(btcutil.SatoshiPerBitcoin), - false, - 0, - 0, - }, - } - - prefAttach := NewConstrainedPrefAttachment(constraints) - - for i, testCase := range testCases { - amtToAllocate, numMore, needMore := prefAttach.NeedMoreChans( - testCase.channels, testCase.walletAmt, - ) - - if amtToAllocate != testCase.amtAvailable { - t.Fatalf("test #%v: expected %v, got %v", - i, testCase.amtAvailable, amtToAllocate) - } - if needMore != testCase.needMore { - t.Fatalf("test #%v: expected %v, got %v", - i, testCase.needMore, needMore) - } - if numMore != testCase.numMore { - t.Fatalf("test #%v: expected %v, got %v", - i, testCase.numMore, numMore) - } - } -} - type genGraphFunc func() (testGraph, func(), error) type testGraph interface { From d0c4e253c6898b3a897666e06794cdbc352c21e1 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 19 Dec 2018 14:54:53 +0100 Subject: [PATCH 04/10] autopilot/interface: remove NeedMoreChans from AttachmentHeuristic Since the agent is now querying the constraints directly, NeedMoreChans is no longer needed on the heuristic. --- autopilot/agent_test.go | 5 ----- autopilot/interface.go | 10 ---------- autopilot/prefattach.go | 17 ----------------- 3 files changed, 32 deletions(-) diff --git a/autopilot/agent_test.go b/autopilot/agent_test.go index f3b70edf1..e7b463063 100644 --- a/autopilot/agent_test.go +++ b/autopilot/agent_test.go @@ -74,11 +74,6 @@ type mockHeuristic struct { quit chan struct{} } -func (m *mockHeuristic) NeedMoreChans(chans []Channel, - balance btcutil.Amount) (btcutil.Amount, uint32, bool) { - return 0, 0, false -} - type directiveArg struct { graph ChannelGraph amt btcutil.Amount diff --git a/autopilot/interface.go b/autopilot/interface.go index e0d0cb90d..a612914a6 100644 --- a/autopilot/interface.go +++ b/autopilot/interface.go @@ -111,16 +111,6 @@ type AttachmentDirective struct { // the interface is to allow an auto-pilot agent to decide if it needs more // channels, and if so, which exact channels should be opened. type AttachmentHeuristic interface { - // NeedMoreChans is a predicate that should return true if, given the - // passed parameters, and its internal state, more channels should be - // opened within the channel graph. If the heuristic decides that we do - // indeed need more channels, then the second argument returned will - // represent the amount of additional funds to be used towards creating - // channels. This method should also return the exact *number* of - // additional channels that are needed in order to converge towards our - // ideal state. - NeedMoreChans(chans []Channel, balance btcutil.Amount) (btcutil.Amount, uint32, bool) - // NodeScores is a method that given the current channel graph, current // set of local channels and funds available, scores the given nodes // according to the preference of opening a channel with them. The diff --git a/autopilot/prefattach.go b/autopilot/prefattach.go index 3b5ee9a74..55587cd0a 100644 --- a/autopilot/prefattach.go +++ b/autopilot/prefattach.go @@ -42,23 +42,6 @@ func NewConstrainedPrefAttachment( // AttachmentHeuristic interface. var _ AttachmentHeuristic = (*ConstrainedPrefAttachment)(nil) -// NeedMoreChans is a predicate that should return true if, given the passed -// parameters, and its internal state, more channels should be opened within -// the channel graph. If the heuristic decides that we do indeed need more -// channels, then the second argument returned will represent the amount of -// additional funds to be used towards creating channels. -// -// NOTE: This is a part of the AttachmentHeuristic interface. -func (p *ConstrainedPrefAttachment) NeedMoreChans(channels []Channel, - funds btcutil.Amount) (btcutil.Amount, uint32, bool) { - - // We'll try to open more channels as long as the constraints allow it. - availableFunds, availableChans := p.constraints.ChannelBudget( - channels, funds, - ) - return availableFunds, availableChans, availableChans > 0 -} - // NodeID is a simple type that holds an EC public key serialized in compressed // format. type NodeID [33]byte From d5f3714f862c0f827d67f7631aedd113cd5d5392 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 19 Dec 2018 14:54:53 +0100 Subject: [PATCH 05/10] autopilot/agent: return early if no funds available If there are less funds available than the minumum channel size, there's no reason to score candidates, so we continue early. --- autopilot/agent.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/autopilot/agent.go b/autopilot/agent.go index bc9009946..86ce85e11 100644 --- a/autopilot/agent.go +++ b/autopilot/agent.go @@ -484,7 +484,15 @@ func (a *Agent) controller() { availableFunds, numChans := a.cfg.Constraints.ChannelBudget( totalChans, a.totalBalance, ) - if numChans == 0 { + switch { + case numChans == 0: + continue + + // If the amount is too small, we don't want to attempt opening + // another channel. + case availableFunds == 0: + continue + case availableFunds < a.cfg.Constraints.MinChanSize(): continue } From cfd237bf1f2670dca4daf2f8c0898abbb2d0e1b0 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 19 Dec 2018 14:54:53 +0100 Subject: [PATCH 06/10] autopilot: move determining chanSize from heuristic to agent Since we want to combine scores from multiple heuristics, things get complicated if the heuristics report their own individual channel sizes. Therefore we change the NodeScores interface slightly, letting the agent specify the wanted channel size, and let the heuristic score the nodes accordingly. --- autopilot/agent.go | 8 +++++++- autopilot/interface.go | 12 ++++++------ autopilot/prefattach.go | 18 ++++-------------- autopilot/prefattach_test.go | 21 ++++++++++----------- 4 files changed, 27 insertions(+), 32 deletions(-) diff --git a/autopilot/agent.go b/autopilot/agent.go index 86ce85e11..5a7ca0c01 100644 --- a/autopilot/agent.go +++ b/autopilot/agent.go @@ -547,10 +547,16 @@ func (a *Agent) openChans(availableFunds btcutil.Amount, numChans uint32, return fmt.Errorf("unable to get graph nodes: %v", err) } + // As channel size we'll use the maximum channel size available. + chanSize := a.cfg.Constraints.MaxChanSize() + if availableFunds-chanSize < 0 { + chanSize = availableFunds + } + // Use the heuristic to calculate a score for each node in the // graph. scores, err := a.cfg.Heuristic.NodeScores( - a.cfg.Graph, totalChans, availableFunds, nodes, + a.cfg.Graph, totalChans, chanSize, nodes, ) if err != nil { return fmt.Errorf("unable to calculate node scores : %v", err) diff --git a/autopilot/interface.go b/autopilot/interface.go index a612914a6..cb9ff9abd 100644 --- a/autopilot/interface.go +++ b/autopilot/interface.go @@ -111,11 +111,11 @@ type AttachmentDirective struct { // the interface is to allow an auto-pilot agent to decide if it needs more // channels, and if so, which exact channels should be opened. type AttachmentHeuristic interface { - // NodeScores is a method that given the current channel graph, current - // set of local channels and funds available, scores the given nodes - // according to the preference of opening a channel with them. The - // returned channel candidates maps the NodeID to an attachemnt - // directive containing a score and a channel size. + // NodeScores is a method that given the current channel graph and + // current set of local channels, scores the given nodes according to + // the preference of opening a channel of the given size with them. The + // returned channel candidates maps the NodeID to an attachment + // directive containing a score. // // The scores will be in the range [0, M], where 0 indicates no // improvement in connectivity if a channel is opened to this node, @@ -126,7 +126,7 @@ type AttachmentHeuristic interface { // NOTE: A NodeID not found in the returned map is implicitly given a // score of 0. NodeScores(g ChannelGraph, chans []Channel, - fundsAvailable btcutil.Amount, nodes map[NodeID]struct{}) ( + chanSize btcutil.Amount, nodes map[NodeID]struct{}) ( map[NodeID]*AttachmentDirective, error) } diff --git a/autopilot/prefattach.go b/autopilot/prefattach.go index 55587cd0a..b71ca8f95 100644 --- a/autopilot/prefattach.go +++ b/autopilot/prefattach.go @@ -53,9 +53,9 @@ func NewNodeID(pub *btcec.PublicKey) NodeID { return n } -// NodeScores is a method that given the current channel graph, current set of -// local channels and funds available, scores the given nodes according the the -// preference of opening a channel with them. +// NodeScores is a method that given the current channel graph and +// current set of local channels, scores the given nodes according to +// the preference of opening a channel of the given size with them. // // The heuristic employed by this method is one that attempts to promote a // scale-free network globally, via local attachment preferences for new nodes @@ -71,7 +71,7 @@ func NewNodeID(pub *btcec.PublicKey) NodeID { // // NOTE: This is a part of the AttachmentHeuristic interface. func (p *ConstrainedPrefAttachment) NodeScores(g ChannelGraph, chans []Channel, - fundsAvailable btcutil.Amount, nodes map[NodeID]struct{}) ( + chanSize btcutil.Amount, nodes map[NodeID]struct{}) ( map[NodeID]*AttachmentDirective, error) { // Count the number of channels in the graph. We'll also count the @@ -124,11 +124,6 @@ func (p *ConstrainedPrefAttachment) NodeScores(g ChannelGraph, chans []Channel, // in the graph, and use that as the score. candidates := make(map[NodeID]*AttachmentDirective) for nID, nodeChans := range nodeChanNum { - // As channel size we'll use the maximum channel size available. - chanSize := p.constraints.MaxChanSize() - if fundsAvailable-chanSize < 0 { - chanSize = fundsAvailable - } _, ok := existingPeers[nID] addrs := addresses[nID] @@ -140,11 +135,6 @@ func (p *ConstrainedPrefAttachment) NodeScores(g ChannelGraph, chans []Channel, case ok: continue - // If the amount is too small, we don't want to attempt opening - // another channel. - case chanSize == 0 || chanSize < p.constraints.MinChanSize(): - 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: diff --git a/autopilot/prefattach_test.go b/autopilot/prefattach_test.go index 94d021238..2c5f15148 100644 --- a/autopilot/prefattach_test.go +++ b/autopilot/prefattach_test.go @@ -241,9 +241,8 @@ func TestConstrainedPrefAttachmentSelectTwoVertexes(t *testing.T) { // With the necessary state initialized, we'll now // attempt to get our candidates channel score given // the current state of the graph. - const walletFunds = btcutil.SatoshiPerBitcoin * 10 candidates, err := prefAttach.NodeScores(graph, nil, - walletFunds, nodes) + maxChanSize, nodes) if err != nil { t1.Fatalf("unable to select attachment "+ "directives: %v", err) @@ -351,7 +350,7 @@ func TestConstrainedPrefAttachmentSelectInsufficientFunds(t *testing.T) { // With the necessary state initialized, we'll now // attempt to get the score for our list of nodes, // passing zero for the amount of wallet funds. This - // should return an all-zero score set. + // should return candidates with zero-value channels. scores, err := prefAttach.NodeScores(graph, nil, 0, nodes) if err != nil { @@ -361,9 +360,11 @@ func TestConstrainedPrefAttachmentSelectInsufficientFunds(t *testing.T) { // Since all should be given a score of 0, the map // should be empty. - if len(scores) != 0 { - t1.Fatalf("expected empty score map, "+ - "instead got %v ", len(scores)) + for _, s := range scores { + if s.ChanAmt != 0 { + t1.Fatalf("expected zero channel, "+ + "instead got %v ", s.ChanAmt) + } } }) if !success { @@ -466,9 +467,8 @@ func TestConstrainedPrefAttachmentSelectGreedyAllocation(t *testing.T) { // 50/50 allocation, and have 3 BTC in channels. As a // result, the heuristic should try to greedily // allocate funds to channels. - const availableBalance = btcutil.SatoshiPerBitcoin * 2.5 scores, err := prefAttach.NodeScores(graph, nil, - availableBalance, nodes) + maxChanSize, nodes) if err != nil { t1.Fatalf("unable to select attachment "+ "directives: %v", err) @@ -598,9 +598,8 @@ func TestConstrainedPrefAttachmentSelectSkipNodes(t *testing.T) { // With our graph created, we'll now get the scores for // all nodes in the graph. - const availableBalance = btcutil.SatoshiPerBitcoin * 2.5 scores, err := prefAttach.NodeScores(graph, nil, - availableBalance, nodes) + maxChanSize, nodes) if err != nil { t1.Fatalf("unable to select attachment "+ "directives: %v", err) @@ -646,7 +645,7 @@ func TestConstrainedPrefAttachmentSelectSkipNodes(t *testing.T) { // then all nodes should have a score of zero, since we // already got channels to them. scores, err = prefAttach.NodeScores(graph, chans, - availableBalance, nodes) + maxChanSize, nodes) if err != nil { t1.Fatalf("unable to select attachment "+ "directives: %v", err) From ccf4b7feabf253684c423eb2c758811530bb76b1 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 19 Dec 2018 14:54:54 +0100 Subject: [PATCH 07/10] 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 5a7ca0c01..44d99ec94 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 e7b463063..964dd144e 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 b71ca8f95..3c52e2319 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 2c5f15148..3deda3b54 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 From 3739c19ef833eb25c280b0660f472e7d49c4d0ed Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 19 Dec 2018 14:54:54 +0100 Subject: [PATCH 08/10] autopilot/pref_attachment: rename ConstrainedPrefAttachment->PrefAttachment Since the ConstrainedPrefAttachment no longers require the heuristic to be aware of the autopilot constraints, we rename it PrefAttachment. --- autopilot/prefattach.go | 41 +++++++---------- autopilot/prefattach_test.go | 85 +++++++----------------------------- pilot.go | 7 +-- 3 files changed, 33 insertions(+), 100 deletions(-) diff --git a/autopilot/prefattach.go b/autopilot/prefattach.go index 3c52e2319..1e5f62c57 100644 --- a/autopilot/prefattach.go +++ b/autopilot/prefattach.go @@ -8,38 +8,29 @@ import ( "github.com/btcsuite/btcutil" ) -// ConstrainedPrefAttachment is an implementation of the AttachmentHeuristic -// interface that implement a constrained non-linear preferential attachment -// heuristic. This means that given a threshold to allocate to automatic -// channel establishment, the heuristic will attempt to favor connecting to -// nodes which already have a set amount of links, selected by sampling from a -// power law distribution. The attachment is non-linear in that it favors -// nodes with a higher in-degree but less so that regular linear preferential -// attachment. As a result, this creates smaller and less clusters than regular -// linear preferential attachment. +// PrefAttachment is an implementation of the AttachmentHeuristic interface +// that implement a non-linear preferential attachment heuristic. This means +// that given a threshold to allocate to automatic channel establishment, the +// heuristic will attempt to favor connecting to nodes which already have a set +// amount of links, selected by sampling from a power law distribution. The +// attachment is non-linear in that it favors nodes with a higher in-degree but +// less so than regular linear preferential attachment. As a result, this +// creates smaller and less clusters than regular linear preferential +// attachment. // // TODO(roasbeef): BA, with k=-3 -type ConstrainedPrefAttachment struct { - constraints AgentConstraints +type PrefAttachment struct { } -// NewConstrainedPrefAttachment creates a new instance of a -// ConstrainedPrefAttachment heuristics given bounds on allowed channel sizes, -// and an allocation amount which is interpreted as a percentage of funds that -// is to be committed to channels at all times. -func NewConstrainedPrefAttachment( - cfg AgentConstraints) *ConstrainedPrefAttachment { - +// NewPrefAttachment creates a new instance of a PrefAttachment heuristic. +func NewPrefAttachment() *PrefAttachment { prand.Seed(time.Now().Unix()) - - return &ConstrainedPrefAttachment{ - constraints: cfg, - } + return &PrefAttachment{} } -// A compile time assertion to ensure ConstrainedPrefAttachment meets the +// A compile time assertion to ensure PrefAttachment meets the // AttachmentHeuristic interface. -var _ AttachmentHeuristic = (*ConstrainedPrefAttachment)(nil) +var _ AttachmentHeuristic = (*PrefAttachment)(nil) // NodeID is a simple type that holds an EC public key serialized in compressed // format. @@ -69,7 +60,7 @@ func NewNodeID(pub *btcec.PublicKey) NodeID { // given to nodes already having high connectivity in the graph. // // NOTE: This is a part of the AttachmentHeuristic interface. -func (p *ConstrainedPrefAttachment) NodeScores(g ChannelGraph, chans []Channel, +func (p *PrefAttachment) NodeScores(g ChannelGraph, chans []Channel, chanSize btcutil.Amount, nodes map[NodeID]struct{}) ( map[NodeID]*AttachmentDirective, error) { diff --git a/autopilot/prefattach_test.go b/autopilot/prefattach_test.go index 3deda3b54..2c1dfa9c1 100644 --- a/autopilot/prefattach_test.go +++ b/autopilot/prefattach_test.go @@ -71,25 +71,14 @@ var chanGraphs = []struct { }, } -// TestConstrainedPrefAttachmentSelectEmptyGraph ensures that when passed an +// TestPrefAttachmentSelectEmptyGraph ensures that when passed an // empty graph, the NodeSores function always returns a score of 0. -func TestConstrainedPrefAttachmentSelectEmptyGraph(t *testing.T) { +func TestPrefAttachmentSelectEmptyGraph(t *testing.T) { const ( - minChanSize = 0 maxChanSize = btcutil.Amount(btcutil.SatoshiPerBitcoin) - chanLimit = 3 - threshold = 0.5 ) - constraints := NewConstraints( - minChanSize, - maxChanSize, - chanLimit, - 0, - threshold, - ) - - prefAttach := NewConstrainedPrefAttachment(constraints) + prefAttach := NewPrefAttachment() // Create a random public key, which we will query to get a score for. pub, err := randKey() @@ -175,27 +164,16 @@ func completeGraph(t *testing.T, g testGraph, numNodes int) { } } -// TestConstrainedPrefAttachmentSelectTwoVertexes ensures that when passed a +// TestPrefAttachmentSelectTwoVertexes ensures that when passed a // graph with only two eligible vertexes, then both are given the same score, // and the funds are appropriately allocated across each peer. -func TestConstrainedPrefAttachmentSelectTwoVertexes(t *testing.T) { +func TestPrefAttachmentSelectTwoVertexes(t *testing.T) { t.Parallel() prand.Seed(time.Now().Unix()) const ( - minChanSize = 0 maxChanSize = btcutil.Amount(btcutil.SatoshiPerBitcoin) - chanLimit = 3 - threshold = 0.5 - ) - - constraints := NewConstraints( - minChanSize, - maxChanSize, - chanLimit, - 0, - threshold, ) for _, graph := range chanGraphs { @@ -208,7 +186,7 @@ func TestConstrainedPrefAttachmentSelectTwoVertexes(t *testing.T) { defer cleanup() } - prefAttach := NewConstrainedPrefAttachment(constraints) + prefAttach := NewPrefAttachment() // For this set, we'll load the memory graph with two // nodes, and a random channel connecting them. @@ -295,27 +273,16 @@ func TestConstrainedPrefAttachmentSelectTwoVertexes(t *testing.T) { } } -// TestConstrainedPrefAttachmentSelectInsufficientFunds ensures that if the +// TestPrefAttachmentSelectInsufficientFunds ensures that if the // balance of the backing wallet is below the set min channel size, then it // never recommends candidates to attach to. -func TestConstrainedPrefAttachmentSelectInsufficientFunds(t *testing.T) { +func TestPrefAttachmentSelectInsufficientFunds(t *testing.T) { t.Parallel() prand.Seed(time.Now().Unix()) const ( - minChanSize = 0 maxChanSize = btcutil.Amount(btcutil.SatoshiPerBitcoin) - chanLimit = 3 - threshold = 0.5 - ) - - constraints := NewConstraints( - minChanSize, - maxChanSize, - chanLimit, - 0, - threshold, ) for _, graph := range chanGraphs { @@ -332,7 +299,7 @@ func TestConstrainedPrefAttachmentSelectInsufficientFunds(t *testing.T) { // them. completeGraph(t, graph, 10) - prefAttach := NewConstrainedPrefAttachment(constraints) + prefAttach := NewPrefAttachment() nodes := make(map[NodeID]struct{}) if err := graph.ForEachNode(func(n Node) error { @@ -368,27 +335,16 @@ func TestConstrainedPrefAttachmentSelectInsufficientFunds(t *testing.T) { } } -// TestConstrainedPrefAttachmentSelectGreedyAllocation tests that if upon +// TestPrefAttachmentSelectGreedyAllocation tests that if upon // returning node scores, the NodeScores method will attempt to greedily // allocate all funds to each vertex (up to the max channel size). -func TestConstrainedPrefAttachmentSelectGreedyAllocation(t *testing.T) { +func TestPrefAttachmentSelectGreedyAllocation(t *testing.T) { t.Parallel() prand.Seed(time.Now().Unix()) const ( - minChanSize = 0 maxChanSize = btcutil.Amount(btcutil.SatoshiPerBitcoin) - chanLimit = 3 - threshold = 0.5 - ) - - constraints := NewConstraints( - minChanSize, - maxChanSize, - chanLimit, - 0, - threshold, ) for _, graph := range chanGraphs { @@ -401,7 +357,7 @@ func TestConstrainedPrefAttachmentSelectGreedyAllocation(t *testing.T) { defer cleanup() } - prefAttach := NewConstrainedPrefAttachment(constraints) + prefAttach := NewPrefAttachment() const chanCapacity = btcutil.SatoshiPerBitcoin @@ -525,27 +481,16 @@ func TestConstrainedPrefAttachmentSelectGreedyAllocation(t *testing.T) { } } -// TestConstrainedPrefAttachmentSelectSkipNodes ensures that if a node was +// TestPrefAttachmentSelectSkipNodes ensures that if a node was // already selected as a channel counterparty, then that node will get a score // of zero during scoring. -func TestConstrainedPrefAttachmentSelectSkipNodes(t *testing.T) { +func TestPrefAttachmentSelectSkipNodes(t *testing.T) { t.Parallel() prand.Seed(time.Now().Unix()) const ( - minChanSize = 0 maxChanSize = btcutil.Amount(btcutil.SatoshiPerBitcoin) - chanLimit = 3 - threshold = 0.5 - ) - - constraints := NewConstraints( - minChanSize, - maxChanSize, - chanLimit, - 0, - threshold, ) for _, graph := range chanGraphs { @@ -558,7 +503,7 @@ func TestConstrainedPrefAttachmentSelectSkipNodes(t *testing.T) { defer cleanup() } - prefAttach := NewConstrainedPrefAttachment(constraints) + prefAttach := NewPrefAttachment() // Next, we'll create a simple topology of two nodes, // with a single channel connecting them. diff --git a/pilot.go b/pilot.go index e68a3d2b0..b6d9f63f7 100644 --- a/pilot.go +++ b/pilot.go @@ -95,11 +95,8 @@ func initAutoPilot(svr *server, cfg *autoPilotConfig) *autopilot.ManagerCfg { cfg.Allocation, ) - // First, we'll create the preferential attachment heuristic, - // initialized with the passed auto pilot configuration parameters. - prefAttachment := autopilot.NewConstrainedPrefAttachment( - atplConstraints, - ) + // First, we'll create the preferential attachment heuristic. + prefAttachment := autopilot.NewPrefAttachment() // With the heuristic itself created, we can now populate the remainder // of the items that the autopilot agent needs to perform its duties. From 25de66d27bd1c24ae41283e393ae428be4c9d8b1 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 19 Dec 2018 15:22:33 +0100 Subject: [PATCH 09/10] autopilot/interface+choice: add NodeScore type We create a new type NodeScore which is a tuple (NodeID, score). The weightedChoice and chooseN algorithms are altered to expect this type. This is done in order to simplify the types we are using, since we were only using a subset of the fields in AttachmentDirective. --- autopilot/agent.go | 39 ++++++++++++++++++++++++++++++--------- autopilot/choice.go | 10 +++++----- autopilot/choice_test.go | 14 +++++++------- autopilot/interface.go | 12 ++++++++++++ 4 files changed, 54 insertions(+), 21 deletions(-) diff --git a/autopilot/agent.go b/autopilot/agent.go index 44d99ec94..0e121d7ab 100644 --- a/autopilot/agent.go +++ b/autopilot/agent.go @@ -571,22 +571,43 @@ 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 - // nodes to attempt to open channels to. - chanCandidates, err := chooseN(numChans, scores) + // Temporary convert to NodeScore. + nodeScores := make(map[NodeID]*NodeScore) + for k, v := range scores { + nodeScores[k] = &NodeScore{ + NodeID: v.NodeID, + Score: v.Score, + } + } + + // Now use the score to make a weighted choice which nodes to attempt + // to open channels to. + nodeScores, err = chooseN(numChans, nodeScores) if err != nil { return fmt.Errorf("Unable to make weighted choice: %v", err) } + chanCandidates := make(map[NodeID]*AttachmentDirective) + for nID := range nodeScores { + // Add addresses to the candidates. + addrs := addresses[nID] + + // If the node has no known addresses, we cannot connect to it, + // so we'll skip it. + if len(addrs) == 0 { + continue + } + + chanCandidates[nID] = &AttachmentDirective{ + NodeID: nID, + ChanAmt: chanSize, + Addrs: addrs, + } + } + if len(chanCandidates) == 0 { log.Infof("No eligible candidates to connect to") return nil diff --git a/autopilot/choice.go b/autopilot/choice.go index e30525354..661f58d93 100644 --- a/autopilot/choice.go +++ b/autopilot/choice.go @@ -46,10 +46,10 @@ func weightedChoice(w []float64) (int, error) { return 0, fmt.Errorf("unable to make choice") } -// chooseN picks at random min[n, len(s)] nodes if from the -// AttachmentDirectives map, with a probability weighted by their score. -func chooseN(n uint32, s map[NodeID]*AttachmentDirective) ( - map[NodeID]*AttachmentDirective, error) { +// chooseN picks at random min[n, len(s)] nodes if from the NodeScore map, with +// a probability weighted by their score. +func chooseN(n uint32, s map[NodeID]*NodeScore) ( + map[NodeID]*NodeScore, error) { // Keep track of the number of nodes not yet chosen, in addition to // their scores and NodeIDs. @@ -65,7 +65,7 @@ func chooseN(n uint32, s map[NodeID]*AttachmentDirective) ( // Pick a weighted choice from the remaining nodes as long as there are // nodes left, and we haven't already picked n. - chosen := make(map[NodeID]*AttachmentDirective) + chosen := make(map[NodeID]*NodeScore) for len(chosen) < int(n) && rem > 0 { choice, err := weightedChoice(scores) if err == ErrNoPositive { diff --git a/autopilot/choice_test.go b/autopilot/choice_test.go index 6c17c6098..d984d1b41 100644 --- a/autopilot/choice_test.go +++ b/autopilot/choice_test.go @@ -173,7 +173,7 @@ func TestWeightedChoiceDistribution(t *testing.T) { func TestChooseNEmptyMap(t *testing.T) { t.Parallel() - nodes := map[NodeID]*AttachmentDirective{} + nodes := map[NodeID]*NodeScore{} property := func(n uint32) bool { res, err := chooseN(n, nodes) if err != nil { @@ -191,12 +191,12 @@ func TestChooseNEmptyMap(t *testing.T) { // candidateMapVarLen is a type we'll use to generate maps of various lengths // up to 255 to be used during QuickTests. -type candidateMapVarLen map[NodeID]*AttachmentDirective +type candidateMapVarLen map[NodeID]*NodeScore // Generate generates a value of type candidateMapVarLen to be used during // QuickTests. func (candidateMapVarLen) Generate(rand *rand.Rand, size int) reflect.Value { - nodes := make(map[NodeID]*AttachmentDirective) + nodes := make(map[NodeID]*NodeScore) // To avoid creating huge maps, we restrict them to max uint8 len. n := uint8(rand.Uint32()) @@ -212,7 +212,7 @@ func (candidateMapVarLen) Generate(rand *rand.Rand, size int) reflect.Value { var nID [33]byte binary.BigEndian.PutUint32(nID[:], uint32(i)) - nodes[nID] = &AttachmentDirective{ + nodes[nID] = &NodeScore{ Score: s, } } @@ -226,7 +226,7 @@ func TestChooseNMinimum(t *testing.T) { t.Parallel() // Helper to count the number of positive scores in the given map. - numPositive := func(nodes map[NodeID]*AttachmentDirective) int { + numPositive := func(nodes map[NodeID]*NodeScore) int { cnt := 0 for _, v := range nodes { if v.Score > 0 { @@ -274,7 +274,7 @@ func TestChooseNSample(t *testing.T) { const maxIterations = 100000 fifth := uint32(numNodes / 5) - nodes := make(map[NodeID]*AttachmentDirective) + nodes := make(map[NodeID]*NodeScore) // we make 5 buckets of nodes: 0, 0.1, 0.2, 0.4 and 0.8 score. We want // to check that zero scores never gets chosen, while a doubling the @@ -299,7 +299,7 @@ func TestChooseNSample(t *testing.T) { var nID [33]byte binary.BigEndian.PutUint32(nID[:], i) - nodes[nID] = &AttachmentDirective{ + nodes[nID] = &NodeScore{ Score: s, } } diff --git a/autopilot/interface.go b/autopilot/interface.go index cb9ff9abd..75e15e6d3 100644 --- a/autopilot/interface.go +++ b/autopilot/interface.go @@ -81,6 +81,18 @@ type ChannelGraph interface { ForEachNode(func(Node) error) error } +// NodeScore is a tuple mapping a NodeID to a score indicating the preference +// of opening a channel with it. +type NodeScore struct { + // NodeID is the serialized compressed pubkey of the node that is being + // scored. + NodeID NodeID + + // Score is the score given by the heuristic for opening a channel of + // the given size to this node. + Score float64 +} + // AttachmentDirective describes a channel attachment proscribed by an // AttachmentHeuristic. It details to which node a channel should be created // to, and also the parameters which should be used in the channel creation. From 5b1e72a0194e7f399e492440defb46ad703fccb1 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 19 Dec 2018 15:24:17 +0100 Subject: [PATCH 10/10] autopilot/interface+agent: return NodeScore from NodeScores Since NodeScores no longer returns fully populated AttachmentDirectives, we make this explicit by defining a new type NodeScore that includes a subset of what the AttachmentDirective does. --- autopilot/agent.go | 13 +-- autopilot/agent_test.go | 179 +++++++++++++++++++++++------------ autopilot/interface.go | 10 +- autopilot/prefattach.go | 11 +-- autopilot/prefattach_test.go | 89 ----------------- 5 files changed, 129 insertions(+), 173 deletions(-) diff --git a/autopilot/agent.go b/autopilot/agent.go index 0e121d7ab..51d3b2cc3 100644 --- a/autopilot/agent.go +++ b/autopilot/agent.go @@ -573,25 +573,16 @@ func (a *Agent) openChans(availableFunds btcutil.Amount, numChans uint32, log.Debugf("Got scores for %d nodes", len(scores)) - // Temporary convert to NodeScore. - nodeScores := make(map[NodeID]*NodeScore) - for k, v := range scores { - nodeScores[k] = &NodeScore{ - NodeID: v.NodeID, - Score: v.Score, - } - } - // Now use the score to make a weighted choice which nodes to attempt // to open channels to. - nodeScores, err = chooseN(numChans, nodeScores) + scores, err = chooseN(numChans, scores) if err != nil { return fmt.Errorf("Unable to make weighted choice: %v", err) } chanCandidates := make(map[NodeID]*AttachmentDirective) - for nID := range nodeScores { + for nID := range scores { // Add addresses to the candidates. addrs := addresses[nID] diff --git a/autopilot/agent_test.go b/autopilot/agent_test.go index 964dd144e..df368ca5d 100644 --- a/autopilot/agent_test.go +++ b/autopilot/agent_test.go @@ -67,7 +67,7 @@ func (m *mockConstraints) MaxChanSize() btcutil.Amount { var _ AgentConstraints = (*mockConstraints)(nil) type mockHeuristic struct { - nodeScoresResps chan map[NodeID]*AttachmentDirective + nodeScoresResps chan map[NodeID]*NodeScore nodeScoresArgs chan directiveArg quit chan struct{} @@ -82,7 +82,7 @@ type directiveArg struct { func (m *mockHeuristic) NodeScores(g ChannelGraph, chans []Channel, fundsAvailable btcutil.Amount, nodes map[NodeID]struct{}) ( - map[NodeID]*AttachmentDirective, error) { + map[NodeID]*NodeScore, error) { if m.nodeScoresArgs != nil { directive := directiveArg{ @@ -160,7 +160,7 @@ func TestAgentChannelOpenSignal(t *testing.T) { quit := make(chan struct{}) heuristic := &mockHeuristic{ - nodeScoresResps: make(chan map[NodeID]*AttachmentDirective), + nodeScoresResps: make(chan map[NodeID]*NodeScore), quit: quit, } constraints := &mockConstraints{ @@ -250,7 +250,7 @@ func TestAgentChannelOpenSignal(t *testing.T) { // If this send success, then Select was erroneously called and the // test should be failed. - case heuristic.nodeScoresResps <- map[NodeID]*AttachmentDirective{}: + case heuristic.nodeScoresResps <- map[NodeID]*NodeScore{}: t.Fatalf("Select was called but shouldn't have been") // This is the correct path as Select should've be called. @@ -295,7 +295,7 @@ func TestAgentChannelFailureSignal(t *testing.T) { quit := make(chan struct{}) heuristic := &mockHeuristic{ - nodeScoresResps: make(chan map[NodeID]*AttachmentDirective), + nodeScoresResps: make(chan map[NodeID]*NodeScore), quit: quit, } constraints := &mockConstraints{ @@ -363,14 +363,13 @@ func TestAgentChannelFailureSignal(t *testing.T) { // At this point, the agent should now be querying the heuristic to // request attachment directives, return a fake so the agent will // attempt to open a channel. - var fakeDirective = &AttachmentDirective{ - NodeID: NewNodeID(node), - ChanAmt: btcutil.SatoshiPerBitcoin, - Score: 0.5, + var fakeDirective = &NodeScore{ + NodeID: NewNodeID(node), + Score: 0.5, } select { - case heuristic.nodeScoresResps <- map[NodeID]*AttachmentDirective{ + case heuristic.nodeScoresResps <- map[NodeID]*NodeScore{ NewNodeID(node): fakeDirective, }: case <-time.After(time.Second * 10): @@ -387,7 +386,7 @@ func TestAgentChannelFailureSignal(t *testing.T) { } select { - case heuristic.nodeScoresResps <- map[NodeID]*AttachmentDirective{}: + case heuristic.nodeScoresResps <- map[NodeID]*NodeScore{}: case <-time.After(time.Second * 10): t.Fatal("heuristic wasn't queried in time") } @@ -408,7 +407,7 @@ func TestAgentChannelCloseSignal(t *testing.T) { quit := make(chan struct{}) heuristic := &mockHeuristic{ - nodeScoresResps: make(chan map[NodeID]*AttachmentDirective), + nodeScoresResps: make(chan map[NodeID]*NodeScore), quit: quit, } constraints := &mockConstraints{ @@ -505,7 +504,7 @@ func TestAgentChannelCloseSignal(t *testing.T) { // If this send success, then Select was erroneously called and the // test should be failed. - case heuristic.nodeScoresResps <- map[NodeID]*AttachmentDirective{}: + case heuristic.nodeScoresResps <- map[NodeID]*NodeScore{}: t.Fatalf("Select was called but shouldn't have been") // This is the correct path as Select should've be called. @@ -528,7 +527,7 @@ func TestAgentBalanceUpdate(t *testing.T) { quit := make(chan struct{}) heuristic := &mockHeuristic{ - nodeScoresResps: make(chan map[NodeID]*AttachmentDirective), + nodeScoresResps: make(chan map[NodeID]*NodeScore), quit: quit, } constraints := &mockConstraints{ @@ -626,7 +625,7 @@ func TestAgentBalanceUpdate(t *testing.T) { // If this send success, then Select was erroneously called and the // test should be failed. - case heuristic.nodeScoresResps <- map[NodeID]*AttachmentDirective{}: + case heuristic.nodeScoresResps <- map[NodeID]*NodeScore{}: t.Fatalf("Select was called but shouldn't have been") // This is the correct path as Select should've be called. @@ -648,7 +647,7 @@ func TestAgentImmediateAttach(t *testing.T) { quit := make(chan struct{}) heuristic := &mockHeuristic{ - nodeScoresResps: make(chan map[NodeID]*AttachmentDirective), + nodeScoresResps: make(chan map[NodeID]*NodeScore), quit: quit, } constraints := &mockConstraints{ @@ -707,7 +706,7 @@ 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) + directives := make(map[NodeID]*NodeScore) nodeKeys := make(map[NodeID]struct{}) for i := 0; i < numChans; i++ { pub, err := memGraph.addRandNode() @@ -715,10 +714,9 @@ func TestAgentImmediateAttach(t *testing.T) { t.Fatalf("unable to generate key: %v", err) } nodeID := NewNodeID(pub) - directives[nodeID] = &AttachmentDirective{ - NodeID: nodeID, - ChanAmt: btcutil.SatoshiPerBitcoin, - Score: 0.5, + directives[nodeID] = &NodeScore{ + NodeID: nodeID, + Score: 0.5, } nodeKeys[nodeID] = struct{}{} } @@ -786,7 +784,7 @@ func TestAgentPrivateChannels(t *testing.T) { quit := make(chan struct{}) heuristic := &mockHeuristic{ - nodeScoresResps: make(chan map[NodeID]*AttachmentDirective), + nodeScoresResps: make(chan map[NodeID]*NodeScore), quit: quit, } constraints := &mockConstraints{ @@ -848,16 +846,15 @@ func TestAgentPrivateChannels(t *testing.T) { // 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) + directives := make(map[NodeID]*NodeScore) 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, + directives[NewNodeID(pub)] = &NodeScore{ + NodeID: NewNodeID(pub), + Score: 0.5, } } @@ -914,7 +911,7 @@ func TestAgentPendingChannelState(t *testing.T) { quit := make(chan struct{}) heuristic := &mockHeuristic{ - nodeScoresResps: make(chan map[NodeID]*AttachmentDirective), + nodeScoresResps: make(chan map[NodeID]*NodeScore), quit: quit, } constraints := &mockConstraints{ @@ -980,10 +977,9 @@ func TestAgentPendingChannelState(t *testing.T) { t.Fatalf("unable to generate key: %v", err) } nodeID := NewNodeID(nodeKey) - nodeDirective := &AttachmentDirective{ - NodeID: nodeID, - ChanAmt: 0.5 * btcutil.SatoshiPerBitcoin, - Score: 0.5, + nodeDirective := &NodeScore{ + NodeID: nodeID, + Score: 0.5, } // Once again, we'll start by telling the agent as part of its first @@ -1002,7 +998,7 @@ func TestAgentPendingChannelState(t *testing.T) { constraints.moreChanArgs = make(chan moreChanArg) select { - case heuristic.nodeScoresResps <- map[NodeID]*AttachmentDirective{ + case heuristic.nodeScoresResps <- map[NodeID]*NodeScore{ nodeID: nodeDirective, }: case <-time.After(time.Second * 10): @@ -1014,9 +1010,10 @@ func TestAgentPendingChannelState(t *testing.T) { // A request to open the channel should've also been sent. select { case openChan := <-chanController.openChanSignals: - if openChan.amt != nodeDirective.ChanAmt { + chanAmt := constraints.MaxChanSize() + if openChan.amt != chanAmt { t.Fatalf("invalid chan amt: expected %v, got %v", - nodeDirective.ChanAmt, openChan.amt) + chanAmt, openChan.amt) } if !openChan.target.IsEqual(nodeKey) { t.Fatalf("unexpected key: expected %x, got %x", @@ -1044,13 +1041,14 @@ func TestAgentPendingChannelState(t *testing.T) { // one that we just created, otherwise the agent isn't properly // updating its internal state. case req := <-constraints.moreChanArgs: + chanAmt := constraints.MaxChanSize() if len(req.chans) != 1 { t.Fatalf("should include pending chan in current "+ "state, instead have %v chans", len(req.chans)) } - if req.chans[0].Capacity != nodeDirective.ChanAmt { + if req.chans[0].Capacity != chanAmt { t.Fatalf("wrong chan capacity: expected %v, got %v", - req.chans[0].Capacity, nodeDirective.ChanAmt) + req.chans[0].Capacity, chanAmt) } if req.chans[0].Node != nodeID { t.Fatalf("wrong node ID: expected %x, got %x", @@ -1100,7 +1098,7 @@ func TestAgentPendingOpenChannel(t *testing.T) { quit := make(chan struct{}) heuristic := &mockHeuristic{ - nodeScoresResps: make(chan map[NodeID]*AttachmentDirective), + nodeScoresResps: make(chan map[NodeID]*NodeScore), quit: quit, } constraints := &mockConstraints{ @@ -1172,7 +1170,7 @@ func TestAgentPendingOpenChannel(t *testing.T) { // There shouldn't be a call to the Select method as we've returned // "false" for NeedMoreChans above. select { - case heuristic.nodeScoresResps <- map[NodeID]*AttachmentDirective{}: + case heuristic.nodeScoresResps <- map[NodeID]*NodeScore{}: t.Fatalf("Select was called but shouldn't have been") default: } @@ -1195,7 +1193,7 @@ func TestAgentOnNodeUpdates(t *testing.T) { quit := make(chan struct{}) heuristic := &mockHeuristic{ - nodeScoresResps: make(chan map[NodeID]*AttachmentDirective), + nodeScoresResps: make(chan map[NodeID]*NodeScore), quit: quit, } constraints := &mockConstraints{ @@ -1259,7 +1257,7 @@ func TestAgentOnNodeUpdates(t *testing.T) { // Send over an empty list of attachment directives, which should cause // the agent to return to waiting on a new signal. select { - case heuristic.nodeScoresResps <- map[NodeID]*AttachmentDirective{}: + case heuristic.nodeScoresResps <- map[NodeID]*NodeScore{}: case <-time.After(time.Second * 10): t.Fatalf("Select was not called but should have been") } @@ -1284,7 +1282,7 @@ func TestAgentOnNodeUpdates(t *testing.T) { // It's not important that this list is also empty, so long as the node // updates signal is causing the agent to make this attempt. select { - case heuristic.nodeScoresResps <- map[NodeID]*AttachmentDirective{}: + case heuristic.nodeScoresResps <- map[NodeID]*NodeScore{}: case <-time.After(time.Second * 10): t.Fatalf("Select was not called but should have been") } @@ -1308,7 +1306,8 @@ func TestAgentSkipPendingConns(t *testing.T) { quit := make(chan struct{}) heuristic := &mockHeuristic{ - nodeScoresResps: make(chan map[NodeID]*AttachmentDirective), + nodeScoresArgs: make(chan directiveArg), + nodeScoresResps: make(chan map[NodeID]*NodeScore), quit: quit, } constraints := &mockConstraints{ @@ -1384,12 +1383,20 @@ func TestAgentSkipPendingConns(t *testing.T) { if err != nil { t.Fatalf("unable to generate key: %v", err) } - nodeDirective := &AttachmentDirective{ - NodeID: NewNodeID(nodeKey), - ChanAmt: 0.5 * btcutil.SatoshiPerBitcoin, - Score: 0.5, + nodeID := NewNodeID(nodeKey) + nodeDirective := &NodeScore{ + NodeID: nodeID, + Score: 0.5, } + // We'll also add a second node to the graph, to keep the first one + // company. + nodeKey2, err := memGraph.addRandNode() + if err != nil { + t.Fatalf("unable to generate key: %v", err) + } + nodeID2 := NewNodeID(nodeKey2) + // 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. @@ -1402,14 +1409,34 @@ func TestAgentSkipPendingConns(t *testing.T) { t.Fatalf("heuristic wasn't queried in time") } + // Both nodes should be part of the arguments. select { - case heuristic.nodeScoresResps <- map[NodeID]*AttachmentDirective{ + case req := <-heuristic.nodeScoresArgs: + if len(req.nodes) != 2 { + t.Fatalf("expected %v nodes, instead "+ + "had %v", 2, len(req.nodes)) + } + if _, ok := req.nodes[nodeID]; !ok { + t.Fatalf("node not included in arguments") + } + if _, ok := req.nodes[nodeID2]; !ok { + t.Fatalf("node not included in arguments") + } + case <-time.After(time.Second * 10): + t.Fatalf("select wasn't queried in time") + } + + // Respond with a scored directive. We skip node2 for now, implicitly + // giving it a zero-score. + select { + case heuristic.nodeScoresResps <- map[NodeID]*NodeScore{ NewNodeID(nodeKey): nodeDirective, }: case <-time.After(time.Second * 10): t.Fatalf("heuristic wasn't queried in time") } + // The agent should attempt connection to the node. var errChan chan error select { case errChan = <-connect: @@ -1430,17 +1457,30 @@ func TestAgentSkipPendingConns(t *testing.T) { t.Fatalf("heuristic wasn't queried in time") } - // Send a directive for the same node, which already has a pending conn. + // Since the node now has a pending connection, it should be skipped + // and not part of the nodes attempting to be scored. select { - case heuristic.nodeScoresResps <- map[NodeID]*AttachmentDirective{ - NewNodeID(nodeKey): nodeDirective, - }: + case req := <-heuristic.nodeScoresArgs: + if len(req.nodes) != 1 { + t.Fatalf("expected %v nodes, instead "+ + "had %v", 1, len(req.nodes)) + } + if _, ok := req.nodes[nodeID2]; !ok { + t.Fatalf("node not included in arguments") + } + case <-time.After(time.Second * 10): + t.Fatalf("select wasn't queried in time") + } + + // Respond with an emtpty score set. + select { + case heuristic.nodeScoresResps <- map[NodeID]*NodeScore{}: case <-time.After(time.Second * 10): t.Fatalf("heuristic wasn't queried in time") } - // This time, the agent should skip trying to connect to the node with a - // pending connection. + // The agent should not attempt any connection, since no nodes were + // scored. select { case <-connect: t.Fatalf("agent should not have attempted connection") @@ -1466,20 +1506,39 @@ func TestAgentSkipPendingConns(t *testing.T) { t.Fatalf("heuristic wasn't queried in time") } - // Send a directive for the same node, which already has a pending conn. + // The node should now be marked as "failed", which should make it + // being skipped during scoring. Again check that it won't be among the + // score request. select { - case heuristic.nodeScoresResps <- map[NodeID]*AttachmentDirective{ - NewNodeID(nodeKey): nodeDirective, + case req := <-heuristic.nodeScoresArgs: + if len(req.nodes) != 1 { + t.Fatalf("expected %v nodes, instead "+ + "had %v", 1, len(req.nodes)) + } + if _, ok := req.nodes[nodeID2]; !ok { + t.Fatalf("node not included in arguments") + } + case <-time.After(time.Second * 10): + t.Fatalf("select wasn't queried in time") + } + + // Send a directive for the second node. + nodeDirective2 := &NodeScore{ + NodeID: nodeID2, + Score: 0.5, + } + select { + case heuristic.nodeScoresResps <- map[NodeID]*NodeScore{ + nodeID2: nodeDirective2, }: case <-time.After(time.Second * 10): t.Fatalf("heuristic wasn't queried in time") } - // This time, the agent should try the connection since the peer has - // been removed from the pending map. + // This time, the agent should try the connection to the second node. select { case <-connect: case <-time.After(time.Second * 10): - t.Fatalf("agent have attempted connection") + t.Fatalf("agent should have attempted connection") } } diff --git a/autopilot/interface.go b/autopilot/interface.go index 75e15e6d3..51405e7fc 100644 --- a/autopilot/interface.go +++ b/autopilot/interface.go @@ -110,10 +110,6 @@ type AttachmentDirective struct { // Addrs is a list of addresses that the target peer may be reachable // at. Addrs []net.Addr - - // Score is the score given by the heuristic for opening a channel of - // the given size to this node. - Score float64 } // AttachmentHeuristic is one of the primary interfaces within this package. @@ -126,8 +122,8 @@ type AttachmentHeuristic interface { // NodeScores is a method that given the current channel graph and // current set of local channels, scores the given nodes according to // the preference of opening a channel of the given size with them. The - // returned channel candidates maps the NodeID to an attachment - // directive containing a score. + // returned channel candidates maps the NodeID to a NodeScore for the + // node. // // The scores will be in the range [0, M], where 0 indicates no // improvement in connectivity if a channel is opened to this node, @@ -139,7 +135,7 @@ type AttachmentHeuristic interface { // score of 0. NodeScores(g ChannelGraph, chans []Channel, chanSize btcutil.Amount, nodes map[NodeID]struct{}) ( - map[NodeID]*AttachmentDirective, error) + map[NodeID]*NodeScore, error) } // ChannelController is a simple interface that allows an auto-pilot agent to diff --git a/autopilot/prefattach.go b/autopilot/prefattach.go index 1e5f62c57..af5f45695 100644 --- a/autopilot/prefattach.go +++ b/autopilot/prefattach.go @@ -62,7 +62,7 @@ func NewNodeID(pub *btcec.PublicKey) NodeID { // NOTE: This is a part of the AttachmentHeuristic interface. func (p *PrefAttachment) NodeScores(g ChannelGraph, chans []Channel, chanSize btcutil.Amount, nodes map[NodeID]struct{}) ( - map[NodeID]*AttachmentDirective, error) { + map[NodeID]*NodeScore, 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. @@ -108,7 +108,7 @@ func (p *PrefAttachment) NodeScores(g ChannelGraph, chans []Channel, // For each node in the set of nodes, count their fraction of channels // in the graph, and use that as the score. - candidates := make(map[NodeID]*AttachmentDirective) + candidates := make(map[NodeID]*NodeScore) for nID, nodeChans := range nodeChanNum { _, ok := existingPeers[nID] @@ -129,10 +129,9 @@ func (p *PrefAttachment) NodeScores(g ChannelGraph, chans []Channel, // Otherwise we score the node according to its fraction of // channels in the graph. score := float64(nodeChans) / float64(graphChans) - candidates[nID] = &AttachmentDirective{ - NodeID: nID, - ChanAmt: chanSize, - Score: score, + candidates[nID] = &NodeScore{ + NodeID: nID, + Score: score, } } diff --git a/autopilot/prefattach_test.go b/autopilot/prefattach_test.go index 2c1dfa9c1..b34910417 100644 --- a/autopilot/prefattach_test.go +++ b/autopilot/prefattach_test.go @@ -247,15 +247,6 @@ func TestPrefAttachmentSelectTwoVertexes(t *testing.T) { nodeID[:]) } - // As the number of funds available exceed the - // max channel size, both edges should consume - // the maximum channel size. - if candidate.ChanAmt != maxChanSize { - t1.Fatalf("max channel size should be "+ - "allocated, instead %v was: ", - maxChanSize) - } - // Since each of the nodes has 1 channel, out // of only one channel in the graph, we expect // their score to be 0.5. @@ -273,68 +264,6 @@ func TestPrefAttachmentSelectTwoVertexes(t *testing.T) { } } -// TestPrefAttachmentSelectInsufficientFunds ensures that if the -// balance of the backing wallet is below the set min channel size, then it -// never recommends candidates to attach to. -func TestPrefAttachmentSelectInsufficientFunds(t *testing.T) { - t.Parallel() - - prand.Seed(time.Now().Unix()) - - const ( - maxChanSize = btcutil.Amount(btcutil.SatoshiPerBitcoin) - ) - - for _, graph := range chanGraphs { - success := t.Run(graph.name, func(t1 *testing.T) { - graph, cleanup, err := graph.genFunc() - if err != nil { - t1.Fatalf("unable to create graph: %v", err) - } - if cleanup != nil { - defer cleanup() - } - - // Add 10 nodes to the graph, with channels between - // them. - completeGraph(t, graph, 10) - - prefAttach := NewPrefAttachment() - - nodes := make(map[NodeID]struct{}) - if err := graph.ForEachNode(func(n Node) error { - nodes[n.PubKey()] = struct{}{} - return nil - }); err != nil { - t1.Fatalf("unable to traverse graph: %v", err) - } - - // With the necessary state initialized, we'll now - // attempt to get the score for our list of nodes, - // passing zero for the amount of wallet funds. This - // should return candidates with zero-value channels. - scores, err := prefAttach.NodeScores(graph, nil, - 0, nodes) - if err != nil { - t1.Fatalf("unable to select attachment "+ - "directives: %v", err) - } - - // Since all should be given a score of 0, the map - // should be empty. - for _, s := range scores { - if s.ChanAmt != 0 { - t1.Fatalf("expected zero channel, "+ - "instead got %v ", s.ChanAmt) - } - } - }) - if !success { - break - } - } -} - // TestPrefAttachmentSelectGreedyAllocation tests that if upon // returning node scores, the NodeScores method will attempt to greedily // allocate all funds to each vertex (up to the max channel size). @@ -437,12 +366,6 @@ func TestPrefAttachmentSelectGreedyAllocation(t *testing.T) { if candidate.Score == 0 { t1.Fatalf("Expected non-zero score") } - - if candidate.ChanAmt != maxChanSize { - t1.Fatalf("expected recommendation "+ - "of %v, instead got %v", - maxChanSize, candidate.ChanAmt) - } } // Imagine a few channels are being opened, and there's @@ -467,12 +390,6 @@ func TestPrefAttachmentSelectGreedyAllocation(t *testing.T) { if candidate.Score == 0 { t1.Fatalf("Expected non-zero score") } - - if candidate.ChanAmt != remBalance { - t1.Fatalf("expected recommendation "+ - "of %v, instead got %v", - remBalance, candidate.ChanAmt) - } } }) if !success { @@ -546,12 +463,6 @@ func TestPrefAttachmentSelectSkipNodes(t *testing.T) { if candidate.Score == 0 { t1.Fatalf("Expected non-zero score") } - - if candidate.ChanAmt != maxChanSize { - t1.Fatalf("expected recommendation "+ - "of %v, instead got %v", - maxChanSize, candidate.ChanAmt) - } } // We'll simulate a channel update by adding the nodes