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

Conversation

taokayan
Copy link
Contributor

this fixes issue #5589
compare hex transaction id with adaptive prefix length ranging from 8 bytes(short id) to 64 bytes(full id)


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.

auto txn_id_matched = [&p](const transaction_id_type &id)->bool { // hex prefix comparison
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.

@arhag
Copy link
Contributor

arhag commented Sep 18, 2018

Replaced by #5723.

@arhag arhag closed this Sep 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants