Piano: Extremely Simple, Single-server PIR with Sublinear Server Computation #196

cxiao129 · 2024-10-29T13:36:21Z

Piano: Extremely Simple, Single-server PIR with Sublinear Server Computation

…utation

github-actions · 2024-10-29T13:36:39Z

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

cxiao129 · 2024-10-29T13:37:07Z

I have read the CLA Document and I hereby sign the CLA

qxzhou1010

请把piano 的实现整体放在 psi/experimental/ 目录下

qxzhou1010 · 2024-11-20T06:28:26Z

psi/piano/util.h

+std::string Uint128ToBytes(uint128_t value);
+
+// Generates a random 128-bit key using the provided RNG
+PrfKey128 RandKey128(std::mt19937_64& rng);


使用yacl::crypto::Prg ，其余涉及到 rng 到地方也替换为同样的Prg.

qxzhou1010 · 2024-11-20T06:31:05Z

psi/piano/util.h

+
+namespace psi::piano {
+
+constexpr size_t DBEntrySize = 8;  // has to be a multiple of 8


这里指的是数据库里一行数据的长度吗？单位是 bit 还是字节？以及这里取8的出处是？

单位是字节，原本使用vector<uint64_t>表示数据，所以取8，现在改为使用vector<uint8_t>，同时根据参数确定数据库里一行数据的字节数

qxzhou1010 · 2024-11-20T06:38:34Z

psi/piano/util.h

+constexpr size_t DBEntrySize = 8;  // has to be a multiple of 8
+constexpr size_t DBEntryLength = DBEntrySize / 8;
+
+using PrfKey128 = uint128_t;


引入这个头文件#include "yacl/base/int128.h" ，显式的使用yacl里提供的uint128_t 这个类型

qxzhou1010 · 2024-11-20T06:40:43Z

psi/piano/util.h

+PrfKey RandKey(std::mt19937_64& rng);
+
+// Evaluates PRF using 128-bit key and returns a 64-bit result
+uint64_t PRFEval128(const PrfKey128* key, uint64_t x);


对于 uint128_t 来说，可以直接pass by value ，不必使用pointer

qxzhou1010 · 2024-11-20T06:45:25Z

psi/piano/util.h

+constexpr size_t DBEntryLength = DBEntrySize / 8;
+
+using PrfKey128 = uint128_t;
+using DBEntry = std::array<uint64_t, DBEntryLength>;


看到了后面大量的关于DBEntry 相关的操作。是否可以考虑，将DBEntry 进一步抽象为一个class 或者 struct？

已封装为class

qxzhou1010 · 2024-11-20T06:55:48Z

psi/piano/util.h

+using DBEntry = std::array<uint64_t, DBEntryLength>;
+using PrfKey = PrfKey128;
+
+uint128_t BytesToUint128(const std::string& bytes);


使用 yacl中ByteContainerView，或者std::vector<uint8_t> 来表示 bytes，而不是std::string

qxzhou1010 · 2024-11-20T07:10:58Z

psi/piano/client.h

+
+class QueryServiceClient {
+ public:
+  static constexpr uint64_t FailureProbLog2 = 40;


kFailureProbLog2

qxzhou1010 · 2024-11-20T07:11:37Z

psi/piano/client.h

+class QueryServiceClient {
+ public:
+  static constexpr uint64_t FailureProbLog2 = 40;
+  uint64_t totalQueryNum{};


请放在private 下

qxzhou1010 · 2024-11-20T07:17:20Z

psi/piano/client.h

+
+  std::vector<LocalSet> primary_sets_;
+  std::vector<LocalBackupSet> local_backup_sets_;
+  std::map<uint64_t, DBEntry> local_cache_;


如果不要求有序，使用std::unordered_map 即可

qxzhou1010 · 2024-11-20T07:37:06Z

psi/piano/util.h

+
+class PRSetWithShortTag {
+ public:
+  uint32_t Tag;


放在 private 下，或者用 struct 即可

qxzhou1010 · 2024-11-20T07:47:57Z

psi/piano/util.cc

+  return entry;
+}
+
+uint64_t DefaultHash(uint64_t key) {


这个方法的出处是？

这里只是为了生成测试数据，所以使用了简单高效的FNV哈希，已转移到测试代码中实现，逻辑和参数选取参考自https://en.wikipedia.org/wiki/Fowler%E2%80%93Noll%E2%80%93Vo_hash_function

… private, etc

qxzhou1010 · 2024-12-10T07:30:48Z

experimental/pir/piano/piano_test.cc

-  SPDLOG_INFO("DB N: %lu, Entry Size %lu Bytes, DB Size %lu MB\n",
-              params.db_size, DBEntrySize,
-              params.db_size * DBEntrySize / 1024 / 1024);
+  SPDLOG_INFO("DB N: %lu, Entry Size %lu Bytes, DB Size %lu MB\n", entry_num,


你这个SPDLOG_INFO写错了，直接用{} 当占位符。测试跑出来是这样：

[2024-12-10 07:22:51.291] [info] [piano_test.cc:91] DB N: %lu, Entry Size %lu Bytes, DB Size %lu MB [2024-12-10 07:22:51.291] [info] [piano_test.cc:95] Chunk Size: %lu, Set Size: %lu [2024-12-10 07:22:51.307] [info] [piano_test.cc:99] DB Real N: %lu `` 哥，你写了代码，好歹自己跑一下测试，看一眼啊

在之前的测试中只运行了bazel test，导致输出信息不全，对此我深感抱歉。现在我已经在bazel run和bazel-bin的二进制文件下都进行了测试，确保信息能正确显示。

qxzhou1010 · 2024-12-10T08:04:50Z

experimental/pir/piano/client.h


 private:
-  uint64_t db_size_;
+  static constexpr uint64_t kFailureProbLog2 = 40;


这个const没必要定义在这里吧？一般放在一个文件最上方的位置

qxzhou1010 · 2024-12-10T08:05:32Z

experimental/pir/piano/client.h

@@ -78,24 +69,28 @@ class QueryServiceClient {
  DBEntry OnlineSingleQuery(uint64_t x);
  std::vector<DBEntry> OnlineMultipleQueries(
      const std::vector<uint64_t>& queries);
+  uint64_t getTotalQueryNumber() const { return total_query_num_; };


方法名首字母统一用大写，请自己检查其他地方类似的问题，然后修改掉。

已检查并修改

qxzhou1010 · 2024-12-10T08:09:04Z

experimental/pir/piano/client.h

-  uint64_t db_size_;
+  static constexpr uint64_t kFailureProbLog2 = 40;
+  uint64_t total_query_num_{};
+  uint64_t entry_num_;


qxzhou1010 · 2024-12-10T08:11:45Z

experimental/pir/piano/client.cc

@@ -67,8 +68,17 @@ void QueryServiceClient::InitializeLocalSets() {
  local_miss_elements_.clear();
  uint32_t tagCounter = 0;


局部变量的命令风格统一用下划线来连接小写字符 ，代码里还有很多类似的命名风格不统一的地方，请自己检查并修改。

已检查并修改

qxzhou1010 · 2024-12-10T08:17:26Z

experimental/pir/piano/piano_test.cc

@@ -45,13 +46,34 @@ std::vector<DBEntry> RunClient(QueryServiceClient& client,
  return client.OnlineMultipleQueries(queries);
 }

+std::vector<uint8_t> FNVHash(uint64_t key) {
+  constexpr uint64_t FNV_offset_basis = 14695981039346656037ULL;


常量命名风格用k开头，后面遵循驼峰规则，且不用定义在方法里。

qxzhou1010 · 2024-12-10T08:18:21Z

experimental/pir/piano/piano_test.cc

+  const auto actual_query_num = params.is_total_query_num
+                                    ? client.getTotalQueryNumber()
+                                    : params.query_num;
+  const auto queries = GenerateQueries(actual_query_num, entry_num);


生成 query 不应该是 Client 提供的方法之一吗？为什么这里是一个 free function ？

这里是用于生成随机索引作为测试数据，在两方场景下，可以在客户端本地生成，再通过OnlineMultipleQueries函数将其作为参数读取。此外，需要模拟根据明文索引直接查询数据库，来验证PIR方案的正确性。

qxzhou1010 · 2024-12-10T08:25:34Z

experimental/pir/piano/util.cc

-std::pair<uint64_t, uint64_t> GenParams(const uint64_t db_size) {
-  const double targetChunkSize = 2 * std::sqrt(static_cast<double>(db_size));
+std::pair<uint64_t, uint64_t> GenParams(const uint64_t entry_num) {
+  const double targetChunkSize = 2 * std::sqrt(static_cast<double>(entry_num));
  uint64_t ChunkSize = 1;


变量命名风格

qxzhou1010 · 2024-12-10T08:32:40Z

experimental/pir/piano/client.cc

+
+namespace pir::piano {
+
+uint64_t primaryNumParam(const double q, const double chunk_size,


方法名风格问题，最好注明这个计算方式的来源

已添加注释说明计算公式

qxzhou1010 · 2024-12-10T08:47:40Z

experimental/pir/piano/client.h

+
+namespace pir::piano {
+
+struct LocalSet {


LocalSet, LocalBackupSet, LocalBackupSetGroup, LocalReplacementGroup

这几个类型主要拿来干嘛的？请添加有效注释说明一下。

已添加注释说明

好的，修改后请提交最新的commit

已提交commit，请问我是否需要将代码从 psi/experimental/ 移动到 psi/experiment/ 目录下，我看已有实验性代码都在该目录下

需要的，请移动到psi/experiment/pir/ 目录下。这一点非常抱歉，我在之前的comments中，写成了experimental

…ode correctness with test cases

qxzhou1010 · 2024-12-19T07:45:14Z

experimental/pir/piano/client.h

+
+namespace pir::piano {
+
+struct LocalSet {


需要的，请移动到psi/experiment/pir/ 目录下。这一点非常抱歉，我在之前的comments中，写成了experimental

qxzhou1010 · 2024-12-19T07:46:45Z

experimental/pir/piano/client.h

+  const uint64_t log2_k_ = 40;
+  // Converts log2_k_ from base-2 to natural logarithm using the change of base
+  // formula
+  const double natural_log_k_ = std::log(2) * static_cast<double>(log2_k_);


这两个变量建议可以作为一个全局的const variable，放在某个.h 文件的 namespace 下。

修改为constexpr，并放在client.h文件的namespace下

qxzhou1010 · 2024-12-19T09:17:14Z

experimental/pir/piano/client.cc

+    const uint64_t primary_set_per_thread =
+        ((primary_set_num_ + thread_num_ - 1) / thread_num_) + 1;
+    const uint64_t backup_set_per_thread =
+        ((total_backup_set_num_ + thread_num_ - 1) / thread_num_) + 1;


前面的计算不是已经表示向上取整了，为什么最外层还需要再+1 呢？

是的，这里的+1是多余的，已经删除了

qxzhou1010 · 2024-12-19T09:36:21Z

experimental/pir/piano/client.cc

+
+    // Store the replacement
+    yacl::crypto::Prg<uint64_t> prg(yacl::crypto::SecureRandU64());
+    for (uint64_t k = 0; k < backup_set_num_per_chunk_; k++) {


这里没看太明白，这里应该对应论文Figrue-1中的，Offline 预处理阶段的第三个阶段的Update back table 。

论文的描述中，for i in {0, 1, , 2..} / {j} ，这里的 i 应该是遍历每一个chunk 吧？当然对于当前正在处理的第 j 个 chunk 会跳过去，但是代码里好像没有看到这样的逻辑，而是把每一个i都无区别的用来做了这里的处理？

之前是在多线程中处理所有chunk，出来后再XOR i-th chunk，以实现跳过的逻辑。但这样会有一些不必要的计算开销，已经修改为，在多线程内根据backup hints和chunk间的对应关系，实现跳过处理。

…ckup hint generation logic

Piano: Extremely Simple, Single-server PIR with Sublinear Server Comp…

500d454

…utation

cxiao129 added 3 commits October 29, 2024 21:40

Modify Readme format.

22bab60

Modify for unittest

057d545

Modify for macos_ut

cd4cc90

qxzhou1010 requested review from qxzhou1010 and huocun-ant November 1, 2024 02:20

qxzhou1010 reviewed Nov 1, 2024

View reviewed changes

cxiao129 added 2 commits November 4, 2024 11:26

Move to experimental/pir/piano

0f90718

Delete original file

b25e91a

qxzhou1010 reviewed Nov 20, 2024

View reviewed changes

cxiao129 and others added 4 commits November 26, 2024 17:48

Abstract DBEntry into a class, use prg in yacl, make member variables…

14b90b4

… private, etc

Merge branch 'secretflow:main' into main

4f61534

Format piano.proto with clang-format

5397010

Merge branch 'main' of github.com:cxiao129/psi into main

af8e4a1

qxzhou1010 self-requested a review December 10, 2024 07:11

qxzhou1010 reviewed Dec 10, 2024

View reviewed changes

cxiao129 added 2 commits December 18, 2024 18:04

Unified naming conventions, added meaningful comments, and verified c…

9f900cb

…ode correctness with test cases

Updated the README for correct rendering of math formulas on github

2b114c7

qxzhou1010 reviewed Dec 19, 2024

View reviewed changes

Move to specified directory, optimize global variables, and modify ba…

d04c226

…ckup hint generation logic


		namespace psi::piano {

		constexpr size_t DBEntrySize = 8; // has to be a multiple of 8

		@@ -67,8 +68,17 @@ void QueryServiceClient::InitializeLocalSets() {
		local_miss_elements_.clear();
		uint32_t tagCounter = 0;


		namespace pir::piano {

		uint64_t primaryNumParam(const double q, const double chunk_size,

Piano: Extremely Simple, Single-server PIR with Sublinear Server Computation #196

Are you sure you want to change the base?

Piano: Extremely Simple, Single-server PIR with Sublinear Server Computation #196

Conversation

cxiao129 commented Oct 29, 2024

github-actions bot commented Oct 29, 2024 • edited Loading

cxiao129 commented Oct 29, 2024

qxzhou1010 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cxiao129 Dec 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Oct 29, 2024 •

edited

Loading

cxiao129 Dec 20, 2024 •

edited

Loading