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

[anchors] HTLC update fee clamp #4795

Closed
wants to merge 4 commits into from
Closed
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"`

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,
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
68 changes: 53 additions & 15 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 @@ -3123,16 +3127,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 @@ -3192,6 +3186,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)
Copy link
Member

Choose a reason for hiding this comment

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

While we're here, maybe also reduce this to 6-18 confs? As lately the 3 block estimate can be rather high

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean for all channel types, or only for anchors?

If it is for the general case, I would prefer to do it in a separate PR. Maybe even make it a config option?

if err != nil {
msg.err <- err
return
}

// For anchor channels cap the initial commit fee rate at our defined
// maximum.
if commitType == lnwallet.CommitmentTypeAnchors &&
Copy link
Member

Choose a reason for hiding this comment

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

I think one other failure mode of this that we'll need to consider, is that until the second-level HTLCs are 1 sat/byte each, then rising fees can end up soft borking a channel until the fee wave subsides. However this is already more or less possible today if the fee estimate rises to a point that lnd considered excessive, as even before that most of the initiator's balance ill have been bled over to fees.

Copy link
Member

Choose a reason for hiding this comment

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

I guess this is only the case if HTLCs regularly stagnate on the commitment transaction.

One other more common impact of this change would be reducing the efficacy of MPP for smaller channels since at most (for all active concurrent payments), they'll be able to use only a handful of slots. It's difficult to try and have a better "risk assessment" based on the size of the channel itself, as then we'd also need to factor in exactly how the node is used as well.

commitFeePerKw > f.cfg.MaxAnchorsCommitFeeRate {

commitFeePerKw = f.cfg.MaxAnchorsCommitFeeRate
}

req := &lnwallet.InitFundingReserveMsg{
ChainHash: &msg.chainHash,
PendingChanID: chanID,
Expand Down Expand Up @@ -3224,6 +3237,36 @@ func (f *fundingManager) handleInitFundingMsg(msg *initFundingMsg) {
// SubtractFees=true.
capacity := reservation.Capacity()

// We'll use the current value of the channels and our default policy
// to determine of required commitment constraints for the remote
// party.
chanReserve := f.cfg.RequiredRemoteChanReserve(capacity, ourDustLimit)

// As a sanity check, we ensure that the user is not trying to open a
// tiny anchor channel that would be unusable because of the fee siphon
// check we do.
if commitType == lnwallet.CommitmentTypeAnchors {
timeoutFee := lnwallet.HtlcTimeoutFee(
channeldb.AnchorOutputsBit,
f.cfg.MaxAnchorsCommitFeeRate,
)

maxNumHtlcs := chanReserve / timeoutFee
if maxNumHtlcs < lnwallet.MinAnchorHtlcSlots {
if err := reservation.Cancel(); err != nil {
fndgLog.Errorf("unable to cancel "+
"reservation: %v", err)
}

maxHtlcErr := fmt.Errorf("tiny channel, only room "+
"for %d HTLCs. Consider making a larger "+
"channel or reducing the max commit fee rate",
maxNumHtlcs)
msg.err <- maxHtlcErr
return
}
}

fndgLog.Infof("Target commit tx sat/kw for pendingID(%x): %v", chanID,
int64(commitFeePerKw))

Expand Down Expand Up @@ -3279,11 +3322,6 @@ func (f *fundingManager) handleInitFundingMsg(msg *initFundingMsg) {
// request to the remote peer, kicking off the funding workflow.
ourContribution := reservation.OurContribution()

// Finally, we'll use the current value of the channels and our default
// policy to determine of required commitment constraints for the
// remote party.
chanReserve := f.cfg.RequiredRemoteChanReserve(capacity, ourDustLimit)

fndgLog.Infof("Starting funding workflow with %v for pending_id(%x), "+
"committype=%v", msg.peer.Address(), chanID, commitType)

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 @@ -1083,7 +1087,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
Loading