multi: add max local csv config option

To allow nodes more control over the amount of time that their funds
will be locked up, we add a MaxLocalCSVDelay option which sets the
maximum csv delay we will accept for all channels. We default to the
existing constant of 10000, and set a sane minimum on this value so that
clients cannot set unreasonably low maximum csv delays which will result
in their node rejecting all channels.
This commit is contained in:
carla 2020-10-29 11:29:53 +02:00
parent fbb430ad82
commit f4136decae
No known key found for this signature in database
GPG Key ID: 4CA7FE54A6213C91
8 changed files with 235 additions and 19 deletions

View File

@ -112,6 +112,11 @@ const (
// concurrent HTLCs the remote party may add to commitment transactions.
// This value can be overridden with --default-remote-max-htlcs.
defaultRemoteMaxHtlcs = 483
// defaultMaxLocalCSVDelay is the maximum delay we accept on our
// commitment output.
// TODO(halseth): find a more scientific choice of value.
defaultMaxLocalCSVDelay = 10000
)
var (
@ -344,6 +349,7 @@ func DefaultConfig() Config {
BaseFee: chainreg.DefaultBitcoinBaseFeeMSat,
FeeRate: chainreg.DefaultBitcoinFeeRate,
TimeLockDelta: chainreg.DefaultBitcoinTimeLockDelta,
MaxLocalDelay: defaultMaxLocalCSVDelay,
Node: "btcd",
},
BtcdMode: &lncfg.Btcd{
@ -362,6 +368,7 @@ func DefaultConfig() Config {
BaseFee: chainreg.DefaultLitecoinBaseFeeMSat,
FeeRate: chainreg.DefaultLitecoinFeeRate,
TimeLockDelta: chainreg.DefaultLitecoinTimeLockDelta,
MaxLocalDelay: defaultMaxLocalCSVDelay,
Node: "ltcd",
},
LtcdMode: &lncfg.Btcd{
@ -822,10 +829,11 @@ func ValidateConfig(cfg Config, usageMessage string) (*Config, error) {
"litecoin.active must be set to 1 (true)", funcName)
case cfg.Litecoin.Active:
if cfg.Litecoin.TimeLockDelta < minTimeLockDelta {
return nil, fmt.Errorf("timelockdelta must be at least %v",
minTimeLockDelta)
err := cfg.Litecoin.Validate(minTimeLockDelta, minLtcRemoteDelay)
if err != nil {
return nil, err
}
// Multiple networks can't be selected simultaneously. Count
// number of network flags passed; assign active network params
// while we're at it.
@ -946,9 +954,9 @@ func ValidateConfig(cfg Config, usageMessage string) (*Config, error) {
return nil, err
}
if cfg.Bitcoin.TimeLockDelta < minTimeLockDelta {
return nil, fmt.Errorf("timelockdelta must be at least %v",
minTimeLockDelta)
err := cfg.Bitcoin.Validate(minTimeLockDelta, minBtcRemoteDelay)
if err != nil {
return nil, err
}
switch cfg.Bitcoin.Node {

View File

@ -339,6 +339,10 @@ type fundingConfig struct {
// incoming channels having a non-zero push amount.
RejectPush bool
// MaxLocalCSVDelay is the maximum csv delay we will allow for our
// commit output. Channels that exceed this value will be failed.
MaxLocalCSVDelay uint16
// NotifyOpenChannelEvent informs the ChannelNotifier when channels
// transition from pending open to open.
NotifyOpenChannelEvent func(wire.OutPoint)
@ -1342,7 +1346,9 @@ func (f *fundingManager) handleFundingOpen(peer lnpeer.Peer,
MaxAcceptedHtlcs: msg.MaxAcceptedHTLCs,
CsvDelay: msg.CsvDelay,
}
err = reservation.CommitConstraints(channelConstraints)
err = reservation.CommitConstraints(
channelConstraints, f.cfg.MaxLocalCSVDelay,
)
if err != nil {
fndgLog.Errorf("Unacceptable channel constraints: %v", err)
f.failFundingFlow(peer, msg.PendingChannelID, err)
@ -1525,7 +1531,9 @@ func (f *fundingManager) handleFundingAccept(peer lnpeer.Peer,
MaxAcceptedHtlcs: msg.MaxAcceptedHTLCs,
CsvDelay: msg.CsvDelay,
}
err = resCtx.reservation.CommitConstraints(channelConstraints)
err = resCtx.reservation.CommitConstraints(
channelConstraints, f.cfg.MaxLocalCSVDelay,
)
if err != nil {
fndgLog.Warnf("Unacceptable channel constraints: %v", err)
f.failFundingFlow(peer, msg.PendingChannelID, err)

View File

@ -37,6 +37,7 @@ import (
"github.com/lightningnetwork/lnd/lnwallet"
"github.com/lightningnetwork/lnd/lnwallet/chainfee"
"github.com/lightningnetwork/lnd/lnwire"
"github.com/stretchr/testify/require"
)
const (
@ -433,6 +434,7 @@ func createTestFundingManager(t *testing.T, privKey *btcec.PrivateKey,
ZombieSweeperInterval: 1 * time.Hour,
ReservationTimeout: 1 * time.Nanosecond,
MaxChanSize: MaxFundingAmount,
MaxLocalCSVDelay: defaultMaxLocalCSVDelay,
MaxPendingChannels: lncfg.DefaultMaxPendingChannels,
NotifyOpenChannelEvent: evt.NotifyOpenChannelEvent,
OpenChannelPredicate: chainedAcceptor,
@ -1273,6 +1275,159 @@ func TestFundingManagerNormalWorkflow(t *testing.T) {
assertNoChannelState(t, alice, bob, fundingOutPoint)
}
// TestFundingManagerRejectCSV tests checking of local CSV values against our
// local CSV limit for incoming and outgoing channels.
func TestFundingManagerRejectCSV(t *testing.T) {
t.Run("csv too high", func(t *testing.T) {
testLocalCSVLimit(t, 400, 500)
})
t.Run("csv within limit", func(t *testing.T) {
testLocalCSVLimit(t, 600, 500)
})
}
// testLocalCSVLimit creates two funding managers, alice and bob, where alice
// has a limit on her maximum local CSV and bob sets his required CSV for alice.
// We test an incoming and outgoing channel, ensuring that alice accepts csvs
// below her maximum, and rejects those above it.
func testLocalCSVLimit(t *testing.T, aliceMaxCSV, bobRequiredCSV uint16) {
t.Parallel()
alice, bob := setupFundingManagers(t)
defer tearDownFundingManagers(t, alice, bob)
// Set a maximum local delay in alice's config to aliceMaxCSV and overwrite
// bob's required remote delay function to return bobRequiredCSV.
alice.fundingMgr.cfg.MaxLocalCSVDelay = aliceMaxCSV
bob.fundingMgr.cfg.RequiredRemoteDelay = func(_ btcutil.Amount) uint16 {
return bobRequiredCSV
}
// For convenience, we bump our max pending channels to 2 so that we
// can test incoming and outgoing channels without needing to step
// through the full funding process.
alice.fundingMgr.cfg.MaxPendingChannels = 2
bob.fundingMgr.cfg.MaxPendingChannels = 2
// If our maximum is less than the value bob sets, we expect this test
// to fail.
expectFail := aliceMaxCSV < bobRequiredCSV
// First, we will initiate an outgoing channel from Alice -> Bob.
errChan := make(chan error, 1)
updateChan := make(chan *lnrpc.OpenStatusUpdate)
initReq := &openChanReq{
targetPubkey: bob.privKey.PubKey(),
chainHash: *fundingNetParams.GenesisHash,
localFundingAmt: 200000,
fundingFeePerKw: 1000,
updates: updateChan,
err: errChan,
}
// Alice should have sent the OpenChannel message to Bob.
alice.fundingMgr.initFundingWorkflow(bob, initReq)
var aliceMsg lnwire.Message
select {
case aliceMsg = <-alice.msgChan:
case err := <-initReq.err:
t.Fatalf("error init funding workflow: %v", err)
case <-time.After(time.Second * 5):
t.Fatalf("alice did not send OpenChannel message")
}
openChannelReq, ok := aliceMsg.(*lnwire.OpenChannel)
require.True(t, ok)
// Let Bob handle the init message.
bob.fundingMgr.ProcessFundingMsg(openChannelReq, alice)
// Bob should answer with an AcceptChannel message.
acceptChannelResponse := assertFundingMsgSent(
t, bob.msgChan, "AcceptChannel",
).(*lnwire.AcceptChannel)
// They now should both have pending reservations for this channel
// active.
assertNumPendingReservations(t, alice, bobPubKey, 1)
assertNumPendingReservations(t, bob, alicePubKey, 1)
// Forward the response to Alice.
alice.fundingMgr.ProcessFundingMsg(acceptChannelResponse, bob)
// At this point, Alice has received an AcceptChannel message from
// bob with the CSV value that he has set for her, and has to evaluate
// whether she wants to accept this channel. If we get an error, we
// assert that we expected the channel to fail, otherwise we assert that
// she proceeded with the channel open as usual.
select {
case err := <-errChan:
require.Error(t, err)
require.True(t, expectFail)
case msg := <-alice.msgChan:
_, ok := msg.(*lnwire.FundingCreated)
require.True(t, ok)
require.False(t, expectFail)
case <-time.After(time.Second):
t.Fatal("funding flow was not failed")
}
// We do not need to complete the rest of the funding flow (it is
// covered in other tests). So now we test that Alice will appropriately
// handle incoming channels, opening a channel from Bob->Alice.
errChan = make(chan error, 1)
updateChan = make(chan *lnrpc.OpenStatusUpdate)
initReq = &openChanReq{
targetPubkey: alice.privKey.PubKey(),
chainHash: *fundingNetParams.GenesisHash,
localFundingAmt: 200000,
fundingFeePerKw: 1000,
updates: updateChan,
err: errChan,
}
bob.fundingMgr.initFundingWorkflow(alice, initReq)
// Bob should have sent the OpenChannel message to Alice.
var bobMsg lnwire.Message
select {
case bobMsg = <-bob.msgChan:
case err := <-initReq.err:
t.Fatalf("bob OpenChannel message failed: %v", err)
case <-time.After(time.Second * 5):
t.Fatalf("bob did not send OpenChannel message")
}
openChannelReq, ok = bobMsg.(*lnwire.OpenChannel)
require.True(t, ok)
// Let Alice handle the init message.
alice.fundingMgr.ProcessFundingMsg(openChannelReq, bob)
// We expect a error message from Alice if we're expecting the channel
// to fail, otherwise we expect her to proceed with the channel as
// usual.
select {
case msg := <-alice.msgChan:
var ok bool
if expectFail {
_, ok = msg.(*lnwire.Error)
} else {
_, ok = msg.(*lnwire.AcceptChannel)
}
require.True(t, ok)
case <-time.After(time.Second * 5):
t.Fatal("funding flow was not failed")
}
}
func TestFundingManagerRestartBehavior(t *testing.T) {
t.Parallel()

View File

@ -1,6 +1,10 @@
package lncfg
import "github.com/lightningnetwork/lnd/lnwire"
import (
"fmt"
"github.com/lightningnetwork/lnd/lnwire"
)
// Chain holds the configuration options for the daemon's chain settings.
type Chain struct {
@ -16,9 +20,28 @@ type Chain struct {
DefaultNumChanConfs int `long:"defaultchanconfs" description:"The default number of confirmations a channel must have before it's considered open. If this is not set, we will scale the value according to the channel size."`
DefaultRemoteDelay int `long:"defaultremotedelay" description:"The default number of blocks we will require our channel counterparty to wait before accessing its funds in case of unilateral close. If this is not set, we will scale the value according to the channel size."`
MaxLocalDelay uint16 `long:"maxlocaldelay" description:"The maximum blocks we will allow our funds to be timelocked before accessing its funds in case of unilateral close. If a peer proposes a value greater than this, we will reject the channel."`
MinHTLCIn lnwire.MilliSatoshi `long:"minhtlc" description:"The smallest HTLC we are willing to accept on our channels, in millisatoshi"`
MinHTLCOut lnwire.MilliSatoshi `long:"minhtlcout" description:"The smallest HTLC we are willing to send out on our channels, in millisatoshi"`
BaseFee lnwire.MilliSatoshi `long:"basefee" description:"The base fee in millisatoshi we will charge for forwarding payments on our channels"`
FeeRate lnwire.MilliSatoshi `long:"feerate" description:"The fee rate used when forwarding payments on our channels. The total fee charged is basefee + (amount * feerate / 1000000), where amount is the forwarded amount."`
TimeLockDelta uint32 `long:"timelockdelta" description:"The CLTV delta we will subtract from a forwarded HTLC's timelock value"`
}
// Validate performs validation on our chain config.
func (c *Chain) Validate(minTimeLockDelta uint32, minDelay uint16) error {
if c.TimeLockDelta < minTimeLockDelta {
return fmt.Errorf("timelockdelta must be at least %v",
minTimeLockDelta)
}
// Check that our max local delay isn't set below some reasonable
// minimum value. We do this to prevent setting an unreasonably low
// delay, which would mean that the node would accept no channels.
if c.MaxLocalDelay < minDelay {
return fmt.Errorf("MaxLocalDelay must be at least: %v",
minDelay)
}
return nil
}

View File

@ -91,6 +91,8 @@ var (
bobAddr, _ = net.ResolveTCPAddr("tcp", "10.0.0.2:9000")
aliceAddr, _ = net.ResolveTCPAddr("tcp", "10.0.0.3:9000")
defaultMaxLocalCsvDelay uint16 = 10000
)
// assertProperBalance asserts than the total value of the unspent outputs
@ -447,7 +449,9 @@ func testDualFundingReservationWorkflow(miner *rpctest.Harness,
MaxAcceptedHtlcs: input.MaxHTLCNumber / 2,
CsvDelay: csvDelay,
}
err = aliceChanReservation.CommitConstraints(channelConstraints)
err = aliceChanReservation.CommitConstraints(
channelConstraints, defaultMaxLocalCsvDelay,
)
if err != nil {
t.Fatalf("unable to verify constraints: %v", err)
}
@ -481,7 +485,9 @@ func testDualFundingReservationWorkflow(miner *rpctest.Harness,
if err != nil {
t.Fatalf("bob unable to init channel reservation: %v", err)
}
err = bobChanReservation.CommitConstraints(channelConstraints)
err = bobChanReservation.CommitConstraints(
channelConstraints, defaultMaxLocalCsvDelay,
)
if err != nil {
t.Fatalf("unable to verify constraints: %v", err)
}
@ -893,7 +899,9 @@ func testSingleFunderReservationWorkflow(miner *rpctest.Harness,
MaxAcceptedHtlcs: input.MaxHTLCNumber / 2,
CsvDelay: csvDelay,
}
err = aliceChanReservation.CommitConstraints(channelConstraints)
err = aliceChanReservation.CommitConstraints(
channelConstraints, defaultMaxLocalCsvDelay,
)
if err != nil {
t.Fatalf("unable to verify constraints: %v", err)
}
@ -934,7 +942,9 @@ func testSingleFunderReservationWorkflow(miner *rpctest.Harness,
if err != nil {
t.Fatalf("unable to create bob reservation: %v", err)
}
err = bobChanReservation.CommitConstraints(channelConstraints)
err = bobChanReservation.CommitConstraints(
channelConstraints, defaultMaxLocalCsvDelay,
)
if err != nil {
t.Fatalf("unable to verify constraints: %v", err)
}

View File

@ -383,15 +383,14 @@ func (r *ChannelReservation) SetNumConfsRequired(numConfs uint16) {
// of satoshis that can be transferred in a single commitment. This function
// will also attempt to verify the constraints for sanity, returning an error
// if the parameters are seemed unsound.
func (r *ChannelReservation) CommitConstraints(c *channeldb.ChannelConstraints) error {
func (r *ChannelReservation) CommitConstraints(c *channeldb.ChannelConstraints,
maxLocalCSVDelay uint16) error {
r.Lock()
defer r.Unlock()
// Fail if we consider csvDelay excessively large.
// TODO(halseth): find a more scientific choice of value.
const maxDelay = 10000
if c.CsvDelay > maxDelay {
return ErrCsvDelayTooLarge(c.CsvDelay, maxDelay)
// Fail if the csv delay for our funds exceeds our maximum.
if c.CsvDelay > maxLocalCSVDelay {
return ErrCsvDelayTooLarge(c.CsvDelay, maxLocalCSVDelay)
}
// The channel reserve should always be greater or equal to the dust

View File

@ -384,6 +384,12 @@ bitcoin.node=btcd
; will scale the value according to the channel size.
; bitcoin.defaultremotedelay=144
; The maximum number of blocks we will limit the wait that our own funds are
; encumbered by in the case when our node unilaterally closes. If a remote peer
; proposes a channel with a delay above this amount, lnd will reject the
; channel.
; bitcoin.maxlocaldelay=2016
; The smallest HTLC we are willing to accept on our channels, in millisatoshi.
; bitcoin.minhtlc=1
@ -550,6 +556,12 @@ litecoin.node=ltcd
; will scale the value according to the channel size.
; litecoin.defaultremotedelay=144
; The maximum number of blocks we will limit the wait that our own funds are
; encumbered by in the case when our node unilaterally closes. If a remote peer
; proposes a channel with a delay above this amount, lnd will reject the
; channel.
; litecoin.maxlocaldelay=2016
; The smallest HTLC we are willing to accept on our channels, in millisatoshi.
; litecoin.minhtlc=1

View File

@ -1174,6 +1174,7 @@ func newServer(cfg *Config, listenAddrs []net.Addr,
MaxChanSize: btcutil.Amount(cfg.MaxChanSize),
MaxPendingChannels: cfg.MaxPendingChannels,
RejectPush: cfg.RejectPush,
MaxLocalCSVDelay: chainCfg.MaxLocalDelay,
NotifyOpenChannelEvent: s.channelNotifier.NotifyOpenChannelEvent,
OpenChannelPredicate: chanPredicate,
NotifyPendingOpenChannelEvent: s.channelNotifier.NotifyPendingOpenChannelEvent,