Skip to content

[custom channels]: Assert fix for duplicate script key issues in MPP payments #908

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Nov 27, 2024
Merged

[custom channels]: Assert fix for duplicate script key issues in MPP payments #908

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f68c9c4
mod: bump to latest tapd and lnd versions
guggero Nov 27, 2024
81f25b7
itest: add more assertions, run force close with MPP shards
guggero Nov 27, 2024
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
4 changes: 2 additions & 2 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@ require (
github.com/lightninglabs/lightning-node-connect v0.3.2-alpha.0.20240822142323-ee4e7ff52f83
github.com/lightninglabs/lightning-terminal/autopilotserverrpc v0.0.1
github.com/lightninglabs/lightning-terminal/litrpc v1.0.0
github.com/lightninglabs/lndclient v0.18.4-6
github.com/lightninglabs/lndclient v0.18.4-7
github.com/lightninglabs/loop v0.28.8-beta.0.20241022072406-1e8ae31ddc27
github.com/lightninglabs/loop/looprpc v1.0.0
github.com/lightninglabs/loop/swapserverrpc v1.0.10
github.com/lightninglabs/pool v0.6.5-beta.0.20241015105339-044cb451b5df
github.com/lightninglabs/pool/auctioneerrpc v1.1.2
github.com/lightninglabs/pool/poolrpc v1.0.0
github.com/lightninglabs/taproot-assets v0.4.2-0.20241125202051-783fb1e5ab03
github.com/lightninglabs/taproot-assets v0.5.0-rc1
github.com/lightningnetwork/lnd v0.18.4-beta.rc1
github.com/lightningnetwork/lnd/cert v1.2.2
github.com/lightningnetwork/lnd/fn v1.2.3
Expand Down
8 changes: 4 additions & 4 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1157,8 +1157,8 @@ github.com/lightninglabs/lightning-node-connect v0.3.2-alpha.0.20240822142323-ee
github.com/lightninglabs/lightning-node-connect v0.3.2-alpha.0.20240822142323-ee4e7ff52f83/go.mod h1:+SasPOt0evcJdfApb/ALTaTz4x3a2/kWy5KqFoTpiX8=
github.com/lightninglabs/lightning-node-connect/hashmailrpc v1.0.2 h1:Er1miPZD2XZwcfE4xoS5AILqP1mj7kqnhbBSxW9BDxY=
github.com/lightninglabs/lightning-node-connect/hashmailrpc v1.0.2/go.mod h1:antQGRDRJiuyQF6l+k6NECCSImgCpwaZapATth2Chv4=
github.com/lightninglabs/lndclient v0.18.4-6 h1:2PYXJLvXtDmt2A12Q7/RVRdh1CRaMyVAqJmuGsxK/d8=
github.com/lightninglabs/lndclient v0.18.4-6/go.mod h1:qaIx+eqEV+Bdf1j7GVeJiDqJbtZXsr9XTfHu/8HmgQU=
github.com/lightninglabs/lndclient v0.18.4-7 h1:3lV3jeaL66wtxFeR+7YTo+1ZJ8YzD3gYHG8U9yas3YM=
github.com/lightninglabs/lndclient v0.18.4-7/go.mod h1:qaIx+eqEV+Bdf1j7GVeJiDqJbtZXsr9XTfHu/8HmgQU=
github.com/lightninglabs/loop v0.28.8-beta.0.20241022072406-1e8ae31ddc27 h1:eZBvG9XvDL0zsUIqFfD7SCk+Ex8rGWEL8j5UQ/aqjco=
github.com/lightninglabs/loop v0.28.8-beta.0.20241022072406-1e8ae31ddc27/go.mod h1:4B1DqrcOc5Yv9KyclAeQJY9Ah9UMX7RpI4Uru7aEzl4=
github.com/lightninglabs/loop/looprpc v1.0.0 h1:xry4QPCZShPww660xJm1BVcNFj8etgNeN2vMpfsv3c4=
Expand All @@ -1177,8 +1177,8 @@ github.com/lightninglabs/pool/poolrpc v1.0.0 h1:vvosrgNx9WXF4mcHGqLjZOW8wNM0q+BL
github.com/lightninglabs/pool/poolrpc v1.0.0/go.mod h1:ZqpEpBFRMMBAerMmilEjh27tqauSXDwLaLR0O3jvmMA=
github.com/lightninglabs/protobuf-go-hex-display v1.34.2-hex-display h1:w7FM5LH9Z6CpKxl13mS48idsu6F+cEZf0lkyiV+Dq9g=
github.com/lightninglabs/protobuf-go-hex-display v1.34.2-hex-display/go.mod h1:qYOHts0dSfpeUzUFpOMr/WGzszTmLH+DiWniOlNbLDw=
github.com/lightninglabs/taproot-assets v0.4.2-0.20241125202051-783fb1e5ab03 h1:O/+vkoVnoB0HsUnBPRDFg5E4khFapbi82OEZg+PScEY=
github.com/lightninglabs/taproot-assets v0.4.2-0.20241125202051-783fb1e5ab03/go.mod h1:xWtQ/I7KAUIJlLU0fdiQgwRUXIKoXuNCjpqLcdge6eQ=
github.com/lightninglabs/taproot-assets v0.5.0-rc1 h1:UuDuvOJErqvapdF5gA8gfTy7X8Dfi96fCoNhrRuB9ZA=
github.com/lightninglabs/taproot-assets v0.5.0-rc1/go.mod h1:3MxoqsBdZGju3ExSIA9kFle5nqjWeb4alK4aVlfT0tA=
github.com/lightningnetwork/lightning-onion v1.2.1-0.20240712235311-98bd56499dfb h1:yfM05S8DXKhuCBp5qSMZdtSwvJ+GFzl94KbXMNB1JDY=
github.com/lightningnetwork/lightning-onion v1.2.1-0.20240712235311-98bd56499dfb/go.mod h1:c0kvRShutpj3l6B9WtTsNTBUtjSmjZXbJd9ZBRQOSKI=
github.com/lightningnetwork/lnd v0.18.4-beta.rc1 h1:z6hFKvtbfo8udPrIb81GbSoKlUWd06d4LRxTkD19IMQ=
Expand Down
131 changes: 111 additions & 20 deletions itest/litd_custom_channels_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2773,9 +2773,20 @@ func testCustomChannelsFee(_ context.Context,
// testCustomChannelsHtlcForceClose tests that we can force close a channel
// with HTLCs in both directions and that the HTLC outputs are correctly
// swept.
func testCustomChannelsHtlcForceClose(_ context.Context, net *NetworkHarness,
func testCustomChannelsHtlcForceClose(ctxb context.Context, net *NetworkHarness,
t *harnessTest) {

runCustomChannelsHtlcForceClose(ctxb, t, net, false)
runCustomChannelsHtlcForceClose(ctxb, t, net, true)
}

// runCustomChannelsHtlcForceClose is a helper function that runs the HTLC force
// close test with the given MPP setting.
func runCustomChannelsHtlcForceClose(ctxb context.Context, t *harnessTest,
net *NetworkHarness, mpp bool) {

t.Logf("Running test with MPP: %v", mpp)

lndArgs := slices.Clone(lndArgsTemplate)
litdArgs := slices.Clone(litdArgsTemplate)

Expand Down Expand Up @@ -2829,7 +2840,6 @@ func testCustomChannelsHtlcForceClose(_ context.Context, net *NetworkHarness,
// With the assets created, and synced -- we'll now open the channel
// between Alice and Bob.
t.Logf("Opening asset channels...")
ctxb := context.Background()
assetFundResp, err := aliceTap.FundChannel(
ctxb, &tchrpc.FundChannelRequest{
AssetAmount: fundingAmount,
Expand All @@ -2853,7 +2863,7 @@ func testCustomChannelsHtlcForceClose(_ context.Context, net *NetworkHarness,
// to be able to extend HTLCs in the other direction.
const (
numPayments = 10
keySendAmount = 1_000
keySendAmount = 2_500
)
for i := 0; i < numPayments; i++ {
sendAssetKeySendPayment(
Expand All @@ -2866,11 +2876,19 @@ func testCustomChannelsHtlcForceClose(_ context.Context, net *NetworkHarness,
// Now that both parties have some funds, we'll move onto the main test.
//
// We'll make 2 hodl invoice for each peer, so 4 total. From Alice's
// PoV, she'll have two outgoing HTLCs, and two incoming HTLCs.
// PoV, she'll have two outgoing HTLCs (or +4 with MPP), and two
// incoming HTLCs.
var (
bobHodlInvoices []assetHodlInvoice
aliceHodlInvoices []assetHodlInvoice
assetInvoiceAmt = 100

// The default oracle rate is 17_180 mSat/asset unit, so 10_000
// will be equal to 171_800_000 mSat. When we use the mpp bool
// for the smallShards param of payInvoiceWithAssets, that
// means we'll split the payment into shards of 80_000_000 mSat
// max. So we'll get three shards per payment.
assetInvoiceAmt = 10_000
assetsPerMPPShard = 4656
)
for i := 0; i < 2; i++ {
bobHodlInvoices = append(
Expand All @@ -2892,7 +2910,7 @@ func testCustomChannelsHtlcForceClose(_ context.Context, net *NetworkHarness,
// yet.
for _, aliceInvoice := range aliceHodlInvoices {
payInvoiceWithAssets(
t.t, bob, alice, aliceInvoice.payReq, assetID, false,
t.t, bob, alice, aliceInvoice.payReq, assetID, mpp,
fn.Some(lnrpc.Payment_IN_FLIGHT),
)
}
Expand All @@ -2903,9 +2921,15 @@ func testCustomChannelsHtlcForceClose(_ context.Context, net *NetworkHarness,
)
}

// At this point, both sides should have 4 HTLCs active.
assertNumHtlcs(t.t, alice, 4)
assertNumHtlcs(t.t, bob, 4)
// At this point, both sides should have 4 (or +4 with MPP) HTLCs
// active.
numHtlcs := 4
if mpp {
numAdditionalShards := assetInvoiceAmt / assetsPerMPPShard
numHtlcs += numAdditionalShards * 2
}
assertNumHtlcs(t.t, alice, numHtlcs)
assertNumHtlcs(t.t, bob, numHtlcs)

// Before we force close, we'll grab the current height, the CSV delay
// needed, and also the absolute timeout of the set of active HTLCs.
Expand Down Expand Up @@ -2947,8 +2971,8 @@ func testCustomChannelsHtlcForceClose(_ context.Context, net *NetworkHarness,
// At this point, the commitment transaction has been mined, and we have
// 4 total HTLCs on Alice's commitment transaction:
//
// * 2x outgoing HTLCs to Alice to Bob
// * 2x incoming HTLCs from Bob to Alice
// * 2x outgoing HTLCs from Alice to Bob
// * 2x incoming HTLCs from Bob to Alice (+2 with MPP)
//
// We'll leave half the HTLCs timeout, while pulling the other half.
// To start, we'll signal Bob to settle one of his incoming HTLCs on
Expand All @@ -2971,7 +2995,7 @@ func testCustomChannelsHtlcForceClose(_ context.Context, net *NetworkHarness,
// We'll mine an empty block to get the sweeper to tick.
mineBlocks(t, net, 1, 0)

sweepTx1, err := waitForNTxsInMempool(
bobSweepTx1, err := waitForNTxsInMempool(
net.Miner.Client, 1, shortTimeout,
)
require.NoError(t.t, err)
Expand All @@ -2984,15 +3008,17 @@ func testCustomChannelsHtlcForceClose(_ context.Context, net *NetworkHarness,
// At this point, we should have the next sweep transaction in the
// mempool: Bob's incoming HTLC sweep directly off the commitment
// transaction.
sweepTx2, err := waitForNTxsInMempool(net.Miner.Client, 1, shortTimeout)
bobSweepTx2, err := waitForNTxsInMempool(
net.Miner.Client, 1, shortTimeout,
)
require.NoError(t.t, err)

// We'll now mine the next block, which should confirm Bob's HTLC sweep
// transaction.
mineBlocks(t, net, 1, 1)

bobSweepTransfer1 := locateAssetTransfers(t.t, bobTap, *sweepTx1[0])
bobSweepTransfer2 := locateAssetTransfers(t.t, bobTap, *sweepTx2[0])
bobSweepTransfer1 := locateAssetTransfers(t.t, bobTap, *bobSweepTx1[0])
bobSweepTransfer2 := locateAssetTransfers(t.t, bobTap, *bobSweepTx2[0])
t.Logf("Bob's sweep transfer 1: %v",
toProtoJSON(t.t, bobSweepTransfer1))
t.Logf("Bob's sweep transfer 2: %v",
Expand All @@ -3006,9 +3032,8 @@ func testCustomChannelsHtlcForceClose(_ context.Context, net *NetworkHarness,
// RFQ conversion.
bobExpectedBalance := closeExpiryInfo.remoteAssetBalance +
uint64(assetInvoiceAmt-1)
assertSpendableBalance(
t.t, bobTap, assetID, bobExpectedBalance,
)
t.Logf("Expecting Bob's balance to be %d", bobExpectedBalance)
assertSpendableBalance(t.t, bobTap, assetID, bobExpectedBalance)

// With Bob's HTLC settled, we'll now have Alice do the same. For her,
// it'll be a 2nd level sweep, which requires an extra transaction.
Expand All @@ -3018,15 +3043,26 @@ func testCustomChannelsHtlcForceClose(_ context.Context, net *NetworkHarness,
// that she's swept everything properly. With the way the sweeper works,
// we need to mine one extra block before the sweeper picks things up.
mineBlocks(t, net, 1, 0)
time.Sleep(time.Second * 1)

aliceSweepTx1, err := waitForNTxsInMempool(
net.Miner.Client, 1, shortTimeout,
)
require.NoError(t.t, err)

mineBlocks(t, net, 1, 1)

aliceSweepTransfer1 := locateAssetTransfers(
t.t, aliceTap, *aliceSweepTx1[0],
)
t.Logf("Alice's sweep transfer 1: %v",
toProtoJSON(t.t, aliceSweepTransfer1))

t.Logf("Confirming Alice's to-local sweep")

// With this extra block mined, Alice's settled balance should be the
// starting balance, minus the 2 HTLCs, plus her settled balance.
aliceExpectedBalance := itestAsset.Amount - fundingAmount
aliceExpectedBalance += uint64(closeExpiryInfo.localAssetBalance)
aliceExpectedBalance += closeExpiryInfo.localAssetBalance
assertSpendableBalance(
t.t, aliceTap, assetID, aliceExpectedBalance,
)
Expand Down Expand Up @@ -3056,9 +3092,27 @@ func testCustomChannelsHtlcForceClose(_ context.Context, net *NetworkHarness,
// If the block mined above didn't also mine our sweep, then we'll mine
// one final block which will confirm Alice's sweep transaction.
if len(sweepBlocks[0].Transactions) == 1 {
sweepTx, err := waitForNTxsInMempool(
net.Miner.Client, 1, shortTimeout,
)
require.NoError(t.t, err)

// With the sweep transaction in the mempool, we'll mine a block
// to confirm the sweep.
mineBlocks(t, net, 1, 1)

aliceSweepTransfer := locateAssetTransfers(
t.t, aliceTap, *sweepTx[0],
)
t.Logf("Alice's first-level sweep transfer: %v",
toProtoJSON(t.t, aliceSweepTransfer))
} else {
sweepTx := sweepBlocks[0].Transactions[1]
aliceSweepTransfer := locateAssetTransfers(
t.t, aliceTap, sweepTx.TxHash(),
)
t.Logf("Alice's first-level sweep transfer: %v",
Copy link
Contributor

@gijswijs gijswijs Nov 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

nit:
This if-else statement could be extracted into its own function, with a string parameter for the Logf statement. This prevents the code duplication that is going on in this test.

toProtoJSON(t.t, aliceSweepTransfer))
}

t.Logf("Confirming Alice's second level remote HTLC success sweep")
Expand All @@ -3085,7 +3139,25 @@ func testCustomChannelsHtlcForceClose(_ context.Context, net *NetworkHarness,
// If the block mined above didn't also mine our sweep, then we'll mine
// one final block which will confirm Alice's sweep transaction.
if len(sweepBlocks[0].Transactions) == 1 {
sweepTx, err := waitForNTxsInMempool(
net.Miner.Client, 1, shortTimeout,
)
require.NoError(t.t, err)

mineBlocks(t, net, 1, 1)

aliceSweepTransfer := locateAssetTransfers(
t.t, aliceTap, *sweepTx[0],
)
t.Logf("Alice's second-level sweep transfer: %v",
toProtoJSON(t.t, aliceSweepTransfer))
} else {
sweepTx := sweepBlocks[0].Transactions[1]
aliceSweepTransfer := locateAssetTransfers(
t.t, aliceTap, sweepTx.TxHash(),
)
t.Logf("Alice's second-level sweep transfer: %v",
toProtoJSON(t.t, aliceSweepTransfer))
}

// With the sweep transaction confirmed, Alice's balance should have
Expand Down Expand Up @@ -3153,14 +3225,33 @@ func testCustomChannelsHtlcForceClose(_ context.Context, net *NetworkHarness,
// If the block mined above didn't also mine our sweep, then we'll mine
// one final block which will confirm Alice's sweep transaction.
if len(sweepBlocks[0].Transactions) == 1 {
sweepTx, err := waitForNTxsInMempool(
net.Miner.Client, 1, shortTimeout,
)
require.NoError(t.t, err)

// We'll mine one final block which will confirm Alice's sweep
// transaction.
mineBlocks(t, net, 1, 1)

aliceSweepTransfer := locateAssetTransfers(
t.t, aliceTap, *sweepTx[0],
)
t.Logf("Alice's final timeout sweep transfer: %v",
toProtoJSON(t.t, aliceSweepTransfer))
} else {
sweepTx := sweepBlocks[0].Transactions[1]
aliceSweepTransfer := locateAssetTransfers(
t.t, aliceTap, sweepTx.TxHash(),
)
t.Logf("Alice's final timeout sweep transfer: %v",
toProtoJSON(t.t, aliceSweepTransfer))
}

// Finally, we'll assert that Alice's balance has been incremented by
// the timeout value.
aliceExpectedBalance += uint64(assetInvoiceAmt - 1)
t.Logf("Expecting Alice's balance to be %d", aliceExpectedBalance)
assertSpendableBalance(
t.t, aliceTap, assetID, aliceExpectedBalance,
)
Expand Down
Loading