Merge pull request #2181 from wpaulino/btcwallet-notify-received

build+lnwallet: notify wallet upon relevant transaction confirmation

Gopkg.lock

@@ -106,7 +106,7 @@
   revision = "ab6388e0c60ae4834a1f57511e20c17b5f78be4b"
 
 [[projects]]
-  digest = "1:2995aa2bcb95d13a8df309e1dcb6ac20786acb90df5a090bf5e07c2086219ce8"
+  digest = "1:014bf3112e2bc78db2409f1d7b328c642fe27f2e0b5983595b240bf12578f335"
   name = "github.com/btcsuite/btcwallet"
   packages = [
     "chain",
@@ -127,7 +127,7 @@
     "wtxmgr",
   ]
   pruneopts = "UT"
-  revision = "6d43b2e29b5eef0f000a301ee6fbd146db75d118"
+  revision = "4c01c0878c4ea6ff80711dbfe49e49199ca07607"
 
 [[projects]]
   branch = "master"

Gopkg.toml

@@ -72,7 +72,7 @@
 
 [[constraint]]
   name = "github.com/btcsuite/btcwallet"
-  revision = "6d43b2e29b5eef0f000a301ee6fbd146db75d118"
+  revision = "4c01c0878c4ea6ff80711dbfe49e49199ca07607"
 
 [[constraint]]
   name = "github.com/tv42/zbase32"

lnwallet/btcwallet/btcwallet.go

@@ -153,26 +153,13 @@ func (b *BtcWallet) InternalWallet() *base.Wallet {
 //
 // This is a part of the WalletController interface.
 func (b *BtcWallet) Start() error {
-	// Establish an RPC connection in addition to starting the goroutines
+	// We'll start by unlocking the wallet and ensuring that the KeyScope:
-	// in the underlying wallet.
+	// (1017, 1) exists within the internal waddrmgr. We'll need this in
-	if err := b.chain.Start(); err != nil {
+	// order to properly generate the keys required for signing various
-		return err
+	// contracts.
-	}
-
-	// Start the underlying btcwallet core.
-	b.wallet.Start()
-
-	// Pass the rpc client into the wallet so it can sync up to the
-	// current main chain.
-	b.wallet.SynchronizeRPC(b.chain)
-
 	if err := b.wallet.Unlock(b.cfg.PrivatePass, nil); err != nil {
 		return err
 	}
-
-	// We'll now ensure that the KeyScope: (1017, 1) exists within the
-	// internal waddrmgr. We'll need this in order to properly generate the
-	// keys required for signing various contracts.
 	_, err := b.wallet.Manager.FetchScopedKeyManager(b.chainKeyScope)
 	if err != nil {
 		// If the scope hasn't yet been created (it wouldn't been
@@ -191,6 +178,19 @@ func (b *BtcWallet) Start() error {
 		}
 	}
 
+	// Establish an RPC connection in addition to starting the goroutines
+	// in the underlying wallet.
+	if err := b.chain.Start(); err != nil {
+		return err
+	}
+
+	// Start the underlying btcwallet core.
+	b.wallet.Start()
+
+	// Pass the rpc client into the wallet so it can sync up to the
+	// current main chain.
+	b.wallet.SynchronizeRPC(b.chain)
+
 	return nil
 }
 
@@ -714,6 +714,11 @@ func (b *BtcWallet) SubscribeTransactions() (lnwallet.TransactionSubscription, e
 //
 // This is a part of the WalletController interface.
 func (b *BtcWallet) IsSynced() (bool, int64, error) {
+	// First, we'll ensure the wallet is not currently undergoing a rescan.
+	if !b.wallet.ChainSynced() {
+		return false, 0, nil
+	}
+
 	// Grab the best chain state the wallet is currently aware of.
 	syncState := b.wallet.Manager.SyncedTo()
 

lnwallet/btcwallet/signer.go

@@ -44,7 +44,14 @@ func (b *BtcWallet) FetchInputInfo(prevOut *wire.OutPoint) (*wire.TxOut, error)
 		return nil, lnwallet.ErrNotMine
 	}
 
+	// With the output retrieved, we'll make an additional check to ensure
+	// we actually have control of this output. We do this because the check
+	// above only guarantees that the transaction is somehow relevant to us,
+	// like in the event of us being the sender of the transaction.
 	output = txDetail.TxRecord.MsgTx.TxOut[prevOut.Index]
+	if _, err := b.fetchOutputAddr(output.PkScript); err != nil {
+		return nil, err
+	}
 
 	b.cacheMtx.Lock()
 	b.utxoCache[*prevOut] = output
@@ -72,7 +79,7 @@ func (b *BtcWallet) fetchOutputAddr(script []byte) (waddrmgr.ManagedAddress, err
 		}
 	}
 
-	return nil, errors.Errorf("address not found")
+	return nil, lnwallet.ErrNotMine
 }
 
 // fetchPrivKey attempts to retrieve the raw private key corresponding to the
@@ -196,7 +203,7 @@ func (b *BtcWallet) ComputeInputScript(tx *wire.MsgTx,
 	outputScript := signDesc.Output.PkScript
 	walletAddr, err := b.fetchOutputAddr(outputScript)
 	if err != nil {
-		return nil, nil
+		return nil, err
 	}
 
 	pka := walletAddr.(waddrmgr.ManagedPubKeyAddress)

lnwallet/interface_test.go

@@ -134,6 +134,124 @@ func assertReservationDeleted(res *lnwallet.ChannelReservation, t *testing.T) {
 	}
 }
 
+// mineAndAssertTxInBlock asserts that a transaction is included within the next
+// block mined.
+func mineAndAssertTxInBlock(t *testing.T, miner *rpctest.Harness,
+	txid chainhash.Hash) {
+
+	t.Helper()
+
+	// First, we'll wait for the transaction to arrive in the mempool.
+	if err := waitForMempoolTx(miner, &txid); err != nil {
+		t.Fatalf("unable to find %v in the mempool: %v", txid, err)
+	}
+
+	// We'll mined a block to confirm it.
+	blockHashes, err := miner.Node.Generate(1)
+	if err != nil {
+		t.Fatalf("unable to generate new block: %v", err)
+	}
+
+	// Finally, we'll check it was actually mined in this block.
+	block, err := miner.Node.GetBlock(blockHashes[0])
+	if err != nil {
+		t.Fatalf("unable to get block %v: %v", blockHashes[0], err)
+	}
+	if len(block.Transactions) != 2 {
+		t.Fatalf("expected 2 transactions in block, found %d",
+			len(block.Transactions))
+	}
+	txHash := block.Transactions[1].TxHash()
+	if txHash != txid {
+		t.Fatalf("expected transaction %v to be mined, found %v", txid,
+			txHash)
+	}
+}
+
+// newPkScript generates a new public key script of the given address type.
+func newPkScript(t *testing.T, w *lnwallet.LightningWallet,
+	addrType lnwallet.AddressType) []byte {
+
+	t.Helper()
+
+	addr, err := w.NewAddress(addrType, false)
+	if err != nil {
+		t.Fatalf("unable to create new address: %v", err)
+	}
+	pkScript, err := txscript.PayToAddrScript(addr)
+	if err != nil {
+		t.Fatalf("unable to create output script: %v", err)
+	}
+
+	return pkScript
+}
+
+// sendCoins is a helper function that encompasses all the things needed for two
+// parties to send on-chain funds to each other.
+func sendCoins(t *testing.T, miner *rpctest.Harness,
+	sender, receiver *lnwallet.LightningWallet, output *wire.TxOut,
+	feeRate lnwallet.SatPerKWeight) *wire.MsgTx {
+
+	t.Helper()
+
+	tx, err := sender.SendOutputs([]*wire.TxOut{output}, 2500)
+	if err != nil {
+		t.Fatalf("unable to send transaction: %v", err)
+	}
+
+	mineAndAssertTxInBlock(t, miner, tx.TxHash())
+
+	if err := waitForWalletSync(miner, sender); err != nil {
+		t.Fatalf("unable to sync alice: %v", err)
+	}
+	if err := waitForWalletSync(miner, receiver); err != nil {
+		t.Fatalf("unable to sync bob: %v", err)
+	}
+
+	return tx
+}
+
+// assertTxInWallet asserts that a transaction exists in the wallet with the
+// expected confirmation status.
+func assertTxInWallet(t *testing.T, w *lnwallet.LightningWallet,
+	txHash chainhash.Hash, confirmed bool) {
+
+	t.Helper()
+
+	// If the backend is Neutrino, then we can't determine unconfirmed
+	// transactions since it's not aware of the mempool.
+	if !confirmed && w.BackEnd() == "neutrino" {
+		return
+	}
+
+	// We'll fetch all of our transaction and go through each one until
+	// finding the expected transaction with its expected confirmation
+	// status.
+	txs, err := w.ListTransactionDetails()
+	if err != nil {
+		t.Fatalf("unable to retrieve transactions: %v", err)
+	}
+	for _, tx := range txs {
+		if tx.Hash != txHash {
+			continue
+		}
+		if tx.NumConfirmations <= 0 && confirmed {
+			t.Fatalf("expected transaction %v to be confirmed",
+				txHash)
+		}
+		if tx.NumConfirmations > 0 && !confirmed {
+			t.Fatalf("expected transaction %v to be unconfirmed",
+				txHash)
+		}
+
+		// We've found the transaction and it matches the desired
+		// confirmation status, so we can exit.
+		return
+	}
+
+	t.Fatalf("transaction %v not found", txHash)
+}
+
 // calcStaticFee calculates appropriate fees for commitment transactions.  This
 // function provides a simple way to allow test balance assertions to take fee
 // calculations into account.
@@ -1962,6 +2080,90 @@ func testReorgWalletBalance(r *rpctest.Harness, w *lnwallet.LightningWallet,
 	}
 }
 
+// testChangeOutputSpendConfirmation ensures that when we attempt to spend a
+// change output created by the wallet, the wallet receives its confirmation
+// once included in the chain.
+func testChangeOutputSpendConfirmation(r *rpctest.Harness,
+	alice, bob *lnwallet.LightningWallet, t *testing.T) {
+
+	// In order to test that we see the confirmation of a transaction that
+	// spends an output created by SendOutputs, we'll start by emptying
+	// Alice's wallet so that no other UTXOs can be picked. To do so, we'll
+	// generate an address for Bob, who will receive all the coins.
+	// Assuming a balance of 80 BTC and a transaction fee of 2500 sat/kw,
+	// we'll craft the following transaction so that Alice doesn't have any
+	// UTXOs left.
+	aliceBalance, err := alice.ConfirmedBalance(0)
+	if err != nil {
+		t.Fatalf("unable to retrieve alice's balance: %v", err)
+	}
+	bobPkScript := newPkScript(t, bob, lnwallet.WitnessPubKey)
+
+	// We'll use a transaction fee of 13020 satoshis, which will allow us to
+	// sweep all of Alice's balance in one transaction containing 1 input
+	// and 1 output.
+	//
+	// TODO(wilmer): replace this once SendOutputs easily supports sending
+	// all funds in one transaction.
+	txFeeRate := lnwallet.SatPerKWeight(2500)
+	txFee := btcutil.Amount(14380)
+	output := &wire.TxOut{
+		Value:    int64(aliceBalance - txFee),
+		PkScript: bobPkScript,
+	}
+	tx := sendCoins(t, r, alice, bob, output, txFeeRate)
+	txHash := tx.TxHash()
+	assertTxInWallet(t, alice, txHash, true)
+	assertTxInWallet(t, bob, txHash, true)
+
+	// With the transaction sent and confirmed, Alice's balance should now
+	// be 0.
+	aliceBalance, err = alice.ConfirmedBalance(0)
+	if err != nil {
+		t.Fatalf("unable to retrieve alice's balance: %v", err)
+	}
+	if aliceBalance != 0 {
+		t.Fatalf("expected alice's balance to be 0 BTC, found %v",
+			aliceBalance)
+	}
+
+	// Now, we'll send an output back to Alice from Bob of 1 BTC.
+	alicePkScript := newPkScript(t, alice, lnwallet.WitnessPubKey)
+	output = &wire.TxOut{
+		Value:    btcutil.SatoshiPerBitcoin,
+		PkScript: alicePkScript,
+	}
+	tx = sendCoins(t, r, bob, alice, output, txFeeRate)
+	txHash = tx.TxHash()
+	assertTxInWallet(t, alice, txHash, true)
+	assertTxInWallet(t, bob, txHash, true)
+
+	// Alice now has an available output to spend, but it was not a change
+	// output, which is what the test expects. Therefore, we'll generate one
+	// by sending Bob back some coins.
+	output = &wire.TxOut{
+		Value:    btcutil.SatoshiPerBitcent,
+		PkScript: bobPkScript,
+	}
+	tx = sendCoins(t, r, alice, bob, output, txFeeRate)
+	txHash = tx.TxHash()
+	assertTxInWallet(t, alice, txHash, true)
+	assertTxInWallet(t, bob, txHash, true)
+
+	// Then, we'll spend the change output and ensure we see its
+	// confirmation come in.
+	tx = sendCoins(t, r, alice, bob, output, txFeeRate)
+	txHash = tx.TxHash()
+	assertTxInWallet(t, alice, txHash, true)
+	assertTxInWallet(t, bob, txHash, true)
+
+	// Finally, we'll replenish Alice's wallet with some more coins to
+	// ensure she has enough for any following test cases.
+	if err := loadTestCredits(r, alice, 20, 4); err != nil {
+		t.Fatalf("unable to replenish alice's wallet: %v", err)
+	}
+}
+
 type walletTestCase struct {
 	name string
 	test func(miner *rpctest.Harness, alice, bob *lnwallet.LightningWallet,
@@ -1969,6 +2171,13 @@ type walletTestCase struct {
 }
 
 var walletTests = []walletTestCase{
+	{
+		// TODO(wilmer): this test should remain first until the wallet
+		// can properly craft a transaction that spends all of its
+		// on-chain funds.
+		name: "change output spend confirmation",
+		test: testChangeOutputSpendConfirmation,
+	},
 	{
 		name: "insane fee reject",
 		test: testReservationInitiatorBalanceBelowDustCancel,