Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
69 changes: 66 additions & 3 deletions docs/developer/adr/0010-keyvault-encryption-layer.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,44 @@ to each lock, unlock, encrypt, decrypt, or derivation operation.
Keyvault accesses persistence through `db.Store`. It does not talk directly to
SQL backends.

The vault API also includes a digest level signing method. `DerivePubKey` and
`Sign` keep private key use inside the same wallet scoped boundary that owns
lock state and key lifecycle. Callers identify keys with a sealed
`wallet/signing.KeyLocator` variant: `FullHDKeyLocator` for a full HD
derivation path, `AccountKeyLocator` for an account row child reference, or
`ScriptPubKeyLocator` for raw or imported rows without exposing private key
material or database routing.

Wallet code builds a sealed `wallet/signing.Request` containing both the key

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

is there another consumer of signing? I don't see the necessity of making it a public module, nor making it an independent module. We can just put all the code under wallet/internal/keyvault as the vault is the only consumer?

locator and a caller computed `[32]byte` digest, then calls `Sign` with that
request. Requests do not accept transactions, PSBTs, scripts, witnesses, or
sighash types. Transaction sighash construction, PSBT assembly, signature
serialization, appended sighash bytes, witness construction, and Taproot tree
construction remain wallet layer responsibilities.

`Sign` accepts a sealed `wallet/signing.Request` variant instead of a
flag-heavy parameter struct. The sealed variants describe ECDSA, compact
recoverable ECDSA, Schnorr, and tweaked Schnorr primitive operations, making
invalid mode combinations unrepresentable outside the shared signing package.
The shared package also defines sealed signature result variants so keyvault
and later public wallet signing code can use the same primitive request and
result machinery.

The Schnorr tweak request uses BIP340 terminology for even-Y key normalization
and carries an already computed additive secp256k1 scalar. This scalar is the
final key-tweak value, not a Taproot script root, merkle root, or BIP341 tweak
input byte string. These references describe the key and tweak concepts only.
They do not move transaction, script, or Taproot tree knowledge into the vault.

`wallet/signing.CompactECDSARequest` remains digest-level, and the returned
compact recoverable ECDSA bytes remain the caller's responsibility for compact
signature use and wallet-layer transaction assembly.

`wallet/signing.SchnorrKeyTweak` is a value descriptor with a
`Scalar btcec.ModNScalar` and `NormalizeEvenY bool`. Passing the additive scalar
as data lets the vault apply the private key operation internally without
accepting a private key callback from the wallet layer.

```mermaid
flowchart TD
wallet[Wallet]
Expand Down Expand Up @@ -85,9 +123,10 @@ wallet facing lock, unlock, encryption, decryption, and derivation APIs.
zeroing.

2. **Expose typed domain interfaces**
Keyvault returns domain types such as `*btcec.PrivateKey`,
`*btcec.PublicKey`, and `btcutil.Address` instead of exposing encrypted byte
slices to wallet code.
Keyvault exposes typed domain operations and results such as public keys,
signatures, and encrypted bytes as appropriate. Private key material remains
inside the vault instead of being returned to wallet code as raw key bytes or
blobs.

3. **Handle HD derivation**
Keyvault uses `btcutil/hdkeychain` for BIP32 and BIP44 derivation and returns
Expand All @@ -109,6 +148,11 @@ wallet facing lock, unlock, encryption, decryption, and derivation APIs.
New code uses keyvault while legacy code continues to rely on `waddrmgr`
until the migration is complete.

8. **Sign caller computed digests**
Keyvault signs sealed ECDSA, compact recoverable ECDSA, Schnorr, and tweaked
Schnorr digest requests for located wallet keys while keeping transaction,
PSBT, script, witness, sighash, and Taproot tree logic in the wallet layer.

## 3. Consequences

The wallet facing `keyvault.Vault` API intentionally does not expose wallet ID
Expand All @@ -128,6 +172,13 @@ code.
Auto lock timeout scheduling is a wallet or controller lifecycle policy, not
part of `keyvault.Vault`.

The signing surface follows the same boundary. Wallet code chooses the key,
constructs the 32-byte digest, builds the appropriate sealed
`wallet/signing.Request` carrying both values, calls `Sign`, then serializes and
attaches the returned sealed `wallet/signing.Signature` in the format required by
the transaction or PSBT flow. This keeps private key access inside the vault
without making the vault a Bitcoin transaction signer.

### Pros

1. **Separation of concerns**
Expand Down Expand Up @@ -163,6 +214,18 @@ part of `keyvault.Vault`.
Keyvault can coexist with `waddrmgr`, allowing new SQL backed paths to move
behind the new boundary before legacy paths are removed.

9. **Narrow signing boundary**
Digest level signing keeps private key operations in one locked component
without duplicating transaction, PSBT, script, witness, or Taproot tree
responsibilities inside keyvault.

10. **Tweak data stays explicit**
`wallet/signing.SchnorrKeyTweak` carries the already computed additive
secp256k1 scalar and even-Y normalization flag as data, so callers can
describe Taproot key tweaks without passing script roots, merkle roots,
BIP341 tweak input bytes, or private key callbacks across the vault
boundary.

### Cons

1. **Additional abstraction**
Expand Down
12 changes: 6 additions & 6 deletions wallet/account_derive_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
"github.com/btcsuite/btcd/btcutil/v2/hdkeychain"
"github.com/btcsuite/btcd/chaincfg/v2"
"github.com/btcsuite/btcwallet/waddrmgr"
walletmock "github.com/btcsuite/btcwallet/wallet/internal/bwtest/mock"
vaultmock "github.com/btcsuite/btcwallet/wallet/internal/bwtest/mock/vault"
"github.com/btcsuite/btcwallet/wallet/internal/db"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
Expand All @@ -22,8 +22,8 @@ import (
// crypt — Encrypt and Decrypt both return a fresh copy of the input bytes
// with no error. This lets tests roundtrip derived account material
// without bringing in real cryptoKey wiring.
func newIdentityVault() *walletmock.Vault {
vault := &walletmock.Vault{}
func newIdentityVault() *vaultmock.Vault {
vault := &vaultmock.Vault{}
identity := func(_ waddrmgr.CryptoKeyType, b []byte) []byte {
out := make([]byte, len(b))
copy(out, b)
Expand All @@ -45,8 +45,8 @@ var errEncryptForTest = errors.New("encrypt boom")
// newEncryptErrorVault returns a Vault whose Encrypt call always fails
// with errEncryptForTest. Decrypt is left unconfigured because the tests
// that consume this vault never reach a decrypt path.
func newEncryptErrorVault() *walletmock.Vault {
vault := &walletmock.Vault{}
func newEncryptErrorVault() *vaultmock.Vault {
vault := &vaultmock.Vault{}
vault.On("Encrypt", mock.Anything, mock.Anything).Return(
nil, errEncryptForTest,
)
Expand Down Expand Up @@ -111,7 +111,7 @@ func TestNewAccountDeriveFn_MaxAccountNumber(t *testing.T) {
masterKey := testMasterKey(t)
scope := db.KeyScope{Purpose: 84, Coin: 0}

derive := newAccountDeriveFn(masterKey, &walletmock.Vault{}, 0)
derive := newAccountDeriveFn(masterKey, &vaultmock.Vault{}, 0)
data, err := derive(t.Context(), scope, db.MaxAccountNumber+1, false)

require.Nil(t, data)
Expand Down
5 changes: 3 additions & 2 deletions wallet/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
bwmock "github.com/btcsuite/btcwallet/bwtest/mock"
"github.com/btcsuite/btcwallet/waddrmgr"
walletmock "github.com/btcsuite/btcwallet/wallet/internal/bwtest/mock"
vaultmock "github.com/btcsuite/btcwallet/wallet/internal/bwtest/mock/vault"
"github.com/btcsuite/btcwallet/wallet/internal/db"
"github.com/btcsuite/btcwallet/walletdb"
_ "github.com/btcsuite/btcwallet/walletdb/bdb"
Expand Down Expand Up @@ -97,7 +98,7 @@ func setupTestDB(t *testing.T) (walletdb.DB, func()) {
// mockWalletDeps holds the mocked dependencies for the Wallet.
type mockWalletDeps struct {
addrStore *bwmock.AddrStore
vault *walletmock.Vault
vault *vaultmock.Vault
store *walletmock.Store
txStore *bwmock.TxStore
syncer *mockChainSyncer
Expand All @@ -118,7 +119,7 @@ func createTestWalletWithMocks(t *testing.T) (*Wallet, *mockWalletDeps) {
t.Cleanup(cleanup)

mockAddrStore := &bwmock.AddrStore{}
mockVault := &walletmock.Vault{}
mockVault := &vaultmock.Vault{}
mockStore := &walletmock.Store{}
mockTxStore := &bwmock.TxStore{}
mockSyncer := &mockChainSyncer{}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@
// Use of this source code is governed by an ISC
// license that can be found in the LICENSE file.

package mock
package vaultmock

import (
"context"

"github.com/btcsuite/btcd/btcec/v2"
"github.com/btcsuite/btcwallet/waddrmgr"
"github.com/btcsuite/btcwallet/wallet/internal/keyvault"
"github.com/btcsuite/btcwallet/wallet/signing"
"github.com/stretchr/testify/mock"
)

Expand All @@ -16,6 +19,9 @@ type Vault struct {
mock.Mock
}

// Ensure Vault implements the keyvault.Vault interface.
var _ keyvault.Vault = (*Vault)(nil)

// Encrypt forwards to the configured testify expectations.
func (m *Vault) Encrypt(keyType waddrmgr.CryptoKeyType,
plaintext []byte) ([]byte, error) {
Expand All @@ -34,6 +40,42 @@ func (m *Vault) Decrypt(keyType waddrmgr.CryptoKeyType,
return returnBytes(args, keyType, ciphertext), args.Error(1)
}

// DerivePubKey forwards to the configured testify expectations.
func (m *Vault) DerivePubKey(ctx context.Context,
locator signing.KeyLocator) (*btcec.PublicKey, error) {

args := m.Called(ctx, locator)

if args.Get(0) == nil {
return nil, args.Error(1)
}

pk, ok := args.Get(0).(*btcec.PublicKey)
if !ok {
return nil, args.Error(1)
}

return pk, args.Error(1)
}

// Sign forwards to the configured testify expectations.
func (m *Vault) Sign(ctx context.Context,
req signing.Request) (signing.Signature, error) {

args := m.Called(ctx, req)

if args.Get(0) == nil {
return nil, args.Error(1)
}

sig, ok := args.Get(0).(signing.Signature)
if !ok {
return nil, args.Error(1)
}

return sig, args.Error(1)
}

// Unlock forwards to the configured testify expectations.
func (m *Vault) Unlock(ctx context.Context, passphrase []byte) error {
args := m.Called(ctx, passphrase)
Expand Down
28 changes: 28 additions & 0 deletions wallet/internal/keyvault/keyvault.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ import (
"context"
"errors"

"github.com/btcsuite/btcd/btcec/v2"
"github.com/btcsuite/btcwallet/waddrmgr"
"github.com/btcsuite/btcwallet/wallet/signing"
)

// ErrInvalidPassphrase reports that the provided vault passphrase is wrong.
Expand All @@ -19,6 +21,12 @@ var ErrVaultLocked = errors.New("vault is locked")
// vault was already unlocked.
var ErrVaultUnlocked = errors.New("vault is already unlocked")

// ErrVaultKeyOpsUnimplemented reports that the vault does not yet implement
// key derivation or signing operations.
var ErrVaultKeyOpsUnimplemented = errors.New(
"vault key operations are unimplemented",
)

// Vault manages the lock lifecycle and cryptographic operations for wallet key
// material.
type Vault interface {
Expand Down Expand Up @@ -49,4 +57,24 @@ type Vault interface {
// private passphrase. The vault must already be unlocked when this method
// is called.
ChangePassphrase(ctx context.Context, newPassphrase []byte) error

// DerivePubKey derives the public key for the provided key locator.
//
// The caller provides a wallet key locator, and the vault returns the
// corresponding public key without exposing private key material. The vault
// must already be unlocked; implementations must return ErrVaultLocked if
// called while locked.
DerivePubKey(ctx context.Context,
keyLocator signing.KeyLocator) (*btcec.PublicKey, error)

// Sign signs the caller-computed 32-byte digest for the located key using
// the primitive signing mode described by req.
//
// The caller is responsible for digest construction, sighash selection,
// signature serialization, transaction assembly, and any appended sighash
// byte. Signing requests do not carry transactions, PSBTs, scripts,
// witnesses, sighash types, or Taproot tree data. The vault must already be
// unlocked; implementations must return ErrVaultLocked if called while
// locked.
Sign(ctx context.Context, req signing.Request) (signing.Signature, error)
}
17 changes: 17 additions & 0 deletions wallet/internal/keyvault/vault.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
package keyvault

import (
"context"
"errors"
"sync"

"github.com/btcsuite/btcd/btcec/v2"
"github.com/btcsuite/btcd/btcutil/v2/hdkeychain"
"github.com/btcsuite/btcwallet/snacl"
"github.com/btcsuite/btcwallet/wallet/internal/db"
"github.com/btcsuite/btcwallet/wallet/signing"
)

var (
Expand Down Expand Up @@ -63,6 +66,20 @@ func NewWalletVault(store db.Store, walletID uint32,
}
}

// DerivePubKey reports that WalletVault does not yet expose key operations.
func (v *WalletVault) DerivePubKey(context.Context,
signing.KeyLocator) (*btcec.PublicKey, error) {

return nil, ErrVaultKeyOpsUnimplemented
}

// Sign reports that WalletVault does not yet expose key operations.
func (v *WalletVault) Sign(context.Context, signing.Request) (signing.Signature,
error) {

return nil, ErrVaultKeyOpsUnimplemented
}

// zero clears the runtime secret material held by the unlocked state.
func (s *unlockedState) zero() {
if s == nil {
Expand Down
6 changes: 3 additions & 3 deletions wallet/signer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
"github.com/btcsuite/btcd/wire/v2"
bwmock "github.com/btcsuite/btcwallet/bwtest/mock"
"github.com/btcsuite/btcwallet/waddrmgr"
walletmock "github.com/btcsuite/btcwallet/wallet/internal/bwtest/mock"
vaultmock "github.com/btcsuite/btcwallet/wallet/internal/bwtest/mock/vault"
"github.com/btcsuite/btcwallet/wallet/internal/db"
sqlitedb "github.com/btcsuite/btcwallet/wallet/internal/db/sqlite"
"github.com/btcsuite/btcwallet/walletdb"
Expand Down Expand Up @@ -1332,7 +1332,7 @@ func createDummyTestTx(pkScript []byte) (*wire.TxOut, *wire.MsgTx) {
// newSQLAddressSigningWallet returns a started, unlocked wallet whose address
// records are stored in SQLite while signing keys come from legacy waddrmgr.
func newSQLAddressSigningWallet(t *testing.T) (*Wallet, *bwmock.Chain,
*walletmock.Vault) {
*vaultmock.Vault) {

t.Helper()

Expand Down Expand Up @@ -1384,7 +1384,7 @@ func newSQLAddressSigningWallet(t *testing.T) (*Wallet, *bwmock.Chain,
require.NoError(t, err)

chain := &bwmock.Chain{}
vault := &walletmock.Vault{}
vault := &vaultmock.Vault{}

w = &Wallet{
addrStore: addrStore,
Expand Down
3 changes: 3 additions & 0 deletions wallet/signing/doc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// Package signing defines shared key locators, primitive digest signing
// requests, and signature results for wallet signing boundaries.
package signing
Loading
Loading