major refactor based on README.md and TODOs

Updated types, improved units tests, isolated most test data to have
it in one place, renamed and created new files to improve readability,
and fixed a bunch of minor TODOs.

1 files changed, 94 insertions, 208 deletions

diff --git a/trillian_test.go b/trillian_test.go
index b3c1653..1b0c923 100644
--- a/trillian_test.go
+++ b/trillian_test.go
@@ -5,11 +5,9 @@ import (
 	"testing"
 
 	"github.com/google/trillian"
-	"github.com/google/trillian/types"
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
-
-	"github.com/system-transparency/stfe/namespace/testdata"
+	ttypes "github.com/google/trillian/types"
+	"github.com/system-transparency/stfe/testdata"
+	"github.com/system-transparency/stfe/types"
 )
 
 func TestCheckQueueLeaf(t *testing.T) {
@@ -20,30 +18,31 @@ func TestCheckQueueLeaf(t *testing.T) {
 		wantErr     bool
 	}{
 		{
-			description: "bad response: trillian error",
+			description: "invalid: no Trillian response: error",
 			err:         fmt.Errorf("backend error"),
 			wantErr:     true,
 		},
 		{
-			description: "bad response: empty",
+			description: "invalid: no Trillian response: nil",
 			wantErr:     true,
 		},
 		{
-			description: "bad response: no queued leaf",
+			description: "invalid: no Trillian response: empty",
 			rsp:         &trillian.QueueLeafResponse{},
 			wantErr:     true,
 		},
 		{
-			description: "ok response: duplicate leaf",
-			rsp:         makeTrillianQueueLeafResponse(t, testPackage, testdata.Ed25519Vk, testdata.Ed25519Sk, true),
+			description: "valid: gRPC status: duplicate",
+			rsp:         testdata.DefaultTQlr(t, true),
 		},
 		{
-			description: "ok response: new leaf",
-			rsp:         makeTrillianQueueLeafResponse(t, testPackage, testdata.Ed25519Vk, testdata.Ed25519Sk, false),
+			description: "valid: gRPC status: ok",
+			rsp:         testdata.DefaultTQlr(t, false),
 		},
 	} {
-		if err := checkQueueLeaf(table.rsp, table.err); (err != nil) != table.wantErr {
-			t.Errorf("got error %v, but wanted error %v in test %q", err, table.wantErr, table.description)
+		err := checkQueueLeaf(table.rsp, table.err)
+		if got, want := err != nil, table.wantErr; got != want {
+			t.Errorf("got error %v but wanted %v in test %q", got, want, table.description)
 		}
 	}
 }
@@ -51,101 +50,90 @@ func TestCheckQueueLeaf(t *testing.T) {
 func TestCheckGetLeavesByRange(t *testing.T) {
 	for _, table := range []struct {
 		description string
-		req         *GetEntriesRequest
+		req         *types.GetEntriesV1
 		rsp         *trillian.GetLeavesByRangeResponse
 		err         error
 		wantErr     bool
 	}{
 		{
-			description: "bad response: trillian error",
-			req:         &GetEntriesRequest{Start: 0, End: 1},
+			description: "invalid: no Trillian response: error",
+			req:         &types.GetEntriesV1{Start: 0, End: 1},
 			err:         fmt.Errorf("backend error"),
 			wantErr:     true,
 		},
 		{
-			description: "bad response: empty",
-			req:         &GetEntriesRequest{Start: 0, End: 1},
+			description: "invalid: no Trillian response: nil",
+			req:         &types.GetEntriesV1{Start: 0, End: 1},
 			wantErr:     true,
 		},
 		{
-			description: "bad response: no leaves",
-			req:         &GetEntriesRequest{Start: 0, End: 1},
+			description: "invalid: bad Trillian response: no leaves",
+			req:         &types.GetEntriesV1{Start: 0, End: 1},
 			rsp: func(rsp *trillian.GetLeavesByRangeResponse) *trillian.GetLeavesByRangeResponse {
 				rsp.Leaves = nil
 				return rsp
-			}(makeTrillianGetLeavesByRangeResponse(t, 0, 1, testPackage, testdata.Ed25519Vk, testdata.Ed25519Sk)),
+			}(testdata.DefaultTGlbrr(t, 0, 1)),
 			wantErr: true,
 		},
 		{
-			description: "bad response: no signed log root",
-			req:         &GetEntriesRequest{Start: 0, End: 1},
+			description: "invalid: bad Trillian response: no signed log root",
+			req:         &types.GetEntriesV1{Start: 0, End: 1},
 			rsp: func(rsp *trillian.GetLeavesByRangeResponse) *trillian.GetLeavesByRangeResponse {
 				rsp.SignedLogRoot = nil
 				return rsp
-			}(makeTrillianGetLeavesByRangeResponse(t, 0, 1, testPackage, testdata.Ed25519Vk, testdata.Ed25519Sk)),
+			}(testdata.DefaultTGlbrr(t, 0, 1)),
 			wantErr: true,
 		},
 		{
-			description: "bad response: no log root",
-			req:         &GetEntriesRequest{Start: 0, End: 1},
+			description: "invalid: bad Trillian response: no log root",
+			req:         &types.GetEntriesV1{Start: 0, End: 1},
 			rsp: func(rsp *trillian.GetLeavesByRangeResponse) *trillian.GetLeavesByRangeResponse {
 				rsp.SignedLogRoot.LogRoot = nil
 				return rsp
-			}(makeTrillianGetLeavesByRangeResponse(t, 0, 1, testPackage, testdata.Ed25519Vk, testdata.Ed25519Sk)),
+			}(testdata.DefaultTGlbrr(t, 0, 1)),
 			wantErr: true,
 		},
 		{
-			description: "bad response: truncated root",
-			req:         &GetEntriesRequest{Start: 0, End: 1},
+			description: "invalid: bad Trillian response: truncated log root",
+			req:         &types.GetEntriesV1{Start: 0, End: 1},
 			rsp: func(rsp *trillian.GetLeavesByRangeResponse) *trillian.GetLeavesByRangeResponse {
 				rsp.SignedLogRoot.LogRoot = rsp.SignedLogRoot.LogRoot[1:]
 				return rsp
-			}(makeTrillianGetLeavesByRangeResponse(t, 0, 1, testPackage, testdata.Ed25519Vk, testdata.Ed25519Sk)),
+			}(testdata.DefaultTGlbrr(t, 0, 1)),
 			wantErr: true,
 		},
 		{
-			description: "bad response: too many leaves",
-			req:         &GetEntriesRequest{Start: 0, End: 1},
-			rsp:         makeTrillianGetLeavesByRangeResponse(t, 0, 2, testPackage, testdata.Ed25519Vk, testdata.Ed25519Sk),
+			description: "invalid: bad Trillian response: too many leaves",
+			req:         &types.GetEntriesV1{Start: 0, End: 1},
+			rsp:         testdata.DefaultTGlbrr(t, 0, 2),
 			wantErr:     true,
 		},
 		{
-			description: "bad response: start is not a valid index",
-			req:         &GetEntriesRequest{Start: int64(testTreeSize), End: int64(testTreeSize)},
-			rsp: func(rsp *trillian.GetLeavesByRangeResponse) *trillian.GetLeavesByRangeResponse {
-				rsp.SignedLogRoot = makeLatestSignedLogRootResponse(t, 0, testTreeSize, testNodeHash).SignedLogRoot
-				return rsp
-			}(makeTrillianGetLeavesByRangeResponse(t, int64(testTreeSize)-1, int64(testTreeSize)-1, testPackage, testdata.Ed25519Vk, testdata.Ed25519Sk)),
-			wantErr: true,
-		},
-		{
-			description: "bad response: invalid leaf indices",
-			req:         &GetEntriesRequest{Start: 10, End: 11},
-			rsp:         makeTrillianGetLeavesByRangeResponse(t, 11, 12, testPackage, testdata.Ed25519Vk, testdata.Ed25519Sk),
+			description: "invalid: bad Trillian response: start is not a valid index",
+			req:         &types.GetEntriesV1{Start: 10, End: 10},
+			rsp:         testdata.DefaultTGlbrr(t, 9, 9),
 			wantErr:     true,
 		},
 		{
-			description: "ok response: interval refers to the latest leaf",
-			req:         &GetEntriesRequest{Start: int64(testTreeSize) - 1, End: int64(testTreeSize) - 1},
-			rsp: func(rsp *trillian.GetLeavesByRangeResponse) *trillian.GetLeavesByRangeResponse {
-				rsp.SignedLogRoot = makeLatestSignedLogRootResponse(t, 0, testTreeSize, testNodeHash).SignedLogRoot
-				return rsp
-			}(makeTrillianGetLeavesByRangeResponse(t, int64(testTreeSize)-1, int64(testTreeSize)-1, testPackage, testdata.Ed25519Vk, testdata.Ed25519Sk)),
+			description: "invalid: bad Trillian response: invalid leaf indices",
+			req:         &types.GetEntriesV1{Start: 10, End: 11},
+			rsp:         testdata.DefaultTGlbrr(t, 11, 12),
+			wantErr:     true,
 		},
 		{
-			description: "ok response: a bunch of leaves",
-			req:         &GetEntriesRequest{Start: 10, End: 20},
-			rsp:         makeTrillianGetLeavesByRangeResponse(t, 10, 20, testPackage, testdata.Ed25519Vk, testdata.Ed25519Sk),
+			description: "valid",
+			req:         &types.GetEntriesV1{Start: 10, End: 20},
+			rsp:         testdata.DefaultTGlbrr(t, 10, 20),
 		},
 	} {
-		if _, err := checkGetLeavesByRange(table.req, table.rsp, table.err); (err != nil) != table.wantErr {
-			t.Errorf("got error %v, but wanted error %v in test %q", err, table.wantErr, table.description)
+		err := checkGetLeavesByRange(table.req, table.rsp, table.err)
+		if got, want := err != nil, table.wantErr; got != want {
+			t.Errorf("got error %v but wanted %v in test %q", got, want, table.description)
 		}
 	}
 }
 
 func TestCheckGetInclusionProofByHash(t *testing.T) {
-	lp := makeTestLogParameters(t, nil)
 	for _, table := range []struct {
 		description string
 		rsp         *trillian.GetInclusionProofByHashResponse
@@ -153,16 +141,16 @@ func TestCheckGetInclusionProofByHash(t *testing.T) {
 		wantErr     bool
 	}{
 		{
-			description: "bad response: trillian error",
+			description: "invalid: no Trillian response: error",
 			err:         fmt.Errorf("backend failure"),
 			wantErr:     true,
 		},
 		{
-			description: "bad response: empty",
+			description: "invalid: no Trillian response: nil",
 			wantErr:     true,
 		},
 		{
-			description: "bad response: no proofs",
+			description: "invalid: bad Trillian response: no proofs",
 			rsp:         &trillian.GetInclusionProofByHashResponse{},
 			wantErr:     true,
 		},
@@ -171,27 +159,30 @@ func TestCheckGetInclusionProofByHash(t *testing.T) {
 			rsp: func(rsp *trillian.GetInclusionProofByHashResponse) *trillian.GetInclusionProofByHashResponse {
 				rsp.Proof[0] = nil
 				return rsp
-			}(makeTrillianGetInclusionProofByHashResponse(t, int64(testIndex), testProof)),
+			}(testdata.DefaultTGipbhr(t)),
 			wantErr: true,
 		},
 		{
 			description: "bad response: proof with invalid node hash",
-			rsp:         makeTrillianGetInclusionProofByHashResponse(t, int64(testIndex), [][]byte{make([]byte, testHashLen-1)}),
-			wantErr:     true,
+			rsp: func(rsp *trillian.GetInclusionProofByHashResponse) *trillian.GetInclusionProofByHashResponse {
+				rsp.Proof[0].Hashes = append(rsp.Proof[0].Hashes, make([]byte, 0))
+				return rsp
+			}(testdata.DefaultTGipbhr(t)),
+			wantErr: true,
 		},
 		{
-			description: "ok response",
-			rsp:         makeTrillianGetInclusionProofByHashResponse(t, int64(testIndex), testProof),
+			description: "valid",
+			rsp:         testdata.DefaultTGipbhr(t),
 		},
 	} {
-		if err := checkGetInclusionProofByHash(lp, table.rsp, table.err); (err != nil) != table.wantErr {
-			t.Errorf("got error %v, but wanted error %v in test %q", err, table.wantErr, table.description)
+		err := checkGetInclusionProofByHash(newLogParameters(t, nil), table.rsp, table.err)
+		if got, want := err != nil, table.wantErr; got != want {
+			t.Errorf("got error %v but wanted %v in test %q", got, want, table.description)
 		}
 	}
 }
 
 func TestCheckGetConsistencyProof(t *testing.T) {
-	lp := makeTestLogParameters(t, nil)
 	for _, table := range []struct {
 		description string
 		rsp         *trillian.GetConsistencyProofResponse
@@ -199,37 +190,40 @@ func TestCheckGetConsistencyProof(t *testing.T) {
 		wantErr     bool
 	}{
 		{
-			description: "bad response: trillian error",
+			description: "invalid: no Trillian response: error",
 			err:         fmt.Errorf("backend failure"),
 			wantErr:     true,
 		},
 		{
-			description: "bad response: empty",
+			description: "invalid: no Trillian response: nil",
 			wantErr:     true,
 		},
 		{
-			description: "bad response: no proof",
+			description: "invalid: bad Trillian response: no proof",
 			rsp:         &trillian.GetConsistencyProofResponse{},
 			wantErr:     true,
 		},
 		{
-			description: "bad response: proof with invalid node hash",
-			rsp:         makeTrillianGetConsistencyProofResponse(t, [][]byte{make([]byte, testHashLen-1)}),
-			wantErr:     true,
+			description: "invalid: bad Trillian response: proof with invalid node hash",
+			rsp: func(rsp *trillian.GetConsistencyProofResponse) *trillian.GetConsistencyProofResponse {
+				rsp.Proof.Hashes = append(rsp.Proof.Hashes, make([]byte, 0))
+				return rsp
+			}(testdata.DefaultTGcpr(t)),
+			wantErr: true,
 		},
 		{
-			description: "ok response",
-			rsp:         makeTrillianGetConsistencyProofResponse(t, testProof),
+			description: "valid",
+			rsp:         testdata.DefaultTGcpr(t),
 		},
 	} {
-		if err := checkGetConsistencyProof(lp, table.rsp, table.err); (err != nil) != table.wantErr {
-			t.Errorf("got error %v, but wanted error %v in test %q", err, table.wantErr, table.description)
+		err := checkGetConsistencyProof(newLogParameters(t, nil), table.rsp, table.err)
+		if got, want := err != nil, table.wantErr; got != want {
+			t.Errorf("got error %v but wanted %v in test %q", got, want, table.description)
 		}
 	}
 }
 
 func TestCheckGetLatestSignedLogRoot(t *testing.T) {
-	lp := makeTestLogParameters(t, nil)
 	for _, table := range []struct {
 		description string
 		rsp         *trillian.GetLatestSignedLogRootResponse
@@ -237,160 +231,52 @@ func TestCheckGetLatestSignedLogRoot(t *testing.T) {
 		wantErr     bool
 	}{
 		{
-			description: "bad trillian response: error",
+			description: "invalid: no Trillian response: error",
 			err:         fmt.Errorf("backend failure"),
 			wantErr:     true,
 		},
 		{
-			description: "bad trillian response: empty",
+			description: "invalid: no Trillian response: nil",
 			wantErr:     true,
 		},
 		{
-			description: "bad trillian response: no signed log root",
-			rsp:         &trillian.GetLatestSignedLogRootResponse{SignedLogRoot: nil},
-			wantErr:     true,
+			description: "invalid: bad Trillian response: no signed log root",
+			rsp: func(rsp *trillian.GetLatestSignedLogRootResponse) *trillian.GetLatestSignedLogRootResponse {
+				rsp.SignedLogRoot = nil
+				return rsp
+			}(testdata.DefaultTSlr(t)),
+			wantErr: true,
 		},
 		{
-			description: "bad trillian response: no log root",
+			description: "invalid: bad Trillian response: no log root",
 			rsp: func(rsp *trillian.GetLatestSignedLogRootResponse) *trillian.GetLatestSignedLogRootResponse {
 				rsp.SignedLogRoot.LogRoot = nil
 				return rsp
-			}(makeLatestSignedLogRootResponse(t, 0, 0, testNodeHash)),
+			}(testdata.DefaultTSlr(t)),
 			wantErr: true,
 		},
 		{
-			description: "bad trillian response: truncated log root",
+			description: "invalid: bad Trillian response: truncated log root",
 			rsp: func(rsp *trillian.GetLatestSignedLogRootResponse) *trillian.GetLatestSignedLogRootResponse {
 				rsp.SignedLogRoot.LogRoot = rsp.SignedLogRoot.LogRoot[1:]
 				return rsp
-			}(makeLatestSignedLogRootResponse(t, 0, 0, testNodeHash)),
+			}(testdata.DefaultTSlr(t)),
 			wantErr: true,
 		},
 		{
-			description: "bad trillian response: invalid root hash size",
-			rsp:         makeLatestSignedLogRootResponse(t, 0, 0, make([]byte, testHashLen-1)),
+			description: "invalid: bad Trillian response: truncated root hash",
+			rsp:         testdata.Tslr(t, testdata.Tlr(t, testdata.TreeSize, testdata.Timestamp, make([]byte, 31))),
 			wantErr:     true,
 		},
 		{
-			description: "ok response",
-			rsp:         makeLatestSignedLogRootResponse(t, 0, 0, testNodeHash),
+			description: "valid",
+			rsp:         testdata.DefaultTSlr(t),
 		},
 	} {
-		var lr types.LogRootV1
-		if err := checkGetLatestSignedLogRoot(lp, table.rsp, table.err, &lr); (err != nil) != table.wantErr {
-			t.Errorf("got error %v, but wanted error %v in test %q", err, table.wantErr, table.description)
+		var lr ttypes.LogRootV1
+		err := checkGetLatestSignedLogRoot(newLogParameters(t, nil), table.rsp, table.err, &lr)
+		if got, want := err != nil, table.wantErr; got != want {
+			t.Errorf("got error %v but wanted %v in test %q", got, want, table.description)
 		}
 	}
 }
-
-// makeTrillianQueueLeafResponse creates a valid trillian QueueLeafResponse
-// for a package `name` where the checksum is all zeros (32 bytes).  Namespace
-// is based on vk and sk (ed25519).
-//
-// Note: MerkleLeafHash and LeafIdentityHash are unset (not used by stfe).
-func makeTrillianQueueLeafResponse(t *testing.T, name, vk, sk []byte, dupCode bool) *trillian.QueueLeafResponse {
-	t.Helper()
-	leaf, signature := mustMakeEd25519ChecksumV1(t, name, make([]byte, 32), vk, sk)
-	s := status.New(codes.OK, "ok").Proto()
-	if dupCode {
-		s = status.New(codes.AlreadyExists, "duplicate").Proto()
-	}
-	return &trillian.QueueLeafResponse{
-		QueuedLeaf: &trillian.QueuedLogLeaf{
-			Leaf: &trillian.LogLeaf{
-				MerkleLeafHash:   nil, // not used by stfe
-				LeafValue:        leaf,
-				ExtraData:        signature,
-				LeafIndex:        0,   // not applicable (log is not pre-ordered)
-				LeafIdentityHash: nil, // not used by stfe
-			},
-			Status: s,
-		},
-	}
-}
-
-// makeTrillianGetInclusionProofByHashResponse populates a get-proof-by-hash
-// response.
-//
-// Note: SignedLogRoot is unset (not used by stfe).
-func makeTrillianGetInclusionProofByHashResponse(t *testing.T, index int64, path [][]byte) *trillian.GetInclusionProofByHashResponse {
-	t.Helper()
-	return &trillian.GetInclusionProofByHashResponse{
-		Proof: []*trillian.Proof{
-			&trillian.Proof{
-				LeafIndex: index,
-				Hashes:    path,
-			},
-		},
-		SignedLogRoot: nil,
-	}
-}
-
-// makeTrillianGetConsistencyProofResponse populates a get-consistency response.
-//
-// Note: LeafIndex is not applicable for a consistency proof (0), and
-// SignedLogRoot is unset (not used by stfe).
-func makeTrillianGetConsistencyProofResponse(t *testing.T, path [][]byte) *trillian.GetConsistencyProofResponse {
-	t.Helper()
-	return &trillian.GetConsistencyProofResponse{
-		Proof: &trillian.Proof{
-			LeafIndex: 0,
-			Hashes:    path,
-		},
-		SignedLogRoot: nil,
-	}
-}
-
-// makeTrillianGetLeavesByRangeResponse creates a range of leaves [start,end]
-// such that the package is `name`_<index> and the checksum is all zeros (32
-// bytes).  An Ed25519 namespace is used based on vk and sk.
-//
-// Note: MerkleLeafHash and LeafIdentityHash are unset (not used by stfe).
-func makeTrillianGetLeavesByRangeResponse(t *testing.T, start, end int64, name, vk, sk []byte) *trillian.GetLeavesByRangeResponse {
-	t.Helper()
-	leaves := make([]*trillian.LogLeaf, 0, end-start+1)
-	for i, n := start, end+1; i < n; i++ {
-		leaf, signature := mustMakeEd25519ChecksumV1(t, append(name, []byte(fmt.Sprintf("_%d", i))...), make([]byte, 32), vk, sk)
-		leaves = append(leaves, &trillian.LogLeaf{
-			MerkleLeafHash:   nil,
-			LeafValue:        leaf,
-			ExtraData:        signature,
-			LeafIndex:        i,
-			LeafIdentityHash: nil,
-		})
-	}
-	return &trillian.GetLeavesByRangeResponse{
-		Leaves:        leaves,
-		SignedLogRoot: makeLatestSignedLogRootResponse(t, 0, uint64(end)+1, make([]byte, 32)).SignedLogRoot,
-	}
-}
-
-// makeTrillianLogRoot: docdoc
-func makeTrillianLogRoot(t *testing.T, timestamp, size uint64, hash []byte) *types.LogRootV1 {
-	t.Helper()
-	return &types.LogRootV1{
-		TreeSize:       size,
-		RootHash:       hash,
-		TimestampNanos: timestamp,
-		Revision:       0,   // not used by stfe
-		Metadata:       nil, // not used by stfe
-	}
-}
-
-// makeLatestSignedLogRootResponse creates a new trillian STH.  Revision,
-// Metadata, Proof, KeyHint, and LogRootSignature are unsset.
-func makeLatestSignedLogRootResponse(t *testing.T, timestamp, size uint64, hash []byte) *trillian.GetLatestSignedLogRootResponse {
-	t.Helper()
-	rootBytes, err := makeTrillianLogRoot(t, timestamp, size, hash).MarshalBinary()
-	if err != nil {
-		t.Fatalf("failed to marshal root in test: %v", err)
-	}
-	return &trillian.GetLatestSignedLogRootResponse{
-		SignedLogRoot: &trillian.SignedLogRoot{
-			KeyHint:          nil, // not used by stfe
-			LogRoot:          rootBytes,
-			LogRootSignature: nil, // not used by stfe
-		},
-		Proof: nil, // not used by stfe
-	}
-}