Skip to content

Commit d924bee

Browse files
authored
Merge pull request #9472 from Roasbeef/0-18-5-branch-rc2
release: make v0.18.5 rc2 branch
2 parents 288f0e1 + 422a88e commit d924bee

File tree

16 files changed

+787
-454
lines changed

16 files changed

+787
-454
lines changed

build/version.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ const (
4747

4848
// AppPreRelease MUST only contain characters from semanticAlphabet per
4949
// the semantic versioning spec.
50-
AppPreRelease = "beta.rc1"
50+
AppPreRelease = "beta.rc2"
5151
)
5252

5353
func init() {

channeldb/invoices.go

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -650,18 +650,13 @@ func (d *DB) UpdateInvoice(_ context.Context, ref invpkg.InvoiceRef,
650650
return err
651651
}
652652

653-
// If the set ID hint is non-nil, then we'll use that to filter
654-
// out the HTLCs for AMP invoice so we don't need to read them
655-
// all out to satisfy the invoice callback below. If it's nil,
656-
// then we pass in the zero set ID which means no HTLCs will be
657-
// read out.
658-
var invSetID invpkg.SetID
659-
660-
if setIDHint != nil {
661-
invSetID = *setIDHint
662-
}
653+
// setIDHint can also be nil here, which means all the HTLCs
654+
// for AMP invoices are fetched. If the blank setID is passed
655+
// in, then no HTLCs are fetched for the AMP invoice. If a
656+
// specific setID is passed in, then only the HTLCs for that
657+
// setID are fetched for a particular sub-AMP invoice.
663658
invoice, err := fetchInvoice(
664-
invoiceNum, invoices, []*invpkg.SetID{&invSetID}, false,
659+
invoiceNum, invoices, []*invpkg.SetID{setIDHint}, false,
665660
)
666661
if err != nil {
667662
return err
@@ -691,7 +686,7 @@ func (d *DB) UpdateInvoice(_ context.Context, ref invpkg.InvoiceRef,
691686
// If this is an AMP update, then limit the returned AMP state
692687
// to only the requested set ID.
693688
if setIDHint != nil {
694-
filterInvoiceAMPState(updatedInvoice, &invSetID)
689+
filterInvoiceAMPState(updatedInvoice, setIDHint)
695690
}
696691

697692
return nil
@@ -848,7 +843,10 @@ func (k *kvInvoiceUpdater) Finalize(updateType invpkg.UpdateType) error {
848843
return k.storeSettleHodlInvoiceUpdate()
849844

850845
case invpkg.CancelInvoiceUpdate:
851-
return k.serializeAndStoreInvoice()
846+
// Persist all changes which where made when cancelling the
847+
// invoice. All HTLCs which were accepted are now canceled, so
848+
// we persist this state.
849+
return k.storeCancelHtlcsUpdate()
852850
}
853851

854852
return fmt.Errorf("unknown update type: %v", updateType)

cmd/commands/walletrpc_active.go

Lines changed: 33 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -270,9 +270,10 @@ var bumpFeeCommand = cli.Command{
270270
cli.Uint64Flag{
271271
Name: "conf_target",
272272
Usage: `
273-
The deadline in number of blocks that the input should be spent within.
274-
When not set, for new inputs, the default value (1008) is used; for
275-
exiting inputs, their current values will be retained.`,
273+
The conf target is the starting fee rate of the fee function expressed
274+
in number of blocks. So instead of using sat_per_vbyte the conf target
275+
can be specified and LND will query its fee estimator for the current
276+
fee rate for the given target.`,
276277
},
277278
cli.Uint64Flag{
278279
Name: "sat_per_byte",
@@ -309,6 +310,14 @@ var bumpFeeCommand = cli.Command{
309310
the budget for fee bumping; for existing inputs, their current budgets
310311
will be retained.`,
311312
},
313+
cli.Uint64Flag{
314+
Name: "deadline_delta",
315+
Usage: `
316+
The deadline delta in number of blocks that this input should be spent
317+
within to bump the transaction. When specified also a budget value is
318+
required. When the deadline is reached, ALL the budget will be spent as
319+
fee.`,
320+
},
312321
},
313322
Action: actionDecorator(bumpFee),
314323
}
@@ -346,11 +355,12 @@ func bumpFee(ctx *cli.Context) error {
346355
}
347356

348357
resp, err := client.BumpFee(ctxc, &walletrpc.BumpFeeRequest{
349-
Outpoint: protoOutPoint,
350-
TargetConf: uint32(ctx.Uint64("conf_target")),
351-
Immediate: immediate,
352-
Budget: ctx.Uint64("budget"),
353-
SatPerVbyte: ctx.Uint64("sat_per_vbyte"),
358+
Outpoint: protoOutPoint,
359+
TargetConf: uint32(ctx.Uint64("conf_target")),
360+
Immediate: immediate,
361+
Budget: ctx.Uint64("budget"),
362+
SatPerVbyte: ctx.Uint64("sat_per_vbyte"),
363+
DeadlineDelta: uint32(ctx.Uint64("deadline_delta")),
354364
})
355365
if err != nil {
356366
return err
@@ -379,9 +389,10 @@ var bumpCloseFeeCommand = cli.Command{
379389
cli.Uint64Flag{
380390
Name: "conf_target",
381391
Usage: `
382-
The deadline in number of blocks that the input should be spent within.
383-
When not set, for new inputs, the default value (1008) is used; for
384-
exiting inputs, their current values will be retained.`,
392+
The conf target is the starting fee rate of the fee function expressed
393+
in number of blocks. So instead of using sat_per_vbyte the conf target
394+
can be specified and LND will query its fee estimator for the current
395+
fee rate for the given target.`,
385396
},
386397
cli.Uint64Flag{
387398
Name: "sat_per_byte",
@@ -438,9 +449,17 @@ var bumpForceCloseFeeCommand = cli.Command{
438449
cli.Uint64Flag{
439450
Name: "conf_target",
440451
Usage: `
441-
The deadline in number of blocks that the input should be spent within.
442-
When not set, for new inputs, the default value (1008) is used; for
443-
exiting inputs, their current values will be retained.`,
452+
The conf target is the starting fee rate of the fee function expressed
453+
in number of blocks. So instead of using sat_per_vbyte the conf target
454+
can be specified and LND will query its fee estimator for the current
455+
fee rate for the given target.`,
456+
},
457+
cli.Uint64Flag{
458+
Name: "deadline_delta",
459+
Usage: `
460+
The deadline delta in number of blocks that the anchor output should
461+
be spent within to bump the closing transaction. When the deadline is
462+
reached, ALL the budget will be spent as fees.`,
444463
},
445464
cli.Uint64Flag{
446465
Name: "sat_per_byte",

docs/release-notes/release-notes-0.18.5.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,19 @@
6767
* [Golang was updated to
6868
`v1.22.11`](https://github.com/lightningnetwork/lnd/pull/9462).
6969

70+
* [Improved user experience](https://github.com/lightningnetwork/lnd/pull/9454)
71+
by returning a custom error code when HTLC carries incorrect custom records.
72+
73+
* [Make input validation stricter](https://github.com/lightningnetwork/lnd/pull/9470)
74+
when using the `BumpFee`, `BumpCloseFee(deprecated)` and `BumpForceCloseFee`
75+
RPCs. For the `BumpFee` RPC the new param `deadline_delta` is introduced. For
76+
the `BumpForceCloseFee` RPC the param `conf_target` was added. The conf_target
77+
changed in its meaning for all the RPCs which had it before. Now it is used
78+
for estimating the starting fee rate instead of being treated as the deadline,
79+
and it cannot be set together with `StartingFeeRate`. Moreover if the user now
80+
specifies the `deadline_delta` param, the budget value has to be set as well.
81+
82+
7083
## Tooling and Documentation
7184

7285
# Contributors (Alphabetical Order)

invoices/interface.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,11 @@ type InvoiceDB interface {
5656
// passed payment hash. If an invoice matching the passed payment hash
5757
// doesn't exist within the database, then the action will fail with a
5858
// "not found" error.
59+
// The setIDHint is used to signal whether AMP HTLCs should be fetched
60+
// for the invoice. If a blank setID is passed no HTLCs will be fetched
61+
// in case of an AMP invoice. Nil means all HTLCs for all sub AMP
62+
// invoices will be fetched and if a specific setID is supplied only
63+
// HTLCs for that setID will be fetched.
5964
//
6065
// The update is performed inside the same database transaction that
6166
// fetches the invoice and is therefore atomic. The fields to update

invoices/invoiceregistry.go

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -704,7 +704,10 @@ func (i *InvoiceRegistry) cancelSingleHtlc(invoiceRef InvoiceRef,
704704
// Try to mark the specified htlc as canceled in the invoice database.
705705
// Intercept the update descriptor to set the local updated variable. If
706706
// no invoice update is performed, we can return early.
707+
// setID is only set for AMP HTLCs, so it can be nil and it is expected
708+
// to be nil for non-AMP HTLCs.
707709
setID := (*SetID)(invoiceRef.SetID())
710+
708711
var updated bool
709712
invoice, err := i.idb.UpdateInvoice(
710713
context.Background(), invoiceRef, setID,
@@ -1014,6 +1017,9 @@ func (i *InvoiceRegistry) notifyExitHopHtlcLocked(
10141017
HtlcResolution, invoiceExpiry, error) {
10151018

10161019
invoiceRef := ctx.invoiceRef()
1020+
1021+
// This setID is only set for AMP HTLCs, so it can be nil and it is
1022+
// also expected to be nil for non-AMP HTLCs.
10171023
setID := (*SetID)(ctx.setID())
10181024

10191025
// We need to look up the current state of the invoice in order to send
@@ -1370,7 +1376,15 @@ func (i *InvoiceRegistry) SettleHodlInvoice(ctx context.Context,
13701376

13711377
hash := preimage.Hash()
13721378
invoiceRef := InvoiceRefByHash(hash)
1373-
invoice, err := i.idb.UpdateInvoice(ctx, invoiceRef, nil, updateInvoice)
1379+
1380+
// AMP hold invoices are not supported so we set the setID to nil.
1381+
// For non-AMP invoices this parameter is ignored during the fetching
1382+
// of the database state.
1383+
setID := (*SetID)(nil)
1384+
1385+
invoice, err := i.idb.UpdateInvoice(
1386+
ctx, invoiceRef, setID, updateInvoice,
1387+
)
13741388
if err != nil {
13751389
log.Errorf("SettleHodlInvoice with preimage %v: %v",
13761390
preimage, err)
@@ -1454,10 +1468,14 @@ func (i *InvoiceRegistry) cancelInvoiceImpl(ctx context.Context,
14541468
}, nil
14551469
}
14561470

1471+
// If it's an AMP invoice we need to fetch all AMP HTLCs here so that
1472+
// we can cancel all of HTLCs which are in the accepted state across
1473+
// different setIDs.
1474+
setID := (*SetID)(nil)
14571475
invoiceRef := InvoiceRefByHash(payHash)
1458-
1459-
// We pass a nil setID which means no HTLCs will be read out.
1460-
invoice, err := i.idb.UpdateInvoice(ctx, invoiceRef, nil, updateInvoice)
1476+
invoice, err := i.idb.UpdateInvoice(
1477+
ctx, invoiceRef, setID, updateInvoice,
1478+
)
14611479

14621480
// Implement idempotency by returning success if the invoice was already
14631481
// canceled.
@@ -1483,8 +1501,8 @@ func (i *InvoiceRegistry) cancelInvoiceImpl(ctx context.Context,
14831501
// that are waiting for resolution. Any htlcs that were already canceled
14841502
// before, will be notified again. This isn't necessary but doesn't hurt
14851503
// either.
1486-
//
1487-
// TODO(ziggie): Also consider AMP HTLCs here.
1504+
// For AMP invoices we fetched all AMP HTLCs for all sub AMP invoices
1505+
// here so we can clean up all of them.
14881506
for key, htlc := range invoice.Htlcs {
14891507
if htlc.State != HtlcStateCanceled {
14901508
continue
@@ -1496,6 +1514,7 @@ func (i *InvoiceRegistry) cancelInvoiceImpl(ctx context.Context,
14961514
),
14971515
)
14981516
}
1517+
14991518
i.notifyClients(payHash, invoice, nil)
15001519

15011520
// Attempt to also delete the invoice if requested through the registry

invoices/invoiceregistry_test.go

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,10 @@ func TestInvoiceRegistry(t *testing.T) {
117117
name: "FailPartialAMPPayment",
118118
test: testFailPartialAMPPayment,
119119
},
120+
{
121+
name: "CancelAMPInvoicePendingHTLCs",
122+
test: testCancelAMPInvoicePendingHTLCs,
123+
},
120124
}
121125

122126
makeKeyValueDB := func(t *testing.T) (invpkg.InvoiceDB,
@@ -2441,3 +2445,130 @@ func testFailPartialAMPPayment(t *testing.T,
24412445
"expected HTLC to be canceled")
24422446
}
24432447
}
2448+
2449+
// testCancelAMPInvoicePendingHTLCs tests the case where an AMP invoice is
2450+
// canceled and the remaining HTLCs are also canceled so that no HTLCs are left
2451+
// in the accepted state.
2452+
func testCancelAMPInvoicePendingHTLCs(t *testing.T,
2453+
makeDB func(t *testing.T) (invpkg.InvoiceDB, *clock.TestClock)) {
2454+
2455+
t.Parallel()
2456+
2457+
ctx := newTestContext(t, nil, makeDB)
2458+
ctxb := context.Background()
2459+
2460+
const (
2461+
expiry = uint32(testCurrentHeight + 20)
2462+
numShards = 4
2463+
)
2464+
2465+
var (
2466+
shardAmt = testInvoiceAmount / lnwire.MilliSatoshi(numShards)
2467+
payAddr [32]byte
2468+
)
2469+
_, err := rand.Read(payAddr[:])
2470+
require.NoError(t, err)
2471+
2472+
// Create an AMP invoice we are going to pay via a multi-part payment.
2473+
ampInvoice := newInvoice(t, false, true)
2474+
2475+
// An AMP invoice is referenced by the payment address.
2476+
ampInvoice.Terms.PaymentAddr = payAddr
2477+
2478+
_, err = ctx.registry.AddInvoice(
2479+
ctxb, ampInvoice, testInvoicePaymentHash,
2480+
)
2481+
require.NoError(t, err)
2482+
2483+
htlcPayloadSet1 := &mockPayload{
2484+
mpp: record.NewMPP(testInvoiceAmount, payAddr),
2485+
// We are not interested in settling the AMP HTLC so we don't
2486+
// use valid shares.
2487+
amp: record.NewAMP([32]byte{1}, [32]byte{1}, 1),
2488+
}
2489+
2490+
// Send first HTLC which pays part of the invoice.
2491+
hodlChan1 := make(chan interface{}, 1)
2492+
resolution, err := ctx.registry.NotifyExitHopHtlc(
2493+
lntypes.Hash{1}, shardAmt, expiry, testCurrentHeight,
2494+
getCircuitKey(1), hodlChan1, nil, htlcPayloadSet1,
2495+
)
2496+
require.NoError(t, err)
2497+
require.Nil(t, resolution, "did not expect direct resolution")
2498+
2499+
htlcPayloadSet2 := &mockPayload{
2500+
mpp: record.NewMPP(testInvoiceAmount, payAddr),
2501+
// We are not interested in settling the AMP HTLC so we don't
2502+
// use valid shares.
2503+
amp: record.NewAMP([32]byte{2}, [32]byte{2}, 1),
2504+
}
2505+
2506+
// Send htlc 2 which should be added to the invoice as expected.
2507+
hodlChan2 := make(chan interface{}, 1)
2508+
resolution, err = ctx.registry.NotifyExitHopHtlc(
2509+
lntypes.Hash{2}, shardAmt, expiry, testCurrentHeight,
2510+
getCircuitKey(2), hodlChan2, nil, htlcPayloadSet2,
2511+
)
2512+
require.NoError(t, err)
2513+
require.Nil(t, resolution, "did not expect direct resolution")
2514+
2515+
require.Eventuallyf(t, func() bool {
2516+
inv, err := ctx.registry.LookupInvoice(
2517+
ctxb, testInvoicePaymentHash,
2518+
)
2519+
require.NoError(t, err)
2520+
2521+
return len(inv.Htlcs) == 2
2522+
}, testTimeout, time.Millisecond*100, "HTLCs not added to invoice")
2523+
2524+
// expire the invoice here.
2525+
ctx.clock.SetTime(testTime.Add(65 * time.Minute))
2526+
2527+
// Expect HLTC 1 to be canceled via the MPPTimeout fail resolution.
2528+
select {
2529+
case resolution := <-hodlChan1:
2530+
htlcResolution, _ := resolution.(invpkg.HtlcResolution)
2531+
_, ok := htlcResolution.(*invpkg.HtlcFailResolution)
2532+
require.True(
2533+
t, ok, "expected fail resolution, got: %T", resolution,
2534+
)
2535+
2536+
case <-time.After(testTimeout):
2537+
t.Fatal("timeout waiting for HTLC resolution")
2538+
}
2539+
2540+
// Expect HLTC 2 to be canceled via the MPPTimeout fail resolution.
2541+
select {
2542+
case resolution := <-hodlChan2:
2543+
htlcResolution, _ := resolution.(invpkg.HtlcResolution)
2544+
_, ok := htlcResolution.(*invpkg.HtlcFailResolution)
2545+
require.True(
2546+
t, ok, "expected fail resolution, got: %T", resolution,
2547+
)
2548+
2549+
case <-time.After(testTimeout):
2550+
t.Fatal("timeout waiting for HTLC resolution")
2551+
}
2552+
2553+
require.Eventuallyf(t, func() bool {
2554+
inv, err := ctx.registry.LookupInvoice(
2555+
ctxb, testInvoicePaymentHash,
2556+
)
2557+
require.NoError(t, err)
2558+
2559+
return inv.State == invpkg.ContractCanceled
2560+
}, testTimeout, time.Millisecond*100, "invoice not canceled")
2561+
2562+
// Fetch the invoice again and compare the number of cancelled HTLCs.
2563+
inv, err := ctx.registry.LookupInvoice(
2564+
ctxb, testInvoicePaymentHash,
2565+
)
2566+
require.NoError(t, err)
2567+
2568+
// Make sure all HTLCs are in the cancelled state.
2569+
require.Len(t, inv.Htlcs, 2)
2570+
for _, htlc := range inv.Htlcs {
2571+
require.Equal(t, invpkg.HtlcStateCanceled, htlc.State,
2572+
"expected HTLC to be canceled")
2573+
}
2574+
}

0 commit comments

Comments
 (0)