Skip to content

Commit f6ed748

Browse files
committed
Add SegWit support to importmulti with some ProcessImport cleanup
1 parent 2a2cac7 commit f6ed748

File tree

2 files changed

+124
-157
lines changed

2 files changed

+124
-157
lines changed

src/wallet/rpcdump.cpp

Lines changed: 118 additions & 151 deletions
Original file line numberDiff line numberDiff line change
@@ -809,29 +809,24 @@ UniValue dumpwallet(const JSONRPCRequest& request)
809809
static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)
810810
{
811811
try {
812-
bool success = false;
813-
814-
// Required fields.
812+
// First ensure scriptPubKey has either a script or JSON with "address" string
815813
const UniValue& scriptPubKey = data["scriptPubKey"];
816-
817-
// Should have script or JSON with "address".
818-
if (!(scriptPubKey.getType() == UniValue::VOBJ && scriptPubKey.exists("address")) && !(scriptPubKey.getType() == UniValue::VSTR)) {
819-
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid scriptPubKey");
814+
bool isScript = scriptPubKey.getType() == UniValue::VSTR;
815+
if (!isScript && !(scriptPubKey.getType() == UniValue::VOBJ && scriptPubKey.exists("address"))) {
816+
throw JSONRPCError(RPC_INVALID_PARAMETER, "scriptPubKey must be string with script or JSON with address string");
820817
}
818+
const std::string& output = isScript ? scriptPubKey.get_str() : scriptPubKey["address"].get_str();
821819

822820
// Optional fields.
823821
const std::string& strRedeemScript = data.exists("redeemscript") ? data["redeemscript"].get_str() : "";
822+
const std::string& witness_script_hex = data.exists("witnessscript") ? data["witnessscript"].get_str() : "";
824823
const UniValue& pubKeys = data.exists("pubkeys") ? data["pubkeys"].get_array() : UniValue();
825824
const UniValue& keys = data.exists("keys") ? data["keys"].get_array() : UniValue();
826825
const bool internal = data.exists("internal") ? data["internal"].get_bool() : false;
827826
const bool watchOnly = data.exists("watchonly") ? data["watchonly"].get_bool() : false;
828-
const std::string& label = data.exists("label") && !internal ? data["label"].get_str() : "";
829-
830-
bool isScript = scriptPubKey.getType() == UniValue::VSTR;
831-
bool isP2SH = strRedeemScript.length() > 0;
832-
const std::string& output = isScript ? scriptPubKey.get_str() : scriptPubKey["address"].get_str();
827+
const std::string& label = data.exists("label") ? data["label"].get_str() : "";
833828

834-
// Parse the output.
829+
// Generate the script and destination for the scriptPubKey provided
835830
CScript script;
836831
CTxDestination dest;
837832

@@ -855,35 +850,38 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con
855850

856851
// Watchonly and private keys
857852
if (watchOnly && keys.size()) {
858-
throw JSONRPCError(RPC_INVALID_PARAMETER, "Incompatibility found between watchonly and keys");
853+
throw JSONRPCError(RPC_INVALID_PARAMETER, "Watch-only addresses should not include private keys");
859854
}
860855

861-
// Internal + Label
856+
// Internal addresses should not have a label
862857
if (internal && data.exists("label")) {
863-
throw JSONRPCError(RPC_INVALID_PARAMETER, "Incompatibility found between internal and label");
858+
throw JSONRPCError(RPC_INVALID_PARAMETER, "Internal addresses should not have a label");
864859
}
865860

866-
// Keys / PubKeys size check.
867-
if (!isP2SH && (keys.size() > 1 || pubKeys.size() > 1)) { // Address / scriptPubKey
868-
throw JSONRPCError(RPC_INVALID_PARAMETER, "More than private key given for one address");
861+
// Force users to provide the witness script in its field rather than redeemscript
862+
if (!strRedeemScript.empty() && script.IsPayToWitnessScriptHash()) {
863+
throw JSONRPCError(RPC_INVALID_PARAMETER, "P2WSH addresses have an empty redeemscript. Please provide the witnessscript instead.");
869864
}
870865

871-
// Invalid P2SH redeemScript
872-
if (isP2SH && !IsHex(strRedeemScript)) {
873-
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid redeem script");
874-
}
875-
876-
// Process. //
866+
CScript scriptpubkey_script = script;
867+
CTxDestination scriptpubkey_dest = dest;
868+
bool allow_p2wpkh = true;
877869

878870
// P2SH
879-
if (isP2SH) {
871+
if (!strRedeemScript.empty() && script.IsPayToScriptHash()) {
872+
// Check the redeemScript is valid
873+
if (!IsHex(strRedeemScript)) {
874+
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid redeem script: must be hex string");
875+
}
876+
880877
// Import redeem script.
881878
std::vector<unsigned char> vData(ParseHex(strRedeemScript));
882879
CScript redeemScript = CScript(vData.begin(), vData.end());
880+
CScriptID redeem_id(redeemScript);
883881

884-
// Invalid P2SH address
885-
if (!script.IsPayToScriptHash()) {
886-
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid P2SH address / script");
882+
// Check that the redeemScript and scriptPubKey match
883+
if (GetScriptForDestination(redeem_id) != script) {
884+
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "The redeemScript does not match the scriptPubKey");
887885
}
888886

889887
pwallet->MarkDirty();
@@ -892,103 +890,83 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con
892890
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");
893891
}
894892

895-
CScriptID redeem_id(redeemScript);
896893
if (!pwallet->HaveCScript(redeem_id) && !pwallet->AddCScript(redeemScript)) {
897894
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding p2sh redeemScript to wallet");
898895
}
899896

900-
CScript redeemDestination = GetScriptForDestination(redeem_id);
897+
// Now set script to the redeemScript so we parse the inner script as P2WSH or P2WPKH below
898+
script = redeemScript;
899+
ExtractDestination(script, dest);
900+
}
901901

902-
if (::IsMine(*pwallet, redeemDestination) == ISMINE_SPENDABLE) {
903-
throw JSONRPCError(RPC_WALLET_ERROR, "The wallet already contains the private key for this address or script");
902+
// (P2SH-)P2WSH
903+
if (!witness_script_hex.empty() && script.IsPayToWitnessScriptHash()) {
904+
if (!IsHex(witness_script_hex)) {
905+
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid witness script: must be hex string");
904906
}
905907

906-
pwallet->MarkDirty();
908+
// Generate the scripts
909+
std::vector<unsigned char> witness_script_parsed(ParseHex(witness_script_hex));
910+
CScript witness_script = CScript(witness_script_parsed.begin(), witness_script_parsed.end());
911+
CScriptID witness_id(witness_script);
907912

908-
if (!pwallet->AddWatchOnly(redeemDestination, timestamp)) {
909-
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");
913+
// Check that the witnessScript and scriptPubKey match
914+
if (GetScriptForDestination(WitnessV0ScriptHash(witness_script)) != script) {
915+
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "The witnessScript does not match the scriptPubKey or redeemScript");
910916
}
911917

912-
// add to address book or update label
913-
if (IsValidDestination(dest)) {
914-
pwallet->SetAddressBook(dest, label, "receive");
918+
// Add the witness script as watch only only if it is not for P2SH-P2WSH
919+
if (!scriptpubkey_script.IsPayToScriptHash() && !pwallet->AddWatchOnly(witness_script, timestamp)) {
920+
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");
915921
}
916922

917-
// Import private keys.
918-
if (keys.size()) {
919-
for (size_t i = 0; i < keys.size(); i++) {
920-
const std::string& privkey = keys[i].get_str();
921-
922-
CKey key = DecodeSecret(privkey);
923-
924-
if (!key.IsValid()) {
925-
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key encoding");
926-
}
927-
928-
CPubKey pubkey = key.GetPubKey();
929-
assert(key.VerifyPubKey(pubkey));
930-
931-
CKeyID vchAddress = pubkey.GetID();
932-
pwallet->MarkDirty();
933-
pwallet->SetAddressBook(vchAddress, label, "receive");
934-
935-
if (pwallet->HaveKey(vchAddress)) {
936-
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Already have this key");
937-
}
938-
939-
pwallet->mapKeyMetadata[vchAddress].nCreateTime = timestamp;
923+
if (!pwallet->HaveCScript(witness_id) && !pwallet->AddCScript(witness_script)) {
924+
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding p2wsh witnessScript to wallet");
925+
}
940926

941-
if (!pwallet->AddKeyPubKey(key, pubkey)) {
942-
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding key to wallet");
943-
}
927+
// Now set script to the witnessScript so we parse the inner script as P2PK or P2PKH below
928+
script = witness_script;
929+
ExtractDestination(script, dest);
930+
allow_p2wpkh = false; // P2WPKH cannot be embedded in P2WSH
931+
}
944932

945-
pwallet->UpdateTimeFirstKey(timestamp);
946-
}
933+
// (P2SH-)P2PK/P2PKH/P2WPKH
934+
if (dest.type() == typeid(CKeyID) || dest.type() == typeid(WitnessV0KeyHash)) {
935+
if (!allow_p2wpkh && dest.type() == typeid(WitnessV0KeyHash)) {
936+
throw JSONRPCError(RPC_INVALID_PARAMETER, "P2WPKH cannot be embedded in P2WSH");
947937
}
948-
949-
success = true;
950-
} else {
951-
// Import public keys.
952-
if (pubKeys.size() && keys.size() == 0) {
938+
if (keys.size() > 1 || pubKeys.size() > 1) {
939+
throw JSONRPCError(RPC_INVALID_PARAMETER, "More than one key given for one single-key address");
940+
}
941+
CPubKey pubkey;
942+
if (keys.size()) {
943+
pubkey = DecodeSecret(keys[0].get_str()).GetPubKey();
944+
}
945+
if (pubKeys.size()) {
953946
const std::string& strPubKey = pubKeys[0].get_str();
954-
955947
if (!IsHex(strPubKey)) {
956948
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey must be a hex string");
957949
}
958-
959-
std::vector<unsigned char> vData(ParseHex(strPubKey));
960-
CPubKey pubKey(vData.begin(), vData.end());
961-
962-
if (!pubKey.IsFullyValid()) {
963-
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey is not a valid public key");
964-
}
965-
966-
CTxDestination pubkey_dest = pubKey.GetID();
967-
968-
// Consistency check.
969-
if (!(pubkey_dest == dest)) {
970-
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Consistency check failed");
971-
}
972-
973-
CScript pubKeyScript = GetScriptForDestination(pubkey_dest);
974-
975-
if (::IsMine(*pwallet, pubKeyScript) == ISMINE_SPENDABLE) {
976-
throw JSONRPCError(RPC_WALLET_ERROR, "The wallet already contains the private key for this address or script");
950+
std::vector<unsigned char> vData(ParseHex(pubKeys[0].get_str()));
951+
CPubKey pubkey_temp(vData.begin(), vData.end());
952+
if (pubkey.size() && pubkey_temp != pubkey) {
953+
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Private key does not match public key for address");
977954
}
978-
979-
pwallet->MarkDirty();
980-
981-
if (!pwallet->AddWatchOnly(pubKeyScript, timestamp)) {
982-
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");
955+
pubkey = pubkey_temp;
956+
}
957+
if (pubkey.size() > 0) {
958+
if (!pubkey.IsFullyValid()) {
959+
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey is not a valid public key");
983960
}
984961

985-
// add to address book or update label
986-
if (IsValidDestination(pubkey_dest)) {
987-
pwallet->SetAddressBook(pubkey_dest, label, "receive");
962+
// Check the key corresponds to the destination given
963+
std::vector<CTxDestination> destinations = GetAllDestinationsForKey(pubkey);
964+
if (std::find(destinations.begin(), destinations.end(), dest) == destinations.end()) {
965+
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Key does not match address destination");
988966
}
989967

990-
// TODO Is this necessary?
991-
CScript scriptRawPubKey = GetScriptForRawPubKey(pubKey);
968+
// This is necessary to force the wallet to import the pubKey
969+
CScript scriptRawPubKey = GetScriptForRawPubKey(pubkey);
992970

993971
if (::IsMine(*pwallet, scriptRawPubKey) == ISMINE_SPENDABLE) {
994972
throw JSONRPCError(RPC_WALLET_ERROR, "The wallet already contains the private key for this address or script");
@@ -999,73 +977,61 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con
999977
if (!pwallet->AddWatchOnly(scriptRawPubKey, timestamp)) {
1000978
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");
1001979
}
1002-
1003-
success = true;
1004980
}
981+
}
1005982

1006-
// Import private keys.
1007-
if (keys.size()) {
1008-
const std::string& strPrivkey = keys[0].get_str();
1009-
1010-
// Checks.
1011-
CKey key = DecodeSecret(strPrivkey);
1012-
1013-
if (!key.IsValid()) {
1014-
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key encoding");
1015-
}
1016-
1017-
CPubKey pubKey = key.GetPubKey();
1018-
assert(key.VerifyPubKey(pubKey));
1019-
1020-
CTxDestination pubkey_dest = pubKey.GetID();
983+
// Import the address
984+
if (::IsMine(*pwallet, scriptpubkey_script) == ISMINE_SPENDABLE) {
985+
throw JSONRPCError(RPC_WALLET_ERROR, "The wallet already contains the private key for this address or script");
986+
}
1021987

1022-
// Consistency check.
1023-
if (!(pubkey_dest == dest)) {
1024-
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Consistency check failed");
1025-
}
988+
pwallet->MarkDirty();
1026989

1027-
CKeyID vchAddress = pubKey.GetID();
1028-
pwallet->MarkDirty();
1029-
pwallet->SetAddressBook(vchAddress, label, "receive");
990+
if (!pwallet->AddWatchOnly(scriptpubkey_script, timestamp)) {
991+
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");
992+
}
1030993

1031-
if (pwallet->HaveKey(vchAddress)) {
1032-
throw JSONRPCError(RPC_WALLET_ERROR, "The wallet already contains the private key for this address or script");
1033-
}
994+
if (!watchOnly && !pwallet->HaveCScript(CScriptID(scriptpubkey_script)) && !pwallet->AddCScript(scriptpubkey_script)) {
995+
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding scriptPubKey script to wallet");
996+
}
1034997

1035-
pwallet->mapKeyMetadata[vchAddress].nCreateTime = timestamp;
998+
// add to address book or update label
999+
if (IsValidDestination(scriptpubkey_dest)) {
1000+
pwallet->SetAddressBook(scriptpubkey_dest, label, "receive");
1001+
}
10361002

1037-
if (!pwallet->AddKeyPubKey(key, pubKey)) {
1038-
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding key to wallet");
1039-
}
1003+
// Import private keys.
1004+
for (size_t i = 0; i < keys.size(); i++) {
1005+
const std::string& strPrivkey = keys[i].get_str();
10401006

1041-
pwallet->UpdateTimeFirstKey(timestamp);
1007+
// Checks.
1008+
CKey key = DecodeSecret(strPrivkey);
10421009

1043-
success = true;
1010+
if (!key.IsValid()) {
1011+
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key encoding");
10441012
}
10451013

1046-
// Import scriptPubKey only.
1047-
if (pubKeys.size() == 0 && keys.size() == 0) {
1048-
if (::IsMine(*pwallet, script) == ISMINE_SPENDABLE) {
1049-
throw JSONRPCError(RPC_WALLET_ERROR, "The wallet already contains the private key for this address or script");
1050-
}
1014+
CPubKey pubKey = key.GetPubKey();
1015+
assert(key.VerifyPubKey(pubKey));
10511016

1052-
pwallet->MarkDirty();
1017+
CKeyID vchAddress = pubKey.GetID();
1018+
pwallet->MarkDirty();
10531019

1054-
if (!pwallet->AddWatchOnly(script, timestamp)) {
1055-
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");
1056-
}
1020+
if (pwallet->HaveKey(vchAddress)) {
1021+
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Already have this key");
1022+
}
10571023

1058-
// add to address book or update label
1059-
if (IsValidDestination(dest)) {
1060-
pwallet->SetAddressBook(dest, label, "receive");
1061-
}
1024+
pwallet->mapKeyMetadata[vchAddress].nCreateTime = timestamp;
10621025

1063-
success = true;
1026+
if (!pwallet->AddKeyPubKey(key, pubKey)) {
1027+
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding key to wallet");
10641028
}
1029+
1030+
pwallet->UpdateTimeFirstKey(timestamp);
10651031
}
10661032

10671033
UniValue result = UniValue(UniValue::VOBJ);
1068-
result.pushKV("success", UniValue(success));
1034+
result.pushKV("success", UniValue(true));
10691035
return result;
10701036
} catch (const UniValue& e) {
10711037
UniValue result = UniValue(UniValue::VOBJ);
@@ -1118,7 +1084,8 @@ UniValue importmulti(const JSONRPCRequest& mainRequest)
11181084
" \"now\" can be specified to bypass scanning, for keys which are known to never have been used, and\n"
11191085
" 0 can be specified to scan the entire blockchain. Blocks up to 2 hours before the earliest key\n"
11201086
" creation time of all keys being imported by the importmulti call will be scanned.\n"
1121-
" \"redeemscript\": \"<script>\" , (string, optional) Allowed only if the scriptPubKey is a P2SH address or a P2SH scriptPubKey\n"
1087+
" \"redeemscript\": \"<script>\" , (string, optional) Allowed only if the scriptPubKey is a P2SH or P2SH-P2WSH address/scriptPubKey\n"
1088+
" \"witnessscript\": \"<script>\" , (string, optional) Allowed only if the scriptPubKey is a P2SH-P2WSH or P2WSH address/scriptPubKey\n"
11221089
" \"pubkeys\": [\"<pubKey>\", ... ] , (array, optional) Array of strings giving pubkeys that must occur in the output or redeemscript\n"
11231090
" \"keys\": [\"<key>\", ... ] , (array, optional) Array of strings giving private keys whose corresponding public keys must occur in the output or redeemscript\n"
11241091
" \"internal\": <true> , (boolean, optional, default: false) Stating whether matching outputs should be treated as not incoming payments\n"

0 commit comments

Comments
 (0)