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

Comments

Make check-set mutation semantics optional#1307

Merged
gdbelvin merged 1 commit intogoogle:masterfrom
gdbelvin:rm_previous
Jul 15, 2019
Merged

Make check-set mutation semantics optional#1307
gdbelvin merged 1 commit intogoogle:masterfrom
gdbelvin:rm_previous

Conversation

@gdbelvin
Copy link
Contributor

@gdbelvin gdbelvin commented Jul 2, 2019

The hash chain in user entries is not currently being used. Entry.Previous
was intended to help make account history verification efficient, but it did
not meet that goal.

In the meantime, Entry.Previous enforces quite a tight sequence of get,
modify, update. Any delay in Key Transparency sequencing causes these updates
to fail, and continuously retrying exacerbates the issue.

@gdbelvin gdbelvin requested a review from a team as a code owner July 2, 2019 17:00
@codecov

This comment has been minimized.

@gdbelvin gdbelvin requested a review from jtoohill July 3, 2019 10:31
Copy link
Contributor

@mhutchinson mhutchinson left a comment

Choose a reason for hiding this comment

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

I'm happy with this from an implementation perspective. Will leave for someone on KT client side to determine whether the approach of removing the backchain is inline with expected direction.

@gdbelvin
Copy link
Contributor Author

gdbelvin commented Jul 9, 2019

After discussion in the "Rubber Duck Club™", we realized that preserving "Check Set" semantics is still important for clients that want to do "Fetch Modify Push".

I'll modify this CL to make "Check Set" optional rather than removed.

@gdbelvin gdbelvin force-pushed the rm_previous branch 2 times, most recently from ae70868 to 03e5d50 Compare July 9, 2019 09:53
@gdbelvin gdbelvin changed the title Remove Entry.Previous Make check-set mutation semantics optional Jul 9, 2019
@gdbelvin
Copy link
Contributor Author

This PR should greatly reduce the production error rate. PTAL

@gdbelvin gdbelvin assigned mhutchinson and unassigned libinjG Jul 15, 2019
@gdbelvin gdbelvin merged commit cdd3046 into google:master Jul 15, 2019
@gdbelvin gdbelvin deleted the rm_previous branch July 15, 2019 18:05
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.

4 participants