From 91f3df07e4dcf90f77ed88bd5f0bb80f806a0bc8 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Tue, 18 Dec 2018 09:02:27 +0100 Subject: [PATCH] lnwallet: prevent static fee estimator fees from being modified Modifying the static fees is not thread safe. In this commit the fees are made immutable. --- breacharbiter_test.go | 4 ++-- chainregistry.go | 12 ++++++------ fundingmanager_test.go | 2 +- htlcswitch/link_test.go | 10 +++++----- htlcswitch/test_utils.go | 2 +- lnwallet/channel_test.go | 2 +- lnwallet/fee_estimator.go | 25 ++++++++++++++++++------- lnwallet/fee_estimator_test.go | 4 +--- lnwallet/interface_test.go | 4 ++-- lnwallet/test_utils.go | 2 +- peer_test.go | 4 ++-- test_utils.go | 2 +- 12 files changed, 41 insertions(+), 32 deletions(-) diff --git a/breacharbiter_test.go b/breacharbiter_test.go index 65a5bc5b..d0d2cc3d 100644 --- a/breacharbiter_test.go +++ b/breacharbiter_test.go @@ -1336,7 +1336,7 @@ func createTestArbiter(t *testing.T, contractBreaches chan *ContractBreachEvent, ba := newBreachArbiter(&BreachConfig{ CloseLink: func(_ *wire.OutPoint, _ htlcswitch.ChannelCloseType) {}, DB: db, - Estimator: &lnwallet.StaticFeeEstimator{FeePerKW: 12500}, + Estimator: lnwallet.NewStaticFeeEstimator(12500, 0), GenSweepScript: func() ([]byte, error) { return nil, nil }, ContractBreaches: contractBreaches, Signer: signer, @@ -1476,7 +1476,7 @@ func createInitChannels(revocationWindow int) (*lnwallet.LightningChannel, *lnwa return nil, nil, nil, err } - estimator := &lnwallet.StaticFeeEstimator{FeePerKW: 12500} + estimator := lnwallet.NewStaticFeeEstimator(12500, 0) feePerKw, err := estimator.EstimateFeePerKW(1) if err != nil { return nil, nil, nil, err diff --git a/chainregistry.go b/chainregistry.go index cce2696e..56137b73 100644 --- a/chainregistry.go +++ b/chainregistry.go @@ -150,9 +150,9 @@ func newChainControlFromConfig(cfg *config, chanDB *channeldb.DB, FeeRate: cfg.Bitcoin.FeeRate, TimeLockDelta: cfg.Bitcoin.TimeLockDelta, } - cc.feeEstimator = lnwallet.StaticFeeEstimator{ - FeePerKW: defaultBitcoinStaticFeePerKW, - } + cc.feeEstimator = lnwallet.NewStaticFeeEstimator( + defaultBitcoinStaticFeePerKW, 0, + ) case litecoinChain: cc.routingPolicy = htlcswitch.ForwardingPolicy{ MinHTLC: cfg.Litecoin.MinHTLC, @@ -160,9 +160,9 @@ func newChainControlFromConfig(cfg *config, chanDB *channeldb.DB, FeeRate: cfg.Litecoin.FeeRate, TimeLockDelta: cfg.Litecoin.TimeLockDelta, } - cc.feeEstimator = lnwallet.StaticFeeEstimator{ - FeePerKW: defaultLitecoinStaticFeePerKW, - } + cc.feeEstimator = lnwallet.NewStaticFeeEstimator( + defaultLitecoinStaticFeePerKW, 0, + ) default: return nil, nil, fmt.Errorf("Default routing policy for "+ "chain %v is unknown", registeredChains.PrimaryChain()) diff --git a/fundingmanager_test.go b/fundingmanager_test.go index c182d015..87e2767b 100644 --- a/fundingmanager_test.go +++ b/fundingmanager_test.go @@ -231,7 +231,7 @@ func createTestFundingManager(t *testing.T, privKey *btcec.PrivateKey, addr *lnwire.NetAddress, tempTestDir string) (*testNode, error) { netParams := activeNetParams.Params - estimator := lnwallet.StaticFeeEstimator{FeePerKW: 62500} + estimator := lnwallet.NewStaticFeeEstimator(62500, 0) chainNotifier := &mockNotifier{ oneConfChannel: make(chan *chainntnfs.TxConfirmation, 1), diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go index 517196db..5fbaa765 100644 --- a/htlcswitch/link_test.go +++ b/htlcswitch/link_test.go @@ -1795,7 +1795,7 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { coreLink.cfg.HodlMask = hodl.MaskFromFlags(hodl.ExitSettle) coreLink.cfg.DebugHTLC = true - estimator := &lnwallet.StaticFeeEstimator{FeePerKW: 6000} + estimator := lnwallet.NewStaticFeeEstimator(6000, 0) feePerKw, err := estimator.EstimateFeePerKW(1) if err != nil { t.Fatalf("unable to query fee estimator: %v", err) @@ -2206,7 +2206,7 @@ func TestChannelLinkBandwidthConsistencyOverflow(t *testing.T) { aliceMsgs = coreLink.cfg.Peer.(*mockPeer).sentMsgs ) - estimator := &lnwallet.StaticFeeEstimator{FeePerKW: 6000} + estimator := lnwallet.NewStaticFeeEstimator(6000, 0) feePerKw, err := estimator.EstimateFeePerKW(1) if err != nil { t.Fatalf("unable to query fee estimator: %v", err) @@ -2453,7 +2453,7 @@ func TestChannelLinkTrimCircuitsPending(t *testing.T) { // Compute the static fees that will be used to determine the // correctness of Alice's bandwidth when forwarding HTLCs. - estimator := &lnwallet.StaticFeeEstimator{FeePerKW: 6000} + estimator := lnwallet.NewStaticFeeEstimator(6000, 0) feePerKw, err := estimator.EstimateFeePerKW(1) if err != nil { t.Fatalf("unable to query fee estimator: %v", err) @@ -2731,7 +2731,7 @@ func TestChannelLinkTrimCircuitsNoCommit(t *testing.T) { // Compute the static fees that will be used to determine the // correctness of Alice's bandwidth when forwarding HTLCs. - estimator := &lnwallet.StaticFeeEstimator{FeePerKW: 6000} + estimator := lnwallet.NewStaticFeeEstimator(6000, 0) feePerKw, err := estimator.EstimateFeePerKW(1) if err != nil { t.Fatalf("unable to query fee estimator: %v", err) @@ -2989,7 +2989,7 @@ func TestChannelLinkBandwidthChanReserve(t *testing.T) { aliceMsgs = coreLink.cfg.Peer.(*mockPeer).sentMsgs ) - estimator := &lnwallet.StaticFeeEstimator{FeePerKW: 6000} + estimator := lnwallet.NewStaticFeeEstimator(6000, 0) feePerKw, err := estimator.EstimateFeePerKW(1) if err != nil { t.Fatalf("unable to query fee estimator: %v", err) diff --git a/htlcswitch/test_utils.go b/htlcswitch/test_utils.go index c29619f5..ab15aa8b 100644 --- a/htlcswitch/test_utils.go +++ b/htlcswitch/test_utils.go @@ -273,7 +273,7 @@ func createTestChannel(alicePrivKey, bobPrivKey []byte, return nil, nil, nil, nil, err } - estimator := &lnwallet.StaticFeeEstimator{FeePerKW: 6000} + estimator := lnwallet.NewStaticFeeEstimator(6000, 0) feePerKw, err := estimator.EstimateFeePerKW(1) if err != nil { return nil, nil, nil, nil, err diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index e3cd3e15..5eba3bc4 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -1131,7 +1131,7 @@ func TestHTLCSigNumber(t *testing.T) { } // Calculate two values that will be below and above Bob's dust limit. - estimator := &StaticFeeEstimator{FeePerKW: 6000} + estimator := NewStaticFeeEstimator(6000, 0) feePerKw, err := estimator.EstimateFeePerKW(1) if err != nil { t.Fatalf("unable to get fee: %v", err) diff --git a/lnwallet/fee_estimator.go b/lnwallet/fee_estimator.go index 6ac6a38a..76acc63a 100644 --- a/lnwallet/fee_estimator.go +++ b/lnwallet/fee_estimator.go @@ -68,22 +68,33 @@ type FeeEstimator interface { // StaticFeeEstimator will return a static value for all fee calculation // requests. It is designed to be replaced by a proper fee calculation -// implementation. +// implementation. The fees are not accessible directly, because changing them +// would not be thread safe. type StaticFeeEstimator struct { - // FeePerKW is the static fee rate in satoshis-per-vbyte that will be + // feePerKW is the static fee rate in satoshis-per-vbyte that will be // returned by this fee estimator. - FeePerKW SatPerKWeight + feePerKW SatPerKWeight - // RelayFee is the minimum fee rate required for transactions to be + // relayFee is the minimum fee rate required for transactions to be // relayed. - RelayFee SatPerKWeight + relayFee SatPerKWeight +} + +// NewStaticFeeEstimator returns a new static fee estimator instance. +func NewStaticFeeEstimator(feePerKW, + relayFee SatPerKWeight) *StaticFeeEstimator { + + return &StaticFeeEstimator{ + feePerKW: feePerKW, + relayFee: relayFee, + } } // EstimateFeePerKW will return a static value for fee calculations. // // NOTE: This method is part of the FeeEstimator interface. func (e StaticFeeEstimator) EstimateFeePerKW(numBlocks uint32) (SatPerKWeight, error) { - return e.FeePerKW, nil + return e.feePerKW, nil } // RelayFeePerKW returns the minimum fee rate required for transactions to be @@ -91,7 +102,7 @@ func (e StaticFeeEstimator) EstimateFeePerKW(numBlocks uint32) (SatPerKWeight, e // // NOTE: This method is part of the FeeEstimator interface. func (e StaticFeeEstimator) RelayFeePerKW() SatPerKWeight { - return e.RelayFee + return e.relayFee } // Start signals the FeeEstimator to start any processes or goroutines diff --git a/lnwallet/fee_estimator_test.go b/lnwallet/fee_estimator_test.go index d4608b9b..77d60916 100644 --- a/lnwallet/fee_estimator_test.go +++ b/lnwallet/fee_estimator_test.go @@ -74,9 +74,7 @@ func TestStaticFeeEstimator(t *testing.T) { const feePerKw = lnwallet.FeePerKwFloor - feeEstimator := &lnwallet.StaticFeeEstimator{ - FeePerKW: feePerKw, - } + feeEstimator := lnwallet.NewStaticFeeEstimator(feePerKw, 0) if err := feeEstimator.Start(); err != nil { t.Fatalf("unable to start fee estimator: %v", err) } diff --git a/lnwallet/interface_test.go b/lnwallet/interface_test.go index 23fc296b..c95b82a0 100644 --- a/lnwallet/interface_test.go +++ b/lnwallet/interface_test.go @@ -368,7 +368,7 @@ func createTestWallet(tempTestDir string, miningNode *rpctest.Harness, WalletController: wc, Signer: signer, ChainIO: bio, - FeeEstimator: lnwallet.StaticFeeEstimator{FeePerKW: 2500}, + FeeEstimator: lnwallet.NewStaticFeeEstimator(2500, 0), DefaultConstraints: channeldb.ChannelConstraints{ DustLimit: 500, MaxPendingAmount: lnwire.NewMSatFromSatoshis(btcutil.SatoshiPerBitcoin) * 100, @@ -2440,7 +2440,7 @@ func runTests(t *testing.T, walletDriver *lnwallet.WalletDriver, } case "neutrino": - feeEstimator = lnwallet.StaticFeeEstimator{FeePerKW: 62500} + feeEstimator = lnwallet.NewStaticFeeEstimator(62500, 0) // Set some package-level variable to speed up // operation for tests. diff --git a/lnwallet/test_utils.go b/lnwallet/test_utils.go index 60e98d3b..db2cbb10 100644 --- a/lnwallet/test_utils.go +++ b/lnwallet/test_utils.go @@ -229,7 +229,7 @@ func CreateTestChannels() (*LightningChannel, *LightningChannel, func(), error) return nil, nil, nil, err } - estimator := &StaticFeeEstimator{FeePerKW: 6000} + estimator := NewStaticFeeEstimator(6000, 0) feePerKw, err := estimator.EstimateFeePerKW(1) if err != nil { return nil, nil, nil, err diff --git a/peer_test.go b/peer_test.go index 86ba3c16..ccb7cd41 100644 --- a/peer_test.go +++ b/peer_test.go @@ -157,7 +157,7 @@ func TestPeerChannelClosureAcceptFeeInitiator(t *testing.T) { dummyDeliveryScript), } - estimator := lnwallet.StaticFeeEstimator{FeePerKW: 12500} + estimator := lnwallet.NewStaticFeeEstimator(12500, 0) feePerKw, err := estimator.EstimateFeePerKW(1) if err != nil { t.Fatalf("unable to query fee estimator: %v", err) @@ -447,7 +447,7 @@ func TestPeerChannelClosureFeeNegotiationsInitiator(t *testing.T) { msg: respShutdown, } - estimator := lnwallet.StaticFeeEstimator{FeePerKW: 12500} + estimator := lnwallet.NewStaticFeeEstimator(12500, 0) initiatorIdealFeeRate, err := estimator.EstimateFeePerKW(1) if err != nil { t.Fatalf("unable to query fee estimator: %v", err) diff --git a/test_utils.go b/test_utils.go index 283efabc..b3b1d4c7 100644 --- a/test_utils.go +++ b/test_utils.go @@ -202,7 +202,7 @@ func createTestPeer(notifier chainntnfs.ChainNotifier, return nil, nil, nil, nil, err } - estimator := &lnwallet.StaticFeeEstimator{FeePerKW: 12500} + estimator := lnwallet.NewStaticFeeEstimator(12500, 0) feePerKw, err := estimator.EstimateFeePerKW(1) if err != nil { return nil, nil, nil, nil, err