Skip to content
This repository has been archived by the owner on Oct 11, 2024. It is now read-only.

Verify Revisions in StreamRevisions #1323

Merged
merged 6 commits into from
Jul 16, 2019
Merged

Conversation

gdbelvin
Copy link
Contributor

By verifying revisions inside of StreamRevisions,
we can reuse VerifyGetRevision and have better control
over the updating of LogRoot

@gdbelvin gdbelvin requested a review from a team as a code owner July 16, 2019 15:55
@gdbelvin gdbelvin requested a review from pav-kv July 16, 2019 16:01
@codecov
Copy link

codecov bot commented Jul 16, 2019

Codecov Report

Merging #1323 into master will increase coverage by 0.09%.
The diff coverage is 7.4%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1323      +/-   ##
==========================================
+ Coverage   29.96%   30.06%   +0.09%     
==========================================
  Files          47       47              
  Lines        3841     3829      -12     
==========================================
  Hits         1151     1151              
+ Misses       2512     2500      -12     
  Partials      178      178
Impacted Files Coverage Δ
core/client/get_and_verify.go 23.59% <0%> (ø) ⬆️
core/integration/monitor_tests.go 0% <0%> (ø) ⬆️
core/client/mutations.go 0% <0%> (ø) ⬆️
core/monitor/verify.go 0% <0%> (ø) ⬆️
core/monitor/monitor.go 18.66% <16.66%> (+1.79%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d5aba9b...4ad6b04. Read the comment docs.

Copy link
Contributor

@pav-kv pav-kv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM % nits.

cmd/keytransparency-monitor/main.go Show resolved Hide resolved
@@ -94,7 +94,7 @@ func (c *Client) VerifiedGetLatestRevision(ctx context.Context) (*types.LogRootV
// VerifiedGetRevision fetches the requested revision from the key server.
// It also verifies the consistency of the latest log root against the last seen log root.
// Returns the latest log root and the requested map root.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update the comment.

core/client/mutations.go Outdated Show resolved Hide resolved
core/client/mutations.go Outdated Show resolved Hide resolved
@gdbelvin gdbelvin merged commit 0e5e668 into google:master Jul 16, 2019
@gdbelvin gdbelvin deleted the monitor branch July 16, 2019 16:38
gdbelvin added a commit to gdbelvin/keytransparency that referenced this pull request Jul 18, 2019
* 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)
  ...
gdbelvin added a commit to gdbelvin/keytransparency that referenced this pull request Jul 18, 2019
* 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)
  ...
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants