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

fix get_table_rows_by_seckey conversion #9605

Merged
merged 8 commits into from
Nov 3, 2020

Conversation

kimjh2005
Copy link
Contributor

Change Description

fixed function get_table_rows_by_seckey converts lower and upper value in eosio::name incorrectly.

Change Type

Select ONE

  • Documentation
  • Stability bug fix
  • Other
  • Other - special case

Consensus Changes

  • Consensus Changes

API Changes

  • API Changes

Documentation Additions

  • Documentation Additions

@kimjh2005 kimjh2005 marked this pull request as ready for review October 28, 2020 16:44
@dimas1185
Copy link
Contributor

can you explain why get_table_rows_by_seckey was working incorrectly if you used string representation of name instead of string representaton of underlying integer?

@kimjh2005
Copy link
Contributor Author

convert_to_type calls boost::lexical_cast<uint64_t>( str.c_str())
Before the fix, lexical_cast<uint64_t>("1111" ) is called,
After the fix, lexical_cast<uint64_t>("1324356665524535") is called. //"1324356665524535" is eosio::name in uint64_t string.

The return value is:
Before the fix is 1111,
After the fix is eosio::name in uint64_t number.

template<>
uint64_t convert_to_type(const string& str, const string& desc) {
try {
return boost::lexical_cast<uint64_t>(str.c_str(), str.size());
} catch( ... ) { }

@dimas1185
Copy link
Contributor

@kimjh2005 that is understood. I don't get why lexical_cast<uint64_t>("1111" ) is an issue. it should return valid value, isn't it? so why raw uint64 representation is preferred?

@kimjh2005
Copy link
Contributor Author

kimjh2005 commented Oct 29, 2020

the return value 1111 is not correct value for eosio::name for "1111"_n. It should have uint64_t number which is not 1111.

@dimas1185
Copy link
Contributor

@kimjh2005 eosio::chain::name "1111"_n is valid name. However I looked in the code and I understood why it fails. I'll propose some alternative solution inline in comments.

@dimas1185 dimas1185 requested a review from larryk85 October 29, 2020 23:46
SecKeyType lv = convert_to_type<SecKeyType>( s.to_string(), "lower_bound name" ); // avoids compiler error
std::get<1>(lower_bound_lookup_tuple) = conv( lv );
if constexpr (std::is_same_v<uint64_t, SecKeyType>) {
SecKeyType lv = convert_to_type(s, "lower_bound name");
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you need L-value here. This should work as long as reference is const:

SecKeyType lv = convert_to_type({p.lower_bound}, "lower_bound name");

@kimjh2005 kimjh2005 requested a review from larryk85 November 2, 2020 14:13
@brianjohnson5972 brianjohnson5972 dismissed their stale review November 3, 2020 00:31

I misread the change

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.

4 participants