tl;dr - could be improved with batch requests but shouldn't be a blocker for this pull, current approach is very reasonable.
This would benefit from a batch request approach, albeit quite modestly. On my machine, test runtime decreased from ~0.35s to ~0.33s. Overall runtime is very short so I don't think we need to go out of our way to optimize here, but would be a best practice. I'm thinking we should add streamlined batch request handling (~send_batch_request as implemented here) on the AuthServiceProxy level so tests can use it with minimal overhead. But I guess I should just make a separate pull for that - there's probably room for improvement in other tests that iterate over single requests.
<details>
<summary>git diff on 3accc773c</summary>
diff --git a/test/functional/feature_utxo_set_hash.py b/test/functional/feature_utxo_set_hash.py
index 46a1813d4..a328d6805 100755
--- a/test/functional/feature_utxo_set_hash.py
+++ b/test/functional/feature_utxo_set_hash.py
@@ -9,13 +9,16 @@ from test_framework.messages import (
CTxOut,
)
from test_framework.muhash import MuHash3072
-from test_framework.test_framework import BitcoinTestFramework
+from test_framework.test_framework import BitcoinTestFramework, TestNode
from test_framework.util import assert_equal
from test_framework.wallet import MiniWallet
+from typing import Dict, List, Any
-def txout_serialize(txid_hex, n, height, is_coinbase, value, scriptPubKey_hex):
- data = COutPoint(int(txid_hex, 16), n).serialize()
+
+def txout_serialize(txid_hex: str, output_index: int, height: int, is_coinbase: bool, value: float,
+ scriptPubKey_hex: str) -> bytes:
+ data = COutPoint(int(txid_hex, 16), output_index).serialize()
data += (height * 2 + is_coinbase).to_bytes(4, 'little')
data += CTxOut(int(value * COIN), bytes.fromhex(scriptPubKey_hex)).serialize()
return data
@@ -43,20 +46,23 @@ class UTXOSetHashTest(BitcoinTestFramework):
# apply (add/remove) them to the MuHash object accordingly
muhash = MuHash3072()
- for height in range(1, node.getblockcount() + 1):
- for tx in node.getblock(blockhash=node.getblockhash(height), verbose=3)['tx']:
+ hashes = send_batch_request(node, "getblockhash", [[i] for i in range(1, node.getblockcount() + 1)])
+ blocks = send_batch_request(node, "getblock", [{"blockhash": h, "verbosity": 3} for h in hashes])
+ for block in blocks:
+ height = block["height"]
+ for tx in block["tx"]:
# add created coins
is_coinbase_tx = 'coinbase' in tx['vin'][0]
for o in tx['vout']:
if o['scriptPubKey']['type'] != 'nulldata':
muhash.insert(txout_serialize(tx['txid'], o['n'], height, is_coinbase_tx,
- o['value'], o['scriptPubKey']['hex']))
+ o['value'], o['scriptPubKey']['hex']))
# remove spent coins
if not is_coinbase_tx:
for i in tx['vin']:
prev = i['prevout']
muhash.remove(txout_serialize(i['txid'], i['vout'], prev['height'], prev['generated'],
- prev['value'], prev['scriptPubKey']['hex']))
+ prev['value'], prev['scriptPubKey']['hex']))
finalized = muhash.digest()
node_muhash = node.gettxoutsetinfo("muhash")['muhash']
@@ -71,5 +77,17 @@ class UTXOSetHashTest(BitcoinTestFramework):
self.test_muhash_implementation()
+def send_batch_request(node: TestNode, method: str, params: List[Any]) -> List[Any]:
+ """Send batch request and parse all results"""
+ data = [{"method": method, "params": p} for p in params]
+ response = node.batch(data)
+ result = []
+ for item in response:
+ assert item["error"] is None, item["error"]
+ result.append(item["result"])
+
+ return result
+
+
if __name__ == '__main__':
UTXOSetHashTest().main()
</details>
<details>
<summary>benchmark results</summary>
unpatched (3accc773c)
------------------------
% repeat 5 time ./test/functional/feature_utxo_set_hash.py > /dev/null
./test/functional/feature_utxo_set_hash.py > /dev/null 0.34s user 0.10s system 35% cpu 1.235 total
./test/functional/feature_utxo_set_hash.py > /dev/null 0.35s user 0.09s system 33% cpu 1.315 total
./test/functional/feature_utxo_set_hash.py > /dev/null 0.35s user 0.09s system 35% cpu 1.231 total
./test/functional/feature_utxo_set_hash.py > /dev/null 0.35s user 0.09s system 37% cpu 1.183 total
./test/functional/feature_utxo_set_hash.py > /dev/null 0.35s user 0.09s system 35% cpu 1.247 total
patch (diff above)
------------------
% repeat 5 time ./test/functional/feature_utxo_set_hash.py > /dev/null
./test/functional/feature_utxo_set_hash.py > /dev/null 0.32s user 0.08s system 28% cpu 1.406 total
./test/functional/feature_utxo_set_hash.py > /dev/null 0.33s user 0.08s system 27% cpu 1.508 total
./test/functional/feature_utxo_set_hash.py > /dev/null 0.33s user 0.08s system 34% cpu 1.207 total
./test/functional/feature_utxo_set_hash.py > /dev/null 0.32s user 0.09s system 27% cpu 1.490 total
./test/functional/feature_utxo_set_hash.py > /dev/null 0.32s user 0.08s system 33% cpu 1.211 total
</details>
