Merge pull request #1681 from EOSIO/fix-multi_index-secondary-index-bug

fix bug in multi_index::modify and add related test case

contracts/eosiolib/multi_index.hpp

@@ -689,8 +689,10 @@ class multi_index
             if( hana::at_c<index_type::index_number>(secondary_keys) != secondary ) {
                auto indexitr = mutableitem.__iters[index_type::number()];
 
-               if( indexitr < 0 )
-                  indexitr = mutableitem.__iters[index_type::number()] = db_idx_find_primary( _code, _scope, index_type::name(), pk,  secondary );
+               if( indexitr < 0 ) {
+                  typename index_type::secondary_key_type temp_secondary_key;
+                  indexitr = mutableitem.__iters[index_type::number()] = db_idx_find_primary( _code, _scope, index_type::name(), pk,  temp_secondary_key );
+               }
 
                db_idx_update( indexitr, payer, secondary );
             }

contracts/test_api/test_api.hpp

@@ -138,6 +138,9 @@ struct test_multi_index {
    static void idx64_general();
    static void idx64_store_only();
    static void idx64_check_without_storing();
+   static void idx128_general();
+   static void idx128_store_only();
+   static void idx128_check_without_storing();
    static void idx128_autoincrement_test();
    static void idx128_autoincrement_test_part1();
    static void idx128_autoincrement_test_part2();

contracts/test_api_multi_index/test_api_multi_index.cpp

@@ -18,6 +18,9 @@ extern "C" {
       WASM_TEST_HANDLER(test_multi_index, idx64_general);
       WASM_TEST_HANDLER(test_multi_index, idx64_store_only);
       WASM_TEST_HANDLER(test_multi_index, idx64_check_without_storing);
+      WASM_TEST_HANDLER(test_multi_index, idx128_general);
+      WASM_TEST_HANDLER(test_multi_index, idx128_store_only);
+      WASM_TEST_HANDLER(test_multi_index, idx128_check_without_storing);
       WASM_TEST_HANDLER(test_multi_index, idx128_autoincrement_test);
       WASM_TEST_HANDLER(test_multi_index, idx128_autoincrement_test_part1);
       WASM_TEST_HANDLER(test_multi_index, idx128_autoincrement_test_part2);

contracts/test_api_multi_index/test_multi_index.cpp

@@ -178,6 +178,68 @@ namespace _test_multi_index {
       }
    }
 
+   template<uint64_t TableName>
+   void idx128_store_only()
+   {
+      using namespace eosio;
+
+      typedef record_idx128 record;
+
+
+      // Construct and fill table using multi_index
+      multi_index<TableName, record,
+         indexed_by< N(bysecondary), const_mem_fun<record, uint128_t, &record::get_secondary> >
+      > table( current_receiver(), current_receiver() );
+
+      auto payer = current_receiver();
+
+      for (uint64_t i = 0; i < 5; ++i) {
+         table.emplace( payer, [&]( auto& r ) {
+            r.id = i;
+            r.sec = static_cast<uint128_t>(1ULL << 63) * i;
+         });
+      }
+   }
+
+   template<uint64_t TableName>
+   void idx128_check_without_storing()
+   {
+      using namespace eosio;
+
+      typedef record_idx128 record;
+
+      // Load table using multi_index
+      multi_index<TableName, record,
+         indexed_by< N(bysecondary), const_mem_fun<record, uint128_t, &record::get_secondary> >
+      > table( current_receiver(), current_receiver() );
+
+      auto payer = current_receiver();
+
+      auto secondary_index = table.template get_index<N(bysecondary)>();
+
+      table.modify(table.get(3), payer, [&]( auto& r ) {
+         r.sec *= 2;
+      });
+
+      {
+         uint128_t multiplier = 1ULL << 63;
+
+         auto itr = secondary_index.begin();
+         eosio_assert( itr->primary_key() == 0 && itr->get_secondary() == multiplier*0, "idx128_general - secondary key sort" );
+         ++itr;
+         eosio_assert( itr->primary_key() == 1 && itr->get_secondary() == multiplier*1, "idx128_general - secondary key sort" );
+         ++itr;
+         eosio_assert( itr->primary_key() == 2 && itr->get_secondary() == multiplier*2, "idx128_general - secondary key sort" );
+         ++itr;
+         eosio_assert( itr->primary_key() == 4 && itr->get_secondary() == multiplier*4, "idx128_general - secondary key sort" );
+         ++itr;
+         eosio_assert( itr->primary_key() == 3 && itr->get_secondary() == multiplier*6, "idx128_general - secondary key sort" );
+         ++itr;
+         eosio_assert( itr == secondary_index.end(), "idx128_general - secondary key sort" );
+      }
+
+   }
+
 } /// _test_multi_index
 
 void test_multi_index::idx64_store_only()
@@ -196,6 +258,22 @@ void test_multi_index::idx64_general()
    _test_multi_index::idx64_check_without_storing<N(indextable2)>();
 }
 
+void test_multi_index::idx128_store_only()
+{
+   _test_multi_index::idx128_store_only<N(indextable3)>();
+}
+
+void test_multi_index::idx128_check_without_storing()
+{
+   _test_multi_index::idx128_check_without_storing<N(indextable3)>();
+}
+
+void test_multi_index::idx128_general()
+{
+   _test_multi_index::idx128_store_only<N(indextable4)>();
+   _test_multi_index::idx128_check_without_storing<N(indextable4)>();
+}
+
 void test_multi_index::idx128_autoincrement_test()
 {
    using namespace eosio;
@@ -203,7 +281,7 @@ void test_multi_index::idx128_autoincrement_test()
 
    typedef record_idx128 record;
 
-   const uint64_t table_name = N(indextable3);
+   const uint64_t table_name = N(autoinctbl1);
    auto payer = current_receiver();
 
    multi_index<table_name, record,
@@ -251,7 +329,7 @@ void test_multi_index::idx128_autoincrement_test_part1()
 
    typedef record_idx128 record;
 
-   const uint64_t table_name = N(indextable4);
+   const uint64_t table_name = N(autoinctbl2);
    auto payer = current_receiver();
 
    multi_index<table_name, record,
@@ -284,7 +362,7 @@ void test_multi_index::idx128_autoincrement_test_part2()
 
    typedef record_idx128 record;
 
-   const uint64_t table_name = N(indextable4);
+   const uint64_t table_name = N(autoinctbl2);
    auto payer = current_receiver();
 
    {

tests/api_tests/api_tests.cpp

@@ -674,19 +674,22 @@ BOOST_FIXTURE_TEST_CASE(db_tests, tester) { try {
  * multi_index_tests test case
  *************************************************************************************/
 BOOST_FIXTURE_TEST_CASE(multi_index_tests, tester) { try {
-	produce_blocks(1);
-	create_account( N(testapi) );
-	produce_blocks(1);
-	set_code( N(testapi), test_api_multi_index_wast );
-	produce_blocks(1);
+   produce_blocks(1);
+   create_account( N(testapi) );
+   produce_blocks(1);
+   set_code( N(testapi), test_api_multi_index_wast );
+   produce_blocks(1);
 
-	CALL_TEST_FUNCTION( *this, "test_multi_index", "idx64_general", {});
-	CALL_TEST_FUNCTION( *this, "test_multi_index", "idx64_store_only", {});
-	CALL_TEST_FUNCTION( *this, "test_multi_index", "idx64_check_without_storing", {});
-	CALL_TEST_FUNCTION( *this, "test_multi_index", "idx128_autoincrement_test", {});
-	CALL_TEST_FUNCTION( *this, "test_multi_index", "idx128_autoincrement_test_part1", {});
-	CALL_TEST_FUNCTION( *this, "test_multi_index", "idx128_autoincrement_test_part2", {});
-	CALL_TEST_FUNCTION( *this, "test_multi_index", "idx256_general", {});
+   CALL_TEST_FUNCTION( *this, "test_multi_index", "idx64_general", {});
+   CALL_TEST_FUNCTION( *this, "test_multi_index", "idx64_store_only", {});
+   CALL_TEST_FUNCTION( *this, "test_multi_index", "idx64_check_without_storing", {});
+   CALL_TEST_FUNCTION( *this, "test_multi_index", "idx128_general", {});
+   CALL_TEST_FUNCTION( *this, "test_multi_index", "idx128_store_only", {});
+   CALL_TEST_FUNCTION( *this, "test_multi_index", "idx128_check_without_storing", {});
+   CALL_TEST_FUNCTION( *this, "test_multi_index", "idx128_autoincrement_test", {});
+   CALL_TEST_FUNCTION( *this, "test_multi_index", "idx128_autoincrement_test_part1", {});
+   CALL_TEST_FUNCTION( *this, "test_multi_index", "idx128_autoincrement_test_part2", {});
+   CALL_TEST_FUNCTION( *this, "test_multi_index", "idx256_general", {});
    CALL_TEST_FUNCTION( *this, "test_multi_index", "idx_double_general", {});
 } FC_LOG_AND_RETHROW() }