From 35b4b237c997a5a57dcc8dc06a5f85aa703d6df6 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 17 Feb 2021 16:24:08 -0800 Subject: [PATCH 01/10] wallet: include BIP 32 derivation paths for inputs in PSBTs Watch-only accounts are usually backed by an external hardware signer, some of which require derivation paths to be populated for each relevant input to sign. --- wallet/psbt.go | 11 +++++++++-- wallet/utxos.go | 38 ++++++++++++++++++++++++++++---------- wallet/utxos_test.go | 29 ++++++++++++++++++++++++++++- 3 files changed, 65 insertions(+), 13 deletions(-) diff --git a/wallet/psbt.go b/wallet/psbt.go index 234f48a..3fd81c8 100644 --- a/wallet/psbt.go +++ b/wallet/psbt.go @@ -70,7 +70,7 @@ func (w *Wallet) FundPsbt(packet *psbt.Packet, account uint32, addInputInfo := func(inputs []*wire.TxIn) error { packet.Inputs = make([]psbt.PInput, len(inputs)) for idx, in := range inputs { - tx, utxo, _, err := w.FetchInputInfo( + tx, utxo, derivationPath, _, err := w.FetchInputInfo( &in.PreviousOutPoint, ) if err != nil { @@ -91,6 +91,11 @@ func (w *Wallet) FundPsbt(packet *psbt.Packet, account uint32, } packet.Inputs[idx].SighashType = txscript.SigHashAll + // Include the derivation path for each input. + packet.Inputs[idx].Bip32Derivation = []*psbt.Bip32Derivation{ + derivationPath, + } + // We don't want to include the witness or any script // just yet. packet.UnsignedTx.TxIn[idx].Witness = wire.TxWitness{} @@ -227,6 +232,8 @@ func (w *Wallet) FundPsbt(packet *psbt.Packet, account uint32, // // NOTE: This method does NOT publish the transaction after it's been finalized // successfully. +// +// TODO: require account and check if watch only to avoid signing. func (w *Wallet) FinalizePsbt(packet *psbt.Packet) error { // Let's check that this is actually something we can and want to sign. // We need at least one input and one output. @@ -259,7 +266,7 @@ func (w *Wallet) FinalizePsbt(packet *psbt.Packet) error { // We can only sign this input if it's ours, so we try to map it // to a coin we own. If we can't, then we'll continue as it // isn't our input. - fullTx, txOut, _, err := w.FetchInputInfo( + fullTx, txOut, _, _, err := w.FetchInputInfo( &txIn.PreviousOutPoint, ) if err != nil { diff --git a/wallet/utxos.go b/wallet/utxos.go index a29d094..d22cc90 100644 --- a/wallet/utxos.go +++ b/wallet/utxos.go @@ -11,6 +11,8 @@ import ( "github.com/btcsuite/btcd/txscript" "github.com/btcsuite/btcd/wire" + "github.com/btcsuite/btcutil/hdkeychain" + "github.com/btcsuite/btcutil/psbt" "github.com/btcsuite/btcwallet/waddrmgr" "github.com/btcsuite/btcwallet/walletdb" ) @@ -105,15 +107,15 @@ func (w *Wallet) UnspentOutputs(policy OutputSelectionPolicy) ([]*TransactionOut // full transaction, the target txout and the number of confirmations are // returned. Otherwise, a non-nil error value of ErrNotMine is returned instead. func (w *Wallet) FetchInputInfo(prevOut *wire.OutPoint) (*wire.MsgTx, - *wire.TxOut, int64, error) { + *wire.TxOut, *psbt.Bip32Derivation, int64, error) { // We manually look up the output within the tx store. txid := &prevOut.Hash txDetail, err := UnstableAPI(w).TxDetails(txid) if err != nil { - return nil, nil, 0, err + return nil, nil, nil, 0, err } else if txDetail == nil { - return nil, nil, 0, ErrNotMine + return nil, nil, nil, 0, ErrNotMine } // With the output retrieved, we'll make an additional check to ensure @@ -122,19 +124,25 @@ func (w *Wallet) FetchInputInfo(prevOut *wire.OutPoint) (*wire.MsgTx, // like in the event of us being the sender of the transaction. numOutputs := uint32(len(txDetail.TxRecord.MsgTx.TxOut)) if prevOut.Index >= numOutputs { - return nil, nil, 0, fmt.Errorf("invalid output index %v for "+ + return nil, nil, nil, 0, fmt.Errorf("invalid output index %v for "+ "transaction with %v outputs", prevOut.Index, numOutputs) } pkScript := txDetail.TxRecord.MsgTx.TxOut[prevOut.Index].PkScript - if _, err := w.fetchOutputAddr(pkScript); err != nil { - return nil, nil, 0, err + addr, err := w.fetchOutputAddr(pkScript) + if err != nil { + return nil, nil, nil, 0, err } + pubKeyAddr, ok := addr.(waddrmgr.ManagedPubKeyAddress) + if !ok { + return nil, nil, nil, 0, err + } + keyScope, derivationPath, _ := pubKeyAddr.DerivationInfo() // Determine the number of confirmations the output currently has. _, currentHeight, err := w.chainClient.GetBestBlock() if err != nil { - return nil, nil, 0, fmt.Errorf("unable to retrieve current "+ + return nil, nil, nil, 0, fmt.Errorf("unable to retrieve current "+ "height: %v", err) } confs := int64(0) @@ -143,9 +151,19 @@ func (w *Wallet) FetchInputInfo(prevOut *wire.OutPoint) (*wire.MsgTx, } return &txDetail.TxRecord.MsgTx, &wire.TxOut{ - Value: txDetail.TxRecord.MsgTx.TxOut[prevOut.Index].Value, - PkScript: pkScript, - }, confs, nil + Value: txDetail.TxRecord.MsgTx.TxOut[prevOut.Index].Value, + PkScript: pkScript, + }, &psbt.Bip32Derivation{ + PubKey: pubKeyAddr.PubKey().SerializeCompressed(), + MasterKeyFingerprint: 0, // TODO + Bip32Path: []uint32{ + keyScope.Purpose + hdkeychain.HardenedKeyStart, + keyScope.Coin + hdkeychain.HardenedKeyStart, + derivationPath.Account, + derivationPath.Branch, + derivationPath.Index, + }, + }, confs, nil } // fetchOutputAddr attempts to fetch the managed address corresponding to the diff --git a/wallet/utxos_test.go b/wallet/utxos_test.go index 3181c80..adeef80 100644 --- a/wallet/utxos_test.go +++ b/wallet/utxos_test.go @@ -10,6 +10,7 @@ import ( "github.com/btcsuite/btcd/txscript" "github.com/btcsuite/btcd/wire" + "github.com/btcsuite/btcutil/hdkeychain" "github.com/btcsuite/btcwallet/waddrmgr" ) @@ -43,7 +44,7 @@ func TestFetchInputInfo(t *testing.T) { Hash: incomingTx.TxHash(), Index: 0, } - tx, out, confirmations, err := w.FetchInputInfo(prevOut) + tx, out, derivationPath, confirmations, err := w.FetchInputInfo(prevOut) if err != nil { t.Fatalf("error fetching input info: %v", err) } @@ -54,6 +55,32 @@ func TestFetchInputInfo(t *testing.T) { t.Fatalf("unexpected TX out, got %v wanted %v", tx.TxOut[prevOut.Index].PkScript, utxOut) } + if len(derivationPath.Bip32Path) != 5 { + t.Fatalf("expected derivation path of length %v, got %v", 3, + len(derivationPath.Bip32Path)) + } + if derivationPath.Bip32Path[0] != waddrmgr.KeyScopeBIP0084.Purpose { + t.Fatalf("expected purpose %v, got %v", + waddrmgr.KeyScopeBIP0084.Purpose, + derivationPath.Bip32Path[0]) + } + if derivationPath.Bip32Path[1] != waddrmgr.KeyScopeBIP0084.Coin { + t.Fatalf("expected coin type %v, got %v", + waddrmgr.KeyScopeBIP0084.Coin, + derivationPath.Bip32Path[1]) + } + if derivationPath.Bip32Path[2] != hdkeychain.HardenedKeyStart { + t.Fatalf("expected account %v, got %v", + hdkeychain.HardenedKeyStart, derivationPath.Bip32Path[2]) + } + if derivationPath.Bip32Path[3] != 0 { + t.Fatalf("expected branch %v, got %v", 0, + derivationPath.Bip32Path[3]) + } + if derivationPath.Bip32Path[4] != 0 { + t.Fatalf("expected index %v, got %v", 0, + derivationPath.Bip32Path[4]) + } if confirmations != int64(0-testBlockHeight) { t.Fatalf("unexpected number of confirmations, got %d wanted %d", confirmations, 0-testBlockHeight) From 7fa80abc44eb79ee6db0eb5a6770ef0db4560dd1 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 17 Feb 2021 16:24:11 -0800 Subject: [PATCH 02/10] waddrmgr: include master key fingerprint in derivation path Following the previous commit, some external hardware signers require a master key fingerprint to be present within the PSBT input derivation paths so that the signer can recognize which inputs are relevant and must be signed. --- waddrmgr/manager.go | 2 +- waddrmgr/scoped_manager.go | 45 +++++++++++++++++++++++--------------- wallet/utxos.go | 2 +- 3 files changed, 29 insertions(+), 20 deletions(-) diff --git a/waddrmgr/manager.go b/waddrmgr/manager.go index cf7b9aa..f8acb34 100644 --- a/waddrmgr/manager.go +++ b/waddrmgr/manager.go @@ -1218,7 +1218,7 @@ func (m *Manager) Unlock(ns walletdb.ReadBucket, passphrase []byte) error { // We'll also derive any private keys that are pending due to // them being created while the address manager was locked. for _, info := range manager.deriveOnUnlock { - addressKey, _, err := manager.deriveKeyFromPath( + addressKey, _, _, err := manager.deriveKeyFromPath( ns, info.managedAddr.InternalAccount(), info.branch, info.index, true, ) diff --git a/waddrmgr/scoped_manager.go b/waddrmgr/scoped_manager.go index 70fe48b..962ab8e 100644 --- a/waddrmgr/scoped_manager.go +++ b/waddrmgr/scoped_manager.go @@ -72,6 +72,12 @@ type DerivationPath struct { // Index is the final child in the derivation path. This denotes the // key index within as a child of the account and branch. Index uint32 + + // MasterKeyFingerprint represents the fingerprint of the root key (also + // known as the key with derivation path m/) corresponding to the + // account public key. This may be required by some hardware wallets for + // proper identification and signing. + MasterKeyFingerprint uint32 } // KeyScope represents a restricted key scope from the primary root key within @@ -444,10 +450,11 @@ func (s *ScopedKeyManager) loadAccountInfo(ns walletdb.ReadBucket, index-- } lastExtAddrPath := DerivationPath{ - InternalAccount: account, - Account: acctInfo.acctKeyPub.ChildIndex(), - Branch: branch, - Index: index, + InternalAccount: account, + Account: acctInfo.acctKeyPub.ChildIndex(), + Branch: branch, + Index: index, + MasterKeyFingerprint: acctInfo.masterKeyFingerprint, } lastExtKey, err := s.deriveKey(acctInfo, branch, index, hasPrivateKey) if err != nil { @@ -465,10 +472,11 @@ func (s *ScopedKeyManager) loadAccountInfo(ns walletdb.ReadBucket, index-- } lastIntAddrPath := DerivationPath{ - InternalAccount: account, - Account: acctInfo.acctKeyPub.ChildIndex(), - Branch: branch, - Index: index, + InternalAccount: account, + Account: acctInfo.acctKeyPub.ChildIndex(), + Branch: branch, + Index: index, + MasterKeyFingerprint: acctInfo.masterKeyFingerprint, } lastIntKey, err := s.deriveKey(acctInfo, branch, index, hasPrivateKey) if err != nil { @@ -580,7 +588,7 @@ func (s *ScopedKeyManager) DeriveFromKeyPath(ns walletdb.ReadBucket, watchOnly := s.rootManager.WatchOnly() private := !s.rootManager.IsLocked() && !watchOnly - addrKey, _, err := s.deriveKeyFromPath( + addrKey, _, _, err := s.deriveKeyFromPath( ns, kp.InternalAccount, kp.Branch, kp.Index, private, ) if err != nil { @@ -601,18 +609,18 @@ func (s *ScopedKeyManager) DeriveFromKeyPath(ns walletdb.ReadBucket, // This function MUST be called with the manager lock held for writes. func (s *ScopedKeyManager) deriveKeyFromPath(ns walletdb.ReadBucket, internalAccount, branch, index uint32, private bool) ( - *hdkeychain.ExtendedKey, *hdkeychain.ExtendedKey, error) { + *hdkeychain.ExtendedKey, *hdkeychain.ExtendedKey, uint32, error) { // Look up the account key information. acctInfo, err := s.loadAccountInfo(ns, internalAccount) if err != nil { - return nil, nil, err + return nil, nil, 0, err } private = private && acctInfo.acctKeyPriv != nil addrKey, err := s.deriveKey(acctInfo, branch, index, private) if err != nil { - return nil, nil, err + return nil, nil, 0, err } acctKey := acctInfo.acctKeyPub @@ -620,7 +628,7 @@ func (s *ScopedKeyManager) deriveKeyFromPath(ns walletdb.ReadBucket, acctKey = acctInfo.acctKeyPriv } - return addrKey, acctKey, nil + return addrKey, acctKey, acctInfo.masterKeyFingerprint, nil } // chainAddressRowToManaged returns a new managed address based on chained @@ -634,7 +642,7 @@ func (s *ScopedKeyManager) chainAddressRowToManaged(ns walletdb.ReadBucket, // function, we use the internal isLocked to avoid a deadlock. private := !s.rootManager.isLocked() && !s.rootManager.watchOnly() - addressKey, acctKey, err := s.deriveKeyFromPath( + addressKey, acctKey, masterKeyFingerprint, err := s.deriveKeyFromPath( ns, row.account, row.branch, row.index, private, ) if err != nil { @@ -647,10 +655,11 @@ func (s *ScopedKeyManager) chainAddressRowToManaged(ns walletdb.ReadBucket, } return s.keyToManaged( addressKey, DerivationPath{ - InternalAccount: row.account, - Account: acctKey.ChildIndex(), - Branch: row.branch, - Index: row.index, + InternalAccount: row.account, + Account: acctKey.ChildIndex(), + Branch: row.branch, + Index: row.index, + MasterKeyFingerprint: masterKeyFingerprint, }, acctInfo, ) } diff --git a/wallet/utxos.go b/wallet/utxos.go index d22cc90..dc1f209 100644 --- a/wallet/utxos.go +++ b/wallet/utxos.go @@ -155,7 +155,7 @@ func (w *Wallet) FetchInputInfo(prevOut *wire.OutPoint) (*wire.MsgTx, PkScript: pkScript, }, &psbt.Bip32Derivation{ PubKey: pubKeyAddr.PubKey().SerializeCompressed(), - MasterKeyFingerprint: 0, // TODO + MasterKeyFingerprint: derivationPath.MasterKeyFingerprint, Bip32Path: []uint32{ keyScope.Purpose + hdkeychain.HardenedKeyStart, keyScope.Coin + hdkeychain.HardenedKeyStart, From 2301069644d4bcd2572aee4ae64d80e72eb6a242 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 17 Feb 2021 16:24:14 -0800 Subject: [PATCH 03/10] wallet: require key scope of account for transaction creation methods Now that we're able to fund transactions from multiple accounts within different key scopes, we extend our transaction creation methods to accept a key scope parameter as well, to determine the correct account to select inputs from. --- rpc/legacyrpc/methods.go | 11 ++++---- wallet/createtx.go | 55 ++++++++++++++++++++++++++++------------ wallet/createtx_test.go | 9 ++++--- wallet/psbt.go | 32 ++++++++++++++--------- wallet/psbt_test.go | 4 +-- wallet/utxos_test.go | 6 +++-- wallet/wallet.go | 46 +++++++++++++++++++++------------ 7 files changed, 106 insertions(+), 57 deletions(-) diff --git a/rpc/legacyrpc/methods.go b/rpc/legacyrpc/methods.go index fff9d59..d29874e 100644 --- a/rpc/legacyrpc/methods.go +++ b/rpc/legacyrpc/methods.go @@ -1359,13 +1359,14 @@ func makeOutputs(pairs map[string]btcutil.Amount, chainParams *chaincfg.Params) // It returns the transaction hash in string format upon success // All errors are returned in btcjson.RPCError format func sendPairs(w *wallet.Wallet, amounts map[string]btcutil.Amount, - account uint32, minconf int32, feeSatPerKb btcutil.Amount) (string, error) { + keyScope waddrmgr.KeyScope, account uint32, minconf int32, + feeSatPerKb btcutil.Amount) (string, error) { outputs, err := makeOutputs(amounts, w.ChainParams()) if err != nil { return "", err } - tx, err := w.SendOutputs(outputs, account, minconf, feeSatPerKb, "") + tx, err := w.SendOutputs(outputs, &keyScope, account, minconf, feeSatPerKb, "") if err != nil { if err == txrules.ErrAmountNegative { return "", ErrNeedPositiveAmount @@ -1433,7 +1434,7 @@ func sendFrom(icmd interface{}, w *wallet.Wallet, chainClient *chain.RPCClient) cmd.ToAddress: amt, } - return sendPairs(w, pairs, account, minConf, + return sendPairs(w, pairs, waddrmgr.KeyScopeBIP0044, account, minConf, txrules.DefaultRelayFeePerKb) } @@ -1475,7 +1476,7 @@ func sendMany(icmd interface{}, w *wallet.Wallet) (interface{}, error) { pairs[k] = amt } - return sendPairs(w, pairs, account, minConf, txrules.DefaultRelayFeePerKb) + return sendPairs(w, pairs, waddrmgr.KeyScopeBIP0044, account, minConf, txrules.DefaultRelayFeePerKb) } // sendToAddress handles a sendtoaddress RPC request by creating a new @@ -1511,7 +1512,7 @@ func sendToAddress(icmd interface{}, w *wallet.Wallet) (interface{}, error) { } // sendtoaddress always spends from the default account, this matches bitcoind - return sendPairs(w, pairs, waddrmgr.DefaultAccountNum, 1, + return sendPairs(w, pairs, waddrmgr.KeyScopeBIP0044, waddrmgr.DefaultAccountNum, 1, txrules.DefaultRelayFeePerKb) } diff --git a/wallet/createtx.go b/wallet/createtx.go index 358e650..5fd8ea1 100644 --- a/wallet/createtx.go +++ b/wallet/createtx.go @@ -99,14 +99,17 @@ func (s secretSource) GetScript(addr btcutil.Address) ([]byte, error) { // txToOutputs creates a signed transaction which includes each output from // outputs. Previous outputs to reedeem are chosen from the passed account's // UTXO set and minconf policy. An additional output may be added to return -// change to the wallet. An appropriate fee is included based on the wallet's -// current relay fee. The wallet must be unlocked to create the transaction. +// change to the wallet. This output will have an address generated from the +// given key scope and account. If a key scope is not specified, the address +// will always be generated from the P2WKH key scope. An appropriate fee is +// included based on the wallet's current relay fee. The wallet must be +// unlocked to create the transaction. // // NOTE: The dryRun argument can be set true to create a tx that doesn't alter // the database. A tx created with this set to true will intentionally have no // input scripts added and SHOULD NOT be broadcasted. -func (w *Wallet) txToOutputs(outputs []*wire.TxOut, account uint32, - minconf int32, feeSatPerKb btcutil.Amount, dryRun bool) ( +func (w *Wallet) txToOutputs(outputs []*wire.TxOut, keyScope *waddrmgr.KeyScope, + account uint32, minconf int32, feeSatPerKb btcutil.Amount, dryRun bool) ( tx *txauthor.AuthoredTx, err error) { chainClient, err := w.requireChainClient() @@ -120,7 +123,9 @@ func (w *Wallet) txToOutputs(outputs []*wire.TxOut, account uint32, } defer func() { _ = dbtx.Rollback() }() - addrmgrNs, changeSource := w.addrMgrWithChangeSource(dbtx, account) + addrmgrNs, changeSource := w.addrMgrWithChangeSource( + dbtx, keyScope, account, + ) // Get current block's height and hash. bs, err := chainClient.BlockStamp() @@ -128,14 +133,17 @@ func (w *Wallet) txToOutputs(outputs []*wire.TxOut, account uint32, return nil, err } - eligible, err := w.findEligibleOutputs(dbtx, account, minconf, bs) + eligible, err := w.findEligibleOutputs( + dbtx, keyScope, account, minconf, bs, + ) if err != nil { return nil, err } inputSource := makeInputSource(eligible) - tx, err = txauthor.NewUnsignedTransaction(outputs, feeSatPerKb, - inputSource, changeSource) + tx, err = txauthor.NewUnsignedTransaction( + outputs, feeSatPerKb, inputSource, changeSource, + ) if err != nil { return nil, err } @@ -193,7 +201,10 @@ func (w *Wallet) txToOutputs(outputs []*wire.TxOut, account uint32, return tx, nil } -func (w *Wallet) findEligibleOutputs(dbtx walletdb.ReadTx, account uint32, minconf int32, bs *waddrmgr.BlockStamp) ([]wtxmgr.Credit, error) { +func (w *Wallet) findEligibleOutputs(dbtx walletdb.ReadTx, + keyScope *waddrmgr.KeyScope, account uint32, minconf int32, + bs *waddrmgr.BlockStamp) ([]wtxmgr.Credit, error) { + addrmgrNs := dbtx.ReadBucket(waddrmgrNamespaceKey) txmgrNs := dbtx.ReadBucket(wtxmgrNamespaceKey) @@ -239,8 +250,14 @@ func (w *Wallet) findEligibleOutputs(dbtx walletdb.ReadTx, account uint32, minco if err != nil || len(addrs) != 1 { continue } - _, addrAcct, err := w.Manager.AddrAccount(addrmgrNs, addrs[0]) - if err != nil || addrAcct != account { + scopedMgr, addrAcct, err := w.Manager.AddrAccount(addrmgrNs, addrs[0]) + if err != nil { + continue + } + if keyScope != nil && scopedMgr.Scope() != *keyScope { + continue + } + if addrAcct != account { continue } eligible = append(eligible, *output) @@ -249,9 +266,13 @@ func (w *Wallet) findEligibleOutputs(dbtx walletdb.ReadTx, account uint32, minco } // addrMgrWithChangeSource returns the address manager bucket and a change -// source function that returns change addresses from said address manager. +// source function that returns change addresses from said address manager. The +// change addresses will come from the specified key scope and account, unless +// a key scope is not specified. In that case, change addresses will always +// come from the P2WKH key scope. func (w *Wallet) addrMgrWithChangeSource(dbtx walletdb.ReadWriteTx, - account uint32) (walletdb.ReadWriteBucket, txauthor.ChangeSource) { + changeKeyScope *waddrmgr.KeyScope, account uint32) (walletdb.ReadWriteBucket, + txauthor.ChangeSource) { addrmgrNs := dbtx.ReadWriteBucket(waddrmgrNamespaceKey) changeSource := func() ([]byte, error) { @@ -261,14 +282,16 @@ func (w *Wallet) addrMgrWithChangeSource(dbtx walletdb.ReadWriteTx, // are created from account 0. var changeAddr btcutil.Address var err error - changeKeyScope := waddrmgr.KeyScopeBIP0084 + if changeKeyScope == nil { + changeKeyScope = &waddrmgr.KeyScopeBIP0084 + } if account == waddrmgr.ImportedAddrAccount { changeAddr, err = w.newChangeAddress( - addrmgrNs, 0, changeKeyScope, + addrmgrNs, 0, *changeKeyScope, ) } else { changeAddr, err = w.newChangeAddress( - addrmgrNs, account, changeKeyScope, + addrmgrNs, account, *changeKeyScope, ) } if err != nil { diff --git a/wallet/createtx_test.go b/wallet/createtx_test.go index 31a8753..a3e915c 100644 --- a/wallet/createtx_test.go +++ b/wallet/createtx_test.go @@ -34,7 +34,8 @@ func TestTxToOutputsDryRun(t *testing.T) { defer cleanup() // Create an address we can use to send some coins to. - addr, err := w.CurrentAddress(0, waddrmgr.KeyScopeBIP0044) + keyScope := waddrmgr.KeyScopeBIP0049Plus + addr, err := w.CurrentAddress(0, keyScope) if err != nil { t.Fatalf("unable to get current address: %v", addr) } @@ -70,7 +71,7 @@ func TestTxToOutputsDryRun(t *testing.T) { // First do a few dry-runs, making sure the number of addresses in the // database us not inflated. - dryRunTx, err := w.txToOutputs(txOuts, 0, 1, 1000, true) + dryRunTx, err := w.txToOutputs(txOuts, nil, 0, 1, 1000, true) if err != nil { t.Fatalf("unable to author tx: %v", err) } @@ -85,7 +86,7 @@ func TestTxToOutputsDryRun(t *testing.T) { t.Fatalf("expected 1 address, found %v", len(addresses)) } - dryRunTx2, err := w.txToOutputs(txOuts, 0, 1, 1000, true) + dryRunTx2, err := w.txToOutputs(txOuts, nil, 0, 1, 1000, true) if err != nil { t.Fatalf("unable to author tx: %v", err) } @@ -118,7 +119,7 @@ func TestTxToOutputsDryRun(t *testing.T) { // Now we do a proper, non-dry run. This should add a change address // to the database. - tx, err := w.txToOutputs(txOuts, 0, 1, 1000, false) + tx, err := w.txToOutputs(txOuts, nil, 0, 1, 1000, false) if err != nil { t.Fatalf("unable to author tx: %v", err) } diff --git a/wallet/psbt.go b/wallet/psbt.go index 3fd81c8..2a7fbd5 100644 --- a/wallet/psbt.go +++ b/wallet/psbt.go @@ -12,6 +12,7 @@ import ( "github.com/btcsuite/btcd/wire" "github.com/btcsuite/btcutil" "github.com/btcsuite/btcutil/psbt" + "github.com/btcsuite/btcwallet/waddrmgr" "github.com/btcsuite/btcwallet/wallet/txauthor" "github.com/btcsuite/btcwallet/wallet/txrules" "github.com/btcsuite/btcwallet/wtxmgr" @@ -24,17 +25,22 @@ import ( // is created and the index -1 is returned. // // NOTE: If the packet doesn't contain any inputs, coin selection is performed -// automatically. If the packet does contain any inputs, it is assumed that full -// coin selection happened externally and no additional inputs are added. If the -// specified inputs aren't enough to fund the outputs with the given fee rate, -// an error is returned. +// automatically, only selecting inputs from the account based on the given key +// scope and account number. If a key scope is not specified, then inputs from +// accounts matching the account number provided across all key scopes may be +// selected. This is done to handle the default account case, where a user wants +// to fund a PSBT with inputs regardless of their type (NP2WKH, P2WKH, etc.). If +// the packet does contain any inputs, it is assumed that full coin selection +// happened externally and no additional inputs are added. If the specified +// inputs aren't enough to fund the outputs with the given fee rate, an error is +// returned. // // NOTE: A caller of the method should hold the global coin selection lock of // the wallet. However, no UTXO specific lock lease is acquired for any of the // selected/validated inputs by this method. It is in the caller's // responsibility to lock the inputs before handing the partial transaction out. -func (w *Wallet) FundPsbt(packet *psbt.Packet, account uint32, - feeSatPerKB btcutil.Amount) (int32, error) { +func (w *Wallet) FundPsbt(packet *psbt.Packet, keyScope *waddrmgr.KeyScope, + account uint32, feeSatPerKB btcutil.Amount) (int32, error) { // Make sure the packet is well formed. We only require there to be at // least one output but not necessarily any inputs. @@ -113,8 +119,8 @@ func (w *Wallet) FundPsbt(packet *psbt.Packet, account uint32, // includes everything we need, specifically fee estimation and // change address creation. tx, err = w.CreateSimpleTx( - account, packet.UnsignedTx.TxOut, 1, feeSatPerKB, - false, + keyScope, account, packet.UnsignedTx.TxOut, 1, + feeSatPerKB, false, ) if err != nil { return 0, fmt.Errorf("error creating funding TX: %v", @@ -166,7 +172,9 @@ func (w *Wallet) FundPsbt(packet *psbt.Packet, account uint32, if err != nil { return 0, err } - _, changeSource := w.addrMgrWithChangeSource(dbtx, account) + _, changeSource := w.addrMgrWithChangeSource( + dbtx, keyScope, account, + ) // Ask the txauthor to create a transaction with our selected // coins. This will perform fee estimation and add a change @@ -232,9 +240,9 @@ func (w *Wallet) FundPsbt(packet *psbt.Packet, account uint32, // // NOTE: This method does NOT publish the transaction after it's been finalized // successfully. -// -// TODO: require account and check if watch only to avoid signing. -func (w *Wallet) FinalizePsbt(packet *psbt.Packet) error { +func (w *Wallet) FinalizePsbt(keyScope *waddrmgr.KeyScope, account uint32, + packet *psbt.Packet) error { + // Let's check that this is actually something we can and want to sign. // We need at least one input and one output. err := psbt.VerifyInputOutputLen(packet, true, true) diff --git a/wallet/psbt_test.go b/wallet/psbt_test.go index af9896b..5ef2b3a 100644 --- a/wallet/psbt_test.go +++ b/wallet/psbt_test.go @@ -219,7 +219,7 @@ func TestFundPsbt(t *testing.T) { tc := tc t.Run(tc.name, func(t *testing.T) { changeIndex, err := w.FundPsbt( - tc.packet, 0, tc.feeRateSatPerKB, + tc.packet, nil, 0, tc.feeRateSatPerKB, ) // In any case, unlock the UTXO before continuing, we @@ -391,7 +391,7 @@ func TestFinalizePsbt(t *testing.T) { } // Finalize it to add all witness data then extract the final TX. - err = w.FinalizePsbt(packet) + err = w.FinalizePsbt(nil, 0, packet) if err != nil { t.Fatalf("error finalizing PSBT packet: %v", err) } diff --git a/wallet/utxos_test.go b/wallet/utxos_test.go index adeef80..744c97b 100644 --- a/wallet/utxos_test.go +++ b/wallet/utxos_test.go @@ -59,12 +59,14 @@ func TestFetchInputInfo(t *testing.T) { t.Fatalf("expected derivation path of length %v, got %v", 3, len(derivationPath.Bip32Path)) } - if derivationPath.Bip32Path[0] != waddrmgr.KeyScopeBIP0084.Purpose { + if derivationPath.Bip32Path[0] != + waddrmgr.KeyScopeBIP0084.Purpose+hdkeychain.HardenedKeyStart { t.Fatalf("expected purpose %v, got %v", waddrmgr.KeyScopeBIP0084.Purpose, derivationPath.Bip32Path[0]) } - if derivationPath.Bip32Path[1] != waddrmgr.KeyScopeBIP0084.Coin { + if derivationPath.Bip32Path[1] != + waddrmgr.KeyScopeBIP0084.Coin+hdkeychain.HardenedKeyStart { t.Fatalf("expected coin type %v, got %v", waddrmgr.KeyScopeBIP0084.Coin, derivationPath.Bip32Path[1]) diff --git a/wallet/wallet.go b/wallet/wallet.go index c571ce6..5e9896c 100644 --- a/wallet/wallet.go +++ b/wallet/wallet.go @@ -1125,6 +1125,7 @@ func logFilterBlocksResp(block wtxmgr.BlockMeta, type ( createTxRequest struct { + keyScope *waddrmgr.KeyScope account uint32 outputs []*wire.TxOut minconf int32 @@ -1159,8 +1160,10 @@ out: txr.resp <- createTxResponse{nil, err} continue } - tx, err := w.txToOutputs(txr.outputs, txr.account, - txr.minconf, txr.feeSatPerKB, txr.dryRun) + tx, err := w.txToOutputs( + txr.outputs, txr.keyScope, txr.account, + txr.minconf, txr.feeSatPerKB, txr.dryRun, + ) heldUnlock.release() txr.resp <- createTxResponse{tx, err} case <-quit: @@ -1170,20 +1173,25 @@ out: w.wg.Done() } -// CreateSimpleTx creates a new signed transaction spending unspent P2PKH -// outputs with at least minconf confirmations spending to any number of -// address/amount pairs. Change and an appropriate transaction fee are -// automatically included, if necessary. All transaction creation through this -// function is serialized to prevent the creation of many transactions which -// spend the same outputs. +// CreateSimpleTx creates a new signed transaction spending unspent outputs with +// at least minconf confirmations spending to any number of address/amount +// pairs. Only unspent outputs belonging to the given key scope and account will +// be selected, unless a key scope is not specified. In that case, inputs from all +// accounts may be selected, no matter what key scope they belong to. This is +// done to handle the default account case, where a user wants to fund a PSBT +// with inputs regardless of their type (NP2WKH, P2WKH, etc.). Change and an +// appropriate transaction fee are automatically included, if necessary. All +// transaction creation through this function is serialized to prevent the +// creation of many transactions which spend the same outputs. // // NOTE: The dryRun argument can be set true to create a tx that doesn't alter // the database. A tx created with this set to true SHOULD NOT be broadcasted. -func (w *Wallet) CreateSimpleTx(account uint32, outputs []*wire.TxOut, - minconf int32, satPerKb btcutil.Amount, dryRun bool) ( - *txauthor.AuthoredTx, error) { +func (w *Wallet) CreateSimpleTx(keyScope *waddrmgr.KeyScope, account uint32, + outputs []*wire.TxOut, minconf int32, satPerKb btcutil.Amount, + dryRun bool) (*txauthor.AuthoredTx, error) { req := createTxRequest{ + keyScope: keyScope, account: account, outputs: outputs, minconf: minconf, @@ -3104,10 +3112,16 @@ func (w *Wallet) TotalReceivedForAddr(addr btcutil.Address, minConf int32) (btcu return amount, err } -// SendOutputs creates and sends payment transactions. It returns the -// transaction upon success. -func (w *Wallet) SendOutputs(outputs []*wire.TxOut, account uint32, - minconf int32, satPerKb btcutil.Amount, label string) (*wire.MsgTx, error) { +// SendOutputs creates and sends payment transactions. Coin selection is +// performed by the wallet, choosing inputs that belong to the given key scope +// and account, unless a key scope is not specified. In that case, inputs from +// accounts matching the account number provided across all key scopes may be +// selected. This is done to handle the default account case, where a user wants +// to fund a PSBT with inputs regardless of their type (NP2WKH, P2WKH, etc.). It +// returns the transaction upon success. +func (w *Wallet) SendOutputs(outputs []*wire.TxOut, keyScope *waddrmgr.KeyScope, + account uint32, minconf int32, satPerKb btcutil.Amount, + label string) (*wire.MsgTx, error) { // Ensure the outputs to be created adhere to the network's consensus // rules. @@ -3125,7 +3139,7 @@ func (w *Wallet) SendOutputs(outputs []*wire.TxOut, account uint32, // continue to re-broadcast the transaction upon restarts until it has // been confirmed. createdTx, err := w.CreateSimpleTx( - account, outputs, minconf, satPerKb, false, + keyScope, account, outputs, minconf, satPerKb, false, ) if err != nil { return nil, err From f5845dfb428111bc318d126b1a4be8e5be98382c Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 17 Feb 2021 16:24:17 -0800 Subject: [PATCH 04/10] wallet: prevent input signing for transactions on watch-only accounts Watch-only accounts don't have any type of private key information stored, so we avoid populating input signatures in those cases. --- waddrmgr/manager.go | 23 +++++++++++++++++++++++ waddrmgr/scoped_manager.go | 16 ++++++++++++++++ wallet/createtx.go | 30 ++++++++++++++++++++++++++---- wallet/psbt.go | 34 ++++++++++++++++++++++++++++++++-- 4 files changed, 97 insertions(+), 6 deletions(-) diff --git a/waddrmgr/manager.go b/waddrmgr/manager.go index f8acb34..6706703 100644 --- a/waddrmgr/manager.go +++ b/waddrmgr/manager.go @@ -405,6 +405,29 @@ func (m *Manager) watchOnly() bool { return m.watchingOnly } +// IsWatchOnlyAccount determines if the account with the given key scope is set +// up as watch-only. +func (m *Manager) IsWatchOnlyAccount(ns walletdb.ReadBucket, keyScope KeyScope, + account uint32) (bool, error) { + + if m.WatchOnly() { + return true, nil + } + + // Assume the default imported account has no private keys. + // + // TODO: Actually check whether it does. + if account == ImportedAddrAccount { + return true, nil + } + + scopedMgr, err := m.FetchScopedKeyManager(keyScope) + if err != nil { + return false, err + } + return scopedMgr.IsWatchOnlyAccount(ns, account) +} + // lock performs a best try effort to remove and zero all secret keys associated // with the address manager. // diff --git a/waddrmgr/scoped_manager.go b/waddrmgr/scoped_manager.go index 962ab8e..0d0d70f 100644 --- a/waddrmgr/scoped_manager.go +++ b/waddrmgr/scoped_manager.go @@ -2186,6 +2186,22 @@ func (s *ScopedKeyManager) ForEachInternalActiveAddress(ns walletdb.ReadBucket, return nil } +// IsWatchOnlyAccount determines if the given account belonging to this scoped +// manager is set up as watch-only. +func (s *ScopedKeyManager) IsWatchOnlyAccount(ns walletdb.ReadBucket, + account uint32) (bool, error) { + + s.mtx.Lock() + defer s.mtx.Unlock() + + acctInfo, err := s.loadAccountInfo(ns, account) + if err != nil { + return false, err + } + + return acctInfo.acctKeyPriv == nil, nil +} + // cloneKeyWithVersion clones an extended key to use the version corresponding // to the manager's key scope. This should only be used for non-watch-only // accounts as they are stored within the database using the legacy BIP-0044 diff --git a/wallet/createtx.go b/wallet/createtx.go index 5fd8ea1..4df8237 100644 --- a/wallet/createtx.go +++ b/wallet/createtx.go @@ -163,14 +163,36 @@ func (w *Wallet) txToOutputs(outputs []*wire.TxOut, keyScope *waddrmgr.KeyScope, return tx, nil } - err = tx.AddAllInputScripts(secretSource{w.Manager, addrmgrNs}) + // Before committing the transaction, we'll sign our inputs. If the + // inputs are part of a watch-only account, there's no private key + // information stored, so we'll skip signing such. + var watchOnly bool + if keyScope == nil { + // If a key scope wasn't specified, then coin selection was + // performed from the default wallet accounts (NP2WKH, P2WKH), + // so any key scope provided doesn't impact the result of this + // call. + watchOnly, err = w.Manager.IsWatchOnlyAccount( + addrmgrNs, waddrmgr.KeyScopeBIP0084, account, + ) + } else { + watchOnly, err = w.Manager.IsWatchOnlyAccount( + addrmgrNs, *keyScope, account, + ) + } if err != nil { return nil, err } + if !watchOnly { + err = tx.AddAllInputScripts(secretSource{w.Manager, addrmgrNs}) + if err != nil { + return nil, err + } - err = validateMsgTx(tx.Tx, tx.PrevScripts, tx.PrevInputValues) - if err != nil { - return nil, err + err = validateMsgTx(tx.Tx, tx.PrevScripts, tx.PrevInputValues) + if err != nil { + return nil, err + } } if err := dbtx.Commit(); err != nil { diff --git a/wallet/psbt.go b/wallet/psbt.go index 2a7fbd5..58d8f71 100644 --- a/wallet/psbt.go +++ b/wallet/psbt.go @@ -15,6 +15,7 @@ import ( "github.com/btcsuite/btcwallet/waddrmgr" "github.com/btcsuite/btcwallet/wallet/txauthor" "github.com/btcsuite/btcwallet/wallet/txrules" + "github.com/btcsuite/btcwallet/walletdb" "github.com/btcsuite/btcwallet/wtxmgr" ) @@ -313,8 +314,37 @@ func (w *Wallet) FinalizePsbt(keyScope *waddrmgr.KeyScope, account uint32, } } - // Finally, we'll sign the input as is, and populate the input - // with the witness and sigScript (if needed). + // Finally, if the input doesn't belong to a watch-only account, + // then we'll sign it as is, and populate the input with the + // witness and sigScript (if needed). + watchOnly := false + err = walletdb.View(w.db, func(tx walletdb.ReadTx) error { + ns := tx.ReadBucket(waddrmgrNamespaceKey) + var err error + if keyScope == nil { + // If a key scope wasn't specified, then coin + // selection was performed from the default + // wallet accounts (NP2WKH, P2WKH), so any key + // scope provided doesn't impact the result of + // this call. + watchOnly, err = w.Manager.IsWatchOnlyAccount( + ns, waddrmgr.KeyScopeBIP0084, account, + ) + } else { + watchOnly, err = w.Manager.IsWatchOnlyAccount( + ns, *keyScope, account, + ) + } + return err + }) + if err != nil { + return fmt.Errorf("unable to determine if account is "+ + "watch-only: %v", err) + } + if watchOnly { + continue + } + witness, sigScript, err := w.ComputeInputScript( tx, signOutput, idx, sigHashes, in.SighashType, nil, ) From a180b0fcaa8410ad2628de025cce8a32e5d4e988 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 17 Feb 2021 16:24:20 -0800 Subject: [PATCH 05/10] wallet: allow filtering of transactions by account --- rpc/legacyrpc/methods.go | 16 +++++----------- rpc/rpcserver/server.go | 2 +- wallet/wallet.go | 23 +++++++++-------------- 3 files changed, 15 insertions(+), 26 deletions(-) diff --git a/rpc/legacyrpc/methods.go b/rpc/legacyrpc/methods.go index d29874e..0eff722 100644 --- a/rpc/legacyrpc/methods.go +++ b/rpc/legacyrpc/methods.go @@ -1293,20 +1293,14 @@ func listAllTransactions(icmd interface{}, w *wallet.Wallet) (interface{}, error func listUnspent(icmd interface{}, w *wallet.Wallet) (interface{}, error) { cmd := icmd.(*btcjson.ListUnspentCmd) - var addresses map[string]struct{} - if cmd.Addresses != nil { - addresses = make(map[string]struct{}) - // confirm that all of them are good: - for _, as := range *cmd.Addresses { - a, err := decodeAddress(as, w.ChainParams()) - if err != nil { - return nil, err - } - addresses[a.EncodeAddress()] = struct{}{} + if cmd.Addresses != nil && len(*cmd.Addresses) > 0 { + return nil, &btcjson.RPCError{ + Code: btcjson.ErrRPCInvalidParameter, + Message: "Filtering by addresses has been deprecated", } } - return w.ListUnspent(int32(*cmd.MinConf), int32(*cmd.MaxConf), addresses) + return w.ListUnspent(int32(*cmd.MinConf), int32(*cmd.MaxConf), "") } // lockUnspent handles the lockunspent command. diff --git a/rpc/rpcserver/server.go b/rpc/rpcserver/server.go index e9efe59..72a82a5 100644 --- a/rpc/rpcserver/server.go +++ b/rpc/rpcserver/server.go @@ -408,7 +408,7 @@ func (s *walletServer) GetTransactions(ctx context.Context, req *pb.GetTransacti _ = minRecentTxs - gtr, err := s.wallet.GetTransactions(startBlock, endBlock, ctx.Done()) + gtr, err := s.wallet.GetTransactions(startBlock, endBlock, "", ctx.Done()) if err != nil { return nil, translateError(err) } diff --git a/wallet/wallet.go b/wallet/wallet.go index 5e9896c..9c0d8a4 100644 --- a/wallet/wallet.go +++ b/wallet/wallet.go @@ -2186,7 +2186,9 @@ type GetTransactionsResult struct { // Transaction results are organized by blocks in ascending order and unmined // transactions in an unspecified order. Mined transactions are saved in a // Block structure which records properties about the block. -func (w *Wallet) GetTransactions(startBlock, endBlock *BlockIdentifier, cancel <-chan struct{}) (*GetTransactionsResult, error) { +func (w *Wallet) GetTransactions(startBlock, endBlock *BlockIdentifier, + accountName string, cancel <-chan struct{}) (*GetTransactionsResult, error) { + var start, end int32 = 0, -1 w.chainClientLock.Lock() @@ -2508,7 +2510,7 @@ func (s creditSlice) Swap(i, j int) { // contained within it will be considered. If we know nothing about a // transaction an empty array will be returned. func (w *Wallet) ListUnspent(minconf, maxconf int32, - addresses map[string]struct{}) ([]*btcjson.ListUnspentResult, error) { + accountName string) ([]*btcjson.ListUnspentResult, error) { var results []*btcjson.ListUnspentResult err := walletdb.View(w.db, func(tx walletdb.ReadTx) error { @@ -2517,7 +2519,7 @@ func (w *Wallet) ListUnspent(minconf, maxconf int32, syncBlock := w.Manager.SyncedTo() - filter := len(addresses) != 0 + filter := accountName != "" unspent, err := w.TxStore.UnspentOutputs(txmgrNs) if err != nil { return err @@ -2556,7 +2558,7 @@ func (w *Wallet) ListUnspent(minconf, maxconf int32, // // This will be unnecessary once transactions and outputs are // grouped under the associated account in the db. - acctName := defaultAccountName + outputAcctName := defaultAccountName sc, addrs, _, err := txscript.ExtractPkScriptAddrs( output.PkScript, w.chainParams) if err != nil { @@ -2567,22 +2569,15 @@ func (w *Wallet) ListUnspent(minconf, maxconf int32, if err == nil { s, err := smgr.AccountName(addrmgrNs, acct) if err == nil { - acctName = s + outputAcctName = s } } } - if filter { - for _, addr := range addrs { - _, ok := addresses[addr.EncodeAddress()] - if ok { - goto include - } - } + if filter && outputAcctName != accountName { continue } - include: // At the moment watch-only addresses are not supported, so all // recorded outputs that are not multisig are "spendable". // Multisig outputs are only "spendable" if all keys are @@ -2622,7 +2617,7 @@ func (w *Wallet) ListUnspent(minconf, maxconf int32, result := &btcjson.ListUnspentResult{ TxID: output.OutPoint.Hash.String(), Vout: output.OutPoint.Index, - Account: acctName, + Account: outputAcctName, ScriptPubKey: hex.EncodeToString(output.PkScript), Amount: output.Amount.ToBTC(), Confirmations: int64(confs), From bbd7f8f887ca1b212d647828670a4eab3a5352e2 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 17 Feb 2021 16:24:23 -0800 Subject: [PATCH 06/10] waddrmgr+wallet: expose LookupAccount This exposes a mapping of account name to its corresponding key scope and internal account number to facilitate the use of external APIs by users. --- waddrmgr/manager.go | 19 +++++++++++++++++++ wallet/wallet.go | 16 ++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/waddrmgr/manager.go b/waddrmgr/manager.go index 6706703..2fcfbcd 100644 --- a/waddrmgr/manager.go +++ b/waddrmgr/manager.go @@ -1299,6 +1299,25 @@ func ValidateAccountName(name string) error { return nil } +// LookupAccount returns the corresponding key scope and account number for the +// account with the given name. +func (m *Manager) LookupAccount(ns walletdb.ReadBucket, name string) (KeyScope, + uint32, error) { + + m.mtx.RLock() + defer m.mtx.RUnlock() + + for keyScope, scopedMgr := range m.scopedManagers { + acct, err := scopedMgr.LookupAccount(ns, name) + if err == nil { + return keyScope, acct, nil + } + } + + str := fmt.Sprintf("account name '%s' not found", name) + return KeyScope{}, 0, managerError(ErrAccountNotFound, str, nil) +} + // selectCryptoKey selects the appropriate crypto key based on the key type. An // error is returned when an invalid key type is specified or the requested key // requires the manager to be unlocked when it isn't. diff --git a/wallet/wallet.go b/wallet/wallet.go index 9c0d8a4..c4d7d43 100644 --- a/wallet/wallet.go +++ b/wallet/wallet.go @@ -1762,6 +1762,22 @@ func (w *Wallet) AccountProperties(scope waddrmgr.KeyScope, acct uint32) (*waddr return props, err } +// LookupAccount returns the corresponding key scope and account number for the +// account with the given name. +func (w *Wallet) LookupAccount(name string) (waddrmgr.KeyScope, uint32, error) { + var ( + keyScope waddrmgr.KeyScope + account uint32 + ) + err := walletdb.View(w.db, func(tx walletdb.ReadTx) error { + ns := tx.ReadBucket(waddrmgrNamespaceKey) + var err error + keyScope, account, err = w.Manager.LookupAccount(ns, name) + return err + }) + return keyScope, account, err +} + // RenameAccount sets the name for an account number to newName. func (w *Wallet) RenameAccount(scope waddrmgr.KeyScope, account uint32, newName string) error { manager, err := w.Manager.FetchScopedKeyManager(scope) From ddbe5ecee4b90cebe9113a59dc866db30a6ff9b3 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 17 Feb 2021 16:24:25 -0800 Subject: [PATCH 07/10] wallet: expose AccountPropertiesByName --- wallet/wallet.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/wallet/wallet.go b/wallet/wallet.go index c4d7d43..549199f 100644 --- a/wallet/wallet.go +++ b/wallet/wallet.go @@ -1762,6 +1762,30 @@ func (w *Wallet) AccountProperties(scope waddrmgr.KeyScope, acct uint32) (*waddr return props, err } +// AccountPropertiesByName returns the properties of an account by its name. It +// first fetches the desynced information from the address manager, then updates +// the indexes based on the address pools. +func (w *Wallet) AccountPropertiesByName(scope waddrmgr.KeyScope, + name string) (*waddrmgr.AccountProperties, error) { + + manager, err := w.Manager.FetchScopedKeyManager(scope) + if err != nil { + return nil, err + } + + var props *waddrmgr.AccountProperties + err = walletdb.View(w.db, func(tx walletdb.ReadTx) error { + waddrmgrNs := tx.ReadBucket(waddrmgrNamespaceKey) + acct, err := manager.LookupAccount(waddrmgrNs, name) + if err != nil { + return err + } + props, err = manager.AccountProperties(waddrmgrNs, acct) + return err + }) + return props, err +} + // LookupAccount returns the corresponding key scope and account number for the // account with the given name. func (w *Wallet) LookupAccount(name string) (waddrmgr.KeyScope, uint32, error) { From b318e99f4feed9c2ed2470237aa077aa3f6dd208 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Mon, 15 Mar 2021 17:36:16 -0700 Subject: [PATCH 08/10] wallet: extend ChangeSource to support all key scopes --- cmd/sweepaccount/main.go | 13 +++++- go.mod | 1 + wallet/createtx.go | 73 +++++++++++++++++++++++++--------- wallet/psbt.go | 5 ++- wallet/txauthor/author.go | 32 +++++++++------ wallet/txauthor/author_test.go | 31 ++++++++------- wallet/txsizes/size.go | 24 ++++++++--- wallet/txsizes/size_test.go | 6 ++- 8 files changed, 131 insertions(+), 54 deletions(-) diff --git a/cmd/sweepaccount/main.go b/cmd/sweepaccount/main.go index c27ca8f..97a3841 100644 --- a/cmd/sweepaccount/main.go +++ b/cmd/sweepaccount/main.go @@ -22,6 +22,7 @@ import ( "github.com/btcsuite/btcwallet/netparams" "github.com/btcsuite/btcwallet/wallet/txauthor" "github.com/btcsuite/btcwallet/wallet/txrules" + "github.com/btcsuite/btcwallet/wallet/txsizes" "github.com/jessevdk/go-flags" ) @@ -190,14 +191,22 @@ func makeInputSource(outputs []btcjson.ListUnspentResult) txauthor.InputSource { // makeDestinationScriptSource creates a ChangeSource which is used to receive // all correlated previous input value. A non-change address is created by this // function. -func makeDestinationScriptSource(rpcClient *rpcclient.Client, accountName string) txauthor.ChangeSource { - return func() ([]byte, error) { +func makeDestinationScriptSource(rpcClient *rpcclient.Client, accountName string) *txauthor.ChangeSource { + + // GetNewAddress always returns a P2PKH address since it assumes + // BIP-0044. + newChangeScript := func() ([]byte, error) { destinationAddress, err := rpcClient.GetNewAddress(accountName) if err != nil { return nil, err } return txscript.PayToAddrScript(destinationAddress) } + + return &txauthor.ChangeSource{ + ScriptSize: txsizes.P2PKHPkScriptSize, + NewScript: newChangeScript, + } } func main() { diff --git a/go.mod b/go.mod index 82182d9..2f2df85 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( github.com/btcsuite/btcutil/psbt v1.0.3-0.20201208143702-a53e38424cce github.com/btcsuite/btcwallet/wallet/txauthor v1.0.0 github.com/btcsuite/btcwallet/wallet/txrules v1.0.0 + github.com/btcsuite/btcwallet/wallet/txsizes v1.0.0 github.com/btcsuite/btcwallet/walletdb v1.3.4 github.com/btcsuite/btcwallet/wtxmgr v1.2.0 github.com/btcsuite/websocket v0.0.0-20150119174127-31079b680792 diff --git a/wallet/createtx.go b/wallet/createtx.go index 4df8237..900be64 100644 --- a/wallet/createtx.go +++ b/wallet/createtx.go @@ -15,6 +15,7 @@ import ( "github.com/btcsuite/btcutil" "github.com/btcsuite/btcwallet/waddrmgr" "github.com/btcsuite/btcwallet/wallet/txauthor" + "github.com/btcsuite/btcwallet/wallet/txsizes" "github.com/btcsuite/btcwallet/walletdb" "github.com/btcsuite/btcwallet/wtxmgr" ) @@ -123,9 +124,12 @@ func (w *Wallet) txToOutputs(outputs []*wire.TxOut, keyScope *waddrmgr.KeyScope, } defer func() { _ = dbtx.Rollback() }() - addrmgrNs, changeSource := w.addrMgrWithChangeSource( + addrmgrNs, changeSource, err := w.addrMgrWithChangeSource( dbtx, keyScope, account, ) + if err != nil { + return nil, err + } // Get current block's height and hash. bs, err := chainClient.BlockStamp() @@ -288,25 +292,54 @@ func (w *Wallet) findEligibleOutputs(dbtx walletdb.ReadTx, } // addrMgrWithChangeSource returns the address manager bucket and a change -// source function that returns change addresses from said address manager. The -// change addresses will come from the specified key scope and account, unless -// a key scope is not specified. In that case, change addresses will always -// come from the P2WKH key scope. +// source that returns change addresses from said address manager. The change +// addresses will come from the specified key scope and account, unless a key +// scope is not specified. In that case, change addresses will always come from +// the P2WKH key scope. func (w *Wallet) addrMgrWithChangeSource(dbtx walletdb.ReadWriteTx, - changeKeyScope *waddrmgr.KeyScope, account uint32) (walletdb.ReadWriteBucket, - txauthor.ChangeSource) { + changeKeyScope *waddrmgr.KeyScope, account uint32) ( + walletdb.ReadWriteBucket, *txauthor.ChangeSource, error) { + // Determine the address type for change addresses of the given account. + if changeKeyScope == nil { + changeKeyScope = &waddrmgr.KeyScopeBIP0084 + } + addrType := waddrmgr.ScopeAddrMap[*changeKeyScope].InternalAddrType + + // It's possible for the account to have an address schema override, so + // prefer that if it exists. addrmgrNs := dbtx.ReadWriteBucket(waddrmgrNamespaceKey) - changeSource := func() ([]byte, error) { - // Derive the change output script. We'll use the default key - // scope responsible for P2WPKH addresses to do so. As a hack to - // allow spending from the imported account, change addresses - // are created from account 0. - var changeAddr btcutil.Address - var err error - if changeKeyScope == nil { - changeKeyScope = &waddrmgr.KeyScopeBIP0084 - } + scopeMgr, err := w.Manager.FetchScopedKeyManager(*changeKeyScope) + if err != nil { + return nil, nil, err + } + accountInfo, err := scopeMgr.AccountProperties(addrmgrNs, account) + if err != nil { + return nil, nil, err + } + if accountInfo.AddrSchema != nil { + addrType = accountInfo.AddrSchema.InternalAddrType + } + + // Compute the expected size of the script for the change address type. + var scriptSize int + switch addrType { + case waddrmgr.PubKeyHash: + scriptSize = txsizes.P2PKHPkScriptSize + case waddrmgr.NestedWitnessPubKey: + scriptSize = txsizes.NestedP2WPKHPkScriptSize + case waddrmgr.WitnessPubKey: + scriptSize = txsizes.P2WPKHPkScriptSize + } + + newChangeScript := func() ([]byte, error) { + // Derive the change output script. As a hack to allow spending + // from the imported account, change addresses are created from + // account 0. + var ( + changeAddr btcutil.Address + err error + ) if account == waddrmgr.ImportedAddrAccount { changeAddr, err = w.newChangeAddress( addrmgrNs, 0, *changeKeyScope, @@ -321,7 +354,11 @@ func (w *Wallet) addrMgrWithChangeSource(dbtx walletdb.ReadWriteTx, } return txscript.PayToAddrScript(changeAddr) } - return addrmgrNs, changeSource + + return addrmgrNs, &txauthor.ChangeSource{ + ScriptSize: scriptSize, + NewScript: newChangeScript, + }, nil } // validateMsgTx verifies transaction input scripts for tx. All previous output diff --git a/wallet/psbt.go b/wallet/psbt.go index 58d8f71..fc9085b 100644 --- a/wallet/psbt.go +++ b/wallet/psbt.go @@ -173,9 +173,12 @@ func (w *Wallet) FundPsbt(packet *psbt.Packet, keyScope *waddrmgr.KeyScope, if err != nil { return 0, err } - _, changeSource := w.addrMgrWithChangeSource( + _, changeSource, err := w.addrMgrWithChangeSource( dbtx, keyScope, account, ) + if err != nil { + return 0, err + } // Ask the txauthor to create a transaction with our selected // coins. This will perform fee estimation and add a change diff --git a/wallet/txauthor/author.go b/wallet/txauthor/author.go index 5290c77..dbeaa05 100644 --- a/wallet/txauthor/author.go +++ b/wallet/txauthor/author.go @@ -60,8 +60,15 @@ type AuthoredTx struct { ChangeIndex int // negative if no change } -// ChangeSource provides P2PKH change output scripts for transaction creation. -type ChangeSource func() ([]byte, error) +// ChangeSource provides change output scripts for transaction creation. +type ChangeSource struct { + // NewScript is a closure that produces unique change output scripts per + // invocation. + NewScript func() ([]byte, error) + + // ScriptSize is the size in bytes of scripts produced by `NewScript`. + ScriptSize int +} // NewUnsignedTransaction creates an unsigned transaction paying to one or more // non-change outputs. An appropriate transaction fee is included based on the @@ -84,10 +91,12 @@ type ChangeSource func() ([]byte, error) // // BUGS: Fee estimation may be off when redeeming non-compressed P2PKH outputs. func NewUnsignedTransaction(outputs []*wire.TxOut, feeRatePerKb btcutil.Amount, - fetchInputs InputSource, fetchChange ChangeSource) (*AuthoredTx, error) { + fetchInputs InputSource, changeSource *ChangeSource) (*AuthoredTx, error) { targetAmount := SumOutputValues(outputs) - estimatedSize := txsizes.EstimateVirtualSize(0, 1, 0, outputs, true) + estimatedSize := txsizes.EstimateVirtualSize( + 0, 1, 0, outputs, changeSource.ScriptSize, + ) targetFee := txrules.FeeForSerializeSize(feeRatePerKb, estimatedSize) for { @@ -115,8 +124,9 @@ func NewUnsignedTransaction(outputs []*wire.TxOut, feeRatePerKb btcutil.Amount, } } - maxSignedSize := txsizes.EstimateVirtualSize(p2pkh, p2wpkh, - nested, outputs, true) + maxSignedSize := txsizes.EstimateVirtualSize( + p2pkh, p2wpkh, nested, outputs, changeSource.ScriptSize, + ) maxRequiredFee := txrules.FeeForSerializeSize(feeRatePerKb, maxSignedSize) remainingAmount := inputAmount - targetAmount if remainingAmount < maxRequiredFee { @@ -130,18 +140,16 @@ func NewUnsignedTransaction(outputs []*wire.TxOut, feeRatePerKb btcutil.Amount, TxOut: outputs, LockTime: 0, } + changeIndex := -1 changeAmount := inputAmount - targetAmount - maxRequiredFee if changeAmount != 0 && !txrules.IsDustAmount(changeAmount, - txsizes.P2WPKHPkScriptSize, txrules.DefaultRelayFeePerKb) { - changeScript, err := fetchChange() + changeSource.ScriptSize, txrules.DefaultRelayFeePerKb) { + + changeScript, err := changeSource.NewScript() if err != nil { return nil, err } - if len(changeScript) > txsizes.P2WPKHPkScriptSize { - return nil, errors.New("fee estimation requires change " + - "scripts no larger than P2WPKH output scripts") - } change := wire.NewTxOut(int64(changeAmount), changeScript) l := len(outputs) unsignedTransaction.TxOut = append(outputs[:l:l], change) diff --git a/wallet/txauthor/author_test.go b/wallet/txauthor/author_test.go index 0217417..2100518 100644 --- a/wallet/txauthor/author_test.go +++ b/wallet/txauthor/author_test.go @@ -61,7 +61,7 @@ func TestNewUnsignedTransaction(t *testing.T) { Outputs: p2pkhOutputs(1e6), RelayFee: 1e3, ChangeAmount: 1e8 - 1e6 - txrules.FeeForSerializeSize(1e3, - txsizes.EstimateVirtualSize(1, 0, 0, p2pkhOutputs(1e6), true)), + txsizes.EstimateVirtualSize(1, 0, 0, p2pkhOutputs(1e6), txsizes.P2WPKHPkScriptSize)), InputCount: 1, }, 2: { @@ -69,7 +69,7 @@ func TestNewUnsignedTransaction(t *testing.T) { Outputs: p2pkhOutputs(1e6), RelayFee: 1e4, ChangeAmount: 1e8 - 1e6 - txrules.FeeForSerializeSize(1e4, - txsizes.EstimateVirtualSize(1, 0, 0, p2pkhOutputs(1e6), true)), + txsizes.EstimateVirtualSize(1, 0, 0, p2pkhOutputs(1e6), txsizes.P2WPKHPkScriptSize)), InputCount: 1, }, 3: { @@ -77,7 +77,7 @@ func TestNewUnsignedTransaction(t *testing.T) { Outputs: p2pkhOutputs(1e6, 1e6, 1e6), RelayFee: 1e4, ChangeAmount: 1e8 - 3e6 - txrules.FeeForSerializeSize(1e4, - txsizes.EstimateVirtualSize(1, 0, 0, p2pkhOutputs(1e6, 1e6, 1e6), true)), + txsizes.EstimateVirtualSize(1, 0, 0, p2pkhOutputs(1e6, 1e6, 1e6), txsizes.P2WPKHPkScriptSize)), InputCount: 1, }, 4: { @@ -85,7 +85,7 @@ func TestNewUnsignedTransaction(t *testing.T) { Outputs: p2pkhOutputs(1e6, 1e6, 1e6), RelayFee: 2.55e3, ChangeAmount: 1e8 - 3e6 - txrules.FeeForSerializeSize(2.55e3, - txsizes.EstimateVirtualSize(1, 0, 0, p2pkhOutputs(1e6, 1e6, 1e6), true)), + txsizes.EstimateVirtualSize(1, 0, 0, p2pkhOutputs(1e6, 1e6, 1e6), txsizes.P2WPKHPkScriptSize)), InputCount: 1, }, @@ -93,7 +93,7 @@ func TestNewUnsignedTransaction(t *testing.T) { 5: { UnspentOutputs: p2pkhOutputs(1e8), Outputs: p2pkhOutputs(1e8 - 545 - txrules.FeeForSerializeSize(1e3, - txsizes.EstimateVirtualSize(1, 0, 0, p2pkhOutputs(0), true))), + txsizes.EstimateVirtualSize(1, 0, 0, p2pkhOutputs(0), txsizes.P2WPKHPkScriptSize))), RelayFee: 1e3, ChangeAmount: 545, InputCount: 1, @@ -101,7 +101,7 @@ func TestNewUnsignedTransaction(t *testing.T) { 6: { UnspentOutputs: p2pkhOutputs(1e8), Outputs: p2pkhOutputs(1e8 - 546 - txrules.FeeForSerializeSize(1e3, - txsizes.EstimateVirtualSize(1, 0, 0, p2pkhOutputs(0), true))), + txsizes.EstimateVirtualSize(1, 0, 0, p2pkhOutputs(0), txsizes.P2WPKHPkScriptSize))), RelayFee: 1e3, ChangeAmount: 546, InputCount: 1, @@ -111,7 +111,7 @@ func TestNewUnsignedTransaction(t *testing.T) { 7: { UnspentOutputs: p2pkhOutputs(1e8), Outputs: p2pkhOutputs(1e8 - 1392 - txrules.FeeForSerializeSize(2.55e3, - txsizes.EstimateVirtualSize(1, 0, 0, p2pkhOutputs(0), true))), + txsizes.EstimateVirtualSize(1, 0, 0, p2pkhOutputs(0), txsizes.P2WPKHPkScriptSize))), RelayFee: 2.55e3, ChangeAmount: 1392, InputCount: 1, @@ -119,7 +119,7 @@ func TestNewUnsignedTransaction(t *testing.T) { 8: { UnspentOutputs: p2pkhOutputs(1e8), Outputs: p2pkhOutputs(1e8 - 1393 - txrules.FeeForSerializeSize(2.55e3, - txsizes.EstimateVirtualSize(1, 0, 0, p2pkhOutputs(0), true))), + txsizes.EstimateVirtualSize(1, 0, 0, p2pkhOutputs(0), txsizes.P2WPKHPkScriptSize))), RelayFee: 2.55e3, ChangeAmount: 1393, InputCount: 1, @@ -131,7 +131,7 @@ func TestNewUnsignedTransaction(t *testing.T) { 9: { UnspentOutputs: p2pkhOutputs(1e8, 1e8), Outputs: p2pkhOutputs(1e8 - 546 - txrules.FeeForSerializeSize(1e3, - txsizes.EstimateVirtualSize(1, 0, 0, p2pkhOutputs(0), true))), + txsizes.EstimateVirtualSize(1, 0, 0, p2pkhOutputs(0), txsizes.P2WPKHPkScriptSize))), RelayFee: 1e3, ChangeAmount: 546, InputCount: 1, @@ -145,7 +145,7 @@ func TestNewUnsignedTransaction(t *testing.T) { 10: { UnspentOutputs: p2pkhOutputs(1e8, 1e8), Outputs: p2pkhOutputs(1e8 - 545 - txrules.FeeForSerializeSize(1e3, - txsizes.EstimateVirtualSize(1, 0, 0, p2pkhOutputs(0), true))), + txsizes.EstimateVirtualSize(1, 0, 0, p2pkhOutputs(0), txsizes.P2WPKHPkScriptSize))), RelayFee: 1e3, ChangeAmount: 545, InputCount: 1, @@ -157,7 +157,7 @@ func TestNewUnsignedTransaction(t *testing.T) { Outputs: p2pkhOutputs(1e8), RelayFee: 1e3, ChangeAmount: 1e8 - txrules.FeeForSerializeSize(1e3, - txsizes.EstimateVirtualSize(2, 0, 0, p2pkhOutputs(1e8), true)), + txsizes.EstimateVirtualSize(2, 0, 0, p2pkhOutputs(1e8), txsizes.P2WPKHPkScriptSize)), InputCount: 2, }, @@ -172,9 +172,12 @@ func TestNewUnsignedTransaction(t *testing.T) { }, } - changeSource := func() ([]byte, error) { - // Only length matters for these tests. - return make([]byte, txsizes.P2WPKHPkScriptSize), nil + changeSource := &ChangeSource{ + NewScript: func() ([]byte, error) { + // Only length matters for these tests. + return make([]byte, txsizes.P2WPKHPkScriptSize), nil + }, + ScriptSize: txsizes.P2WPKHPkScriptSize, } for i, test := range tests { diff --git a/wallet/txsizes/size.go b/wallet/txsizes/size.go index da3d54c..4b47284 100644 --- a/wallet/txsizes/size.go +++ b/wallet/txsizes/size.go @@ -82,6 +82,16 @@ const ( // - 4 bytes sequence RedeemP2WPKHInputSize = 32 + 4 + 1 + RedeemP2WPKHScriptSize + 4 + // NestedP2WPKHPkScriptSize is the size of a transaction output script + // that pays to a pay-to-witness-key hash nested in P2SH (P2SH-P2WPKH). + // It is calculated as: + // + // - OP_HASH160 + // - OP_DATA_20 + // - 20 bytes script hash + // - OP_EQUAL + NestedP2WPKHPkScriptSize = 1 + 1 + 20 + 1 + // RedeemNestedP2WPKHScriptSize is the worst case size of a transaction // input script that redeems a pay-to-witness-key hash nested in P2SH // (P2SH-P2WPKH). It is calculated as: @@ -150,12 +160,14 @@ func EstimateSerializeSize(inputCount int, txOuts []*wire.TxOut, addChangeOutput // from txOuts. The estimate is incremented for an additional P2PKH // change output if addChangeOutput is true. func EstimateVirtualSize(numP2PKHIns, numP2WPKHIns, numNestedP2WPKHIns int, - txOuts []*wire.TxOut, addChangeOutput bool) int { - changeSize := 0 + txOuts []*wire.TxOut, changeScriptSize int) int { outputCount := len(txOuts) - if addChangeOutput { - // We are always using P2WPKH as change output. - changeSize = P2WPKHOutputSize + + changeOutputSize := 0 + if changeScriptSize > 0 { + changeOutputSize = 8 + + wire.VarIntSerializeSize(uint64(changeScriptSize)) + + changeScriptSize outputCount++ } @@ -170,7 +182,7 @@ func EstimateVirtualSize(numP2PKHIns, numP2WPKHIns, numNestedP2WPKHIns int, numP2WPKHIns*RedeemP2WPKHInputSize + numNestedP2WPKHIns*RedeemNestedP2WPKHInputSize + SumOutputSerializeSizes(txOuts) + - changeSize + changeOutputSize // If this transaction has any witness inputs, we must count the // witness data. diff --git a/wallet/txsizes/size_test.go b/wallet/txsizes/size_test.go index 6594c33..9f3cd0c 100644 --- a/wallet/txsizes/size_test.go +++ b/wallet/txsizes/size_test.go @@ -163,8 +163,12 @@ func TestEstimateVirtualSize(t *testing.T) { t.Fatalf("unable to get test tx: %v", err) } + changeScriptSize := 0 + if test.change { + changeScriptSize = P2WPKHPkScriptSize + } est := EstimateVirtualSize(test.p2pkhIns, test.p2wpkhIns, - test.nestedp2wpkhIns, tx.TxOut, test.change) + test.nestedp2wpkhIns, tx.TxOut, changeScriptSize) if est != test.result { t.Fatalf("expected estimated vsize to be %d, "+ From b9b4d4efe1b6830a5af83e05f1ebeffbe552dc99 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Mon, 29 Mar 2021 14:22:51 -0700 Subject: [PATCH 09/10] wallet: include redeem script for NP2WKH inputs in PSBT generation This allows external signers to properly sign NP2WKH inputs. Co-authored-by: Oliver Gugger --- wallet/psbt.go | 14 +++++- wallet/signer.go | 117 ++++++++++++++++++++++++++++------------------- 2 files changed, 82 insertions(+), 49 deletions(-) diff --git a/wallet/psbt.go b/wallet/psbt.go index fc9085b..a0617d8 100644 --- a/wallet/psbt.go +++ b/wallet/psbt.go @@ -104,9 +104,21 @@ func (w *Wallet) FundPsbt(packet *psbt.Packet, keyScope *waddrmgr.KeyScope, } // We don't want to include the witness or any script - // just yet. + // on the unsigned TX just yet. packet.UnsignedTx.TxIn[idx].Witness = wire.TxWitness{} packet.UnsignedTx.TxIn[idx].SignatureScript = nil + + // For nested P2WKH we need to add the redeem script to + // the input, otherwise an offline wallet won't be able + // to sign for it. For normal P2WKH this will be nil. + addr, witnessProgram, _, err := w.scriptForOutput(utxo) + if err != nil { + return fmt.Errorf("error fetching UTXO "+ + "script: %v", err) + } + if addr.AddrType() == waddrmgr.NestedWitnessPubKey { + packet.Inputs[idx].RedeemScript = witnessProgram + } } return nil diff --git a/wallet/signer.go b/wallet/signer.go index 390022b..db5a10e 100644 --- a/wallet/signer.go +++ b/wallet/signer.go @@ -5,6 +5,8 @@ package wallet import ( + "fmt" + "github.com/btcsuite/btcd/btcec" "github.com/btcsuite/btcd/txscript" "github.com/btcsuite/btcd/wire" @@ -12,6 +14,71 @@ import ( "github.com/btcsuite/btcwallet/waddrmgr" ) +// scriptForOutput returns the address, witness program and redeem script for a +// given UTXO. An error is returned if the UTXO does not belong to our wallet or +// it is not a managed pubKey address. +func (w *Wallet) scriptForOutput(output *wire.TxOut) ( + waddrmgr.ManagedPubKeyAddress, []byte, []byte, error) { + + // First make sure we can sign for the input by making sure the script + // in the UTXO belongs to our wallet and we have the private key for it. + walletAddr, err := w.fetchOutputAddr(output.PkScript) + if err != nil { + return nil, nil, nil, err + } + + pubKeyAddr, ok := walletAddr.(waddrmgr.ManagedPubKeyAddress) + if !ok { + return nil, nil, nil, fmt.Errorf("address %s is not a "+ + "p2wkh or np2wkh address", walletAddr.Address()) + } + + var ( + witnessProgram []byte + sigScript []byte + ) + + switch { + // If we're spending p2wkh output nested within a p2sh output, then + // we'll need to attach a sigScript in addition to witness data. + case walletAddr.AddrType() == waddrmgr.NestedWitnessPubKey: + pubKey := pubKeyAddr.PubKey() + pubKeyHash := btcutil.Hash160(pubKey.SerializeCompressed()) + + // Next, we'll generate a valid sigScript that will allow us to + // spend the p2sh output. The sigScript will contain only a + // single push of the p2wkh witness program corresponding to + // the matching public key of this address. + p2wkhAddr, err := btcutil.NewAddressWitnessPubKeyHash( + pubKeyHash, w.chainParams, + ) + if err != nil { + return nil, nil, nil, err + } + witnessProgram, err = txscript.PayToAddrScript(p2wkhAddr) + if err != nil { + return nil, nil, nil, err + } + + bldr := txscript.NewScriptBuilder() + bldr.AddData(witnessProgram) + sigScript, err = bldr.Script() + if err != nil { + return nil, nil, nil, err + } + + // Otherwise, this is a regular p2wkh output, so we include the + // witness program itself as the subscript to generate the proper + // sighash digest. As part of the new sighash digest algorithm, the + // p2wkh witness program will be expanded into a regular p2kh + // script. + default: + witnessProgram = output.PkScript + } + + return pubKeyAddr, witnessProgram, sigScript, nil +} + // PrivKeyTweaker is a function type that can be used to pass in a callback for // tweaking a private key before it's used to sign an input. type PrivKeyTweaker func(*btcec.PrivateKey) (*btcec.PrivateKey, error) @@ -25,62 +92,16 @@ func (w *Wallet) ComputeInputScript(tx *wire.MsgTx, output *wire.TxOut, hashType txscript.SigHashType, tweaker PrivKeyTweaker) (wire.TxWitness, []byte, error) { - // First make sure we can sign for the input by making sure the script - // in the UTXO belongs to our wallet and we have the private key for it. - walletAddr, err := w.fetchOutputAddr(output.PkScript) + walletAddr, witnessProgram, sigScript, err := w.scriptForOutput(output) if err != nil { return nil, nil, err } - pka := walletAddr.(waddrmgr.ManagedPubKeyAddress) - privKey, err := pka.PrivKey() + privKey, err := walletAddr.PrivKey() if err != nil { return nil, nil, err } - var ( - witnessProgram []byte - sigScript []byte - ) - - switch { - // If we're spending p2wkh output nested within a p2sh output, then - // we'll need to attach a sigScript in addition to witness data. - case pka.AddrType() == waddrmgr.NestedWitnessPubKey: - pubKey := privKey.PubKey() - pubKeyHash := btcutil.Hash160(pubKey.SerializeCompressed()) - - // Next, we'll generate a valid sigScript that will allow us to - // spend the p2sh output. The sigScript will contain only a - // single push of the p2wkh witness program corresponding to - // the matching public key of this address. - p2wkhAddr, err := btcutil.NewAddressWitnessPubKeyHash( - pubKeyHash, w.chainParams, - ) - if err != nil { - return nil, nil, err - } - witnessProgram, err = txscript.PayToAddrScript(p2wkhAddr) - if err != nil { - return nil, nil, err - } - - bldr := txscript.NewScriptBuilder() - bldr.AddData(witnessProgram) - sigScript, err = bldr.Script() - if err != nil { - return nil, nil, err - } - - // Otherwise, this is a regular p2wkh output, so we include the - // witness program itself as the subscript to generate the proper - // sighash digest. As part of the new sighash digest algorithm, the - // p2wkh witness program will be expanded into a regular p2kh - // script. - default: - witnessProgram = output.PkScript - } - // If we need to maybe tweak our private key, do it now. if tweaker != nil { privKey, err = tweaker(privKey) From 02fedd6780ab0209f913a4922c9ae9c3b97e1f31 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Mon, 29 Mar 2021 16:01:27 -0700 Subject: [PATCH 10/10] build: remove goveralls --- .travis.yml | 1 - Makefile | 11 ----------- 2 files changed, 12 deletions(-) diff --git a/.travis.yml b/.travis.yml index ff1946b..b6aced7 100644 --- a/.travis.yml +++ b/.travis.yml @@ -11,4 +11,3 @@ script: - make lint - make unit-race - make unit-cover - - make goveralls diff --git a/Makefile b/Makefile index 5c2de92..df3a936 100644 --- a/Makefile +++ b/Makefile @@ -1,12 +1,10 @@ PKG := github.com/btcsuite/btcwallet -GOVERALLS_PKG := github.com/mattn/goveralls LINT_PKG := github.com/golangci/golangci-lint/cmd/golangci-lint GOACC_PKG := github.com/ory/go-acc GOIMPORTS_PKG := golang.org/x/tools/cmd/goimports GO_BIN := ${GOPATH}/bin -GOVERALLS_BIN := $(GO_BIN)/goveralls LINT_BIN := $(GO_BIN)/golangci-lint GOACC_BIN := $(GO_BIN)/go-acc @@ -49,10 +47,6 @@ all: build check # DEPENDENCIES # ============ -$(GOVERALLS_BIN): - @$(call print, "Fetching goveralls.") - go get -u $(GOVERALLS_PKG) - $(LINT_BIN): @$(call print, "Fetching linter") $(DEPGET) $(LINT_PKG)@$(LINT_COMMIT) @@ -97,10 +91,6 @@ unit-race: @$(call print, "Running unit race tests.") env CGO_ENABLED=1 GORACE="history_size=7 halt_on_errors=1" $(GOLIST) | $(XARGS) env $(GOTEST) -race -goveralls: $(GOVERALLS_BIN) - @$(call print, "Sending coverage report.") - $(GOVERALLS_BIN) -coverprofile=coverage.txt -service=travis-ci - # ========= # UTILITIES # ========= @@ -126,7 +116,6 @@ clean: unit \ unit-cover \ unit-race \ - goveralls \ fmt \ lint \ clean