Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

fix prefix comparison in get_transaction in history_plugin #5607

Closed
wants to merge 1 commit into from
Closed

fix prefix comparison in get_transaction in history_plugin #5607

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions plugins/history_plugin/history_plugin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,13 +440,22 @@ namespace eosio {
read_only::get_transaction_result read_only::get_transaction( const read_only::get_transaction_params& p )const {
auto& chain = history->chain_plug->chain();
const auto abi_serializer_max_time = history->chain_plug->get_abi_serializer_max_time();
auto short_id = fc::variant(p.id).as_string().substr(0,8);

const auto& db = chain.db();
const auto& idx = db.get_index<action_history_index, by_trx_id>();
auto itr = idx.lower_bound( boost::make_tuple(p.id) );

bool in_history = (itr != idx.end() && fc::variant(itr->trx_id).as_string().substr(0,8) == short_id );
auto txn_id_matched = [&p](const transaction_id_type &id)->bool { // hex prefix comparison
Copy link
Contributor

Choose a reason for hiding this comment

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

Since the type of id in get_transaction_params is transaction_id_type, by the time read_only::get_transaction is called critical information from the RPC API caller has been lost. Specifically that information is how many bytes from the beginning of transaction ID were actually provided by the caller. Without that information, it is not possible to know within this function whether trailing zeros in p.id are legitimate zeros provided by the caller that must be enforced in the matching, or if they should act as wildcards.

In order to properly implement this function, I think you will need to change the id parameter to a string. Luckily that shouldn't actually change the schema of the JSON input nor does it change the requirements of what that string should represent (some initial part of a transaction ID), so it shouldn't be a breaking change to the history plugin RPC API.

string p_str = (string)p.id;
size_t prefix_len = p_str.length();
string id_str = (string)id;
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer to see the comparisons be done on the bytes of transaction_id_types directly rather than on characters of the string converted from a transaction_id_type. That way the conversion from the p.id string to transaction_id_type can be done once (outside of the lambda function), and no string conversions are needed at all inside the lambda function.

while (prefix_len > 1 && p_str[prefix_len - 1] == '0') prefix_len--;
if (prefix_len < 8) prefix_len = 8;
if (p_str.length() < prefix_len || id_str.length() < prefix_len) return false;
return memcmp(p_str.c_str(), id_str.c_str(), prefix_len) == 0;
};

bool in_history = (itr != idx.end() && txn_id_matched(itr->trx_id));

if( !in_history && !p.block_num_hint ) {
EOS_THROW(tx_not_found, "Transaction ${id} not found in history and no block hint was given", ("id",p.id));
Expand Down Expand Up @@ -509,7 +518,7 @@ namespace eosio {
if (receipt.trx.contains<packed_transaction>()) {
auto& pt = receipt.trx.get<packed_transaction>();
auto mtrx = transaction_metadata(pt);
if (fc::variant(mtrx.id).as_string().substr(0, 8) == short_id) {
if (txn_id_matched(mtrx.id)) {
result.id = mtrx.id;
result.last_irreversible_block = chain.last_irreversible_block_num();
result.block_num = *p.block_num_hint;
Expand All @@ -522,7 +531,7 @@ namespace eosio {
}
} else {
auto& id = receipt.trx.get<transaction_id_type>();
if (fc::variant(id).as_string().substr(0, 8) == short_id) {
if (txn_id_matched(id)) {
result.id = id;
result.last_irreversible_block = chain.last_irreversible_block_num();
result.block_num = *p.block_num_hint;
Expand Down