diff --git a/lnwallet/chainfee/estimator.go b/lnwallet/chainfee/estimator.go index 0ce9ae9b8..49b0790a7 100644 --- a/lnwallet/chainfee/estimator.go +++ b/lnwallet/chainfee/estimator.go @@ -628,9 +628,10 @@ var _ Estimator = (*BitcoindEstimator)(nil) // implementation of this interface in order to allow the WebAPIEstimator to // be fully generic in its logic. type WebAPIFeeSource interface { - // GetFeeMap will query the web API, parse the response and return a - // map of confirmation targets to sat/kw fees. - GetFeeMap() (map[uint32]uint32, error) + // GetFeeInfo will query the web API, parse the response into a + // WebAPIResponse which contains a map of confirmation targets to + // sat/kw fees and min relay feerate. + GetFeeInfo() (WebAPIResponse, error) } // SparseConfFeeSource is an implementation of the WebAPIFeeSource that utilizes @@ -642,30 +643,43 @@ type SparseConfFeeSource struct { URL string } +// WebAPIResponse is the response returned by the fee estimation API. +type WebAPIResponse struct { + // FeeByBlockTarget is a map of confirmation targets to sat/kvb fees. + FeeByBlockTarget map[uint32]uint32 `json:"fee_by_block_target"` + + // MinRelayFeerate is the minimum relay fee in sat/kvb. + MinRelayFeerate SatPerKVByte `json:"min_relay_feerate"` +} + // parseResponse attempts to parse the body of the response generated by the // above query URL. Typically this will be JSON, but the specifics are left to // the WebAPIFeeSource implementation. func (s SparseConfFeeSource) parseResponse(r io.Reader) ( - map[uint32]uint32, error) { + WebAPIResponse, error) { - type jsonResp struct { - FeeByBlockTarget map[uint32]uint32 `json:"fee_by_block_target"` - } - - resp := jsonResp{ + resp := WebAPIResponse{ FeeByBlockTarget: make(map[uint32]uint32), + MinRelayFeerate: 0, } jsonReader := json.NewDecoder(r) if err := jsonReader.Decode(&resp); err != nil { - return nil, err + return WebAPIResponse{}, err } - return resp.FeeByBlockTarget, nil + if resp.MinRelayFeerate == 0 { + log.Errorf("No min relay fee rate available, using default %v", + FeePerKwFloor) + resp.MinRelayFeerate = FeePerKwFloor.FeePerKVByte() + } + + return resp, nil } -// GetFeeMap will query the web API, parse the response and return a map of -// confirmation targets to sat/kw fees. -func (s SparseConfFeeSource) GetFeeMap() (map[uint32]uint32, error) { +// GetFeeInfo will query the web API, parse the response and return a map of +// confirmation targets to sat/kw fees and min relay feerate in a parsed +// response. +func (s SparseConfFeeSource) GetFeeInfo() (WebAPIResponse, error) { // Rather than use the default http.Client, we'll make a custom one // which will allow us to control how long we'll wait to read the // response from the service. This way, if the service is down or @@ -688,20 +702,20 @@ func (s SparseConfFeeSource) GetFeeMap() (map[uint32]uint32, error) { if err != nil { log.Errorf("unable to query web api for fee response: %v", err) - return nil, err + return WebAPIResponse{}, err } defer resp.Body.Close() // Once we've obtained the response, we'll instruct the WebAPIFeeSource // to parse out the body to obtain our final result. - feesByBlockTarget, err := s.parseResponse(resp.Body) + parsedResp, err := s.parseResponse(resp.Body) if err != nil { log.Errorf("unable to parse fee api response: %v", err) - return nil, err + return WebAPIResponse{}, err } - return feesByBlockTarget, nil + return parsedResp, nil } // A compile-time assertion to ensure that SparseConfFeeSource implements the @@ -726,6 +740,7 @@ type WebAPIEstimator struct { // rather than re-querying the API, to prevent an inadvertent DoS attack. feesMtx sync.Mutex feeByBlockTarget map[uint32]uint32 + minRelayFeerate SatPerKVByte // noCache determines whether the web estimator should cache fee // estimates. @@ -837,6 +852,7 @@ func (w *WebAPIEstimator) Start() error { go w.feeUpdateManager() }) + return err } @@ -866,7 +882,15 @@ func (w *WebAPIEstimator) Stop() error { // // NOTE: This method is part of the Estimator interface. func (w *WebAPIEstimator) RelayFeePerKW() SatPerKWeight { - return FeePerKwFloor + // Get fee estimates now if we don't refresh periodically. + if w.noCache { + w.updateFeeEstimates() + } + + log.Infof("Web API returning %v for min relay feerate", + w.minRelayFeerate) + + return w.minRelayFeerate.FeePerKWeight() } // randomFeeUpdateTimeout returns a random timeout between minFeeUpdateTimeout @@ -956,14 +980,21 @@ func (w *WebAPIEstimator) getCachedFee(numBlocks uint32) (uint32, error) { func (w *WebAPIEstimator) updateFeeEstimates() { // Once we've obtained the response, we'll instruct the WebAPIFeeSource // to parse out the body to obtain our final result. - feesByBlockTarget, err := w.apiSource.GetFeeMap() + resp, err := w.apiSource.GetFeeInfo() if err != nil { log.Errorf("unable to get fee response: %v", err) return } + log.Debugf("Received response from source: %s", newLogClosure( + func() string { + resp, _ := json.Marshal(resp) + return string(resp) + })) + w.feesMtx.Lock() - w.feeByBlockTarget = feesByBlockTarget + w.feeByBlockTarget = resp.FeeByBlockTarget + w.minRelayFeerate = resp.MinRelayFeerate w.feesMtx.Unlock() } diff --git a/lnwallet/chainfee/estimator_test.go b/lnwallet/chainfee/estimator_test.go index 51ab5bf29..c8303130d 100644 --- a/lnwallet/chainfee/estimator_test.go +++ b/lnwallet/chainfee/estimator_test.go @@ -107,17 +107,20 @@ func TestSparseConfFeeSource(t *testing.T) { 2: 42, 3: 54321, } - testJSON := map[string]map[uint32]uint32{ - "fee_by_block_target": testFees, + testMinRelayFee := SatPerKVByte(1000) + testResp := WebAPIResponse{ + MinRelayFeerate: testMinRelayFee, + FeeByBlockTarget: testFees, } - jsonResp, err := json.Marshal(testJSON) + + jsonResp, err := json.Marshal(testResp) require.NoError(t, err, "unable to marshal JSON API response") reader := bytes.NewReader(jsonResp) // Finally, ensure the expected map is returned without error. - fees, err := feeSource.parseResponse(reader) + resp, err := feeSource.parseResponse(reader) require.NoError(t, err, "unable to parse API response") - require.Equal(t, testFees, fees, "unexpected fee map returned") + require.Equal(t, testResp, resp, "unexpected resp returned") // Test parsing an improperly formatted JSON API response. badFees := map[string]uint32{"hi": 12345, "hello": 42, "satoshi": 54321} @@ -131,6 +134,45 @@ func TestSparseConfFeeSource(t *testing.T) { require.Error(t, err, "expected error when parsing bad JSON") } +// TestFeeSourceCompatibility checks that when a fee source doesn't return a +// `min_relay_feerate` field in its response, the floor feerate is used. +// +// NOTE: Field `min_relay_feerate` was added in v0.18.3. +func TestFeeSourceCompatibility(t *testing.T) { + t.Parallel() + + // Test that GenQueryURL returns the URL as is. + url := "test" + feeSource := SparseConfFeeSource{URL: url} + + // Test parsing a properly formatted JSON API response. + // + // Create the resp without the `min_relay_feerate` field. + testFees := map[uint32]uint32{ + 1: 12345, + } + testResp := struct { + // FeeByBlockTarget is a map of confirmation targets to sat/kvb + // fees. + FeeByBlockTarget map[uint32]uint32 `json:"fee_by_block_target"` + }{ + FeeByBlockTarget: testFees, + } + + jsonResp, err := json.Marshal(testResp) + require.NoError(t, err, "unable to marshal JSON API response") + reader := bytes.NewReader(jsonResp) + + // Ensure the expected map is returned without error. + resp, err := feeSource.parseResponse(reader) + require.NoError(t, err, "unable to parse API response") + require.Equal(t, testResp.FeeByBlockTarget, resp.FeeByBlockTarget, + "unexpected resp returned") + + // Expect the floor feerate to be used. + require.Equal(t, FeePerKwFloor.FeePerKVByte(), resp.MinRelayFeerate) +} + // TestWebAPIFeeEstimator checks that the WebAPIFeeEstimator returns fee rates // as expected. func TestWebAPIFeeEstimator(t *testing.T) { @@ -194,14 +236,17 @@ func TestWebAPIFeeEstimator(t *testing.T) { // This will create a `feeByBlockTarget` map with the following values, // - 2: 4000 sat/kb // - 6: 2000 sat/kb. - feeRateResp := map[uint32]uint32{ + feeRates := map[uint32]uint32{ minTarget: maxFeeRate, maxTarget: minFeeRate, } + resp := WebAPIResponse{ + FeeByBlockTarget: feeRates, + } // Create a mock fee source and mock its returned map. feeSource := &mockFeeSource{} - feeSource.On("GetFeeMap").Return(feeRateResp, nil) + feeSource.On("GetFeeInfo").Return(resp, nil) estimator, _ := NewWebAPIEstimator( feeSource, false, minFeeUpdateTimeout, maxFeeUpdateTimeout, @@ -234,7 +279,7 @@ func TestWebAPIFeeEstimator(t *testing.T) { exp := SatPerKVByte(tc.expectedFeeRate).FeePerKWeight() require.Equalf(t, exp, est, "target %v failed, fee "+ - "map is %v", tc.target, feeRateResp) + "map is %v", tc.target, feeRate) }) } diff --git a/lnwallet/chainfee/log.go b/lnwallet/chainfee/log.go index d5d040578..914583033 100644 --- a/lnwallet/chainfee/log.go +++ b/lnwallet/chainfee/log.go @@ -27,3 +27,19 @@ func DisableLog() { func UseLogger(logger btclog.Logger) { log = logger } + +// logClosure is used to provide a closure over expensive logging operations so +// don't have to be performed when the logging level doesn't warrant it. +type logClosure func() string + +// String invokes the underlying function and returns the result. +func (c logClosure) String() string { + return c() +} + +// newLogClosure returns a new closure over a function that returns a string +// which itself provides a Stringer interface so that it can be used with the +// logging system. +func newLogClosure(c func() string) logClosure { + return logClosure(c) +} diff --git a/lnwallet/chainfee/mocks.go b/lnwallet/chainfee/mocks.go index e14340d91..3a6025005 100644 --- a/lnwallet/chainfee/mocks.go +++ b/lnwallet/chainfee/mocks.go @@ -12,10 +12,10 @@ type mockFeeSource struct { // WebAPIFeeSource interface. var _ WebAPIFeeSource = (*mockFeeSource)(nil) -func (m *mockFeeSource) GetFeeMap() (map[uint32]uint32, error) { +func (m *mockFeeSource) GetFeeInfo() (WebAPIResponse, error) { args := m.Called() - return args.Get(0).(map[uint32]uint32), args.Error(1) + return args.Get(0).(WebAPIResponse), args.Error(1) } // MockEstimator implements the `Estimator` interface and is used by diff --git a/lnwallet/chainfee/rates.go b/lnwallet/chainfee/rates.go index f5294e4d1..20f4f1e1d 100644 --- a/lnwallet/chainfee/rates.go +++ b/lnwallet/chainfee/rates.go @@ -53,7 +53,7 @@ func (s SatPerKVByte) FeePerKWeight() SatPerKWeight { // String returns a human-readable string of the fee rate. func (s SatPerKVByte) String() string { - return fmt.Sprintf("%v sat/kb", int64(s)) + return fmt.Sprintf("%v sat/kvb", int64(s)) } // SatPerKWeight represents a fee rate in sat/kw.