Refactor common STR verification functionality

[masomel]

Since the client and auditor both need to perform some common consistency checks on the STRs, all this functionality should be refactored into a generic auditor module. This will also require the implementation of STR verification over a range of epochs.

See https://github.com/coniks-sys/coniks-go/pull/155#discussion_r104835245

[liamsi]

We will merge #155 first

[vqhuy]

Generally, I think this can be:

• have a generic interface for different auditor types (i.e. Auditor interface)
• have a default auditor (AudState) which implements Auditor interface, and unexport functions such as CompareWithVerified or VerifySTRConsistency, etc.
• make AudState a nested struct of AuditLog and ConsistencyChecks, and extend the interface implementations as needed.

[masomel]

have a default auditor (AudState) which implements Auditor interface, and unexport functions such as CompareWithVerified or VerifySTRConsistency, etc.

I don't fully understand what we gain by doing this. Can you please elaborate on this idea?

[vqhuy]

I think with #173, client- and auditor-side would share (very much) the same logic code of Auditor interface implementation (e.g., Audit function). All they do are:

• fetch the latest (or a range of) STR(s) from the server
• verify the consistency & update the state.

By doing that, we can let each side extend the interface as needed, and reuse the verifying logic from AudState, much like what we did with SignedTreeRoot and DirSTR.Serialize(): https://github.com/coniks-sys/coniks-go/blob/master/protocol/str.go#L21-L23.

[masomel]

with #173, client- and auditor-side would share (very much) the same logic code of Auditor interface implementation (e.g., Audit function) All they do are:

• fetch the latest (or a range of) STR(s) from the server
• verify the consistency & update the state.

Is this because after #173 the clients will start sending the separate STRs requests to the servers as auditors do now? I agree that the fetching of STRs from client-->auditor and auditor-->server is very similar, but the actual Auditing isn't that similar:

• Clients need to: (1) Verify the STR history from the server and save the latest STR, and (2) Verify the STR history from the auditor and compare the latest against the verified STR.
• Auditors need to: Verify the STR history from the server and save all STRs.

There is definitely functionality that can be refactored here, and I think I've done that so far, but I don't think we can efficiently use the same Audit function for both. But I do agree that we can definitely refactor the default functionality some more.

[vqhuy]

I agreed with what you said. But I think some functions like CompareWithVerified and VerifySTRConsistency are used only for verify the STR history from the server, and that is what I mean "share the same logic code". The other parts could be done by extending the interface implementation in separate functions.

For this pull, I think you can refactor the STR verification functionality, and use it in the AuditLog, since most of the code of ConsistencyChecks would be rewritten towards #173 (I'm doing it now).

[masomel]

I understand ConsistencyChecks will change as part of #173, but I'll still update it for now to demonstrate how the refactored logic could be used. You can then change it however you see fit for #173.

[vqhuy]

If it's possible, please remove the empty line after the function name. Please update the documentation in the comments (some function names are uppercase, or not matched with the new name).

[masomel]

Am I right to assume that this is ready to be merged? No new comments in over a week.