Refactor HMSS to remove sketch params

[renjiezh]

Depends on public api change world-federation-of-advertisers/cross-media-measurement-api#210

[wfa-reviewable]

This change is 

[SanjayVas]

Reviewed 25 of 25 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ple13 and @renjiezh)

---

src/main/kotlin/org/wfanet/measurement/duchy/service/api/v2alpha/RequisitionFulfillmentService.kt line 129 at r1 (raw file):

            recordLlv2RequisitionLocally(computationToken, externalRequisitionKey, blob.blobKey)
          }
          val seed =

Why is this changed? It doesn't appear to be related to the API change.

---

src/main/proto/wfa/measurement/system/v1alpha/computation.proto line 168 at r1 (raw file):

    // Configuration for the Honest Majority Share Shuffle protocol.
    message HonestMajorityShareShuffle {

FYI this change is only okay because this hasn't been used yet. Otherwise the system API follows similar rules to the CMMS public API.

---

src/test/kotlin/org/wfanet/measurement/duchy/daemon/mill/shareshuffle/HonestMajorityShareShuffleMillTest.kt line 960 at r1 (raw file):

            registerCount = 100
            maximumCombinedFrequency = 30
            ringModulus = 127

If changing the value in HMSS_PARAMETERS would also require this value to change, then this should just come from that constant.

[renjiezh]

Reviewable status: 23 of 25 files reviewed, 2 unresolved discussions (waiting on @ple13 and @SanjayVas)

---

src/main/kotlin/org/wfanet/measurement/duchy/service/api/v2alpha/RequisitionFulfillmentService.kt line 129 at r1 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

Why is this changed? It doesn't appear to be related to the API change.

This is unused code which raises compiler warning.

---

src/test/kotlin/org/wfanet/measurement/duchy/daemon/mill/shareshuffle/HonestMajorityShareShuffleMillTest.kt line 960 at r1 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

If changing the value in HMSS_PARAMETERS would also require this value to change, then this should just come from that constant.

Done.

[ple13]

Reviewed 23 of 25 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @renjiezh and @SanjayVas)

---

src/main/k8s/testing/secretfiles/hmss_protocol_config_config.textproto line 6 at r2 (raw file):

  noise_mechanism: DISCRETE_GAUSSIAN
  reach_and_frequency_ring_modulus: 127
  reach_ring_modulus: 127

1. nit: use reach_only to mirror the reach_and_frequency?
2. The modulus for the reach only protocol is a prime number. We should have a comment to indicate that the modulus used in the reach only protocol must be a prime.

---

src/main/proto/wfa/measurement/internal/kingdom/protocol_config.proto line 145 at r2 (raw file):

    // The modulus used in the protocol for Reach Measurement.
    int32 reach_ring_modulus = 4;

nit: same as above. pls consider reach_only_ring_modulus to mirror the R/F name.

---

src/main/proto/wfa/measurement/system/v1alpha/computation.proto line 173 at r2 (raw file):

          [(google.api.field_behavior) = REQUIRED];
      // The modulus used in the MPC protocol for Reach Measurement.
      int32 reach_ring_modulus = 2 [(google.api.field_behavior) = REQUIRED];

nit: same about the name as above.

[SanjayVas]

Reviewed 1 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @ple13 and @renjiezh)

---

src/main/k8s/testing/secretfiles/hmss_protocol_config_config.textproto line 6 at r2 (raw file):

nit: use reach_only to mirror the reach_and_frequency?

This is mirroring the Measurement types of REACH and REACH_AND_FREQUENCY.

The only place we use "reach only" is for reach only LLv2, as that's a separate protocol. The Measurement type is still REACH.

[renjiezh]

Reviewable status: 24 of 25 files reviewed, 1 unresolved discussion (waiting on @ple13 and @SanjayVas)

---

src/main/k8s/testing/secretfiles/hmss_protocol_config_config.textproto line 6 at r2 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

nit: use reach_only to mirror the reach_and_frequency?

This is mirroring the Measurement types of REACH and REACH_AND_FREQUENCY.

The only place we use "reach only" is for reach only LLv2, as that's a separate protocol. The Measurement type is still REACH.

Done.
Comment added in protocol_config.proto.

[ple13]

Reviewed 1 of 1 files at r3, 16 of 16 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @renjiezh)

[stevenwarejones]

Reviewed 10 of 25 files at r1, 1 of 2 files at r2, 1 of 1 files at r3, 16 of 16 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @renjiezh)

[SanjayVas]

Reviewed 1 of 2 files at r2, 1 of 1 files at r3, 16 of 16 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @renjiezh)

---

src/main/proto/wfa/measurement/internal/duchy/protocol/BUILD.bazel line 126 at r4 (raw file):

proto_library(
    name = "share_shuffle_sketch_params_proto",

proto library name must match file name

Code quote:

share_shuffle_sketch_params_proto

[renjiezh]

Reviewable status: 27 of 28 files reviewed, 1 unresolved discussion (waiting on @ple13, @SanjayVas, and @stevenwarejones)

---

src/main/proto/wfa/measurement/internal/duchy/protocol/BUILD.bazel line 126 at r4 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

proto library name must match file name

Done.
Nice catch!

[SanjayVas]

Reviewed 2 of 2 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @renjiezh)

[renjiezh]

Reviewed 9 of 25 files at r1, 1 of 2 files at r2, 1 of 1 files at r3, 13 of 16 files at r4, 2 of 2 files at r5, 3 of 3 files at r6, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @renjiezh)