waddrmgr+wallet: support transaction creation and signing for watch-only accounts #733
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Roasbeef
requested changes
Feb 25, 2021
There was a problem hiding this comment.
One step closer! The diff reads well, but I'm having a hard time wrapping my head around exactly what an account uint32 means in various areas of the codebase now. Perhaps we should introduce a new term like accountID that refers to the auto incrementing account number within a given scoped key manager?
guggero
approved these changes
Feb 26, 2021
waddrmgr/scoped_manager.go
Outdated
| @@ -342,7 +340,9 @@ func (s *ScopedKeyManager) loadAccountInfo(ns walletdb.ReadBucket, | |||
| nextInternalIndex: row.nextInternalIndex, | |||
| } | |||
|
|
|||
| if !s.rootManager.isLocked() { | |||
| watchOnly := s.rootManager.WatchOnly() || len(acctInfo.acctKeyEncrypted) == 0 | |||
There was a problem hiding this comment.
I think we have to be careful where/when we call the exported (WatchOnly, IsLocked) vs the internal 'watchOnly(), isLocked()) methods since the exported ones try to acquire a read lock. I noticed that there is a deadlock when trying to use wallet.DumpPrivKeys() for example. Pretty sure that's pre-existing, but should at least be careful with new code.
There was a problem hiding this comment.
Thanks for the pointer, I fixed a case I introduced.
|
Needs a rebase! |
|
I've tested this with the diff --git a/np2wkh.patch b/np2wkh.patch
index 36e92d95..e69de29b 100644
--- a/np2wkh.patch
+++ b/np2wkh.patch
@@ -1,147 +0,0 @@
-diff --git a/wallet/psbt.go b/wallet/psbt.go
-index fc9085b4..a0617d82 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 390022bf..881c8510 100644
---- a/wallet/signer.go
-+++ b/wallet/signer.go
-@@ -5,6 +5,7 @@
- package wallet
-
- import (
-+ "fmt"
- "github.com/btcsuite/btcd/btcec"
- "github.com/btcsuite/btcd/txscript"
- "github.com/btcsuite/btcd/wire"
-@@ -12,30 +13,24 @@ import (
- "github.com/btcsuite/btcwallet/waddrmgr"
- )
-
--// 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)
--
--// ComputeInputScript generates a complete InputScript for the passed
--// transaction with the signature as defined within the passed SignDescriptor.
--// This method is capable of generating the proper input script for both
--// regular p2wkh output and p2wkh outputs nested within a regular p2sh output.
--func (w *Wallet) ComputeInputScript(tx *wire.MsgTx, output *wire.TxOut,
-- inputIndex int, sigHashes *txscript.TxSigHashes,
-- hashType txscript.SigHashType, tweaker PrivKeyTweaker) (wire.TxWitness,
-- []byte, error) {
-+// 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, err
-+ return nil, nil, nil, err
- }
-
-- pka := walletAddr.(waddrmgr.ManagedPubKeyAddress)
-- privKey, err := pka.PrivKey()
-- if err != nil {
-- return 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 (
-@@ -46,8 +41,8 @@ func (w *Wallet) ComputeInputScript(tx *wire.MsgTx, output *wire.TxOut,
- 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()
-+ 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
-@@ -58,18 +53,18 @@ func (w *Wallet) ComputeInputScript(tx *wire.MsgTx, output *wire.TxOut,
- pubKeyHash, w.chainParams,
- )
- if err != nil {
-- return nil, nil, err
-+ return nil, nil, nil, err
- }
- witnessProgram, err = txscript.PayToAddrScript(p2wkhAddr)
- if err != nil {
-- return nil, nil, err
-+ return nil, nil, nil, err
- }
-
- bldr := txscript.NewScriptBuilder()
- bldr.AddData(witnessProgram)
- sigScript, err = bldr.Script()
- if err != nil {
-- return nil, nil, err
-+ return nil, nil, nil, err
- }
-
- // Otherwise, this is a regular p2wkh output, so we include the
-@@ -81,6 +76,32 @@ func (w *Wallet) ComputeInputScript(tx *wire.MsgTx, output *wire.TxOut,
- 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)
-+
-+// ComputeInputScript generates a complete InputScript for the passed
-+// transaction with the signature as defined within the passed SignDescriptor.
-+// This method is capable of generating the proper input script for both
-+// regular p2wkh output and p2wkh outputs nested within a regular p2sh output.
-+func (w *Wallet) ComputeInputScript(tx *wire.MsgTx, output *wire.TxOut,
-+ inputIndex int, sigHashes *txscript.TxSigHashes,
-+ hashType txscript.SigHashType, tweaker PrivKeyTweaker) (wire.TxWitness,
-+ []byte, error) {
-+
-+ walletAddr, witnessProgram, sigScript, err := w.scriptForOutput(output)
-+ if err != nil {
-+ return nil, nil, err
-+ }
-+
-+ privKey, err := walletAddr.PrivKey()
-+ if err != nil {
-+ return nil, nil, err
-+ }
-+
- // If we need to maybe tweak our private key, do it now.
- if tweaker != nil {
- privKey, err = tweaker(privKey)
diff --git a/wallet/psbt.go b/wallet/psbt.go
index fc9085b4..a0617d82 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 390022bf..33de184d 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,30 +14,24 @@ import (
"github.com/btcsuite/btcwallet/waddrmgr"
)
-// 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)
-
-// ComputeInputScript generates a complete InputScript for the passed
-// transaction with the signature as defined within the passed SignDescriptor.
-// This method is capable of generating the proper input script for both
-// regular p2wkh output and p2wkh outputs nested within a regular p2sh output.
-func (w *Wallet) ComputeInputScript(tx *wire.MsgTx, output *wire.TxOut,
- inputIndex int, sigHashes *txscript.TxSigHashes,
- hashType txscript.SigHashType, tweaker PrivKeyTweaker) (wire.TxWitness,
- []byte, error) {
+// 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, err
+ return nil, nil, nil, err
}
- pka := walletAddr.(waddrmgr.ManagedPubKeyAddress)
- privKey, err := pka.PrivKey()
- if err != nil {
- return 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 (
@@ -46,8 +42,8 @@ func (w *Wallet) ComputeInputScript(tx *wire.MsgTx, output *wire.TxOut,
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()
+ 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
@@ -58,18 +54,18 @@ func (w *Wallet) ComputeInputScript(tx *wire.MsgTx, output *wire.TxOut,
pubKeyHash, w.chainParams,
)
if err != nil {
- return nil, nil, err
+ return nil, nil, nil, err
}
witnessProgram, err = txscript.PayToAddrScript(p2wkhAddr)
if err != nil {
- return nil, nil, err
+ return nil, nil, nil, err
}
bldr := txscript.NewScriptBuilder()
bldr.AddData(witnessProgram)
sigScript, err = bldr.Script()
if err != nil {
- return nil, nil, err
+ return nil, nil, nil, err
}
// Otherwise, this is a regular p2wkh output, so we include the
@@ -81,6 +77,32 @@ func (w *Wallet) ComputeInputScript(tx *wire.MsgTx, output *wire.TxOut,
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)
+
+// ComputeInputScript generates a complete InputScript for the passed
+// transaction with the signature as defined within the passed SignDescriptor.
+// This method is capable of generating the proper input script for both
+// regular p2wkh output and p2wkh outputs nested within a regular p2sh output.
+func (w *Wallet) ComputeInputScript(tx *wire.MsgTx, output *wire.TxOut,
+ inputIndex int, sigHashes *txscript.TxSigHashes,
+ hashType txscript.SigHashType, tweaker PrivKeyTweaker) (wire.TxWitness,
+ []byte, error) {
+
+ walletAddr, witnessProgram, sigScript, err := w.scriptForOutput(output)
+ if err != nil {
+ return nil, nil, err
+ }
+
+ privKey, err := walletAddr.PrivKey()
+ if err != nil {
+ return nil, nil, err
+ }
+
// If we need to maybe tweak our private key, do it now.
if tweaker != nil {
privKey, err = tweaker(privKey) |
|
Nice find @guggero! |
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.
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.
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.
Watch-only accounts don't have any type of private key information stored, so we avoid populating input signatures in those cases.
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.
This allows external signers to properly sign NP2WKH inputs. Co-authored-by: Oliver Gugger <[email protected]>
Roasbeef
approved these changes
Mar 29, 2021
| maxSignedSize := txsizes.EstimateVirtualSize(p2pkh, p2wpkh, | ||
| nested, outputs, true) | ||
| maxSignedSize := txsizes.EstimateVirtualSize( | ||
| p2pkh, p2wpkh, nested, outputs, changeSource.ScriptSize, |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR aims to address the missing gaps for watch-only accounts to actually be used in the context of transaction creation and signing. It also includes some minor changes and surfaces some additional account information that will serve useful for higher level applications and their use of watch-only accounts.
Depends on #732.