From efcba4a8360465a57f3af0e4dcc98fa16dc20b48 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Thu, 19 Oct 2023 22:52:35 +0800 Subject: [PATCH] chainfee: simplify `WebAPIFeeSource` interface This commit removes the interface methods `GenQueryURL` and `ParseResponse` so the `WebAPIEstimator` can care less about the implementation details of this interface. --- lnwallet/chainfee/estimator.go | 31 ++++++----------------------- lnwallet/chainfee/estimator_test.go | 30 +++++++--------------------- 2 files changed, 13 insertions(+), 48 deletions(-) diff --git a/lnwallet/chainfee/estimator.go b/lnwallet/chainfee/estimator.go index 8bf61e0ac..dbeb87ee5 100644 --- a/lnwallet/chainfee/estimator.go +++ b/lnwallet/chainfee/estimator.go @@ -493,16 +493,6 @@ var _ Estimator = (*BitcoindEstimator)(nil) // implementation of this interface in order to allow the WebAPIEstimator to // be fully generic in its logic. type WebAPIFeeSource interface { - // GenQueryURL generates the full query URL. The value returned by this - // method should be able to be used directly as a path for an HTTP GET - // request. - GenQueryURL() string - - // 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. - ParseResponse(r io.Reader) (map[uint32]uint32, error) - // 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) @@ -517,21 +507,12 @@ type SparseConfFeeSource struct { URL string } -// GenQueryURL generates the full query URL. The value returned by this -// method should be able to be used directly as a path for an HTTP GET -// request. -// -// NOTE: Part of the WebAPIFeeSource interface. -func (s SparseConfFeeSource) GenQueryURL() string { - return s.URL -} - -// ParseResponse attempts to parse the body of the response generated by the +// 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. -// -// NOTE: Part of the WebAPIFeeSource interface. -func (s SparseConfFeeSource) ParseResponse(r io.Reader) (map[uint32]uint32, error) { +func (s SparseConfFeeSource) parseResponse(r io.Reader) ( + map[uint32]uint32, error) { + type jsonResp struct { FeeByBlockTarget map[uint32]uint32 `json:"fee_by_block_target"` } @@ -567,7 +548,7 @@ func (s SparseConfFeeSource) GetFeeMap() (map[uint32]uint32, error) { // With the client created, we'll query the API source to fetch the URL // that we should use to query for the fee estimation. - targetURL := s.GenQueryURL() + targetURL := s.URL resp, err := netClient.Get(targetURL) if err != nil { log.Errorf("unable to query web api for fee response: %v", @@ -578,7 +559,7 @@ func (s SparseConfFeeSource) GetFeeMap() (map[uint32]uint32, error) { // 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) + feesByBlockTarget, err := s.parseResponse(resp.Body) if err != nil { log.Errorf("unable to parse fee api response: %v", err) diff --git a/lnwallet/chainfee/estimator_test.go b/lnwallet/chainfee/estimator_test.go index 75bf950d8..f962df5ec 100644 --- a/lnwallet/chainfee/estimator_test.go +++ b/lnwallet/chainfee/estimator_test.go @@ -3,8 +3,6 @@ package chainfee import ( "bytes" "encoding/json" - "io" - "reflect" "testing" "github.com/btcsuite/btcd/btcutil" @@ -16,14 +14,6 @@ type mockSparseConfFeeSource struct { fees map[uint32]uint32 } -func (e mockSparseConfFeeSource) GenQueryURL() string { - return e.url -} - -func (e mockSparseConfFeeSource) ParseResponse(r io.Reader) (map[uint32]uint32, error) { - return e.fees, nil -} - func (e mockSparseConfFeeSource) GetFeeMap() (map[uint32]uint32, error) { return e.fees, nil } @@ -117,10 +107,6 @@ func TestSparseConfFeeSource(t *testing.T) { // Test that GenQueryURL returns the URL as is. url := "test" feeSource := SparseConfFeeSource{URL: url} - queryURL := feeSource.GenQueryURL() - if queryURL != url { - t.Fatalf("expected query URL of %v, got %v", url, queryURL) - } // Test parsing a properly formatted JSON API response. // First, create the response as a bytes.Reader. @@ -129,17 +115,17 @@ func TestSparseConfFeeSource(t *testing.T) { 2: 42, 3: 54321, } - testJSON := map[string]map[uint32]uint32{"fee_by_block_target": testFees} + testJSON := map[string]map[uint32]uint32{ + "fee_by_block_target": testFees, + } jsonResp, err := json.Marshal(testJSON) 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) + fees, err := feeSource.parseResponse(reader) require.NoError(t, err, "unable to parse API response") - if !reflect.DeepEqual(fees, testFees) { - t.Fatalf("expected %v, got %v", testFees, fees) - } + require.Equal(t, testFees, fees, "unexpected fee map returned") // Test parsing an improperly formatted JSON API response. badFees := map[string]uint32{"hi": 12345, "hello": 42, "satoshi": 54321} @@ -149,10 +135,8 @@ func TestSparseConfFeeSource(t *testing.T) { reader = bytes.NewReader(jsonResp) // Finally, ensure the improperly formatted fees error. - _, err = feeSource.ParseResponse(reader) - if err == nil { - t.Fatalf("expected ParseResponse to fail") - } + _, err = feeSource.parseResponse(reader) + require.Error(t, err, "expected error when parsing bad JSON") } // TestWebAPIFeeEstimator checks that the WebAPIFeeEstimator returns fee rates