Merge pull request lightningnetwork#5266 from wpaulino/import-dry-run

walletrpc: expose dry run support for ImportAccount

cmd/lncli/walletrpc_active.go

@@ -4,7 +4,6 @@ package main
 
 import (
 	"encoding/base64"
-	"encoding/binary"
 	"encoding/hex"
 	"encoding/json"
 	"errors"
@@ -966,6 +965,10 @@ var importAccountCommand = cli.Command{
 				"(derivation path m/) corresponding to the " +
 				"account public key",
 		},
+		cli.BoolFlag{
+			Name:  "dry_run",
+			Usage: "(optional) perform a dry run",
+		},
 	},
 	Action: actionDecorator(importAccount),
 }
@@ -975,7 +978,7 @@ func importAccount(ctx *cli.Context) error {
 
 	// Display the command's help message if we do not have the expected
 	// number of arguments/flags.
-	if ctx.NArg() != 2 || ctx.NumFlags() > 2 {
+	if ctx.NArg() != 2 || ctx.NumFlags() > 3 {
 		return cli.ShowCommandHelp(ctx, "import")
 	}
 
@@ -984,23 +987,26 @@ func importAccount(ctx *cli.Context) error {
 		return err
 	}
 
-	var masterKeyFingerprint uint32
+	var mkfpBytes []byte
 	if ctx.IsSet("master_key_fingerprint") {
-		mkfp, err := hex.DecodeString(ctx.String("master_key_fingerprint"))
+		mkfpBytes, err = hex.DecodeString(
+			ctx.String("master_key_fingerprint"),
+		)
 		if err != nil {
 			return fmt.Errorf("invalid master key fingerprint: %v", err)
 		}
-		masterKeyFingerprint = binary.LittleEndian.Uint32(mkfp)
 	}
 
 	walletClient, cleanUp := getWalletClient(ctx)
 	defer cleanUp()
 
+	dryRun := ctx.Bool("dry_run")
 	req := &walletrpc.ImportAccountRequest{
 		Name:                 ctx.Args().Get(1),
 		ExtendedPublicKey:    ctx.Args().Get(0),
-		MasterKeyFingerprint: masterKeyFingerprint,
+		MasterKeyFingerprint: mkfpBytes,
 		AddressType:          addrType,
+		DryRun:               dryRun,
 	}
 	resp, err := walletClient.ImportAccount(ctxc, req)
 	if err != nil {

go.mod

@@ -9,7 +9,7 @@ require (
 	github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f
 	github.com/btcsuite/btcutil v1.0.3-0.20201208143702-a53e38424cce
 	github.com/btcsuite/btcutil/psbt v1.0.3-0.20201208143702-a53e38424cce
-	github.com/btcsuite/btcwallet v0.11.1-0.20210429224804-a7a9234968e8
+	github.com/btcsuite/btcwallet v0.11.1-0.20210507012717-82fa030bdad6
 	github.com/btcsuite/btcwallet/wallet/txauthor v1.0.1-0.20210329233242-e0607006dce6
 	github.com/btcsuite/btcwallet/wallet/txrules v1.0.0
 	github.com/btcsuite/btcwallet/wallet/txsizes v1.0.1-0.20210329233242-e0607006dce6 // indirect

go.sum

@@ -37,8 +37,8 @@ github.com/btcsuite/btcutil v1.0.3-0.20201208143702-a53e38424cce h1:YtWJF7RHm2pY
 github.com/btcsuite/btcutil v1.0.3-0.20201208143702-a53e38424cce/go.mod h1:0DVlHczLPewLcPGEIeUEzfOJhqGPQ0mJJRDBtD307+o=
 github.com/btcsuite/btcutil/psbt v1.0.3-0.20201208143702-a53e38424cce h1:3PRwz+js0AMMV1fHRrCdQ55akoomx4Q3ulozHC3BDDY=
 github.com/btcsuite/btcutil/psbt v1.0.3-0.20201208143702-a53e38424cce/go.mod h1:LVveMu4VaNSkIRTZu2+ut0HDBRuYjqGocxDMNS1KuGQ=
-github.com/btcsuite/btcwallet v0.11.1-0.20210429224804-a7a9234968e8 h1:e+sUdZhAtb40Rp0vmJQ6xBdBoi7KFn3Mg6h+SGpCrMU=
-github.com/btcsuite/btcwallet v0.11.1-0.20210429224804-a7a9234968e8/go.mod h1:vpYHloeKzEe1/g+301F73iOJrcAaXQWqKaU9Rmu0HRw=
+github.com/btcsuite/btcwallet v0.11.1-0.20210507012717-82fa030bdad6 h1:t874udi1ajYJ4AM26EVh0sEdf4chfcH/gLgSMCungVM=
+github.com/btcsuite/btcwallet v0.11.1-0.20210507012717-82fa030bdad6/go.mod h1:vpYHloeKzEe1/g+301F73iOJrcAaXQWqKaU9Rmu0HRw=
 github.com/btcsuite/btcwallet/wallet/txauthor v1.0.0/go.mod h1:VufDts7bd/zs3GV13f/lXc/0lXrPnvxD/NvmpG/FEKU=
 github.com/btcsuite/btcwallet/wallet/txauthor v1.0.1-0.20210329233242-e0607006dce6 h1:mO7NxcfgLe75paLDHx+LWNG5BskiDQigHnSVT2KvNZA=
 github.com/btcsuite/btcwallet/wallet/txauthor v1.0.1-0.20210329233242-e0607006dce6/go.mod h1:VufDts7bd/zs3GV13f/lXc/0lXrPnvxD/NvmpG/FEKU=

lnrpc/walletrpc/walletkit.proto

@@ -324,9 +324,10 @@ message Account {
     /*
     The fingerprint of the root key from which the account public key was
     derived from. This will always be zero for the default imported account in
-    which single public keys are imported into.
+    which single public keys are imported into. The bytes are in big-endian
+    order.
     */
-    uint32 master_key_fingerprint = 4;
+    bytes master_key_fingerprint = 4;
 
     /*
     The derivation path corresponding to the account public key. This will
@@ -377,18 +378,44 @@ message ImportAccountRequest {
     /*
     The fingerprint of the root key (also known as the key with derivation path
     m/) from which the account public key was derived from. This may be required
-    by some hardware wallets for proper identification and signing.
+    by some hardware wallets for proper identification and signing. The bytes
+    must be in big-endian order.
     */
-    uint32 master_key_fingerprint = 3;
+    bytes master_key_fingerprint = 3;
 
     /*
     An address type is only required when the extended account public key has a
     legacy version (xpub, tpub, etc.), such that the wallet cannot detect what
     address scheme it belongs to.
     */
     AddressType address_type = 4;
+
+    /*
+    Whether a dry run should be attempted when importing the account. This
+    serves as a way to confirm whether the account is being imported correctly
+    by returning the first N addresses for the external and internal branches of
+    the account. If these addresses match as expected, then it should be safe to
+    import the account as is.
+    */
+    bool dry_run = 5;
 }
 message ImportAccountResponse {
+    // The details of the imported account.
+    Account account = 1;
+
+    /*
+    The first N addresses that belong to the external branch of the account.
+    The external branch is typically used for external non-change addresses.
+    These are only returned if a dry run was specified within the request.
+    */
+    repeated string dry_run_external_addrs = 2;
+
+    /*
+    The first N addresses that belong to the internal branch of the account.
+    The internal branch is typically used for change addresses. These are only
+    returned if a dry run was specified within the request.
+    */
+    repeated string dry_run_internal_addrs = 3;
 }
 
 message ImportPublicKeyRequest {

lnrpc/walletrpc/walletkit.swagger.json

@@ -861,9 +861,9 @@
           "description": "The public key backing the account that all keys are derived from\nrepresented as an extended key. This will always be empty for the default\nimported account in which single public keys are imported into."
         },
         "master_key_fingerprint": {
-          "type": "integer",
-          "format": "int64",
-          "description": "The fingerprint of the root key from which the account public key was\nderived from. This will always be zero for the default imported account in\nwhich single public keys are imported into."
+          "type": "string",
+          "format": "byte",
+          "description": "The fingerprint of the root key from which the account public key was\nderived from. This will always be zero for the default imported account in\nwhich single public keys are imported into. The bytes are in big-endian\norder."
         },
         "derivation_path": {
           "type": "string",
@@ -1057,18 +1057,43 @@
           "description": "A public key that corresponds to a wallet account represented as an extended\nkey. It must conform to a derivation path of the form\nm/purpose'/coin_type'/account'."
         },
         "master_key_fingerprint": {
-          "type": "integer",
-          "format": "int64",
-          "description": "The fingerprint of the root key (also known as the key with derivation path\nm/) from which the account public key was derived from. This may be required\nby some hardware wallets for proper identification and signing."
+          "type": "string",
+          "format": "byte",
+          "description": "The fingerprint of the root key (also known as the key with derivation path\nm/) from which the account public key was derived from. This may be required\nby some hardware wallets for proper identification and signing. The bytes\nmust be in big-endian order."
         },
         "address_type": {
           "$ref": "#/definitions/walletrpcAddressType",
           "description": "An address type is only required when the extended account public key has a\nlegacy version (xpub, tpub, etc.), such that the wallet cannot detect what\naddress scheme it belongs to."
+        },
+        "dry_run": {
+          "type": "boolean",
+          "format": "boolean",
+          "description": "Whether a dry run should be attempted when importing the account. This\nserves as a way to confirm whether the account is being imported correctly\nby returning the first N addresses for the external and internal branches of\nthe account. If these addresses match as expected, then it should be safe to\nimport the account as is."
         }
       }
     },
     "walletrpcImportAccountResponse": {
-      "type": "object"
+      "type": "object",
+      "properties": {
+        "account": {
+          "$ref": "#/definitions/walletrpcAccount",
+          "description": "The details of the imported account."
+        },
+        "dry_run_external_addrs": {
+          "type": "array",
+          "items": {
+            "type": "string"
+          },
+          "description": "The first N addresses that belong to the external branch of the account.\nThe external branch is typically used for external non-change addresses.\nThese are only returned if a dry run was specified within the request."
+        },
+        "dry_run_internal_addrs": {
+          "type": "array",
+          "items": {
+            "type": "string"
+          },
+          "description": "The first N addresses that belong to the internal branch of the account.\nThe internal branch is typically used for change addresses. These are only\nreturned if a dry run was specified within the request."
+        }
+      }
     },
     "walletrpcImportPublicKeyRequest": {
       "type": "object",

lnrpc/walletrpc/walletkit_server.go

@@ -5,6 +5,7 @@ package walletrpc
 import (
 	"bytes"
 	"context"
+	"encoding/binary"
 	"errors"
 	"fmt"
 	"io/ioutil"
@@ -1253,8 +1254,8 @@ func marshalWalletAccount(account *waddrmgr.AccountProperties) (*Account, error)
 			break
 		}
 
-		switch account.AddrSchema {
-		case &waddrmgr.KeyScopeBIP0049AddrSchema:
+		switch *account.AddrSchema {
+		case waddrmgr.KeyScopeBIP0049AddrSchema:
 			addrType = AddressType_NESTED_WITNESS_PUBKEY_HASH
 		default:
 			return nil, fmt.Errorf("unsupported address schema %v",
@@ -1283,7 +1284,13 @@ func marshalWalletAccount(account *waddrmgr.AccountProperties) (*Account, error)
 		nonHardenedIndex := account.AccountPubKey.ChildIndex() -
 			hdkeychain.HardenedKeyStart
 		rpcAccount.ExtendedPublicKey = account.AccountPubKey.String()
-		rpcAccount.MasterKeyFingerprint = account.MasterKeyFingerprint
+		if account.MasterKeyFingerprint != 0 {
+			var mkfp [4]byte
+			binary.BigEndian.PutUint32(
+				mkfp[:], account.MasterKeyFingerprint,
+			)
+			rpcAccount.MasterKeyFingerprint = mkfp[:]
+		}
 		rpcAccount.DerivationPath = fmt.Sprintf("%v/%v'",
 			account.KeyScope, nonHardenedIndex)
 	}
@@ -1396,19 +1403,52 @@ func (w *WalletKit) ImportAccount(ctx context.Context,
 	if err != nil {
 		return nil, err
 	}
+
+	var mkfp uint32
+	switch len(req.MasterKeyFingerprint) {
+	// No master key fingerprint provided, which is fine as it's not
+	// required.
+	case 0:
+	// Expected length.
+	case 4:
+		mkfp = binary.BigEndian.Uint32(req.MasterKeyFingerprint)
+	default:
+		return nil, errors.New("invalid length for master key " +
+			"fingerprint, expected 4 bytes in big-endian")
+	}
+
 	addrType, err := parseAddrType(req.AddressType, false)
 	if err != nil {
 		return nil, err
 	}
 
-	err = w.cfg.Wallet.ImportAccount(
-		req.Name, accountPubKey, req.MasterKeyFingerprint, addrType,
+	accountProps, extAddrs, intAddrs, err := w.cfg.Wallet.ImportAccount(
+		req.Name, accountPubKey, mkfp, addrType, req.DryRun,
 	)
 	if err != nil {
 		return nil, err
 	}
 
-	return &ImportAccountResponse{}, nil
+	rpcAccount, err := marshalWalletAccount(accountProps)
+	if err != nil {
+		return nil, err
+	}
+
+	resp := &ImportAccountResponse{Account: rpcAccount}
+	if !req.DryRun {
+		return resp, nil
+	}
+
+	resp.DryRunExternalAddrs = make([]string, len(extAddrs))
+	for i := 0; i < len(extAddrs); i++ {
+		resp.DryRunExternalAddrs[i] = extAddrs[i].String()
+	}
+	resp.DryRunInternalAddrs = make([]string, len(intAddrs))
+	for i := 0; i < len(intAddrs); i++ {
+		resp.DryRunInternalAddrs[i] = intAddrs[i].String()
+	}
+
+	return resp, nil
 }
 
 // ImportPublicKey imports a single derived public key into the wallet. The

lntest/mock/walletcontroller.go

@@ -88,8 +88,9 @@ func (w *WalletController) ListAccounts(_ string,
 
 // ImportAccount currently returns a dummy value.
 func (w *WalletController) ImportAccount(string, *hdkeychain.ExtendedKey,
-	uint32, *waddrmgr.AddressType) error {
-	return nil
+	uint32, *waddrmgr.AddressType, bool) (*waddrmgr.AccountProperties,
+	[]btcutil.Address, []btcutil.Address, error) {
+	return nil, nil, nil, nil
 }
 
 // ImportPublicKey currently returns a dummy value.

lnwallet/btcwallet/btcwallet.go

@@ -36,6 +36,11 @@ const (
 	defaultAccount  = uint32(waddrmgr.DefaultAccountNum)
 	importedAccount = uint32(waddrmgr.ImportedAddrAccount)
 
+	// dryRunImportAccountNumAddrs represents the number of addresses we'll
+	// derive for an imported account's external and internal branch when a
+	// dry run is attempted.
+	dryRunImportAccountNumAddrs = 5
+
 	// UnconfirmedHeight is the special case end height that is used to
 	// obtain unconfirmed transactions from ListTransactionDetails.
 	UnconfirmedHeight int32 = -1
@@ -566,12 +571,42 @@ func (b *BtcWallet) ListAccounts(name string,
 //
 // This is a part of the WalletController interface.
 func (b *BtcWallet) ImportAccount(name string, accountPubKey *hdkeychain.ExtendedKey,
-	masterKeyFingerprint uint32, addrType *waddrmgr.AddressType) error {
+	masterKeyFingerprint uint32, addrType *waddrmgr.AddressType,
+	dryRun bool) (*waddrmgr.AccountProperties, []btcutil.Address,
+	[]btcutil.Address, error) {
 
-	_, err := b.wallet.ImportAccount(
+	if !dryRun {
+		accountProps, err := b.wallet.ImportAccount(
+			name, accountPubKey, masterKeyFingerprint, addrType,
+		)
+		if err != nil {
+			return nil, nil, nil, err
+		}
+		return accountProps, nil, nil, nil
+	}
+
+	// Derive addresses from both the external and internal branches of the
+	// account. There's no risk of address inflation as this is only done
+	// for dry runs.
+	accountProps, extAddrs, intAddrs, err := b.wallet.ImportAccountDryRun(
 		name, accountPubKey, masterKeyFingerprint, addrType,
+		dryRunImportAccountNumAddrs,
 	)
-	return err
+	if err != nil {
+		return nil, nil, nil, err
+	}
+
+	externalAddrs := make([]btcutil.Address, len(extAddrs))
+	for i := 0; i < len(extAddrs); i++ {
+		externalAddrs[i] = extAddrs[i].Address()
+	}
+
+	internalAddrs := make([]btcutil.Address, len(intAddrs))
+	for i := 0; i < len(intAddrs); i++ {
+		internalAddrs[i] = intAddrs[i].Address()
+	}
+
+	return accountProps, externalAddrs, internalAddrs, nil
 }
 
 // ImportPublicKey imports a single derived public key into the wallet. The

lnwallet/interface.go

@@ -219,7 +219,9 @@ type WalletController interface {
 	// witness pubkeys everywhere) and our own BIP-0049Plus address schema
 	// (nested pubkeys externally, witness pubkeys internally).
 	ImportAccount(name string, accountPubKey *hdkeychain.ExtendedKey,
-		masterKeyFingerprint uint32, addrType *waddrmgr.AddressType) error
+		masterKeyFingerprint uint32, addrType *waddrmgr.AddressType,
+		dryRun bool) (*waddrmgr.AccountProperties, []btcutil.Address,
+		[]btcutil.Address, error)
 
 	// ImportPublicKey imports a single derived public key into the wallet.
 	// The address type can usually be inferred from the key's version, but