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

fix get_table_rows_by_seckey conversion - #9605

Merged
kimjh2005 merged 8 commits into
developfrom
fix_get_table_rows_by_seckey
Nov 3, 2020
Merged

kimjh2005 merged 8 commits into
developfrom
fix_get_table_rows_by_seckey

Conversation

@kimjh2005

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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

kimjh2005 commented Oct 29, 2020 •

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
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
Copy Markdown
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