Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rpc: Add level 3 verbosity to getblock RPC call (#21245 modified) #22918

Merged
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions doc/release-notes.md
Expand Up @@ -76,6 +76,14 @@ Updated RPCs
`gettransaction verbose=true` and REST endpoints `/rest/tx`, `/rest/getutxos`,
`/rest/block` no longer return the `addresses` and `reqSigs` fields, which
were previously deprecated in 22.0. (#22650)
- The `getblock` RPC command now supports verbose level 3 containing transaction inputs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: verbose -> verbosity and inputs -> inputs'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. I will fix in a next rebase or in a follow-up PR.

`prevout` information. The existing `/rest/block/` REST endpoint is modified to contain
this information too. Every `vin` field will contain an additional `prevout` subfield
describing the spent output. `prevout` contains the following keys:
- `generated` - true if the spent coins was a coinbase.
- `height`
- `value`
- `scriptPubKey`

- `listunspent` now includes `ancestorcount`, `ancestorsize`, and
`ancestorfees` for each transaction output that is still in the mempool.
Expand Down
4 changes: 2 additions & 2 deletions src/bench/rpc_blockchain.cpp
Expand Up @@ -40,7 +40,7 @@ static void BlockToJsonVerbose(benchmark::Bench& bench)
{
TestBlockAndIndex data;
bench.run([&] {
auto univalue = blockToJSON(data.block, &data.blockindex, &data.blockindex, /*verbose*/ true);
auto univalue = blockToJSON(data.block, &data.blockindex, &data.blockindex, TxVerbosity::SHOW_DETAILS_AND_PREVOUT);
ankerl::nanobench::doNotOptimizeAway(univalue);
});
}
Expand All @@ -50,7 +50,7 @@ BENCHMARK(BlockToJsonVerbose);
static void BlockToJsonVerboseWrite(benchmark::Bench& bench)
{
TestBlockAndIndex data;
auto univalue = blockToJSON(data.block, &data.blockindex, &data.blockindex, /*verbose*/ true);
auto univalue = blockToJSON(data.block, &data.blockindex, &data.blockindex, TxVerbosity::SHOW_DETAILS_AND_PREVOUT);
bench.run([&] {
auto str = univalue.write();
ankerl::nanobench::doNotOptimizeAway(str);
Expand Down
11 changes: 10 additions & 1 deletion src/core_io.h
Expand Up @@ -20,6 +20,15 @@ class uint256;
class UniValue;
class CTxUndo;

/**
* Verbose level for block's transaction
*/
enum class TxVerbosity {
SHOW_TXID, //!< Only TXID for each block's transaction
SHOW_DETAILS, //!< Include TXID, inputs, outputs, and other common block's transaction information
SHOW_DETAILS_AND_PREVOUT //!< The same as previous option with information about prevouts if available
};

// core_read.cpp
CScript ParseScript(const std::string& s);
std::string ScriptToAsmStr(const CScript& script, const bool fAttemptSighashDecode = false);
Expand All @@ -46,6 +55,6 @@ std::string EncodeHexTx(const CTransaction& tx, const int serializeFlags = 0);
std::string SighashToStr(unsigned char sighash_type);
void ScriptPubKeyToUniv(const CScript& scriptPubKey, UniValue& out, bool include_hex, bool include_address = true);
void ScriptToUniv(const CScript& script, UniValue& out);
void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex = true, int serialize_flags = 0, const CTxUndo* txundo = nullptr);
void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex = true, int serialize_flags = 0, const CTxUndo* txundo = nullptr, TxVerbosity verbosity = TxVerbosity::SHOW_DETAILS);

#endif // BITCOIN_CORE_IO_H
23 changes: 21 additions & 2 deletions src/core_write.cpp
Expand Up @@ -163,7 +163,7 @@ void ScriptPubKeyToUniv(const CScript& scriptPubKey, UniValue& out, bool include
out.pushKV("type", GetTxnOutputType(type));
}

void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex, int serialize_flags, const CTxUndo* txundo)
void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex, int serialize_flags, const CTxUndo* txundo, TxVerbosity verbosity)
{
entry.pushKV("txid", tx.GetHash().GetHex());
entry.pushKV("hash", tx.GetWitnessHash().GetHex());
Expand Down Expand Up @@ -204,8 +204,27 @@ void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry,
in.pushKV("txinwitness", txinwitness);
}
if (calculate_fee) {
const CTxOut& prev_txout = txundo->vprevout[i].out;
const Coin& prev_coin = txundo->vprevout[i];
const CTxOut& prev_txout = prev_coin.out;

amt_total_in += prev_txout.nValue;
switch (verbosity) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

51dbc16

nit, we only care about one value, what's wrong with

if (verbosity == TxVerbosity::SHOW_DETAILS_AND_PREVOUT) {

If you do this, then declare prev_coin in the inner scope.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. I would like to address that a next rebase or in a follow-up PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have actually proposed the same in the original PR: #21245 (comment)

case TxVerbosity::SHOW_TXID:
case TxVerbosity::SHOW_DETAILS:
break;

case TxVerbosity::SHOW_DETAILS_AND_PREVOUT:
UniValue o_script_pub_key(UniValue::VOBJ);
ScriptPubKeyToUniv(prev_txout.scriptPubKey, o_script_pub_key, /* includeHex */ true);

UniValue p(UniValue::VOBJ);
p.pushKV("generated", bool(prev_coin.fCoinBase));
p.pushKV("height", uint64_t(prev_coin.nHeight));
p.pushKV("value", ValueFromAmount(prev_txout.nValue));
p.pushKV("scriptPubKey", o_script_pub_key);
in.pushKV("prevout", p);
break;
}
Copy link
Contributor

@jonatack jonatack Oct 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per doc/developer-notes.md::L672-694,the following comment and assert are usually employed with self-contained switch statements so the compiler can warn about missing cases. It looks like that could work here as the switch is scoped within the have_undo conditional:

    } // no default case, so the compiler can warn about missing cases
    assert(false);
}

(usually, the case statements have the same indentation as switch, without blank lines between them)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. I would like to address that a next rebase or in a follow-up PR.

}
in.pushKV("sequence", (int64_t)txin.nSequence);
vin.push_back(in);
Expand Down
8 changes: 4 additions & 4 deletions src/rest.cpp
Expand Up @@ -260,7 +260,7 @@ static bool rest_headers(const std::any& context,
static bool rest_block(const std::any& context,
HTTPRequest* req,
const std::string& strURIPart,
bool showTxDetails)
TxVerbosity tx_verbosity)
{
if (!CheckWarmup(req))
return false;
Expand Down Expand Up @@ -312,7 +312,7 @@ static bool rest_block(const std::any& context,
}

case RetFormat::JSON: {
UniValue objBlock = blockToJSON(block, tip, pblockindex, showTxDetails);
UniValue objBlock = blockToJSON(block, tip, pblockindex, tx_verbosity);
std::string strJSON = objBlock.write() + "\n";
req->WriteHeader("Content-Type", "application/json");
req->WriteReply(HTTP_OK, strJSON);
Expand All @@ -327,12 +327,12 @@ static bool rest_block(const std::any& context,

static bool rest_block_extended(const std::any& context, HTTPRequest* req, const std::string& strURIPart)
{
return rest_block(context, req, strURIPart, true);
return rest_block(context, req, strURIPart, TxVerbosity::SHOW_DETAILS_AND_PREVOUT);
}

static bool rest_block_notxdetails(const std::any& context, HTTPRequest* req, const std::string& strURIPart)
{
return rest_block(context, req, strURIPart, false);
return rest_block(context, req, strURIPart, TxVerbosity::SHOW_TXID);
}

// A bit of a hack - dependency on a function defined in rpc/blockchain.cpp
Expand Down
53 changes: 35 additions & 18 deletions src/rpc/blockchain.cpp
Expand Up @@ -200,30 +200,37 @@ UniValue blockheaderToJSON(const CBlockIndex* tip, const CBlockIndex* blockindex
return result;
}

UniValue blockToJSON(const CBlock& block, const CBlockIndex* tip, const CBlockIndex* blockindex, bool txDetails)
UniValue blockToJSON(const CBlock& block, const CBlockIndex* tip, const CBlockIndex* blockindex, TxVerbosity verbosity)
{
UniValue result = blockheaderToJSON(tip, blockindex);

result.pushKV("strippedsize", (int)::GetSerializeSize(block, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS));
result.pushKV("size", (int)::GetSerializeSize(block, PROTOCOL_VERSION));
result.pushKV("weight", (int)::GetBlockWeight(block));
UniValue txs(UniValue::VARR);
if (txDetails) {
CBlockUndo blockUndo;
const bool have_undo = !IsBlockPruned(blockindex) && UndoReadFromDisk(blockUndo, blockindex);
for (size_t i = 0; i < block.vtx.size(); ++i) {
const CTransactionRef& tx = block.vtx.at(i);
// coinbase transaction (i == 0) doesn't have undo data
const CTxUndo* txundo = (have_undo && i) ? &blockUndo.vtxundo.at(i - 1) : nullptr;
UniValue objTx(UniValue::VOBJ);
TxToUniv(*tx, uint256(), objTx, true, RPCSerializationFlags(), txundo);
txs.push_back(objTx);
}
} else {
for (const CTransactionRef& tx : block.vtx) {
txs.push_back(tx->GetHash().GetHex());
}

switch (verbosity) {
case TxVerbosity::SHOW_TXID:
for (const CTransactionRef& tx : block.vtx) {
txs.push_back(tx->GetHash().GetHex());
}
break;

case TxVerbosity::SHOW_DETAILS:
case TxVerbosity::SHOW_DETAILS_AND_PREVOUT:
CBlockUndo blockUndo;
const bool have_undo = !IsBlockPruned(blockindex) && UndoReadFromDisk(blockUndo, blockindex);

for (size_t i = 0; i < block.vtx.size(); ++i) {
const CTransactionRef& tx = block.vtx.at(i);
// coinbase transaction (i.e. i == 0) doesn't have undo data
const CTxUndo* txundo = (have_undo && i > 0) ? &blockUndo.vtxundo.at(i - 1) : nullptr;
UniValue objTx(UniValue::VOBJ);
TxToUniv(*tx, uint256(), objTx, true, RPCSerializationFlags(), txundo, verbosity);
txs.push_back(objTx);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3cc9534

nit, add break;?

}

result.pushKV("tx", txs);

return result;
Expand Down Expand Up @@ -931,7 +938,8 @@ static RPCHelpMan getblock()
return RPCHelpMan{"getblock",
"\nIf verbosity is 0, returns a string that is serialized, hex-encoded data for block 'hash'.\n"
"If verbosity is 1, returns an Object with information about block <hash>.\n"
"If verbosity is 2, returns an Object with information about block <hash> and information about each transaction. \n",
"If verbosity is 2, returns an Object with information about block <hash> and information about each transaction.\n"
"If verbosity is 3, returns an Object with information about block <hash> and information about each transaction, including prevout information for inputs (only for unpruned blocks in the current best chain).\n",
{
{"blockhash", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The block hash"},
{"verbosity|verbose", RPCArg::Type::NUM, RPCArg::Default{1}, "0 for hex-encoded data, 1 for a json object, and 2 for json object with transaction data"},
Expand Down Expand Up @@ -1018,7 +1026,16 @@ static RPCHelpMan getblock()
return strHex;
}

return blockToJSON(block, tip, pblockindex, verbosity >= 2);
TxVerbosity tx_verbosity;
if (verbosity == 1) {
tx_verbosity = TxVerbosity::SHOW_TXID;
} else if (verbosity == 2) {
tx_verbosity = TxVerbosity::SHOW_DETAILS;
} else {
tx_verbosity = TxVerbosity::SHOW_DETAILS_AND_PREVOUT;
}

return blockToJSON(block, tip, pblockindex, tx_verbosity);
},
};
}
Expand Down
3 changes: 2 additions & 1 deletion src/rpc/blockchain.h
Expand Up @@ -6,6 +6,7 @@
#define BITCOIN_RPC_BLOCKCHAIN_H

#include <consensus/amount.h>
#include <core_io.h>
#include <streams.h>
#include <sync.h>

Expand Down Expand Up @@ -38,7 +39,7 @@ double GetDifficulty(const CBlockIndex* blockindex);
void RPCNotifyBlockChange(const CBlockIndex*);

/** Block description to JSON */
UniValue blockToJSON(const CBlock& block, const CBlockIndex* tip, const CBlockIndex* blockindex, bool txDetails = false) LOCKS_EXCLUDED(cs_main);
UniValue blockToJSON(const CBlock& block, const CBlockIndex* tip, const CBlockIndex* blockindex, TxVerbosity verbosity) LOCKS_EXCLUDED(cs_main);

/** Mempool information to JSON */
UniValue MempoolInfoToJSON(const CTxMemPool& pool);
Expand Down
9 changes: 9 additions & 0 deletions test/functional/interface_rest.py
Expand Up @@ -311,6 +311,15 @@ def run_test(self):
if 'coinbase' not in tx['vin'][0]}
assert_equal(non_coinbase_txs, set(txs))

# Verify that the non-coinbase tx has "prevout" key set
for tx_obj in json_obj["tx"]:
for vin in tx_obj["vin"]:
if "coinbase" not in vin:
assert "prevout" in vin
assert_equal(vin["prevout"]["generated"], False)
else:
assert "prevout" not in vin

# Check the same but without tx details
json_obj = self.test_rest_request(f"/block/notxdetails/{newblockhash[0]}")
for tx in txs:
Expand Down
66 changes: 53 additions & 13 deletions test/functional/rpc_blockchain.py
Expand Up @@ -434,17 +434,55 @@ def _test_getblock(self):
miniwallet.send_self_transfer(fee_rate=fee_per_kb, from_node=node)
blockhash = self.generate(node, 1)[0]

self.log.info("Test getblock with verbosity 1 doesn't include fee")
block = node.getblock(blockhash, 1)
assert 'fee' not in block['tx'][1]

self.log.info('Test getblock with verbosity 2 includes expected fee')
block = node.getblock(blockhash, 2)
tx = block['tx'][1]
assert 'fee' in tx
assert_equal(tx['fee'], tx['vsize'] * fee_per_byte)

self.log.info("Test getblock with verbosity 2 still works with pruned Undo data")
def assert_fee_not_in_block(verbosity):
block = node.getblock(blockhash, verbosity)
assert 'fee' not in block['tx'][1]

def assert_fee_in_block(verbosity):
block = node.getblock(blockhash, verbosity)
tx = block['tx'][1]
assert 'fee' in tx
assert_equal(tx['fee'], tx['vsize'] * fee_per_byte)

def assert_vin_contains_prevout(verbosity):
block = node.getblock(blockhash, verbosity)
tx = block["tx"][1]
total_vin = Decimal("0.00000000")
total_vout = Decimal("0.00000000")
for vin in tx["vin"]:
assert "prevout" in vin
assert_equal(set(vin["prevout"].keys()), set(("value", "height", "generated", "scriptPubKey")))
assert_equal(vin["prevout"]["generated"], True)
total_vin += vin["prevout"]["value"]
for vout in tx["vout"]:
total_vout += vout["value"]
assert_equal(total_vin, total_vout + tx["fee"])

def assert_vin_does_not_contain_prevout(verbosity):
block = node.getblock(blockhash, verbosity)
tx = block["tx"][1]
if isinstance(tx, str):
# In verbosity level 1, only the transaction hashes are written
pass
else:
for vin in tx["vin"]:
assert "prevout" not in vin

self.log.info("Test that getblock with verbosity 1 doesn't include fee")
assert_fee_not_in_block(1)

self.log.info('Test that getblock with verbosity 2 and 3 includes expected fee')
assert_fee_in_block(2)
assert_fee_in_block(3)

self.log.info("Test that getblock with verbosity 1 and 2 does not include prevout")
assert_vin_does_not_contain_prevout(1)
assert_vin_does_not_contain_prevout(2)

self.log.info("Test that getblock with verbosity 3 includes prevout")
assert_vin_contains_prevout(3)

self.log.info("Test that getblock with verbosity 2 and 3 still works with pruned Undo data")
datadir = get_datadir_path(self.options.tmpdir, 0)

self.log.info("Test getblock with invalid verbosity type returns proper error message")
Expand All @@ -458,8 +496,10 @@ def move_block_file(old, new):
# Move instead of deleting so we can restore chain state afterwards
move_block_file('rev00000.dat', 'rev_wrong')

block = node.getblock(blockhash, 2)
assert 'fee' not in block['tx'][1]
assert_fee_not_in_block(2)
assert_fee_not_in_block(3)
assert_vin_does_not_contain_prevout(2)
assert_vin_does_not_contain_prevout(3)

# Restore chain state
move_block_file('rev_wrong', 'rev00000.dat')
Expand Down