Skip to content

Remove UniValue::push_backV() and UniValue::pushKVs()

The methods for bulk-appends to UniValue arrays/objects are removed for the following performance and code quality reasons (fitting within #51):

  • In practice, this codebase abuses bulk-appends to unnecessarily create intermediate objects instead of writing directly to the target object.
  • The distinct capacity management logic of bulk-appends interferes with the amortised constant complexity guarantee of individual appends.
  • The lack of duplicate key checks in pushKVs() (contrary to pushKV()) is confusing - adding duplicate key checks to pushKVs() would even introduce a severe transaction construction bug.

To preserve the functionality of call sites, the following new overloads were added to the UniValue API:

  • Overloads of setArray() accepting std::vector<UniValue> (both copy and move).
  • Non-const overload of getObjectEntries().
  • Non-const overload of getArrayValues().

While updating the call sites, I also made some other UniValue-related changes to improve performance.

This MR is expected to improve performance of the following RPC commands:

  • listtransactions
  • getaddressinfo (uses DescribeWalletAddress)
  • validateaddress (uses DescribeAddress)
  • createrawtransaction, createpsbt, walletcreatefundedpsbt (use ConstructTransaction)

AFAIK there are no benchmarks available to measure these, but the theoretical arguments given above are convincing on their own.

Test plan: ninja bench_bitcoin check-univalue check check-functional

Edited by BigBlockIfTrue

Merge request reports