From df11b0fed040c68ac4fea8837ccbfcd220652e0a Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Fri, 13 Aug 2021 16:40:35 -0700 Subject: [PATCH 1/4] keychain: add basic benchmark for privkey derivation --- keychain/bench_test.go | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 keychain/bench_test.go diff --git a/keychain/bench_test.go b/keychain/bench_test.go new file mode 100644 index 000000000..5c04eac51 --- /dev/null +++ b/keychain/bench_test.go @@ -0,0 +1,41 @@ +package keychain + +import ( + "testing" + + "github.com/btcsuite/btcd/btcec" + "github.com/stretchr/testify/require" +) + +func BenchmarkDerivePrivKey(t *testing.B) { + cleanUp, wallet, err := createTestBtcWallet( + CoinTypeBitcoin, + ) + if err != nil { + t.Fatalf("unable to create wallet: %v", err) + } + + keyRing := NewBtcWalletKeyRing(wallet, CoinTypeBitcoin) + + defer cleanUp() + + var ( + privKey *btcec.PrivateKey + ) + + keyDesc := KeyDescriptor{ + KeyLocator: KeyLocator{ + Family: KeyFamilyMultiSig, + Index: 1, + }, + } + + t.ReportAllocs() + t.ResetTimer() + + for i := 0; i < t.N; i++ { + privKey, err = keyRing.DerivePrivKey(keyDesc) + } + require.NoError(t, err) + require.NotNil(t, privKey) +} From 2206eba91a074b9d693b569daa7058c06ed0ea8c Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Fri, 13 Aug 2021 16:29:57 -0700 Subject: [PATCH 2/4] build: point to latest btcwallet w/ new cached privkey method --- go.mod | 2 +- go.sum | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index 691107c67..47bebf9a6 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f github.com/btcsuite/btcutil v1.0.3-0.20210527170813-e2ba6805a890 github.com/btcsuite/btcutil/psbt v1.0.3-0.20210527170813-e2ba6805a890 - github.com/btcsuite/btcwallet v0.12.1-0.20210822222949-9b5a201c344c + github.com/btcsuite/btcwallet v0.12.1-0.20210826004415-4ef582f76b02 github.com/btcsuite/btcwallet/wallet/txauthor v1.0.2-0.20210803004036-eebed51155ec github.com/btcsuite/btcwallet/wallet/txrules v1.0.0 github.com/btcsuite/btcwallet/walletdb v1.3.6-0.20210803004036-eebed51155ec diff --git a/go.sum b/go.sum index 86d525ec2..c846d7858 100644 --- a/go.sum +++ b/go.sum @@ -84,8 +84,8 @@ github.com/btcsuite/btcutil v1.0.3-0.20210527170813-e2ba6805a890/go.mod h1:0DVlH github.com/btcsuite/btcutil/psbt v1.0.3-0.20201208143702-a53e38424cce/go.mod h1:LVveMu4VaNSkIRTZu2+ut0HDBRuYjqGocxDMNS1KuGQ= github.com/btcsuite/btcutil/psbt v1.0.3-0.20210527170813-e2ba6805a890 h1:0xUNvvwJ7RjzBs4nCF+YrK28S5P/b4uHkpPxY1ovGY4= github.com/btcsuite/btcutil/psbt v1.0.3-0.20210527170813-e2ba6805a890/go.mod h1:LVveMu4VaNSkIRTZu2+ut0HDBRuYjqGocxDMNS1KuGQ= -github.com/btcsuite/btcwallet v0.12.1-0.20210822222949-9b5a201c344c h1:lOUYaSw0aqCHgLk+hA2QSYXhquRXdAvT6rB3sJMXI8w= -github.com/btcsuite/btcwallet v0.12.1-0.20210822222949-9b5a201c344c/go.mod h1:SdqXKJoEEi5LJq6zU67PcKiyqF97AcUOfBfyQHC7rqQ= +github.com/btcsuite/btcwallet v0.12.1-0.20210826004415-4ef582f76b02 h1:Q8Scm1SXNRyiXzD3a7O1C6bLFIUxUQSnWAd9aat8Xd0= +github.com/btcsuite/btcwallet v0.12.1-0.20210826004415-4ef582f76b02/go.mod h1:SdqXKJoEEi5LJq6zU67PcKiyqF97AcUOfBfyQHC7rqQ= github.com/btcsuite/btcwallet/wallet/txauthor v1.0.0/go.mod h1:VufDts7bd/zs3GV13f/lXc/0lXrPnvxD/NvmpG/FEKU= github.com/btcsuite/btcwallet/wallet/txauthor v1.0.1-0.20210329233242-e0607006dce6/go.mod h1:VufDts7bd/zs3GV13f/lXc/0lXrPnvxD/NvmpG/FEKU= github.com/btcsuite/btcwallet/wallet/txauthor v1.0.2-0.20210803004036-eebed51155ec h1:nuO8goa4gbgDM4iegCztF7mTq8io9NT1DAMoPrEI6S4= @@ -710,7 +710,6 @@ golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fq golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= -golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.5 h1:i6eZZ+zk0SOf0xgBpEpPD18qWcJda6q1sxt3S0kzyUQ= golang.org/x/text v0.3.5/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= From d6524ea517b5b3905908546777edcf140d69a4e3 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Fri, 13 Aug 2021 16:30:13 -0700 Subject: [PATCH 3/4] keychain+lnwallet: when fetching priv keys or signing try to use cache In this commit, we start to optimistically use the new private key cache that was added to btcwallet. As is, btcwallet will cache the decrypted account keys for each scope in memory. However, the existing methods to derive a child key from those account keys requires a write database transaction, and will re-derive the private key using BIP-32 each time. The newly added `DeriveFromKeyPathCache` lets us skip all this and directly use a cache assuming the account info is already cached. The new logic will try to use this method, but if it fails fall back to the existing `DeriveFromKeyPath` method. All calls after this will use this new cached key. Fixes https://github.com/lightningnetwork/lnd/issues/5125. Basic benchmark before the btcwallet change and after: ``` benchmark old ns/op new ns/op delta BenchmarkDerivePrivKey-8 22840583 125 -100.00% benchmark old allocs new allocs delta BenchmarkDerivePrivKey-8 89 2 -97.75% benchmark old bytes new bytes delta BenchmarkDerivePrivKey-8 10225 24 -99.77% ``` --- keychain/btcwallet.go | 28 ++++++++++++++++++++++------ lnwallet/btcwallet/signer.go | 14 ++++++++++++++ 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/keychain/btcwallet.go b/keychain/btcwallet.go index 02b721399..8d5dab3bd 100644 --- a/keychain/btcwallet.go +++ b/keychain/btcwallet.go @@ -252,14 +252,30 @@ func (b *BtcWalletKeyRing) DerivePrivKey(keyDesc KeyDescriptor) ( var key *btcec.PrivateKey - db := b.wallet.Database() - err := walletdb.Update(db, func(tx walletdb.ReadWriteTx) error { - addrmgrNs := tx.ReadWriteBucket(waddrmgrNamespaceKey) + scope, err := b.keyScope() + if err != nil { + return nil, err + } - scope, err := b.keyScope() - if err != nil { - return err + // First, attempt to see if we can read the key directly from + // btcwallet's internal cache, if we can then we can skip all the + // operations below (fast path). + if keyDesc.PubKey == nil { + keyPath := waddrmgr.DerivationPath{ + InternalAccount: uint32(keyDesc.Family), + Account: uint32(keyDesc.Family), + Branch: 0, + Index: keyDesc.Index, } + privKey, err := scope.DeriveFromKeyPathCache(keyPath) + if err == nil { + return privKey, nil + } + } + + db := b.wallet.Database() + err = walletdb.Update(db, func(tx walletdb.ReadWriteTx) error { + addrmgrNs := tx.ReadWriteBucket(waddrmgrNamespaceKey) // If the account doesn't exist, then we may need to create it // for the first time in order to derive the keys that we diff --git a/lnwallet/btcwallet/signer.go b/lnwallet/btcwallet/signer.go index 953ec1006..bc48360e6 100644 --- a/lnwallet/btcwallet/signer.go +++ b/lnwallet/btcwallet/signer.go @@ -75,6 +75,20 @@ func (b *BtcWallet) deriveKeyByLocator(keyLoc keychain.KeyLocator) (*btcec.Priva return nil, err } + // First try to read the key from the cached store, if this fails, then + // we'll fall through to the method below that requires a database + // transaction. + path := waddrmgr.DerivationPath{ + InternalAccount: uint32(keyLoc.Family), + Account: uint32(keyLoc.Family), + Branch: 0, + Index: keyLoc.Index, + } + privKey, err := scopedMgr.DeriveFromKeyPathCache(path) + if err == nil { + return privKey, nil + } + var key *btcec.PrivateKey err = walletdb.Update(b.db, func(tx walletdb.ReadWriteTx) error { addrmgrNs := tx.ReadWriteBucket(waddrmgrNamespaceKey) From abb2448718bed18edc05c2aa14940ed0bbb1ce47 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Wed, 25 Aug 2021 18:44:55 -0700 Subject: [PATCH 4/4] docs/release-notes: add section w/ priv key caching in 0.14 --- docs/release-notes/release-notes-0.14.0.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/release-notes/release-notes-0.14.0.md b/docs/release-notes/release-notes-0.14.0.md index 94a8df3a3..369e42a2b 100644 --- a/docs/release-notes/release-notes-0.14.0.md +++ b/docs/release-notes/release-notes-0.14.0.md @@ -177,6 +177,12 @@ you. when encoding/decoding messages. Such that most of the heap escapes are fixed, resulting in less memory being used when running `lnd`. +* [`lnd` will now no longer (in a steady state) need to open a new database + transaction each time a private key needs to be derived for signing or ECDH + operations]https://github.com/lightningnetwork/lnd/pull/5629). This results + in a massive performance improvement across several routine operations at the + cost of a small amount of memory allocated for a new cache. + ## Log system * [Save compressed log files from logrorate during