diff --git a/autopilot/agent.go b/autopilot/agent.go index adba58c9..af119f06 100644 --- a/autopilot/agent.go +++ b/autopilot/agent.go @@ -362,7 +362,7 @@ func (a *Agent) controller(startingBalance btcutil.Amount) { // consult our channel attachment heuristic to // determine if we should open up any additional // channels or modify existing channels. - availableFunds, needMore := a.cfg.Heuristic.NeedMoreChans( + availableFunds, numChans, needMore := a.cfg.Heuristic.NeedMoreChans( totalChans, a.totalBalance, ) if !needMore { @@ -387,7 +387,7 @@ func (a *Agent) controller(startingBalance btcutil.Amount) { // for us to use. chanCandidates, err := a.cfg.Heuristic.Select( a.cfg.Self, a.cfg.Graph, availableFunds, - nodesToSkip, + numChans, nodesToSkip, ) if err != nil { log.Errorf("Unable to select candidates for "+ @@ -419,6 +419,7 @@ func (a *Agent) controller(startingBalance btcutil.Amount) { go func(directive AttachmentDirective) { pub := directive.PeerKey err := a.cfg.ChanController.OpenChannel( + directive.PeerKey, directive.ChanAmt, directive.Addrs, diff --git a/autopilot/agent_test.go b/autopilot/agent_test.go index b4587b5b..d8a3d57e 100644 --- a/autopilot/agent_test.go +++ b/autopilot/agent_test.go @@ -8,7 +8,6 @@ import ( "time" "errors" - "fmt" "github.com/roasbeef/btcd/btcec" "github.com/roasbeef/btcd/wire" @@ -17,6 +16,7 @@ import ( type moreChansResp struct { needMore bool + numMore uint32 amt btcutil.Amount } @@ -34,7 +34,7 @@ type mockHeuristic struct { } func (m *mockHeuristic) NeedMoreChans(chans []Channel, - balance btcutil.Amount) (btcutil.Amount, bool) { + balance btcutil.Amount) (btcutil.Amount, uint32, bool) { if m.moreChanArgs != nil { m.moreChanArgs <- moreChanArg{ @@ -45,7 +45,7 @@ func (m *mockHeuristic) NeedMoreChans(chans []Channel, } resp := <-m.moreChansResps - return resp.amt, resp.needMore + return resp.amt, resp.numMore, resp.needMore } type directiveArg struct { @@ -56,8 +56,8 @@ type directiveArg struct { } func (m *mockHeuristic) Select(self *btcec.PublicKey, graph ChannelGraph, - - amtToUse btcutil.Amount, skipChans map[NodeID]struct{}) ([]AttachmentDirective, error) { + amtToUse btcutil.Amount, numChans uint32, + skipChans map[NodeID]struct{}) ([]AttachmentDirective, error) { if m.directiveArgs != nil { m.directiveArgs <- directiveArg{ @@ -160,7 +160,7 @@ func TestAgentChannelOpenSignal(t *testing.T) { wg.Add(1) go func() { select { - case heuristic.moreChansResps <- moreChansResp{false, 0}: + case heuristic.moreChansResps <- moreChansResp{false, 0, 0}: wg.Done() return case <-time.After(time.Second * 10): @@ -185,7 +185,7 @@ func TestAgentChannelOpenSignal(t *testing.T) { wg.Add(1) go func() { select { - case heuristic.moreChansResps <- moreChansResp{false, 0}: + case heuristic.moreChansResps <- moreChansResp{false, 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. @@ -290,15 +290,14 @@ 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, 5 * btcutil.SatoshiPerBitcoin}: - fmt.Println("Returning 5BTC from heuristic") + case heuristic.moreChansResps <- moreChansResp{true, 1, 5 * btcutil.SatoshiPerBitcoin}: case <-time.After(time.Second * 10): t.Fatal("heuristic wasn't queried in time") } // 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. + // request attachment directives, return a fake so the agent will + // attempt to open a channel. var fakeDirective = AttachmentDirective{ PeerKey: self, ChanAmt: btcutil.SatoshiPerBitcoin, @@ -311,7 +310,6 @@ func TestAgentChannelFailureSignal(t *testing.T) { select { case heuristic.directiveResps <- []AttachmentDirective{fakeDirective}: - fmt.Println("Returning a node to connect to from heuristic") case <-time.After(time.Second * 10): t.Fatal("heuristic wasn't queried in time") } @@ -320,15 +318,13 @@ func TestAgentChannelFailureSignal(t *testing.T) { // Now ensure that the controller loop is re-executed. select { - case heuristic.moreChansResps <- moreChansResp{true, 5 * btcutil.SatoshiPerBitcoin}: - fmt.Println("Returning need more channels from heuristic") + case heuristic.moreChansResps <- moreChansResp{true, 1, 5 * btcutil.SatoshiPerBitcoin}: case <-time.After(time.Second * 10): t.Fatal("heuristic wasn't queried in time") } select { case heuristic.directiveResps <- []AttachmentDirective{}: - fmt.Println("Returning an empty directives list") case <-time.After(time.Second * 10): t.Fatal("heuristic wasn't queried in time") } @@ -397,7 +393,7 @@ func TestAgentChannelCloseSignal(t *testing.T) { wg.Add(1) go func() { select { - case heuristic.moreChansResps <- moreChansResp{false, 0}: + case heuristic.moreChansResps <- moreChansResp{false, 0, 0}: wg.Done() return case <-time.After(time.Second * 10): @@ -418,7 +414,7 @@ func TestAgentChannelCloseSignal(t *testing.T) { wg.Add(1) go func() { select { - case heuristic.moreChansResps <- moreChansResp{false, 0}: + case heuristic.moreChansResps <- moreChansResp{false, 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. @@ -509,7 +505,7 @@ func TestAgentBalanceUpdate(t *testing.T) { wg.Add(1) go func() { select { - case heuristic.moreChansResps <- moreChansResp{false, 0}: + case heuristic.moreChansResps <- moreChansResp{false, 0, 0}: wg.Done() return case <-time.After(time.Second * 10): @@ -531,7 +527,7 @@ func TestAgentBalanceUpdate(t *testing.T) { wg.Add(1) go func() { select { - case heuristic.moreChansResps <- moreChansResp{false, 0}: + case heuristic.moreChansResps <- moreChansResp{false, 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. @@ -619,6 +615,8 @@ func TestAgentImmediateAttach(t *testing.T) { var wg sync.WaitGroup + const numChans = 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. @@ -629,7 +627,7 @@ 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{true, 5 * btcutil.SatoshiPerBitcoin}: + case heuristic.moreChansResps <- moreChansResp{true, numChans, 5 * btcutil.SatoshiPerBitcoin}: wg.Done() return case <-time.After(time.Second * 10): @@ -644,7 +642,6 @@ 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. - const numChans = 5 directives := make([]AttachmentDirective, numChans) for i := 0; i < numChans; i++ { directives[i] = AttachmentDirective{ @@ -762,7 +759,7 @@ func TestAgentPendingChannelState(t *testing.T) { // We'll send over a response indicating that it should // establish more channels, and give it a budget of 1 BTC to do // so. - case heuristic.moreChansResps <- moreChansResp{true, btcutil.SatoshiPerBitcoin}: + case heuristic.moreChansResps <- moreChansResp{true, 1, btcutil.SatoshiPerBitcoin}: wg.Done() return case <-time.After(time.Second * 10): @@ -856,7 +853,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, btcutil.SatoshiPerBitcoin}: + case heuristic.moreChansResps <- moreChansResp{true, 1, btcutil.SatoshiPerBitcoin}: case <-time.After(time.Second * 10): t.Fatalf("need more chans wasn't queried in time") } diff --git a/autopilot/interface.go b/autopilot/interface.go index 3e7bef63..034045e8 100644 --- a/autopilot/interface.go +++ b/autopilot/interface.go @@ -111,17 +111,20 @@ type AttachmentHeuristic interface { // 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. - // - // TODO(roasbeef): return number of chans? ensure doesn't go over - NeedMoreChans(chans []Channel, balance btcutil.Amount) (btcutil.Amount, bool) + // 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) // Select is a method that given the current state of the channel // graph, a set of nodes to ignore, and an amount of available funds, // should return a set of attachment directives which describe which // additional channels should be opened within the graph to push the - // heuristic back towards its equilibrium state. - Select(self *btcec.PublicKey, graph ChannelGraph, amtToUse btcutil.Amount, + // heuristic back towards its equilibrium state. The numNewChans + // argument represents the additional number of channels that should be + // open. + Select(self *btcec.PublicKey, graph ChannelGraph, + amtToUse btcutil.Amount, numNewChans uint32, skipNodes map[NodeID]struct{}) ([]AttachmentDirective, error) } diff --git a/autopilot/prefattach.go b/autopilot/prefattach.go index 89046a22..53200d2c 100644 --- a/autopilot/prefattach.go +++ b/autopilot/prefattach.go @@ -58,14 +58,19 @@ var _ AttachmentHeuristic = (*ConstrainedPrefAttachment)(nil) // // NOTE: This is a part of the AttachmentHeuristic interface. func (p *ConstrainedPrefAttachment) NeedMoreChans(channels []Channel, - funds btcutil.Amount) (btcutil.Amount, bool) { + funds btcutil.Amount) (btcutil.Amount, uint32, bool) { // 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(p.chanLimit) { - return 0, false + return 0, 0, false } + // 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(p.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 @@ -86,14 +91,14 @@ func (p *ConstrainedPrefAttachment) NeedMoreChans(channels []Channel, // of channels to attempt to open. needMore := fundsFraction < p.threshold if !needMore { - return 0, false + return 0, 0, false } // 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) * p.threshold) fundsAvailable := targetAllocation - totalChanAllocation - return fundsAvailable, true + return fundsAvailable, numAdditionalChans, true } // NodeID is a simple type that holds a EC public key serialized in compressed @@ -137,7 +142,7 @@ func shuffleCandidates(candidates []Node) []Node { // // NOTE: This is a part of the AttachmentHeuristic interface. func (p *ConstrainedPrefAttachment) Select(self *btcec.PublicKey, g ChannelGraph, - fundsAvailable btcutil.Amount, + fundsAvailable btcutil.Amount, numNewChans uint32, skipNodes map[NodeID]struct{}) ([]AttachmentDirective, error) { // TODO(roasbeef): rename? @@ -151,8 +156,7 @@ func (p *ConstrainedPrefAttachment) Select(self *btcec.PublicKey, g ChannelGraph // We'll continue our attachment loop until we've exhausted the current // amount of available funds. visited := make(map[NodeID]struct{}) - chanLimit := p.chanLimit - uint16(len(skipNodes)) - for i := uint16(0); i < chanLimit; i++ { + for i := uint32(0); i < numNewChans; i++ { // selectionSlice will be used to randomly select a node // according to a power law distribution. For each connected // edge, we'll add an instance of the node to this slice. Thus, diff --git a/autopilot/prefattach_test.go b/autopilot/prefattach_test.go index 0af711c5..e1da5192 100644 --- a/autopilot/prefattach_test.go +++ b/autopilot/prefattach_test.go @@ -38,6 +38,7 @@ func TestConstrainedPrefAttachmentNeedMoreChan(t *testing.T) { needMore bool amtAvailable btcutil.Amount + numMore uint32 }{ // Many available funds, but already have too many active open // channels. @@ -59,6 +60,7 @@ func TestConstrainedPrefAttachmentNeedMoreChan(t *testing.T) { btcutil.Amount(btcutil.SatoshiPerBitcoin * 10), false, 0, + 0, }, // Ratio of funds in channels and total funds meets the @@ -77,13 +79,15 @@ func TestConstrainedPrefAttachmentNeedMoreChan(t *testing.T) { 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. + // recommended. We should also request 2 more channels as the + // limit is 3. { []Channel{ { @@ -94,14 +98,16 @@ func TestConstrainedPrefAttachmentNeedMoreChan(t *testing.T) { 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. + // 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{ { @@ -116,6 +122,7 @@ func TestConstrainedPrefAttachmentNeedMoreChan(t *testing.T) { btcutil.Amount(btcutil.SatoshiPerBitcoin * 10), true, btcutil.Amount(btcutil.SatoshiPerBitcoin * 3), + 1, }, // Ratio of funds in channels and total funds is above the @@ -134,6 +141,7 @@ func TestConstrainedPrefAttachmentNeedMoreChan(t *testing.T) { btcutil.Amount(btcutil.SatoshiPerBitcoin), false, 0, + 0, }, } @@ -141,8 +149,9 @@ func TestConstrainedPrefAttachmentNeedMoreChan(t *testing.T) { chanLimit, threshold) for i, testCase := range testCases { - amtToAllocate, needMore := prefAttach.NeedMoreChans(testCase.channels, - testCase.walletAmt) + amtToAllocate, numMore, needMore := prefAttach.NeedMoreChans( + testCase.channels, testCase.walletAmt, + ) if amtToAllocate != testCase.amtAvailable { t.Fatalf("test #%v: expected %v, got %v", @@ -152,6 +161,10 @@ func TestConstrainedPrefAttachmentNeedMoreChan(t *testing.T) { 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) + } } } @@ -247,7 +260,7 @@ func TestConstrainedPrefAttachmentSelectEmptyGraph(t *testing.T) { // creation given the current state of the graph. const walletFunds = btcutil.SatoshiPerBitcoin directives, err := prefAttach.Select(self, graph, - walletFunds, skipNodes) + walletFunds, 5, skipNodes) if err != nil { t1.Fatalf("unable to select attachment "+ "directives: %v", err) @@ -316,7 +329,7 @@ func TestConstrainedPrefAttachmentSelectTwoVertexes(t *testing.T) { // creation given the current state of the graph. const walletFunds = btcutil.SatoshiPerBitcoin * 10 directives, err := prefAttach.Select(self, graph, - walletFunds, skipNodes) + walletFunds, 2, skipNodes) if err != nil { t1.Fatalf("unable to select attachment directives: %v", err) } @@ -395,7 +408,7 @@ func TestConstrainedPrefAttachmentSelectInsufficientFunds(t *testing.T) { // passing zero for the amount of wallet funds. This // should return an empty slice of directives. directives, err := prefAttach.Select(self, graph, 0, - skipNodes) + 0, skipNodes) if err != nil { t1.Fatalf("unable to select attachment "+ "directives: %v", err) @@ -503,7 +516,7 @@ func TestConstrainedPrefAttachmentSelectGreedyAllocation(t *testing.T) { // allocate funds to channels. const availableBalance = btcutil.SatoshiPerBitcoin * 2.5 directives, err := prefAttach.Select(self, graph, - availableBalance, skipNodes) + availableBalance, 5, skipNodes) if err != nil { t1.Fatalf("unable to select attachment "+ "directives: %v", err) @@ -594,7 +607,7 @@ func TestConstrainedPrefAttachmentSelectSkipNodes(t *testing.T) { // candidates. const availableBalance = btcutil.SatoshiPerBitcoin * 2.5 directives, err := prefAttach.Select(self, graph, - availableBalance, skipNodes) + availableBalance, 2, skipNodes) if err != nil { t1.Fatalf("unable to select attachment "+ "directives: %v", err) @@ -618,7 +631,7 @@ func TestConstrainedPrefAttachmentSelectSkipNodes(t *testing.T) { // should get no new directives as both nodes has // already been attached to. directives, err = prefAttach.Select(self, graph, - availableBalance, skipNodes) + availableBalance, 2, skipNodes) if err != nil { t1.Fatalf("unable to select attachment "+ "directives: %v", err)