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

multi: cap anchors feerate at configurable maximum #4855

Merged
merged 3 commits into from
Dec 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"github.com/lightningnetwork/lnd/lncfg"
"github.com/lightningnetwork/lnd/lnrpc/routerrpc"
"github.com/lightningnetwork/lnd/lnrpc/signrpc"
"github.com/lightningnetwork/lnd/lnwallet"
"github.com/lightningnetwork/lnd/routing"
"github.com/lightningnetwork/lnd/tor"
)
Expand Down Expand Up @@ -289,6 +290,8 @@ type Config struct {

MaxChannelFeeAllocation float64 `long:"max-channel-fee-allocation" description:"The maximum percentage of total funds that can be allocated to a channel's commitment fee. This only applies for the initiator of the channel. Valid values are within [0.1, 1]."`

MaxCommitFeeRateAnchors uint64 `long:"max-commit-fee-rate-anchors" description:"The maximum fee rate in sat/vbyte that will be used for commitments of channels of the anchors type. Must be large enough to ensure transaction propagation"`

DryRunMigration bool `long:"dry-run-migration" description:"If true, lnd will abort committing a migration if it would otherwise have been successful. This leaves the database unmodified, and still compatible with the previously active version of lnd."`

net tor.Net
Expand Down Expand Up @@ -475,6 +478,7 @@ func DefaultConfig() Config {
},
MaxOutgoingCltvExpiry: htlcswitch.DefaultMaxOutgoingCltvExpiry,
MaxChannelFeeAllocation: htlcswitch.DefaultMaxLinkFeeAllocation,
MaxCommitFeeRateAnchors: lnwallet.DefaultAnchorsCommitMaxFeeRateSatPerVByte,
Roasbeef marked this conversation as resolved.
Show resolved Hide resolved
LogWriter: build.NewRotatingLogWriter(),
DB: lncfg.DefaultDB(),
registeredChains: chainreg.NewChainRegistry(),
Expand Down Expand Up @@ -735,6 +739,12 @@ func ValidateConfig(cfg Config, usageMessage string) (*Config, error) {
cfg.MaxChannelFeeAllocation)
}

if cfg.MaxCommitFeeRateAnchors < 1 {
return nil, fmt.Errorf("invalid max commit fee rate anchors: "+
"%v, must be at least 1 sat/vbyte",
cfg.MaxCommitFeeRateAnchors)
}

// Validate the Tor config parameters.
socks, err := lncfg.ParseAddressString(
cfg.Tor.SOCKS, strconv.Itoa(defaultTorSOCKSPort),
Expand Down
33 changes: 23 additions & 10 deletions fundingmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,10 @@ type fundingConfig struct {
// RegisteredChains keeps track of all chains that have been registered
// with the daemon.
RegisteredChains *chainreg.ChainRegistry

// MaxAnchorsCommitFeeRate is the max commitment fee rate we'll use as
// the initiator for channels of the anchor type.
MaxAnchorsCommitFeeRate chainfee.SatPerKWeight
}

// fundingManager acts as an orchestrator/bridge between the wallet's
Expand Down Expand Up @@ -3122,16 +3126,6 @@ func (f *fundingManager) handleInitFundingMsg(msg *initFundingMsg) {
msg.pushAmt, msg.chainHash, peerKey.SerializeCompressed(),
ourDustLimit, msg.minConfs)

// First, we'll query the fee estimator for a fee that should get the
// commitment transaction confirmed by the next few blocks (conf target
// of 3). We target the near blocks here to ensure that we'll be able
// to execute a timely unilateral channel closure if needed.
commitFeePerKw, err := f.cfg.FeeEstimator.EstimateFeePerKW(3)
if err != nil {
msg.err <- err
return
}

// We set the channel flags to indicate whether we want this channel to
// be announced to the network.
var channelFlags lnwire.FundingFlag
Expand Down Expand Up @@ -3190,6 +3184,25 @@ func (f *fundingManager) handleInitFundingMsg(msg *initFundingMsg) {
commitType := commitmentType(
msg.peer.LocalFeatures(), msg.peer.RemoteFeatures(),
)

// First, we'll query the fee estimator for a fee that should get the
// commitment transaction confirmed by the next few blocks (conf target
// of 3). We target the near blocks here to ensure that we'll be able
// to execute a timely unilateral channel closure if needed.
commitFeePerKw, err := f.cfg.FeeEstimator.EstimateFeePerKW(3)
if err != nil {
msg.err <- err
return
}

// For anchor channels cap the initial commit fee rate at our defined
// maximum.
if commitType == lnwallet.CommitmentTypeAnchorsZeroFeeHtlcTx &&
commitFeePerKw > f.cfg.MaxAnchorsCommitFeeRate {

commitFeePerKw = f.cfg.MaxAnchorsCommitFeeRate
}

req := &lnwallet.InitFundingReserveMsg{
ChainHash: &msg.chainHash,
PendingChanID: chanID,
Expand Down
9 changes: 8 additions & 1 deletion htlcswitch/link.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,10 @@ type ChannelLinkConfig struct {
// initiator of the channel.
MaxFeeAllocation float64

// MaxAnchorsCommitFeeRate is the max commitment fee rate we'll use as
// the initiator for channels of the anchor type.
MaxAnchorsCommitFeeRate chainfee.SatPerKWeight

// NotifyActiveLink allows the link to tell the ChannelNotifier when a
// link is first started.
NotifyActiveLink func(wire.OutPoint)
Expand Down Expand Up @@ -1090,7 +1094,10 @@ func (l *channelLink) htlcManager() {
// based on our current set fee rate. We'll cap the new
// fee rate to our max fee allocation.
commitFee := l.channel.CommitFeeRate()
maxFee := l.channel.MaxFeeRate(l.cfg.MaxFeeAllocation)
maxFee := l.channel.MaxFeeRate(
l.cfg.MaxFeeAllocation,
l.cfg.MaxAnchorsCommitFeeRate,
)
newCommitFee := chainfee.SatPerKWeight(
math.Min(float64(netFee), float64(maxFee)),
)
Expand Down
1 change: 1 addition & 0 deletions htlcswitch/test_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -1185,6 +1185,7 @@ func (h *hopNetwork) createChannelLink(server, peer *mockServer,
OutgoingCltvRejectDelta: 3,
MaxOutgoingCltvExpiry: DefaultMaxOutgoingCltvExpiry,
MaxFeeAllocation: DefaultMaxLinkFeeAllocation,
MaxAnchorsCommitFeeRate: chainfee.SatPerKVByte(10 * 1000).FeePerKWeight(),
NotifyActiveLink: func(wire.OutPoint) {},
NotifyActiveChannel: func(wire.OutPoint) {},
NotifyInactiveChannel: func(wire.OutPoint) {},
Expand Down
6 changes: 5 additions & 1 deletion lntest/itest/lnd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1166,8 +1166,12 @@ func (c commitType) calcStaticFee(numHTLCs int) btcutil.Amount {

// The anchor commitment type is slightly heavier, and we must also add
// the value of the two anchors to the resulting fee the initiator
// pays.
// pays. In addition the fee rate is capped at 10 sat/vbyte for anchor
// channels.
if c == commitTypeAnchors {
feePerKw = chainfee.SatPerKVByte(
lnwallet.DefaultAnchorsCommitMaxFeeRateSatPerVByte * 1000,
).FeePerKWeight()
commitWeight = input.AnchorCommitWeight
anchors = 2 * anchorSize
}
Expand Down
16 changes: 13 additions & 3 deletions lnwallet/channel.go
Original file line number Diff line number Diff line change
Expand Up @@ -6809,11 +6809,14 @@ func (lc *LightningChannel) CalcFee(feeRate chainfee.SatPerKWeight) btcutil.Amou

// MaxFeeRate returns the maximum fee rate given an allocation of the channel
// initiator's spendable balance. This can be useful to determine when we should
// stop proposing fee updates that exceed our maximum allocation.
// stop proposing fee updates that exceed our maximum allocation. We also take
// a fee rate cap that should be used for anchor type channels.
//
// NOTE: This should only be used for channels in which the local commitment is
// the initiator.
func (lc *LightningChannel) MaxFeeRate(maxAllocation float64) chainfee.SatPerKWeight {
func (lc *LightningChannel) MaxFeeRate(maxAllocation float64,
maxAnchorFeeRate chainfee.SatPerKWeight) chainfee.SatPerKWeight {

lc.RLock()
defer lc.RUnlock()

Expand All @@ -6828,9 +6831,16 @@ func (lc *LightningChannel) MaxFeeRate(maxAllocation float64) chainfee.SatPerKWe
// Ensure the fee rate doesn't dip below the fee floor.
_, weight := lc.availableBalance()
maxFeeRate := maxFee / (float64(weight) / 1000)
return chainfee.SatPerKWeight(
feeRate := chainfee.SatPerKWeight(
math.Max(maxFeeRate, float64(chainfee.FeePerKwFloor)),
)

// Cap anchor fee rates.
if lc.channelState.ChanType.HasAnchors() && feeRate > maxAnchorFeeRate {
Copy link
Contributor

Choose a reason for hiding this comment

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

above we use commitType == lnwallet.CommitmentTypeAnchorsZeroFeeHtlcTx, should that match here? otherwise the maximum will also apply to anchor_outputs channels?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, see this commit in the zero htlc fee PR: 7b0212f#diff-e308f199858d836b06a0c920d2dd019fbb3acb7ef0d9e3d8a360601623128d78R322

So we'll always set both the anchors bit and the zero fee htlc bit if ommitType == lnwallet.CommitmentTypeAnchorsZeroFeeHtlcTx

Copy link
Member

Choose a reason for hiding this comment

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

Correct that this'll apply to "regular" anchors as well, though with the dependent PR we essentially don't advertise it at all from teh perspective of the public network.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it will also apply the the existing few legacy anchor channels, but I think that's okay.

return maxAnchorFeeRate
}

return feeRate
}

// RemoteNextRevocation returns the channelState's RemoteNextRevocation.
Expand Down
48 changes: 36 additions & 12 deletions lnwallet/channel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7996,6 +7996,19 @@ func TestForceCloseBorkedState(t *testing.T) {
func TestChannelMaxFeeRate(t *testing.T) {
t.Parallel()

assertMaxFeeRate := func(c *LightningChannel,
maxAlloc float64, anchorMax, expFeeRate chainfee.SatPerKWeight) {

t.Helper()

maxFeeRate := c.MaxFeeRate(maxAlloc, anchorMax)
if maxFeeRate != expFeeRate {
t.Fatalf("expected max fee rate of %v with max "+
"allocation of %v, got %v", expFeeRate,
maxAlloc, maxFeeRate)
}
}

aliceChannel, _, cleanUp, err := CreateTestChannels(
channeldb.SingleFunderTweaklessBit,
)
Expand All @@ -8004,21 +8017,32 @@ func TestChannelMaxFeeRate(t *testing.T) {
}
defer cleanUp()

assertMaxFeeRate := func(maxAlloc float64,
expFeeRate chainfee.SatPerKWeight) {
assertMaxFeeRate(aliceChannel, 1.0, 0, 690607734)
assertMaxFeeRate(aliceChannel, 0.001, 0, 690607)
assertMaxFeeRate(aliceChannel, 0.000001, 0, 690)
assertMaxFeeRate(aliceChannel, 0.0000001, 0, chainfee.FeePerKwFloor)

maxFeeRate := aliceChannel.MaxFeeRate(maxAlloc)
if maxFeeRate != expFeeRate {
t.Fatalf("expected max fee rate of %v with max "+
"allocation of %v, got %v", expFeeRate,
maxAlloc, maxFeeRate)
}
// Check that anchor channels are capped at their max fee rate.
anchorChannel, _, cleanUp, err := CreateTestChannels(
channeldb.SingleFunderTweaklessBit | channeldb.AnchorOutputsBit,
)
if err != nil {
t.Fatalf("unable to create test channels: %v", err)
}
defer cleanUp()

assertMaxFeeRate(1.0, 690607734)
assertMaxFeeRate(0.001, 690607)
assertMaxFeeRate(0.000001, 690)
assertMaxFeeRate(0.0000001, chainfee.FeePerKwFloor)
// Anchor commitments are heavier, hence will the same allocation lead
// to slightly lower fee rates.
assertMaxFeeRate(
anchorChannel, 1.0, chainfee.FeePerKwFloor,
chainfee.FeePerKwFloor,
)
assertMaxFeeRate(anchorChannel, 0.001, 1000000, 444839)
assertMaxFeeRate(anchorChannel, 0.001, 300000, 300000)
assertMaxFeeRate(anchorChannel, 0.000001, 700, 444)
assertMaxFeeRate(
anchorChannel, 0.0000001, 1000000, chainfee.FeePerKwFloor,
)
}

// TestChannelFeeRateFloor asserts that valid commitments can be proposed and
Expand Down
5 changes: 5 additions & 0 deletions lnwallet/commitment.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ import (
// anchorSize is the constant anchor output size.
const anchorSize = btcutil.Amount(330)

// DefaultAnchorsCommitMaxFeeRateSatPerVByte is the default max fee rate in
// sat/vbyte the initiator will use for anchor channels. This should be enough
// to ensure propagation before anchoring down the commitment transaction.
const DefaultAnchorsCommitMaxFeeRateSatPerVByte = 10
Copy link
Contributor

Choose a reason for hiding this comment

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

very char. much wow


// CommitmentKeyRing holds all derived keys needed to construct commitment and
// HTLC transactions. The keys are derived differently depending whether the
// commitment transaction is ours or the remote peer's. Private keys associated
Expand Down
5 changes: 5 additions & 0 deletions peer/brontide.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,10 @@ type Config struct {
// commitment fee. This only applies for the initiator of the channel.
MaxChannelFeeAllocation float64

// MaxAnchorsCommitFeeRate is the maximum fee rate we'll use as an
// initiator for anchor channel commitments.
MaxAnchorsCommitFeeRate chainfee.SatPerKWeight

// ServerPubKey is the serialized, compressed public key of our lnd node.
// It is used to determine which policy (channel edge) to pass to the
// ChannelLink.
Expand Down Expand Up @@ -815,6 +819,7 @@ func (p *Brontide) addLink(chanPoint *wire.OutPoint,
TowerClient: towerClient,
MaxOutgoingCltvExpiry: p.cfg.MaxOutgoingCltvExpiry,
MaxFeeAllocation: p.cfg.MaxChannelFeeAllocation,
MaxAnchorsCommitFeeRate: p.cfg.MaxAnchorsCommitFeeRate,
NotifyActiveLink: p.cfg.ChannelNotifier.NotifyActiveLinkEvent,
NotifyActiveChannel: p.cfg.ChannelNotifier.NotifyActiveChannelEvent,
NotifyInactiveChannel: p.cfg.ChannelNotifier.NotifyInactiveChannelEvent,
Expand Down
5 changes: 5 additions & 0 deletions sample-lnd.conf
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,11 @@
; values are within [0.1, 1]. (default: 0.5)
; max-channel-fee-allocation=0.9

; The maximum fee rate in sat/vbyte that will be used for commitments of
; channels of the anchors type. Must be large enough to ensure transaction
; propagation (default: 10)
; max-commit-fee-rate-anchors=5

; If true, lnd will abort committing a migration if it would otherwise have been
; successful. This leaves the database unmodified, and still compatible with the
; previously active version of lnd.
Expand Down
6 changes: 5 additions & 1 deletion server.go
Original file line number Diff line number Diff line change
Expand Up @@ -1185,6 +1185,8 @@ func newServer(cfg *Config, listenAddrs []net.Addr,
NotifyPendingOpenChannelEvent: s.channelNotifier.NotifyPendingOpenChannelEvent,
EnableUpfrontShutdown: cfg.EnableUpfrontShutdown,
RegisteredChains: cfg.registeredChains,
MaxAnchorsCommitFeeRate: chainfee.SatPerKVByte(
s.cfg.MaxCommitFeeRateAnchors * 1000).FeePerKWeight(),
})
if err != nil {
return nil, err
Expand Down Expand Up @@ -3119,7 +3121,9 @@ func (s *server) peerConnected(conn net.Conn, connReq *connmgr.ConnReq,
UnsafeReplay: s.cfg.UnsafeReplay,
MaxOutgoingCltvExpiry: s.cfg.MaxOutgoingCltvExpiry,
MaxChannelFeeAllocation: s.cfg.MaxChannelFeeAllocation,
Quit: s.quit,
MaxAnchorsCommitFeeRate: chainfee.SatPerKVByte(
s.cfg.MaxCommitFeeRateAnchors * 1000).FeePerKWeight(),
Quit: s.quit,
}

copy(pCfg.PubKeyBytes[:], peerAddr.IdentityKey.SerializeCompressed())
Expand Down