diff --git a/config_builder.go b/config_builder.go index 0f563d6f267..63f13dde84c 100644 --- a/config_builder.go +++ b/config_builder.go @@ -15,6 +15,7 @@ import ( "sync/atomic" "time" + "github.com/btcsuite/btcd/btcutil/hdkeychain" "github.com/btcsuite/btcd/chaincfg" "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/btcsuite/btcd/wire" @@ -84,6 +85,12 @@ const ( paymentMigration = 14 ) +var ( + // waddrmgrNamespaceKey is the namespace key where btcwallet stores the + // address manager state. + waddrmgrNamespaceKey = []byte("waddrmgr") +) + // GrpcRegistrar is an interface that must be satisfied by an external subserver // that wants to be able to register its own gRPC server onto lnd's main // grpc.Server instance. @@ -1554,7 +1561,9 @@ func waitForWalletPassword(cfg *Config, func importWatchOnlyAccounts(wallet *wallet.Wallet, initMsg *walletunlocker.WalletInitMsg) error { - scopes := make([]waddrmgr.ScopedIndex, 0, len(initMsg.WatchOnlyAccounts)) + scopes := make( + []waddrmgr.ScopedIndex, 0, len(initMsg.WatchOnlyAccounts), + ) for scope := range initMsg.WatchOnlyAccounts { scopes = append(scopes, scope) } @@ -1587,12 +1596,14 @@ func importWatchOnlyAccounts(wallet *wallet.Wallet, name = "default" } - _, err := wallet.ImportAccountWithScope( - name, initMsg.WatchOnlyAccounts[scope], - initMsg.WatchOnlyMasterFingerprint, scope.Scope, - addrSchema, + err := importWatchOnlyAccount( + wallet, scope, name, initMsg.WatchOnlyAccounts[scope], + initMsg.WatchOnlyMasterFingerprint, addrSchema, ) if err != nil { + // Each account import runs in its own db transaction + // (as with the previous import path), so an error here + // does not roll back accounts imported before. return fmt.Errorf("could not import account %v: %w", name, err) } @@ -1601,6 +1612,120 @@ func importWatchOnlyAccounts(wallet *wallet.Wallet, return nil } +// importWatchOnlyAccount imports a single watch-only account. +// +// Account index 0 uses ImportAccountWithScope, which allocates the next +// sequential account number internally. This preserves the "default" account +// semantics and naming that the rest of lnd expects for on-chain operations. +// +// Non-zero accounts use NewRawAccountWatchingOnly to create the account at an +// explicit account number matching the key family index. This is critical for +// remote-signer setups where DeriveKey(KeyFamily=N) must resolve to btcwallet +// account N. Without explicit placement, btcwallet would assign contiguous +// numbers (1, 2, 3, ...) regardless of the intended key family, breaking +// sparse families like N=43210. +// +// After NewRawAccountWatchingOnly creates the account, it has the synthetic +// name "act:". We rename it to lnd's human-readable format +// (e.g. "1017'/1'/43210'") so the account appears correctly in RPCs and UI. +// +// NOTE: Each account import runs in its own walletdb transaction. For the +// explicit (non-zero) path, both the account creation and rename happen in the +// same transaction: if any step fails, that account is rolled back atomically. +// However, accounts imported before a failure are already committed. +func importWatchOnlyAccount(wallet *wallet.Wallet, scope waddrmgr.ScopedIndex, + name string, accountPubKey *hdkeychain.ExtendedKey, + masterKeyFingerprint uint32, + addrSchema waddrmgr.ScopeAddrSchema) error { + + if scope.Index == 0 { + _, err := wallet.ImportAccountWithScope( + name, accountPubKey, masterKeyFingerprint, scope.Scope, + addrSchema, + ) + + return err + } + + // Validate the account xpub using btcwallet's own dry-run import + // path. This avoids duplicating btcwallet's internal validation + // (validateExtendedPubKey / isPubKeyForNet) while keeping the same + // rejection semantics as ImportAccountWithScope. The dry-run rolls + // back its database transaction, so no persistent state is created. + // + // We pass WitnessPubKey as the address type because custom-scope + // keys use standard version bytes (tpub/xpub) that require an + // explicit type for btcwallet's scope detection; the actual scope + // is handled by our own walletdb transaction below. + addrType := waddrmgr.WitnessPubKey + _, _, _, err := wallet.ImportAccountDryRun( + name, accountPubKey, masterKeyFingerprint, &addrType, 1, + ) + if err != nil { + return err + } + + db := wallet.Database() + + return walletdb.Update(db, func(tx walletdb.ReadWriteTx) error { + addrmgrNs := tx.ReadWriteBucket(waddrmgrNamespaceKey) + if addrmgrNs == nil { + return fmt.Errorf("waddrmgr namespace not found") + } + + // Mirror what ImportAccountWithScope in btcwallet itself does: + // try to find an already-registered manager for this key scope. + // If the scope was already set up, we reuse it. Otherwise we + // create the scope in the DB and register the in-memory + // manager. + scopedMgr, err := wallet.Manager.FetchScopedKeyManager( + scope.Scope, + ) + if err != nil { + var manErr waddrmgr.ManagerError + if !errors.As(err, &manErr) || + manErr.ErrorCode != waddrmgr.ErrScopeNotFound { + + return err + } + + scopedMgr, err = wallet.Manager.NewScopedKeyManager( + addrmgrNs, scope.Scope, addrSchema, + ) + if err != nil { + return err + } + } + + // Ensure we don't accidentally overwrite an existing account. + _, err = scopedMgr.AccountName(addrmgrNs, scope.Index) + if err == nil { + return fmt.Errorf("account %d already exists in "+ + "scope %v", scope.Index, scope.Scope) + } + + var manErr waddrmgr.ManagerError + if !errors.As(err, &manErr) || + manErr.ErrorCode != waddrmgr.ErrAccountNotFound { + + return err + } + + err = scopedMgr.NewRawAccountWatchingOnly( + addrmgrNs, scope.Index, accountPubKey, + masterKeyFingerprint, &addrSchema, + ) + if err != nil { + return err + } + + // NewRawAccountWatchingOnly assigns the synthetic account name + // "act:". Rename it to lnd's expected human-readable + // account name to preserve existing RPC/UI naming behavior. + return scopedMgr.RenameAccount(addrmgrNs, scope.Index, name) + }) +} + // handleNeutrinoPostgresDBMigration handles the migration of the neutrino db // to postgres. Initially we kept the neutrino db in the bolt db when running // with kvdb postgres backend. Now now move it to postgres as well. However we diff --git a/config_builder_watchonly_test.go b/config_builder_watchonly_test.go new file mode 100644 index 00000000000..e7a2759bf23 --- /dev/null +++ b/config_builder_watchonly_test.go @@ -0,0 +1,174 @@ +package lnd + +import ( + "errors" + "fmt" + "testing" + "time" + + "github.com/btcsuite/btcd/btcutil/hdkeychain" + "github.com/btcsuite/btcd/chaincfg" + "github.com/btcsuite/btcwallet/waddrmgr" + "github.com/btcsuite/btcwallet/wallet" + "github.com/btcsuite/btcwallet/walletdb" + "github.com/lightningnetwork/lnd/keychain" + "github.com/lightningnetwork/lnd/kvdb" + "github.com/lightningnetwork/lnd/walletunlocker" + "github.com/stretchr/testify/require" +) + +// TestImportWatchOnlyAccountsPreservesSparseAccountNumbers ensures watch-only +// initialization preserves explicit sparse account indices so deriving keys by +// KeyFamily uses the intended internal account number. +func TestImportWatchOnlyAccountsPreservesSparseAccountNumbers(t *testing.T) { + const ( + pubPass = "public-pass" + sparseAccount = uint32(43210) + masterFingerprint = uint32(0x12345678) + ) + + loader := wallet.NewLoader( + &chaincfg.TestNet3Params, t.TempDir(), true, + kvdb.DefaultDBTimeout, 0, + ) + testWallet, err := loader.CreateNewWatchingOnlyWallet( + []byte(pubPass), time.Unix(0, 0), + ) + require.NoError(t, err) + + t.Cleanup(func() { + require.NoError(t, loader.UnloadWallet()) + }) + + rootKey, err := hdkeychain.NewMaster( + []byte("01234567890123456789012345678901"), + &chaincfg.TestNet3Params, + ) + require.NoError(t, err) + + bip84AccountXpub, err := deriveAccountXpub( + rootKey, waddrmgr.KeyScopeBIP0084.Purpose, + waddrmgr.KeyScopeBIP0084.Coin, 0, + ) + require.NoError(t, err) + + customScope := waddrmgr.KeyScope{ + Purpose: keychain.BIP0043Purpose, + Coin: keychain.CoinTypeTestnet, + } + customAccountXpub, err := deriveAccountXpub( + rootKey, customScope.Purpose, customScope.Coin, sparseAccount, + ) + require.NoError(t, err) + + watchOnlyAccounts := map[waddrmgr.ScopedIndex]*hdkeychain.ExtendedKey{ + { + Scope: waddrmgr.KeyScopeBIP0084, + Index: 0, + }: bip84AccountXpub, + { + Scope: customScope, + Index: sparseAccount, + }: customAccountXpub, + } + initMsg := &walletunlocker.WalletInitMsg{ + WatchOnlyMasterFingerprint: masterFingerprint, + WatchOnlyAccounts: watchOnlyAccounts, + } + require.NoError(t, importWatchOnlyAccounts(testWallet, initMsg)) + + // This is the same call path WalletKit.DeriveKey uses for watch-only + // wallets: no account auto-creation, account number must already exist. + keyRing := keychain.NewBtcWalletKeyRing( + testWallet, keychain.CoinTypeTestnet, + ) + keyDesc, err := keyRing.DeriveKey(keychain.KeyLocator{ + Family: keychain.KeyFamily(sparseAccount), + Index: 0, + }) + require.NoError(t, err) + require.NotNil(t, keyDesc.PubKey) + + db := testWallet.Database() + + err = walletdb.View(db, func(tx walletdb.ReadTx) error { + addrmgrNs := tx.ReadBucket(waddrmgrNamespaceKey) + if addrmgrNs == nil { + return fmt.Errorf("waddrmgr namespace not found") + } + + // Ensure the sparse account number was preserved. + customMgr, err := testWallet.Manager.FetchScopedKeyManager( + customScope, + ) + if err != nil { + return err + } + + name, err := customMgr.AccountName(addrmgrNs, sparseAccount) + if err != nil { + return err + } + expectedName := fmt.Sprintf( + "%s/%d'", customScope.String(), sparseAccount, + ) + if name != expectedName { + return fmt.Errorf("expected account %d name %q, got %q", + sparseAccount, expectedName, name) + } + + // The old behavior would have created this account + // contiguously. + _, err = customMgr.AccountName(addrmgrNs, 1) + var managerErr waddrmgr.ManagerError + if !errors.As(err, &managerErr) || + managerErr.ErrorCode != waddrmgr.ErrAccountNotFound { + + return fmt.Errorf("expected account 1 to be missing, "+ + "got: %v", err) + } + + // Ensure account 0 in the default BIP84 scope keeps its + // "default" naming behavior. + bip84Mgr, err := testWallet.Manager.FetchScopedKeyManager( + waddrmgr.KeyScopeBIP0084, + ) + if err != nil { + return err + } + defaultName, err := bip84Mgr.AccountName(addrmgrNs, 0) + if err != nil { + return err + } + if defaultName != "default" { + return fmt.Errorf("expected account 0 name default, "+ + "got %q", defaultName) + } + + return nil + }) + require.NoError(t, err) +} + +// deriveAccountXpub derives and neuters an account-level key at +// m/purpose'/coin_type'/account'. +func deriveAccountXpub(rootKey *hdkeychain.ExtendedKey, purpose, coinType, + account uint32) (*hdkeychain.ExtendedKey, error) { + + path := []uint32{ + purpose + hdkeychain.HardenedKeyStart, + coinType + hdkeychain.HardenedKeyStart, + account + hdkeychain.HardenedKeyStart, + } + + current := rootKey + var err error + for _, pathPart := range path { + current, err = current.Derive(pathPart) + if err != nil { + return nil, err + } + } + + return current.Neuter() +} diff --git a/docs/release-notes/release-notes-0.21.0.md b/docs/release-notes/release-notes-0.21.0.md index ad58f77da7f..3a10f14a866 100644 --- a/docs/release-notes/release-notes-0.21.0.md +++ b/docs/release-notes/release-notes-0.21.0.md @@ -70,6 +70,15 @@ the chain watch filter on restart. This was a pre-existing bug since private taproot channels were first introduced. +- Watch-only wallet account imports + [now preserve](https://github.com/lightningnetwork/lnd/pull/10644) + explicit non-zero account numbers per scope. This fixes remote-signer setups + where `DeriveKey(KeyFamily=N)` failed for sparse custom key families (e.g. + `N=43210`) because accounts were previously remapped contiguously. Account + numbers are written to the wallet database at creation time and are not + re-imported on unlock, so downgrading after wallet creation does not remap + them. + # New Features - [Basic Support](https://github.com/lightningnetwork/lnd/pull/9868) for onion diff --git a/docs/remote-signing.md b/docs/remote-signing.md index 0f06463b3f2..cacef27c2e8 100644 --- a/docs/remote-signing.md +++ b/docs/remote-signing.md @@ -190,9 +190,19 @@ watching-only`): - Purpose: 49, coin type 0, account 0 (`m/49'/0'/0'`, np2wkh) - Purpose: 84, coin type 0, account 0 (`m/84'/0'/0'`, p2wkh) - Purpose: 86, coin type 0, account 0 (`m/86'/0'/0'`, p2tr) -- Purpose: 1017, coin type X (mainnet: 0, testnet/regtest: 1), account 0 to 255 - (`m/1017'/X'/0'` up to `m/1017'/X'/255'`, internal to `lnd`, used for node - identity, channel keys, watchtower sessions and so on). +- Purpose: 1017, coin type X (mainnet: 0, testnet/regtest: 1), account 0 to + 255 (`m/1017'/X'/0'` up to `m/1017'/X'/255'`, internal to `lnd`, used for + node identity, channel keys, watchtower sessions and so on). +- Any additional custom `m/1017'/X'/N'` accounts that your signer actively + uses (for example `N=43210`). Starting with `lnd v0.21.0`, these explicit + account numbers are preserved in watch-only wallets, so + `DeriveKey(KeyFamily=N)` works without requiring contiguous account creation. + +**Note on older versions:** `lnd` versions before `v0.21.0` import watch-only +accounts contiguously, so a watch-only wallet *created* on an older version +will have sparse accounts remapped (e.g. account 43210 becomes account 1). +Downgrading after wallet creation is safe: accounts are already persisted at +the correct indices and are not re-imported on unlock. ## Example initialization script diff --git a/go.mod b/go.mod index 3aae8ace11f..afae2d077c9 100644 --- a/go.mod +++ b/go.mod @@ -230,3 +230,6 @@ replace google.golang.org/protobuf => github.com/lightninglabs/protobuf-go-hex-d go 1.25.5 retract v0.0.2 + +// TODO(Boris): remove this when https://github.com/btcsuite/btcwallet/pull/1207 is merged +replace github.com/btcsuite/btcwallet => github.com/starius/btcwallet v0.16.17-fix-side-effects diff --git a/go.sum b/go.sum index e99d976a411..7b207d80248 100644 --- a/go.sum +++ b/go.sum @@ -67,8 +67,6 @@ github.com/btcsuite/btclog v1.0.0/go.mod h1:w7xnGOhwT3lmrS4H3b/D1XAXxvh+tbhUm8xe github.com/btcsuite/btclog/v2 v2.0.1-0.20250728225537-6090e87c6c5b h1:MQ+Q6sDy37V1wP1Yu79A5KqJutolqUGwA99UZWQDWZM= github.com/btcsuite/btclog/v2 v2.0.1-0.20250728225537-6090e87c6c5b/go.mod h1:XItGUfVOxotJL8kkuk2Hj3EVow5KCugXl3wWfQ6K0AE= github.com/btcsuite/btcutil v0.0.0-20190425235716-9e5f4b9a998d/go.mod h1:+5NJ2+qvTyV9exUAL/rxXi3DcLg2Ts+ymUAY5y4NvMg= -github.com/btcsuite/btcwallet v0.16.17-0.20260213031108-70a94ea39e9c h1:XJN7vcvFiclslw5Zixe9t4YRprTpADcQlBf2IN3nbW8= -github.com/btcsuite/btcwallet v0.16.17-0.20260213031108-70a94ea39e9c/go.mod h1:4TTru0cgIPbCZpY4aRfAVwX87zrQw4GXM8MH6+A5xZw= github.com/btcsuite/btcwallet/wallet/txauthor v1.3.5 h1:Rr0njWI3r341nhSPesKQ2JF+ugDSzdPoeckS75SeDZk= github.com/btcsuite/btcwallet/wallet/txauthor v1.3.5/go.mod h1:+tXJ3Ym0nlQc/iHSwW1qzjmPs3ev+UVWMbGgfV1OZqU= github.com/btcsuite/btcwallet/wallet/txrules v1.2.2 h1:YEO+Lx1ZJJAtdRrjuhXjWrYsmAk26wLTlNzxt2q0lhk= @@ -505,6 +503,8 @@ github.com/soheilhy/cmux v0.1.5 h1:jjzc5WVemNEDTLwv9tlmemhC73tI08BNOIGwBOo10Js= github.com/soheilhy/cmux v0.1.5/go.mod h1:T7TcVDs9LWfQgPlPsdngu6I6QIoyIFZDDC6sNE1GqG0= github.com/spf13/pflag v1.0.6 h1:jFzHGLGAlb3ruxLB8MhbI6A8+AQX/2eW4qeyNZXNp2o= github.com/spf13/pflag v1.0.6/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= +github.com/starius/btcwallet v0.16.17-fix-side-effects h1:gax4Z6o2ePMlNBWdPR0IqMMjhFFOHnFWkO+4kgraAFA= +github.com/starius/btcwallet v0.16.17-fix-side-effects/go.mod h1:4TTru0cgIPbCZpY4aRfAVwX87zrQw4GXM8MH6+A5xZw= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.2.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoHMkEqE= diff --git a/itest/lnd_remote_signer_test.go b/itest/lnd_remote_signer_test.go index eac22828b5e..426c19bc005 100644 --- a/itest/lnd_remote_signer_test.go +++ b/itest/lnd_remote_signer_test.go @@ -27,6 +27,10 @@ var remoteSignerTestCases = []*lntest.TestCase{ Name: "account import", TestFunc: testRemoteSignerAccountImport, }, + { + Name: "sparse account import", + TestFunc: testRemoteSignerSparseAccountImport, + }, { Name: "tapscript import", TestFunc: testRemoteSignerTapscriptImport, @@ -116,6 +120,7 @@ type remoteSignerTestCase struct { randomSeed bool sendCoins bool commitType lnrpc.CommitmentType + accounts []*lnrpc.WatchOnlyAccount fn func(tt *lntest.HarnessTest, wo, carol *node.HarnessNode) } @@ -135,6 +140,10 @@ func prepareRemoteSignerTest(ht *lntest.HarnessTest, tc remoteSignerTestCase) ( signer *node.HarnessNode err error ) + if len(tc.accounts) > 0 { + watchOnlyAccounts = tc.accounts + } + if !tc.randomSeed { signer = ht.RestoreNodeWithSeed( "Signer", nil, password, nil, rootKey, 0, nil, @@ -232,6 +241,35 @@ func testRemoteSignerAccountImport(ht *lntest.HarnessTest) { tc.fn(ht, watchOnly, carol) } +// testRemoteSignerSparseAccountImport verifies that a sparse custom key family +// account is preserved when initializing a watch-only wallet for remote signer +// mode, so DeriveKey on the watch-only node matches the signer node. +func testRemoteSignerSparseAccountImport(ht *lntest.HarnessTest) { + const sparseAccount = 43210 + + tc := remoteSignerTestCase{ + name: "sparse account import", + accounts: deriveCustomScopeAccounts(ht.T, sparseAccount), + } + + signer, watchOnly, _ := prepareRemoteSignerTest(ht, tc) + + keyLoc := &signrpc.KeyLocator{ + KeyFamily: sparseAccount, + KeyIndex: 0, + } + signerKey := signer.RPC.DeriveKey(keyLoc) + watchOnlyKey := watchOnly.RPC.DeriveKey(keyLoc) + + require.Equal( + ht, signerKey.KeyLoc.KeyFamily, watchOnlyKey.KeyLoc.KeyFamily, + ) + require.Equal( + ht, signerKey.KeyLoc.KeyIndex, watchOnlyKey.KeyLoc.KeyIndex, + ) + require.Equal(ht, signerKey.RawKeyBytes, watchOnlyKey.RawKeyBytes) +} + func testRemoteSignerTapscriptImport(ht *lntest.HarnessTest) { tc := remoteSignerTestCase{ name: "tapscript import", @@ -421,10 +459,14 @@ func testRemoteSignerTaproot(ht *lntest.HarnessTest) { tc.fn(ht, watchOnly, carol) } -// deriveCustomScopeAccounts derives the first 255 default accounts of the custom lnd -// internal key scope. -func deriveCustomScopeAccounts(t *testing.T) []*lnrpc.WatchOnlyAccount { - allAccounts := make([]*lnrpc.WatchOnlyAccount, 0, 255+len(accounts)) +// deriveCustomScopeAccounts derives the first 255 default accounts of the +// custom lnd internal key scope. Additional account indices can be passed in to +// include sparse accounts as needed by specific tests. +func deriveCustomScopeAccounts(t *testing.T, + extraAccounts ...uint32) []*lnrpc.WatchOnlyAccount { + + maxCap := 255 + len(accounts) + len(extraAccounts) + allAccounts := make([]*lnrpc.WatchOnlyAccount, 0, maxCap) allAccounts = append(allAccounts, accounts...) extendedRootKey, err := hdkeychain.NewKeyFromString(rootKey) @@ -436,6 +478,8 @@ func deriveCustomScopeAccounts(t *testing.T) []*lnrpc.WatchOnlyAccount { } coinTypeKey, err := derivePath(extendedRootKey, path) require.NoError(t, err) + + // Add default accounts. for idx := uint32(0); idx <= 255; idx++ { accountPath := []uint32{idx + hdkeychain.HardenedKeyStart} accountKey, err := derivePath(coinTypeKey, accountPath) @@ -452,6 +496,23 @@ func deriveCustomScopeAccounts(t *testing.T) []*lnrpc.WatchOnlyAccount { }) } + // Add sparse accounts. + for _, idx := range extraAccounts { + accountPath := []uint32{idx + hdkeychain.HardenedKeyStart} + accountKey, err := derivePath(coinTypeKey, accountPath) + require.NoError(t, err) + + accountXPub, err := accountKey.Neuter() + require.NoError(t, err) + + allAccounts = append(allAccounts, &lnrpc.WatchOnlyAccount{ + Purpose: keychain.BIP0043Purpose, + CoinType: harnessNetParams.HDCoinType, + Account: idx, + Xpub: accountXPub.String(), + }) + } + return allAccounts }