mtca: Serve CQRP Landmark Service compatible checkpoints - #8974
mtca: Serve CQRP Landmark Service compatible checkpoints#8974beautifulentropy wants to merge 1 commit into
Conversation
|
@beautifulentropy, this PR appears to contain configuration and/or SQL schema changes. Please ensure that a corresponding deployment ticket has been filed with the new values. |
aarongable
left a comment
aarongable
left a comment
There was a problem hiding this comment.
LGTM with nits, modulo upstream PR being updated for tests to pass.
|
|
||
| // Mirror identifies the mirror whose cosignatures must be verified | ||
| // before the MTCA serves them as part of a checkpoint. | ||
| Mirror cmd.MirrorConfig `validate:"required"` |
There was a problem hiding this comment.
Slight preference for a shared trees/config/ package which can hold both this and config.ID.
| if err != nil { | ||
| return nil, fmt.Errorf("creating mirror verifier: %s", err) | ||
| } | ||
| m.mirrorVerifier = mirrorVerifier |
There was a problem hiding this comment.
Build this earlier, so it can be populated when mtca is first built on line 68, rather than assigning after the fact.
| mirrorID string | ||
| mirrorVerifier *cosignature.Verifier | ||
|
|
||
| servedCheckpointID int64 |
There was a problem hiding this comment.
IMO, .New() should read from S3 to populate this.
There was a problem hiding this comment.
Ideally Preflight() rather than New(); that's where we do other storage-involved work at startup. It's nice to be able to separate that from simply constructing the object.
|
Could you fill out the PR description? Also, a PR naming nit: CQRP refers to "Chrome's Landmark Service", not "CQRP Landmark Service." But I think it would be simplest to say "mtca: serve checkpoints with 2 cosignatures," and mention in the description that this is a requirement of the CQRP. https://googlechrome.github.io/chromerootprogram/cqrp/draft-policy/ |
3 participants
No description provided.