From 3a1ec1cffcb0ed0b3560dac73e8c856f166ed3c8 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Fri, 17 Apr 2026 20:41:28 -0500 Subject: [PATCH 1/2] lndclient: attach key locator when any component is non-zero MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SignOutputRawKeyLocator was introduced to propagate the key locator alongside the raw public key, but its marshaller required both the family and the index to be non-zero before attaching the KeyLoc. That silently dropped the locator for valid keys whose slot has a zero component — most visibly LND's node identity key at family 6 / index 0, but any first-index key in a non-zero family is affected. Attach the locator whenever either component is non-zero, and keep omitting it only for the all-zero KeyLocator, which is the struct's zero value and therefore indistinguishable from "locator not set" at call sites. This lets callers that sign against the node identity key path, or any first-index key in a restored-from-seed wallet, resolve through the fully populated descriptor instead of falling back to pubkey scanning. --- signer_client.go | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/signer_client.go b/signer_client.go index 0a605b4..f401909 100644 --- a/signer_client.go +++ b/signer_client.go @@ -271,19 +271,24 @@ func marshallSignDescriptors(signDescriptors []*SignDescriptor, //nolint:lll // fullDescriptor is a helper method that creates a fully populated sign // descriptor that includes both the public key and the key locator (if - // available). For the locator we explicitly check that both the family - // _and_ the index is non-zero. In some applications it's possible that - // the family is always set (because only a specific family is used), - // but the index might be zero because it's the first key, or because it - // isn't known at that particular moment. + // available). The locator is attached whenever either the family _or_ + // the index is non-zero; only the all-zero KeyLocator (which is the + // struct's zero value and therefore indistinguishable from "unset") is + // omitted. A prior version of this helper required both components to + // be non-zero, which silently dropped the locator for valid keys whose + // slot has a zero component — most notably LND's node identity key at + // KeyFamilyNodeKey (6) / index 0. Signers that must re-derive the key + // from its locator (restored-from-seed wallets, or identity-key paths + // where only the family is meaningful) could not resolve those keys. // We aim to be compatible with this method in lnd's wallet: // https://github.com/lightningnetwork/lnd/blob/master/lnwallet/btcwallet/signer.go#L286 - // Because we know all custom families (0 to 255) are derived at wallet - // creation, and the very first index of each family/account is always - // derived, we know that only using the public key for that very first - // index will work. But for a freshly initialized wallet (e.g. restored - // from seed), we won't know any indexes greater than 0, so we _need_ to - // also specify the key locator and not just the public key. + // The very first index of each custom family is always derived at + // wallet creation, so for callers that know the pubkey the backend can + // resolve the key without a locator. But wallets restored from seed + // won't have pre-derived indexes greater than 0, and keys pinned to a + // fixed family slot (like the node identity key) must have the locator + // forwarded so the signer can re-derive them — hence we forward a + // locator whenever the caller has populated even one component. fullDescriptor := func( d keychain.KeyDescriptor) *signrpc.KeyDescriptor { @@ -292,7 +297,7 @@ func marshallSignDescriptors(signDescriptors []*SignDescriptor, keyDesc.RawKeyBytes = d.PubKey.SerializeCompressed() } - if d.KeyLocator.Family != 0 && d.KeyLocator.Index != 0 { + if d.KeyLocator.Family != 0 || d.KeyLocator.Index != 0 { keyDesc.KeyLoc = &signrpc.KeyLocator{ KeyFamily: int32(d.KeyLocator.Family), KeyIndex: int32(d.KeyLocator.Index), From b5f542c4da6d2699f57344021029d7114fbd1bf3 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Fri, 17 Apr 2026 20:43:04 -0500 Subject: [PATCH 2/2] lndclient: add marshallSignDescriptors locator-attach tests Pin the contract that SignOutputRawKeyLocator's marshaller attaches the key locator whenever either the family or the index is non-zero and omits it only for the all-zero KeyLocator. The family-only case (KeyFamilyNodeKey / index 0) is the specific regression guard for the previously "both components non-zero" predicate that silently dropped the locator for LND's node identity key slot. Also cover the legacy partial descriptor path used by SignOutputRaw to pin that it still populates either the public key or the locator but never both, matching the behavior Loop and other downstream callers adjusted themselves to. --- signer_client_test.go | 203 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 203 insertions(+) create mode 100644 signer_client_test.go diff --git a/signer_client_test.go b/signer_client_test.go new file mode 100644 index 0000000..5af4d2e --- /dev/null +++ b/signer_client_test.go @@ -0,0 +1,203 @@ +package lndclient + +import ( + "testing" + + "github.com/btcsuite/btcd/btcec/v2" + "github.com/btcsuite/btcd/wire" + "github.com/lightningnetwork/lnd/keychain" + "github.com/stretchr/testify/require" +) + +// newTestPubKey returns a fresh public key for use in descriptor +// marshalling tests. +func newTestPubKey(t *testing.T) *btcec.PublicKey { + t.Helper() + + priv, err := btcec.NewPrivateKey() + require.NoError(t, err) + + return priv.PubKey() +} + +// TestMarshallSignDescriptorsFullLocator pins the contract of the full +// descriptor marshaller used by SignOutputRawKeyLocator: the key locator +// is attached whenever either the family or the index is non-zero, and +// only the all-zero KeyLocator (the struct's zero value, indistinguishable +// from "unset") is omitted. +// +// The family-6 / index-0 case is the regression guard. A prior version of +// the helper required both components to be non-zero and therefore +// silently dropped the locator for LND's node identity key slot. Signers +// that must re-derive the key from its locator — restored-from-seed +// wallets, or family-pinned identity keys — could not resolve the key. +func TestMarshallSignDescriptorsFullLocator(t *testing.T) { + t.Parallel() + + pubKey := newTestPubKey(t) + output := &wire.TxOut{PkScript: []byte{0x51}, Value: 1000} + + tests := []struct { + name string + family keychain.KeyFamily + index uint32 + omitPubKey bool + expectLocator bool + }{ + { + name: "all-zero locator is omitted", + family: 0, + index: 0, + expectLocator: false, + }, + { + name: "family-only locator is attached", + family: keychain.KeyFamilyNodeKey, + index: 0, + expectLocator: true, + }, + { + name: "index-only locator is attached", + family: 0, + index: 5, + expectLocator: true, + }, + { + name: "fully populated locator is attached", + family: keychain.KeyFamilyNodeKey, + index: 5, + expectLocator: true, + }, + { + name: "family-only locator is attached with " + + "nil pubkey", + family: keychain.KeyFamilyNodeKey, + index: 0, + omitPubKey: true, + expectLocator: true, + }, + { + name: "index-only locator is attached with " + + "nil pubkey", + family: 0, + index: 5, + omitPubKey: true, + expectLocator: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + keyDesc := keychain.KeyDescriptor{ + KeyLocator: keychain.KeyLocator{ + Family: tc.family, + Index: tc.index, + }, + } + if !tc.omitPubKey { + keyDesc.PubKey = pubKey + } + + descs := []*SignDescriptor{{ + KeyDesc: keyDesc, + Output: output, + }} + + rpcDescs := marshallSignDescriptors(descs, true) + require.Len(t, rpcDescs, 1) + + rpcKeyDesc := rpcDescs[0].KeyDesc + require.NotNil(t, rpcKeyDesc) + + if tc.omitPubKey { + require.Empty(t, rpcKeyDesc.RawKeyBytes) + } else { + require.Equal(t, + pubKey.SerializeCompressed(), + rpcKeyDesc.RawKeyBytes, + ) + } + + if !tc.expectLocator { + require.Nil(t, rpcKeyDesc.KeyLoc) + return + } + + require.NotNil(t, rpcKeyDesc.KeyLoc) + require.Equal(t, + int32(tc.family), + rpcKeyDesc.KeyLoc.KeyFamily, + ) + require.Equal(t, + int32(tc.index), + rpcKeyDesc.KeyLoc.KeyIndex, + ) + }) + } +} + +// TestMarshallSignDescriptorsPartialOnlyOneOf verifies that the partial +// descriptor marshaller used by the legacy SignOutputRaw populates either +// the public key or the locator, but never both. Applications like Loop +// have adjusted themselves to this behavior, which is why the fix for the +// locator-drop bug lives in the full descriptor path instead. +func TestMarshallSignDescriptorsPartialOnlyOneOf(t *testing.T) { + t.Parallel() + + pubKey := newTestPubKey(t) + output := &wire.TxOut{PkScript: []byte{0x51}, Value: 1000} + + t.Run("pubkey present drops locator", func(t *testing.T) { + t.Parallel() + + descs := []*SignDescriptor{{ + KeyDesc: keychain.KeyDescriptor{ + PubKey: pubKey, + KeyLocator: keychain.KeyLocator{ + Family: keychain.KeyFamilyNodeKey, + Index: 5, + }, + }, + Output: output, + }} + + rpcDescs := marshallSignDescriptors(descs, false) + require.Len(t, rpcDescs, 1) + + require.Equal(t, + pubKey.SerializeCompressed(), + rpcDescs[0].KeyDesc.RawKeyBytes, + ) + require.Nil(t, rpcDescs[0].KeyDesc.KeyLoc) + }) + + t.Run("pubkey absent uses locator", func(t *testing.T) { + t.Parallel() + + descs := []*SignDescriptor{{ + KeyDesc: keychain.KeyDescriptor{ + KeyLocator: keychain.KeyLocator{ + Family: keychain.KeyFamilyNodeKey, + Index: 5, + }, + }, + Output: output, + }} + + rpcDescs := marshallSignDescriptors(descs, false) + require.Len(t, rpcDescs, 1) + + require.Empty(t, rpcDescs[0].KeyDesc.RawKeyBytes) + require.NotNil(t, rpcDescs[0].KeyDesc.KeyLoc) + require.Equal(t, + int32(keychain.KeyFamilyNodeKey), + rpcDescs[0].KeyDesc.KeyLoc.KeyFamily, + ) + require.Equal(t, + int32(5), + rpcDescs[0].KeyDesc.KeyLoc.KeyIndex, + ) + }) +}