diff --git a/autopilot/agent.go b/autopilot/agent.go index 8e625c14..2fe123c9 100644 --- a/autopilot/agent.go +++ b/autopilot/agent.go @@ -584,6 +584,7 @@ func (a *Agent) openChans(availableFunds btcutil.Amount, numChans uint32, // Use the heuristic to calculate a score for each node in the // graph. + log.Debugf("Scoring %d nodes for chan_size=%v", len(nodes), chanSize) scores, err := a.cfg.Heuristic.NodeScores( a.cfg.Graph, totalChans, chanSize, nodes, ) diff --git a/autopilot/agent_test.go b/autopilot/agent_test.go index 92b2b358..bd703c9e 100644 --- a/autopilot/agent_test.go +++ b/autopilot/agent_test.go @@ -1196,7 +1196,7 @@ func TestAgentChannelSizeAllocation(t *testing.T) { t.Parallel() // Total number of nodes in our mock graph. - const numNodes = 10 + const numNodes = 20 testCtx, cleanup := setup(t, nil) defer cleanup() @@ -1240,6 +1240,8 @@ func TestAgentChannelSizeAllocation(t *testing.T) { numNewChannels, nodeScores, ) + // We expect the autopilot to have allocated all funds towards + // channels. expectedAllocation := testCtx.constraints.MaxChanSize() * btcutil.Amount(numNewChannels) nodes := checkChannelOpens( t, testCtx, expectedAllocation, numNewChannels, @@ -1259,34 +1261,40 @@ func TestAgentChannelSizeAllocation(t *testing.T) { waitForNumChans := func(expChans int) { t.Helper() + var ( + numChans int + balance btcutil.Amount + ) + Loop: for { select { case arg := <-testCtx.constraints.moreChanArgs: + numChans = len(arg.chans) + balance = arg.balance + // As long as the number of existing channels // is below our expected number of channels, - // we'll keep responding with "no more - // channels". - if len(arg.chans) != expChans { - select { - case testCtx.constraints.moreChansResps <- moreChansResp{0, 0}: - case <-time.After(time.Second * 3): - t.Fatalf("heuristic wasn't " + - "queried in time") - } - continue + // and the balance is not what we expect, we'll + // keep responding with "no more channels". + if numChans == expChans && + balance == testCtx.walletBalance { + break Loop } - if arg.balance != testCtx.walletBalance { - t.Fatalf("expectd agent to have %v "+ - "balance, had %v", - testCtx.walletBalance, - arg.balance) + select { + case testCtx.constraints.moreChansResps <- moreChansResp{0, 0}: + case <-time.After(time.Second * 3): + t.Fatalf("heuristic wasn't queried " + + "in time") } - break Loop case <-time.After(time.Second * 3): - t.Fatalf("heuristic wasn't queried in time") + t.Fatalf("did not receive expected "+ + "channels(%d) and balance(%d), "+ + "instead got %d and %d", expChans, + testCtx.walletBalance, numChans, + balance) } } } @@ -1308,5 +1316,30 @@ func TestAgentChannelSizeAllocation(t *testing.T) { // To stay within the budget, we expect the autopilot to open 2 // channels. - checkChannelOpens(t, testCtx, channelBudget, 2) + expectedAllocation = channelBudget + nodes = checkChannelOpens(t, testCtx, expectedAllocation, 2) + numExistingChannels = 7 + + for _, node := range nodes { + delete(nodeScores, node) + } + + waitForNumChans(numExistingChannels) + + // Finally check that we make maximum channels if we are well within + // our budget. + channelBudget = btcutil.SatoshiPerBitcoin * 5 + numNewChannels = 2 + respondWithScores( + t, testCtx, channelBudget, numExistingChannels, + numNewChannels, nodeScores, + ) + + // We now expect the autopilot to open 2 channels, and since it has + // more than enough balance within the budget, they should both be of + // maximum size. + expectedAllocation = testCtx.constraints.MaxChanSize() * + btcutil.Amount(numNewChannels) + + checkChannelOpens(t, testCtx, expectedAllocation, numNewChannels) } diff --git a/autopilot/interface.go b/autopilot/interface.go index 34ef7597..b63661e3 100644 --- a/autopilot/interface.go +++ b/autopilot/interface.go @@ -188,10 +188,11 @@ func init() { // open a channel within the graph to a target peer, close targeted channels, // or add/remove funds from existing channels via a splice in/out mechanisms. type ChannelController interface { - // OpenChannel opens a channel to a target peer, with a capacity of the - // specified amount. This function should un-block immediately after - // the funding transaction that marks the channel open has been - // broadcast. + // OpenChannel opens a channel to a target peer, using at most amt + // funds. This means that the resulting channel capacity might be + // slightly less to account for fees. This function should un-block + // immediately after the funding transaction that marks the channel + // open has been broadcast. OpenChannel(target *btcec.PublicKey, amt btcutil.Amount) error // CloseChannel attempts to close out the target channel. diff --git a/fundingmanager.go b/fundingmanager.go index 3f67db0c..0ede9720 100644 --- a/fundingmanager.go +++ b/fundingmanager.go @@ -337,6 +337,14 @@ type fundingConfig struct { // due to fees. MinChanSize btcutil.Amount + // MaxPendingChannels is the maximum number of pending channels we + // allow for each peer. + MaxPendingChannels int + + // RejectPush is set true if the fundingmanager should reject any + // incoming channels having a non-zero push amount. + RejectPush bool + // NotifyOpenChannelEvent informs the ChannelNotifier when channels // transition from pending open to open. NotifyOpenChannelEvent func(wire.OutPoint) @@ -1012,7 +1020,7 @@ func (f *fundingManager) handleFundingOpen(fmsg *fundingOpenMsg) { // TODO(roasbeef): modify to only accept a _single_ pending channel per // block unless white listed - if numPending >= cfg.MaxPendingChannels { + if numPending >= f.cfg.MaxPendingChannels { f.failFundingFlow( fmsg.peer, fmsg.msg.PendingChannelID, lnwire.ErrMaxPendingChannels, @@ -1057,7 +1065,7 @@ func (f *fundingManager) handleFundingOpen(fmsg *fundingOpenMsg) { // If request specifies non-zero push amount and 'rejectpush' is set, // signal an error. - if cfg.RejectPush && msg.PushAmount > 0 { + if f.cfg.RejectPush && msg.PushAmount > 0 { f.failFundingFlow( fmsg.peer, fmsg.msg.PendingChannelID, lnwallet.ErrNonZeroPushAmount()) @@ -1076,16 +1084,16 @@ func (f *fundingManager) handleFundingOpen(fmsg *fundingOpenMsg) { // funds to the channel ourselves. chainHash := chainhash.Hash(msg.ChainHash) req := &lnwallet.InitFundingReserveMsg{ - ChainHash: &chainHash, - NodeID: fmsg.peer.IdentityKey(), - NodeAddr: fmsg.peer.Address(), - FundingAmount: 0, - Capacity: amt, - CommitFeePerKw: lnwallet.SatPerKWeight(msg.FeePerKiloWeight), - FundingFeePerKw: 0, - PushMSat: msg.PushAmount, - Flags: msg.ChannelFlags, - MinConfs: 1, + ChainHash: &chainHash, + NodeID: fmsg.peer.IdentityKey(), + NodeAddr: fmsg.peer.Address(), + LocalFundingAmt: 0, + RemoteFundingAmt: amt, + CommitFeePerKw: lnwallet.SatPerKWeight(msg.FeePerKiloWeight), + FundingFeePerKw: 0, + PushMSat: msg.PushAmount, + Flags: msg.ChannelFlags, + MinConfs: 1, } reservation, err := f.cfg.Wallet.InitChannelReservation(req) @@ -1714,8 +1722,8 @@ func (f *fundingManager) handleFundingSigned(fmsg *fundingSignedMsg) { return case shortChanID, ok = <-confChan: if !ok { - fndgLog.Errorf("waiting for funding confirmation" + - " failed") + fndgLog.Errorf("waiting for funding " + + "confirmation failed") return } } @@ -1885,8 +1893,9 @@ func makeFundingScript(channel *channeldb.OpenChannel) ([]byte, error) { // when a channel has become active for lightning transactions. // The wait can be canceled by closing the cancelChan. In case of success, // a *lnwire.ShortChannelID will be passed to confChan. -func (f *fundingManager) waitForFundingConfirmation(completeChan *channeldb.OpenChannel, - cancelChan <-chan struct{}, confChan chan<- *lnwire.ShortChannelID) { +func (f *fundingManager) waitForFundingConfirmation( + completeChan *channeldb.OpenChannel, cancelChan <-chan struct{}, + confChan chan<- *lnwire.ShortChannelID) { defer close(confChan) @@ -1896,16 +1905,19 @@ func (f *fundingManager) waitForFundingConfirmation(completeChan *channeldb.Open fundingScript, err := makeFundingScript(completeChan) if err != nil { fndgLog.Errorf("unable to create funding script for "+ - "ChannelPoint(%v): %v", completeChan.FundingOutpoint, err) + "ChannelPoint(%v): %v", completeChan.FundingOutpoint, + err) return } numConfs := uint32(completeChan.NumConfsRequired) confNtfn, err := f.cfg.Notifier.RegisterConfirmationsNtfn( - &txid, fundingScript, numConfs, completeChan.FundingBroadcastHeight, + &txid, fundingScript, numConfs, + completeChan.FundingBroadcastHeight, ) if err != nil { fndgLog.Errorf("Unable to register for confirmation of "+ - "ChannelPoint(%v): %v", completeChan.FundingOutpoint, err) + "ChannelPoint(%v): %v", completeChan.FundingOutpoint, + err) return } @@ -1920,14 +1932,17 @@ func (f *fundingManager) waitForFundingConfirmation(completeChan *channeldb.Open select { case confDetails, ok = <-confNtfn.Confirmed: // fallthrough + case <-cancelChan: fndgLog.Warnf("canceled waiting for funding confirmation, "+ "stopping funding flow for ChannelPoint(%v)", completeChan.FundingOutpoint) return + case <-f.quit: fndgLog.Warnf("fundingManager shutting down, stopping funding "+ - "flow for ChannelPoint(%v)", completeChan.FundingOutpoint) + "flow for ChannelPoint(%v)", + completeChan.FundingOutpoint) return } @@ -1940,6 +1955,18 @@ func (f *fundingManager) waitForFundingConfirmation(completeChan *channeldb.Open fundingPoint := completeChan.FundingOutpoint chanID := lnwire.NewChanIDFromOutPoint(&fundingPoint) + if int(fundingPoint.Index) >= len(confDetails.Tx.TxOut) { + fndgLog.Warnf("Funding point index does not exist for "+ + "ChannelPoint(%v)", completeChan.FundingOutpoint) + return + } + + outputAmt := btcutil.Amount(confDetails.Tx.TxOut[fundingPoint.Index].Value) + if outputAmt != completeChan.Capacity { + fndgLog.Warnf("Invalid output value for ChannelPoint(%v)", + completeChan.FundingOutpoint) + return + } fndgLog.Infof("ChannelPoint(%v) is now active: ChannelID(%x)", fundingPoint, chanID[:]) @@ -2734,8 +2761,6 @@ func (f *fundingManager) handleInitFundingMsg(msg *initFundingMsg) { var ( peerKey = msg.peer.IdentityKey() localAmt = msg.localFundingAmt - remoteAmt = msg.remoteFundingAmt - capacity = localAmt + remoteAmt minHtlc = msg.minHtlc remoteCsvDelay = msg.remoteCsvDelay ) @@ -2749,10 +2774,11 @@ func (f *fundingManager) handleInitFundingMsg(msg *initFundingMsg) { ourDustLimit = defaultLitecoinDustLimit } - fndgLog.Infof("Initiating fundingRequest(localAmt=%v, remoteAmt=%v, "+ - "capacity=%v, chainhash=%v, peer=%x, dustLimit=%v, min_confs=%v)", - localAmt, msg.pushAmt, capacity, msg.chainHash, - peerKey.SerializeCompressed(), ourDustLimit, msg.minConfs) + fndgLog.Infof("Initiating fundingRequest(local_amt=%v "+ + "(subtract_fees=%v), push_amt=%v, chain_hash=%v, peer=%x, "+ + "dust_limit=%v, min_confs=%v)", localAmt, msg.subtractFees, + msg.pushAmt, msg.chainHash, peerKey.SerializeCompressed(), + ourDustLimit, msg.minConfs) // First, we'll query the fee estimator for a fee that should get the // commitment transaction confirmed by the next few blocks (conf target @@ -2776,16 +2802,17 @@ func (f *fundingManager) handleInitFundingMsg(msg *initFundingMsg) { // wallet doesn't have enough funds to commit to this channel, then the // request will fail, and be aborted. req := &lnwallet.InitFundingReserveMsg{ - ChainHash: &msg.chainHash, - NodeID: peerKey, - NodeAddr: msg.peer.Address(), - FundingAmount: localAmt, - Capacity: capacity, - CommitFeePerKw: commitFeePerKw, - FundingFeePerKw: msg.fundingFeePerKw, - PushMSat: msg.pushAmt, - Flags: channelFlags, - MinConfs: msg.minConfs, + ChainHash: &msg.chainHash, + NodeID: peerKey, + NodeAddr: msg.peer.Address(), + SubtractFees: msg.subtractFees, + LocalFundingAmt: localAmt, + RemoteFundingAmt: 0, + CommitFeePerKw: commitFeePerKw, + FundingFeePerKw: msg.fundingFeePerKw, + PushMSat: msg.pushAmt, + Flags: channelFlags, + MinConfs: msg.minConfs, } reservation, err := f.cfg.Wallet.InitChannelReservation(req) @@ -2794,6 +2821,12 @@ func (f *fundingManager) handleInitFundingMsg(msg *initFundingMsg) { return } + // Now that we have successfully reserved funds for this channel in the + // wallet, we can fetch the final channel capacity. This is done at + // this point since the final capacity might change in case of + // SubtractFees=true. + capacity := reservation.Capacity() + // Obtain a new pending channel ID which is used to track this // reservation throughout its lifetime. chanID := f.nextPendingChanID() diff --git a/fundingmanager_test.go b/fundingmanager_test.go index 891f5eab..d90f691c 100644 --- a/fundingmanager_test.go +++ b/fundingmanager_test.go @@ -236,7 +236,8 @@ func createTestWallet(cdb *channeldb.DB, netParams *chaincfg.Params, } func createTestFundingManager(t *testing.T, privKey *btcec.PrivateKey, - addr *lnwire.NetAddress, tempTestDir string) (*testNode, error) { + addr *lnwire.NetAddress, tempTestDir string, + options ...cfgOption) (*testNode, error) { netParams := activeNetParams.Params estimator := lnwallet.NewStaticFeeEstimator(62500, 0) @@ -282,7 +283,7 @@ func createTestFundingManager(t *testing.T, privKey *btcec.PrivateKey, var chanIDSeed [32]byte - f, err := newFundingManager(fundingConfig{ + fundingCfg := fundingConfig{ IDKey: privKey.PubKey(), Wallet: lnw, Notifier: chainNotifier, @@ -363,8 +364,15 @@ func createTestFundingManager(t *testing.T, privKey *btcec.PrivateKey, }, ZombieSweeperInterval: 1 * time.Hour, ReservationTimeout: 1 * time.Nanosecond, + MaxPendingChannels: DefaultMaxPendingChannels, NotifyOpenChannelEvent: func(wire.OutPoint) {}, - }) + } + + for _, op := range options { + op(&fundingCfg) + } + + f, err := newFundingManager(fundingCfg) if err != nil { t.Fatalf("failed creating fundingManager: %v", err) } @@ -468,12 +476,10 @@ func recreateAliceFundingManager(t *testing.T, alice *testNode) { } } -func setupFundingManagers(t *testing.T, maxPendingChannels int) (*testNode, *testNode) { - // We need to set the global config, as fundingManager uses - // MaxPendingChannels, and it is usually set in lndMain(). - cfg = &config{ - MaxPendingChannels: maxPendingChannels, - } +type cfgOption func(*fundingConfig) + +func setupFundingManagers(t *testing.T, + options ...cfgOption) (*testNode, *testNode) { aliceTestDir, err := ioutil.TempDir("", "alicelnwallet") if err != nil { @@ -481,7 +487,7 @@ func setupFundingManagers(t *testing.T, maxPendingChannels int) (*testNode, *tes } alice, err := createTestFundingManager( - t, alicePrivKey, aliceAddr, aliceTestDir, + t, alicePrivKey, aliceAddr, aliceTestDir, options..., ) if err != nil { t.Fatalf("failed creating fundingManager: %v", err) @@ -492,7 +498,9 @@ func setupFundingManagers(t *testing.T, maxPendingChannels int) (*testNode, *tes t.Fatalf("unable to create temp directory: %v", err) } - bob, err := createTestFundingManager(t, bobPrivKey, bobAddr, bobTestDir) + bob, err := createTestFundingManager( + t, bobPrivKey, bobAddr, bobTestDir, options..., + ) if err != nil { t.Fatalf("failed creating fundingManager: %v", err) } @@ -544,14 +552,35 @@ func tearDownFundingManagers(t *testing.T, a, b *testNode) { // transaction is confirmed on-chain. Returns the funding out point. func openChannel(t *testing.T, alice, bob *testNode, localFundingAmt, pushAmt btcutil.Amount, numConfs uint32, - updateChan chan *lnrpc.OpenStatusUpdate, announceChan bool) *wire.OutPoint { + updateChan chan *lnrpc.OpenStatusUpdate, announceChan bool) ( + *wire.OutPoint, *wire.MsgTx) { + + publ := fundChannel( + t, alice, bob, localFundingAmt, pushAmt, false, numConfs, + updateChan, announceChan, + ) + fundingOutPoint := &wire.OutPoint{ + Hash: publ.TxHash(), + Index: 0, + } + return fundingOutPoint, publ +} + +// fundChannel takes the funding process to the point where the funding +// transaction is confirmed on-chain. Returns the funding tx. +func fundChannel(t *testing.T, alice, bob *testNode, localFundingAmt, + pushAmt btcutil.Amount, subtractFees bool, numConfs uint32, + updateChan chan *lnrpc.OpenStatusUpdate, announceChan bool) *wire.MsgTx { + // Create a funding request and start the workflow. errChan := make(chan error, 1) initReq := &openChanReq{ targetPubkey: bob.privKey.PubKey(), chainHash: *activeNetParams.GenesisHash, + subtractFees: subtractFees, localFundingAmt: localFundingAmt, pushAmt: lnwire.NewMSatFromSatoshis(pushAmt), + fundingFeePerKw: 1000, private: !announceChan, updates: updateChan, err: errChan, @@ -636,17 +665,12 @@ func openChannel(t *testing.T, alice, bob *testNode, localFundingAmt, t.Fatalf("alice did not publish funding tx") } - fundingOutPoint := &wire.OutPoint{ - Hash: publ.TxHash(), - Index: 0, - } - // Finally, make sure neither have active reservation for the channel // now pending open in the database. assertNumPendingReservations(t, alice, bobPubKey, 0) assertNumPendingReservations(t, bob, alicePubKey, 0) - return fundingOutPoint + return publ } func assertErrorNotSent(t *testing.T, msgChan chan lnwire.Message) { @@ -674,6 +698,8 @@ func assertErrorSent(t *testing.T, msgChan chan lnwire.Message) { func assertFundingMsgSent(t *testing.T, msgChan chan lnwire.Message, msgType string) lnwire.Message { + t.Helper() + var msg lnwire.Message select { case msg = <-msgChan: @@ -1029,7 +1055,9 @@ func assertHandleFundingLocked(t *testing.T, alice, bob *testNode) { } func TestFundingManagerNormalWorkflow(t *testing.T) { - alice, bob := setupFundingManagers(t, DefaultMaxPendingChannels) + t.Parallel() + + alice, bob := setupFundingManagers(t) defer tearDownFundingManagers(t, alice, bob) // We will consume the channel updates as we go, so no buffering is needed. @@ -1040,16 +1068,21 @@ func TestFundingManagerNormalWorkflow(t *testing.T) { localAmt := btcutil.Amount(500000) pushAmt := btcutil.Amount(0) capacity := localAmt + pushAmt - fundingOutPoint := openChannel(t, alice, bob, localAmt, pushAmt, 1, - updateChan, true) + fundingOutPoint, fundingTx := openChannel( + t, alice, bob, localAmt, pushAmt, 1, updateChan, true, + ) // Check that neither Alice nor Bob sent an error message. assertErrorNotSent(t, alice.msgChan) assertErrorNotSent(t, bob.msgChan) // Notify that transaction was mined. - alice.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{} - bob.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{} + alice.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } + bob.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } // The funding transaction was mined, so assert that both funding // managers now have the state of this channel 'markedOpen' in their @@ -1090,8 +1123,12 @@ func TestFundingManagerNormalWorkflow(t *testing.T) { assertHandleFundingLocked(t, alice, bob) // Notify that six confirmations has been reached on funding transaction. - alice.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{} - bob.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{} + alice.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } + bob.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } // Make sure the fundingManagers exchange announcement signatures. assertAnnouncementSignatures(t, alice, bob) @@ -1102,7 +1139,9 @@ func TestFundingManagerNormalWorkflow(t *testing.T) { } func TestFundingManagerRestartBehavior(t *testing.T) { - alice, bob := setupFundingManagers(t, DefaultMaxPendingChannels) + t.Parallel() + + alice, bob := setupFundingManagers(t) defer tearDownFundingManagers(t, alice, bob) // Run through the process of opening the channel, up until the funding @@ -1111,8 +1150,9 @@ func TestFundingManagerRestartBehavior(t *testing.T) { pushAmt := btcutil.Amount(0) capacity := localAmt + pushAmt updateChan := make(chan *lnrpc.OpenStatusUpdate) - fundingOutPoint := openChannel(t, alice, bob, localAmt, pushAmt, 1, - updateChan, true) + fundingOutPoint, fundingTx := openChannel( + t, alice, bob, localAmt, pushAmt, 1, updateChan, true, + ) // After the funding transaction gets mined, both nodes will send the // fundingLocked message to the other peer. If the funding node fails @@ -1133,8 +1173,12 @@ func TestFundingManagerRestartBehavior(t *testing.T) { } // Notify that transaction was mined - alice.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{} - bob.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{} + alice.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } + bob.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } // The funding transaction was mined, so assert that both funding // managers now have the state of this channel 'markedOpen' in their @@ -1225,8 +1269,12 @@ func TestFundingManagerRestartBehavior(t *testing.T) { time.Sleep(300 * time.Millisecond) // Notify that six confirmations has been reached on funding transaction. - alice.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{} - bob.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{} + alice.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } + bob.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } // Make sure the fundingManagers exchange announcement signatures. assertAnnouncementSignatures(t, alice, bob) @@ -1240,7 +1288,9 @@ func TestFundingManagerRestartBehavior(t *testing.T) { // server to notify when the peer comes online, in case sending the // fundingLocked message fails the first time. func TestFundingManagerOfflinePeer(t *testing.T) { - alice, bob := setupFundingManagers(t, DefaultMaxPendingChannels) + t.Parallel() + + alice, bob := setupFundingManagers(t) defer tearDownFundingManagers(t, alice, bob) // Run through the process of opening the channel, up until the funding @@ -1249,8 +1299,9 @@ func TestFundingManagerOfflinePeer(t *testing.T) { pushAmt := btcutil.Amount(0) capacity := localAmt + pushAmt updateChan := make(chan *lnrpc.OpenStatusUpdate) - fundingOutPoint := openChannel(t, alice, bob, localAmt, pushAmt, 1, - updateChan, true) + fundingOutPoint, fundingTx := openChannel( + t, alice, bob, localAmt, pushAmt, 1, updateChan, true, + ) // After the funding transaction gets mined, both nodes will send the // fundingLocked message to the other peer. If the funding node fails @@ -1272,8 +1323,12 @@ func TestFundingManagerOfflinePeer(t *testing.T) { } // Notify that transaction was mined - alice.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{} - bob.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{} + alice.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } + bob.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } // The funding transaction was mined, so assert that both funding // managers now have the state of this channel 'markedOpen' in their @@ -1358,8 +1413,12 @@ func TestFundingManagerOfflinePeer(t *testing.T) { assertHandleFundingLocked(t, alice, bob) // Notify that six confirmations has been reached on funding transaction. - alice.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{} - bob.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{} + alice.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } + bob.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } // Make sure both fundingManagers send the expected announcement // signatures. @@ -1374,7 +1433,9 @@ func TestFundingManagerOfflinePeer(t *testing.T) { // will properly clean up a zombie reservation that times out after the // initFundingMsg has been handled. func TestFundingManagerPeerTimeoutAfterInitFunding(t *testing.T) { - alice, bob := setupFundingManagers(t, DefaultMaxPendingChannels) + t.Parallel() + + alice, bob := setupFundingManagers(t) defer tearDownFundingManagers(t, alice, bob) // We will consume the channel updates as we go, so no buffering is needed. @@ -1434,7 +1495,9 @@ func TestFundingManagerPeerTimeoutAfterInitFunding(t *testing.T) { // will properly clean up a zombie reservation that times out after the // fundingOpenMsg has been handled. func TestFundingManagerPeerTimeoutAfterFundingOpen(t *testing.T) { - alice, bob := setupFundingManagers(t, DefaultMaxPendingChannels) + t.Parallel() + + alice, bob := setupFundingManagers(t) defer tearDownFundingManagers(t, alice, bob) // We will consume the channel updates as we go, so no buffering is needed. @@ -1503,7 +1566,9 @@ func TestFundingManagerPeerTimeoutAfterFundingOpen(t *testing.T) { // will properly clean up a zombie reservation that times out after the // fundingAcceptMsg has been handled. func TestFundingManagerPeerTimeoutAfterFundingAccept(t *testing.T) { - alice, bob := setupFundingManagers(t, DefaultMaxPendingChannels) + t.Parallel() + + alice, bob := setupFundingManagers(t) defer tearDownFundingManagers(t, alice, bob) // We will consume the channel updates as we go, so no buffering is needed. @@ -1577,7 +1642,9 @@ func TestFundingManagerPeerTimeoutAfterFundingAccept(t *testing.T) { } func TestFundingManagerFundingTimeout(t *testing.T) { - alice, bob := setupFundingManagers(t, DefaultMaxPendingChannels) + t.Parallel() + + alice, bob := setupFundingManagers(t) defer tearDownFundingManagers(t, alice, bob) // We will consume the channel updates as we go, so no buffering is needed. @@ -1585,7 +1652,7 @@ func TestFundingManagerFundingTimeout(t *testing.T) { // Run through the process of opening the channel, up until the funding // transaction is broadcasted. - _ = openChannel(t, alice, bob, 500000, 0, 1, updateChan, true) + _, _ = openChannel(t, alice, bob, 500000, 0, 1, updateChan, true) // Bob will at this point be waiting for the funding transaction to be // confirmed, so the channel should be considered pending. @@ -1621,8 +1688,9 @@ func TestFundingManagerFundingTimeout(t *testing.T) { // TestFundingManagerFundingNotTimeoutInitiator checks that if the user was // the channel initiator, that it does not timeout when the lnd restarts. func TestFundingManagerFundingNotTimeoutInitiator(t *testing.T) { + t.Parallel() - alice, bob := setupFundingManagers(t, DefaultMaxPendingChannels) + alice, bob := setupFundingManagers(t) defer tearDownFundingManagers(t, alice, bob) // We will consume the channel updates as we go, so no buffering is needed. @@ -1630,7 +1698,7 @@ func TestFundingManagerFundingNotTimeoutInitiator(t *testing.T) { // Run through the process of opening the channel, up until the funding // transaction is broadcasted. - _ = openChannel(t, alice, bob, 500000, 0, 1, updateChan, true) + _, _ = openChannel(t, alice, bob, 500000, 0, 1, updateChan, true) // Alice will at this point be waiting for the funding transaction to be // confirmed, so the channel should be considered pending. @@ -1689,7 +1757,9 @@ func TestFundingManagerFundingNotTimeoutInitiator(t *testing.T) { // continues to operate as expected in case we receive a duplicate fundingLocked // message. func TestFundingManagerReceiveFundingLockedTwice(t *testing.T) { - alice, bob := setupFundingManagers(t, DefaultMaxPendingChannels) + t.Parallel() + + alice, bob := setupFundingManagers(t) defer tearDownFundingManagers(t, alice, bob) // We will consume the channel updates as we go, so no buffering is needed. @@ -1700,12 +1770,17 @@ func TestFundingManagerReceiveFundingLockedTwice(t *testing.T) { localAmt := btcutil.Amount(500000) pushAmt := btcutil.Amount(0) capacity := localAmt + pushAmt - fundingOutPoint := openChannel(t, alice, bob, localAmt, pushAmt, 1, - updateChan, true) + fundingOutPoint, fundingTx := openChannel( + t, alice, bob, localAmt, pushAmt, 1, updateChan, true, + ) // Notify that transaction was mined - alice.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{} - bob.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{} + alice.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } + bob.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } // The funding transaction was mined, so assert that both funding // managers now have the state of this channel 'markedOpen' in their @@ -1766,8 +1841,12 @@ func TestFundingManagerReceiveFundingLockedTwice(t *testing.T) { } // Notify that six confirmations has been reached on funding transaction. - alice.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{} - bob.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{} + alice.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } + bob.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } // Make sure the fundingManagers exchange announcement signatures. assertAnnouncementSignatures(t, alice, bob) @@ -1781,7 +1860,9 @@ func TestFundingManagerReceiveFundingLockedTwice(t *testing.T) { // handles receiving a fundingLocked after the its own fundingLocked and channel // announcement is sent and gets restarted. func TestFundingManagerRestartAfterChanAnn(t *testing.T) { - alice, bob := setupFundingManagers(t, DefaultMaxPendingChannels) + t.Parallel() + + alice, bob := setupFundingManagers(t) defer tearDownFundingManagers(t, alice, bob) // We will consume the channel updates as we go, so no buffering is needed. @@ -1792,12 +1873,17 @@ func TestFundingManagerRestartAfterChanAnn(t *testing.T) { localAmt := btcutil.Amount(500000) pushAmt := btcutil.Amount(0) capacity := localAmt + pushAmt - fundingOutPoint := openChannel(t, alice, bob, localAmt, pushAmt, 1, - updateChan, true) + fundingOutPoint, fundingTx := openChannel( + t, alice, bob, localAmt, pushAmt, 1, updateChan, true, + ) // Notify that transaction was mined - alice.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{} - bob.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{} + alice.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } + bob.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } // The funding transaction was mined, so assert that both funding // managers now have the state of this channel 'markedOpen' in their @@ -1843,8 +1929,12 @@ func TestFundingManagerRestartAfterChanAnn(t *testing.T) { assertHandleFundingLocked(t, alice, bob) // Notify that six confirmations has been reached on funding transaction. - alice.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{} - bob.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{} + alice.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } + bob.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } // Make sure both fundingManagers send the expected channel announcements. assertAnnouncementSignatures(t, alice, bob) @@ -1858,7 +1948,9 @@ func TestFundingManagerRestartAfterChanAnn(t *testing.T) { // fundingManager continues to operate as expected after it has received // fundingLocked and then gets restarted. func TestFundingManagerRestartAfterReceivingFundingLocked(t *testing.T) { - alice, bob := setupFundingManagers(t, DefaultMaxPendingChannels) + t.Parallel() + + alice, bob := setupFundingManagers(t) defer tearDownFundingManagers(t, alice, bob) // We will consume the channel updates as we go, so no buffering is needed. @@ -1869,12 +1961,17 @@ func TestFundingManagerRestartAfterReceivingFundingLocked(t *testing.T) { localAmt := btcutil.Amount(500000) pushAmt := btcutil.Amount(0) capacity := localAmt + pushAmt - fundingOutPoint := openChannel(t, alice, bob, localAmt, pushAmt, 1, - updateChan, true) + fundingOutPoint, fundingTx := openChannel( + t, alice, bob, localAmt, pushAmt, 1, updateChan, true, + ) // Notify that transaction was mined - alice.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{} - bob.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{} + alice.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } + bob.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } // The funding transaction was mined, so assert that both funding // managers now have the state of this channel 'markedOpen' in their @@ -1916,8 +2013,12 @@ func TestFundingManagerRestartAfterReceivingFundingLocked(t *testing.T) { assertAddedToRouterGraph(t, alice, bob, fundingOutPoint) // Notify that six confirmations has been reached on funding transaction. - alice.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{} - bob.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{} + alice.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } + bob.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } // Make sure both fundingManagers send the expected channel announcements. assertAnnouncementSignatures(t, alice, bob) @@ -1931,7 +2032,9 @@ func TestFundingManagerRestartAfterReceivingFundingLocked(t *testing.T) { // (a channel not supposed to be announced to the rest of the network), // the announcementSignatures nor the nodeAnnouncement messages are sent. func TestFundingManagerPrivateChannel(t *testing.T) { - alice, bob := setupFundingManagers(t, DefaultMaxPendingChannels) + t.Parallel() + + alice, bob := setupFundingManagers(t) defer tearDownFundingManagers(t, alice, bob) // We will consume the channel updates as we go, so no buffering is needed. @@ -1942,12 +2045,17 @@ func TestFundingManagerPrivateChannel(t *testing.T) { localAmt := btcutil.Amount(500000) pushAmt := btcutil.Amount(0) capacity := localAmt + pushAmt - fundingOutPoint := openChannel(t, alice, bob, localAmt, pushAmt, 1, - updateChan, false) + fundingOutPoint, fundingTx := openChannel( + t, alice, bob, localAmt, pushAmt, 1, updateChan, false, + ) // Notify that transaction was mined - alice.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{} - bob.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{} + alice.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } + bob.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } // The funding transaction was mined, so assert that both funding // managers now have the state of this channel 'markedOpen' in their @@ -1985,8 +2093,12 @@ func TestFundingManagerPrivateChannel(t *testing.T) { assertHandleFundingLocked(t, alice, bob) // Notify that six confirmations has been reached on funding transaction. - alice.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{} - bob.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{} + alice.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } + bob.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } // Since this is a private channel, we shouldn't receive the // announcement signatures. @@ -2033,7 +2145,9 @@ func TestFundingManagerPrivateChannel(t *testing.T) { // announcement signatures nor the node announcement messages are sent upon // restart. func TestFundingManagerPrivateRestart(t *testing.T) { - alice, bob := setupFundingManagers(t, DefaultMaxPendingChannels) + t.Parallel() + + alice, bob := setupFundingManagers(t) defer tearDownFundingManagers(t, alice, bob) // We will consume the channel updates as we go, so no buffering is needed. @@ -2044,12 +2158,17 @@ func TestFundingManagerPrivateRestart(t *testing.T) { localAmt := btcutil.Amount(500000) pushAmt := btcutil.Amount(0) capacity := localAmt + pushAmt - fundingOutPoint := openChannel(t, alice, bob, localAmt, pushAmt, 1, - updateChan, false) + fundingOutPoint, fundingTx := openChannel( + t, alice, bob, localAmt, pushAmt, 1, updateChan, false, + ) // Notify that transaction was mined - alice.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{} - bob.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{} + alice.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } + bob.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } // The funding transaction was mined, so assert that both funding // managers now have the state of this channel 'markedOpen' in their @@ -2092,8 +2211,12 @@ func TestFundingManagerPrivateRestart(t *testing.T) { assertHandleFundingLocked(t, alice, bob) // Notify that six confirmations has been reached on funding transaction. - alice.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{} - bob.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{} + alice.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } + bob.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } // Since this is a private channel, we shouldn't receive the public // channel announcement messages. @@ -2155,7 +2278,9 @@ func TestFundingManagerPrivateRestart(t *testing.T) { // TestFundingManagerCustomChannelParameters checks that custom requirements we // specify during the channel funding flow is preserved correcly on both sides. func TestFundingManagerCustomChannelParameters(t *testing.T) { - alice, bob := setupFundingManagers(t, DefaultMaxPendingChannels) + t.Parallel() + + alice, bob := setupFundingManagers(t) defer tearDownFundingManagers(t, alice, bob) // This is the custom parameters we'll use. @@ -2348,15 +2473,20 @@ func TestFundingManagerCustomChannelParameters(t *testing.T) { } // Wait for Alice to published the funding tx to the network. + var fundingTx *wire.MsgTx select { - case <-alice.publTxChan: + case fundingTx = <-alice.publTxChan: case <-time.After(time.Second * 5): t.Fatalf("alice did not publish funding tx") } // Notify that transaction was mined. - alice.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{} - bob.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{} + alice.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } + bob.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{ + Tx: fundingTx, + } // After the funding transaction is mined, Alice will send // fundingLocked to Bob. @@ -2383,9 +2513,15 @@ func TestFundingManagerCustomChannelParameters(t *testing.T) { // TestFundingManagerMaxPendingChannels checks that trying to open another // channel with the same peer when MaxPending channels are pending fails. func TestFundingManagerMaxPendingChannels(t *testing.T) { + t.Parallel() + const maxPending = 4 - alice, bob := setupFundingManagers(t, maxPending) + alice, bob := setupFundingManagers( + t, func(cfg *fundingConfig) { + cfg.MaxPendingChannels = maxPending + }, + ) defer tearDownFundingManagers(t, alice, bob) // Create openChanReqs for maxPending+1 channels. @@ -2483,6 +2619,7 @@ func TestFundingManagerMaxPendingChannels(t *testing.T) { ).(*lnwire.Error) // Give the FundingSigned messages to Alice. + var txs []*wire.MsgTx for i, sign := range signs { alice.fundingMgr.processFundingSigned(sign, bob) @@ -2501,7 +2638,8 @@ func TestFundingManagerMaxPendingChannels(t *testing.T) { } select { - case <-alice.publTxChan: + case tx := <-alice.publTxChan: + txs = append(txs, tx) case <-time.After(time.Second * 5): t.Fatalf("alice did not publish funding tx") } @@ -2518,8 +2656,12 @@ func TestFundingManagerMaxPendingChannels(t *testing.T) { // Notify that the transactions were mined. for i := 0; i < maxPending; i++ { - alice.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{} - bob.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{} + alice.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{ + Tx: txs[i], + } + bob.mockNotifier.oneConfChannel <- &chainntnfs.TxConfirmation{ + Tx: txs[i], + } // Expect both to be sending FundingLocked. _ = assertFundingMsgSent( @@ -2544,14 +2686,15 @@ func TestFundingManagerMaxPendingChannels(t *testing.T) { // TestFundingManagerRejectPush checks behaviour of 'rejectpush' // option, namely that non-zero incoming push amounts are disabled. func TestFundingManagerRejectPush(t *testing.T) { + t.Parallel() + // Enable 'rejectpush' option and initialize funding managers. - alice, bob := setupFundingManagers(t, DefaultMaxPendingChannels) - rejectPush := cfg.RejectPush - cfg.RejectPush = true - defer func() { - tearDownFundingManagers(t, alice, bob) - cfg.RejectPush = rejectPush - }() + alice, bob := setupFundingManagers( + t, func(cfg *fundingConfig) { + cfg.RejectPush = true + }, + ) + defer tearDownFundingManagers(t, alice, bob) // Create a funding request and start the workflow. updateChan := make(chan *lnrpc.OpenStatusUpdate) @@ -2607,7 +2750,7 @@ func TestFundingManagerRejectPush(t *testing.T) { func TestFundingManagerMaxConfs(t *testing.T) { t.Parallel() - alice, bob := setupFundingManagers(t, DefaultMaxPendingChannels) + alice, bob := setupFundingManagers(t) defer tearDownFundingManagers(t, alice, bob) // Create a funding request and start the workflow. @@ -2669,3 +2812,102 @@ func TestFundingManagerMaxConfs(t *testing.T) { string(err.Data)) } } + +// TestFundingManagerFundAll tests that we can initiate a funding request to +// use the funds remaining in the wallet. This should produce a funding tx with +// no change output. +func TestFundingManagerFundAll(t *testing.T) { + t.Parallel() + + // We set up our mock wallet to control a list of UTXOs that sum to + // less than the max channel size. + allCoins := []*lnwallet.Utxo{ + { + AddressType: lnwallet.WitnessPubKey, + Value: btcutil.Amount( + 0.05 * btcutil.SatoshiPerBitcoin, + ), + PkScript: make([]byte, 22), + OutPoint: wire.OutPoint{ + Hash: chainhash.Hash{}, + Index: 0, + }, + }, + { + AddressType: lnwallet.WitnessPubKey, + Value: btcutil.Amount( + 0.06 * btcutil.SatoshiPerBitcoin, + ), + PkScript: make([]byte, 22), + OutPoint: wire.OutPoint{ + Hash: chainhash.Hash{}, + Index: 1, + }, + }, + } + + tests := []struct { + spendAmt btcutil.Amount + change bool + }{ + { + // We will spend all the funds in the wallet, and + // expects no change output. + spendAmt: btcutil.Amount( + 0.11 * btcutil.SatoshiPerBitcoin, + ), + change: false, + }, + { + // We spend a little less than the funds in the wallet, + // so a change output should be created. + spendAmt: btcutil.Amount( + 0.10 * btcutil.SatoshiPerBitcoin, + ), + change: true, + }, + } + + for _, test := range tests { + alice, bob := setupFundingManagers(t) + defer tearDownFundingManagers(t, alice, bob) + + alice.fundingMgr.cfg.Wallet.WalletController.(*mockWalletController).utxos = allCoins + + // We will consume the channel updates as we go, so no + // buffering is needed. + updateChan := make(chan *lnrpc.OpenStatusUpdate) + + // Initiate a fund channel, and inspect the funding tx. + pushAmt := btcutil.Amount(0) + fundingTx := fundChannel( + t, alice, bob, test.spendAmt, pushAmt, true, 1, + updateChan, true, + ) + + // Check whether the expected change output is present. + if test.change && len(fundingTx.TxOut) != 2 { + t.Fatalf("expected 2 outputs, had %v", + len(fundingTx.TxOut)) + } + + if !test.change && len(fundingTx.TxOut) != 1 { + t.Fatalf("expected 1 output, had %v", + len(fundingTx.TxOut)) + } + + // Inputs should be all funds in the wallet. + if len(fundingTx.TxIn) != len(allCoins) { + t.Fatalf("Had %d inputs, expected %d", + len(fundingTx.TxIn), len(allCoins)) + } + + for i, txIn := range fundingTx.TxIn { + if txIn.PreviousOutPoint != allCoins[i].OutPoint { + t.Fatalf("expected outpoint to be %v, was %v", + allCoins[i].OutPoint, + txIn.PreviousOutPoint) + } + } + } +} diff --git a/lnwallet/interface_test.go b/lnwallet/interface_test.go index ca7d9a27..ac650778 100644 --- a/lnwallet/interface_test.go +++ b/lnwallet/interface_test.go @@ -415,15 +415,15 @@ func testDualFundingReservationWorkflow(miner *rpctest.Harness, t.Fatalf("unable to query fee estimator: %v", err) } aliceReq := &lnwallet.InitFundingReserveMsg{ - ChainHash: chainHash, - NodeID: bobPub, - NodeAddr: bobAddr, - FundingAmount: fundingAmount, - Capacity: fundingAmount * 2, - CommitFeePerKw: feePerKw, - FundingFeePerKw: feePerKw, - PushMSat: 0, - Flags: lnwire.FFAnnounceChannel, + ChainHash: chainHash, + NodeID: bobPub, + NodeAddr: bobAddr, + LocalFundingAmt: fundingAmount, + RemoteFundingAmt: fundingAmount, + CommitFeePerKw: feePerKw, + FundingFeePerKw: feePerKw, + PushMSat: 0, + Flags: lnwire.FFAnnounceChannel, } aliceChanReservation, err := alice.InitChannelReservation(aliceReq) if err != nil { @@ -458,15 +458,15 @@ func testDualFundingReservationWorkflow(miner *rpctest.Harness, // receives' Alice's contribution, and consumes that so we can continue // the funding process. bobReq := &lnwallet.InitFundingReserveMsg{ - ChainHash: chainHash, - NodeID: alicePub, - NodeAddr: aliceAddr, - FundingAmount: fundingAmount, - Capacity: fundingAmount * 2, - CommitFeePerKw: feePerKw, - FundingFeePerKw: feePerKw, - PushMSat: 0, - Flags: lnwire.FFAnnounceChannel, + ChainHash: chainHash, + NodeID: alicePub, + NodeAddr: aliceAddr, + LocalFundingAmt: fundingAmount, + RemoteFundingAmt: fundingAmount, + CommitFeePerKw: feePerKw, + FundingFeePerKw: feePerKw, + PushMSat: 0, + Flags: lnwire.FFAnnounceChannel, } bobChanReservation, err := bob.InitChannelReservation(bobReq) if err != nil { @@ -618,15 +618,15 @@ func testFundingTransactionLockedOutputs(miner *rpctest.Harness, t.Fatalf("unable to query fee estimator: %v", err) } req := &lnwallet.InitFundingReserveMsg{ - ChainHash: chainHash, - NodeID: bobPub, - NodeAddr: bobAddr, - FundingAmount: fundingAmount, - Capacity: fundingAmount, - CommitFeePerKw: feePerKw, - FundingFeePerKw: feePerKw, - PushMSat: 0, - Flags: lnwire.FFAnnounceChannel, + ChainHash: chainHash, + NodeID: bobPub, + NodeAddr: bobAddr, + LocalFundingAmt: fundingAmount, + RemoteFundingAmt: 0, + CommitFeePerKw: feePerKw, + FundingFeePerKw: feePerKw, + PushMSat: 0, + Flags: lnwire.FFAnnounceChannel, } if _, err := alice.InitChannelReservation(req); err != nil { t.Fatalf("unable to initialize funding reservation 1: %v", err) @@ -640,15 +640,15 @@ func testFundingTransactionLockedOutputs(miner *rpctest.Harness, t.Fatalf("unable to create amt: %v", err) } failedReq := &lnwallet.InitFundingReserveMsg{ - ChainHash: chainHash, - NodeID: bobPub, - NodeAddr: bobAddr, - FundingAmount: amt, - Capacity: amt, - CommitFeePerKw: feePerKw, - FundingFeePerKw: feePerKw, - PushMSat: 0, - Flags: lnwire.FFAnnounceChannel, + ChainHash: chainHash, + NodeID: bobPub, + NodeAddr: bobAddr, + LocalFundingAmt: amt, + RemoteFundingAmt: 0, + CommitFeePerKw: feePerKw, + FundingFeePerKw: feePerKw, + PushMSat: 0, + Flags: lnwire.FFAnnounceChannel, } failedReservation, err := alice.InitChannelReservation(failedReq) if err == nil { @@ -676,15 +676,15 @@ func testFundingCancellationNotEnoughFunds(miner *rpctest.Harness, t.Fatalf("unable to create amt: %v", err) } req := &lnwallet.InitFundingReserveMsg{ - ChainHash: chainHash, - NodeID: bobPub, - NodeAddr: bobAddr, - FundingAmount: fundingAmount, - Capacity: fundingAmount, - CommitFeePerKw: feePerKw, - FundingFeePerKw: feePerKw, - PushMSat: 0, - Flags: lnwire.FFAnnounceChannel, + ChainHash: chainHash, + NodeID: bobPub, + NodeAddr: bobAddr, + LocalFundingAmt: fundingAmount, + RemoteFundingAmt: 0, + CommitFeePerKw: feePerKw, + FundingFeePerKw: feePerKw, + PushMSat: 0, + Flags: lnwire.FFAnnounceChannel, } chanReservation, err := alice.InitChannelReservation(req) if err != nil { @@ -753,9 +753,9 @@ func testCancelNonExistentReservation(miner *rpctest.Harness, func testReservationInitiatorBalanceBelowDustCancel(miner *rpctest.Harness, alice, _ *lnwallet.LightningWallet, t *testing.T) { - // We'll attempt to create a new reservation with an extremely high fee - // rate. This should push our balance into the negative and result in a - // failure to create the reservation. + // We'll attempt to create a new reservation with an extremely high + // commitment fee rate. This should push our balance into the negative + // and result in a failure to create the reservation. const numBTC = 4 fundingAmount, err := btcutil.NewAmount(numBTC) if err != nil { @@ -766,15 +766,15 @@ func testReservationInitiatorBalanceBelowDustCancel(miner *rpctest.Harness, numBTC * numBTC * btcutil.SatoshiPerBitcoin, ) req := &lnwallet.InitFundingReserveMsg{ - ChainHash: chainHash, - NodeID: bobPub, - NodeAddr: bobAddr, - FundingAmount: fundingAmount, - Capacity: fundingAmount, - CommitFeePerKw: feePerKw, - FundingFeePerKw: feePerKw, - PushMSat: 0, - Flags: lnwire.FFAnnounceChannel, + ChainHash: chainHash, + NodeID: bobPub, + NodeAddr: bobAddr, + LocalFundingAmt: fundingAmount, + RemoteFundingAmt: 0, + CommitFeePerKw: feePerKw, + FundingFeePerKw: 1000, + PushMSat: 0, + Flags: lnwire.FFAnnounceChannel, } _, err = alice.InitChannelReservation(req) switch { @@ -847,15 +847,15 @@ func testSingleFunderReservationWorkflow(miner *rpctest.Harness, t.Fatalf("unable to query fee estimator: %v", err) } aliceReq := &lnwallet.InitFundingReserveMsg{ - ChainHash: chainHash, - NodeID: bobPub, - NodeAddr: bobAddr, - FundingAmount: fundingAmt, - Capacity: fundingAmt, - CommitFeePerKw: feePerKw, - FundingFeePerKw: feePerKw, - PushMSat: pushAmt, - Flags: lnwire.FFAnnounceChannel, + ChainHash: chainHash, + NodeID: bobPub, + NodeAddr: bobAddr, + LocalFundingAmt: fundingAmt, + RemoteFundingAmt: 0, + CommitFeePerKw: feePerKw, + FundingFeePerKw: feePerKw, + PushMSat: pushAmt, + Flags: lnwire.FFAnnounceChannel, } aliceChanReservation, err := alice.InitChannelReservation(aliceReq) if err != nil { @@ -890,15 +890,15 @@ func testSingleFunderReservationWorkflow(miner *rpctest.Harness, // Next, Bob receives the initial request, generates a corresponding // reservation initiation, then consume Alice's contribution. bobReq := &lnwallet.InitFundingReserveMsg{ - ChainHash: chainHash, - NodeID: alicePub, - NodeAddr: aliceAddr, - FundingAmount: 0, - Capacity: fundingAmt, - CommitFeePerKw: feePerKw, - FundingFeePerKw: feePerKw, - PushMSat: pushAmt, - Flags: lnwire.FFAnnounceChannel, + ChainHash: chainHash, + NodeID: alicePub, + NodeAddr: aliceAddr, + LocalFundingAmt: 0, + RemoteFundingAmt: fundingAmt, + CommitFeePerKw: feePerKw, + FundingFeePerKw: feePerKw, + PushMSat: pushAmt, + Flags: lnwire.FFAnnounceChannel, } bobChanReservation, err := bob.InitChannelReservation(bobReq) if err != nil { diff --git a/lnwallet/reservation.go b/lnwallet/reservation.go index 287eead8..555d0ded 100644 --- a/lnwallet/reservation.go +++ b/lnwallet/reservation.go @@ -127,7 +127,7 @@ type ChannelReservation struct { // used only internally by lnwallet. In order to concurrent safety, the // creation of all channel reservations should be carried out via the // lnwallet.InitChannelReservation interface. -func NewChannelReservation(capacity, fundingAmt btcutil.Amount, +func NewChannelReservation(capacity, localFundingAmt btcutil.Amount, commitFeePerKw SatPerKWeight, wallet *LightningWallet, id uint64, pushMSat lnwire.MilliSatoshi, chainHash *chainhash.Hash, flags lnwire.FundingFlag) (*ChannelReservation, error) { @@ -139,14 +139,16 @@ func NewChannelReservation(capacity, fundingAmt btcutil.Amount, ) commitFee := commitFeePerKw.FeeForWeight(input.CommitWeight) - fundingMSat := lnwire.NewMSatFromSatoshis(fundingAmt) + localFundingMSat := lnwire.NewMSatFromSatoshis(localFundingAmt) + // TODO(halseth): make method take remote funding amount direcly + // instead of inferring it from capacity and local amt. capacityMSat := lnwire.NewMSatFromSatoshis(capacity) feeMSat := lnwire.NewMSatFromSatoshis(commitFee) // If we're the responder to a single-funder reservation, then we have // no initial balance in the channel unless the remote party is pushing // some funds to us within the first commitment state. - if fundingAmt == 0 { + if localFundingAmt == 0 { ourBalance = pushMSat theirBalance = capacityMSat - feeMSat - pushMSat initiator = false @@ -163,7 +165,7 @@ func NewChannelReservation(capacity, fundingAmt btcutil.Amount, // TODO(roasbeef): need to rework fee structure in general and // also when we "unlock" dual funder within the daemon - if capacity == fundingAmt { + if capacity == localFundingAmt { // If we're initiating a single funder workflow, then // we pay all the initial fees within the commitment // transaction. We also deduct our balance by the @@ -174,8 +176,8 @@ func NewChannelReservation(capacity, fundingAmt btcutil.Amount, // Otherwise, this is a dual funder workflow where both // slides split the amount funded and the commitment // fee. - ourBalance = fundingMSat - (feeMSat / 2) - theirBalance = capacityMSat - fundingMSat - (feeMSat / 2) + pushMSat + ourBalance = localFundingMSat - (feeMSat / 2) + theirBalance = capacityMSat - localFundingMSat - (feeMSat / 2) + pushMSat } initiator = true @@ -513,6 +515,13 @@ func (r *ChannelReservation) FundingOutpoint() *wire.OutPoint { return &r.partialState.FundingOutpoint } +// Capacity returns the channel capacity for this reservation. +func (r *ChannelReservation) Capacity() btcutil.Amount { + r.RLock() + defer r.RUnlock() + return r.partialState.Capacity +} + // Cancel abandons this channel reservation. This method should be called in // the scenario that communications with the counterparty break down. Upon // cancellation, all resources previously reserved for this pending payment diff --git a/lnwallet/wallet.go b/lnwallet/wallet.go index 5d2646bf..3a2a0160 100644 --- a/lnwallet/wallet.go +++ b/lnwallet/wallet.go @@ -69,12 +69,20 @@ type InitFundingReserveMsg struct { // workflow. NodeAddr net.Addr - // FundingAmount is the amount of funds requested for this channel. - FundingAmount btcutil.Amount + // SubtractFees should be set if we intend to spend exactly + // LocalFundingAmt when opening the channel, subtracting the fees from + // the funding output. This can be used for instance to use all our + // remaining funds to open the channel, since it will take fees into + // account. + SubtractFees bool - // Capacity is the total capacity of the channel which includes the - // amount of funds the remote party contributes (if any). - Capacity btcutil.Amount + // LocalFundingAmt is the amount of funds requested from us for this + // channel. + LocalFundingAmt btcutil.Amount + + // RemoteFundingAmnt is the amount of funds the remote will contribute + // to this channel. + RemoteFundingAmt btcutil.Amount // CommitFeePerKw is the starting accepted satoshis/Kw fee for the set // of initial commitment transactions. In order to ensure timely @@ -431,7 +439,7 @@ func (l *LightningWallet) InitChannelReservation( // validate a funding reservation request. func (l *LightningWallet) handleFundingReserveRequest(req *InitFundingReserveMsg) { // It isn't possible to create a channel with zero funds committed. - if req.FundingAmount+req.Capacity == 0 { + if req.LocalFundingAmt+req.RemoteFundingAmt == 0 { err := ErrZeroCapacity() req.err <- err req.resp <- nil @@ -449,128 +457,59 @@ func (l *LightningWallet) handleFundingReserveRequest(req *InitFundingReserveMsg return } - id := atomic.AddUint64(&l.nextFundingID, 1) - reservation, err := NewChannelReservation( - req.Capacity, req.FundingAmount, req.CommitFeePerKw, l, id, - req.PushMSat, l.Cfg.NetParams.GenesisHash, req.Flags, + localFundingAmt := req.LocalFundingAmt + + var ( + selected *coinSelection + err error ) - if err != nil { - req.err <- err - req.resp <- nil - return - } - - // Grab the mutex on the ChannelReservation to ensure thread-safety - reservation.Lock() - defer reservation.Unlock() - - reservation.nodeAddr = req.NodeAddr - reservation.partialState.IdentityPub = req.NodeID // If we're on the receiving end of a single funder channel then we - // don't need to perform any coin selection. Otherwise, attempt to - // obtain enough coins to meet the required funding amount. - if req.FundingAmount != 0 { + // don't need to perform any coin selection, and the remote contributes + // all funds. Otherwise, attempt to obtain enough coins to meet the + // required funding amount. + if req.LocalFundingAmt != 0 { // Coin selection is done on the basis of sat/kw, so we'll use // the fee rate passed in to perform coin selection. - err := l.selectCoinsAndChange( - req.FundingFeePerKw, req.FundingAmount, req.MinConfs, - reservation.ourContribution, + var err error + selected, err = l.selectCoinsAndChange( + req.FundingFeePerKw, req.LocalFundingAmt, req.MinConfs, + req.SubtractFees, ) if err != nil { req.err <- err req.resp <- nil return } + + localFundingAmt = selected.fundingAmt } - // Next, we'll grab a series of keys from the wallet which will be used - // for the duration of the channel. The keys include: our multi-sig - // key, the base revocation key, the base htlc key,the base payment - // key, and the delayed payment key. - // - // TODO(roasbeef): "salt" each key as well? - reservation.ourContribution.MultiSigKey, err = l.DeriveNextKey( - keychain.KeyFamilyMultiSig, - ) - if err != nil { - req.err <- err - req.resp <- nil - return - } - reservation.ourContribution.RevocationBasePoint, err = l.DeriveNextKey( - keychain.KeyFamilyRevocationBase, - ) - if err != nil { - req.err <- err - req.resp <- nil - return - } - reservation.ourContribution.HtlcBasePoint, err = l.DeriveNextKey( - keychain.KeyFamilyHtlcBase, - ) - if err != nil { - req.err <- err - req.resp <- nil - return - } - reservation.ourContribution.PaymentBasePoint, err = l.DeriveNextKey( - keychain.KeyFamilyPaymentBase, - ) - if err != nil { - req.err <- err - req.resp <- nil - return - } - reservation.ourContribution.DelayBasePoint, err = l.DeriveNextKey( - keychain.KeyFamilyDelayBase, + // The total channel capacity will be the size of the funding output we + // created plus the remote contribution. + capacity := localFundingAmt + req.RemoteFundingAmt + + id := atomic.AddUint64(&l.nextFundingID, 1) + reservation, err := NewChannelReservation( + capacity, localFundingAmt, req.CommitFeePerKw, l, id, + req.PushMSat, l.Cfg.NetParams.GenesisHash, req.Flags, ) if err != nil { + selected.unlockCoins() req.err <- err req.resp <- nil return } - // With the above keys created, we'll also need to initialization our - // initial revocation tree state. - nextRevocationKeyDesc, err := l.DeriveNextKey( - keychain.KeyFamilyRevocationRoot, + err = l.initOurContribution( + reservation, selected, req.NodeAddr, req.NodeID, ) if err != nil { + selected.unlockCoins() req.err <- err req.resp <- nil return } - revocationRoot, err := l.DerivePrivKey(nextRevocationKeyDesc) - if err != nil { - req.err <- err - req.resp <- nil - return - } - - // Once we have the root, we can then generate our shachain producer - // and from that generate the per-commitment point. - revRoot, err := chainhash.NewHash(revocationRoot.Serialize()) - if err != nil { - req.err <- err - req.resp <- nil - return - } - producer := shachain.NewRevocationProducer(*revRoot) - firstPreimage, err := producer.AtIndex(0) - if err != nil { - req.err <- err - req.resp <- nil - return - } - reservation.ourContribution.FirstCommitmentPoint = input.ComputeCommitmentPoint( - firstPreimage[:], - ) - - reservation.partialState.RevocationProducer = producer - reservation.ourContribution.ChannelConstraints = l.Cfg.DefaultConstraints - - // TODO(roasbeef): turn above into: initContribution() // Create a limbo and record entry for this newly pending funding // request. @@ -585,6 +524,96 @@ func (l *LightningWallet) handleFundingReserveRequest(req *InitFundingReserveMsg req.err <- nil } +// initOurContribution initializes the given ChannelReservation with our coins +// and change reserved for the channel, and derives the keys to use for this +// channel. +func (l *LightningWallet) initOurContribution(reservation *ChannelReservation, + selected *coinSelection, nodeAddr net.Addr, nodeID *btcec.PublicKey) error { + + // Grab the mutex on the ChannelReservation to ensure thread-safety + reservation.Lock() + defer reservation.Unlock() + + if selected != nil { + reservation.ourContribution.Inputs = selected.coins + reservation.ourContribution.ChangeOutputs = selected.change + } + + reservation.nodeAddr = nodeAddr + reservation.partialState.IdentityPub = nodeID + + // Next, we'll grab a series of keys from the wallet which will be used + // for the duration of the channel. The keys include: our multi-sig + // key, the base revocation key, the base htlc key,the base payment + // key, and the delayed payment key. + // + // TODO(roasbeef): "salt" each key as well? + var err error + reservation.ourContribution.MultiSigKey, err = l.DeriveNextKey( + keychain.KeyFamilyMultiSig, + ) + if err != nil { + return err + } + reservation.ourContribution.RevocationBasePoint, err = l.DeriveNextKey( + keychain.KeyFamilyRevocationBase, + ) + if err != nil { + return err + } + reservation.ourContribution.HtlcBasePoint, err = l.DeriveNextKey( + keychain.KeyFamilyHtlcBase, + ) + if err != nil { + return err + } + reservation.ourContribution.PaymentBasePoint, err = l.DeriveNextKey( + keychain.KeyFamilyPaymentBase, + ) + if err != nil { + return err + } + reservation.ourContribution.DelayBasePoint, err = l.DeriveNextKey( + keychain.KeyFamilyDelayBase, + ) + if err != nil { + return err + } + + // With the above keys created, we'll also need to initialization our + // initial revocation tree state. + nextRevocationKeyDesc, err := l.DeriveNextKey( + keychain.KeyFamilyRevocationRoot, + ) + if err != nil { + return err + } + revocationRoot, err := l.DerivePrivKey(nextRevocationKeyDesc) + if err != nil { + return err + } + + // Once we have the root, we can then generate our shachain producer + // and from that generate the per-commitment point. + revRoot, err := chainhash.NewHash(revocationRoot.Serialize()) + if err != nil { + return err + } + producer := shachain.NewRevocationProducer(*revRoot) + firstPreimage, err := producer.AtIndex(0) + if err != nil { + return err + } + reservation.ourContribution.FirstCommitmentPoint = input.ComputeCommitmentPoint( + firstPreimage[:], + ) + + reservation.partialState.RevocationProducer = producer + reservation.ourContribution.ChannelConstraints = l.Cfg.DefaultConstraints + + return nil +} + // handleFundingReserveCancel cancels an existing channel reservation. As part // of the cancellation, outputs previously selected as inputs for the funding // transaction via coin selection are freed allowing future reservations to @@ -1269,14 +1298,24 @@ func (l *LightningWallet) WithCoinSelectLock(f func() error) error { return f() } +// coinSelection holds the result from selectCoinsAndChange. +type coinSelection struct { + coins []*wire.TxIn + change []*wire.TxOut + fundingAmt btcutil.Amount + unlockCoins func() +} + // selectCoinsAndChange performs coin selection in order to obtain witness -// outputs which sum to at least 'numCoins' amount of satoshis. If coin -// selection is successful/possible, then the selected coins are available -// within the passed contribution's inputs. If necessary, a change address will -// also be generated. +// outputs which sum to at least 'amt' amount of satoshis. If necessary, +// a change address will also be generated. If coin selection is +// successful/possible, then the selected coins and change outputs are +// returned, and the value of the resulting funding output. This method locks +// the selected outputs, and a function closure to unlock them in case of an +// error is returned. func (l *LightningWallet) selectCoinsAndChange(feeRate SatPerKWeight, - amt btcutil.Amount, minConfs int32, - contribution *ChannelContribution) error { + amt btcutil.Amount, minConfs int32, subtractFees bool) ( + *coinSelection, error) { // We hold the coin select mutex while querying for outputs, and // performing coin selection in order to avoid inadvertent double @@ -1291,21 +1330,66 @@ func (l *LightningWallet) selectCoinsAndChange(feeRate SatPerKWeight, // number of confirmations required. coins, err := l.ListUnspentWitness(minConfs, math.MaxInt32) if err != nil { - return err + return nil, err } + var ( + selectedCoins []*Utxo + fundingAmt btcutil.Amount + changeAmt btcutil.Amount + ) + // Perform coin selection over our available, unlocked unspent outputs // in order to find enough coins to meet the funding amount // requirements. - selectedCoins, changeAmt, err := coinSelect(feeRate, amt, coins) - if err != nil { - return err + switch { + // In case this request want the fees subtracted from the local amount, + // we'll call the specialized method for that. This ensures that we + // won't deduct more that the specified balance from our wallet. + case subtractFees: + dustLimit := l.Cfg.DefaultConstraints.DustLimit + selectedCoins, fundingAmt, changeAmt, err = coinSelectSubtractFees( + feeRate, amt, dustLimit, coins, + ) + if err != nil { + return nil, err + } + + // Ótherwise do a normal coin selection where we target a given funding + // amount. + default: + fundingAmt = amt + selectedCoins, changeAmt, err = coinSelect(feeRate, amt, coins) + if err != nil { + return nil, err + } + } + + // Record any change output(s) generated as a result of the coin + // selection, but only if the addition of the output won't lead to the + // creation of dust. + var changeOutputs []*wire.TxOut + if changeAmt != 0 && changeAmt > DefaultDustLimit() { + changeAddr, err := l.NewAddress(WitnessPubKey, true) + if err != nil { + return nil, err + } + changeScript, err := txscript.PayToAddrScript(changeAddr) + if err != nil { + return nil, err + } + + changeOutputs = make([]*wire.TxOut, 1) + changeOutputs[0] = &wire.TxOut{ + Value: int64(changeAmt), + PkScript: changeScript, + } } // Lock the selected coins. These coins are now "reserved", this // prevents concurrent funding requests from referring to and this // double-spending the same set of coins. - contribution.Inputs = make([]*wire.TxIn, len(selectedCoins)) + inputs := make([]*wire.TxIn, len(selectedCoins)) for i, coin := range selectedCoins { outpoint := &coin.OutPoint l.lockedOutPoints[*outpoint] = struct{}{} @@ -1313,30 +1397,26 @@ func (l *LightningWallet) selectCoinsAndChange(feeRate SatPerKWeight, // Empty sig script, we'll actually sign if this reservation is // queued up to be completed (the other side accepts). - contribution.Inputs[i] = wire.NewTxIn(outpoint, nil, nil) + inputs[i] = wire.NewTxIn(outpoint, nil, nil) } - // Record any change output(s) generated as a result of the coin - // selection, but only if the addition of the output won't lead to the - // creation of dust. - if changeAmt != 0 && changeAmt > DefaultDustLimit() { - changeAddr, err := l.NewAddress(WitnessPubKey, true) - if err != nil { - return err - } - changeScript, err := txscript.PayToAddrScript(changeAddr) - if err != nil { - return err - } + unlock := func() { + l.coinSelectMtx.Lock() + defer l.coinSelectMtx.Unlock() - contribution.ChangeOutputs = make([]*wire.TxOut, 1) - contribution.ChangeOutputs[0] = &wire.TxOut{ - Value: int64(changeAmt), - PkScript: changeScript, + for _, coin := range selectedCoins { + outpoint := &coin.OutPoint + delete(l.lockedOutPoints, *outpoint) + l.UnlockOutpoint(*outpoint) } } - return nil + return &coinSelection{ + coins: inputs, + change: changeOutputs, + fundingAmt: fundingAmt, + unlockCoins: unlock, + }, nil } // DeriveStateHintObfuscator derives the bytes to be used for obfuscating the @@ -1416,7 +1496,7 @@ func coinSelect(feeRate SatPerKWeight, amt btcutil.Amount, case NestedWitnessPubKey: weightEstimate.AddNestedP2WKHInput() default: - return nil, 0, fmt.Errorf("Unsupported address type: %v", + return nil, 0, fmt.Errorf("unsupported address type: %v", utxo.AddressType) } } @@ -1428,6 +1508,8 @@ func coinSelect(feeRate SatPerKWeight, amt btcutil.Amount, // // TODO: Handle wallets that generate non-witness change // addresses. + // TODO(halseth): make coinSelect not estimate change output + // for dust change. weightEstimate.AddP2WKHOutput() // The difference between the selected amount and the amount @@ -1453,3 +1535,95 @@ func coinSelect(feeRate SatPerKWeight, amt btcutil.Amount, return selectedUtxos, changeAmt, nil } } + +// coinSelectSubtractFees attempts to select coins such that we'll spend up to +// amt in total after fees, adhering to the specified fee rate. The selected +// coins, the final output and change values are returned. +func coinSelectSubtractFees(feeRate SatPerKWeight, amt, + dustLimit btcutil.Amount, coins []*Utxo) ([]*Utxo, btcutil.Amount, + btcutil.Amount, error) { + + // First perform an initial round of coin selection to estimate + // the required fee. + totalSat, selectedUtxos, err := selectInputs(amt, coins) + if err != nil { + return nil, 0, 0, err + } + + var weightEstimate input.TxWeightEstimator + for _, utxo := range selectedUtxos { + switch utxo.AddressType { + case WitnessPubKey: + weightEstimate.AddP2WKHInput() + case NestedWitnessPubKey: + weightEstimate.AddNestedP2WKHInput() + default: + return nil, 0, 0, fmt.Errorf("unsupported "+ + "address type: %v", utxo.AddressType) + } + } + + // Channel funding multisig output is P2WSH. + weightEstimate.AddP2WSHOutput() + + // At this point we've got two possibilities, either create a + // change output, or not. We'll first try without creating a + // change output. + // + // Estimate the fee required for a transaction without a change + // output. + totalWeight := int64(weightEstimate.Weight()) + requiredFee := feeRate.FeeForWeight(totalWeight) + + // For a transaction without a change output, we'll let everything go + // to our multi-sig output after subtracting fees. + outputAmt := totalSat - requiredFee + changeAmt := btcutil.Amount(0) + + // If the the output is too small after subtracting the fee, the coin + // selection cannot be performed with an amount this small. + if outputAmt <= dustLimit { + return nil, 0, 0, fmt.Errorf("output amount(%v) after "+ + "subtracting fees(%v) below dust limit(%v)", outputAmt, + requiredFee, dustLimit) + } + + // We were able to create a transaction with no change from the + // selected inputs. We'll remember the resulting values for + // now, while we try to add a change output. Assume that change output + // is a P2WKH output. + weightEstimate.AddP2WKHOutput() + + // Now that we have added the change output, redo the fee + // estimate. + totalWeight = int64(weightEstimate.Weight()) + requiredFee = feeRate.FeeForWeight(totalWeight) + + // For a transaction with a change output, everything we don't spend + // will go to change. + newChange := totalSat - amt + newOutput := amt - requiredFee + + // If adding a change output leads to both outputs being above + // the dust limit, we'll add the change output. Otherwise we'll + // go with the no change tx we originally found. + if newChange > dustLimit && newOutput > dustLimit { + outputAmt = newOutput + changeAmt = newChange + } + + // Sanity check the resulting output values to make sure we + // don't burn a great part to fees. + totalOut := outputAmt + changeAmt + fee := totalSat - totalOut + + // Fail if more than 20% goes to fees. + // TODO(halseth): smarter fee limit. Make configurable or dynamic wrt + // total funding size? + if fee > totalOut/5 { + return nil, 0, 0, fmt.Errorf("fee %v on total output"+ + "value %v", fee, totalOut) + } + + return selectedUtxos, outputAmt, changeAmt, nil +} diff --git a/lnwallet/wallet_test.go b/lnwallet/wallet_test.go new file mode 100644 index 00000000..f9a6c0d3 --- /dev/null +++ b/lnwallet/wallet_test.go @@ -0,0 +1,350 @@ +package lnwallet + +import ( + "testing" + + "github.com/btcsuite/btcutil" + "github.com/lightningnetwork/lnd/input" +) + +// fundingFee is a helper method that returns the fee estimate used for a tx +// with the given number of inputs and the optional change output. This matches +// the estimate done by the wallet. +func fundingFee(feeRate SatPerKWeight, numInput int, change bool) btcutil.Amount { + var weightEstimate input.TxWeightEstimator + + // All inputs. + for i := 0; i < numInput; i++ { + weightEstimate.AddP2WKHInput() + } + + // The multisig funding output. + weightEstimate.AddP2WSHOutput() + + // Optionally count a change output. + if change { + weightEstimate.AddP2WKHOutput() + } + + totalWeight := int64(weightEstimate.Weight()) + return feeRate.FeeForWeight(totalWeight) +} + +// TestCoinSelect tests that we pick coins adding up to the expected amount +// when creating a funding transaction, and that the calculated change is the +// expected amount. +// +// NOTE: coinSelect will always attempt to add a change output, so we must +// account for this in the tests. +func TestCoinSelect(t *testing.T) { + t.Parallel() + + const feeRate = SatPerKWeight(100) + const dust = btcutil.Amount(100) + + type testCase struct { + name string + outputValue btcutil.Amount + coins []*Utxo + + expectedInput []btcutil.Amount + expectedChange btcutil.Amount + expectErr bool + } + + testCases := []testCase{ + { + // We have 1.0 BTC available, and wants to send 0.5. + // This will obviously lead to a change output of + // almost 0.5 BTC. + name: "big change", + coins: []*Utxo{ + { + AddressType: WitnessPubKey, + Value: 1 * btcutil.SatoshiPerBitcoin, + }, + }, + outputValue: 0.5 * btcutil.SatoshiPerBitcoin, + + // The one and only input will be selected. + expectedInput: []btcutil.Amount{ + 1 * btcutil.SatoshiPerBitcoin, + }, + // Change will be what's left minus the fee. + expectedChange: 0.5*btcutil.SatoshiPerBitcoin - fundingFee(feeRate, 1, true), + }, + { + // We have 1 BTC available, and we want to send 1 BTC. + // This should lead to an error, as we don't have + // enough funds to pay the fee. + name: "nothing left for fees", + coins: []*Utxo{ + { + AddressType: WitnessPubKey, + Value: 1 * btcutil.SatoshiPerBitcoin, + }, + }, + outputValue: 1 * btcutil.SatoshiPerBitcoin, + expectErr: true, + }, + { + // We have a 1 BTC input, and want to create an output + // as big as possible, such that the remaining change + // will be dust. + name: "dust change", + coins: []*Utxo{ + { + AddressType: WitnessPubKey, + Value: 1 * btcutil.SatoshiPerBitcoin, + }, + }, + // We tune the output value by subtracting the expected + // fee and a small dust amount. + outputValue: 1*btcutil.SatoshiPerBitcoin - fundingFee(feeRate, 1, true) - dust, + + expectedInput: []btcutil.Amount{ + 1 * btcutil.SatoshiPerBitcoin, + }, + + // Change will the dust. + expectedChange: dust, + }, + { + // We have a 1 BTC input, and want to create an output + // as big as possible, such that there is nothing left + // for change. + name: "no change", + coins: []*Utxo{ + { + AddressType: WitnessPubKey, + Value: 1 * btcutil.SatoshiPerBitcoin, + }, + }, + // We tune the output value to be the maximum amount + // possible, leaving just enough for fees. + outputValue: 1*btcutil.SatoshiPerBitcoin - fundingFee(feeRate, 1, true), + + expectedInput: []btcutil.Amount{ + 1 * btcutil.SatoshiPerBitcoin, + }, + // We have just enough left to pay the fee, so there is + // nothing left for change. + // TODO(halseth): currently coinselect estimates fees + // assuming a change output. + expectedChange: 0, + }, + } + + for _, test := range testCases { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + selected, changeAmt, err := coinSelect( + feeRate, test.outputValue, test.coins, + ) + if !test.expectErr && err != nil { + t.Fatalf(err.Error()) + } + + if test.expectErr && err == nil { + t.Fatalf("expected error") + } + + // If we got an expected error, there is nothing more to test. + if test.expectErr { + return + } + + // Check that the selected inputs match what we expect. + if len(selected) != len(test.expectedInput) { + t.Fatalf("expected %v inputs, got %v", + len(test.expectedInput), len(selected)) + } + + for i, coin := range selected { + if coin.Value != test.expectedInput[i] { + t.Fatalf("expected input %v to have value %v, "+ + "had %v", i, test.expectedInput[i], + coin.Value) + } + } + + // Assert we got the expected change amount. + if changeAmt != test.expectedChange { + t.Fatalf("expected %v change amt, got %v", + test.expectedChange, changeAmt) + } + }) + } +} + +// TestCoinSelectSubtractFees tests that we pick coins adding up to the +// expected amount when creating a funding transaction, and that a change +// output is created only when necessary. +func TestCoinSelectSubtractFees(t *testing.T) { + t.Parallel() + + const feeRate = SatPerKWeight(100) + const dustLimit = btcutil.Amount(1000) + const dust = btcutil.Amount(100) + + type testCase struct { + name string + spendValue btcutil.Amount + coins []*Utxo + + expectedInput []btcutil.Amount + expectedFundingAmt btcutil.Amount + expectedChange btcutil.Amount + expectErr bool + } + + testCases := []testCase{ + { + // We have 1.0 BTC available, spend them all. This + // should lead to a funding TX with one output, the + // rest goes to fees. + name: "spend all", + coins: []*Utxo{ + { + AddressType: WitnessPubKey, + Value: 1 * btcutil.SatoshiPerBitcoin, + }, + }, + spendValue: 1 * btcutil.SatoshiPerBitcoin, + + // The one and only input will be selected. + expectedInput: []btcutil.Amount{ + 1 * btcutil.SatoshiPerBitcoin, + }, + expectedFundingAmt: 1*btcutil.SatoshiPerBitcoin - fundingFee(feeRate, 1, false), + expectedChange: 0, + }, + { + // The total funds available is below the dust limit + // after paying fees. + name: "dust output", + coins: []*Utxo{ + { + AddressType: WitnessPubKey, + Value: fundingFee(feeRate, 1, false) + dust, + }, + }, + spendValue: fundingFee(feeRate, 1, false) + dust, + + expectErr: true, + }, + { + // After subtracting fees, the resulting change output + // is below the dust limit. The remainder should go + // towards the funding output. + name: "dust change", + coins: []*Utxo{ + { + AddressType: WitnessPubKey, + Value: 1 * btcutil.SatoshiPerBitcoin, + }, + }, + spendValue: 1*btcutil.SatoshiPerBitcoin - dust, + + expectedInput: []btcutil.Amount{ + 1 * btcutil.SatoshiPerBitcoin, + }, + expectedFundingAmt: 1*btcutil.SatoshiPerBitcoin - fundingFee(feeRate, 1, false), + expectedChange: 0, + }, + { + // We got just enough funds to create an output above the dust limit. + name: "output right above dustlimit", + coins: []*Utxo{ + { + AddressType: WitnessPubKey, + Value: fundingFee(feeRate, 1, false) + dustLimit + 1, + }, + }, + spendValue: fundingFee(feeRate, 1, false) + dustLimit + 1, + + expectedInput: []btcutil.Amount{ + fundingFee(feeRate, 1, false) + dustLimit + 1, + }, + expectedFundingAmt: dustLimit + 1, + expectedChange: 0, + }, + { + // Amount left is below dust limit after paying fee for + // a change output, resulting in a no-change tx. + name: "no amount to pay fee for change", + coins: []*Utxo{ + { + AddressType: WitnessPubKey, + Value: fundingFee(feeRate, 1, false) + 2*(dustLimit+1), + }, + }, + spendValue: fundingFee(feeRate, 1, false) + dustLimit + 1, + + expectedInput: []btcutil.Amount{ + fundingFee(feeRate, 1, false) + 2*(dustLimit+1), + }, + expectedFundingAmt: 2 * (dustLimit + 1), + expectedChange: 0, + }, + { + // If more than 20% of funds goes to fees, it should fail. + name: "high fee", + coins: []*Utxo{ + { + AddressType: WitnessPubKey, + Value: 5 * fundingFee(feeRate, 1, false), + }, + }, + spendValue: 5 * fundingFee(feeRate, 1, false), + + expectErr: true, + }, + } + + for _, test := range testCases { + t.Run(test.name, func(t *testing.T) { + selected, localFundingAmt, changeAmt, err := coinSelectSubtractFees( + feeRate, test.spendValue, dustLimit, test.coins, + ) + if !test.expectErr && err != nil { + t.Fatalf(err.Error()) + } + + if test.expectErr && err == nil { + t.Fatalf("expected error") + } + + // If we got an expected error, there is nothing more to test. + if test.expectErr { + return + } + + // Check that the selected inputs match what we expect. + if len(selected) != len(test.expectedInput) { + t.Fatalf("expected %v inputs, got %v", + len(test.expectedInput), len(selected)) + } + + for i, coin := range selected { + if coin.Value != test.expectedInput[i] { + t.Fatalf("expected input %v to have value %v, "+ + "had %v", i, test.expectedInput[i], + coin.Value) + } + } + + // Assert we got the expected change amount. + if localFundingAmt != test.expectedFundingAmt { + t.Fatalf("expected %v local funding amt, got %v", + test.expectedFundingAmt, localFundingAmt) + } + if changeAmt != test.expectedChange { + t.Fatalf("expected %v change amt, got %v", + test.expectedChange, changeAmt) + } + }) + } +} diff --git a/mock.go b/mock.go index 657069f4..cb4892ed 100644 --- a/mock.go +++ b/mock.go @@ -231,6 +231,7 @@ type mockWalletController struct { prevAddres btcutil.Address publishedTransactions chan *wire.MsgTx index uint32 + utxos []*lnwallet.Utxo } // BackEnd returns "mock" to signify a mock wallet controller. @@ -284,6 +285,13 @@ func (*mockWalletController) CreateSimpleTx(outputs []*wire.TxOut, // need one unspent for the funding transaction. func (m *mockWalletController) ListUnspentWitness(minconfirms, maxconfirms int32) ([]*lnwallet.Utxo, error) { + + // If the mock already has a list of utxos, return it. + if m.utxos != nil { + return m.utxos, nil + } + + // Otherwise create one to return. utxo := &lnwallet.Utxo{ AddressType: lnwallet.WitnessPubKey, Value: btcutil.Amount(10 * btcutil.SatoshiPerBitcoin), diff --git a/pilot.go b/pilot.go index 5748cf97..9a0f402c 100644 --- a/pilot.go +++ b/pilot.go @@ -100,6 +100,7 @@ func (c *chanController) OpenChannel(target *btcec.PublicKey, req := &openChanReq{ targetPubkey: target, chainHash: *activeNetParams.GenesisHash, + subtractFees: true, localFundingAmt: amt, pushAmt: 0, minHtlc: minHtlc, diff --git a/server.go b/server.go index f2572ab9..1c2eb050 100644 --- a/server.go +++ b/server.go @@ -1056,6 +1056,8 @@ func newServer(listenAddrs []net.Addr, chanDB *channeldb.DB, ZombieSweeperInterval: 1 * time.Minute, ReservationTimeout: 10 * time.Minute, MinChanSize: btcutil.Amount(cfg.MinChanSize), + MaxPendingChannels: cfg.MaxPendingChannels, + RejectPush: cfg.RejectPush, NotifyOpenChannelEvent: s.channelNotifier.NotifyOpenChannelEvent, }) if err != nil { @@ -2982,8 +2984,8 @@ type openChanReq struct { chainHash chainhash.Hash - localFundingAmt btcutil.Amount - remoteFundingAmt btcutil.Amount + subtractFees bool + localFundingAmt btcutil.Amount pushAmt lnwire.MilliSatoshi