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

Conversation

@gdbelvin
Copy link
Contributor

This brings the relationship between MapRevisions and Log indexes back into parity.

Previously log index = map revision -1
Now, log index = map revision

Fixes #703
Rebased on #760. Merge that first.

Previously, the signer used a verifying trillian log client in the
signer. This:
- Required a separate build step to acquire the verifying key
- Was tied to a confusing and only partially implemented multi-tennant
  configuration system.

This commit simplifies the situation to make a pure grpc call to the
trillian log backend, simplifying debugging, and makes things consistent
consistent with the way we are querying the trillian map server, and is
also consistent with the way the Certificate Transparency frontends
treat their own trillian backends.
The appender interface is a defunct wrapper for the trillian log.

The admin interface is a defunct api for multi-tennant configuration.
This brings the relationship between MapRevisions and Log indexes back
into parity.

Previously log index = map revision -1
Now, log index  = map revision

Fixes google#703
@liamsi
Copy link
Contributor

liamsi commented Aug 17, 2017

How does this relate to #761?

@codecov-io
Copy link

codecov-io commented Aug 17, 2017

Codecov Report

Merging #762 into master will decrease coverage by 0.69%.
The diff coverage is 4.25%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #762     +/-   ##
=========================================
- Coverage   49.18%   48.48%   -0.7%     
=========================================
  Files          30       28      -2     
  Lines        2507     2475     -32     
=========================================
- Hits         1233     1200     -33     
- Misses       1077     1090     +13     
+ Partials      197      185     -12
Impacted Files Coverage Δ
core/keyserver/keyserver.go 0% <0%> (ø) ⬆️
core/signer/signer.go 9.48% <0%> (-1.92%) ⬇️
integration/testutil.go 70% <100%> (+1.11%) ⬆️
core/client/kt/verify.go 52.63% <100%> (ø) ⬆️

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 5a851ef...bdffb7d. Read the comment docs.

@gdbelvin
Copy link
Contributor Author

I made a bit of a confusing swap with the PRs. This is actually the one that you just LGTM'ed
#761 contains an additional fix the the ListHistory command.

Copy link
Contributor

@liamsi liamsi left a comment

Choose a reason for hiding this comment

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

LGTM. These seem to be exactly the changes I've reviewed in #761)

@gdbelvin gdbelvin merged commit 33ad021 into google:master Aug 18, 2017
@gdbelvin gdbelvin deleted the fix/703 branch August 18, 2017 08:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants