Merge pull request #1726 from Roasbeef/fix-closed-chan-summaries-and-chancfg

multi: ensure closed channel summaries are fully populated, ensure KeyLocators are always set

Gopkg.lock

@@ -120,7 +120,7 @@
   revision = "ab6388e0c60ae4834a1f57511e20c17b5f78be4b"
 
 [[projects]]
-  digest = "1:a9bb59675f2aa863eff2bab6f0668b5e6feacaf6b7978fe5a654b068d5ca8e36"
+  digest = "1:04bf3f47dafa64588795c5e0329dc662e867c3afa191051821dbacbe09ba2ca8"
   name = "github.com/btcsuite/btcwallet"
   packages = [
     "chain",
@@ -140,7 +140,7 @@
     "wtxmgr",
   ]
   pruneopts = "UT"
-  revision = "1ede0a1a66bad8f7db796615e496a0a0faba5182"
+  revision = "5fb94231d0c814f02ffc3110eee588278151b4e1"
 
 [[projects]]
   branch = "master"
@@ -279,7 +279,7 @@
   revision = "462a8a75388506b68f76661af8d649f0b88e5301"
 
 [[projects]]
-  digest = "1:461543cea211913463b96ed3bf8cbdadf23c5ac8ec205bcbbc79edeba9e93a9a"
+  digest = "1:11ab77a97c0db5cfe9c82f16cb7c47213612033e8bd711e6ddc9a32615fc747d"
   name = "github.com/lightninglabs/neutrino"
   packages = [
     ".",
@@ -288,7 +288,7 @@
     "headerlist",
   ]
   pruneopts = "UT"
-  revision = "b451667d69910cd20995452c56e02441896a3349"
+  revision = "0d0ce901538af81e234c1b2376babf20fe976b09"
 
 [[projects]]
   digest = "1:58ab6d6525898cbeb86dc29a68f8e9bfe95254b9032134eb9458779574872260"

Gopkg.toml

@@ -44,7 +44,7 @@
 
 [[constraint]]
   name = "github.com/lightninglabs/neutrino"
-  revision = "b451667d69910cd20995452c56e02441896a3349"
+  revision = "0d0ce901538af81e234c1b2376babf20fe976b09"
 
 [[constraint]]
   name = "github.com/lightningnetwork/lightning-onion"
@@ -72,7 +72,7 @@
 
 [[constraint]]
   name = "github.com/btcsuite/btcwallet"
-  revision = "1ede0a1a66bad8f7db796615e496a0a0faba5182"
+  revision = "5fb94231d0c814f02ffc3110eee588278151b4e1"
 
 [[constraint]]
   name = "github.com/tv42/zbase32"

channeldb/channel_test.go

@@ -165,18 +165,38 @@ func createTestChannelState(cdb *DB) (*OpenChannel, error) {
 		CsvDelay: uint16(rand.Int31()),
 		MultiSigKey: keychain.KeyDescriptor{
 			PubKey: privKey.PubKey(),
+			KeyLocator: keychain.KeyLocator{
+				Family: keychain.KeyFamilyMultiSig,
+				Index:  9,
+			},
 		},
 		RevocationBasePoint: keychain.KeyDescriptor{
 			PubKey: privKey.PubKey(),
+			KeyLocator: keychain.KeyLocator{
+				Family: keychain.KeyFamilyRevocationBase,
+				Index:  8,
+			},
 		},
 		PaymentBasePoint: keychain.KeyDescriptor{
 			PubKey: privKey.PubKey(),
+			KeyLocator: keychain.KeyLocator{
+				Family: keychain.KeyFamilyPaymentBase,
+				Index:  7,
+			},
 		},
 		DelayBasePoint: keychain.KeyDescriptor{
 			PubKey: privKey.PubKey(),
+			KeyLocator: keychain.KeyLocator{
+				Family: keychain.KeyFamilyDelayBase,
+				Index:  6,
+			},
 		},
 		HtlcBasePoint: keychain.KeyDescriptor{
 			PubKey: privKey.PubKey(),
+			KeyLocator: keychain.KeyLocator{
+				Family: keychain.KeyFamilyHtlcBase,
+				Index:  5,
+			},
 		},
 	}
 
@@ -772,6 +792,7 @@ func TestFetchClosedChannels(t *testing.T) {
 		TimeLockedBalance: state.RemoteCommitment.LocalBalance.ToSatoshis() + 10000,
 		CloseType:         RemoteForceClose,
 		IsPending:         true,
+		LocalChanConfig:   state.LocalChanCfg,
 	}
 	if err := state.CloseChannel(summary); err != nil {
 		t.Fatalf("unable to close channel: %v", err)

contractcourt/chain_watcher.go

@@ -502,16 +502,19 @@ func (c *chainWatcher) dispatchCooperativeClose(commitSpend *chainntnfs.SpendDet
 	// database. We can do this as a cooperatively closed channel has all
 	// its outputs resolved after only one confirmation.
 	closeSummary := &channeldb.ChannelCloseSummary{
-		ChanPoint:      c.cfg.chanState.FundingOutpoint,
+		ChanPoint:               c.cfg.chanState.FundingOutpoint,
-		ChainHash:      c.cfg.chanState.ChainHash,
+		ChainHash:               c.cfg.chanState.ChainHash,
-		ClosingTXID:    *commitSpend.SpenderTxHash,
+		ClosingTXID:             *commitSpend.SpenderTxHash,
-		RemotePub:      c.cfg.chanState.IdentityPub,
+		RemotePub:               c.cfg.chanState.IdentityPub,
-		Capacity:       c.cfg.chanState.Capacity,
+		Capacity:                c.cfg.chanState.Capacity,
-		CloseHeight:    uint32(commitSpend.SpendingHeight),
+		CloseHeight:             uint32(commitSpend.SpendingHeight),
-		SettledBalance: localAmt,
+		SettledBalance:          localAmt,
-		CloseType:      channeldb.CooperativeClose,
+		CloseType:               channeldb.CooperativeClose,
-		ShortChanID:    c.cfg.chanState.ShortChanID(),
+		ShortChanID:             c.cfg.chanState.ShortChanID(),
-		IsPending:      false,
+		IsPending:               false,
+		RemoteCurrentRevocation: c.cfg.chanState.RemoteCurrentRevocation,
+		RemoteNextRevocation:    c.cfg.chanState.RemoteNextRevocation,
+		LocalChanConfig:         c.cfg.chanState.LocalChanCfg,
 	}
 	err := c.cfg.chanState.CloseChannel(closeSummary)
 	if err != nil && err != channeldb.ErrNoActiveChannels &&
@@ -559,15 +562,18 @@ func (c *chainWatcher) dispatchLocalForceClose(
 	// usage by related sub-systems.
 	chanSnapshot := forceClose.ChanSnapshot
 	closeSummary := &channeldb.ChannelCloseSummary{
-		ChanPoint:   chanSnapshot.ChannelPoint,
+		ChanPoint:               chanSnapshot.ChannelPoint,
-		ChainHash:   chanSnapshot.ChainHash,
+		ChainHash:               chanSnapshot.ChainHash,
-		ClosingTXID: forceClose.CloseTx.TxHash(),
+		ClosingTXID:             forceClose.CloseTx.TxHash(),
-		RemotePub:   &chanSnapshot.RemoteIdentity,
+		RemotePub:               &chanSnapshot.RemoteIdentity,
-		Capacity:    chanSnapshot.Capacity,
+		Capacity:                chanSnapshot.Capacity,
-		CloseType:   channeldb.LocalForceClose,
+		CloseType:               channeldb.LocalForceClose,
-		IsPending:   true,
+		IsPending:               true,
-		ShortChanID: c.cfg.chanState.ShortChanID(),
+		ShortChanID:             c.cfg.chanState.ShortChanID(),
-		CloseHeight: uint32(commitSpend.SpendingHeight),
+		CloseHeight:             uint32(commitSpend.SpendingHeight),
+		RemoteCurrentRevocation: c.cfg.chanState.RemoteCurrentRevocation,
+		RemoteNextRevocation:    c.cfg.chanState.RemoteNextRevocation,
+		LocalChanConfig:         c.cfg.chanState.LocalChanCfg,
 	}
 
 	// If our commitment output isn't dust or we have active HTLC's on the
@@ -739,16 +745,19 @@ func (c *chainWatcher) dispatchContractBreach(spendEvent *chainntnfs.SpendDetail
 	// TODO(roasbeef): instead mark we got all the monies?
 	settledBalance := remoteCommit.LocalBalance.ToSatoshis()
 	closeSummary := channeldb.ChannelCloseSummary{
-		ChanPoint:      c.cfg.chanState.FundingOutpoint,
+		ChanPoint:               c.cfg.chanState.FundingOutpoint,
-		ChainHash:      c.cfg.chanState.ChainHash,
+		ChainHash:               c.cfg.chanState.ChainHash,
-		ClosingTXID:    *spendEvent.SpenderTxHash,
+		ClosingTXID:             *spendEvent.SpenderTxHash,
-		CloseHeight:    spendHeight,
+		CloseHeight:             spendHeight,
-		RemotePub:      c.cfg.chanState.IdentityPub,
+		RemotePub:               c.cfg.chanState.IdentityPub,
-		Capacity:       c.cfg.chanState.Capacity,
+		Capacity:                c.cfg.chanState.Capacity,
-		SettledBalance: settledBalance,
+		SettledBalance:          settledBalance,
-		CloseType:      channeldb.BreachClose,
+		CloseType:               channeldb.BreachClose,
-		IsPending:      true,
+		IsPending:               true,
-		ShortChanID:    c.cfg.chanState.ShortChanID(),
+		ShortChanID:             c.cfg.chanState.ShortChanID(),
+		RemoteCurrentRevocation: c.cfg.chanState.RemoteCurrentRevocation,
+		RemoteNextRevocation:    c.cfg.chanState.RemoteNextRevocation,
+		LocalChanConfig:         c.cfg.chanState.LocalChanCfg,
 	}
 
 	if err := c.cfg.chanState.CloseChannel(&closeSummary); err != nil {

fundingmanager.go

@@ -521,12 +521,17 @@ func (f *fundingManager) Start() error {
 				// mined since the channel was initiated reaches
 				// maxWaitNumBlocksFundingConf and we are not the channel
 				// initiator.
-
+				localBalance := ch.LocalCommitment.LocalBalance.ToSatoshis()
 				closeInfo := &channeldb.ChannelCloseSummary{
-					ChainHash: ch.ChainHash,
+					ChainHash:               ch.ChainHash,
-					ChanPoint: ch.FundingOutpoint,
+					ChanPoint:               ch.FundingOutpoint,
-					RemotePub: ch.IdentityPub,
+					RemotePub:               ch.IdentityPub,
-					CloseType: channeldb.FundingCanceled,
+					Capacity:                ch.Capacity,
+					SettledBalance:          localBalance,
+					CloseType:               channeldb.FundingCanceled,
+					RemoteCurrentRevocation: ch.RemoteCurrentRevocation,
+					RemoteNextRevocation:    ch.RemoteNextRevocation,
+					LocalChanConfig:         ch.LocalChanCfg,
 				}
 
 				if err := ch.CloseChannel(closeInfo); err != nil {
@@ -1371,11 +1376,17 @@ func (f *fundingManager) handleFundingCreated(fmsg *fundingCreatedMsg) {
 	// we use this convenience method to delete the pending OpenChannel
 	// from the database.
 	deleteFromDatabase := func() {
+		localBalance := completeChan.LocalCommitment.LocalBalance.ToSatoshis()
 		closeInfo := &channeldb.ChannelCloseSummary{
-			ChanPoint: completeChan.FundingOutpoint,
+			ChanPoint:               completeChan.FundingOutpoint,
-			ChainHash: completeChan.ChainHash,
+			ChainHash:               completeChan.ChainHash,
-			RemotePub: completeChan.IdentityPub,
+			RemotePub:               completeChan.IdentityPub,
-			CloseType: channeldb.FundingCanceled,
+			CloseType:               channeldb.FundingCanceled,
+			Capacity:                completeChan.Capacity,
+			SettledBalance:          localBalance,
+			RemoteCurrentRevocation: completeChan.RemoteCurrentRevocation,
+			RemoteNextRevocation:    completeChan.RemoteNextRevocation,
+			LocalChanConfig:         completeChan.LocalChanCfg,
 		}
 
 		if err := completeChan.CloseChannel(closeInfo); err != nil {

keychain/btcwallet.go

@@ -139,7 +139,10 @@ func (b *BtcWalletKeyRing) createAccountIfNotExists(
 //
 // NOTE: This is part of the keychain.KeyRing interface.
 func (b *BtcWalletKeyRing) DeriveNextKey(keyFam KeyFamily) (KeyDescriptor, error) {
-	var pubKey *btcec.PublicKey
+	var (
+		pubKey *btcec.PublicKey
+		keyLoc KeyLocator
+	)
 
 	db := b.wallet.Database()
 	err := walletdb.Update(db, func(tx walletdb.ReadWriteTx) error {
@@ -165,7 +168,21 @@ func (b *BtcWalletKeyRing) DeriveNextKey(keyFam KeyFamily) (KeyDescriptor, error
 			return err
 		}
 
-		pubKey = addrs[0].(waddrmgr.ManagedPubKeyAddress).PubKey()
+		// Extract the first address, ensuring that it is of the proper
+		// interface type, otherwise we can't manipulate it below.
+		addr, ok := addrs[0].(waddrmgr.ManagedPubKeyAddress)
+		if !ok {
+			return fmt.Errorf("address is not a managed pubkey " +
+				"addr")
+		}
+
+		pubKey = addr.PubKey()
+
+		_, pathInfo, _ := addr.DerivationInfo()
+		keyLoc = KeyLocator{
+			Family: keyFam,
+			Index:  pathInfo.Index,
+		}
 
 		return nil
 	})
@@ -174,7 +191,8 @@ func (b *BtcWalletKeyRing) DeriveNextKey(keyFam KeyFamily) (KeyDescriptor, error
 	}
 
 	return KeyDescriptor{
-		PubKey: pubKey,
+		PubKey:     pubKey,
+		KeyLocator: keyLoc,
 	}, nil
 }
 

keychain/derivation.go

@@ -83,7 +83,7 @@ const (
 // will vary per channel and use case) is the final element which allows us to
 // deterministically derive keys.
 type KeyLocator struct {
-	// TODO(roasbeef); add the key scope as well??
+	// TODO(roasbeef): add the key scope as well??
 
 	// Family is the family of key being identified.
 	Family KeyFamily
@@ -92,8 +92,8 @@ type KeyLocator struct {
 	Index uint32
 }
 
-// IsEmpty returns true if a KeyLocator is "empty". This may be the case where we
+// IsEmpty returns true if a KeyLocator is "empty". This may be the case where
-// learn of a key from a remote party for a contract, but don't know the
+// we learn of a key from a remote party for a contract, but don't know the
 // precise details of its derivation (as we don't know the private key!).
 func (k KeyLocator) IsEmpty() bool {
 	return k.Family == 0 && k.Index == 0

keychain/interface_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/btcsuite/btcwallet/waddrmgr"
 	"github.com/btcsuite/btcwallet/wallet"
 	"github.com/btcsuite/btcwallet/walletdb"
+	"github.com/davecgh/go-spew/spew"
 
 	_ "github.com/btcsuite/btcwallet/walletdb/bdb" // Required in order to create the default database.
 )
@@ -91,6 +92,14 @@ func createTestBtcWallet(coinType uint32) (func(), *wallet.Wallet, error) {
 	return cleanUp, baseWallet, nil
 }
 
+func assertEqualKeyLocator(t *testing.T, a, b KeyLocator) {
+	t.Helper()
+	if a != b {
+		t.Fatalf("mismatched key locators: expected %v, "+
+			"got %v", spew.Sdump(a), spew.Sdump(b))
+	}
+}
+
 // secretKeyRingConstructor is a function signature that's used as a generic
 // constructor for various implementations of the KeyRing interface. A string
 // naming the returned interface, a function closure that cleans up any
@@ -141,6 +150,8 @@ func TestKeyRingDerivation(t *testing.T) {
 		},
 	}
 
+	const numKeysToDerive = 10
+
 	// For each implementation constructor registered above, we'll execute
 	// an identical set of tests in order to ensure that the interface
 	// adheres to our nominal specification.
@@ -163,10 +174,16 @@ func TestKeyRingDerivation(t *testing.T) {
 					t.Fatalf("unable to derive next for "+
 						"keyFam=%v: %v", keyFam, err)
 				}
+				assertEqualKeyLocator(t,
+					KeyLocator{
+						Family: keyFam,
+						Index:  0,
+					}, keyDesc.KeyLocator,
+				)
 
-				// If we now try to manually derive the *first*
+				// We'll now re-derive that key to ensure that
-				// key, then we should get an identical public
+				// we're able to properly access the key via
-				// key back.
+				// the random access derivation methods.
 				keyLoc := KeyLocator{
 					Family: keyFam,
 					Index:  0,
@@ -176,13 +193,41 @@ func TestKeyRingDerivation(t *testing.T) {
 					t.Fatalf("unable to derive first key for "+
 						"keyFam=%v: %v", keyFam, err)
 				}
-
 				if !keyDesc.PubKey.IsEqual(firstKeyDesc.PubKey) {
-					t.Fatalf("mismatched keys: expected %v, "+
+					t.Fatalf("mismatched keys: expected %x, "+
 						"got %x",
 						keyDesc.PubKey.SerializeCompressed(),
 						firstKeyDesc.PubKey.SerializeCompressed())
 				}
+				assertEqualKeyLocator(t,
+					KeyLocator{
+						Family: keyFam,
+						Index:  0,
+					}, firstKeyDesc.KeyLocator,
+				)
+
+				// If we now try to manually derive the next 10
+				// keys (including the original key), then we
+				// should get an identical public key back and
+				// their KeyLocator information
+				// should be set properly.
+				for i := 0; i < numKeysToDerive+1; i++ {
+					keyLoc := KeyLocator{
+						Family: keyFam,
+						Index:  uint32(i),
+					}
+					keyDesc, err := keyRing.DeriveKey(keyLoc)
+					if err != nil {
+						t.Fatalf("unable to derive first key for "+
+							"keyFam=%v: %v", keyFam, err)
+					}
+
+					// Ensure that the key locator matches
+					// up as well.
+					assertEqualKeyLocator(
+						t, keyLoc, keyDesc.KeyLocator,
+					)
+				}
 
 				// If this succeeds, then we'll also try to
 				// derive a random index within the range.
@@ -191,12 +236,15 @@ func TestKeyRingDerivation(t *testing.T) {
 					Family: keyFam,
 					Index:  randKeyIndex,
 				}
-				_, err = keyRing.DeriveKey(keyLoc)
+				keyDesc, err = keyRing.DeriveKey(keyLoc)
 				if err != nil {
 					t.Fatalf("unable to derive key_index=%v "+
 						"for keyFam=%v: %v",
 						randKeyIndex, keyFam, err)
 				}
+				assertEqualKeyLocator(
+					t, keyLoc, keyDesc.KeyLocator,
+				)
 			}
 		})
 		if !success {

lnwallet/channel.go

@@ -5094,15 +5094,18 @@ func NewUnilateralCloseSummary(chanState *channeldb.OpenChannel, signer Signer,
 	}
 
 	closeSummary := channeldb.ChannelCloseSummary{
-		ChanPoint:      chanState.FundingOutpoint,
+		ChanPoint:               chanState.FundingOutpoint,
-		ChainHash:      chanState.ChainHash,
+		ChainHash:               chanState.ChainHash,
-		ClosingTXID:    *commitSpend.SpenderTxHash,
+		ClosingTXID:             *commitSpend.SpenderTxHash,
-		CloseHeight:    uint32(commitSpend.SpendingHeight),
+		CloseHeight:             uint32(commitSpend.SpendingHeight),
-		RemotePub:      chanState.IdentityPub,
+		RemotePub:               chanState.IdentityPub,
-		Capacity:       chanState.Capacity,
+		Capacity:                chanState.Capacity,
-		SettledBalance: btcutil.Amount(localBalance),
+		SettledBalance:          btcutil.Amount(localBalance),
-		CloseType:      channeldb.RemoteForceClose,
+		CloseType:               channeldb.RemoteForceClose,
-		IsPending:      true,
+		IsPending:               true,
+		RemoteCurrentRevocation: chanState.RemoteCurrentRevocation,
+		RemoteNextRevocation:    chanState.RemoteNextRevocation,
+		LocalChanConfig:         chanState.LocalChanCfg,
 	}
 
 	return &UnilateralCloseSummary{