[Go] Implement GetProofByHash #1249
Conversation
|
Added tests |
|
Another merge crept up, these muddle up the PRs a bit, you should use something along the lines of |
|
Rebased as requested. PTAL |
gdbelvin
force-pushed
the
hash
branch
3 times, most recently
from
6a85d30 to
f537683
Compare
|
Rebased again. PTAL |
| @@ -105,6 +101,19 @@ type getEntryAndProofResponse struct { | |||
| AuditPath []string `json:"audit_path"` // the corresponding proof | |||
| } | |||
|
|
|||
| type getProofByHashResponse struct { | |||
| LeafIndex int `json:"leaf_index"` // The 0-based index of the end entity corresponding to the "hash" parameter. | |||
| AuditPath []string `json:"audit_path"` // An array of base64-encoded Merkle Tree nodes proving the inclusion of the chosen certificate. | |||
There was a problem hiding this comment.
Does it work if you make this field a [][]byte instead (and strip out the base64 decoding at line 423)?
|
Hmm... Am I missing some reference from |
gdbelvin
force-pushed
the
hash
branch
4 times, most recently
from
69dda11 to
53113ef
Compare
|
We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm. |
gdbelvin
force-pushed
the
hash
branch
3 times, most recently
from
7fcc5f9 to
63c35e6
Compare
|
CLAs look good, thanks! |
gdbelvin
force-pushed
the
hash
branch
4 times, most recently
from
67d3d8d to
9f25b9d
Compare
gdbelvin
changed the title
| @@ -319,6 +319,7 @@ type TimestampedEntry struct { | |||
| X509Entry ASN1Cert | |||
| JSONData []byte | |||
| PrecertEntry PreCert | |||
| JSONData []byte | |||
There was a problem hiding this comment.
Same as the one at line 320? This causes a build error. 😉
|
Fixed |
| @@ -308,3 +317,12 @@ func (c *LogClient) GetSTH() (sth *ct.SignedTreeHead, err error) { | |||
| sth.TreeHeadSignature = *ds | |||
| return | |||
| } | |||
|
|
|||
| // GetProofByHash returns an audit path for the hash of an SCT. | |||
| func (c *LogClient) GetProofByHash(hash []byte, treeSize uint64) (GetProofByHashResponse, error) { | |||
There was a problem hiding this comment.
Big enough that GetProofByHashResponse should be returned as a pointer?
There was a problem hiding this comment.
That would also allow returning nil in the case of an error.
There was a problem hiding this comment.
We have some historical baggage, but might as well take a context.Context as the first parameter on new methods?
|
Done with this round of review, should be straightforward... 😃 |
| func (c *LogClient) GetProofByHash(hash []byte, treeSize uint64) (GetProofByHashResponse, error) { | ||
| var resp GetProofByHashResponse | ||
| func (c *LogClient) GetProofByHash(ctx context.Context, hash []byte, treeSize uint64) (*GetProofByHashResponse, error) { | ||
| resp := new(GetProofByHashResponse) |
There was a problem hiding this comment.
Oops, you've seen my comment in #1285? I like var better than new: it seems there's no case where new does anything that you can't do with var and &, so I'd rather have just one way of doing things, for consistency, and let the compiler do the escape analysis...
And either way, I'd still want it declared "at the last moment". 😉
|
LGTM, waiting on Travis to merge. |
|
Will need a rebase (and you can squash into one commit, if you want). |
|
Rebased on master |
This PR includes the code required to fetch inclusion proofs
Contributes to #1159