Skip to content

Commit

Permalink
chainfee: make sure web API has been started before estimating
Browse files Browse the repository at this point in the history
Previously we may get a floor feerate when calling `EstimateFeePerKW`
due to the fee estimator not finishing its startup process, which gives
us an empty fee map.

This is now fixed to return an error if the estimator is not started.
  • Loading branch information
yyforyongyu committed Jul 22, 2024
1 parent c69b439 commit 9871b3e
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 26 deletions.
62 changes: 40 additions & 22 deletions lnwallet/chainfee/estimator.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"net"
"net/http"
"sync"
"sync/atomic"
"time"

"github.com/btcsuite/btcd/btcutil"
Expand Down Expand Up @@ -725,8 +726,8 @@ var _ WebAPIFeeSource = (*SparseConfFeeSource)(nil)
// WebAPIEstimator is an implementation of the Estimator interface that
// queries an HTTP-based fee estimation from an existing web API.
type WebAPIEstimator struct {
started sync.Once
stopped sync.Once
started atomic.Bool
stopped atomic.Bool

// apiSource is the backing web API source we'll use for our queries.
apiSource WebAPIFeeSource
Expand Down Expand Up @@ -792,6 +793,12 @@ func NewWebAPIEstimator(api WebAPIFeeSource, noCache bool,
func (w *WebAPIEstimator) EstimateFeePerKW(numBlocks uint32) (
SatPerKWeight, error) {

// If the estimator hasn't been started yet, we'll return an error as
// we can't provide a fee estimate.
if !w.started.Load() {
return 0, fmt.Errorf("estimator not started")
}

if numBlocks > MaxBlockTarget {
numBlocks = MaxBlockTarget
} else if numBlocks < minBlockTarget {
Expand Down Expand Up @@ -831,49 +838,56 @@ func (w *WebAPIEstimator) EstimateFeePerKW(numBlocks uint32) (
//
// NOTE: This method is part of the Estimator interface.
func (w *WebAPIEstimator) Start() error {
log.Infof("Starting Web API fee estimator...")

// Return an error if it's already been started.
if w.started.Load() {
return fmt.Errorf("web API fee estimator already started")
}
defer w.started.Store(true)

// During startup we'll query the API to initialize the fee map.
w.updateFeeEstimates()

// No update loop is needed when we don't cache.
if w.noCache {
return nil
}

var err error
w.started.Do(func() {
log.Infof("Starting web API fee estimator")
feeUpdateTimeout := w.randomFeeUpdateTimeout()

feeUpdateTimeout := w.randomFeeUpdateTimeout()
log.Infof("Web API fee estimator using update timeout of %v",
feeUpdateTimeout)

log.Infof("Web API fee estimator using update timeout of %v",
feeUpdateTimeout)

w.updateFeeTicker = time.NewTicker(feeUpdateTimeout)
w.updateFeeEstimates()
w.updateFeeTicker = time.NewTicker(feeUpdateTimeout)

w.wg.Add(1)
go w.feeUpdateManager()
w.wg.Add(1)
go w.feeUpdateManager()

})

return err
return nil
}

// Stop stops any spawned goroutines and cleans up the resources used by the
// fee estimator.
//
// NOTE: This method is part of the Estimator interface.
func (w *WebAPIEstimator) Stop() error {
log.Infof("Stopping web API fee estimator")

if w.stopped.Swap(true) {
return fmt.Errorf("web API fee estimator already stopped")
}

// Update loop is not running when we don't cache.
if w.noCache {
return nil
}

w.stopped.Do(func() {
log.Infof("Stopping web API fee estimator")
w.updateFeeTicker.Stop()

w.updateFeeTicker.Stop()
close(w.quit)
w.wg.Wait()

close(w.quit)
w.wg.Wait()
})
return nil
}

Expand All @@ -882,6 +896,10 @@ func (w *WebAPIEstimator) Stop() error {
//
// NOTE: This method is part of the Estimator interface.
func (w *WebAPIEstimator) RelayFeePerKW() SatPerKWeight {
if !w.started.Load() {
log.Error("WebAPIEstimator not started")
}

// Get fee estimates now if we don't refresh periodically.
if w.noCache {
w.updateFeeEstimates()
Expand Down
8 changes: 4 additions & 4 deletions lnwallet/chainfee/estimator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,12 +252,12 @@ func TestWebAPIFeeEstimator(t *testing.T) {
feeSource, false, minFeeUpdateTimeout, maxFeeUpdateTimeout,
)

// Test that requesting a fee when no fees have been cached won't fail.
// Test that when the estimator is not started, an error is returned.
feeRate, err := estimator.EstimateFeePerKW(5)
require.NoErrorf(t, err, "expected no error")
require.Equalf(t, FeePerKwFloor, feeRate, "expected fee rate floor "+
"returned when no cached fee rate found")
require.Error(t, err, "expected an error")
require.Zero(t, feeRate, "expected zero fee rate")

// Start the estimator.
require.NoError(t, estimator.Start(), "unable to start fee estimator")

for _, tc := range testCases {
Expand Down

0 comments on commit 9871b3e

Please sign in to comment.