diff --git a/docs/release-notes/release-notes-0.19.0.md b/docs/release-notes/release-notes-0.19.0.md index bdbf12735..6e93baf9b 100644 --- a/docs/release-notes/release-notes-0.19.0.md +++ b/docs/release-notes/release-notes-0.19.0.md @@ -118,6 +118,10 @@ keysend payment validation is stricter. * [Fixed](https://github.com/lightningnetwork/lnd/pull/9746) a possible panic when running LND with an aux component injected (custom channels). +* [Fixed a bug](https://github.com/lightningnetwork/lnd/pull/9750): if a Taproot + address is added to LND using the `ImportTapscript` RPC, LND previously failed + to perform a cooperative close to that address. + # New Features * Add support for [archiving channel backup](https://github.com/lightningnetwork/lnd/pull/9232) diff --git a/itest/list_exclude_test.go b/itest/list_exclude_test.go index fd0e1c03a..77a7d078e 100644 --- a/itest/list_exclude_test.go +++ b/itest/list_exclude_test.go @@ -13,7 +13,7 @@ import ( // be excluded from the test suite atm. // // TODO(yy): fix these tests and remove them from this list. -var excludedTestsWindows = []string{ +var excludedTestsWindows = append(append([]string{ "batch channel funding", "zero conf channel open", "open channel with unstable utxos", @@ -45,26 +45,12 @@ var excludedTestsWindows = []string{ "wipe forwarding packages", "coop close with htlcs", - "coop close with external delivery", "forward interceptor restart", "forward interceptor dedup htlcs", "invoice HTLC modifier basic", "lookup htlc resolution", - "remote signer-taproot", - "remote signer-account import", - "remote signer-bump fee", - "remote signer-funding input types", - "remote signer-funding async payments taproot", - "remote signer-funding async payments", - "remote signer-random seed", - "remote signer-verify msg", - "remote signer-channel open", - "remote signer-shared key", - "remote signer-psbt", - "remote signer-sign output raw", - "on chain to blinded", "query blinded route", @@ -76,7 +62,12 @@ var excludedTestsWindows = []string{ // more investigation is needed. "channel force close-anchor restart", "channel force close-simple taproot restart", -} +}, extractNames( + "coop close with external delivery", + coopCloseWithExternalTestCases)..., +), + extractNames("remote signer", remoteSignerTestCases)..., +) // filterWindowsFlakyTests filters out the flaky tests that are excluded from // the test suite on Windows. diff --git a/itest/list_on_test.go b/itest/list_on_test.go index 320f69f9e..88b7bb395 100644 --- a/itest/list_on_test.go +++ b/itest/list_on_test.go @@ -5,6 +5,7 @@ package itest import ( "fmt" + "github.com/lightningnetwork/lnd/fn/v2" "github.com/lightningnetwork/lnd/lntest" ) @@ -642,10 +643,6 @@ var allTestCases = []*lntest.TestCase{ Name: "sweep commit output and anchor", TestFunc: testSweepCommitOutputAndAnchor, }, - { - Name: "coop close with external delivery", - TestFunc: testCoopCloseWithExternalDelivery, - }, { Name: "payment failed htlc local swept", TestFunc: testPaymentFailedHTLCLocalSwept, @@ -720,6 +717,13 @@ func appendPrefixed(prefix string, testCases, return testCases } +// extractNames is used to extract tests' names from a group of prefixed tests. +func extractNames(prefix string, subtestCases []*lntest.TestCase) []string { + return fn.Map(subtestCases, func(tc *lntest.TestCase) string { + return fmt.Sprintf("%s-%s", prefix, tc.Name) + }) +} + func init() { // Register subtests. allTestCases = appendPrefixed( @@ -762,6 +766,10 @@ func init() { allTestCases = appendPrefixed( "wallet", allTestCases, walletTestCases, ) + allTestCases = appendPrefixed( + "coop close with external delivery", allTestCases, + coopCloseWithExternalTestCases, + ) // Prepare the test cases for windows to exclude some of the flaky // ones. diff --git a/itest/lnd_coop_close_external_delivery_test.go b/itest/lnd_coop_close_external_delivery_test.go index eea09a48d..4c2f0a5c4 100644 --- a/itest/lnd_coop_close_external_delivery_test.go +++ b/itest/lnd_coop_close_external_delivery_test.go @@ -2,75 +2,209 @@ package itest import ( "fmt" - "testing" "github.com/btcsuite/btcd/btcutil" + "github.com/btcsuite/btcd/txscript" "github.com/lightningnetwork/lnd/lnrpc" + "github.com/lightningnetwork/lnd/lnrpc/walletrpc" "github.com/lightningnetwork/lnd/lntest" "github.com/lightningnetwork/lnd/lntest/wait" "github.com/stretchr/testify/require" ) -func testCoopCloseWithExternalDelivery(ht *lntest.HarnessTest) { - ok := ht.Run("set P2WPKH delivery address at open", func(t *testing.T) { - tt := ht.Subtest(t) - testCoopCloseWithExternalDeliveryImpl( - tt, true, lnrpc.AddressType_UNUSED_WITNESS_PUBKEY_HASH, - ) - }) - // Abort the test if failed. - if !ok { - return - } - - ok = ht.Run("set P2WPKH delivery address at close", func(t *testing.T) { - tt := ht.Subtest(t) - testCoopCloseWithExternalDeliveryImpl( - tt, false, lnrpc.AddressType_UNUSED_WITNESS_PUBKEY_HASH, - ) - }) - // Abort the test if failed. - if !ok { - return - } - - ok = ht.Run("set P2TR delivery address at open", func(t *testing.T) { - tt := ht.Subtest(t) - testCoopCloseWithExternalDeliveryImpl( - tt, true, lnrpc.AddressType_UNUSED_TAPROOT_PUBKEY, - ) - }) - // Abort the test if failed. - if !ok { - return - } - - ht.Run("set P2TR delivery address at close", func(t *testing.T) { - tt := ht.Subtest(t) - testCoopCloseWithExternalDeliveryImpl( - tt, false, lnrpc.AddressType_UNUSED_TAPROOT_PUBKEY, - ) - }) +var coopCloseWithExternalTestCases = []*lntest.TestCase{ + { + Name: "set P2WPKH delivery address at open", + TestFunc: func(ht *lntest.HarnessTest) { + testCoopCloseWithExternalDelivery( + ht, true, false, false, + lnrpc.AddressType_UNUSED_WITNESS_PUBKEY_HASH, + ) + }, + }, + { + Name: "set P2WPKH delivery address at close", + TestFunc: func(ht *lntest.HarnessTest) { + testCoopCloseWithExternalDelivery( + ht, false, false, false, + lnrpc.AddressType_UNUSED_WITNESS_PUBKEY_HASH, + ) + }, + }, + { + Name: "set P2TR delivery address at open", + TestFunc: func(ht *lntest.HarnessTest) { + testCoopCloseWithExternalDelivery( + ht, true, false, false, + lnrpc.AddressType_UNUSED_TAPROOT_PUBKEY, + ) + }, + }, + { + Name: "set P2TR delivery address at close", + TestFunc: func(ht *lntest.HarnessTest) { + testCoopCloseWithExternalDelivery( + ht, false, false, false, + lnrpc.AddressType_UNUSED_TAPROOT_PUBKEY, + ) + }, + }, + { + Name: "set imported P2TR address (ImportTapscript) at open", + TestFunc: func(ht *lntest.HarnessTest) { + testCoopCloseWithExternalDelivery( + ht, true, true, false, + lnrpc.AddressType_UNUSED_TAPROOT_PUBKEY, + ) + }, + }, + { + Name: "set imported P2TR address (ImportTapscript) at close", + TestFunc: func(ht *lntest.HarnessTest) { + testCoopCloseWithExternalDelivery( + ht, false, true, false, + lnrpc.AddressType_UNUSED_TAPROOT_PUBKEY, + ) + }, + }, + { + Name: "set imported P2WPKH address at open", + TestFunc: func(ht *lntest.HarnessTest) { + testCoopCloseWithExternalDelivery( + ht, true, false, true, + lnrpc.AddressType_UNUSED_WITNESS_PUBKEY_HASH, + ) + }, + }, + { + Name: "set imported P2WPKH address at close", + TestFunc: func(ht *lntest.HarnessTest) { + testCoopCloseWithExternalDelivery( + ht, false, false, true, + lnrpc.AddressType_UNUSED_WITNESS_PUBKEY_HASH, + ) + }, + }, + { + Name: "set imported P2TR address (ImportPublicKey) at open", + TestFunc: func(ht *lntest.HarnessTest) { + testCoopCloseWithExternalDelivery( + ht, true, false, true, + lnrpc.AddressType_UNUSED_TAPROOT_PUBKEY, + ) + }, + }, + { + Name: "set imported P2TR address (ImportPublicKey) at close", + TestFunc: func(ht *lntest.HarnessTest) { + testCoopCloseWithExternalDelivery( + ht, false, false, true, + lnrpc.AddressType_UNUSED_TAPROOT_PUBKEY, + ) + }, + }, } -// testCoopCloseWithExternalDeliveryImpl ensures that we have a valid settled +// testCoopCloseWithExternalDelivery ensures that we have a valid settled // balance irrespective of whether the delivery address is in LND's wallet or // not. Some users set this value to be an address in a different wallet and // this should not affect our ability to accurately report the settled balance. -func testCoopCloseWithExternalDeliveryImpl(ht *lntest.HarnessTest, - upfrontShutdown bool, deliveryAddressType lnrpc.AddressType) { +// +// If importTapscript is set, it imports a Taproot script and internal key to +// Alice's LND using ImportTapscript to make sure it doesn't interfere with +// delivery address. If importPubkey is set, the address is imported using +// ImportPublicKey. +func testCoopCloseWithExternalDelivery(ht *lntest.HarnessTest, + upfrontShutdown, importTapscript, importPubkey bool, + deliveryAddressType lnrpc.AddressType) { alice := ht.NewNodeWithCoins("Alice", nil) bob := ht.NewNodeWithCoins("bob", nil) ht.ConnectNodes(alice, bob) + // Make fake taproot internal public key (equal to the final). + // This is only used if importTapscript or importPubkey is set. + taprootPubkey := [32]byte{1, 2, 3} + if upfrontShutdown { + // Make new address for second sub-test not to import + // the same address twice causing an error. + taprootPubkey[3] = 1 + } + pkScriptBytes := append( + []byte{txscript.OP_1, txscript.OP_DATA_32}, + taprootPubkey[:]..., + ) + pkScript, err := txscript.ParsePkScript(pkScriptBytes) + require.NoError(ht, err) + taprootAddress, err := pkScript.Address(harnessNetParams) + require.NoError(ht, err) + + var addr string + switch { + // Use ImportTapscript. + case importTapscript: + addr = taprootAddress.String() + + // Import the taproot address to LND. + req := &walletrpc.ImportTapscriptRequest{ + InternalPublicKey: taprootPubkey[:], + Script: &walletrpc.ImportTapscriptRequest_FullKeyOnly{ + FullKeyOnly: true, + }, + } + res := alice.RPC.ImportTapscript(req) + require.Equal(ht, addr, res.P2TrAddress) + + // Use ImportPublicKey. + case importPubkey: + var ( + address btcutil.Address + pubKey []byte + addressType walletrpc.AddressType + ) + switch deliveryAddressType { + case lnrpc.AddressType_UNUSED_WITNESS_PUBKEY_HASH: + // Make fake public key hash. + pk := [33]byte{2, 3, 4} + if upfrontShutdown { + // Make new address for second sub-test. + pk[1]++ + } + address, err = btcutil.NewAddressWitnessPubKeyHash( + btcutil.Hash160(pk[:]), harnessNetParams, + ) + require.NoError(ht, err) + pubKey = pk[:] + addressType = walletrpc.AddressType_WITNESS_PUBKEY_HASH + + case lnrpc.AddressType_UNUSED_TAPROOT_PUBKEY: + address = taprootAddress + pubKey = taprootPubkey[:] + addressType = walletrpc.AddressType_TAPROOT_PUBKEY + + default: + ht.Fatalf("not allowed address type: %v", + deliveryAddressType) + } + + addr = address.String() + + // Import the address to LND. + alice.RPC.ImportPublicKey(&walletrpc.ImportPublicKeyRequest{ + PublicKey: pubKey, + AddressType: addressType, + }) + // Here we generate a final delivery address in bob's wallet but set by // alice. We do this to ensure that the address is not in alice's LND // wallet. We already correctly track settled balances when the address // is in the LND wallet. - addr := bob.RPC.NewAddress(&lnrpc.NewAddressRequest{ - Type: deliveryAddressType, - }) + default: + res := bob.RPC.NewAddress(&lnrpc.NewAddressRequest{ + Type: deliveryAddressType, + }) + addr = res.Address + } // Prepare for channel open. openParams := lntest.OpenChannelParams{ @@ -80,7 +214,7 @@ func testCoopCloseWithExternalDeliveryImpl(ht *lntest.HarnessTest, // If we are testing the case where we set it on open then we'll set the // upfront shutdown script in the channel open parameters. if upfrontShutdown { - openParams.CloseAddress = addr.Address + openParams.CloseAddress = addr } // Open the channel! @@ -95,14 +229,14 @@ func testCoopCloseWithExternalDeliveryImpl(ht *lntest.HarnessTest, // If we are testing the case where we set the delivery address on // channel close then we will set it in the channel close parameters. if !upfrontShutdown { - closeParams.DeliveryAddress = addr.Address + closeParams.DeliveryAddress = addr } // Close the channel! closeClient := alice.RPC.CloseChannel(&closeParams) // Assert that we got a channel update when we get a closing txid. - _, err := closeClient.Recv() + _, err = closeClient.Recv() require.NoError(ht, err) // Mine the closing transaction. diff --git a/lnwallet/interface.go b/lnwallet/interface.go index 64f854631..9255c513e 100644 --- a/lnwallet/interface.go +++ b/lnwallet/interface.go @@ -660,10 +660,16 @@ func InternalKeyForAddr(wallet WalletController, netParams *chaincfg.Params, return none, nil } + // Imported addresses do not provide private keys, so they do not + // implement waddrmgr.ManagedPubKeyAddress. See RPC ImportTapscript. + if walletAddr.Imported() { + return none, nil + } + pubKeyAddr, ok := walletAddr.(waddrmgr.ManagedPubKeyAddress) if !ok { return none, fmt.Errorf("expected pubkey addr, got %T", - pubKeyAddr) + walletAddr) } _, derivationPath, _ := pubKeyAddr.DerivationInfo()