fix checksum verification review gaps

Author: charpty

kv_cache_manager/client/src/internal/util/checksum_verify_util.h

@@ -6,27 +6,15 @@
 
 namespace kv_cache_manager {
 
-// Two-stage block checksum verification for the read path.
-//
-// Stage 1 (fast): XOR-aggregate expected and actual into one uint64 each and
-// compare once. Each block's contribution is multiplied by a position-dependent
-// odd constant before being XORed in, so a plain block swap (expected=[A,B] vs
-// actual=[B,A]) changes the aggregate — the fast path is order-sensitive.
-// Common case: a Load batch that is fully consistent returns in O(1) compares
-// regardless of batch size, with no per-block branching.
-//
-// Stage 2 (slow, only on fast-path mismatch or when strict_mode=true): walk the
-// batch and report the indices of every block whose checksum disagrees. The
-// caller uses these indices to log per-block diagnostics or publish a per-block
-// ChecksumMismatchEvent.
+// Walk the batch and report the indices of every block whose checksum disagrees.
+// The caller uses these indices to log per-block diagnostics or publish a
+// per-block ChecksumMismatchEvent.
 //
 // Sentinels: expected[i] == 0 means "no checksum was stored for this block"
 // (legacy data or legacy client). Such entries are skipped in both stages.
 //
-// strict_mode (typically driven by KVCM_CHECKSUM_STRICT_MODE) bypasses the fast
-// aggregate and always compares per block. Kept as a diagnostic knob for
-// on-call: when triaging a suspected data-integrity issue you may want per-block
-// index output without relying on the fast-path fallback triggering.
+// strict_mode is kept for API compatibility with earlier revisions; verification
+// is always per-block so the function cannot accept XOR-cancelled batches.
 struct ChecksumVerifyResult {
     bool mismatch = false;
     std::vector<std::size_t> faulty_indices; // populated only when mismatch == true
@@ -35,46 +23,12 @@ struct ChecksumVerifyResult {
 inline ChecksumVerifyResult VerifyBatchChecksums(const std::vector<std::int64_t> &expected,
                                                  const std::vector<std::int64_t> &actual,
                                                  bool strict_mode) {
+    (void)strict_mode;
     ChecksumVerifyResult result;
     if (expected.size() != actual.size()) {
         result.mismatch = true;
         return result;
     }
-    if (strict_mode) {
-        for (std::size_t i = 0; i < actual.size(); ++i) {
-            if (expected[i] == 0) {
-                continue;
-            }
-            if (expected[i] != actual[i]) {
-                result.faulty_indices.push_back(i);
-            }
-        }
-        result.mismatch = !result.faulty_indices.empty();
-        return result;
-    }
-    // fast path: xor aggregate with a position-dependent odd multiplier so a
-    // plain block swap changes the aggregate (integer multiplication is not
-    // GF(2)-linear, so (A*m0)^(B*m1) != (B*m0)^(A*m1) in general). Multiplying
-    // by an odd constant is bijective mod 2^64, preserving per-element entropy.
-    // kIndexSalt is the 2^64/phi constant used by splitmix64/xxhash; any well-
-    // dispersed odd constant works.
-    constexpr std::uint64_t kIndexSalt = 0x9E3779B97F4A7C15ULL;
-    std::uint64_t expected_xor = 0;
-    std::uint64_t actual_xor = 0;
-    bool any_compared = false;
-    for (std::size_t i = 0; i < actual.size(); ++i) {
-        if (expected[i] == 0) {
-            continue;
-        }
-        const std::uint64_t multiplier = (2ULL * i + 1ULL) * kIndexSalt;
-        expected_xor ^= static_cast<std::uint64_t>(expected[i]) * multiplier;
-        actual_xor ^= static_cast<std::uint64_t>(actual[i]) * multiplier;
-        any_compared = true;
-    }
-    if (!any_compared || expected_xor == actual_xor) {
-        return result; // all match (or nothing to check)
-    }
-    // fast path detected a mismatch; locate the offending block(s)
     for (std::size_t i = 0; i < actual.size(); ++i) {
         if (expected[i] == 0) {
             continue;

kv_cache_manager/client/src/internal/util/test/checksum_verify_util_test.cc

@@ -7,7 +7,7 @@ using namespace kv_cache_manager;
 
 class ChecksumVerifyUtilTest : public TESTBASE {};
 
-// Fast path：所有 block 一致 -> mismatch=false, faulty_indices 空。
+// 所有 block 一致 -> mismatch=false, faulty_indices 空。
 TEST_F(ChecksumVerifyUtilTest, FastPathAllMatch) {
     std::vector<int64_t> expected = {0x1111, 0x2222, 0x3333};
     std::vector<int64_t> actual = expected;
@@ -16,7 +16,7 @@ TEST_F(ChecksumVerifyUtilTest, FastPathAllMatch) {
     EXPECT_TRUE(r.faulty_indices.empty());
 }
 
-// Fast path：检测到不匹配后应该回填 faulty_indices 让上层逐块打日志。
+// 检测到不匹配后应该回填 faulty_indices 让上层逐块打日志。
 TEST_F(ChecksumVerifyUtilTest, FastPathDetectsMismatchAndLocatesIndex) {
     std::vector<int64_t> expected = {0x1111, 0x2222, 0x3333};
     std::vector<int64_t> actual = {0x1111, 0xFFFF, 0x3333}; // block #1 错
@@ -26,7 +26,7 @@ TEST_F(ChecksumVerifyUtilTest, FastPathDetectsMismatchAndLocatesIndex) {
     EXPECT_EQ(r.faulty_indices[0], 1u);
 }
 
-// 多个错位 block：fallback 阶段把所有错的都列出来。
+// 多个错位 block：把所有错的都列出来。
 TEST_F(ChecksumVerifyUtilTest, FastPathListsAllFaultyBlocks) {
     std::vector<int64_t> expected = {0x1111, 0x2222, 0x3333, 0x4444};
     std::vector<int64_t> actual = {0xAAAA, 0x2222, 0xBBBB, 0x4444}; // #0, #2 错
@@ -45,7 +45,7 @@ TEST_F(ChecksumVerifyUtilTest, FastPathSentinelZeroIsSkipped) {
     EXPECT_FALSE(r.mismatch);
 }
 
-// 全 sentinel：fast 路径没有可比较的项，等同于全 match。
+// 全 sentinel：没有可比较的项，等同于全 match。
 TEST_F(ChecksumVerifyUtilTest, FastPathAllSentinelsTreatedAsMatch) {
     std::vector<int64_t> expected = {0, 0, 0};
     std::vector<int64_t> actual = {0xAA, 0xBB, 0xCC};
@@ -62,7 +62,7 @@ TEST_F(ChecksumVerifyUtilTest, SizeMismatchReturnsMismatchWithoutIndices) {
     EXPECT_TRUE(r.faulty_indices.empty());
 }
 
-// Strict mode：行为跟 fast fallback 一致 (per-block 比对)，但跳过 fast 聚合阶段。
+// strict_mode 参数保留兼容；当前实现始终逐块比对。
 TEST_F(ChecksumVerifyUtilTest, StrictModeMatchesFastFallback) {
     std::vector<int64_t> expected = {0x1111, 0, 0x3333, 0x4444};
     std::vector<int64_t> actual = {0xAAAA, 0x2222, 0x3333, 0xBBBB}; // #0, #3 错；#1 是 sentinel
@@ -81,8 +81,7 @@ TEST_F(ChecksumVerifyUtilTest, StrictModeAllMatch) {
     EXPECT_FALSE(r.mismatch);
 }
 
-// Block swap (读串): expected=[A,B], actual=[B,A]. 老 XOR 聚合无序会漏，
-// 加了 position-dependent 奇数乘子后 fast path 也能识别并回填两个 faulty index。
+// Block swap (读串): expected=[A,B], actual=[B,A]. 必须识别并回填两个 faulty index。
 TEST_F(ChecksumVerifyUtilTest, FastPathCatchesBlockSwap) {
     std::vector<int64_t> expected = {0xAAAA, 0xBBBB};
     std::vector<int64_t> actual = {0xBBBB, 0xAAAA};
@@ -98,8 +97,7 @@ TEST_F(ChecksumVerifyUtilTest, FastPathCatchesBlockSwap) {
 }
 
 // Same-delta 成对突变：每个 block 都被同一 delta 改写 (expected=[A,B],
-// actual=[A^X, B^X])。老 XOR fast 会 delta 对消而漏；新的乘法聚合不再有 GF(2)-
-// 线性，所以两条路径都能识别。这里同时断言，防止将来实现回退成纯 XOR 时漏检回归。
+// actual=[A^X, B^X])。老 XOR fast 会 delta 对消而漏；逐块比对必须识别。
 TEST_F(ChecksumVerifyUtilTest, DetectsSameDeltaPairedMutation) {
     constexpr int64_t kDelta = 0x0F0F0F0F0F0F0F0FLL;
     std::vector<int64_t> expected = {0xAAAA, 0xBBBB};
@@ -111,3 +109,17 @@ TEST_F(ChecksumVerifyUtilTest, DetectsSameDeltaPairedMutation) {
     ASSERT_TRUE(r_strict.mismatch);
     EXPECT_EQ(r_strict.faulty_indices.size(), 2u);
 }
+
+// High-bit 成对突变：奇数乘法聚合也会让最高位 delta 在偶数个 block 中抵消。
+// 逐块比对不能接受这种 batch。
+TEST_F(ChecksumVerifyUtilTest, DetectsHighBitPairedMutation) {
+    constexpr int64_t kHighBit = static_cast<int64_t>(0x8000000000000000ULL);
+    std::vector<int64_t> expected = {0x1111, 0x2222, 0x3333};
+    std::vector<int64_t> actual = {0x1111 ^ kHighBit, 0x2222 ^ kHighBit, 0x3333};
+
+    auto r = VerifyBatchChecksums(expected, actual, /*strict_mode=*/false);
+    ASSERT_TRUE(r.mismatch);
+    ASSERT_EQ(r.faulty_indices.size(), 2u);
+    EXPECT_EQ(r.faulty_indices[0], 0u);
+    EXPECT_EQ(r.faulty_indices[1], 1u);
+}

kv_cache_manager/client/src/transfer_client_impl.cc

@@ -38,6 +38,15 @@ bool IsChecksumHashableBlock(const BlockBuffer &block_buffer) {
     });
 }
 
+std::vector<size_t> GetIovSizeShape(const BlockBuffer &block_buffer) {
+    std::vector<size_t> shape;
+    shape.reserve(block_buffer.iovs.size());
+    for (const auto &iov : block_buffer.iovs) {
+        shape.push_back(iov.size);
+    }
+    return shape;
+}
+
 bool HashBlocksByIovShape(const BlockBuffers &block_buffers,
                           SdkBufferCheckPool::CellHandle &handle,
                           size_t max_check_iov_num,
@@ -50,7 +59,7 @@ bool HashBlocksByIovShape(const BlockBuffers &block_buffers,
 
     BlockBuffers chunk;
     std::vector<size_t> chunk_indices;
-    size_t chunk_iov_num = 0;
+    std::vector<size_t> chunk_iov_shape;
     size_t chunk_total_iovs = 0;
 
     auto flush_chunk = [&]() -> bool {
@@ -68,7 +77,7 @@ bool HashBlocksByIovShape(const BlockBuffers &block_buffers,
         }
         chunk.clear();
         chunk_indices.clear();
-        chunk_iov_num = 0;
+        chunk_iov_shape.clear();
         chunk_total_iovs = 0;
         return true;
     };
@@ -79,13 +88,14 @@ bool HashBlocksByIovShape(const BlockBuffers &block_buffers,
             KVCM_LOG_ERROR("block [%zu] has invalid iov_num [%zu] for checksum hash", i, iov_num);
             return false;
         }
-        if (!chunk.empty() && (iov_num != chunk_iov_num || chunk_total_iovs + iov_num > max_check_iov_num)) {
+        auto iov_shape = GetIovSizeShape(block_buffers[i]);
+        if (!chunk.empty() && (iov_shape != chunk_iov_shape || chunk_total_iovs + iov_num > max_check_iov_num)) {
             if (!flush_chunk()) {
                 return false;
             }
         }
         if (chunk.empty()) {
-            chunk_iov_num = iov_num;
+            chunk_iov_shape = std::move(iov_shape);
         }
         chunk.push_back(block_buffers[i]);
         chunk_indices.push_back(i);

kv_cache_manager/client/test/transfer_client_test.cc

@@ -305,6 +305,31 @@ TEST_F(TransferClientTest, TestCreateAcceptsMetaChecksumSpec) {
     delete init_params.regist_span;
 }
 
+TEST_F(TransferClientTest, TestCreateRejectsUnsupportedChecksumAlgo) {
+    auto init_params = init_params_;
+    init_params.regist_span = new RegistSpan();
+    init_params.regist_span->base = malloc(1024 * 1024);
+    init_params.regist_span->size = 1024 * 1024;
+    init_params.storage_configs = R"([
+        {
+            "type": "file",
+            "global_unique_name": "test_nfs",
+            "storage_spec": {
+                "root_path": "/tmp/test/",
+                "key_count_per_file": 5
+            },
+            "integrity": {
+                "enable_meta_checksum": true,
+                "algo": "unknown_algo"
+            }
+        }
+    ])";
+    auto client = TransferClient::Create(client_config_, init_params);
+    EXPECT_EQ(client, nullptr);
+    free(init_params.regist_span->base);
+    delete init_params.regist_span;
+}
+
 // expected_checksums 全 0 -> sentinel 跳过校验，行为 == 老路径，不会因 checksum 不匹配返回错误。
 TEST_F(TransferClientTest, TestLoadKvCachesExpectedHashesAllZeroSkipsCheck) {
     auto client = TransferClient::Create(client_config_, init_params_);