Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[2/5]sweep: refactor fee estimation and introduce UtxoAggregator #8422

Merged
4 changes: 2 additions & 2 deletions contractcourt/commit_sweep_resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,8 @@ func (s *mockSweeper) SweepInput(input input.Input, params sweep.Params) (
return result, nil
}

func (s *mockSweeper) CreateSweepTx(inputs []input.Input, feePref sweep.FeePreference,
currentBlockHeight uint32) (*wire.MsgTx, error) {
func (s *mockSweeper) CreateSweepTx(inputs []input.Input,
feePref sweep.FeePreference) (*wire.MsgTx, error) {

// We will wait for the test to supply the sweep tx to return.
sweepTx := <-s.createSweepTxChan
Expand Down
6 changes: 1 addition & 5 deletions contractcourt/htlc_success_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -432,17 +432,13 @@ func (h *htlcSuccessResolver) resolveRemoteCommitOutput() (
// transaction, that we'll use to move these coins back into
// the backing wallet.
//
// TODO: Set tx lock time to current block height instead of
// zero. Will be taken care of once sweeper implementation is
// complete.
//
// TODO: Use time-based sweeper and result chan.
var err error
h.sweepTx, err = h.Sweeper.CreateSweepTx(
[]input.Input{inp},
sweep.FeePreference{
ConfTarget: sweepConfTarget,
}, 0,
},
)
if err != nil {
return nil, err
Expand Down
4 changes: 2 additions & 2 deletions contractcourt/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ type UtxoSweeper interface {
// CreateSweepTx accepts a list of inputs and signs and generates a txn
// that spends from them. This method also makes an accurate fee
// estimate before generating the required witnesses.
CreateSweepTx(inputs []input.Input, feePref sweep.FeePreference,
currentBlockHeight uint32) (*wire.MsgTx, error)
CreateSweepTx(inputs []input.Input,
feePref sweep.FeePreference) (*wire.MsgTx, error)

// RelayFeePerKW returns the minimum fee rate required for transactions
// to be relayed.
Expand Down
1 change: 1 addition & 0 deletions htlcswitch/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ func (m *mockPreimageCache) SubscribeUpdates(
return nil, nil
}

// TODO(yy): replace it with chainfee.MockEstimator.
type mockFeeEstimator struct {
byteFeeIn chan chainfee.SatPerKWeight
relayFee chan chainfee.SatPerKWeight
Expand Down
12 changes: 6 additions & 6 deletions lnrpc/rpc_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,12 +222,12 @@ func CalculateFeeRate(satPerByte, satPerVByte uint64, targetConf uint32,

// Based on the passed fee related parameters, we'll determine an
// appropriate fee rate for this transaction.
feeRate, err := sweep.DetermineFeePerKw(
estimator, sweep.FeePreference{
ConfTarget: targetConf,
FeeRate: satPerKw,
},
)
feePref := sweep.FeePreference{
ConfTarget: targetConf,
FeeRate: satPerKw,
}
// TODO(yy): need to pass the configured max fee here.
feeRate, err := feePref.Estimate(estimator, 0)
if err != nil {
return feeRate, err
}
Expand Down
51 changes: 50 additions & 1 deletion lnwallet/chainfee/mocks.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package chainfee

import "github.com/stretchr/testify/mock"
import (
"github.com/stretchr/testify/mock"
)

type mockFeeSource struct {
mock.Mock
Expand All @@ -15,3 +17,50 @@ func (m *mockFeeSource) GetFeeMap() (map[uint32]uint32, error) {

return args.Get(0).(map[uint32]uint32), args.Error(1)
}

// MockEstimator implements the `Estimator` interface and is used by
// other packages for mock testing.
type MockEstimator struct {
mock.Mock
}

// Compile time assertion that MockEstimator implements Estimator.
var _ Estimator = (*MockEstimator)(nil)

// EstimateFeePerKW takes in a target for the number of blocks until an initial
// confirmation and returns the estimated fee expressed in sat/kw.
func (m *MockEstimator) EstimateFeePerKW(
numBlocks uint32) (SatPerKWeight, error) {

args := m.Called(numBlocks)

if args.Get(0) == nil {
return 0, args.Error(1)
}

return args.Get(0).(SatPerKWeight), args.Error(1)
}

// Start signals the Estimator to start any processes or goroutines it needs to
// perform its duty.
func (m *MockEstimator) Start() error {
args := m.Called()

return args.Error(0)
}

// Stop stops any spawned goroutines and cleans up the resources used by the
// fee estimator.
func (m *MockEstimator) Stop() error {
args := m.Called()

return args.Error(0)
}

// RelayFeePerKW returns the minimum fee rate required for transactions to be
// relayed. This is also the basis for calculation of the dust limit.
func (m *MockEstimator) RelayFeePerKW() SatPerKWeight {
args := m.Called()

return args.Get(0).(SatPerKWeight)
}
12 changes: 7 additions & 5 deletions rpcserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -1174,11 +1174,13 @@ func (r *rpcServer) EstimateFee(ctx context.Context,
// Query the fee estimator for the fee rate for the given confirmation
// target.
target := in.TargetConf
feePerKw, err := sweep.DetermineFeePerKw(
r.server.cc.FeeEstimator, sweep.FeePreference{
ConfTarget: uint32(target),
},
)
feePref := sweep.FeePreference{
ConfTarget: uint32(target),
}

// Since we are providing a fee estimation as an RPC response, there's
// no need to set a max feerate here, so we use 0.
feePerKw, err := feePref.Estimate(r.server.cc.FeeEstimator, 0)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: so we don't cap the fee when sending coins, or what's the use case for the EstimateFee rpc ? When this rpc is used to make decisions when to trigger another RPC (e.g. sendcoins) we probably should also add the MaxFee cap there ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to update a few RPCs to allow specifying a max fee rate cap, including sendcoins. As for the usage of EstimateFee, I think it's used when we wanna open channels?

if err != nil {
return nil, err
}
Expand Down
1 change: 0 additions & 1 deletion server.go
Original file line number Diff line number Diff line change
Expand Up @@ -1064,7 +1064,6 @@ func newServer(cfg *Config, listenAddrs []net.Addr,

s.sweeper = sweep.New(&sweep.UtxoSweeperConfig{
FeeEstimator: cc.FeeEstimator,
DetermineFeePerKw: sweep.DetermineFeePerKw,
GenSweepScript: newSweepPkScriptGen(cc.Wallet),
Signer: cc.Wallet.Cfg.Signer,
Wallet: newSweeperWallet(cc.Wallet),
Expand Down
2 changes: 2 additions & 0 deletions sweep/fee_estimator_mock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
// mockFeeEstimator implements a mock fee estimator. It closely resembles
// lnwallet.StaticFeeEstimator with the addition that fees can be changed for
// testing purposes in a thread safe manner.
//
// TODO(yy): replace it with chainfee.MockEstimator once it's merged.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: isn't this chainfee.MockEstimator part of this PR ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah but I wanna remove mockFeeEstimator and use chainfee.MockEstimator only - should be doable once the sweeper refactor is done.

type mockFeeEstimator struct {
feePerKW chainfee.SatPerKWeight

Expand Down
Loading