fix keypage hash is inconsistent with stateStorage (FISCO-BCOS#4230)

bxq2011hust · Feb 19, 2024 · 5135cc1 · 5135cc1
1 parent 339942c
commit 5135cc1
Show file tree

Hide file tree

Showing 13 changed files with 190 additions and 124 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -7,7 +7,7 @@ endif()
 
 list(APPEND CMAKE_MODULE_PATH ${CMAKE_SOURCE_DIR}/cmake)
 
-set(VERSION "3.2.6")
+set(VERSION "3.2.7")
 set(VERSION_SUFFIX "")
 include(Options)
 configure_project()

diff --git a/bcos-executor/src/executor/TransactionExecutor.cpp b/bcos-executor/src/executor/TransactionExecutor.cpp
@@ -1066,11 +1066,11 @@ void TransactionExecutor::getHash(bcos::protocol::BlockNumber number,
 
     // remove suicides beforehand
     m_blockContext->killSuicides();
-
-    auto hash = last.storage->hash(m_hashImpl,
-        m_blockContext->features().get(ledger::Features::Flag::bugfix_statestorage_hash));
+    auto start = utcTime();
+    auto hash = last.storage->hash(m_hashImpl, m_blockContext->features());
+    auto end = utcTime();
     EXECUTOR_NAME_LOG(INFO) << BLOCK_NUMBER(number) << "GetTableHashes success"
-                            << LOG_KV("hash", hash.hex());
+                            << LOG_KV("hash", hash.hex()) << LOG_KV("time(ms)", (end - start));
 
     callback(nullptr, hash);
 }

diff --git a/bcos-framework/bcos-framework/ledger/Features.h b/bcos-framework/bcos-framework/ledger/Features.h
@@ -32,6 +32,7 @@ class Features
         bugfix_event_log_order,
         bugfix_call_noaddr_return,
         bugfix_precompiled_codehash,
+        bugfix_keypage_system_entry_hash,
         feature_dmc2serial,
     };
 

diff --git a/bcos-framework/bcos-framework/protocol/Protocol.h b/bcos-framework/bcos-framework/protocol/Protocol.h
@@ -113,6 +113,7 @@ enum ProtocolVersion : uint32_t
 
 enum class BlockVersion : uint32_t
 {
+    V3_2_7_VERSION = 0x03020700,
     V3_2_6_VERSION = 0x03020600,
     V3_2_5_VERSION = 0x03020500,
     V3_2_4_VERSION = 0x03020400,
@@ -122,7 +123,7 @@ enum class BlockVersion : uint32_t
     V3_0_VERSION = 0x03000000,
     RC4_VERSION = 4,
     MIN_VERSION = RC4_VERSION,
-    MAX_VERSION = V3_2_6_VERSION,
+    MAX_VERSION = V3_2_7_VERSION,
 };
 const std::string RC4_VERSION_STR = "3.0.0-rc4";
 const std::string V3_0_VERSION_STR = "3.0.0";

diff --git a/bcos-framework/test/unittests/interfaces/FeaturesTest.cpp b/bcos-framework/test/unittests/interfaces/FeaturesTest.cpp
@@ -64,6 +64,10 @@ BOOST_AUTO_TEST_CASE(feature)
     BOOST_CHECK_EQUAL(features5.get(Features::Flag::feature_dmc2serial), true);
     BOOST_CHECK_EQUAL(features5.get("feature_dmc2serial"), true);
 
+    features5.set(Features::Flag::bugfix_keypage_system_entry_hash);
+    BOOST_CHECK_EQUAL(features5.get(Features::Flag::bugfix_keypage_system_entry_hash), true);
+    BOOST_CHECK_EQUAL(features5.get("bugfix_keypage_system_entry_hash"), true);
+
     auto keys = Features::featureKeys();
     BOOST_CHECK_EQUAL(keys.size(), 7);
     BOOST_CHECK_EQUAL(keys[0], "bugfix_revert");
@@ -72,7 +76,8 @@ BOOST_AUTO_TEST_CASE(feature)
     BOOST_CHECK_EQUAL(keys[3], "bugfix_event_log_order");
     BOOST_CHECK_EQUAL(keys[4], "bugfix_call_noaddr_return");
     BOOST_CHECK_EQUAL(keys[5], "bugfix_precompiled_codehash");
-    BOOST_CHECK_EQUAL(keys[6], "feature_dmc2serial");
+    BOOST_CHECK_EQUAL(keys[6], "bugfix_keypage_system_entry_hash");
+    BOOST_CHECK_EQUAL(keys[7], "feature_dmc2serial");
 }
 
 BOOST_AUTO_TEST_SUITE_END()
diff --git a/bcos-table/src/KeyPageStorage.cpp b/bcos-table/src/KeyPageStorage.cpp
@@ -361,8 +361,8 @@ void KeyPageStorage::parallelTraverse(bool onlyDirty,
     KeyPage_LOG(INFO) << LOG_DESC("parallelTraverse") << LOG_KV("size", m_size);
 }
 
-auto KeyPageStorage::hash(const bcos::crypto::Hash::Ptr& hashImpl, bool /*useHashV310*/) const
-    -> crypto::HashType
+auto KeyPageStorage::hash(const bcos::crypto::Hash::Ptr& hashImpl,
+    const ledger::Features& features) const -> crypto::HashType
 {
     bcos::crypto::HashType pagesHash(0);
     bcos::crypto::HashType entriesHash(0);
@@ -397,9 +397,12 @@ auto KeyPageStorage::hash(const bcos::crypto::Hash::Ptr& hashImpl, bool /*useHas
                     }
                     else
                     {  // sys table
-                        auto hash = hashImpl->hash(data->table);
-                        hash ^= hashImpl->hash(data->key);
-                        hash ^= entry.hash(data->table, data->key, hashImpl, m_blockVersion);
+                        auto hash = entry.hash(data->table, data->key, hashImpl, m_blockVersion);
+                        if (!features.get(ledger::Features::Flag::bugfix_keypage_system_entry_hash))
+                        {  // v3.6.1 open this bugfix default
+                            hash ^= hashImpl->hash(data->table);
+                            hash ^= hashImpl->hash(data->key);
+                        }
                         localEntriesHash ^= hash;
                         ++entrycount;
                     }
@@ -413,6 +416,7 @@ auto KeyPageStorage::hash(const bcos::crypto::Hash::Ptr& hashImpl, bool /*useHas
     totalHash ^= pagesHash;
     totalHash ^= entriesHash;
     KeyPage_LOG(INFO) << LOG_DESC("hash") << LOG_KV("size", allData.size())
+                      << LOG_KV("blockVersion", m_blockVersion)
                       << LOG_KV("readLength", m_readLength) << LOG_KV("writeLength", m_writeLength)
                       << LOG_KV("pageCount", pageCount) << LOG_KV("entrycount", entrycount)
                       << LOG_KV("entriesHash", entriesHash.hex())

diff --git a/bcos-table/src/KeyPageStorage.h b/bcos-table/src/KeyPageStorage.h
@@ -144,7 +144,7 @@ class KeyPageStorage : public virtual storage::StateStorageInterface
                                               callback) const override;
 
     crypto::HashType hash(
-        const bcos::crypto::Hash::Ptr& hashImpl, bool /*useHashV310*/) const override;
+        const bcos::crypto::Hash::Ptr& hashImpl, const ledger::Features& features) const override;
 
     void rollback(const Recoder& recoder) override;
 
@@ -888,10 +888,10 @@ class KeyPageStorage : public virtual storage::StateStorageInterface
             m_invalidPageKeys.clear();
             if (!entries.empty() && pageKey != entries.rbegin()->first)
             {
-                KeyPage_LOG(WARNING) << LOG_DESC("import page with invalid pageKey")
-                                     << LOG_KV("pageKey", toHex(pageKey))
-                                     << LOG_KV("validPageKey", toHex(entries.rbegin()->first))
-                                     << LOG_KV("count", entries.size());
+                KeyPage_LOG(DEBUG) << LOG_DESC("import page with invalid pageKey")
+                                   << LOG_KV("pageKey", toHex(pageKey))
+                                   << LOG_KV("validPageKey", toHex(entries.rbegin()->first))
+                                   << LOG_KV("count", entries.size());
                 m_invalidPageKeys.insert(std::string(pageKey));
             }
             if (entries.empty())

diff --git a/bcos-table/src/StateStorage.h b/bcos-table/src/StateStorage.h
@@ -391,11 +391,13 @@ class BaseStorage : public virtual storage::StateStorageInterface,
         STORAGE_LOG(INFO) << "Successful merged records" << LOG_KV("count", count);
     }
 
-    crypto::HashType hash(const bcos::crypto::Hash::Ptr& hashImpl, bool useHashV310) const override
+    crypto::HashType hash(
+        const bcos::crypto::Hash::Ptr& hashImpl, const ledger::Features& features) const override
     {
         bcos::crypto::HashType totalHash;
-        auto blockVersion = useHashV310 ? (uint32_t)bcos::protocol::BlockVersion::V3_1_VERSION :
-                                          (uint32_t)bcos::protocol::BlockVersion::V3_0_VERSION;
+        auto blockVersion = features.get(ledger::Features::Flag::bugfix_statestorage_hash) ?
+                                (uint32_t)bcos::protocol::BlockVersion::V3_1_VERSION :
+                                (uint32_t)bcos::protocol::BlockVersion::V3_0_VERSION;
 
         std::vector<bcos::crypto::HashType> hashes(m_buckets.size());
         tbb::parallel_for(tbb::blocked_range<size_t>(0U, m_buckets.size()), [&, this](
@@ -434,7 +436,6 @@ class BaseStorage : public virtual storage::StateStorageInterface,
             totalHash ^= it;
         }
 
-
         return totalHash;
     }
 

diff --git a/bcos-table/src/StateStorageInterface.h b/bcos-table/src/StateStorageInterface.h
@@ -20,6 +20,7 @@
  */
 #pragma once
 
+#include "bcos-framework/ledger/Features.h"
 #include "bcos-framework/storage/StorageInterface.h"
 #include "bcos-framework/storage/Table.h"
 #include "tbb/enumerable_thread_specific.h"
@@ -107,7 +108,7 @@ class StateStorageInterface : public virtual storage::TraverseStorageInterface
     }
 
     virtual crypto::HashType hash(
-        const bcos::crypto::Hash::Ptr& hashImpl, bool useHashV310) const = 0;
+        const bcos::crypto::Hash::Ptr& hashImpl, const ledger::Features& features) const = 0;
     virtual void setPrev(std::shared_ptr<StorageInterface> prev)
     {
         std::unique_lock<std::shared_mutex> lock(m_prevMutex);

diff --git a/bcos-table/test/unittests/libtable/Table.cpp b/bcos-table/test/unittests/libtable/Table.cpp
@@ -224,7 +224,7 @@ BOOST_AUTO_TEST_CASE(removeFromCache)
     deleteEntry->setStatus(Entry::DELETED);
     BOOST_CHECK_NO_THROW(table->setRow("name", *deleteEntry));
 
-    auto hashs = tableFactory->hash(hashImpl, false);
+    auto hashs = tableFactory->hash(hashImpl, ledger::Features());
 
     auto tableFactory2 = std::make_shared<StateStorage>(nullptr);
     BOOST_CHECK(tableFactory2->createTable(tableName, valueField));
@@ -234,7 +234,7 @@ BOOST_AUTO_TEST_CASE(removeFromCache)
     auto deleteEntry2 = std::make_optional(table2->newEntry());
     deleteEntry2->setStatus(Entry::DELETED);
     BOOST_CHECK_NO_THROW(table2->setRow("name", *deleteEntry2));
-    auto hashs2 = tableFactory2->hash(hashImpl, false);
+    auto hashs2 = tableFactory2->hash(hashImpl, ledger::Features());
 
     BOOST_CHECK_EQUAL_COLLECTIONS(hashs.begin(), hashs.end(), hashs2.begin(), hashs2.end());
 }