Define internal Kingdom ComputationParticipants and MeasurementLogEntries services.

[SanjayVas]

This change is 

[SanjayVas]

These were missed in #52. CC @uakyol

[wangyaopw]

Reviewable status: 0 of 6 files reviewed, 2 unresolved discussions (waiting on @SanjayVas)

---

src/main/kotlin/org/wfanet/measurement/kingdom/deploy/gcloud/spanner/measurement.sdl, line 253 at r1 (raw file):

measurement.internal.kingdom.RequisitionDetails

any benefit using INT64 here?

it is not a key or index, and we are not likely to query on the column in any use case.

---

src/main/proto/wfa/measurement/internal/kingdom/computation_participant.proto, line 70 at r1 (raw file):

DuchyMeasurementLogEntry

Are we going to create a row in the MeasurementLogEntries table during the failComputationParticipant() call?

I am wondering how you will query this field when getting the ComputationParticipant later since this log id is not included anywhere in the ComputationParticipant table

[SanjayVas]

Reviewable status: 0 of 6 files reviewed, 2 unresolved discussions (waiting on @wangyaopw)

---

src/main/kotlin/org/wfanet/measurement/kingdom/deploy/gcloud/spanner/measurement.sdl, line 253 at r1 (raw file):

Previously, wangyaopw (Yao Wang) wrote…

measurement.internal.kingdom.RequisitionDetails

any benefit using INT64 here?

it is not a key or index, and we are not likely to query on the column in any use case.

We use INT64s for any internal IDs, and avoid exposing those values outside of internal services. That principle still applies here, it's just that we don't have a table with this particular ID in its primary key and instead have a config file. If we decide to move from a config file to a Duchies table in the future, we can do so without having to change the column type.

---

src/main/proto/wfa/measurement/internal/kingdom/computation_participant.proto, line 70 at r1 (raw file):

Are we going to create a row in the MeasurementLogEntries table during the failComputationParticipant() call?

Yes.

I am wondering how you will query this field when getting the ComputationParticipant later since this log id is not included anywhere in the ComputationParticipant table.

You know you have an entry in the DuchyMeasurementLogEntries table for a given ComputationParticipant in the FAILED state, and every DuchyMeasurementLogEntries row has a corresponding MeasurementLogEntries row. You can query the DuchyMeasurementLogEntries table using the PK of the ComputationParticipant, since it has those columns.

[SanjayVas]

Reviewable status: 0 of 6 files reviewed, 2 unresolved discussions (waiting on @wangyaopw)

---

src/main/kotlin/org/wfanet/measurement/kingdom/deploy/gcloud/spanner/measurement.sdl, line 253 at r1 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

We use INT64s for any internal IDs, and avoid exposing those values outside of internal services. That principle still applies here, it's just that we don't have a table with this particular ID in its primary key and instead have a config file. If we decide to move from a config file to a Duchies table in the future, we can do so without having to change the column type.

Slight correction: We do have a table with the internal Duchy ID in its primary key, but we don't have a canonical Duchies table.

[wangyaopw]

Reviewable status: 0 of 6 files reviewed, 2 unresolved discussions (waiting on @SanjayVas)

---

src/main/kotlin/org/wfanet/measurement/kingdom/deploy/gcloud/spanner/measurement.sdl, line 253 at r1 (raw file):

canonical
sg

---

src/main/proto/wfa/measurement/internal/kingdom/computation_participant.proto, line 70 at r1 (raw file):

ComputationParticipant
If using the PK of the ComputationParticipant, the query would return all logs for that ComputationParticipant. How do we find the one that should be put here as failure_log_entry? Are we going to assume that only one of those logs is PERMANENT error log? that won't be guaranteed I think.

[SanjayVas]

Reviewable status: 0 of 6 files reviewed, 2 unresolved discussions (waiting on @SanjayVas and @wangyaopw)

---

src/main/proto/wfa/measurement/internal/kingdom/computation_participant.proto, line 70 at r1 (raw file):

Are we going to assume that only one of those logs is PERMANENT error log? that won't be guaranteed I think.

It will be, since the only way to add a DuchyMeasurementLogEntries row for a PERMANENT error is through FailComputationParticipant, and FailComputationParticipant would add that row transactionally with updating the ComputationParticipant state.

I updated the documentation for the internal CreateDuchyMeasurementLogEntryRequest to clarify that it cannot be used to add PERMANENT error entries, similar to the documentation for the system CreateComputationLogEntry method.

[wangyaopw]

Reviewable status: 0 of 6 files reviewed, 2 unresolved discussions (waiting on @SanjayVas)

---

src/main/proto/wfa/measurement/internal/kingdom/computation_participant.proto, line 70 at r1 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

Are we going to assume that only one of those logs is PERMANENT error log? that won't be guaranteed I think.

It will be, since the only way to add a DuchyMeasurementLogEntries row for a PERMANENT error is through FailComputationParticipant, and FailComputationParticipant would add that row transactionally with updating the ComputationParticipant state.

I updated the documentation for the internal CreateDuchyMeasurementLogEntryRequest to clarify that it cannot be used to add PERMANENT error entries, similar to the documentation for the system CreateComputationLogEntry method.

yep, that should work.

[SanjayVas]

Dismissed @wangyaopw from a discussion.
Reviewable status: 0 of 6 files reviewed, all discussions resolved (waiting on @corbantek, @oliver-amzn, and @zachcwc)

[zachcwc]

---

src/main/proto/wfa/measurement/internal/kingdom/measurement_consumer.proto, line 32 at r3 (raw file):

  // The *preferred* Certificate belonging to this MeasurementConsumer.
  Certificate preferred_certificate = 3;

Are we keeping track of preferred cert so that we can fall back on non-preferred certs if preferred becomes compromised or something?

[SanjayVas]

Reviewable status: 0 of 8 files reviewed, 1 unresolved discussion (waiting on @corbantek, @oliver-amzn, and @zachcwc)

---

src/main/proto/wfa/measurement/internal/kingdom/measurement_consumer.proto, line 32 at r3 (raw file):

Previously, zachcwc wrote…

Are we keeping track of preferred cert so that we can fall back on non-preferred certs if preferred becomes compromised or something?

This field is to support the field of the same name in the v2alpha public API. I assume your question is why that field exists there rather than here.

We keep track of all certs since they may be referenced by an existing public API resources (e.g. Measurement or PublicKey). The reason we have the preferred cert here is so that it's easy to figure out which one you should use for a new signature.

For more info, see the section on certificate preference in https://github.com/world-federation-of-advertisers/cross-media-measurement-api/blob/640987b5196e26fe717a47875f603360d6c11346/docs/certificates.md.

[SanjayVas]

Reviewable status: 0 of 8 files reviewed, 1 unresolved discussion (waiting on @corbantek, @oliver-amzn, and @zachcwc)

---

src/main/proto/wfa/measurement/internal/kingdom/measurement_consumer.proto, line 32 at r3 (raw file):

I assume your question is why that field exists there rather than here.

To clarify, I mean that I assume your question is really about why we have a preferred_certificate field in the public API resource message rather than why we have one here in the internal representation.

[zachcwc]

---

src/main/proto/wfa/measurement/internal/kingdom/measurement_consumer.proto, line 32 at r3 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

I assume your question is why that field exists there rather than here.

To clarify, I mean that I assume your question is really about why we have a preferred_certificate field in the public API resource message rather than why we have one here in the internal representation.

Yes that is right thank you!

[zachcwc]

Reviewed 5 of 6 files at r1, 2 of 2 files at r2.
Reviewable status: 6 of 8 files reviewed, all discussions resolved (waiting on @corbantek, @oliver-amzn, and @zachcwc)

[zachcwc]

Reviewed 1 of 2 files at r3.
Reviewable status: 7 of 8 files reviewed, all discussions resolved (waiting on @corbantek, @oliver-amzn, and @zachcwc)

[zachcwc]

Reviewed 1 of 2 files at r3.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @corbantek and @oliver-amzn)