From 30448ffa3d10e24c26489d336e570c0633d80c99 Mon Sep 17 00:00:00 2001 From: Qing Zhang Date: Thu, 17 Dec 2020 13:51:01 -0800 Subject: [PATCH 1/4] chain_plugin db intrinsic table RPC calls incorrectly handling --lower and --upper in certain scenarios --- .../include/eosio/chain/backing_store/db_combined.hpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/libraries/chain/include/eosio/chain/backing_store/db_combined.hpp b/libraries/chain/include/eosio/chain/backing_store/db_combined.hpp index 7a8123d0534..571bdca031d 100644 --- a/libraries/chain/include/eosio/chain/backing_store/db_combined.hpp +++ b/libraries/chain/include/eosio/chain/backing_store/db_combined.hpp @@ -457,17 +457,15 @@ namespace detail { EOS_ASSERT(begin_key < end_key, database_exception, "Invalid iterator_pair request: begin_key was greater than or equal to end_key."); if (is_reverse_) { current_ = session.lower_bound(end_key); - EOS_ASSERT(current_ != session.end(), database_exception, "iterator_pair: failed to find lower bound of end_key"); end_ = session.lower_bound(begin_key); // since this is reverse iterating, then need to iterate backward if this is a greater-than iterator, // to get a greater-than-or-equal reverse iterator - if ((*current_).first > end_key) { + if (current_ == session.end() || (*current_).first > end_key) { --current_; + EOS_ASSERT(current_ != session.end(), database_exception, "iterator_pair: failed to find lower bound of end_key"); } // since this is reverse iterating, then need to iterate backward to get a less-than iterator - if (end_ != session.end()) { - --end_; - } + --end_; } else { current_ = session.lower_bound(begin_key); From b64d4eb3b7fc4b41caee6923daf92d308e78d216 Mon Sep 17 00:00:00 2001 From: Qing Zhang Date: Wed, 30 Dec 2020 00:04:10 -0800 Subject: [PATCH 2/4] add unit tests for chain_plugin db intrinsic table RPC calls incorrectly handling --lower and --upper in certain scenarios --- tests/get_table_seckey_tests.cpp | 74 ++++++++++++++++++++++++++++++++ tests/get_table_tests.cpp | 12 ++++++ 2 files changed, 86 insertions(+) diff --git a/tests/get_table_seckey_tests.cpp b/tests/get_table_seckey_tests.cpp index f47720364d7..60700801317 100644 --- a/tests/get_table_seckey_tests.cpp +++ b/tests/get_table_seckey_tests.cpp @@ -32,6 +32,34 @@ BOOST_AUTO_TEST_SUITE(get_table_seckey_tests) using backing_store_ts = boost::mpl::list; +transaction_trace_ptr +issue_tokens( TESTER& t, account_name issuer, account_name to, const asset& amount, + std::string memo = "", account_name token_contract = "eosio"_n ) +{ + signed_transaction trx; + + trx.actions.emplace_back( t.get_action( token_contract, "issue"_n, + vector{{issuer, config::active_name}}, + mutable_variant_object() + ("to", issuer.to_string()) + ("quantity", amount) + ("memo", memo) + ) ); + + trx.actions.emplace_back( t.get_action( token_contract, "transfer"_n, + vector{{issuer, config::active_name}}, + mutable_variant_object() + ("from", issuer.to_string()) + ("to", to.to_string()) + ("quantity", amount) + ("memo", memo) + ) ); + + t.set_transaction_headers(trx); + trx.sign( t.get_private_key( issuer, "active" ), t.control->get_chain_id() ); + return t.push_transaction( trx ); +} + BOOST_AUTO_TEST_CASE_TEMPLATE( get_table_next_key_test, TESTER_T, backing_store_ts) { try { TESTER_T t; t.create_account("test"_n); @@ -94,4 +122,50 @@ BOOST_AUTO_TEST_CASE_TEMPLATE( get_table_next_key_test, TESTER_T, backing_store_ } FC_LOG_AND_RETHROW() }/// get_table_next_key_test +BOOST_AUTO_TEST_CASE_TEMPLATE( get_table_next_key_reverse_test, TESTER_T, backing_store_ts) { try { + TESTER_T t; + t.produce_blocks(2); + + t.create_accounts({ "eosio.token"_n, "eosio.ram"_n, "eosio.ramfee"_n, "eosio.stake"_n, + "eosio.bpay"_n, "eosio.vpay"_n, "eosio.saving"_n, "eosio.names"_n }); + + std::vector accs{"inita"_n, "initb"_n, "initc"_n, "initd"_n}; + t.create_accounts(accs); + t.produce_block(); + + t.set_code( "eosio"_n, contracts::eosio_token_wasm() ); + t.set_abi( "eosio"_n, contracts::eosio_token_abi().data() ); + t.produce_blocks(1); + + // create currency + auto act = mutable_variant_object() + ("issuer", "eosio") + ("maximum_supply", eosio::chain::asset::from_string("1000000000.0000 SYS")); + t.push_action("eosio"_n, "create"_n, "eosio"_n, act ); + + // issue + for (account_name a: accs) { + issue_tokens( t, config::system_account_name, a, eosio::chain::asset::from_string("999.0000 SYS") ); + } + t.produce_blocks(1); + + // iterate over scope + eosio::chain_apis::read_only plugin(*(t.control), {}, fc::microseconds::maximum()); + eosio::chain_apis::read_only::get_table_by_scope_params param{"eosio"_n, "accounts"_n, "inita", "", 10}; + + param.lower_bound = "a"; + param.upper_bound = "z"; + param.reverse = true; + eosio::chain_apis::read_only::get_table_by_scope_result result = plugin.read_only::get_table_by_scope(param); + BOOST_REQUIRE_EQUAL(5u, result.rows.size()); + BOOST_REQUIRE_EQUAL("", result.more); + if (result.rows.size() >= 5) { + BOOST_REQUIRE_EQUAL(name("initd"_n), result.rows[0].scope); + BOOST_REQUIRE_EQUAL(name("initc"_n), result.rows[1].scope); + BOOST_REQUIRE_EQUAL(name("initb"_n), result.rows[2].scope); + BOOST_REQUIRE_EQUAL(name("inita"_n), result.rows[3].scope); + BOOST_REQUIRE_EQUAL(name("eosio"_n), result.rows[4].scope); + } +} FC_LOG_AND_RETHROW() } /// get_scope_test + BOOST_AUTO_TEST_SUITE_END() diff --git a/tests/get_table_tests.cpp b/tests/get_table_tests.cpp index 4e3fc8ebde4..efbb7786942 100644 --- a/tests/get_table_tests.cpp +++ b/tests/get_table_tests.cpp @@ -116,6 +116,18 @@ BOOST_AUTO_TEST_CASE_TEMPLATE( get_scope_test, TESTER_T, backing_store_ts) { try BOOST_REQUIRE_EQUAL(name("initc"_n), result.rows[1].scope); } + param.lower_bound = "initb"; + param.upper_bound = "initd"; + param.reverse = true; + result = plugin.read_only::get_table_by_scope(param); + BOOST_REQUIRE_EQUAL(3u, result.rows.size()); + BOOST_REQUIRE_EQUAL("", result.more); + if (result.rows.size() >= 3) { + BOOST_REQUIRE_EQUAL(name("initd"_n), result.rows[0].scope); + BOOST_REQUIRE_EQUAL(name("initc"_n), result.rows[1].scope); + BOOST_REQUIRE_EQUAL(name("initb"_n), result.rows[2].scope); + } + param.limit = 1; result = plugin.read_only::get_table_by_scope(param); BOOST_REQUIRE_EQUAL(1u, result.rows.size()); From 188b3c54c6e0d6b8d0d3c2b94392b472647a89f7 Mon Sep 17 00:00:00 2001 From: Qing Zhang Date: Wed, 30 Dec 2020 08:31:40 -0800 Subject: [PATCH 3/4] remove unnecessary checks in unit tests --- tests/get_table_seckey_tests.cpp | 14 +++++++------- tests/get_table_tests.cpp | 9 ++++----- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/tests/get_table_seckey_tests.cpp b/tests/get_table_seckey_tests.cpp index 60700801317..afa7c49790a 100644 --- a/tests/get_table_seckey_tests.cpp +++ b/tests/get_table_seckey_tests.cpp @@ -159,13 +159,13 @@ BOOST_AUTO_TEST_CASE_TEMPLATE( get_table_next_key_reverse_test, TESTER_T, backin eosio::chain_apis::read_only::get_table_by_scope_result result = plugin.read_only::get_table_by_scope(param); BOOST_REQUIRE_EQUAL(5u, result.rows.size()); BOOST_REQUIRE_EQUAL("", result.more); - if (result.rows.size() >= 5) { - BOOST_REQUIRE_EQUAL(name("initd"_n), result.rows[0].scope); - BOOST_REQUIRE_EQUAL(name("initc"_n), result.rows[1].scope); - BOOST_REQUIRE_EQUAL(name("initb"_n), result.rows[2].scope); - BOOST_REQUIRE_EQUAL(name("inita"_n), result.rows[3].scope); - BOOST_REQUIRE_EQUAL(name("eosio"_n), result.rows[4].scope); - } + + BOOST_REQUIRE_EQUAL(name("initd"_n), result.rows[0].scope); + BOOST_REQUIRE_EQUAL(name("initc"_n), result.rows[1].scope); + BOOST_REQUIRE_EQUAL(name("initb"_n), result.rows[2].scope); + BOOST_REQUIRE_EQUAL(name("inita"_n), result.rows[3].scope); + BOOST_REQUIRE_EQUAL(name("eosio"_n), result.rows[4].scope); + } FC_LOG_AND_RETHROW() } /// get_scope_test BOOST_AUTO_TEST_SUITE_END() diff --git a/tests/get_table_tests.cpp b/tests/get_table_tests.cpp index efbb7786942..a03a6786d62 100644 --- a/tests/get_table_tests.cpp +++ b/tests/get_table_tests.cpp @@ -122,11 +122,10 @@ BOOST_AUTO_TEST_CASE_TEMPLATE( get_scope_test, TESTER_T, backing_store_ts) { try result = plugin.read_only::get_table_by_scope(param); BOOST_REQUIRE_EQUAL(3u, result.rows.size()); BOOST_REQUIRE_EQUAL("", result.more); - if (result.rows.size() >= 3) { - BOOST_REQUIRE_EQUAL(name("initd"_n), result.rows[0].scope); - BOOST_REQUIRE_EQUAL(name("initc"_n), result.rows[1].scope); - BOOST_REQUIRE_EQUAL(name("initb"_n), result.rows[2].scope); - } + + BOOST_REQUIRE_EQUAL(name("initd"_n), result.rows[0].scope); + BOOST_REQUIRE_EQUAL(name("initc"_n), result.rows[1].scope); + BOOST_REQUIRE_EQUAL(name("initb"_n), result.rows[2].scope); param.limit = 1; result = plugin.read_only::get_table_by_scope(param); From b1bdb050a37b4a52982bbe5332b28242979464cd Mon Sep 17 00:00:00 2001 From: Qing Zhang Date: Wed, 30 Dec 2020 12:55:29 -0800 Subject: [PATCH 4/4] change the upper bound to make it past the scope --- tests/get_table_tests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/get_table_tests.cpp b/tests/get_table_tests.cpp index a03a6786d62..ace9848365b 100644 --- a/tests/get_table_tests.cpp +++ b/tests/get_table_tests.cpp @@ -117,7 +117,7 @@ BOOST_AUTO_TEST_CASE_TEMPLATE( get_scope_test, TESTER_T, backing_store_ts) { try } param.lower_bound = "initb"; - param.upper_bound = "initd"; + param.upper_bound = "inite"; param.reverse = true; result = plugin.read_only::get_table_by_scope(param); BOOST_REQUIRE_EQUAL(3u, result.rows.size());