This PR removes almost all of the TxToJSON() and ScriptPubKeyToJSON() code from bitcoin-rpc and places it in bitcoin-common, removing about 80 lines of duplicate code. The only part that can’t be moved into bitcoin-common is parsing block contextual information (confirmations and blocktime), which are not available to bitcoin-common code.
This PR should be merged along with #8817 , which ensures that witness data gets returned from calls to TxToUniv()
151@@ -152,6 +152,8 @@ void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry)
152 {
153 entry.pushKV("txid", tx.GetHash().GetHex());
154 entry.pushKV("version", tx.nVersion);
155+ entry.pushKV("size", (int)::GetSerializeSize(tx, SER_NETWORK, PROTOCOL_VERSION));
156+ entry.pushKV("vsize", (GetTransactionWeight(tx) + WITNESS_SCALE_FACTOR - 1) / WITNESS_SCALE_FACTOR);
::GetVirtualTransactionSize(tx)?
GetVirtualTransactionSize() is declared in policy.h, which isn’t included in core_write.
Why is GetVirtualTransactionSize() in policy? Because transaction virtual size isn’t a consensus rule, and is only used for mempool admittance/miner selection. On top of that, after #8365 , virtual size has been overloaded to mean both transaction weight/4 and an adjusted virtual size taking account of sigops cost.
I could update core_write.cpp to include policy, but I don’t think it’s a good idea to include policy more widely than necessary.
152@@ -153,6 +153,8 @@ void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry)
153 entry.pushKV("txid", tx.GetHash().GetHex());
154 entry.pushKV("hash", tx.GetWitnessHash().GetHex());
155 entry.pushKV("version", tx.nVersion);
156+ entry.pushKV("size", (int)::GetSerializeSize(tx, SER_NETWORK, PROTOCOL_VERSION));
157+ entry.pushKV("vsize", (GetTransactionWeight(tx) + WITNESS_SCALE_FACTOR - 1) / WITNESS_SCALE_FACTOR);
std::max(GetTransactionWeight(tx), nSigOpCost * nBytesPerSigOp)?
198@@ -198,6 +199,8 @@ extern std::string HelpRequiringPassphrase();
199 extern std::string HelpExampleCli(const std::string& methodname, const std::string& args);
200 extern std::string HelpExampleRpc(const std::string& methodname, const std::string& args);
201
202+extern void TxToJSON(const CTransaction& tx, const uint256 hashBlock, UniValue& entry);
It looks like this got entangled with a test change?
here’s a rebased version: https://github.com/laanwj/bitcoin/tree/2017_04_jnewbery_JSON_refactor
Was planning to merge it, but the json test changes in a RPC refactoring pull don’t look right.
“size” was added to the getrawtransaction output in #7072. “vsize” was added in #8149. Because bitcoin-tx used TxToUniv() and the rawtransaction RPCs used TxToJSON(), bitcoin-tx functionality fell behind the ratransaction RPC functionality and didn’t include these new fields. This PR removes the duplicate code between TxToUniv() and TxToJSON(), and as a result restores parity between bitcoin-tx and the _rawtransaction RPCs.
Sorry that wasn’t clear in the OP.