Skip to content

Commit edc9e63

Browse files
laanwjlaanwj
committed
Merge #9682: Require timestamps for importmulti keys
266a811 Use MTP for importmulti "now" timestamps (Russell Yanofsky) 3cf9917 Add test to check new importmulti "now" value (Russell Yanofsky) 442887f Require timestamps for importmulti keys (Russell Yanofsky)
2 parents ec66d06 + 266a811 commit edc9e63

File tree

4 files changed

+77
-13
lines changed

4 files changed

+77
-13
lines changed

‎qa/rpc-tests/import-rescan.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ def call_import_rpc(call, data, address, scriptPubKey, pubkey, key, label, node,
3333
"scriptPubKey": {
3434
"address": address
3535
},
36+
"timestamp": "now",
3637
"pubkeys": [pubkey] if data == Data.pub else [],
3738
"keys": [key] if data == Data.priv else [],
3839
"label": label,

‎qa/rpc-tests/importmulti.py

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@ def run_test (self):
5252
result = self.nodes[1].importmulti([{
5353
"scriptPubKey": {
5454
"address": address['address']
55-
}
55+
},
56+
"timestamp": "now",
5657
}])
5758
assert_equal(result[0]['success'], True)
5859
address_assert = self.nodes[1].validateaddress(address['address'])
@@ -65,6 +66,7 @@ def run_test (self):
6566
address = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
6667
result = self.nodes[1].importmulti([{
6768
"scriptPubKey": address['scriptPubKey'],
69+
"timestamp": "now",
6870
"internal": True
6971
}])
7072
assert_equal(result[0]['success'], True)
@@ -76,7 +78,8 @@ def run_test (self):
7678
print("Should not import a scriptPubKey without internal flag")
7779
address = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
7880
result = self.nodes[1].importmulti([{
79-
"scriptPubKey": address['scriptPubKey']
81+
"scriptPubKey": address['scriptPubKey'],
82+
"timestamp": "now",
8083
}])
8184
assert_equal(result[0]['success'], False)
8285
assert_equal(result[0]['error']['code'], -8)
@@ -93,6 +96,7 @@ def run_test (self):
9396
"scriptPubKey": {
9497
"address": address['address']
9598
},
99+
"timestamp": "now",
96100
"pubkeys": [ address['pubkey'] ]
97101
}])
98102
assert_equal(result[0]['success'], True)
@@ -106,6 +110,7 @@ def run_test (self):
106110
address = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
107111
request = [{
108112
"scriptPubKey": address['scriptPubKey'],
113+
"timestamp": "now",
109114
"pubkeys": [ address['pubkey'] ],
110115
"internal": True
111116
}]
@@ -120,6 +125,7 @@ def run_test (self):
120125
address = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
121126
request = [{
122127
"scriptPubKey": address['scriptPubKey'],
128+
"timestamp": "now",
123129
"pubkeys": [ address['pubkey'] ]
124130
}]
125131
result = self.nodes[1].importmulti(request)
@@ -133,16 +139,19 @@ def run_test (self):
133139
# Address + Private key + !watchonly
134140
print("Should import an address with private key")
135141
address = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
142+
timestamp = self.nodes[1].getblock(self.nodes[1].getbestblockhash())['mediantime']
136143
result = self.nodes[1].importmulti([{
137144
"scriptPubKey": {
138145
"address": address['address']
139146
},
147+
"timestamp": "now",
140148
"keys": [ self.nodes[0].dumpprivkey(address['address']) ]
141149
}])
142150
assert_equal(result[0]['success'], True)
143151
address_assert = self.nodes[1].validateaddress(address['address'])
144152
assert_equal(address_assert['iswatchonly'], False)
145153
assert_equal(address_assert['ismine'], True)
154+
assert_equal(address_assert['timestamp'], timestamp)
146155

147156
# Address + Private key + watchonly
148157
print("Should not import an address with private key and with watchonly")
@@ -151,6 +160,7 @@ def run_test (self):
151160
"scriptPubKey": {
152161
"address": address['address']
153162
},
163+
"timestamp": "now",
154164
"keys": [ self.nodes[0].dumpprivkey(address['address']) ],
155165
"watchonly": True
156166
}])
@@ -166,6 +176,7 @@ def run_test (self):
166176
address = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
167177
result = self.nodes[1].importmulti([{
168178
"scriptPubKey": address['scriptPubKey'],
179+
"timestamp": "now",
169180
"keys": [ self.nodes[0].dumpprivkey(address['address']) ],
170181
"internal": True
171182
}])
@@ -179,6 +190,7 @@ def run_test (self):
179190
address = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
180191
result = self.nodes[1].importmulti([{
181192
"scriptPubKey": address['scriptPubKey'],
193+
"timestamp": "now",
182194
"keys": [ self.nodes[0].dumpprivkey(address['address']) ]
183195
}])
184196
assert_equal(result[0]['success'], False)
@@ -203,7 +215,8 @@ def run_test (self):
203215
result = self.nodes[1].importmulti([{
204216
"scriptPubKey": {
205217
"address": multi_sig_script['address']
206-
}
218+
},
219+
"timestamp": "now",
207220
}])
208221
assert_equal(result[0]['success'], True)
209222
address_assert = self.nodes[1].validateaddress(multi_sig_script['address'])
@@ -229,6 +242,7 @@ def run_test (self):
229242
"scriptPubKey": {
230243
"address": multi_sig_script['address']
231244
},
245+
"timestamp": "now",
232246
"redeemscript": multi_sig_script['redeemScript']
233247
}])
234248
assert_equal(result[0]['success'], True)
@@ -253,6 +267,7 @@ def run_test (self):
253267
"scriptPubKey": {
254268
"address": multi_sig_script['address']
255269
},
270+
"timestamp": "now",
256271
"redeemscript": multi_sig_script['redeemScript'],
257272
"keys": [ self.nodes[0].dumpprivkey(sig_address_1['address']), self.nodes[0].dumpprivkey(sig_address_2['address'])]
258273
}])
@@ -277,6 +292,7 @@ def run_test (self):
277292
"scriptPubKey": {
278293
"address": multi_sig_script['address']
279294
},
295+
"timestamp": "now",
280296
"redeemscript": multi_sig_script['redeemScript'],
281297
"keys": [ self.nodes[0].dumpprivkey(sig_address_1['address']), self.nodes[0].dumpprivkey(sig_address_2['address'])],
282298
"watchonly": True
@@ -294,6 +310,7 @@ def run_test (self):
294310
"scriptPubKey": {
295311
"address": address['address']
296312
},
313+
"timestamp": "now",
297314
"pubkeys": [ address2['pubkey'] ]
298315
}])
299316
assert_equal(result[0]['success'], False)
@@ -310,6 +327,7 @@ def run_test (self):
310327
address2 = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
311328
request = [{
312329
"scriptPubKey": address['scriptPubKey'],
330+
"timestamp": "now",
313331
"pubkeys": [ address2['pubkey'] ],
314332
"internal": True
315333
}]
@@ -330,6 +348,7 @@ def run_test (self):
330348
"scriptPubKey": {
331349
"address": address['address']
332350
},
351+
"timestamp": "now",
333352
"keys": [ self.nodes[0].dumpprivkey(address2['address']) ]
334353
}])
335354
assert_equal(result[0]['success'], False)
@@ -346,6 +365,7 @@ def run_test (self):
346365
address2 = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
347366
result = self.nodes[1].importmulti([{
348367
"scriptPubKey": address['scriptPubKey'],
368+
"timestamp": "now",
349369
"keys": [ self.nodes[0].dumpprivkey(address2['address']) ],
350370
"internal": True
351371
}])
@@ -356,5 +376,18 @@ def run_test (self):
356376
assert_equal(address_assert['iswatchonly'], False)
357377
assert_equal(address_assert['ismine'], False)
358378

379+
# Bad or missing timestamps
380+
print("Should throw on invalid or missing timestamp values")
381+
assert_raises_message(JSONRPCException, 'Missing required timestamp field for key',
382+
self.nodes[1].importmulti, [{
383+
"scriptPubKey": address['scriptPubKey'],
384+
}])
385+
assert_raises_message(JSONRPCException, 'Expected number or "now" timestamp value for key. got type string',
386+
self.nodes[1].importmulti, [{
387+
"scriptPubKey": address['scriptPubKey'],
388+
"timestamp": "",
389+
}])
390+
391+
359392
if __name__ == '__main__':
360393
ImportMultiTest ().main ()

‎src/rpc/misc.cpp

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,7 @@ UniValue validateaddress(const JSONRPCRequest& request)
167167
" \"pubkey\" : \"publickeyhex\", (string) The hex value of the raw public key\n"
168168
" \"iscompressed\" : true|false, (boolean) If the address is compressed\n"
169169
" \"account\" : \"account\" (string) DEPRECATED. The account associated with the address, \"\" is the default account\n"
170+
" \"timestamp\" : timestamp, (number, optional) The creation time of the key if available in seconds since epoch (Jan 1 1970 GMT)\n"
170171
" \"hdkeypath\" : \"keypath\" (string, optional) The HD keypath if the key is HD and available\n"
171172
" \"hdmasterkeyid\" : \"<hash160>\" (string, optional) The Hash160 of the HD master pubkey\n"
172173
"}\n"
@@ -204,10 +205,16 @@ UniValue validateaddress(const JSONRPCRequest& request)
204205
if (pwalletMain && pwalletMain->mapAddressBook.count(dest))
205206
ret.push_back(Pair("account", pwalletMain->mapAddressBook[dest].name));
206207
CKeyID keyID;
207-
if (pwalletMain && address.GetKeyID(keyID) && pwalletMain->mapKeyMetadata.count(keyID) && !pwalletMain->mapKeyMetadata[keyID].hdKeypath.empty())
208-
{
209-
ret.push_back(Pair("hdkeypath", pwalletMain->mapKeyMetadata[keyID].hdKeypath));
210-
ret.push_back(Pair("hdmasterkeyid", pwalletMain->mapKeyMetadata[keyID].hdMasterKeyID.GetHex()));
208+
if (pwalletMain) {
209+
const auto& meta = pwalletMain->mapKeyMetadata;
210+
auto it = address.GetKeyID(keyID) ? meta.find(keyID) : meta.end();
211+
if (it != meta.end()) {
212+
ret.push_back(Pair("timestamp", it->second.nCreateTime));
213+
if (!it->second.hdKeypath.empty()) {
214+
ret.push_back(Pair("hdkeypath", it->second.hdKeypath));
215+
ret.push_back(Pair("hdmasterkeyid", it->second.hdMasterKeyID.GetHex()));
216+
}
217+
}
211218
}
212219
#endif
213220
}

‎src/wallet/rpcdump.cpp

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -640,7 +640,8 @@ UniValue dumpwallet(const JSONRPCRequest& request)
640640
}
641641

642642

643-
UniValue processImport(const UniValue& data) {
643+
UniValue ProcessImport(const UniValue& data, const int64_t timestamp)
644+
{
644645
try {
645646
bool success = false;
646647

@@ -659,7 +660,6 @@ UniValue processImport(const UniValue& data) {
659660
const bool& internal = data.exists("internal") ? data["internal"].get_bool() : false;
660661
const bool& watchOnly = data.exists("watchonly") ? data["watchonly"].get_bool() : false;
661662
const string& label = data.exists("label") && !internal ? data["label"].get_str() : "";
662-
const int64_t& timestamp = data.exists("timestamp") && data["timestamp"].get_int64() > 1 ? data["timestamp"].get_int64() : 1;
663663

664664
bool isScript = scriptPubKey.getType() == UniValue::VSTR;
665665
bool isP2SH = strRedeemScript.length() > 0;
@@ -958,6 +958,20 @@ UniValue processImport(const UniValue& data) {
958958
}
959959
}
960960

961+
int64_t GetImportTimestamp(const UniValue& data, int64_t now)
962+
{
963+
if (data.exists("timestamp")) {
964+
const UniValue& timestamp = data["timestamp"];
965+
if (timestamp.isNum()) {
966+
return timestamp.get_int64();
967+
} else if (timestamp.isStr() && timestamp.get_str() == "now") {
968+
return now;
969+
}
970+
throw JSONRPCError(RPC_TYPE_ERROR, strprintf("Expected number or \"now\" timestamp value for key. got type %s", uvTypeName(timestamp.type())));
971+
}
972+
throw JSONRPCError(RPC_TYPE_ERROR, "Missing required timestamp field for key");
973+
}
974+
961975
UniValue importmulti(const JSONRPCRequest& mainRequest)
962976
{
963977
// clang-format off
@@ -970,13 +984,17 @@ UniValue importmulti(const JSONRPCRequest& mainRequest)
970984
" [ (array of json objects)\n"
971985
" {\n"
972986
" \"scriptPubKey\": \"<script>\" | { \"address\":\"<address>\" }, (string / json, required) Type of scriptPubKey (string for script, json for address)\n"
987+
" \"timestamp\": timestamp | \"now\" , (integer / string, required) Creation time of the key in seconds since epoch (Jan 1 1970 GMT),\n"
988+
" or the string \"now\" to substitute the current synced blockchain time. The timestamp of the oldest\n"
989+
" key will determine how far back blockchain rescans need to begin for missing wallet transactions.\n"
990+
" \"now\" can be specified to bypass scanning, for keys which are known to never have been used, and\n"
991+
" 0 can be specified to scan the entire blockchain.\n"
973992
" \"redeemscript\": \"<script>\" , (string, optional) Allowed only if the scriptPubKey is a P2SH address or a P2SH scriptPubKey\n"
974993
" \"pubkeys\": [\"<pubKey>\", ... ] , (array, optional) Array of strings giving pubkeys that must occur in the output or redeemscript\n"
975994
" \"keys\": [\"<key>\", ... ] , (array, optional) Array of strings giving private keys whose corresponding public keys must occur in the output or redeemscript\n"
976995
" \"internal\": <true> , (boolean, optional, default: false) Stating whether matching outputs should be be treated as not incoming payments\n"
977996
" \"watchonly\": <true> , (boolean, optional, default: false) Stating whether matching outputs should be considered watched even when they're not spendable, only allowed if keys are empty\n"
978997
" \"label\": <label> , (string, optional, default: '') Label to assign to the address (aka account name, for now), only allowed with internal=false\n"
979-
" \"timestamp\": 1454686740, (integer, optional, default now) Timestamp\n"
980998
" }\n"
981999
" ,...\n"
9821000
" ]\n"
@@ -1015,6 +1033,12 @@ UniValue importmulti(const JSONRPCRequest& mainRequest)
10151033
LOCK2(cs_main, pwalletMain->cs_wallet);
10161034
EnsureWalletIsUnlocked();
10171035

1036+
// Verify all timestamps are present before importing any keys.
1037+
const int64_t now = chainActive.Tip() ? chainActive.Tip()->GetMedianTimePast() : 0;
1038+
for (const UniValue& data : requests.getValues()) {
1039+
GetImportTimestamp(data, now);
1040+
}
1041+
10181042
bool fRunScan = false;
10191043
const int64_t minimumTimestamp = 1;
10201044
int64_t nLowestTimestamp = 0;
@@ -1028,7 +1052,8 @@ UniValue importmulti(const JSONRPCRequest& mainRequest)
10281052
UniValue response(UniValue::VARR);
10291053

10301054
BOOST_FOREACH (const UniValue& data, requests.getValues()) {
1031-
const UniValue result = processImport(data);
1055+
const int64_t timestamp = std::max(GetImportTimestamp(data, now), minimumTimestamp);
1056+
const UniValue result = ProcessImport(data, timestamp);
10321057
response.push_back(result);
10331058

10341059
if (!fRescan) {
@@ -1041,8 +1066,6 @@ UniValue importmulti(const JSONRPCRequest& mainRequest)
10411066
}
10421067

10431068
// Get the lowest timestamp.
1044-
const int64_t& timestamp = data.exists("timestamp") && data["timestamp"].get_int64() > minimumTimestamp ? data["timestamp"].get_int64() : minimumTimestamp;
1045-
10461069
if (timestamp < nLowestTimestamp) {
10471070
nLowestTimestamp = timestamp;
10481071
}

0 commit comments

Comments
 (0)