Encapsulate Client Verifier State in test vectors #1316
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1316 +/- ##
==========================================
- Coverage 30.06% 29.98% -0.08%
==========================================
Files 46 46
Lines 3815 3828 +13
==========================================
+ Hits 1147 1148 +1
- Misses 2492 2504 +12
Partials 176 176
Continue to review full report at Codecov.
|
| // for the next rpc's request.last_verified_tree_size. | ||
| bool trust_new_log = 2; | ||
| reserved 2; // was trust_new_log | ||
| // last_verified_log_root encodes the client verifier state. |
There was a problem hiding this comment.
optional: In Go generated code the field name is formatted as LastVerifiedLogRoot, which is not consistent with formatting in the comment. My strategy is to not start comments with field names in proto files.
There was a problem hiding this comment.
Alas, this is a common problem. I used to use the Go capitalization so the comments would come out correctly, but the mixing of different capitalization schemes in the proto file seemed messy and language specific.
Unsure if not starting comments with field names fixes the problem, since doing so still violates go style in the generated code.
| // LogRootRequest contains the information needed to requetst and verify LogRoot. | ||
| message LogRootRequest { | ||
| // root_hash is the log root hash. | ||
| bytes root_hash = 1; |
There was a problem hiding this comment.
Why does the log root request contain the log root hash? Isn't it supposed to be returned as a result of the request?
There was a problem hiding this comment.
The LogRootRequest is intended to encode the client's verifier state, as is being done in this PR; but it has a second purpose, which is to make request, response pairs statelessly verifiable. At the moment, an observer of the system, or a server interceptor can't tell if a response is valid because the first hash of the consistency proof is not included in either the request or the response.
|
Updated. PTAL |
* master: Pass along err message (google#1314) Remove unnessesary func() (google#1319)
* master: (106 commits) Remove unused logVerifier (google#1324) Verify Revisions in StreamRevisions (google#1323) Pair verifier functions (google#1322) Split VerifyRevision into Verify{LogRoot,MapRevision (google#1318) Make Previous hash check optional (google#1307) Remove VerifySignedMapRoot from VerifierInterface (google#1320) Remove trailing whitespace (google#1321) Encapsulate Client Verifier State in test vectors (google#1316) Pass along err message (google#1314) Remove unnessesary func() (google#1319) New test vector transcript format (google#1315) Track map revision inside mutation (google#1310) Move verifier to its own package (google#1312) go generate ./... (google#1306) Fix proto copying in revisions and paginator tests. (google#1309) Fix proto copying in server_test. (google#1308) go mod tidy (google#1305) Use new TrillianMapWrite API (google#1304) Configurable maximum queue depth for metric reporting. (google#1303) Proposal to refine docker deployment (google#1302) ...
* master: (95 commits) Remove unused logVerifier (google#1324) Verify Revisions in StreamRevisions (google#1323) Pair verifier functions (google#1322) Split VerifyRevision into Verify{LogRoot,MapRevision (google#1318) Make Previous hash check optional (google#1307) Remove VerifySignedMapRoot from VerifierInterface (google#1320) Remove trailing whitespace (google#1321) Encapsulate Client Verifier State in test vectors (google#1316) Pass along err message (google#1314) Remove unnessesary func() (google#1319) New test vector transcript format (google#1315) Track map revision inside mutation (google#1310) Move verifier to its own package (google#1312) go generate ./... (google#1306) Fix proto copying in revisions and paginator tests. (google#1309) Fix proto copying in server_test. (google#1308) go mod tidy (google#1305) Use new TrillianMapWrite API (google#1304) Configurable maximum queue depth for metric reporting. (google#1303) Proposal to refine docker deployment (google#1302) ...
In order to remove the client mutex, we need to fully extract and isolate client verifier state.
Explicitly putting the client verifier state in the test vectors is the first step towards doing that.
Blocked on #1315
Motivated by #1311