Merge pull request #4467 from cfromknecht/hh-cache-followup

chainntfns: disable height-hint cache followups

chainntnfs/bitcoindnotify/bitcoind_test.go

@@ -40,8 +40,8 @@ func initHintCache(t *testing.T) *chainntnfs.HeightHintCache {
 	if err != nil {
 		t.Fatalf("unable to create db: %v", err)
 	}
-	testCfg := chainntnfs.Config{
+	testCfg := chainntnfs.CacheConfig{
-		HeightHintCacheQueryDisable: false,
+		QueryDisable: false,
 	}
 	hintCache, err := chainntnfs.NewHeightHintCache(testCfg, db)
 	if err != nil {

chainntnfs/btcdnotify/btcd_test.go

@@ -38,8 +38,8 @@ func initHintCache(t *testing.T) *chainntnfs.HeightHintCache {
 	if err != nil {
 		t.Fatalf("unable to create db: %v", err)
 	}
-	testCfg := chainntnfs.Config{
+	testCfg := chainntnfs.CacheConfig{
-		HeightHintCacheQueryDisable: false,
+		QueryDisable: false,
 	}
 	hintCache, err := chainntnfs.NewHeightHintCache(testCfg, db)
 	if err != nil {

chainntnfs/height_hint_cache.go

@@ -35,13 +35,13 @@ var (
 	ErrConfirmHintNotFound = errors.New("confirm hint not found")
 )
 
-// Config contains the HeightHintCache configuration
+// CacheConfig contains the HeightHintCache configuration
-type Config struct {
+type CacheConfig struct {
-	// HeightHintCacheQueryDisable prevents reliance on the Height Hint Cache.
+	// QueryDisable prevents reliance on the Height Hint Cache.  This is
-	// This is necessary to recover from an edge case when the height
+	// necessary to recover from an edge case when the height recorded in
-	// recorded in the cache is higher than the actual height of a spend,
+	// the cache is higher than the actual height of a spend, causing a
-	// causing a channel to become "stuck" in a pending close state.
+	// channel to become "stuck" in a pending close state.
-	HeightHintCacheQueryDisable bool
+	QueryDisable bool
 }
 
 // SpendHintCache is an interface whose duty is to cache spend hints for
@@ -83,7 +83,7 @@ type ConfirmHintCache interface {
 // ConfirmHintCache interfaces backed by a channeldb DB instance where the hints
 // will be stored.
 type HeightHintCache struct {
-	cfg Config
+	cfg CacheConfig
 	db  *channeldb.DB
 }
 
@@ -93,7 +93,7 @@ var _ SpendHintCache = (*HeightHintCache)(nil)
 var _ ConfirmHintCache = (*HeightHintCache)(nil)
 
 // NewHeightHintCache returns a new height hint cache backed by a database.
-func NewHeightHintCache(cfg Config, db *channeldb.DB) (*HeightHintCache, error) {
+func NewHeightHintCache(cfg CacheConfig, db *channeldb.DB) (*HeightHintCache, error) {
 	cache := &HeightHintCache{cfg, db}
 	if err := cache.initBuckets(); err != nil {
 		return nil, err
@@ -158,8 +158,9 @@ func (c *HeightHintCache) CommitSpendHint(height uint32,
 // cache for the outpoint.
 func (c *HeightHintCache) QuerySpendHint(spendRequest SpendRequest) (uint32, error) {
 	var hint uint32
-	if c.cfg.HeightHintCacheQueryDisable {
+	if c.cfg.QueryDisable {
-		Log.Debugf("Ignoring spend height hint for %v (height hint cache query disabled)", spendRequest)
+		Log.Debugf("Ignoring spend height hint for %v (height hint cache "+
+			"query disabled)", spendRequest)
 		return 0, nil
 	}
 	err := kvdb.View(c.db, func(tx kvdb.RTx) error {
@@ -256,8 +257,9 @@ func (c *HeightHintCache) CommitConfirmHint(height uint32,
 // the cache for the transaction hash.
 func (c *HeightHintCache) QueryConfirmHint(confRequest ConfRequest) (uint32, error) {
 	var hint uint32
-	if c.cfg.HeightHintCacheQueryDisable {
+	if c.cfg.QueryDisable {
-		Log.Debugf("Ignoring confirmation height hint for %v (height hint cache query disabled)", confRequest)
+		Log.Debugf("Ignoring confirmation height hint for %v (height hint "+
+			"cache query disabled)", confRequest)
 		return 0, nil
 	}
 	err := kvdb.View(c.db, func(tx kvdb.RTx) error {

chainntnfs/height_hint_cache_test.go

@@ -8,11 +8,22 @@ import (
 	"github.com/btcsuite/btcd/chaincfg/chainhash"
 	"github.com/btcsuite/btcd/wire"
 	"github.com/lightningnetwork/lnd/channeldb"
+	"github.com/stretchr/testify/require"
 )
 
 func initHintCache(t *testing.T) *HeightHintCache {
 	t.Helper()
 
+	defaultCfg := CacheConfig{
+		QueryDisable: false,
+	}
+
+	return initHintCacheWithConfig(t, defaultCfg)
+}
+
+func initHintCacheWithConfig(t *testing.T, cfg CacheConfig) *HeightHintCache {
+	t.Helper()
+
 	tempDir, err := ioutil.TempDir("", "kek")
 	if err != nil {
 		t.Fatalf("unable to create temp dir: %v", err)
@@ -21,10 +32,7 @@ func initHintCache(t *testing.T) *HeightHintCache {
 	if err != nil {
 		t.Fatalf("unable to create db: %v", err)
 	}
-	testCfg := Config{
+	hintCache, err := NewHeightHintCache(cfg, db)
-		HeightHintCacheQueryDisable: false,
-	}
-	hintCache, err := NewHeightHintCache(testCfg, db)
 	if err != nil {
 		t.Fatalf("unable to create hint cache: %v", err)
 	}
@@ -154,3 +162,42 @@ func TestHeightHintCacheSpends(t *testing.T) {
 		}
 	}
 }
+
+// TestQueryDisable asserts querying for confirmation or spend hints always
+// return height zero when QueryDisabled is set to true in the CacheConfig.
+func TestQueryDisable(t *testing.T) {
+	cfg := CacheConfig{
+		QueryDisable: true,
+	}
+
+	hintCache := initHintCacheWithConfig(t, cfg)
+
+	// Insert a new confirmation hint with a non-zero height.
+	const confHeight = 100
+	confRequest := ConfRequest{
+		TxID: chainhash.Hash{0x01, 0x02, 0x03},
+	}
+	err := hintCache.CommitConfirmHint(confHeight, confRequest)
+	require.Nil(t, err)
+
+	// Query for the confirmation hint, which should return zero.
+	cachedConfHeight, err := hintCache.QueryConfirmHint(confRequest)
+	require.Nil(t, err)
+	require.Equal(t, uint32(0), cachedConfHeight)
+
+	// Insert a new spend hint with a non-zero height.
+	const spendHeight = 200
+	spendRequest := SpendRequest{
+		OutPoint: wire.OutPoint{
+			Hash:  chainhash.Hash{0x4, 0x05, 0x06},
+			Index: 42,
+		},
+	}
+	err = hintCache.CommitSpendHint(spendHeight, spendRequest)
+	require.Nil(t, err)
+
+	// Query for the spend hint, which should return zero.
+	cachedSpendHeight, err := hintCache.QuerySpendHint(spendRequest)
+	require.Nil(t, err)
+	require.Equal(t, uint32(0), cachedSpendHeight)
+}

chainntnfs/interface_test.go

@@ -1911,8 +1911,8 @@ func TestInterfaces(t *testing.T) {
 		if err != nil {
 			t.Fatalf("unable to create db: %v", err)
 		}
-		testCfg := chainntnfs.Config{
+		testCfg := chainntnfs.CacheConfig{
-			HeightHintCacheQueryDisable: false,
+			QueryDisable: false,
 		}
 		hintCache, err := chainntnfs.NewHeightHintCache(testCfg, db)
 		if err != nil {

chainregistry.go

@@ -219,8 +219,8 @@ func newChainControlFromConfig(cfg *Config, chanDB *channeldb.DB,
 
 	var err error
 
-	heightHintCacheConfig := chainntnfs.Config{
+	heightHintCacheConfig := chainntnfs.CacheConfig{
-		HeightHintCacheQueryDisable: cfg.HeightHintCacheQueryDisable,
+		QueryDisable: cfg.HeightHintCacheQueryDisable,
 	}
 	if cfg.HeightHintCacheQueryDisable {
 		ltndLog.Infof("Height Hint Cache Queries disabled")

lnwallet/interface_test.go

@@ -3014,8 +3014,8 @@ func TestLightningWallet(t *testing.T) {
 	if err != nil {
 		t.Fatalf("unable to create db: %v", err)
 	}
-	testCfg := chainntnfs.Config{
+	testCfg := chainntnfs.CacheConfig{
-		HeightHintCacheQueryDisable: false,
+		QueryDisable: false,
 	}
 	hintCache, err := chainntnfs.NewHeightHintCache(testCfg, db)
 	if err != nil {