[Bug]: Should not add after zip

[maths644311798]

Issue Type

Usability

Modules Involved

MPC protocol

Have you reproduced the bug with SPU HEAD?

No

Have you searched existing issues?

Yes

SPU Version

spu 0.9.3.dev

OS Platform and Distribution

Ubuntu 22.04

Python Version

3.10

Compiler Version

gcc 13.2

Current Behavior?

In /libspu/mpc/cheetah/ot/ot_util.cc, the function NdArrayRef OpenShare(const NdArrayRef &shr, ReduceOp op, size_t nbits, std::shared_ptr<Communicator> conn) looks strange. The zip operation will harm the ring structure of $Z_m$. If we add after zip, the result may be unexpected. xor is OK. I only doubt the correctness of add.

Standalone code to reproduce the issue

NdArrayRef OpenShare(const NdArrayRef &shr, ReduceOp op, size_t nbits,
                     std::shared_ptr<Communicator> conn) {
  SPU_ENFORCE(conn != nullptr);
  SPU_ENFORCE(shr.eltype().isa<Ring2k>());
  SPU_ENFORCE(op == ReduceOp::ADD or op == ReduceOp::XOR);

  auto field = shr.eltype().as<Ring2k>()->field();
  size_t fwidth = SizeOf(field) * 8;
  if (nbits == 0) {
    nbits = fwidth;
  }
  SPU_ENFORCE(nbits <= fwidth, "nbits out-of-bound");
  bool packable = fwidth > nbits;
  if (not packable) {
    return conn->allReduce(op, shr, "open");
  }

  size_t numel = shr.numel();
  size_t compact_numel = CeilDiv(numel * nbits, fwidth);

  NdArrayRef out(shr.eltype(), {(int64_t)numel});
  DISPATCH_ALL_FIELDS(field, [&]() {
    auto inp = absl::MakeConstSpan(&shr.at<ring2k_t>(0), numel);
    auto oup = absl::MakeSpan(&out.at<ring2k_t>(0), compact_numel);

    size_t used = ZipArray(inp, nbits, oup);
    SPU_ENFORCE_EQ(used, compact_numel);

    std::vector<ring2k_t> opened;
    if (op == ReduceOp::XOR) {
      opened = conn->allReduce<ring2k_t, std::bit_xor>(oup, "open");
    } else {
      opened = conn->allReduce<ring2k_t, std::plus>(oup, "open");
    }

    oup = absl::MakeSpan(&out.at<ring2k_t>(0), numel);
    UnzipArray(absl::MakeConstSpan(opened), nbits, oup);
  });
  return out.reshape(shr.shape());
}

Relevant log output

No response

[fionser]

You are right, for additive shares, we should add 1 more bit between two zipped slots.