rpc: Improve scantxoutset response and help message

promag commented at 4:18 pm on June 25, 2019: member

The new response keys height and bestblock allow the client to know at what point the scan took place.

The help message now has all the response keys (result and txouts were missing) and it’s improved a bit. Note that searched_items key is renamed to txouts, considering scantxoutset is marked experimental.

in src/rpc/blockchain.cpp:2306 in 5002e87cbc outdated

2302             LOCK(cs_main);
2303             ::ChainstateActive().ForceFlushStateToDisk();
2304             pcursor = std::unique_ptr<CCoinsViewCursor>(pcoinsdbview->Cursor());
2305-            assert(pcursor);
2306+            tip = ::ChainActive().Tip();
2307+            assert(pcursor && tip);

MarcoFalke commented at 4:22 pm on June 25, 2019:

Could leave them in separate lines, so that an assert failure is more verbose and helpful?

promag commented at 9:41 pm on June 25, 2019:

Done.

DrahtBot added the label RPC/REST/ZMQ on Jun 25, 2019

DrahtBot added the label Tests on Jun 25, 2019

promag force-pushed on Jun 25, 2019

promag commented at 8:52 pm on July 2, 2019: member

@jonasschnelli mind taking a look?

in src/rpc/blockchain.cpp:2301 in e36bb00f85 outdated

2296@@ -2293,15 +2297,20 @@ UniValue scantxoutset(const JSONRPCRequest& request)
2297         g_scan_progress = 0;
2298         int64_t count = 0;
2299         std::unique_ptr<CCoinsViewCursor> pcursor;
2300+        CBlockIndex* tip;
2301         {

fqlx commented at 11:07 pm on July 5, 2019:

Can you remove unused curly braces

promag commented at 11:15 pm on July 5, 2019:

The block is required for cs_main scoped lock.

fqlx commented at 11:39 pm on July 5, 2019:

Can you show me this documentation? I’m not familiar with it

sipa commented at 11:44 pm on July 5, 2019:

You cannot access variables which are shared across threads without grabbing the correct lock. ::ChainstateActive accesses a global, which is protected by cs_main.

in src/rpc/blockchain.cpp:2306 in e36bb00f85 outdated

2301         {
2302             LOCK(cs_main);
2303             ::ChainstateActive().ForceFlushStateToDisk();
2304             pcursor = std::unique_ptr<CCoinsViewCursor>(pcoinsdbview->Cursor());
2305             assert(pcursor);
2306+            tip = ::ChainActive().Tip();

fqlx commented at 11:08 pm on July 5, 2019:

I’d just declare and initialize tip here instead of at the top and same with pcursor

fqlx changes_requested

laanwj commented at 2:40 pm on July 8, 2019: member

Concept ACK

in src/rpc/blockchain.cpp:2312 in e36bb00f85 outdated

2308         }
2309         bool res = FindScriptPubKey(g_scan_progress, g_should_abort_scan, count, pcursor.get(), needles, coins);
2310         result.pushKV("success", res);
2311-        result.pushKV("searched_items", count);
2312+        result.pushKV("txouts", count);
2313+        result.pushKV("height", tip->nHeight);

jonasschnelli commented at 6:18 am on August 26, 2019:

nit: maybe tipheight or bestblockheight? But maybe height is also okay.

jonasschnelli approved

jonasschnelli commented at 6:19 am on August 26, 2019: contributor

utACK e36bb00f85f0f26063433c112ef675141b88ba56

in src/rpc/blockchain.cpp:2219 in e36bb00f85 outdated

2222             "   ,...], \n"
2223-            " \"total_amount\" : x.xxx,          (numeric) The total amount of all found unspent outputs in " + CURRENCY_UNIT + "\n"
2224+            "  \"total_amount\": x.xxx,          (numeric) The total amount of all found unspent outputs in " + CURRENCY_UNIT + "\n"
2225             "]\n"
2226                 },
2227                 RPCExamples{""},

jonatack commented at 11:14 pm on September 8, 2019:

Would be good to add one or two bitcoin-cli examples here.

promag commented at 11:52 pm on September 8, 2019:

What do you suggest?

in src/rpc/blockchain.cpp:2210 in e36bb00f85 outdated

2212-            "    \"amount\" : x.xxx,             (numeric) The total amount in " + CURRENCY_UNIT + " of the unspent output\n"
2213-            "    \"height\" : n,                 (numeric) Height of the unspent transaction output\n"
2214+            "   {\n"
2215+            "    \"txid\": \"hash\",              (string) The transaction id\n"
2216+            "    \"vout\": n,                   (numeric) the vout value\n"
2217+            "    \"scriptPubKey\": \"script\",    (string) the script key\n"

jonatack commented at 11:15 pm on September 8, 2019:

In lines 2209-2210, s/the/The/

in src/rpc/blockchain.cpp:2215 in e36bb00f85 outdated

2217+            "    \"scriptPubKey\": \"script\",    (string) the script key\n"
2218+            "    \"desc\": \"descriptor\",        (string) A specialized descriptor for the matched scriptPubKey\n"
2219+            "    \"amount\": x.xxx,             (numeric) The total amount in " + CURRENCY_UNIT + " of the unspent output\n"
2220+            "    \"height\": n,                 (numeric) Height of the unspent transaction output\n"
2221             "   }\n"
2222             "   ,...], \n"

jonatack commented at 11:17 pm on September 8, 2019:

Trailing space can be removed here between the comma and \n

jonatack commented at 11:27 pm on September 8, 2019: member

ACK e36bb00 modulo nits. Code review, compiled/ran help, inspected a few of the scantxoutset outputs in the functional test. Concrete CLI examples in the help would be nice to have.

rpc: Improve scantxoutset response and help message fc0c410d6e

qa: Check scantxoutset result against gettxoutsetinfo bdd6a4fd5d

promag force-pushed on Sep 8, 2019

laanwj commented at 6:08 am on September 9, 2019: member

ACK bdd6a4fd5da44c2575be9195ecb4213a13e74511

I think this is enough of an improvement in itself, examples can be added later.

laanwj referenced this in commit 08bb4c3156 on Sep 9, 2019

laanwj merged this on Sep 9, 2019

laanwj closed this on Sep 9, 2019

promag deleted the branch on Sep 9, 2019

jonatack commented at 9:20 am on September 9, 2019: member

I think this is enough of an improvement in itself, examples can be added later.

Yes. I might have a look at adding examples.

jasonbcox referenced this in commit 3e29d52fc3 on Sep 8, 2020

laanwj referenced this in commit 6ac8c4f700 on Dec 7, 2021

sidhujag referenced this in commit f6c0957beb on Dec 7, 2021

MarcoFalke locked this on Dec 16, 2021

rpc: Improve scantxoutset response and help message #16285