Merge pull request #4412 from aantonop/height_hint_cache_query_disable

New config option - disable height hint cache queries

chainntnfs/bitcoindnotify/bitcoind_test.go

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

chainntnfs/btcdnotify/btcd_test.go

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

chainntnfs/height_hint_cache.go

@@ -35,6 +35,15 @@ var (
 	ErrConfirmHintNotFound = errors.New("confirm hint not found")
 )
 
+// Config contains the HeightHintCache configuration
+type Config struct {
+	// HeightHintCacheQueryDisable prevents reliance on the Height Hint Cache.
+	// This is necessary to recover from an edge case when the height
+	// recorded in the cache is higher than the actual height of a spend,
+	// causing a channel to become "stuck" in a pending close state.
+	HeightHintCacheQueryDisable bool
+}
+
 // SpendHintCache is an interface whose duty is to cache spend hints for
 // outpoints. A spend hint is defined as the earliest height in the chain at
 // which an outpoint could have been spent within.
@@ -74,7 +83,8 @@ type ConfirmHintCache interface {
 // ConfirmHintCache interfaces backed by a channeldb DB instance where the hints
 // will be stored.
 type HeightHintCache struct {
-	db *channeldb.DB
+	cfg Config
+	db  *channeldb.DB
 }
 
 // Compile-time checks to ensure HeightHintCache satisfies the SpendHintCache
@@ -83,8 +93,8 @@ var _ SpendHintCache = (*HeightHintCache)(nil)
 var _ ConfirmHintCache = (*HeightHintCache)(nil)
 
 // NewHeightHintCache returns a new height hint cache backed by a database.
-func NewHeightHintCache(db *channeldb.DB) (*HeightHintCache, error) {
-	cache := &HeightHintCache{db}
+func NewHeightHintCache(cfg Config, db *channeldb.DB) (*HeightHintCache, error) {
+	cache := &HeightHintCache{cfg, db}
 	if err := cache.initBuckets(); err != nil {
 		return nil, err
 	}
@@ -148,6 +158,10 @@ 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 {
+		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 {
 		spendHints := tx.ReadBucket(spendHintBucket)
 		if spendHints == nil {
@@ -242,6 +256,10 @@ 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 {
+		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 {
 		confirmHints := tx.ReadBucket(confirmHintBucket)
 		if confirmHints == nil {

chainntnfs/height_hint_cache_test.go

@@ -21,7 +21,10 @@ func initHintCache(t *testing.T) *HeightHintCache {
 	if err != nil {
 		t.Fatalf("unable to create db: %v", err)
 	}
-	hintCache, err := NewHeightHintCache(db)
+	testCfg := Config{
+		HeightHintCacheQueryDisable: false,
+	}
+	hintCache, err := NewHeightHintCache(testCfg, db)
 	if err != nil {
 		t.Fatalf("unable to create hint cache: %v", err)
 	}

chainntnfs/interface_test.go

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

chainregistry.go

@@ -219,8 +219,14 @@ func newChainControlFromConfig(cfg *Config, chanDB *channeldb.DB,
 
 	var err error
 
+	heightHintCacheConfig := chainntnfs.Config{
+		HeightHintCacheQueryDisable: cfg.HeightHintCacheQueryDisable,
+	}
+	if cfg.HeightHintCacheQueryDisable {
+		ltndLog.Infof("Height Hint Cache Queries disabled")
+	}
 	// Initialize the height hint cache within the chain directory.
-	hintCache, err := chainntnfs.NewHeightHintCache(chanDB)
+	hintCache, err := chainntnfs.NewHeightHintCache(heightHintCacheConfig, chanDB)
 	if err != nil {
 		return nil, fmt.Errorf("unable to initialize height hint "+
 			"cache: %v", err)

config.go

@@ -58,6 +58,7 @@ const (
 	defaultChanStatusSampleInterval      = time.Minute
 	defaultChanEnableTimeout             = 19 * time.Minute
 	defaultChanDisableTimeout            = 20 * time.Minute
+	defaultHeightHintCacheQueryDisable   = false
 	defaultMaxLogFiles                   = 3
 	defaultMaxLogFileSize                = 10
 	defaultMinBackoff                    = time.Second
@@ -211,10 +212,10 @@ type Config struct {
 	ChanEnableTimeout             time.Duration `long:"chan-enable-timeout" description:"The duration that a peer connection must be stable before attempting to send a channel update to reenable or cancel a pending disables of the peer's channels on the network."`
 	ChanDisableTimeout            time.Duration `long:"chan-disable-timeout" description:"The duration that must elapse after first detecting that an already active channel is actually inactive and sending channel update disabling it to the network. The pending disable can be canceled if the peer reconnects and becomes stable for chan-enable-timeout before the disable update is sent."`
 	ChanStatusSampleInterval      time.Duration `long:"chan-status-sample-interval" description:"The polling interval between attempts to detect if an active channel has become inactive due to its peer going offline."`
-
-	Alias       string `long:"alias" description:"The node alias. Used as a moniker by peers and intelligence services"`
-	Color       string `long:"color" description:"The color of the node in hex format (i.e. '#3399FF'). Used to customize node appearance in intelligence services"`
-	MinChanSize int64  `long:"minchansize" description:"The smallest channel size (in satoshis) that we should accept. Incoming channels smaller than this will be rejected"`
+	HeightHintCacheQueryDisable   bool          `long:"height-hint-cache-query-disable" description:"Disable queries from the height-hint cache to try to recover channels stuck in the pending close state. Disabling height hint queries may cause longer chain rescans, resulting in a performance hit. Unset this after channels are unstuck so you can get better performance again."`
+	Alias                         string        `long:"alias" description:"The node alias. Used as a moniker by peers and intelligence services"`
+	Color                         string        `long:"color" description:"The color of the node in hex format (i.e. '#3399FF'). Used to customize node appearance in intelligence services"`
+	MinChanSize                   int64         `long:"minchansize" description:"The smallest channel size (in satoshis) that we should accept. Incoming channels smaller than this will be rejected"`
 
 	NumGraphSyncPeers      int           `long:"numgraphsyncpeers" description:"The number of peers that we should receive new graph updates from. This option can be tuned to save bandwidth for light clients or routing nodes."`
 	HistoricalSyncInterval time.Duration `long:"historicalsyncinterval" description:"The polling interval between historical graph sync attempts. Each historical graph sync attempt ensures we reconcile with the remote peer's graph from the genesis block."`
@@ -347,6 +348,7 @@ func DefaultConfig() Config {
 		ChanStatusSampleInterval:      defaultChanStatusSampleInterval,
 		ChanEnableTimeout:             defaultChanEnableTimeout,
 		ChanDisableTimeout:            defaultChanDisableTimeout,
+		HeightHintCacheQueryDisable:   defaultHeightHintCacheQueryDisable,
 		Alias:                         defaultAlias,
 		Color:                         defaultColor,
 		MinChanSize:                   int64(minChanFundingSize),

lnwallet/interface_test.go

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